diff mbox series

[v3,2/5] usb: dwc3-am62: fix error on module removal

Message ID 20240214-for-v6-9-am62-usb-errata-3-0-v3-2-147ec5eae18c@kernel.org
State New
Headers
Series usb: dwc3-am62: module removal and errata fixes |

Commit Message

Roger Quadros Feb. 14, 2024, 9:46 a.m. UTC 
  As runtime PM is enabled, the module can be runtime
suspended when .remove() is called.

Do a pm_runtime_get_sync() to make sure module is active
before doing any register operations.

Doing a pm_runtime_put_sync() should disable the refclk
so no need to disable it again.

Fixes the below warning at module removel.

[   39.705310] ------------[ cut here ]------------
[   39.710004] clk:162:3 already disabled
[   39.713941] WARNING: CPU: 0 PID: 921 at drivers/clk/clk.c:1090 clk_core_disable+0xb0/0xb8

Signed-off-by: Roger Quadros <rogerq@kernel.org>
---
Changelog:

v3: no change

v2: no change
	https://lore.kernel.org/all/20240205141221.56076-3-rogerq@kernel.org/

v1: https://lore.kernel.org/all/20240201121220.5523-3-rogerq@kernel.org/
---
 drivers/usb/dwc3/dwc3-am62.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Thinh Nguyen Feb. 23, 2024, 10:17 p.m. UTC | #1 
On Wed, Feb 14, 2024, Roger Quadros wrote:
> As runtime PM is enabled, the module can be runtime
> suspended when .remove() is called.
> 
> Do a pm_runtime_get_sync() to make sure module is active
> before doing any register operations.
> 
> Doing a pm_runtime_put_sync() should disable the refclk
> so no need to disable it again.
> 
> Fixes the below warning at module removel.
> 
> [   39.705310] ------------[ cut here ]------------
> [   39.710004] clk:162:3 already disabled
> [   39.713941] WARNING: CPU: 0 PID: 921 at drivers/clk/clk.c:1090 clk_core_disable+0xb0/0xb8
> 
> Signed-off-by: Roger Quadros <rogerq@kernel.org>
> ---
> Changelog:
> 
> v3: no change
> 
> v2: no change
> 	https://urldefense.com/v3/__https://lore.kernel.org/all/20240205141221.56076-3-rogerq@kernel.org/__;!!A4F2R9G_pg!b7_3vmJpNLOFy3g_wExMQSAzi949O4PoID3e4rNEvAsbgCvxxkj0DiSDFJxF7857DZM7qy9DMkH6Q5BPD-jX$ 
> 
> v1: https://urldefense.com/v3/__https://lore.kernel.org/all/20240201121220.5523-3-rogerq@kernel.org/__;!!A4F2R9G_pg!b7_3vmJpNLOFy3g_wExMQSAzi949O4PoID3e4rNEvAsbgCvxxkj0DiSDFJxF7857DZM7qy9DMkH6Q1CciylE$ 
> ---
>  drivers/usb/dwc3/dwc3-am62.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-am62.c b/drivers/usb/dwc3/dwc3-am62.c
> index 1bfc9e67614f..600ba9cfefea 100644
> --- a/drivers/usb/dwc3/dwc3-am62.c
> +++ b/drivers/usb/dwc3/dwc3-am62.c
> @@ -273,6 +273,11 @@ static void dwc3_ti_remove(struct platform_device *pdev)
>  	struct dwc3_am62 *am62 = platform_get_drvdata(pdev);
>  	u32 reg;
>  
> +	pm_runtime_get_sync(dev);
> +
> +	device_wakeup_disable(dev);
> +	device_set_wakeup_capable(dev, false);
> +

We have a more convenient function to enable/disable this
(device_init_wakeup()) if you ever want to cleanup the driver later.

>  	of_platform_depopulate(dev);
>  
>  	/* Clear mode valid bit */
> @@ -281,7 +286,6 @@ static void dwc3_ti_remove(struct platform_device *pdev)
>  	dwc3_ti_writel(am62, USBSS_MODE_CONTROL, reg);
>  
>  	pm_runtime_put_sync(dev);
> -	clk_disable_unprepare(am62->usb2_refclk);
>  	pm_runtime_disable(dev);
>  	pm_runtime_set_suspended(dev);
>  }
> 
> -- 
> 2.34.1
> 

Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>

Thanks,
Thinh
Thinh Nguyen Feb. 23, 2024, 10:31 p.m. UTC | #2 
On Wed, Feb 14, 2024, Roger Quadros wrote:
> As runtime PM is enabled, the module can be runtime
> suspended when .remove() is called.
> 
> Do a pm_runtime_get_sync() to make sure module is active
> before doing any register operations.
> 
> Doing a pm_runtime_put_sync() should disable the refclk
> so no need to disable it again.
> 
> Fixes the below warning at module removel.
> 
> [   39.705310] ------------[ cut here ]------------
> [   39.710004] clk:162:3 already disabled
> [   39.713941] WARNING: CPU: 0 PID: 921 at drivers/clk/clk.c:1090 clk_core_disable+0xb0/0xb8
> 

Actually, it will be better to have a fixes tag and separate this from
this series series. You can retain my Acked-by if you resend it with the
Fixes tag and Cc stable.

BR,
Thinh

> Signed-off-by: Roger Quadros <rogerq@kernel.org>
> ---
> Changelog:
> 
> v3: no change
> 
> v2: no change
> 	https://urldefense.com/v3/__https://lore.kernel.org/all/20240205141221.56076-3-rogerq@kernel.org/__;!!A4F2R9G_pg!b7_3vmJpNLOFy3g_wExMQSAzi949O4PoID3e4rNEvAsbgCvxxkj0DiSDFJxF7857DZM7qy9DMkH6Q5BPD-jX$ 
> 
> v1: https://urldefense.com/v3/__https://lore.kernel.org/all/20240201121220.5523-3-rogerq@kernel.org/__;!!A4F2R9G_pg!b7_3vmJpNLOFy3g_wExMQSAzi949O4PoID3e4rNEvAsbgCvxxkj0DiSDFJxF7857DZM7qy9DMkH6Q1CciylE$ 
> ---
>  drivers/usb/dwc3/dwc3-am62.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-am62.c b/drivers/usb/dwc3/dwc3-am62.c
> index 1bfc9e67614f..600ba9cfefea 100644
> --- a/drivers/usb/dwc3/dwc3-am62.c
> +++ b/drivers/usb/dwc3/dwc3-am62.c
> @@ -273,6 +273,11 @@ static void dwc3_ti_remove(struct platform_device *pdev)
>  	struct dwc3_am62 *am62 = platform_get_drvdata(pdev);
>  	u32 reg;
>  
> +	pm_runtime_get_sync(dev);
> +
> +	device_wakeup_disable(dev);
> +	device_set_wakeup_capable(dev, false);
> +
>  	of_platform_depopulate(dev);
>  
>  	/* Clear mode valid bit */
> @@ -281,7 +286,6 @@ static void dwc3_ti_remove(struct platform_device *pdev)
>  	dwc3_ti_writel(am62, USBSS_MODE_CONTROL, reg);
>  
>  	pm_runtime_put_sync(dev);
> -	clk_disable_unprepare(am62->usb2_refclk);
>  	pm_runtime_disable(dev);
>  	pm_runtime_set_suspended(dev);
>  }
> 
> -- 
> 2.34.1
>
Roger Quadros Feb. 26, 2024, 1:19 p.m. UTC | #3 
On 24/02/2024 00:31, Thinh Nguyen wrote:
> On Wed, Feb 14, 2024, Roger Quadros wrote:
>> As runtime PM is enabled, the module can be runtime
>> suspended when .remove() is called.
>>
>> Do a pm_runtime_get_sync() to make sure module is active
>> before doing any register operations.
>>
>> Doing a pm_runtime_put_sync() should disable the refclk
>> so no need to disable it again.
>>
>> Fixes the below warning at module removel.
>>
>> [   39.705310] ------------[ cut here ]------------
>> [   39.710004] clk:162:3 already disabled
>> [   39.713941] WARNING: CPU: 0 PID: 921 at drivers/clk/clk.c:1090 clk_core_disable+0xb0/0xb8
>>
> 
> Actually, it will be better to have a fixes tag and separate this from
> this series series. You can retain my Acked-by if you resend it with the
> Fixes tag and Cc stable.

OK I will resend. please see below.
> 
> BR,
> Thinh
> 
>> Signed-off-by: Roger Quadros <rogerq@kernel.org>
>> ---
>> Changelog:
>>
>> v3: no change
>>
>> v2: no change
>> 	https://urldefense.com/v3/__https://lore.kernel.org/all/20240205141221.56076-3-rogerq@kernel.org/__;!!A4F2R9G_pg!b7_3vmJpNLOFy3g_wExMQSAzi949O4PoID3e4rNEvAsbgCvxxkj0DiSDFJxF7857DZM7qy9DMkH6Q5BPD-jX$ 
>>
>> v1: https://urldefense.com/v3/__https://lore.kernel.org/all/20240201121220.5523-3-rogerq@kernel.org/__;!!A4F2R9G_pg!b7_3vmJpNLOFy3g_wExMQSAzi949O4PoID3e4rNEvAsbgCvxxkj0DiSDFJxF7857DZM7qy9DMkH6Q1CciylE$ 
>> ---
>>  drivers/usb/dwc3/dwc3-am62.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/dwc3/dwc3-am62.c b/drivers/usb/dwc3/dwc3-am62.c
>> index 1bfc9e67614f..600ba9cfefea 100644
>> --- a/drivers/usb/dwc3/dwc3-am62.c
>> +++ b/drivers/usb/dwc3/dwc3-am62.c
>> @@ -273,6 +273,11 @@ static void dwc3_ti_remove(struct platform_device *pdev)
>>  	struct dwc3_am62 *am62 = platform_get_drvdata(pdev);
>>  	u32 reg;
>>  
>> +	pm_runtime_get_sync(dev);
>> +
>> +	device_wakeup_disable(dev);
>> +	device_set_wakeup_capable(dev, false);
>> +

I'll split the wakeup disable changes to a separate patch, so there are less dependencies.

>>  	of_platform_depopulate(dev);
>>  
>>  	/* Clear mode valid bit */
>> @@ -281,7 +286,6 @@ static void dwc3_ti_remove(struct platform_device *pdev)
>>  	dwc3_ti_writel(am62, USBSS_MODE_CONTROL, reg);
>>  
>>  	pm_runtime_put_sync(dev);
>> -	clk_disable_unprepare(am62->usb2_refclk);
>>  	pm_runtime_disable(dev);
>>  	pm_runtime_set_suspended(dev);
>>  }
>>
>> -- 
>> 2.34.1
>>
diff mbox series

Patch

diff --git a/drivers/usb/dwc3/dwc3-am62.c b/drivers/usb/dwc3/dwc3-am62.c
index 1bfc9e67614f..600ba9cfefea 100644
--- a/drivers/usb/dwc3/dwc3-am62.c
+++ b/drivers/usb/dwc3/dwc3-am62.c
@@ -273,6 +273,11 @@  static void dwc3_ti_remove(struct platform_device *pdev)
 	struct dwc3_am62 *am62 = platform_get_drvdata(pdev);
 	u32 reg;
 
+	pm_runtime_get_sync(dev);
+
+	device_wakeup_disable(dev);
+	device_set_wakeup_capable(dev, false);
+
 	of_platform_depopulate(dev);
 
 	/* Clear mode valid bit */
@@ -281,7 +286,6 @@  static void dwc3_ti_remove(struct platform_device *pdev)
 	dwc3_ti_writel(am62, USBSS_MODE_CONTROL, reg);
 
 	pm_runtime_put_sync(dev);
-	clk_disable_unprepare(am62->usb2_refclk);
 	pm_runtime_disable(dev);
 	pm_runtime_set_suspended(dev);
 }