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

[Claudiu Beznea]

Hi, Philipp,

On 1/8/26 13:19, Philipp Zabel wrote:
> On Do, 2026-01-08 at 12:26 +0200, Claudiu wrote:
> > From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
> >
> > The RZ/G2L USBPHY control driver is also used on the RZ/G3S SoC.
> > The RZ/G3S SoC supports a power-saving mode in which power to most USB
> > components (including the USBPHY control block) is turned off. Because of
> > this, the USBPHY control block needs to be reconfigured when returning
> > from power-saving mode.
> >
> > Add suspend/resume support to handle runtime suspend/resume of the device,
> > assert/deassert the reset signal, and reinitialize the USBPHY control
> > block.
> >
> > Reviewed-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
> > ---
> >
> > Changes in v3:
> > - collected tags
> >
> > Changes in v2:
> > - used pm_runtime_put_sync() in rzg2l_usbphy_ctrl_suspend()
> >
> >   drivers/reset/reset-rzg2l-usbphy-ctrl.c | 94 +++++++++++++++++++++----
> >   1 file changed, 79 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/reset/reset-rzg2l-usbphy-ctrl.c b/drivers/reset/reset-rzg2l-usbphy-ctrl.c
> > index 9ce0c1f5d465..1a1581643bf3 100644
> > --- a/drivers/reset/reset-rzg2l-usbphy-ctrl.c
> > +++ b/drivers/reset/reset-rzg2l-usbphy-ctrl.c
> [...]
> > @@ -266,10 +273,67 @@ static void rzg2l_usbphy_ctrl_remove(struct platform_device *pdev)
> [...]
> > +static int rzg2l_usbphy_ctrl_resume(struct device *dev)
> > +{
> > +	struct rzg2l_usbphy_ctrl_priv *priv = dev_get_drvdata(dev);
> > +	int ret;
> > +
> > +	ret = rzg2l_usbphy_ctrl_set_pwrrdy(priv->pwrrdy, true);
> > +	if (ret)
> > +		return ret;
> > +
> > +	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.

> > +	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. In that case, w/o extra software cache, we can generate 
false positives by directly interrogating the registers.

My point here was that the users of these resets will have to properly 
configure their own requested resets, otherwise, they are not doing the 
things in the proper way.

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.

> Saving the relevant RESET bits during
> suspend and restoring them here is probably not useful.

That's what I've tried to avoid.

Thank you,
Claudiu