[PATCH] clk: qcom: sm8450: Enable retention for usb controller gdsc

[PATCH] clk: qcom: sm8450: Enable retention for usb controller gdsc

[Krishna Kurapati]

When USB controller enters runtime suspend while operating in host
mode, then wakeup because of cable disconnect or a button press of
a headset causes the following kind of errors:

Error after button press on a connected headset :

[  355.309260] usb 1-1: reset full-speed USB device number 2 using xhci-hcd
[  355.725844] usb 1-1: device not accepting address 2, error -108

Error on removal of headset device from usb port:

[  157.563136] arm-smmu 15000000.iommu: Unhandled context fault: fsr=0x402
,iova=0xd65504710, fsynr=0x100011, cbfrsynra=0x0, cb=6
[  157.574842] arm-smmu 15000000.iommu: FSR    = 00000402 [Format=2 TF],
SID=0x0
[  157.582181] arm-smmu 15000000.iommu: FSYNR0 = 00100011 [S1CBNDX=16 WNR
PLVL=1]
[  157.589610] xhci-hcd xhci-hcd.0.auto: WARNING: Host Controller Error
[  157.596197] xhci-hcd xhci-hcd.0.auto: WARNING: Host Controller Error

Enabling retention on usb controller GDSC fixes the above issues.

Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
---

Note:
The above mentioned issues pop up after I enabled runtime suspend after
applying [1].
[1]: https://lore.kernel.org/all/20250610091357.2983085-1-krishna.kurapati@oss.qualcomm.com/

 drivers/clk/qcom/gcc-sm8450.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/qcom/gcc-sm8450.c b/drivers/clk/qcom/gcc-sm8450.c
index 65d7d52bce03..f94da4a1c921 100644
--- a/drivers/clk/qcom/gcc-sm8450.c
+++ b/drivers/clk/qcom/gcc-sm8450.c
@@ -3141,7 +3141,7 @@ static struct gdsc usb30_prim_gdsc = {
 	.pd = {
 		.name = "usb30_prim_gdsc",
 	},
-	.pwrsts = PWRSTS_OFF_ON,
+	.pwrsts = PWRSTS_RET_ON,
 };
 
 static struct clk_regmap *gcc_sm8450_clocks[] = {
-- 
2.34.1

Re: [PATCH] clk: qcom: sm8450: Enable retention for usb controller gdsc

[neil.armstrong]

On 10/06/2025 11:22, Krishna Kurapati wrote:
> When USB controller enters runtime suspend while operating in host
> mode, then wakeup because of cable disconnect or a button press of
> a headset causes the following kind of errors:
> 
> Error after button press on a connected headset :
> 
> [  355.309260] usb 1-1: reset full-speed USB device number 2 using xhci-hcd
> [  355.725844] usb 1-1: device not accepting address 2, error -108
> 
> Error on removal of headset device from usb port:
> 
> [  157.563136] arm-smmu 15000000.iommu: Unhandled context fault: fsr=0x402
> ,iova=0xd65504710, fsynr=0x100011, cbfrsynra=0x0, cb=6
> [  157.574842] arm-smmu 15000000.iommu: FSR    = 00000402 [Format=2 TF],
> SID=0x0
> [  157.582181] arm-smmu 15000000.iommu: FSYNR0 = 00100011 [S1CBNDX=16 WNR
> PLVL=1]
> [  157.589610] xhci-hcd xhci-hcd.0.auto: WARNING: Host Controller Error
> [  157.596197] xhci-hcd xhci-hcd.0.auto: WARNING: Host Controller Error
> 
> Enabling retention on usb controller GDSC fixes the above issues.
> 
> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> ---
> 
> Note:
> The above mentioned issues pop up after I enabled runtime suspend after
> applying [1].
> [1]: https://lore.kernel.org/all/20250610091357.2983085-1-krishna.kurapati@oss.qualcomm.com/
> 
>   drivers/clk/qcom/gcc-sm8450.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/qcom/gcc-sm8450.c b/drivers/clk/qcom/gcc-sm8450.c
> index 65d7d52bce03..f94da4a1c921 100644
> --- a/drivers/clk/qcom/gcc-sm8450.c
> +++ b/drivers/clk/qcom/gcc-sm8450.c
> @@ -3141,7 +3141,7 @@ static struct gdsc usb30_prim_gdsc = {
>   	.pd = {
>   		.name = "usb30_prim_gdsc",
>   	},
> -	.pwrsts = PWRSTS_OFF_ON,
> +	.pwrsts = PWRSTS_RET_ON,
>   };
>   
>   static struct clk_regmap *gcc_sm8450_clocks[] = {

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Re: [PATCH] clk: qcom: sm8450: Enable retention for usb controller gdsc

[Dmitry Baryshkov]

On Tue, Jun 10, 2025 at 02:52:53PM +0530, Krishna Kurapati wrote:
> When USB controller enters runtime suspend while operating in host
> mode, then wakeup because of cable disconnect or a button press of
> a headset causes the following kind of errors:
> 
> Error after button press on a connected headset :
> 
> [  355.309260] usb 1-1: reset full-speed USB device number 2 using xhci-hcd
> [  355.725844] usb 1-1: device not accepting address 2, error -108
> 
> Error on removal of headset device from usb port:
> 
> [  157.563136] arm-smmu 15000000.iommu: Unhandled context fault: fsr=0x402
> ,iova=0xd65504710, fsynr=0x100011, cbfrsynra=0x0, cb=6
> [  157.574842] arm-smmu 15000000.iommu: FSR    = 00000402 [Format=2 TF],
> SID=0x0
> [  157.582181] arm-smmu 15000000.iommu: FSYNR0 = 00100011 [S1CBNDX=16 WNR
> PLVL=1]
> [  157.589610] xhci-hcd xhci-hcd.0.auto: WARNING: Host Controller Error
> [  157.596197] xhci-hcd xhci-hcd.0.auto: WARNING: Host Controller Error
> 
> Enabling retention on usb controller GDSC fixes the above issues.

Could you please clarify, if there are other platforms which need this
change? Is there a chance of switching all of them in one patch set?

> 
> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> ---
> 
> Note:
> The above mentioned issues pop up after I enabled runtime suspend after
> applying [1].
> [1]: https://lore.kernel.org/all/20250610091357.2983085-1-krishna.kurapati@oss.qualcomm.com/
> 
>  drivers/clk/qcom/gcc-sm8450.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/qcom/gcc-sm8450.c b/drivers/clk/qcom/gcc-sm8450.c
> index 65d7d52bce03..f94da4a1c921 100644
> --- a/drivers/clk/qcom/gcc-sm8450.c
> +++ b/drivers/clk/qcom/gcc-sm8450.c
> @@ -3141,7 +3141,7 @@ static struct gdsc usb30_prim_gdsc = {
>  	.pd = {
>  		.name = "usb30_prim_gdsc",
>  	},
> -	.pwrsts = PWRSTS_OFF_ON,
> +	.pwrsts = PWRSTS_RET_ON,
>  };
>  
>  static struct clk_regmap *gcc_sm8450_clocks[] = {
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH] clk: qcom: sm8450: Enable retention for usb controller gdsc

[neil.armstrong]

On 10/06/2025 13:10, Dmitry Baryshkov wrote:
> On Tue, Jun 10, 2025 at 02:52:53PM +0530, Krishna Kurapati wrote:
>> When USB controller enters runtime suspend while operating in host
>> mode, then wakeup because of cable disconnect or a button press of
>> a headset causes the following kind of errors:
>>
>> Error after button press on a connected headset :
>>
>> [  355.309260] usb 1-1: reset full-speed USB device number 2 using xhci-hcd
>> [  355.725844] usb 1-1: device not accepting address 2, error -108
>>
>> Error on removal of headset device from usb port:
>>
>> [  157.563136] arm-smmu 15000000.iommu: Unhandled context fault: fsr=0x402
>> ,iova=0xd65504710, fsynr=0x100011, cbfrsynra=0x0, cb=6
>> [  157.574842] arm-smmu 15000000.iommu: FSR    = 00000402 [Format=2 TF],
>> SID=0x0
>> [  157.582181] arm-smmu 15000000.iommu: FSYNR0 = 00100011 [S1CBNDX=16 WNR
>> PLVL=1]
>> [  157.589610] xhci-hcd xhci-hcd.0.auto: WARNING: Host Controller Error
>> [  157.596197] xhci-hcd xhci-hcd.0.auto: WARNING: Host Controller Error
>>
>> Enabling retention on usb controller GDSC fixes the above issues.
> 
> Could you please clarify, if there are other platforms which need this
> change? Is there a chance of switching all of them in one patch set?

Since sm8250, sc8280xp, x1e80100, sm8650 are all affected
- https://lore.kernel.org/r/20221102091320.66007-1-manivannan.sadhasivam@linaro.org
- https://lore.kernel.org/r/20220929161124.18138-1-johan+linaro@kernel.org
- https://lore.kernel.org/r/20240801-x1e80100-clk-gcc-fix-usb-phy-gdscs-pwrsts-v1-1-8df016768a0f@linaro.org
- https://lore.kernel.org/r/20250305-topic-sm8650-upstream-fix-usb-suspend-v1-1-649036ab0557@linaro.org

I think sm8350 & sm8550 also should be switched, perhaps sm8150 also ?

> 
>>
>> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
>> ---
>>
>> Note:
>> The above mentioned issues pop up after I enabled runtime suspend after
>> applying [1].
>> [1]: https://lore.kernel.org/all/20250610091357.2983085-1-krishna.kurapati@oss.qualcomm.com/
>>
>>   drivers/clk/qcom/gcc-sm8450.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/qcom/gcc-sm8450.c b/drivers/clk/qcom/gcc-sm8450.c
>> index 65d7d52bce03..f94da4a1c921 100644
>> --- a/drivers/clk/qcom/gcc-sm8450.c
>> +++ b/drivers/clk/qcom/gcc-sm8450.c
>> @@ -3141,7 +3141,7 @@ static struct gdsc usb30_prim_gdsc = {
>>   	.pd = {
>>   		.name = "usb30_prim_gdsc",
>>   	},
>> -	.pwrsts = PWRSTS_OFF_ON,
>> +	.pwrsts = PWRSTS_RET_ON,
>>   };
>>   
>>   static struct clk_regmap *gcc_sm8450_clocks[] = {
>> -- 
>> 2.34.1
>>
> 

Re: [PATCH] clk: qcom: sm8450: Enable retention for usb controller gdsc

[Bjorn Andersson]

On Tue, Jun 10, 2025 at 02:52:53PM +0530, Krishna Kurapati wrote:
> When USB controller enters runtime suspend while operating in host
> mode, then wakeup because of cable disconnect or a button press of
> a headset causes the following kind of errors:
> 
> Error after button press on a connected headset :
> 
> [  355.309260] usb 1-1: reset full-speed USB device number 2 using xhci-hcd
> [  355.725844] usb 1-1: device not accepting address 2, error -108
> 
> Error on removal of headset device from usb port:
> 
> [  157.563136] arm-smmu 15000000.iommu: Unhandled context fault: fsr=0x402
> ,iova=0xd65504710, fsynr=0x100011, cbfrsynra=0x0, cb=6
> [  157.574842] arm-smmu 15000000.iommu: FSR    = 00000402 [Format=2 TF],
> SID=0x0
> [  157.582181] arm-smmu 15000000.iommu: FSYNR0 = 00100011 [S1CBNDX=16 WNR
> PLVL=1]
> [  157.589610] xhci-hcd xhci-hcd.0.auto: WARNING: Host Controller Error
> [  157.596197] xhci-hcd xhci-hcd.0.auto: WARNING: Host Controller Error
> 
> Enabling retention on usb controller GDSC fixes the above issues.
> 
> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> ---
> 
> Note:
> The above mentioned issues pop up after I enabled runtime suspend after
> applying [1].
> [1]: https://lore.kernel.org/all/20250610091357.2983085-1-krishna.kurapati@oss.qualcomm.com/
> 

It makes sense that the BCR issue appears after the flattening of dwc3,
but why would the suspend issue pop up?

The change matches what we do on many other platforms, but I've been
hoping we either could drop the retention or clearly document why it's
needed - so that we know if this should be on all or none of the SoCs.

Regards,
Bjorn

>  drivers/clk/qcom/gcc-sm8450.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/qcom/gcc-sm8450.c b/drivers/clk/qcom/gcc-sm8450.c
> index 65d7d52bce03..f94da4a1c921 100644
> --- a/drivers/clk/qcom/gcc-sm8450.c
> +++ b/drivers/clk/qcom/gcc-sm8450.c
> @@ -3141,7 +3141,7 @@ static struct gdsc usb30_prim_gdsc = {
>  	.pd = {
>  		.name = "usb30_prim_gdsc",
>  	},
> -	.pwrsts = PWRSTS_OFF_ON,
> +	.pwrsts = PWRSTS_RET_ON,
>  };
>  
>  static struct clk_regmap *gcc_sm8450_clocks[] = {
> -- 
> 2.34.1
> 

Re: [PATCH] clk: qcom: sm8450: Enable retention for usb controller gdsc

[Krishna Kurapati]

On 6/11/2025 9:12 AM, Bjorn Andersson wrote:
> On Tue, Jun 10, 2025 at 02:52:53PM +0530, Krishna Kurapati wrote:
>> When USB controller enters runtime suspend while operating in host
>> mode, then wakeup because of cable disconnect or a button press of
>> a headset causes the following kind of errors:
>>
>> Error after button press on a connected headset :
>>
>> [  355.309260] usb 1-1: reset full-speed USB device number 2 using xhci-hcd
>> [  355.725844] usb 1-1: device not accepting address 2, error -108
>>
>> Error on removal of headset device from usb port:
>>
>> [  157.563136] arm-smmu 15000000.iommu: Unhandled context fault: fsr=0x402
>> ,iova=0xd65504710, fsynr=0x100011, cbfrsynra=0x0, cb=6
>> [  157.574842] arm-smmu 15000000.iommu: FSR    = 00000402 [Format=2 TF],
>> SID=0x0
>> [  157.582181] arm-smmu 15000000.iommu: FSYNR0 = 00100011 [S1CBNDX=16 WNR
>> PLVL=1]
>> [  157.589610] xhci-hcd xhci-hcd.0.auto: WARNING: Host Controller Error
>> [  157.596197] xhci-hcd xhci-hcd.0.auto: WARNING: Host Controller Error
>>
>> Enabling retention on usb controller GDSC fixes the above issues.
>>
>> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
>> ---
>>
>> Note:
>> The above mentioned issues pop up after I enabled runtime suspend after
>> applying [1].
>> [1]: https://lore.kernel.org/all/20250610091357.2983085-1-krishna.kurapati@oss.qualcomm.com/
>>
> 
> It makes sense that the BCR issue appears after the flattening of dwc3,
> but why would the suspend issue pop up?
> 
> The change matches what we do on many other platforms, but I've been
> hoping we either could drop the retention or clearly document why it's
> needed - so that we know if this should be on all or none of the SoCs.
> 

Sure, will try to root cause why we are seeing issues during suspend (on 
sm8450) and come up with another patch.

But I remember on sc7280-herobrine (and IIRC while testing multiport on 
sa8295), when gdsc was not in retention, after entering suspend, we 
would see a disconnect and reconnect of connected peripherals during 
resume (probably because controller was in off state during suspend). 
When I tested with RET_ON in sc7280, I see the re-enumeration after 
resume never popped up.

Regards,
Krishna,

> Regards,
> Bjorn
> 
>>   drivers/clk/qcom/gcc-sm8450.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/qcom/gcc-sm8450.c b/drivers/clk/qcom/gcc-sm8450.c
>> index 65d7d52bce03..f94da4a1c921 100644
>> --- a/drivers/clk/qcom/gcc-sm8450.c
>> +++ b/drivers/clk/qcom/gcc-sm8450.c
>> @@ -3141,7 +3141,7 @@ static struct gdsc usb30_prim_gdsc = {
>>   	.pd = {
>>   		.name = "usb30_prim_gdsc",
>>   	},
>> -	.pwrsts = PWRSTS_OFF_ON,
>> +	.pwrsts = PWRSTS_RET_ON,
>>   };
>>   
>>   static struct clk_regmap *gcc_sm8450_clocks[] = {
>> -- 
>> 2.34.1
>>

Re: [PATCH] clk: qcom: sm8450: Enable retention for usb controller gdsc

[Bjorn Andersson]

On Wed, Jun 11, 2025 at 10:35:36AM +0530, Krishna Kurapati wrote:
> 
> 
> On 6/11/2025 9:12 AM, Bjorn Andersson wrote:
> > On Tue, Jun 10, 2025 at 02:52:53PM +0530, Krishna Kurapati wrote:
> > > When USB controller enters runtime suspend while operating in host
> > > mode, then wakeup because of cable disconnect or a button press of
> > > a headset causes the following kind of errors:
> > > 
> > > Error after button press on a connected headset :
> > > 
> > > [  355.309260] usb 1-1: reset full-speed USB device number 2 using xhci-hcd
> > > [  355.725844] usb 1-1: device not accepting address 2, error -108
> > > 
> > > Error on removal of headset device from usb port:
> > > 
> > > [  157.563136] arm-smmu 15000000.iommu: Unhandled context fault: fsr=0x402
> > > ,iova=0xd65504710, fsynr=0x100011, cbfrsynra=0x0, cb=6
> > > [  157.574842] arm-smmu 15000000.iommu: FSR    = 00000402 [Format=2 TF],
> > > SID=0x0
> > > [  157.582181] arm-smmu 15000000.iommu: FSYNR0 = 00100011 [S1CBNDX=16 WNR
> > > PLVL=1]
> > > [  157.589610] xhci-hcd xhci-hcd.0.auto: WARNING: Host Controller Error
> > > [  157.596197] xhci-hcd xhci-hcd.0.auto: WARNING: Host Controller Error
> > > 
> > > Enabling retention on usb controller GDSC fixes the above issues.
> > > 
> > > Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> > > ---
> > > 
> > > Note:
> > > The above mentioned issues pop up after I enabled runtime suspend after
> > > applying [1].
> > > [1]: https://lore.kernel.org/all/20250610091357.2983085-1-krishna.kurapati@oss.qualcomm.com/
> > > 
> > 
> > It makes sense that the BCR issue appears after the flattening of dwc3,
> > but why would the suspend issue pop up?
> > 
> > The change matches what we do on many other platforms, but I've been
> > hoping we either could drop the retention or clearly document why it's
> > needed - so that we know if this should be on all or none of the SoCs.
> > 
> 
> Sure, will try to root cause why we are seeing issues during suspend (on
> sm8450) and come up with another patch.
> 
> But I remember on sc7280-herobrine (and IIRC while testing multiport on
> sa8295), when gdsc was not in retention, after entering suspend, we would
> see a disconnect and reconnect of connected peripherals during resume
> (probably because controller was in off state during suspend). When I tested
> with RET_ON in sc7280, I see the re-enumeration after resume never popped
> up.
> 

Sounds very reasonable that we'd need to keep the IP block in retention
for such use case. But if that's the case, then it sounds like we need
this for every SoC that supports host mode (I presume host or OTG).

It would also likely imply that we ultimately would want a mechanism to
toggle this behavior from dwc3-qcom to limit it to the times when we're
in host mode, or perhaps even only when something is connected?

If there are any power savings of turning the GDSC off, that is...

Regards,
Bjorn

> Regards,
> Krishna,
> 
> > Regards,
> > Bjorn
> > 
> > >   drivers/clk/qcom/gcc-sm8450.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/clk/qcom/gcc-sm8450.c b/drivers/clk/qcom/gcc-sm8450.c
> > > index 65d7d52bce03..f94da4a1c921 100644
> > > --- a/drivers/clk/qcom/gcc-sm8450.c
> > > +++ b/drivers/clk/qcom/gcc-sm8450.c
> > > @@ -3141,7 +3141,7 @@ static struct gdsc usb30_prim_gdsc = {
> > >   	.pd = {
> > >   		.name = "usb30_prim_gdsc",
> > >   	},
> > > -	.pwrsts = PWRSTS_OFF_ON,
> > > +	.pwrsts = PWRSTS_RET_ON,
> > >   };
> > >   static struct clk_regmap *gcc_sm8450_clocks[] = {
> > > -- 
> > > 2.34.1
> > >