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

[Roger Quadros]

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@xxxxxxxxxx>
>> ---
>> Changelog:
>>
>> v3: no change
>>
>> v2: no change
>> 	https://urldefense.com/v3/__https://lore.kernel.org/all/20240205141221.56076-3-rogerq@xxxxxxxxxx/__;!!A4F2R9G_pg!b7_3vmJpNLOFy3g_wExMQSAzi949O4PoID3e4rNEvAsbgCvxxkj0DiSDFJxF7857DZM7qy9DMkH6Q5BPD-jX$ 
>>
>> v1: https://urldefense.com/v3/__https://lore.kernel.org/all/20240201121220.5523-3-rogerq@xxxxxxxxxx/__;!!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
>>

-- 
cheers,
-roger