Re: [PATCH v2 0/2] reset: rzg2l-usbphy-ctrl: Add suspend to RAM support

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

On 12/5/25 12:17, Biju Das wrote:
> 
> 
>> -----Original Message-----
>> From: Claudiu Beznea <claudiu.beznea@tuxon.dev>
>> Sent: 05 December 2025 10:00
>> To: Biju Das <biju.das.jz@bp.renesas.com>; p.zabel@pengutronix.de
>> Cc: linux-kernel@vger.kernel.org; linux-renesas-soc@vger.kernel.org; Claudiu Beznea
>> <claudiu.beznea.uj@bp.renesas.com>
>> Subject: Re: [PATCH v2 0/2] reset: rzg2l-usbphy-ctrl: Add suspend to RAM support
>>
>> Hi, Biju,
>>
>> On 12/5/25 10:53, Biju Das wrote:
>>>
>>>
>>> Hi Claudiu,
>>>
>>>> -----Original Message-----
>>>> From: Claudiu Beznea <claudiu.beznea@tuxon.dev>
>>>> Sent: 04 December 2025 18:26
>>>> Subject: Re: [PATCH v2 0/2] reset: rzg2l-usbphy-ctrl: Add suspend to
>>>> RAM support
>>>>
>>>> Hi, Philipp,
>>>>
>>>> Could you please let me know if there's anything I should do for this series?
>>>
>>> If rzg2l_usbphy_ctrl_suspend() fails, What is the probability that it
>>> will suspend again without any issue
>>
>> How can I measure this?
>>
>> The idea with this code was the following: if any instruction of suspend fails, the suspend is
>> aborted, thus code in rzg2l_usbphy_ctrl_suspend() is trying to restore the runtime state of the HW so
>> that no runtime users of it to be affected. This is also how core suspend code is doing, e.g.
>> suspend_devices_and_enter().
> 
> The entire system suspend is aborted.

Isn't this the current general approach across different drivers?

Also, take into account that this code will still be executed for suspend
to idle, where power is not lost.

Also, for general case: if we ignore any failure, just because we may
resume from a power down state (where Linux state is preserved in RAM and
most of the SoC parts are powered off), there are resources that are
reference counted (e.g., clocks, some resets). Ignoring failures in those
cases wouldn't necessary make them work after resume just because the
system resumes from a power down state. The reference counters may not have
the right values for the proper registers to be updated.

> See the case 2 in logs[1] and system keeps draining the power.

Case 2 in the pointed logs seems related to resume, are we talk about
suspend, resume or both?

Also, case 2 points to a resume function that returns error w/o taking it
into account. The resume code proposed here takes into account any errors
on the resume path and put the HW in a power saving state as otherwise it
can't be runtime recovered.

> 
> Again, if system tries to do shut down

I guess, here you are talking about suspend with power cut.

> the same device will fail again in similar way and 
> The system will never enter into suspend state.

From my previous experience with suspend/resume implementations, I can say
restoring the system in failure cases in suspend/resume or not, is up to
the subsystem maintainer. So, I'll let Philipp to decide how he wants to go
with it in this driver.

> 
> [1] https://lore.kernel.org/all/TY3PR01MB11346A7B16CB3267F1A57302B86DBA@TY3PR01MB11346.jpnprd01.prod.outlook.com/
> 
> 
> 
>>
>>> as currently we abort system suspend
>>> and enable clocks/deassert reset which keep draining the power.
>> The code is restoring the clocks and resets to their previous runtime state so that any users of it to
>> not be affected. Later, at runtime, if any power needs to be saved the runtime PM framework will do
>> its job.
> 
> The system is enters into suspend state for saving power. Not to consume because of failure.
> Our SoCs will power down during suspend and device reset will happen during wakeup
> So everything will work as usual if there is device specific failures during suspend.
> See case 3 in logs[1]

They are still supporting suspend to idle, where power is maintained,
right? Shouldn't we cover this case?

Thank you,
Claudiu