On 1/15/24 21:02, Andy Shevchenko wrote:
> On Mon, Jan 15, 2024 at 6:16 PM Thomas Richard
> <thomas.richard@xxxxxxxxxxx> wrote:
>>
>> The goal is to extend the active period of pinctrl.
>> Some devices may need active pinctrl after suspend and/or before resume.
>> So move suspend/resume to suspend_noirq/resume_noirq to have active
>> pinctrl until suspend_noirq (included), and from resume_noirq
>> (included).
>
> ->...callback...()
> (Same comment I have given for the first patch)
fixed
>
> ...
>
>> struct pcs_device *pcs;
>>
>> - pcs = platform_get_drvdata(pdev);
>> + pcs = dev_get_drvdata(dev);
>> if (!pcs)
>> return -EINVAL;
>
> Drop dead code.
> This should be simple one line after your change.
>
> struct pcs_device *pcs = dev_get_drvdata(dev);
>
dead code dropped
> ...
>
>> struct pcs_device *pcs;
>>
>> - pcs = platform_get_drvdata(pdev);
>> + pcs = dev_get_drvdata(dev);
>> if (!pcs)
>> return -EINVAL;
>
> Ditto.
>
> ...
dead code dropped
>
>> +static const struct dev_pm_ops pinctrl_single_pm_ops = {
>> + SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(pinctrl_single_suspend_noirq,
>> + pinctrl_single_resume_noirq)
>> +};
>
> Use proper / modern macro.
fixed, use DEFINE_NOIRQ_DEV_PM_OPS now
>
> ...
>
>> #endif
>
> Why ifdeferry is needed (esp. taking into account pm_ptr() use below)?
We may have an "unused variable" warning for pinctrl_single_pm_ops if
CONFIG_PM is undefined (due to pm_ptr).
--
Thomas Richard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
