linux-kernel - Re: [PATCH net-next v3 06/10] dt-bindings: net: Add Synopsys DW xPCS bindings

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: <e5mqaztxz62b7jktr47mojjrz7ht5m4ou4mqsxtozpp3xba7e4@uh7v5zn2pbn2>
Date: Thu, 27 Jun 2024 20:10:48 +0300
From: Serge Semin <fancer.lancer@...il.com>
To: Conor Dooley <conor@...nel.org>
Cc: Andrew Lunn <andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>, 
	Russell King <linux@...linux.org.uk>, Alexandre Torgue <alexandre.torgue@...s.st.com>, 
	Jose Abreu <joabreu@...opsys.com>, Jose Abreu <Jose.Abreu@...opsys.com>, 
	Vladimir Oltean <olteanv@...il.com>, Florian Fainelli <f.fainelli@...il.com>, 
	Maxime Chevallier <maxime.chevallier@...tlin.com>, Rob Herring <robh+dt@...nel.org>, 
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>, Conor Dooley <conor+dt@...nel.org>, 
	"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Sagar Cheluvegowda <quic_scheluve@...cinc.com>, 
	Abhishek Chauhan <quic_abchauha@...cinc.com>, Andrew Halaney <ahalaney@...hat.com>, 
	Jiawen Wu <jiawenwu@...stnetic.com>, Mengyuan Lou <mengyuanlou@...-swift.com>, 
	Tomer Maimon <tmaimon77@...il.com>, openbmc@...ts.ozlabs.org, netdev@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v3 06/10] dt-bindings: net: Add Synopsys DW xPCS
 bindings

On Thu, Jun 27, 2024 at 04:51:22PM +0100, Conor Dooley wrote:
> On Thu, Jun 27, 2024 at 03:41:26AM +0300, Serge Semin wrote:
> > +  clocks:
> > +    description:
> > +      Both MCI and APB3 interfaces are supposed to be equipped with a clock
> > +      source connected via the clk_csr_i line.
> > +
> > +      PCS/PMA layer can be clocked by an internal reference clock source
> > +      (phyN_core_refclk) or by an externally connected (phyN_pad_refclk) clock
> > +      generator. Both clocks can be supplied at a time.
> > +    minItems: 1
> > +    maxItems: 3
> > +
> > +  clock-names:
> > +    oneOf:
> > +      - minItems: 1
> > +        items:
> > +          - enum: [core, pad]
> > +          - const: pad
> > +      - minItems: 1
> > +        items:
> > +          - const: pclk
> > +          - enum: [core, pad]
> > +          - const: pad
> 

> While reading this, I'm kinda struggling to map "clk_csr_i" to a clock
> name. Is that pclk? And why pclk if it is connected to "clk_csr_i"?

Right. It's "pclk". The reason of using the "pclk" name is that it has
turned to be a de-facto standard name in the DT-bindings for the
peripheral bus clock sources utilized for the CSR-space IO buses.
Moreover the STMMAC driver responsible for the parental DW *MAC
devices handling also has the "pclk" name utilized for the clk_csr_i
signal. So using the "pclk" name in the tightly coupled devices (MAC
and PCS) for the same signal seemed a good idea.

> If two interfaces are meant to be "equipped" with that clock, how come
> it is optional? I'm probably missing something...

MCI and APB3 interfaces are basically the same from the bindings
pointer of view. Both of them can be utilized for the DW XPCS
installed on the SoC system bus, so the device could be accessed using
the simple MMIO ops.

The first "clock-names" schema is meant to be applied on the DW XPCS
accessible over an _MDIO_ bus, which obviously doesn't have any
special CSR IO bus. In that case the DW XPCS device is supposed to be
defined as a subnode of the MDIO-bus DT-node.

The second "clock-names" constraint is supposed to be applied to the
DW XPCS synthesized with the MCI/APB3 CSRs IO interface. The device in
that case should be defined in the DT source file as a normal memory
mapped device.

> 
> Otherwise this binding looks fine to me.

Shall I add a note to the clock description that the "clk_csr_i"
signal is named as "pclk"? I'll need to resubmit the series anyway.

Thanks
-Serge(y)

> 
> Wee bit confused,
> Conor.