Re: [PATCH AUTOSEL 5.15 10/19] usb: cdns3: Put the cdns set active part outside the spin lock

[wangxiaolei <xiaolei.wang@windriver.com>]

On 9/11/23 6:00 PM, Pavel Machek wrote:
> Hi!
>
>> From: Xiaolei Wang <xiaolei.wang@windriver.com>
>>
>> [ Upstream commit 2319b9c87fe243327285f2fefd7374ffd75a65fc ]
>>
>> The device may be scheduled during the resume process,
>> so this cannot appear in atomic operations. Since
>> pm_runtime_set_active will resume suppliers, put set
>> active outside the spin lock, which is only used to
>> protect the struct cdns data structure, otherwise the
>> kernel will report the following warning:
> There's something wrong with this patch: cdns_set_active returns
> either void or int depending on config. That can't be intentional.

Thanks for the reminder, I will send a new patch to fix this problem

thanks

xiaolei

>
> Best regards,
> 								Pavel
>
>> +++ b/drivers/usb/cdns3/core.c
>> @@ -556,15 +555,23 @@ int cdns_resume(struct cdns *cdns, u8 set_active)
> ...
>> +
>> +void cdns_set_active(struct cdns *cdns, u8 set_active)
>> +{
>> +	struct device *dev = cdns->dev;
>> +
>>   	if (set_active) {
>>   		pm_runtime_disable(dev);
>>   		pm_runtime_set_active(dev);
>>   		pm_runtime_enable(dev);
>>   	}
>>   
>> -	return 0;
>> +	return;
>>   }
>> +++ b/drivers/usb/cdns3/core.h
>> @@ -125,10 +125,13 @@ int cdns_init(struct cdns *cdns);
>>   int cdns_remove(struct cdns *cdns);
>>   
>>   #ifdef CONFIG_PM_SLEEP
> ...
>>   int cdns_suspend(struct cdns *cdns);
>> +void cdns_set_active(struct cdns *cdns, u8 set_active);
>>   #else /* CONFIG_PM_SLEEP */
> ...
>> +static inline int cdns_set_active(struct cdns *cdns, u8 set_active)
>>   { return 0; }
>>   static inline int cdns_suspend(struct cdns *cdns)
>>   { return 0; }