linux-kernel - RE: [PATCH v2 2/2] reset: Add USB2PHY control 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] [day] [month] [year] [list]

Message-ID:
 <TYCPR01MB12093DB963348A8FD58409E5AC2DE2@TYCPR01MB12093.jpnprd01.prod.outlook.com>
Date: Tue, 18 Mar 2025 12:31:52 +0000
From: Fabrizio Castro <fabrizio.castro.jz@...esas.com>
To: Philipp Zabel <p.zabel@...gutronix.de>, Prabhakar
	<prabhakar.csengg@...il.com>, Geert Uytterhoeven <geert+renesas@...der.be>,
	Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>, Magnus Damm <magnus.damm@...il.com>
CC: "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-renesas-soc@...r.kernel.org" <linux-renesas-soc@...r.kernel.org>, Biju
 Das <biju.das.jz@...renesas.com>, Prabhakar Mahadev Lad
	<prabhakar.mahadev-lad.rj@...renesas.com>, Chris Paterson
	<Chris.Paterson2@...esas.com>
Subject: RE: [PATCH v2 2/2] reset: Add USB2PHY control driver for Renesas
 RZ/V2H(P)

Hi Philipp,

Thanks for your feedback!

> From: Philipp Zabel <p.zabel@...gutronix.de>
> Sent: 13 March 2025 13:06
> Subject: Re: [PATCH v2 2/2] reset: Add USB2PHY control driver for Renesas RZ/V2H(P)
> 
> Hi Fabrizio,
> 
> On Do, 2025-03-13 at 10:14 +0000, Fabrizio Castro wrote:
> > Hi Philipp,
> >
> > Thanks for your feedback!
> >
> > > From: Philipp Zabel <p.zabel@...gutronix.de>
> > > Sent: 13 March 2025 08:37
> > > Subject: Re: [PATCH v2 2/2] reset: Add USB2PHY control driver for Renesas RZ/V2H(P)
> > >
> > > On Mi, 2025-03-05 at 12:39 +0000, Prabhakar wrote:
> > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
> > > >
> > > > Add support for the USB2PHY control driver on the Renesas RZ/V2H(P) SoC.
> > > > Make the driver handle reset and power-down operations for the USB2PHY.
> > > >
> > > > Pass OF data to support future SoCs with similar USB2PHY hardware but
> > > > different register configurations. Define device-specific initialization
> > > > values and control register settings in OF data to ensure flexibility
> > > > for upcoming SoCs.
> > > >
> > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
> > > > ---
> > > >  drivers/reset/Kconfig                    |   7 +
> > > >  drivers/reset/Makefile                   |   1 +
> > > >  drivers/reset/reset-rzv2h-usb2phy-ctrl.c | 223 +++++++++++++++++++++++
> > > >  3 files changed, 231 insertions(+)
> > > >  create mode 100644 drivers/reset/reset-rzv2h-usb2phy-ctrl.c
> > > >
> [...]
> > > > diff --git a/drivers/reset/reset-rzv2h-usb2phy-ctrl.c b/drivers/reset/reset-rzv2h-usb2phy-ctrl.c
> > > > new file mode 100644
> > > > index 000000000000..a6daeaf37e1c
> > > > --- /dev/null
> > > > +++ b/drivers/reset/reset-rzv2h-usb2phy-ctrl.c
> > > > @@ -0,0 +1,223 @@
> [...]
> > > > +static const struct rzv2h_usb2phy_regval rzv2h_init_vals[] = {
> > > > +	{ .reg = 0xc10, .val = 0x67c },
> > > > +	{ .reg = 0xc14, .val = 0x1f },
> > > > +	{ .reg = 0x600, .val = 0x909 },
> > >
> > > What are these registers and what are those values doing?
> >
> > Unfortunately, there are some licensing restrictions on this IP, this is
> > the best that we can do, as per the license agreement.
> 
> How am I expected to review this?
> 
> For now, I'll assume that these registers are not related to reset
> functionality at all, and that this driver should be a phy controller
> driver instead of a reset controller driver.
> 
> Can you convince me otherwise without breaking license agreements?

Sorry about the delay, as you may have figured out, we had to double check with
the LSI team before making any statement.

We can confirm that `rzv2h_init_vals` contains the registers and corresponding
initialization values required to prepare the PHY to receive assert and deassert
requests. This is a one time only thing, done at probe.

After looking into things again, I have noticed that the probe function is missing
calling into the assert sequence, and the status of the reset is undefined, so
that's something to fix for v3 to make it initialize in asserted state.

The assert, deassert, and status operations are only touching reset related registers.
Nothing else.

Therefore we believe this should be a port reset driver.

Thanks for your patience so far, and sorry for being cryptic.

Cheers,
Fab

> 
> regards
> Philipp