Re: [cip-dev] [PATCH 6.12.y-cip 00/17] Backport RZ/G3S USB support

Hi, all,

On 6/17/26 09:03, Biju Das wrote:
> Hi,
> 
>> -----Original Message-----
>> From: [email protected] <[email protected]> On Behalf Of Nobuhiro Iwamatsu via
>> lists.cip-project.org
>> Sent: 16 June 2026 00:51
>> Subject: Re: [cip-dev] [PATCH 6.12.y-cip 00/17] Backport RZ/G3S USB support
>>
>> Hi all,
>>
>>> -----Original Message-----
>>> From: [email protected] <[email protected]> On
>>> Behalf Of Pavel Machek via lists.cip-project.org
>>> Sent: Tuesday, June 16, 2026 6:03 AM
>>> To: Claudiu Beznea <[email protected]>
>>> Cc: [email protected]; iwamatsu nobuhiro(岩松 信洋 □ＤＩＴＣ○ＣＰＴ)
>>> <[email protected]>;
>>> [email protected]
>>> Subject: Re: [cip-dev] [PATCH 6.12.y-cip 00/17] Backport RZ/G3S USB
>>> support
>>>
>>> On Mon 2026-06-15 12:05:03, Claudiu Beznea wrote:
>>>> Hi,
>>>>
>>>> On 6/12/26 00:04, Pavel Machek via lists.cip-project.org wrote:
>>>>> Hi!
>>>>>
>>>>>> Thanks for your patch.
>>>>>> I reviewed this and other series, these seems to be fine.
>>>>>> I can apply this if tests are OK and there are no other comments.
>>>>>>     Reviewed-by: Nobuhiro Iwamatsu
>>>>>> <[email protected]>
>>>>>
>>>>> I'm looking into it. As this modifies generic code, I suggest we
>>>>> are cautious here.
>>>> Please let me know if you want me to limit that code to RZ/G3S SoC only.
>>>
>>> I'm still reviewing the series, sorry for delay.
>>>
>>> My first preffered option would be explanation why this is okay on
>>> 5.10: (Will it change behaviour on non-renesas boards? How sure we are
>>> the behaviour change will not break something, or slow things down
>>> significantly?)
>>>
>>> +++ b/drivers/usb/host/ehci-platform.c
>>> @@ -442,6 +442,17 @@ static int __maybe_unused ehci_platform_suspend(struct device *dev)
>>>          if (pdata->power_suspend)
>>>                  pdata->power_suspend(pdev);
>>>
>>> +       ret = reset_control_assert(priv->rsts);
>>> +       if (ret) {
>>> +               if (pdata->power_on)
>>> +                       pdata->power_on(pdev);
>>> +
>>> +               ehci_resume(hcd, false);
>>> +
>>> +               if (priv->quirk_poll)
>>> +                       quirk_poll_init(priv);
>>> +       }
>>> +
>>>
>>> If we can't have the explanation, limiting to renesas boards would be fine next option.
>>
>> This concern was also discussed when the original patch was posted.
>>     https://lkml.org/lkml/2025/11/7/557
>>
>> If we are only addressing specific SoCs, I think we need to fix it in Upstream first.
> 
> Some discussion related to this thread, and it expects
> 
> https://lore.kernel.org/all/[email protected]/

The patches proposed in this series tried to drop the restore code in case the 
reset_control_assert() fails. This is the code that intended to be dropped:

+       if (ret) {
+               if (pdata->power_on)
+                       pdata->power_on(pdev);
+
+               ehci_resume(hcd, false);
+
+               if (priv->quirk_poll)
+                       quirk_poll_init(priv);
+       }

The assumption that the core suspend/resume code handles it. The assumption was 
wrong. Quoting the Alan Stern (the ohci/ehci drivers maintainer):

"When recovering from a failed suspend transition, the kernel resumes
only the devices that did suspend correctly.  The code that handles this
is in drivers/base/power/main.c, and it's rather intricate and difficult
to follow.

In short, dpm_resume() invokes the resume callback only for devices that
are on dpm_suspended_list.  Devices are added to this list by
dpm_suspend(), and if the suspend callback fails they are supposed to be
removed from the list.

However, I don't see any place in device_suspend() where an error causes
the device to be moved from dpm_suspended_list back to
dpm_prepared_list.  Maybe this is a bug and needs to be fixed.  Or maybe
I have misunderstood something."

And Biju agreed with it in the next thread: 
https://lore.kernel.org/all/TY3PR01MB11346364790DE1F0D2441536786A5A@TY3PR01MB11346.jpnprd01.prod.outlook.com/


> 
> and a patch for fixing assert failure in error path.
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/clk/renesas/rzg2l-cpg.c?h=next-20260616&id=0b0201f259e1158a875c5fd01adf318ae5d32352

This patch ensure the reset control driver leaves the reset line in the 
de-asserted state in case the assert for it fails. It doesn't mean with we can 
drop the recovery code:

+       if (ret) {
+               if (pdata->power_on)
+                       pdata->power_on(pdev);
+
+               ehci_resume(hcd, false);
+
+               if (priv->quirk_poll)
+                       quirk_poll_init(priv);
+       }

just, it ensure the reset controller driver leaves the reset line in the proper 
state in case an assert fails.

That patch is not yet backported to any of the 5.10, 6.1, 6.12. If you prefer, I 
can add it to this series (and the corresponding 6.1, 5.10 ones).

Thank you,
Claudiu