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] [day] [month] [year] [list]

Message-ID: <CAMuHMdWjzR6vgbr_CfR7r-h1FqWxs1nY0hm274kxFmoHjCtRAA@mail.gmail.com>
Date: Thu, 28 Nov 2024 16:46:05 +0100
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Claudiu <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,

CC Ulf

Thanks for your patch!

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?
The extra safety requires duplicating the register bits in both DT
and the SYSC driver.
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?

I am not a big fan of describing register bits in DT, but for the other
SYSC users you list above, syscon+regmap seems to be a valid solution.
For USB and PCIe control, the situation is different. I more liked the
approach with "reset IDs" you had in v1, as it abstracts the DT
description from the register bits, and the USB and PCIe reset bits use
a different polarity (on RZ/G3S). If future SoC integration changes
the polarity, you have to handle that in the consumer (USB or PCIe)
driver, too.  Unfortunately such "reset IDs" are only suitable for
use with the reset or pmdomain frameworks, which didn't survive the
earlier discussions.

One other option would be to let SYSC expose regulators?
While that would work for USB and PCIe control, we would still need
syscon+regmap for the other bits.

So the more I think about it, the more I like your (clever) solution...

> Along with it, add the syscon to the compatible list as it will be
> requested by the consumer drivers. The syscon was added to the rest of
> system controller variants as these are similar with RZ/G3S and can
> benefit from the implementation proposed in this series.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@...renesas.com>

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