Re: [PATCH 2/4] usb: host: ehci-platform: Call reset assert/deassert on suspend/resume

[Claudiu Beznea <claudiu.beznea@tuxon.dev>]

Hi, Geert,

On 11/7/25 10:01, Geert Uytterhoeven wrote:
> Hi Claudiu,
> 
> On Thu, 6 Nov 2025 at 19:56, Claudiu Beznea <claudiu.beznea@tuxon.dev> wrote:
>> On 11/6/25 16:52, Geert Uytterhoeven wrote:
>>> On Thu, 6 Nov 2025 at 15:36, Claudiu <claudiu.beznea@tuxon.dev> wrote:
>>>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>>>
>>>> The Renesas RZ/G3S SoC supports a power-saving mode in which power to most
>>>> of the SoC components is turned off, including the USB blocks. On the
>>>> resume path, the reset signal must be de-asserted before applying any
>>>> settings to the USB registers. To handle this properly, call
>>>> reset_control_assert() and reset_control_deassert() during suspend and
>>>> resume, respectively.
>>>>
>>>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>>
>>>> --- a/drivers/usb/host/ehci-platform.c
>>>> +++ b/drivers/usb/host/ehci-platform.c
>>>> @@ -454,6 +454,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);
>>>
>>> I have my doubts about the effectiveness of this "reverse error
>>> handling".  If the reset_control_assert() failed, what are the chances
>>> that the device will actually work after trying to bring it up again?
>>>
>>> Same comment for next patch.
>>
>> I wasn't sure if I should do this revert or not. In my mind, if the reset
>> assert fails, the reset signal is still de-asserted.
> 
> Possibly.  Most reset implementations either cannot fail, or can
> fail due to a timeout.  What state the device is in in case of the latter is
> hard to guess...

In theory there are also failures returned by the subsystem code (e.g. if
reset is shared and its reference counts don't have the proper values, if
not shared and ops->assert is missing).

In case of this particular driver and the ochi-platform one, as the resets
request is done with devm_reset_control_array_get_optional_shared() the
priv->resets is an array and the assert/de-assert is done through
reset_control_array_assert()/reset_control_array_deassert() which, in case
of failures, reverts the assert/de-assert operations. It is true that the
effectiveness of the revert operation is unknown and depends on the HW, but
the subsystem ensures it reverts the previous state in case of failure.

For the case resets is not an array, it is true, it depends on the reset
driver implementation and hardware.

Could you please let me know how would you suggest going forward with the
implementation for the patches in this series?

Thank you,
Claudiu