linux-kernel - Re: [PATCH 1/2] dt-bindings: soc: ti: pruss: allow ethernet controller in ICSSG node

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <99cc7afbb891de890ff051606f7a120f796e0fbc.camel@ew.tq-group.com>
Date: Thu, 20 Jun 2024 10:26:07 +0200
From: Matthias Schiffer <matthias.schiffer@...tq-group.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
 linux-arm-kernel@...ts.infradead.org, linux@...tq-group.com, Rob Herring
 <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Nishanth Menon <nm@...com>, Vignesh Raghavendra
 <vigneshr@...com>, Tero Kristo <kristo@...nel.org>, Suman Anna
 <s-anna@...com>
Subject: Re: [PATCH 1/2] dt-bindings: soc: ti: pruss: allow ethernet
 controller in ICSSG node

On Thu, 2024-06-20 at 09:24 +0200, Krzysztof Kozlowski wrote:
> On 19/06/2024 13:24, Matthias Schiffer wrote:
> > While the current Device Trees for TI EVMs configure the PRUSS Ethernet
> > controller as a toplevel node with names like "icssg1-eth", allowing to
> > make it a subnode of the ICSSG has a number of advantages:
> 
> What is ICSSG? The sram or ti,prus from the ethernet schema?

ICSSG (Industrial Communication Subsystem (Group?)) is the main device described by the
ti,pruss.yaml binding (ICSS and PRUSS are different variants of similar IP cores); it is the
container for the individual PRU, TXPRU and RTU cores which are referenced by the ti,prus
node of the Ethernet schema.

The entirety of PRU, TXPRU and RTU cores of one ICSSG, each with its own firmware, forms one
Ethernet controller, which is not quite a hardware device, but also not a fully virtual software
device.

The Ethernet controller only exists through the various ICSS subcores, so it doesn't have an MMIO
address of its own. As described, the existing Device Trees define it as a toplevel non-MMIO node;
we propose to allow it as a non-MMIO child node of the ICSSG container instead.

If you consider moving the ethernet node into the ICSSG node a bad approach, we will drop this patch
and try to find a different solution to our issue (the Ethernet device staying in deferred state
forever when the ICSSG node is disabled on Linux).

Best regards,
Matthias



> 
> > 
> > - It makes sense semantically - the Ethernet controller is running on
> >   the ICSSG/PRUSS
> > - Disabling or deleting the ICSSG node implicitly removes the Ethernet
> >   controller node when it is a child node. This can be relevant on SoCs
> >   like the AM64x which come in variants with and without ICSSG; e.g., on
> >   the TQMa64xxL the ICSSG node will be disabled on variants without as a
> >   bootloader fixup.
> >   On Linux, this avoids leaving the Ethernet controller in deferred
> >   state forever while waiting for the ICSSG to become available
> >   (resulting in a warning on newer kernels)
> > 
> > The node name "ethernet" is chosen as it nicely matches the regular
> > "ethernet@<reg>" format of many Ethernet controller nodes, and is also
> > what the prueth binding example (/schemas/net/ti,icssg-prueth.yaml) uses.
> > 
> > Signed-off-by: Matthias Schiffer <matthias.schiffer@...tq-group.com>
> > ---
> >  Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml b/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
> > index c402cb2928e89..89dfcf5ce8434 100644
> > --- a/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
> > +++ b/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
> > @@ -92,6 +92,13 @@ properties:
> >      description: |
> >        This property is as per sci-pm-domain.txt.
> >  
> > +  ethernet:
> > +    description: |
> 
> Do not need '|' unless you need to preserve formatting.
> 
> > +      ICSSG PRUSS Ethernet. Configuration for an Ethernet controller running
> > +      on the PRU-ICSS.
> > +    $ref: /schemas/net/ti,icssg-prueth.yaml#
> > +    type: object
> > +
> >  patternProperties:
> >  
> >    memories@[a-f0-9]+$:
> 
> You are mixing MMIO and non-MMIO nodes. That's odd or even sloppy
> design. It immediately raises questions about your bindings.
> 
> Best regards,
> Krzysztof
> 

-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
https://www.tq-group.com/