Re: [PATCH v3 2/2] reset: rzg2l-usbphy-ctrl: Add suspend/resume support

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

Hi, Philipp,

On 1/8/26 15:09, Philipp Zabel wrote:
> Hi Claudiu,
> 
> On Do, 2026-01-08 at 13:44 +0200, Claudiu Beznea wrote:
> [...]
>>>> +	ret = reset_control_deassert(priv->rstc);
>>>> +	if (ret)
>>>> +		goto pwrrdy_off;
>>>
>>> Do I understand correctly that this reset clears PHY_RESET_PORT[12]
>>> bits in the RESET register such that rzg2l_usbphy_ctrl_init() must be
>>> called below?
>>
>> No, this reset is the reset of this HW block, controlled by another HW
>> block (the clock controller).
>>
>> Bits in PHY_RESET_PORT and other registers specific to this driver could
>> be cleared due to the fact the power to this USB PHY CTRL HW block is
>> turned off in suspend.
>>
>> The Renesas RZ/G3S SoC, that uses this HW block, has a power saving mode
>> where power to most of the SoC components, including USB PHY CTRL, is
>> turned off.
>>
>> Due to this, we need to restore the previous settings. priv->rstc need
>> to also be restored as power to the clock controller is also lost.
> 
> Ok, thank you for the explanation.
> 
>>>
>>>> +	ret = pm_runtime_resume_and_get(dev);
>>>> +	if (ret)
>>>> +		goto reset_assert;
>>>> +
>>>> +	rzg2l_usbphy_ctrl_init(priv);
>>>
>>> This assumes that consumers requested PHY_RESET_PORT[12] resets to be
>>> asserted in their suspend function.
>>
>> That's right!
>>
>>> I think you should warn if that is
>>> not the case during suspend.
>>
>> AFAICT, that could be done by adding extra logic in this driver to store
>> the state of the de-asserted bits. We can't interrogate directly the
>> registers as there might be the case where these resets are used by
>> previous bootloaders (that might let them in the de-assert state) but
>> not by Linux.
> 
> Isn't the RESET register initialized during rzg2l_usbphy_ctrl_probe()
> before rzg2l_usbphy_ctrl_suspend() can ever be called? It seems to me
> that read-back in suspend should never return a value left behind by a
> previous bootloader.

Ah, you're right, I overlooked that.

> 
> [...]
>> I can add those extra software cache for the hw registers but this is
>> what I've tried to avoid.
>>
>> Please let me know how do you want me to proceed and I'll update.
> 
> If reading back the RESET register during suspend can be used, please
> add a warning.

OK, I'll update it like this.

Thank you,
Claudiu