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

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

[Johan Hovold]

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

[Heikki Krogerus]

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

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

[Bjorn Andersson]

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

[Johan Hovold]

[ 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

[Linux regression tracking (Thorsten Leemhuis)]

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

[Johan Hovold]

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

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

[Johan Hovold]

Enabling UCSI on sc8280xp and the Lenovo ThinkPad X13s in particular
results in a number of errors and timeouts during boot:

[    9.012421] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: GET_CONNECTOR_STATUS failed (-95)
[   14.047379] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: timeout waiting for UCSI sync write response
[   14.050708] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: GET_CONNECTOR_STATUS failed (-110)
[   20.192382] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: timeout waiting for UCSI sync write response
[   20.192542] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: GET_CONNECTOR_STATUS failed (-110)

Disable UCSI on sc8280xp until this has been resolved.

Fixes: 4db09e7b967b ("soc: qcom: pmic_glink: enable UCSI by default)
Link: https://lore.kernel.org/r/ZXL5jvDHr-MuxMoz@hovoldconsulting.com
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/soc/qcom/pmic_glink.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/soc/qcom/pmic_glink.c b/drivers/soc/qcom/pmic_glink.c
index 71d8901a9389..511aa40842a1 100644
--- a/drivers/soc/qcom/pmic_glink.c
+++ b/drivers/soc/qcom/pmic_glink.c
@@ -342,6 +342,7 @@ static const unsigned long pmic_glink_sm8450_client_mask = BIT(PMIC_GLINK_CLIENT
 
 static const struct of_device_id pmic_glink_of_match[] = {
 	{ .compatible = "qcom,sc8180x-pmic-glink", .data = &pmic_glink_sc8180x_client_mask },
+	{ .compatible = "qcom,sc8280xp-pmic-glink", .data = &pmic_glink_sc8180x_client_mask },
 	{ .compatible = "qcom,pmic-glink", .data = &pmic_glink_sm8450_client_mask },
 	{}
 };
-- 
2.41.0

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

[Dmitry Baryshkov]

On Fri, 8 Dec 2023 at 14:56, Johan Hovold <johan+linaro@kernel.org> wrote:
>
> Enabling UCSI on sc8280xp and the Lenovo ThinkPad X13s in particular
> results in a number of errors and timeouts during boot:
>
> [    9.012421] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: GET_CONNECTOR_STATUS failed (-95)
> [   14.047379] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: timeout waiting for UCSI sync write response
> [   14.050708] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: GET_CONNECTOR_STATUS failed (-110)
> [   20.192382] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: timeout waiting for UCSI sync write response
> [   20.192542] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: GET_CONNECTOR_STATUS failed (-110)
>
> Disable UCSI on sc8280xp until this has been resolved.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Johan, if you can capture message traces in
pmic_glink_ucsi_locked_write() / pmic_glink_ucsi_read() /
pmic_glink_ucsi_callback(), this would help me understand what is
causing the issue.

>
> Fixes: 4db09e7b967b ("soc: qcom: pmic_glink: enable UCSI by default)
> Link: https://lore.kernel.org/r/ZXL5jvDHr-MuxMoz@hovoldconsulting.com
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  drivers/soc/qcom/pmic_glink.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/soc/qcom/pmic_glink.c b/drivers/soc/qcom/pmic_glink.c
> index 71d8901a9389..511aa40842a1 100644
> --- a/drivers/soc/qcom/pmic_glink.c
> +++ b/drivers/soc/qcom/pmic_glink.c
> @@ -342,6 +342,7 @@ static const unsigned long pmic_glink_sm8450_client_mask = BIT(PMIC_GLINK_CLIENT
>
>  static const struct of_device_id pmic_glink_of_match[] = {
>         { .compatible = "qcom,sc8180x-pmic-glink", .data = &pmic_glink_sc8180x_client_mask },
> +       { .compatible = "qcom,sc8280xp-pmic-glink", .data = &pmic_glink_sc8180x_client_mask },
>         { .compatible = "qcom,pmic-glink", .data = &pmic_glink_sm8450_client_mask },
>         {}
>  };
> --
> 2.41.0
>


-- 
With best wishes
Dmitry

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

[Johan Hovold]

On Fri, Dec 08, 2023 at 06:34:00PM +0200, Dmitry Baryshkov wrote:
> On Fri, 8 Dec 2023 at 14:56, Johan Hovold <johan+linaro@kernel.org> wrote:
> >
> > Enabling UCSI on sc8280xp and the Lenovo ThinkPad X13s in particular
> > results in a number of errors and timeouts during boot:
> >
> > [    9.012421] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: GET_CONNECTOR_STATUS failed (-95)
> > [   14.047379] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: timeout waiting for UCSI sync write response
> > [   14.050708] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: GET_CONNECTOR_STATUS failed (-110)
> > [   20.192382] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: timeout waiting for UCSI sync write response
> > [   20.192542] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: GET_CONNECTOR_STATUS failed (-110)
> >
> > Disable UCSI on sc8280xp until this has been resolved.

> Johan, if you can capture message traces in
> pmic_glink_ucsi_locked_write() / pmic_glink_ucsi_read() /
> pmic_glink_ucsi_callback(), this would help me understand what is
> causing the issue.

I don't have time to dig into this at the moment, and its probably
easiest for everyone if you just try this on your own X13s.

Johan

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

[Bjorn Andersson]

On Fri, 08 Dec 2023 13:57:30 +0100, Johan Hovold wrote:
> Enabling UCSI on sc8280xp and the Lenovo ThinkPad X13s in particular
> results in a number of errors and timeouts during boot:
> 
> [    9.012421] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: GET_CONNECTOR_STATUS failed (-95)
> [   14.047379] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: timeout waiting for UCSI sync write response
> [   14.050708] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: GET_CONNECTOR_STATUS failed (-110)
> [   20.192382] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: timeout waiting for UCSI sync write response
> [   20.192542] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: GET_CONNECTOR_STATUS failed (-110)
> 
> [...]

Applied, thanks!

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

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