[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID:
<TY3PR01MB11346B0B825BC26B1AF690EF7860EA@TY3PR01MB11346.jpnprd01.prod.outlook.com>
Date: Wed, 10 Sep 2025 10:43:54 +0000
From: Biju Das <biju.das.jz@...renesas.com>
To: Philipp Zabel <p.zabel@...gutronix.de>, biju.das.au
<biju.das.au@...il.com>, Vinod Koul <vkoul@...nel.org>, Kishon Vijay Abraham
I <kishon@...nel.org>, Geert Uytterhoeven <geert+renesas@...der.be>,
magnus.damm <magnus.damm@...il.com>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-phy@...ts.infradead.org" <linux-phy@...ts.infradead.org>,
"linux-renesas-soc@...r.kernel.org" <linux-renesas-soc@...r.kernel.org>,
Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@...renesas.com>
Subject: RE: [PATCH v2 04/11] phy: renesas: Add Renesas RZ/G3E USB3.0 PHY
driver
Hi Philipp Zabel,
Thanks for the feedback.
> -----Original Message-----
> From: Philipp Zabel <p.zabel@...gutronix.de>
> Sent: 10 September 2025 10:18
> Subject: Re: [PATCH v2 04/11] phy: renesas: Add Renesas RZ/G3E USB3.0 PHY driver
>
> On Di, 2025-09-09 at 19:07 +0100, Biju wrote:
> > From: Biju Das <biju.das.jz@...renesas.com>
> >
> > Add Renesas RZ/G3E USB3.0 PHY driver. This module is connected between
> > USB3 Host and PHY module. The main functions of this module are:
> > 1) Reset control
> > 2) Control of PHY input pins
> > 3) Monitoring of PHY output pins
> >
> > Signed-off-by: Biju Das <biju.das.jz@...renesas.com>
> > ---
> > v1->v2:
> > * Replaced magic numbers with macros.
> > ---
> > drivers/phy/renesas/Kconfig | 7 +
> > drivers/phy/renesas/Makefile | 1 +
> > drivers/phy/renesas/phy-rzg3e-usb3.c | 271
> > +++++++++++++++++++++++++++
> > 3 files changed, 279 insertions(+)
> > create mode 100644 drivers/phy/renesas/phy-rzg3e-usb3.c
> >
> [...]
> > diff --git a/drivers/phy/renesas/phy-rzg3e-usb3.c
> > b/drivers/phy/renesas/phy-rzg3e-usb3.c
> > new file mode 100644
> > index 000000000000..6463519b2c22
> > --- /dev/null
> > +++ b/drivers/phy/renesas/phy-rzg3e-usb3.c
> > @@ -0,0 +1,271 @@
> [...]
> > +static int rzg3e_phy_usb3_probe(struct platform_device *pdev) {
> > + struct device *dev = &pdev->dev;
> > + struct phy_provider *provider;
> > + struct rz_usb3 *r;
> > + struct phy *phy;
> > + int ret;
> > +
> > + r = devm_kzalloc(dev, sizeof(*r), GFP_KERNEL);
> > + if (!r)
> > + return -ENOMEM;
> > +
> > + r->base = devm_platform_ioremap_resource(pdev, 0);
> > + if (IS_ERR(r->base))
> > + return PTR_ERR(r->base);
> > +
> > + r->rstc = devm_reset_control_get_shared(dev, NULL);
> > + if (IS_ERR(r->rstc))
> > + return dev_err_probe(dev, PTR_ERR(r->rstc), "failed to get
> > +reset\n");
> > +
> > + ret = reset_control_deassert(r->rstc);
> > + if (ret)
> > + return ret;
>
> It looks like you should be able to use
> devm_reset_control_get_shared_deasserted() and let devres handle reset control assertion on error and
> driver removal.
>
> As is, the driver is missing a reset_control_assert() in the probe error paths.
Agreed. Will address this in next version.
Cheers,
Biju
Powered by blists - more mailing lists