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

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

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Hi,

Series adds suspend to RAM support to the rzg2l-usbphy-ctrl driver.
Patch 1/2 is a preparatory change to avoid suppressing any
configuration errors.

Thank you,
Claudiu

Changes in v3:
- collected tags

Changes in v2:
- use pm_runtime_put_sync() in patch 2/2
- collected tags

Claudiu Beznea (2):
  reset: rzg2l-usbphy-ctrl: Propagate the return value of
    regmap_field_update_bits()
  reset: rzg2l-usbphy-ctrl: Add suspend/resume support

 drivers/reset/reset-rzg2l-usbphy-ctrl.c | 105 +++++++++++++++++++-----
 1 file changed, 86 insertions(+), 19 deletions(-)

-- 
2.43.0

[PATCH v3 1/2] reset: rzg2l-usbphy-ctrl: Propagate the return value of regmap_field_update_bits()

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Propagate the return value of regmap_field_update_bits() to avoid losing
any possible error. With this, the return type of
rzg2l_usbphy_ctrl_set_pwrrdy() was updated accordingly.

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v3:
- none

Changes in v2:
- collected tags

 drivers/reset/reset-rzg2l-usbphy-ctrl.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/reset/reset-rzg2l-usbphy-ctrl.c b/drivers/reset/reset-rzg2l-usbphy-ctrl.c
index 4ecb9acb2641..9ce0c1f5d465 100644
--- a/drivers/reset/reset-rzg2l-usbphy-ctrl.c
+++ b/drivers/reset/reset-rzg2l-usbphy-ctrl.c
@@ -117,13 +117,13 @@ static const struct regmap_config rzg2l_usb_regconf = {
 	.max_register = 1,
 };
 
-static void rzg2l_usbphy_ctrl_set_pwrrdy(struct regmap_field *pwrrdy,
-					 bool power_on)
+static int rzg2l_usbphy_ctrl_set_pwrrdy(struct regmap_field *pwrrdy,
+					bool power_on)
 {
 	u32 val = power_on ? 0 : 1;
 
 	/* The initialization path guarantees that the mask is 1 bit long. */
-	regmap_field_update_bits(pwrrdy, 1, val);
+	return regmap_field_update_bits(pwrrdy, 1, val);
 }
 
 static void rzg2l_usbphy_ctrl_pwrrdy_off(void *data)
@@ -138,6 +138,7 @@ static int rzg2l_usbphy_ctrl_pwrrdy_init(struct device *dev)
 	struct regmap *regmap;
 	const int *data;
 	u32 args[2];
+	int ret;
 
 	data = device_get_match_data(dev);
 	if ((uintptr_t)data != RZG2L_USBPHY_CTRL_PWRRDY)
@@ -161,7 +162,9 @@ static int rzg2l_usbphy_ctrl_pwrrdy_init(struct device *dev)
 	if (IS_ERR(pwrrdy))
 		return PTR_ERR(pwrrdy);
 
-	rzg2l_usbphy_ctrl_set_pwrrdy(pwrrdy, true);
+	ret = rzg2l_usbphy_ctrl_set_pwrrdy(priv->pwrrdy, true);
+	if (ret)
+		return ret;
 
 	return devm_add_action_or_reset(dev, rzg2l_usbphy_ctrl_pwrrdy_off, pwrrdy);
 }
-- 
2.43.0

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

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

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@bp.renesas.com>
Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

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
@@ -36,6 +36,7 @@ struct rzg2l_usbphy_ctrl_priv {
 	struct reset_control *rstc;
 	void __iomem *base;
 	struct platform_device *vdev;
+	struct regmap_field *pwrrdy;
 
 	spinlock_t lock;
 };
@@ -92,6 +93,19 @@ static int rzg2l_usbphy_ctrl_status(struct reset_controller_dev *rcdev,
 	return !!(readl(priv->base + RESET) & port_mask);
 }
 
+/* put pll and phy into reset state */
+static void rzg2l_usbphy_ctrl_init(struct rzg2l_usbphy_ctrl_priv *priv)
+{
+	unsigned long flags;
+	u32 val;
+
+	spin_lock_irqsave(&priv->lock, flags);
+	val = readl(priv->base + RESET);
+	val |= RESET_SEL_PLLRESET | RESET_PLLRESET | PHY_RESET_PORT2 | PHY_RESET_PORT1;
+	writel(val, priv->base + RESET);
+	spin_unlock_irqrestore(&priv->lock, flags);
+}
+
 #define RZG2L_USBPHY_CTRL_PWRRDY	1
 
 static const struct of_device_id rzg2l_usbphy_ctrl_match_table[] = {
@@ -131,9 +145,9 @@ static void rzg2l_usbphy_ctrl_pwrrdy_off(void *data)
 	rzg2l_usbphy_ctrl_set_pwrrdy(data, false);
 }
 
-static int rzg2l_usbphy_ctrl_pwrrdy_init(struct device *dev)
+static int rzg2l_usbphy_ctrl_pwrrdy_init(struct device *dev,
+					 struct rzg2l_usbphy_ctrl_priv *priv)
 {
-	struct regmap_field *pwrrdy;
 	struct reg_field field;
 	struct regmap *regmap;
 	const int *data;
@@ -158,15 +172,15 @@ static int rzg2l_usbphy_ctrl_pwrrdy_init(struct device *dev)
 	field.lsb = __ffs(args[1]);
 	field.msb = __fls(args[1]);
 
-	pwrrdy = devm_regmap_field_alloc(dev, regmap, field);
-	if (IS_ERR(pwrrdy))
-		return PTR_ERR(pwrrdy);
+	priv->pwrrdy = devm_regmap_field_alloc(dev, regmap, field);
+	if (IS_ERR(priv->pwrrdy))
+		return PTR_ERR(priv->pwrrdy);
 
 	ret = rzg2l_usbphy_ctrl_set_pwrrdy(priv->pwrrdy, true);
 	if (ret)
 		return ret;
 
-	return devm_add_action_or_reset(dev, rzg2l_usbphy_ctrl_pwrrdy_off, pwrrdy);
+	return devm_add_action_or_reset(dev, rzg2l_usbphy_ctrl_pwrrdy_off, priv->pwrrdy);
 }
 
 static int rzg2l_usbphy_ctrl_probe(struct platform_device *pdev)
@@ -175,9 +189,7 @@ static int rzg2l_usbphy_ctrl_probe(struct platform_device *pdev)
 	struct rzg2l_usbphy_ctrl_priv *priv;
 	struct platform_device *vdev;
 	struct regmap *regmap;
-	unsigned long flags;
 	int error;
-	u32 val;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -191,7 +203,7 @@ static int rzg2l_usbphy_ctrl_probe(struct platform_device *pdev)
 	if (IS_ERR(regmap))
 		return PTR_ERR(regmap);
 
-	error = rzg2l_usbphy_ctrl_pwrrdy_init(dev);
+	error = rzg2l_usbphy_ctrl_pwrrdy_init(dev, priv);
 	if (error)
 		return error;
 
@@ -214,12 +226,7 @@ static int rzg2l_usbphy_ctrl_probe(struct platform_device *pdev)
 		goto err_pm_disable_reset_deassert;
 	}
 
-	/* put pll and phy into reset state */
-	spin_lock_irqsave(&priv->lock, flags);
-	val = readl(priv->base + RESET);
-	val |= RESET_SEL_PLLRESET | RESET_PLLRESET | PHY_RESET_PORT2 | PHY_RESET_PORT1;
-	writel(val, priv->base + RESET);
-	spin_unlock_irqrestore(&priv->lock, flags);
+	rzg2l_usbphy_ctrl_init(priv);
 
 	priv->rcdev.ops = &rzg2l_usbphy_ctrl_reset_ops;
 	priv->rcdev.of_reset_n_cells = 1;
@@ -266,10 +273,67 @@ static void rzg2l_usbphy_ctrl_remove(struct platform_device *pdev)
 	reset_control_assert(priv->rstc);
 }
 
+static int rzg2l_usbphy_ctrl_suspend(struct device *dev)
+{
+	struct rzg2l_usbphy_ctrl_priv *priv = dev_get_drvdata(dev);
+	int ret;
+
+	pm_runtime_put_sync(dev);
+
+	ret = reset_control_assert(priv->rstc);
+	if (ret)
+		goto rpm_resume;
+
+	ret = rzg2l_usbphy_ctrl_set_pwrrdy(priv->pwrrdy, false);
+	if (ret)
+		goto reset_deassert;
+
+	return 0;
+
+reset_deassert:
+	reset_control_deassert(priv->rstc);
+rpm_resume:
+	pm_runtime_resume_and_get(dev);
+	return ret;
+}
+
+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;
+
+	ret = pm_runtime_resume_and_get(dev);
+	if (ret)
+		goto reset_assert;
+
+	rzg2l_usbphy_ctrl_init(priv);
+
+	return 0;
+
+reset_assert:
+	reset_control_assert(priv->rstc);
+pwrrdy_off:
+	rzg2l_usbphy_ctrl_set_pwrrdy(priv->pwrrdy, false);
+	return ret;
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(rzg2l_usbphy_ctrl_pm_ops,
+				rzg2l_usbphy_ctrl_suspend,
+				rzg2l_usbphy_ctrl_resume);
+
 static struct platform_driver rzg2l_usbphy_ctrl_driver = {
 	.driver = {
 		.name		= "rzg2l_usbphy_ctrl",
 		.of_match_table	= rzg2l_usbphy_ctrl_match_table,
+		.pm		= pm_ptr(&rzg2l_usbphy_ctrl_pm_ops),
 	},
 	.probe	= rzg2l_usbphy_ctrl_probe,
 	.remove = rzg2l_usbphy_ctrl_remove,
-- 
2.43.0

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

[Philipp Zabel]

On Do, 2026-01-08 at 12:26 +0200, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> 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@bp.renesas.com>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
> 
> 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?

> +	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. I think you should warn if that is
not the case during suspend. Saving the relevant RESET bits during
suspend and restoring them here is probably not useful.

Otherwise,

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp

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@bp.renesas.com>
>>
>> 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@bp.renesas.com>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>> ---
>>
>> 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

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

[Philipp Zabel]

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.

[...]
> 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. Otherwise let me know what I'm missing, and I'll take
this as-is.

regards
Philipp

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

[Claudiu Beznea]

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