Replace raw MMIO accesses (void __iomem *, readl/writel) with
regmap_read/regmap_write via devm_regmap_init_mmio(). Regmap
provides its own internal locking, so the manual spinlock and
scoped_guard() wrappers are no longer needed.
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
v8->v9:
- New patch
drivers/reset/Kconfig | 1 +
drivers/reset/reset-rzv2h-usb2phy.c | 42 ++++++++++++++++-------------
2 files changed, 24 insertions(+), 19 deletions(-)
diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 5165006be693..c539ca88518f 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -257,6 +257,7 @@ config RESET_RZG2L_USBPHY_CTRL
config RESET_RZV2H_USB2PHY
tristate "Renesas RZ/V2H(P) (and similar SoCs) USB2PHY Reset driver"
depends on ARCH_RENESAS || COMPILE_TEST
+ select REGMAP_MMIO
help
Support for USB2PHY Port reset Control found on the RZ/V2H(P) SoC
(and similar SoCs).
diff --git a/drivers/reset/reset-rzv2h-usb2phy.c b/drivers/reset/reset-rzv2h-usb2phy.c
index 5bdd39274612..4014eff0f017 100644
--- a/drivers/reset/reset-rzv2h-usb2phy.c
+++ b/drivers/reset/reset-rzv2h-usb2phy.c
@@ -5,13 +5,13 @@
* Copyright (C) 2025 Renesas Electronics Corporation
*/
-#include <linux/cleanup.h>
#include <linux/delay.h>
#include <linux/io.h>
#include <linux/module.h>
#include <linux/of.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
#include <linux/reset.h>
#include <linux/reset-controller.h>
@@ -37,10 +37,9 @@ struct rzv2h_usb2phy_reset_of_data {
struct rzv2h_usb2phy_reset_priv {
const struct rzv2h_usb2phy_reset_of_data *data;
- void __iomem *base;
+ struct regmap *regmap;
struct device *dev;
struct reset_controller_dev rcdev;
- spinlock_t lock; /* protects register accesses */
};
static inline struct rzv2h_usb2phy_reset_priv
@@ -55,10 +54,8 @@ static int rzv2h_usbphy_reset_assert(struct reset_controller_dev *rcdev,
struct rzv2h_usb2phy_reset_priv *priv = rzv2h_usbphy_rcdev_to_priv(rcdev);
const struct rzv2h_usb2phy_reset_of_data *data = priv->data;
- scoped_guard(spinlock, &priv->lock) {
- writel(data->reset2_acquire_val, priv->base + data->reset2_reg);
- writel(data->reset_assert_val, priv->base + data->reset_reg);
- }
+ regmap_write(priv->regmap, data->reset2_reg, data->reset2_acquire_val);
+ regmap_write(priv->regmap, data->reset_reg, data->reset_assert_val);
usleep_range(11, 20);
@@ -71,11 +68,9 @@ static int rzv2h_usbphy_reset_deassert(struct reset_controller_dev *rcdev,
struct rzv2h_usb2phy_reset_priv *priv = rzv2h_usbphy_rcdev_to_priv(rcdev);
const struct rzv2h_usb2phy_reset_of_data *data = priv->data;
- scoped_guard(spinlock, &priv->lock) {
- writel(data->reset_deassert_val, priv->base + data->reset_reg);
- writel(data->reset2_release_val, priv->base + data->reset2_reg);
- writel(data->reset_release_val, priv->base + data->reset_reg);
- }
+ regmap_write(priv->regmap, data->reset_reg, data->reset_deassert_val);
+ regmap_write(priv->regmap, data->reset2_reg, data->reset2_release_val);
+ regmap_write(priv->regmap, data->reset_reg, data->reset_release_val);
return 0;
}
@@ -86,7 +81,7 @@ static int rzv2h_usbphy_reset_status(struct reset_controller_dev *rcdev,
struct rzv2h_usb2phy_reset_priv *priv = rzv2h_usbphy_rcdev_to_priv(rcdev);
u32 reg;
- reg = readl(priv->base + priv->data->reset_reg);
+ regmap_read(priv->regmap, priv->data->reset_reg, ®);
return (reg & priv->data->reset_status_bits) == priv->data->reset_status_bits;
}
@@ -104,6 +99,12 @@ static int rzv2h_usb2phy_reset_of_xlate(struct reset_controller_dev *rcdev,
return 0;
}
+static const struct regmap_config rzv2h_usb2phy_reset_regconf = {
+ .reg_bits = 32,
+ .val_bits = 32,
+ .reg_stride = 4,
+};
+
static void rzv2h_usb2phy_reset_pm_runtime_put(void *data)
{
pm_runtime_put(data);
@@ -115,6 +116,7 @@ static int rzv2h_usb2phy_reset_probe(struct platform_device *pdev)
struct rzv2h_usb2phy_reset_priv *priv;
struct device *dev = &pdev->dev;
struct reset_control *rstc;
+ void __iomem *base;
int error;
priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
@@ -124,17 +126,19 @@ static int rzv2h_usb2phy_reset_probe(struct platform_device *pdev)
data = of_device_get_match_data(dev);
priv->data = data;
priv->dev = dev;
- priv->base = devm_platform_ioremap_resource(pdev, 0);
- if (IS_ERR(priv->base))
- return PTR_ERR(priv->base);
+ base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(base))
+ return PTR_ERR(base);
+
+ priv->regmap = devm_regmap_init_mmio(dev, base, &rzv2h_usb2phy_reset_regconf);
+ if (IS_ERR(priv->regmap))
+ return PTR_ERR(priv->regmap);
rstc = devm_reset_control_get_shared_deasserted(dev, NULL);
if (IS_ERR(rstc))
return dev_err_probe(dev, PTR_ERR(rstc),
"failed to get deasserted reset\n");
- spin_lock_init(&priv->lock);
-
error = devm_pm_runtime_enable(dev);
if (error)
return dev_err_probe(dev, error, "Failed to enable pm_runtime\n");
@@ -149,7 +153,7 @@ static int rzv2h_usb2phy_reset_probe(struct platform_device *pdev)
return dev_err_probe(dev, error, "unable to register cleanup action\n");
for (unsigned int i = 0; i < data->init_val_count; i++)
- writel(data->init_vals[i].val, priv->base + data->init_vals[i].reg);
+ regmap_write(priv->regmap, data->init_vals[i].reg, data->init_vals[i].val);
priv->rcdev.ops = &rzv2h_usbphy_reset_ops;
priv->rcdev.of_reset_n_cells = 0;
--
2.43.0On Fr, 2026-03-27 at 19:08 +0100, Tommaso Merciai wrote:
> Replace raw MMIO accesses (void __iomem *, readl/writel) with
> regmap_read/regmap_write via devm_regmap_init_mmio(). Regmap
> provides its own internal locking, so the manual spinlock and
> scoped_guard() wrappers are no longer needed.
>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> ---
> v8->v9:
> - New patch
>
> drivers/reset/Kconfig | 1 +
> drivers/reset/reset-rzv2h-usb2phy.c | 42 ++++++++++++++++-------------
> 2 files changed, 24 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 5165006be693..c539ca88518f 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -257,6 +257,7 @@ config RESET_RZG2L_USBPHY_CTRL
> config RESET_RZV2H_USB2PHY
> tristate "Renesas RZ/V2H(P) (and similar SoCs) USB2PHY Reset driver"
> depends on ARCH_RENESAS || COMPILE_TEST
> + select REGMAP_MMIO
> help
> Support for USB2PHY Port reset Control found on the RZ/V2H(P) SoC
> (and similar SoCs).
> diff --git a/drivers/reset/reset-rzv2h-usb2phy.c b/drivers/reset/reset-rzv2h-usb2phy.c
> index 5bdd39274612..4014eff0f017 100644
> --- a/drivers/reset/reset-rzv2h-usb2phy.c
> +++ b/drivers/reset/reset-rzv2h-usb2phy.c
> @@ -5,13 +5,13 @@
> * Copyright (C) 2025 Renesas Electronics Corporation
> */
>
> -#include <linux/cleanup.h>
> #include <linux/delay.h>
> #include <linux/io.h>
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/platform_device.h>
> #include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> #include <linux/reset.h>
> #include <linux/reset-controller.h>
>
> @@ -37,10 +37,9 @@ struct rzv2h_usb2phy_reset_of_data {
>
> struct rzv2h_usb2phy_reset_priv {
> const struct rzv2h_usb2phy_reset_of_data *data;
> - void __iomem *base;
> + struct regmap *regmap;
> struct device *dev;
> struct reset_controller_dev rcdev;
> - spinlock_t lock; /* protects register accesses */
> };
>
> static inline struct rzv2h_usb2phy_reset_priv
> @@ -55,10 +54,8 @@ static int rzv2h_usbphy_reset_assert(struct reset_controller_dev *rcdev,
> struct rzv2h_usb2phy_reset_priv *priv = rzv2h_usbphy_rcdev_to_priv(rcdev);
> const struct rzv2h_usb2phy_reset_of_data *data = priv->data;
>
> - scoped_guard(spinlock, &priv->lock) {
> - writel(data->reset2_acquire_val, priv->base + data->reset2_reg);
> - writel(data->reset_assert_val, priv->base + data->reset_reg);
> - }
> + regmap_write(priv->regmap, data->reset2_reg, data->reset2_acquire_val);
> + regmap_write(priv->regmap, data->reset_reg, data->reset_assert_val);
What is the spinlock protecting? acquire/assert registers being set
together, without another acquire/assert or deassert/release register
access pair interleaving?
In that case you still need the lock. Or use regmap_multi_reg_write().
You could even directly store the sequences as struct reg_sequence in
rzv2h_usb2phy_reset_of_data.
> usleep_range(11, 20);
>
> @@ -71,11 +68,9 @@ static int rzv2h_usbphy_reset_deassert(struct reset_controller_dev *rcdev,
> struct rzv2h_usb2phy_reset_priv *priv = rzv2h_usbphy_rcdev_to_priv(rcdev);
> const struct rzv2h_usb2phy_reset_of_data *data = priv->data;
>
> - scoped_guard(spinlock, &priv->lock) {
> - writel(data->reset_deassert_val, priv->base + data->reset_reg);
> - writel(data->reset2_release_val, priv->base + data->reset2_reg);
> - writel(data->reset_release_val, priv->base + data->reset_reg);
> - }
> + regmap_write(priv->regmap, data->reset_reg, data->reset_deassert_val);
> + regmap_write(priv->regmap, data->reset2_reg, data->reset2_release_val);
> + regmap_write(priv->regmap, data->reset_reg, data->reset_release_val);
Same as above.
[...]
> @@ -149,7 +153,7 @@ static int rzv2h_usb2phy_reset_probe(struct platform_device *pdev)
> return dev_err_probe(dev, error, "unable to register cleanup action\n");
>
> for (unsigned int i = 0; i < data->init_val_count; i++)
> - writel(data->init_vals[i].val, priv->base + data->init_vals[i].reg);
> + regmap_write(priv->regmap, data->init_vals[i].reg, data->init_vals[i].val);
Not required for locking, but this could use regmap_multi_reg_write()
as well.
regards
Philipp
Hi Philipp,
Thanks for your review.
On Tue, Mar 31, 2026 at 06:36:45PM +0200, Philipp Zabel wrote:
> On Fr, 2026-03-27 at 19:08 +0100, Tommaso Merciai wrote:
> > Replace raw MMIO accesses (void __iomem *, readl/writel) with
> > regmap_read/regmap_write via devm_regmap_init_mmio(). Regmap
> > provides its own internal locking, so the manual spinlock and
> > scoped_guard() wrappers are no longer needed.
> >
> > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > ---
> > v8->v9:
> > - New patch
> >
> > drivers/reset/Kconfig | 1 +
> > drivers/reset/reset-rzv2h-usb2phy.c | 42 ++++++++++++++++-------------
> > 2 files changed, 24 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> > index 5165006be693..c539ca88518f 100644
> > --- a/drivers/reset/Kconfig
> > +++ b/drivers/reset/Kconfig
> > @@ -257,6 +257,7 @@ config RESET_RZG2L_USBPHY_CTRL
> > config RESET_RZV2H_USB2PHY
> > tristate "Renesas RZ/V2H(P) (and similar SoCs) USB2PHY Reset driver"
> > depends on ARCH_RENESAS || COMPILE_TEST
> > + select REGMAP_MMIO
> > help
> > Support for USB2PHY Port reset Control found on the RZ/V2H(P) SoC
> > (and similar SoCs).
> > diff --git a/drivers/reset/reset-rzv2h-usb2phy.c b/drivers/reset/reset-rzv2h-usb2phy.c
> > index 5bdd39274612..4014eff0f017 100644
> > --- a/drivers/reset/reset-rzv2h-usb2phy.c
> > +++ b/drivers/reset/reset-rzv2h-usb2phy.c
> > @@ -5,13 +5,13 @@
> > * Copyright (C) 2025 Renesas Electronics Corporation
> > */
> >
> > -#include <linux/cleanup.h>
> > #include <linux/delay.h>
> > #include <linux/io.h>
> > #include <linux/module.h>
> > #include <linux/of.h>
> > #include <linux/platform_device.h>
> > #include <linux/pm_runtime.h>
> > +#include <linux/regmap.h>
> > #include <linux/reset.h>
> > #include <linux/reset-controller.h>
> >
> > @@ -37,10 +37,9 @@ struct rzv2h_usb2phy_reset_of_data {
> >
> > struct rzv2h_usb2phy_reset_priv {
> > const struct rzv2h_usb2phy_reset_of_data *data;
> > - void __iomem *base;
> > + struct regmap *regmap;
> > struct device *dev;
> > struct reset_controller_dev rcdev;
> > - spinlock_t lock; /* protects register accesses */
> > };
> >
> > static inline struct rzv2h_usb2phy_reset_priv
> > @@ -55,10 +54,8 @@ static int rzv2h_usbphy_reset_assert(struct reset_controller_dev *rcdev,
> > struct rzv2h_usb2phy_reset_priv *priv = rzv2h_usbphy_rcdev_to_priv(rcdev);
> > const struct rzv2h_usb2phy_reset_of_data *data = priv->data;
> >
> > - scoped_guard(spinlock, &priv->lock) {
> > - writel(data->reset2_acquire_val, priv->base + data->reset2_reg);
> > - writel(data->reset_assert_val, priv->base + data->reset_reg);
> > - }
> > + regmap_write(priv->regmap, data->reset2_reg, data->reset2_acquire_val);
> > + regmap_write(priv->regmap, data->reset_reg, data->reset_assert_val);
>
> What is the spinlock protecting? acquire/assert registers being set
> together, without another acquire/assert or deassert/release register
> access pair interleaving?
> In that case you still need the lock. Or use regmap_multi_reg_write().
> You could even directly store the sequences as struct reg_sequence in
> rzv2h_usb2phy_reset_of_data.
You are correct. Thank you.
As per your suggestion I'm planning to use regmap_multi_reg_write().
Plan is to have the:
static const struct reg_sequence rzv2h_init_seqs[] = {
{ .reg = 0xc10, .def = 0x67c },
{ .reg = 0xc14, .def = 0x1f },
{ .reg = 0x600, .def = 0x909 },
};
static const struct reg_sequence rzv2h_assert_seqs[] = {
{ .reg = 0xb04, .def = 0x303 },
{ .reg = 0x000, .def = 0x206 },
};
static const struct reg_sequence rzv2h_deassert_seqs[] = {
{ .reg = 0x000, .def = 0x200 },
{ .reg = 0xb04, .def = 0x003 },
{ .reg = 0x000, .def = 0x000 },
};
static const struct rzv2h_usb2phy_reset_of_data rzv2h_reset_of_data = {
.init_seqs = rzv2h_init_seqs,
.init_nseqs = ARRAY_SIZE(rzv2h_init_seqs),
.assert_seqs = rzv2h_assert_seqs,
.assert_nseqs = ARRAY_SIZE(rzv2h_assert_seqs),
.deassert_seqs = rzv2h_deassert_seqs,
.deassert_nseqs = ARRAY_SIZE(rzv2h_deassert_seqs),
.reset_reg = 0,
.reset_status_bits = BIT(2),
};
With that I can use:
static int rzv2h_usbphy_reset_assert(struct reset_controller_dev *rcdev,
unsigned long id)
{
struct rzv2h_usb2phy_reset_priv *priv = rzv2h_usbphy_rcdev_to_priv(rcdev);
const struct rzv2h_usb2phy_reset_of_data *data = priv->data;
int ret;
ret = regmap_multi_reg_write(priv->regmap, data->assert_seqs,
data->assert_nseqs);
if (ret)
return ret;
usleep_range(11, 20);
return 0;
}
static int rzv2h_usbphy_reset_deassert(struct reset_controller_dev *rcdev,
unsigned long id)
{
struct rzv2h_usb2phy_reset_priv *priv = rzv2h_usbphy_rcdev_to_priv(rcdev);
return regmap_multi_reg_write(priv->regmap, priv->data->deassert_seqs,
priv->data->deassert_nseqs);
}
>
> > usleep_range(11, 20);
> >
> > @@ -71,11 +68,9 @@ static int rzv2h_usbphy_reset_deassert(struct reset_controller_dev *rcdev,
> > struct rzv2h_usb2phy_reset_priv *priv = rzv2h_usbphy_rcdev_to_priv(rcdev);
> > const struct rzv2h_usb2phy_reset_of_data *data = priv->data;
> >
> > - scoped_guard(spinlock, &priv->lock) {
> > - writel(data->reset_deassert_val, priv->base + data->reset_reg);
> > - writel(data->reset2_release_val, priv->base + data->reset2_reg);
> > - writel(data->reset_release_val, priv->base + data->reset_reg);
> > - }
> > + regmap_write(priv->regmap, data->reset_reg, data->reset_deassert_val);
> > + regmap_write(priv->regmap, data->reset2_reg, data->reset2_release_val);
> > + regmap_write(priv->regmap, data->reset_reg, data->reset_release_val);
>
> Same as above.
return regmap_multi_reg_write(priv->regmap, priv->data->deassert_seqs,
priv->data->deassert_nseqs);
>
> [...]
>
> > @@ -149,7 +153,7 @@ static int rzv2h_usb2phy_reset_probe(struct platform_device *pdev)
> > return dev_err_probe(dev, error, "unable to register cleanup action\n");
> >
> > for (unsigned int i = 0; i < data->init_val_count; i++)
> > - writel(data->init_vals[i].val, priv->base + data->init_vals[i].reg);
> > + regmap_write(priv->regmap, data->init_vals[i].reg, data->init_vals[i].val);
>
> Not required for locking, but this could use regmap_multi_reg_write()
> as well.
>
And here we can use:
error = regmap_multi_reg_write(priv->regmap, data->init_seqs, data->init_nseqs);
if (error)
return dev_err_probe(dev, error, "failed to initialize PHY registers\n");
Will send new version hope later today.
Thanks again for the hint!
Kind Regards,
Tommaso
> regards
> Philipp
On Mi, 2026-04-01 at 10:04 +0200, Tommaso Merciai wrote:
> Hi Philipp,
> Thanks for your review.
>
> On Tue, Mar 31, 2026 at 06:36:45PM +0200, Philipp Zabel wrote:
> > On Fr, 2026-03-27 at 19:08 +0100, Tommaso Merciai wrote:
> > > Replace raw MMIO accesses (void __iomem *, readl/writel) with
> > > regmap_read/regmap_write via devm_regmap_init_mmio(). Regmap
> > > provides its own internal locking, so the manual spinlock and
> > > scoped_guard() wrappers are no longer needed.
> > >
> > > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > > ---
> > > v8->v9:
> > > - New patch
> > >
> > > drivers/reset/Kconfig | 1 +
> > > drivers/reset/reset-rzv2h-usb2phy.c | 42 ++++++++++++++++-------------
> > > 2 files changed, 24 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> > > index 5165006be693..c539ca88518f 100644
> > > --- a/drivers/reset/Kconfig
> > > +++ b/drivers/reset/Kconfig
> > > @@ -257,6 +257,7 @@ config RESET_RZG2L_USBPHY_CTRL
> > > config RESET_RZV2H_USB2PHY
> > > tristate "Renesas RZ/V2H(P) (and similar SoCs) USB2PHY Reset driver"
> > > depends on ARCH_RENESAS || COMPILE_TEST
> > > + select REGMAP_MMIO
> > > help
> > > Support for USB2PHY Port reset Control found on the RZ/V2H(P) SoC
> > > (and similar SoCs).
> > > diff --git a/drivers/reset/reset-rzv2h-usb2phy.c b/drivers/reset/reset-rzv2h-usb2phy.c
> > > index 5bdd39274612..4014eff0f017 100644
> > > --- a/drivers/reset/reset-rzv2h-usb2phy.c
> > > +++ b/drivers/reset/reset-rzv2h-usb2phy.c
> > > @@ -5,13 +5,13 @@
> > > * Copyright (C) 2025 Renesas Electronics Corporation
> > > */
> > >
> > > -#include <linux/cleanup.h>
> > > #include <linux/delay.h>
> > > #include <linux/io.h>
> > > #include <linux/module.h>
> > > #include <linux/of.h>
> > > #include <linux/platform_device.h>
> > > #include <linux/pm_runtime.h>
> > > +#include <linux/regmap.h>
> > > #include <linux/reset.h>
> > > #include <linux/reset-controller.h>
> > >
> > > @@ -37,10 +37,9 @@ struct rzv2h_usb2phy_reset_of_data {
> > >
> > > struct rzv2h_usb2phy_reset_priv {
> > > const struct rzv2h_usb2phy_reset_of_data *data;
> > > - void __iomem *base;
> > > + struct regmap *regmap;
> > > struct device *dev;
> > > struct reset_controller_dev rcdev;
> > > - spinlock_t lock; /* protects register accesses */
> > > };
> > >
> > > static inline struct rzv2h_usb2phy_reset_priv
> > > @@ -55,10 +54,8 @@ static int rzv2h_usbphy_reset_assert(struct reset_controller_dev *rcdev,
> > > struct rzv2h_usb2phy_reset_priv *priv = rzv2h_usbphy_rcdev_to_priv(rcdev);
> > > const struct rzv2h_usb2phy_reset_of_data *data = priv->data;
> > >
> > > - scoped_guard(spinlock, &priv->lock) {
> > > - writel(data->reset2_acquire_val, priv->base + data->reset2_reg);
> > > - writel(data->reset_assert_val, priv->base + data->reset_reg);
> > > - }
> > > + regmap_write(priv->regmap, data->reset2_reg, data->reset2_acquire_val);
> > > + regmap_write(priv->regmap, data->reset_reg, data->reset_assert_val);
> >
> > What is the spinlock protecting? acquire/assert registers being set
> > together, without another acquire/assert or deassert/release register
> > access pair interleaving?
> > In that case you still need the lock. Or use regmap_multi_reg_write().
> > You could even directly store the sequences as struct reg_sequence in
> > rzv2h_usb2phy_reset_of_data.
>
> You are correct. Thank you.
> As per your suggestion I'm planning to use regmap_multi_reg_write().
>
> Plan is to have the:
>
> static const struct reg_sequence rzv2h_init_seqs[] = {
Even though the struct is called req_sequence, the whole array is the
sequence. Let's call these _seq, singular.
> { .reg = 0xc10, .def = 0x67c },
> { .reg = 0xc14, .def = 0x1f },
0x01f for consistency?
> { .reg = 0x600, .def = 0x909 },
> };
>
> static const struct reg_sequence rzv2h_assert_seqs[] = {
> { .reg = 0xb04, .def = 0x303 },
> { .reg = 0x000, .def = 0x206 },
Consider setting .delay_us = 11, see below.
> };
>
> static const struct reg_sequence rzv2h_deassert_seqs[] = {
> { .reg = 0x000, .def = 0x200 },
> { .reg = 0xb04, .def = 0x003 },
> { .reg = 0x000, .def = 0x000 },
> };
>
> static const struct rzv2h_usb2phy_reset_of_data rzv2h_reset_of_data = {
> .init_seqs = rzv2h_init_seqs,
> .init_nseqs = ARRAY_SIZE(rzv2h_init_seqs),
> .assert_seqs = rzv2h_assert_seqs,
> .assert_nseqs = ARRAY_SIZE(rzv2h_assert_seqs),
> .deassert_seqs = rzv2h_deassert_seqs,
> .deassert_nseqs = ARRAY_SIZE(rzv2h_deassert_seqs),
> .reset_reg = 0,
> .reset_status_bits = BIT(2),
> };
>
> With that I can use:
>
> static int rzv2h_usbphy_reset_assert(struct reset_controller_dev *rcdev,
> unsigned long id)
> {
> struct rzv2h_usb2phy_reset_priv *priv = rzv2h_usbphy_rcdev_to_priv(rcdev);
> const struct rzv2h_usb2phy_reset_of_data *data = priv->data;
> int ret;
>
> ret = regmap_multi_reg_write(priv->regmap, data->assert_seqs,
> data->assert_nseqs);
> if (ret)
> return ret;
>
> usleep_range(11, 20);
Specifying a delay in rzv2h_assert_seqs[] and setting
rzv2h_usb2phy_reset_regconf.can_sleep = true would have the same
effect.
regards
Philipp
Hi Philipp,
Thanks for your comments.
On Wed, Apr 01, 2026 at 10:23:51AM +0200, Philipp Zabel wrote:
> On Mi, 2026-04-01 at 10:04 +0200, Tommaso Merciai wrote:
> > Hi Philipp,
> > Thanks for your review.
> >
> > On Tue, Mar 31, 2026 at 06:36:45PM +0200, Philipp Zabel wrote:
> > > On Fr, 2026-03-27 at 19:08 +0100, Tommaso Merciai wrote:
> > > > Replace raw MMIO accesses (void __iomem *, readl/writel) with
> > > > regmap_read/regmap_write via devm_regmap_init_mmio(). Regmap
> > > > provides its own internal locking, so the manual spinlock and
> > > > scoped_guard() wrappers are no longer needed.
> > > >
> > > > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > > > ---
> > > > v8->v9:
> > > > - New patch
> > > >
> > > > drivers/reset/Kconfig | 1 +
> > > > drivers/reset/reset-rzv2h-usb2phy.c | 42 ++++++++++++++++-------------
> > > > 2 files changed, 24 insertions(+), 19 deletions(-)
> > > >
> > > > diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> > > > index 5165006be693..c539ca88518f 100644
> > > > --- a/drivers/reset/Kconfig
> > > > +++ b/drivers/reset/Kconfig
> > > > @@ -257,6 +257,7 @@ config RESET_RZG2L_USBPHY_CTRL
> > > > config RESET_RZV2H_USB2PHY
> > > > tristate "Renesas RZ/V2H(P) (and similar SoCs) USB2PHY Reset driver"
> > > > depends on ARCH_RENESAS || COMPILE_TEST
> > > > + select REGMAP_MMIO
> > > > help
> > > > Support for USB2PHY Port reset Control found on the RZ/V2H(P) SoC
> > > > (and similar SoCs).
> > > > diff --git a/drivers/reset/reset-rzv2h-usb2phy.c b/drivers/reset/reset-rzv2h-usb2phy.c
> > > > index 5bdd39274612..4014eff0f017 100644
> > > > --- a/drivers/reset/reset-rzv2h-usb2phy.c
> > > > +++ b/drivers/reset/reset-rzv2h-usb2phy.c
> > > > @@ -5,13 +5,13 @@
> > > > * Copyright (C) 2025 Renesas Electronics Corporation
> > > > */
> > > >
> > > > -#include <linux/cleanup.h>
> > > > #include <linux/delay.h>
> > > > #include <linux/io.h>
> > > > #include <linux/module.h>
> > > > #include <linux/of.h>
> > > > #include <linux/platform_device.h>
> > > > #include <linux/pm_runtime.h>
> > > > +#include <linux/regmap.h>
> > > > #include <linux/reset.h>
> > > > #include <linux/reset-controller.h>
> > > >
> > > > @@ -37,10 +37,9 @@ struct rzv2h_usb2phy_reset_of_data {
> > > >
> > > > struct rzv2h_usb2phy_reset_priv {
> > > > const struct rzv2h_usb2phy_reset_of_data *data;
> > > > - void __iomem *base;
> > > > + struct regmap *regmap;
> > > > struct device *dev;
> > > > struct reset_controller_dev rcdev;
> > > > - spinlock_t lock; /* protects register accesses */
> > > > };
> > > >
> > > > static inline struct rzv2h_usb2phy_reset_priv
> > > > @@ -55,10 +54,8 @@ static int rzv2h_usbphy_reset_assert(struct reset_controller_dev *rcdev,
> > > > struct rzv2h_usb2phy_reset_priv *priv = rzv2h_usbphy_rcdev_to_priv(rcdev);
> > > > const struct rzv2h_usb2phy_reset_of_data *data = priv->data;
> > > >
> > > > - scoped_guard(spinlock, &priv->lock) {
> > > > - writel(data->reset2_acquire_val, priv->base + data->reset2_reg);
> > > > - writel(data->reset_assert_val, priv->base + data->reset_reg);
> > > > - }
> > > > + regmap_write(priv->regmap, data->reset2_reg, data->reset2_acquire_val);
> > > > + regmap_write(priv->regmap, data->reset_reg, data->reset_assert_val);
> > >
> > > What is the spinlock protecting? acquire/assert registers being set
> > > together, without another acquire/assert or deassert/release register
> > > access pair interleaving?
> > > In that case you still need the lock. Or use regmap_multi_reg_write().
> > > You could even directly store the sequences as struct reg_sequence in
> > > rzv2h_usb2phy_reset_of_data.
> >
> > You are correct. Thank you.
> > As per your suggestion I'm planning to use regmap_multi_reg_write().
> >
> > Plan is to have the:
> >
> > static const struct reg_sequence rzv2h_init_seqs[] = {
>
> Even though the struct is called req_sequence, the whole array is the
> sequence. Let's call these _seq, singular.
Ack.
>
> > { .reg = 0xc10, .def = 0x67c },
> > { .reg = 0xc14, .def = 0x1f },
>
> 0x01f for consistency?
Ouch, thank you!
>
> > { .reg = 0x600, .def = 0x909 },
> > };
> >
> > static const struct reg_sequence rzv2h_assert_seqs[] = {
> > { .reg = 0xb04, .def = 0x303 },
> > { .reg = 0x000, .def = 0x206 },
>
> Consider setting .delay_us = 11, see below.
Thanks for the hint.
>
> > };
> >
> > static const struct reg_sequence rzv2h_deassert_seqs[] = {
> > { .reg = 0x000, .def = 0x200 },
> > { .reg = 0xb04, .def = 0x003 },
> > { .reg = 0x000, .def = 0x000 },
> > };
> >
> > static const struct rzv2h_usb2phy_reset_of_data rzv2h_reset_of_data = {
> > .init_seqs = rzv2h_init_seqs,
> > .init_nseqs = ARRAY_SIZE(rzv2h_init_seqs),
> > .assert_seqs = rzv2h_assert_seqs,
> > .assert_nseqs = ARRAY_SIZE(rzv2h_assert_seqs),
> > .deassert_seqs = rzv2h_deassert_seqs,
> > .deassert_nseqs = ARRAY_SIZE(rzv2h_deassert_seqs),
> > .reset_reg = 0,
> > .reset_status_bits = BIT(2),
> > };
> >
> > With that I can use:
> >
> > static int rzv2h_usbphy_reset_assert(struct reset_controller_dev *rcdev,
> > unsigned long id)
> > {
> > struct rzv2h_usb2phy_reset_priv *priv = rzv2h_usbphy_rcdev_to_priv(rcdev);
> > const struct rzv2h_usb2phy_reset_of_data *data = priv->data;
> > int ret;
> >
> > ret = regmap_multi_reg_write(priv->regmap, data->assert_seqs,
> > data->assert_nseqs);
> > if (ret)
> > return ret;
> >
> > usleep_range(11, 20);
>
> Specifying a delay in rzv2h_assert_seqs[] and setting
> rzv2h_usb2phy_reset_regconf.can_sleep = true would have the same
> effect.
Then we can have:
static const struct reg_sequence rzv2h_init_seq[] = {
{ .reg = 0xc10, .def = 0x67c },
{ .reg = 0xc14, .def = 0x01f },
{ .reg = 0x600, .def = 0x909 },
};
static const struct reg_sequence rzv2h_assert_seq[] = {
{ .reg = 0xb04, .def = 0x303 },
{ .reg = 0x000, .def = 0x206, .delay_us = 20 },
};
static const struct reg_sequence rzv2h_deassert_seq[] = {
{ .reg = 0x000, .def = 0x200 },
{ .reg = 0xb04, .def = 0x003 },
{ .reg = 0x000, .def = 0x000 },
};
static const struct rzv2h_usb2phy_reset_of_data rzv2h_reset_of_data = {
.init_seq = rzv2h_init_seq,
.init_nseq = ARRAY_SIZE(rzv2h_init_seq),
.assert_seq = rzv2h_assert_seq,
.assert_nseq = ARRAY_SIZE(rzv2h_assert_seq),
.deassert_seq = rzv2h_deassert_seq,
.deassert_nseq = ARRAY_SIZE(rzv2h_deassert_seq),
.reset_reg = 0,
.reset_status_bits = BIT(2),
};
And I'll add:
static const struct regmap_config rzv2h_usb2phy_reset_regconf = {
.reg_bits = 32,
.val_bits = 32,
.reg_stride = 4,
.can_sleep = true,
};
In this way as you suggested we can drop usleep_range() into
rzv2h_usbphy_reset_assert() in thi way I think we can have
simply:
static int rzv2h_usbphy_reset_assert(struct reset_controller_dev *rcdev,
unsigned long id)
{
struct rzv2h_usb2phy_reset_priv *priv = rzv2h_usbphy_rcdev_to_priv(rcdev);
return regmap_multi_reg_write(priv->regmap, priv->data->assert_seq,
priv->data->assert_nseq);
}
What do you think? Thanks.
Kind Regards,
Tommaso
>
> regards
> Philipp
On Mi, 2026-04-01 at 11:10 +0200, Tommaso Merciai wrote:
[...]
>
> Then we can have:
>
> static const struct reg_sequence rzv2h_init_seq[] = {
> { .reg = 0xc10, .def = 0x67c },
> { .reg = 0xc14, .def = 0x01f },
> { .reg = 0x600, .def = 0x909 },
> };
>
> static const struct reg_sequence rzv2h_assert_seq[] = {
> { .reg = 0xb04, .def = 0x303 },
> { .reg = 0x000, .def = 0x206, .delay_us = 20 },
This will call fsleep(20), which maps to usleep_range(20, 25).
Please comment on why the delay is changed in the commit message.
regards
Philipp
Hi Philipp,
Thanks for your comments.
On Wed, Apr 01, 2026 at 11:25:52AM +0200, Philipp Zabel wrote:
> On Mi, 2026-04-01 at 11:10 +0200, Tommaso Merciai wrote:
> [...]
> >
> > Then we can have:
> >
> > static const struct reg_sequence rzv2h_init_seq[] = {
> > { .reg = 0xc10, .def = 0x67c },
> > { .reg = 0xc14, .def = 0x01f },
> > { .reg = 0x600, .def = 0x909 },
> > };
> >
> > static const struct reg_sequence rzv2h_assert_seq[] = {
> > { .reg = 0xb04, .def = 0x303 },
> > { .reg = 0x000, .def = 0x206, .delay_us = 20 },
>
> This will call fsleep(20), which maps to usleep_range(20, 25).
> Please comment on why the delay is changed in the commit message.
Ouch, thanks.
Reference Manual says:
- This reset must be asserted for more than 10us.
Then I think the right choiche would be:
.delay_us = 11
This will call fsleep(11) which maps usleep_range(11, 13)
Please correct me if I'm wrong.
Thanks,
Tommaso
>
> regards
> Philipp
© 2016 - 2026 Red Hat, Inc.