linux-kernel - Re: [PATCH v2 01/15] dt-bindings: soc: renesas: renesas,rzg2l-sysc: Add #renesas,sysc-signal-cells

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: <CAMuHMdUuRSJu1c2zJvOc8EGrZy1uYcN0aiUG6T7WShawPmCNJg@mail.gmail.com>
Date: Fri, 29 Nov 2024 09:38:21 +0100
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Claudiu Beznea <claudiu.beznea@...on.dev>
Cc: vkoul@...nel.org, kishon@...nel.org, robh@...nel.org, krzk+dt@...nel.org, 
	conor+dt@...nel.org, p.zabel@...gutronix.de, magnus.damm@...il.com, 
	gregkh@...uxfoundation.org, yoshihiro.shimoda.uh@...esas.com, 
	christophe.jaillet@...adoo.fr, linux-phy@...ts.infradead.org, 
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-renesas-soc@...r.kernel.org, linux-usb@...r.kernel.org, 
	Claudiu Beznea <claudiu.beznea.uj@...renesas.com>, Ulf Hansson <ulf.hansson@...aro.org>
Subject: Re: [PATCH v2 01/15] dt-bindings: soc: renesas: renesas,rzg2l-sysc:
 Add #renesas,sysc-signal-cells

Hi Claudiu,

On Fri, Nov 29, 2024 at 9:21 AM Claudiu Beznea <claudiu.beznea@...on.dev> wrote:
> On 28.11.2024 17:46, Geert Uytterhoeven wrote:
> > On Tue, Nov 26, 2024 at 10:21 AM Claudiu <claudiu.beznea@...on.dev> wrote:
> >> From: Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
> >>
> >> The RZ/G3S system controller (SYSC) has registers to control signals that
> >> are routed to various IPs. These signals must be controlled during
> >> configuration of the respective IPs. One such signal is the USB PWRRDY,
> >> which connects the SYSC and the USB PHY. This signal must to be controlled
> >> before and after the power to the USB PHY is turned off/on.
> >>
> >> Other similar signals include the following (according to the RZ/G3S
> >> hardware manual):
> >>
> >> * PCIe:
> >> - ALLOW_ENTER_L1 signal controlled through the SYS_PCIE_CFG register
> >> - PCIE_RST_RSM_B signal controlled through the SYS_PCIE_RST_RSM_B
> >>   register
> >> - MODE_RXTERMINATION signal controlled through SYS_PCIE_PHY register
> >>
> >> * SPI:
> >> - SEL_SPI_OCTA signal controlled through SYS_IPCONT_SEL_SPI_OCTA
> >>   register
> >>
> >> * I2C/I3C:
> >> - af_bypass I2C signals controlled through SYS_I2Cx_CFG registers
> >>   (x=0..3)
> >> - af_bypass I3C signal controlled through SYS_I3C_CFG register
> >>
> >> * Ethernet:
> >> - FEC_GIGA_ENABLE Ethernet signals controlled through SYS_GETHx_CFG
> >>   registers (x=0..1)
> >>
> >> Add #renesas,sysc-signal-cells DT property to allow different SYSC signals
> >> consumers to manage these signals.
> >>
> >> The goal is to enable consumers to specify the required access data for
> >> these signals (through device tree) and let their respective drivers
> >> control these signals via the syscon regmap provided by the system
> >> controller driver. For example, the USB PHY will describe this relation
> >> using the following DT property:
> >>
> >> usb2_phy1: usb-phy@...30200 {
> >>         // ...
> >>         renesas,sysc-signal = <&sysc 0xd70 0x1>;
> >>         // ...
> >> };
> >
> > IIUIC, the consumer driver will  appear to control the SYSC bits
> > directly, but due to the use of custom validating regmap accessors
> > and reference counting in the SYSC driver, this is safe?
>
> I'm not sure I fully understand the safety concern.

Sorry for my bad expression, this was more like a rhetorical question.
I meant that it is safe because:
  1. Consumers cannot perform arbitrary register accesses,
  2. The reference counting guarantees correct operation, despite
      both usb-phy nodes using the same renesas,sysc-signal.

So everything is fine.

> > The extra safety requires duplicating the register bits in both DT
> > and the SYSC driver.
>
> One other option I saw was to have common defines for registers that could
> have been shared b/w driver and DTSes. But it looked better to me the way
> it has been presented in this series.
>
> > Both usb-phy nodes on RZG3S use the same renesas,sysc-signal, so the
> > reference counting is indeed needed.  They are in different power
> > domains, could that be an issue w.r.t. ordering?
>
> In chapter "32.4.2.1 USB/PHY related pins", section "When either Port1 or
> Port2 is unused" of the RZ/G3S HW manual it is mentioned "Since USB_VDD18 /
> USB_VDD33 are common to 2 Port PHY, it is necessary to supply power even
> when one of the
>  ports is not in use".

Does that mean you have to power the other PHY on through the
CPG_BUS_PERI_COM_MSTOP register, too?
(I know you haven't added R9A08G045_PD_USBx to the USB nodes yet,
 as #power-domain-cells is still 0).

> (From the discussions w/ the internal HW team) The PWRRDY is an (software
> controlled) indicator to the USB PHY that power supply is ready.
>
> From that and [1] I get that both PHYs are powered by the same regulators
> (USB_VDD18/USB_VDD33) and the USB PWRRDY signal need to be set before/after
> the USB PHY power off/on. Because of this I consider the order doesn't matter.
>
> [1] https://gcdnb.pbrd.co/images/0a1zYBFZXZVb.png

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds