linux-kernel - Re: [PATCH v4 2/3] reset: Add USB2PHY port reset driver for Renesas RZ/V2H(P)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CA+V-a8uxZRnAC7u2wX10qrQKVVbjJ1yhzN91yQRKQXb=hZWs3A@mail.gmail.com>
Date: Mon, 14 Apr 2025 18:43:02 +0100
From: "Lad, Prabhakar" <prabhakar.csengg@...il.com>
To: Biju Das <biju.das.jz@...renesas.com>
Cc: Philipp Zabel <p.zabel@...gutronix.de>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Geert Uytterhoeven <geert+renesas@...der.be>, Magnus Damm <magnus.damm@...il.com>, 
	"linux-renesas-soc@...r.kernel.org" <linux-renesas-soc@...r.kernel.org>, 
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, 
	Fabrizio Castro <fabrizio.castro.jz@...esas.com>, 
	Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@...renesas.com>
Subject: Re: [PATCH v4 2/3] reset: Add USB2PHY port reset driver for Renesas RZ/V2H(P)

Hi Biju,

Thank you for the review.

On Mon, Apr 14, 2025 at 2:05 PM Biju Das <biju.das.jz@...renesas.com> wrote:
>
> Hi Prabhakar,
>
> Thanks for the patch.
>
> > -----Original Message-----
> > From: Prabhakar <prabhakar.csengg@...il.com>
> > Sent: 14 April 2025 14:00
> > Subject: [PATCH v4 2/3] reset: Add USB2PHY port reset driver for Renesas RZ/V2H(P)
> >
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
> >
> > Implement a USB2PHY port reset driver for the Renesas RZ/V2H(P) SoC.
> > Enable control of USB2.0 PHY reset and power-down operations, including assert and deassert
> > functionalities for the PHY.
> >
> > Leverage device tree (OF) data to support future SoCs with similar USB2PHY hardware but varying
> > register configurations. Define initialization values and control register settings to ensure
> > flexibility for upcoming platforms.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
> > ---
> >  drivers/reset/Kconfig               |   7 +
> >  drivers/reset/Makefile              |   1 +
> >  drivers/reset/reset-rzv2h-usb2phy.c | 241 ++++++++++++++++++++++++++++
> >  3 files changed, 249 insertions(+)
> >  create mode 100644 drivers/reset/reset-rzv2h-usb2phy.c
> >
> > diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig index 99f6f9784e68..6c7dbd8ca580 100644
> > --- a/drivers/reset/Kconfig
> > +++ b/drivers/reset/Kconfig
> > @@ -225,6 +225,13 @@ config RESET_RZG2L_USBPHY_CTRL
> >         Support for USBPHY Control found on RZ/G2L family. It mainly
> >         controls reset and power down of the USB/PHY.
> >
> > +config RESET_RZV2H_USB2PHY
> > +     tristate "Renesas RZ/V2H(P) (and similar SoCs) USB2PHY Reset driver"
> > +     depends on ARCH_RENESAS || COMPILE_TEST
> > +     help
> > +       Support for USB2PHY Port reset Control found on the RZ/V2H(P) SoC
> > +       (and similar SoCs).
> > +
> >  config RESET_SCMI
> >       tristate "Reset driver controlled via ARM SCMI interface"
> >       depends on ARM_SCMI_PROTOCOL || COMPILE_TEST diff --git a/drivers/reset/Makefile
> > b/drivers/reset/Makefile index 31f9904d13f9..30d0fb4ac965 100644
> > --- a/drivers/reset/Makefile
> > +++ b/drivers/reset/Makefile
> > @@ -31,6 +31,7 @@ obj-$(CONFIG_RESET_QCOM_AOSS) += reset-qcom-aoss.o
> >  obj-$(CONFIG_RESET_QCOM_PDC) += reset-qcom-pdc.o
> >  obj-$(CONFIG_RESET_RASPBERRYPI) += reset-raspberrypi.o
> >  obj-$(CONFIG_RESET_RZG2L_USBPHY_CTRL) += reset-rzg2l-usbphy-ctrl.o
> > +obj-$(CONFIG_RESET_RZV2H_USB2PHY) += reset-rzv2h-usb2phy.o
> >  obj-$(CONFIG_RESET_SCMI) += reset-scmi.o
> >  obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o
> >  obj-$(CONFIG_RESET_SOCFPGA) += reset-socfpga.o diff --git a/drivers/reset/reset-rzv2h-usb2phy.c
> > b/drivers/reset/reset-rzv2h-usb2phy.c
> > new file mode 100644
> > index 000000000000..737b768829c9
> > --- /dev/null
> > +++ b/drivers/reset/reset-rzv2h-usb2phy.c
> > @@ -0,0 +1,241 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Renesas RZ/V2H(P) USB2PHY Port reset control driver
> > + *
> > + * 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/reset.h>
> > +#include <linux/reset-controller.h>
> > +
> > +struct rzv2h_usb2phy_regval {
> > +     u16 reg;
> > +     u16 val;
> > +};
> > +
> > +struct rzv2h_usb2phy_reset_of_data {
> > +     const struct rzv2h_usb2phy_regval *init_vals;
> > +     unsigned int init_val_count;
> > +
> > +     u16 reset_reg;
> > +     u16 reset_assert_val;
> > +     u16 reset_deassert_val;
> > +     u16 reset_status_bits;
> > +     u16 reset_release_val;
> > +
> > +     u16 reset2_reg;
> > +     u16 reset2_acquire_val;
> > +     u16 reset2_release_val;
> > +};
> > +
<snip>
> > +static const struct reset_control_ops rzv2h_usbphy_reset_ops = {
> > +     .assert = rzv2h_usbphy_reset_assert,
> > +     .deassert = rzv2h_usbphy_reset_deassert,
> > +     .status = rzv2h_usbphy_reset_status,
> > +};
> > +
> > +static int rzv2h_usb2phy_reset_of_xlate(struct reset_controller_dev *rcdev,
> > +                                     const struct of_phandle_args *reset_spec) {
> > +     /* No special handling needed, we have only one reset line per device */
> > +     return 0;
> > +}
> > +
> > +static int rzv2h_usb2phy_reset_probe(struct platform_device *pdev) {
> > +     const struct rzv2h_usb2phy_reset_of_data *data;
> > +     struct rzv2h_usb2phy_reset_priv *priv;
> > +     struct device *dev = &pdev->dev;
> > +     struct reset_control *rstc;
> > +     int error;
> > +
> > +     priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > +     if (!priv)
> > +             return -ENOMEM;
> > +
> > +     data = of_device_get_match_data(dev);
> > +     if (!data)
> > +             return dev_err_probe(dev, -ENODEV,
> > +                                  "failed to match device\n");
>
> This check is not needed as there is always data.
>
Agreed, I will drop this.

> > +
> > +     priv->data = data;
> > +     priv->dev = dev;
> > +     priv->base = devm_platform_ioremap_resource(pdev, 0);
> > +     if (IS_ERR(priv->base))
> > +             return PTR_ERR(priv->base);
> > +
> > +     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);
> > +     dev_set_drvdata(dev, priv);
and this too and send a new version.


Cheers,
Prabhakar