[v1] soc: qcom: pmic_glink: disable UCSI on sc8280xp

[PATCH] soc: qcom: pmic_glink: disable UCSI on sc8280xp

Posted by Johan Hovold 1 year, 6 months ago

Disconnecting an external display triggers a hypervisor reset on the
Lenovo ThinkPad X13s since 6.10-rc1 which enabled UCSI. Disable it again
until the regression has been fixed properly.

Fixes: 3f91a0bf4a0b ("soc: qcom: pmic_glink: reenable UCSI on sc8280xp")
Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/soc/qcom/pmic_glink.c | 4 ++++
 1 file changed, 4 insertions(+)


Dmitry reported off-list that his X13s was crashing when he disconnected
an external display and I tracked it down to the enablement of UCSI in
6.10-rc1.

I have not had time to debug this further, but I'm flagging this as a
regression and sending an effective so that we have a fall back in case
this is not fixed in timely manner.

Note that this likely affects other Qualcomm SoCs as well.

Johan


#regzbot introduced: 3f91a0bf4a0b



diff --git a/drivers/soc/qcom/pmic_glink.c b/drivers/soc/qcom/pmic_glink.c
index 40fb09d69014..65279243072c 100644
--- a/drivers/soc/qcom/pmic_glink.c
+++ b/drivers/soc/qcom/pmic_glink.c
@@ -348,11 +348,15 @@ static void pmic_glink_remove(struct platform_device *pdev)
 	mutex_unlock(&__pmic_glink_lock);
 }
 
+static const unsigned long pmic_glink_sc8280xp_client_mask = BIT(PMIC_GLINK_CLIENT_BATT) |
+							     BIT(PMIC_GLINK_CLIENT_ALTMODE);
+
 static const unsigned long pmic_glink_sm8450_client_mask = BIT(PMIC_GLINK_CLIENT_BATT) |
 							   BIT(PMIC_GLINK_CLIENT_ALTMODE) |
 							   BIT(PMIC_GLINK_CLIENT_UCSI);
 
 static const struct of_device_id pmic_glink_of_match[] = {
+	{ .compatible = "qcom,sc8280xp-pmic-glink", .data = &pmic_glink_sc8280xp_client_mask },
 	{ .compatible = "qcom,pmic-glink", .data = &pmic_glink_sm8450_client_mask },
 	{}
 };
-- 
2.44.1

Re: [PATCH] soc: qcom: pmic_glink: disable UCSI on sc8280xp

Posted by Johan Hovold 1 year, 6 months ago

[ Trimming CC list ]

Hi Thorsten,

On Sat, Jun 08, 2024 at 01:45:29PM +0200, Johan Hovold wrote:
> Disconnecting an external display triggers a hypervisor reset on the
> Lenovo ThinkPad X13s since 6.10-rc1 which enabled UCSI. Disable it again
> until the regression has been fixed properly.
> 
> Fixes: 3f91a0bf4a0b ("soc: qcom: pmic_glink: reenable UCSI on sc8280xp")
> Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  drivers/soc/qcom/pmic_glink.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> 
> Dmitry reported off-list that his X13s was crashing when he disconnected
> an external display and I tracked it down to the enablement of UCSI in
> 6.10-rc1.
> 
> I have not had time to debug this further, but I'm flagging this as a
> regression and sending an effective so that we have a fall back in case
> this is not fixed in timely manner.
> 
> Note that this likely affects other Qualcomm SoCs as well.
> 
> Johan
> 
> 
> #regzbot introduced: 3f91a0bf4a0b

I noticed that this one did not make into the regression tracker. This
may be related to the fact that this is the second time I'm disabling
UCSI on sc8280xp and apparently I used the same patch Subject last time
so they end up in the same thread on lore:

	https://lore.kernel.org/lkml/20240608114529.23060-1-johan+linaro@kernel.org/

Bjorn has picked up this effective revert for 6.10 now, but I still
figured I'd mentioned this to you in case you want to add it to the
tracker and/or see what can be done to your scripts to avoid having
regressions like this fall through the cracks in the future.

Johan

[ Trying again: ]

#regzbot introduced: 3f91a0bf4a0b

Re: [PATCH] soc: qcom: pmic_glink: disable UCSI on sc8280xp

Posted by Linux regression tracking (Thorsten Leemhuis) 1 year, 6 months ago

On 17.06.24 09:56, Johan Hovold wrote:
> [ Trimming CC list ]
> On Sat, Jun 08, 2024 at 01:45:29PM +0200, Johan Hovold wrote:
>> Disconnecting an external display triggers a hypervisor reset on the
>> Lenovo ThinkPad X13s since 6.10-rc1 which enabled UCSI. Disable it again
>> until the regression has been fixed properly.
>
> I noticed

First off: many thx for telling me about the problem!

> that this one did not make into the regression tracker.

FWIW, it did:

https://linux-regtracking.leemhuis.info/regzbot/regression/lore/20240608114529.23060-1-johan+linaro@kernel.org/

> This
> may be related to the fact that this is the second time I'm disabling
> UCSI on sc8280xp and apparently I used the same patch Subject last time
> so they end up in the same thread on lore:
> 	https://lore.kernel.org/lkml/20240608114529.23060-1-johan+linaro@kernel.org/

Kinda. The thread is not the problem. I told regzbot about the subject
for the fix a few days ago and it then dutifully looked up if a commit
with that subject ended up in next or mainline already. Which normally
is the right thing to do -- but in this case it went sideways for
obvious reasons. :-/ Not sure yet how to handle that better; maybe that
falls into the ugly category called "there are bigger fish to fry,
ignore this corner case for now".

Again, thx for telling me about this!

Ciao, Thorsten

Re: [PATCH] soc: qcom: pmic_glink: disable UCSI on sc8280xp

Posted by Johan Hovold 1 year, 6 months ago

On Mon, Jun 17, 2024 at 10:49:40AM +0200, Linux regression tracking (Thorsten Leemhuis) wrote:
> On 17.06.24 09:56, Johan Hovold wrote:
> > On Sat, Jun 08, 2024 at 01:45:29PM +0200, Johan Hovold wrote:

> > I noticed

> > that this one did not make into the regression tracker.
> 
> FWIW, it did:
> 
> https://linux-regtracking.leemhuis.info/regzbot/regression/lore/20240608114529.23060-1-johan+linaro@kernel.org/
> 
> > This
> > may be related to the fact that this is the second time I'm disabling
> > UCSI on sc8280xp and apparently I used the same patch Subject last time
> > so they end up in the same thread on lore:
> > 	https://lore.kernel.org/lkml/20240608114529.23060-1-johan+linaro@kernel.org/
> 
> Kinda. The thread is not the problem. I told regzbot about the subject
> for the fix a few days ago and it then dutifully looked up if a commit
> with that subject ended up in next or mainline already. Which normally
> is the right thing to do -- but in this case it went sideways for
> obvious reasons. :-/ Not sure yet how to handle that better; maybe that
> falls into the ugly category called "there are bigger fish to fry,
> ignore this corner case for now".

Fair enough. At least it's a known issue then.

Johan

Re: [PATCH] soc: qcom: pmic_glink: disable UCSI on sc8280xp

Posted by Bjorn Andersson 1 year, 6 months ago

On Sat, 08 Jun 2024 13:45:29 +0200, Johan Hovold wrote:
> Disconnecting an external display triggers a hypervisor reset on the
> Lenovo ThinkPad X13s since 6.10-rc1 which enabled UCSI. Disable it again
> until the regression has been fixed properly.
> 
> 

Applied, thanks!

[1/1] soc: qcom: pmic_glink: disable UCSI on sc8280xp
      commit: ad3dd9592b2a1d2f9e6ffeedfd81602f91f1ba09

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>

Re: [PATCH] soc: qcom: pmic_glink: disable UCSI on sc8280xp

Posted by Heikki Krogerus 1 year, 6 months ago

On Sat, Jun 08, 2024 at 01:45:29PM +0200, Johan Hovold wrote:
> Disconnecting an external display triggers a hypervisor reset on the
> Lenovo ThinkPad X13s since 6.10-rc1 which enabled UCSI. Disable it again
> until the regression has been fixed properly.
> 
> Fixes: 3f91a0bf4a0b ("soc: qcom: pmic_glink: reenable UCSI on sc8280xp")
> Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
>  drivers/soc/qcom/pmic_glink.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> 
> Dmitry reported off-list that his X13s was crashing when he disconnected
> an external display and I tracked it down to the enablement of UCSI in
> 6.10-rc1.
> 
> I have not had time to debug this further, but I'm flagging this as a
> regression and sending an effective so that we have a fall back in case
> this is not fixed in timely manner.
> 
> Note that this likely affects other Qualcomm SoCs as well.
> 
> Johan
> 
> 
> #regzbot introduced: 3f91a0bf4a0b
> 
> 
> 
> diff --git a/drivers/soc/qcom/pmic_glink.c b/drivers/soc/qcom/pmic_glink.c
> index 40fb09d69014..65279243072c 100644
> --- a/drivers/soc/qcom/pmic_glink.c
> +++ b/drivers/soc/qcom/pmic_glink.c
> @@ -348,11 +348,15 @@ static void pmic_glink_remove(struct platform_device *pdev)
>  	mutex_unlock(&__pmic_glink_lock);
>  }
>  
> +static const unsigned long pmic_glink_sc8280xp_client_mask = BIT(PMIC_GLINK_CLIENT_BATT) |
> +							     BIT(PMIC_GLINK_CLIENT_ALTMODE);
> +
>  static const unsigned long pmic_glink_sm8450_client_mask = BIT(PMIC_GLINK_CLIENT_BATT) |
>  							   BIT(PMIC_GLINK_CLIENT_ALTMODE) |
>  							   BIT(PMIC_GLINK_CLIENT_UCSI);
>  
>  static const struct of_device_id pmic_glink_of_match[] = {
> +	{ .compatible = "qcom,sc8280xp-pmic-glink", .data = &pmic_glink_sc8280xp_client_mask },
>  	{ .compatible = "qcom,pmic-glink", .data = &pmic_glink_sm8450_client_mask },
>  	{}
>  };
> -- 
> 2.44.1

-- 
heikki