Message-ID: <7adcd6789fb33fef10b7349934374e2cfb5ad164.camel@ew.tq-group.com>
Date: Thu, 20 Jun 2024 10:48:59 +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 10:29 +0200, Krzysztof Kozlowski wrote:
> 
> 
> On 20/06/2024 10:26, Matthias Schiffer wrote:
> > 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.
> 
> So it is not really child of ICSSG.
> 
> > 
> > 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).
> 
> Just disable the ethernet. That's the expected behavior, I don't get
> what is the problem here.

If the disabling happens as a fixup in the bootloader, it needs to know the name of the Ethernet
controller node (or iterate through the DTB to find references to the disabled ICSSG node).

The name is currently not used for anything, and not specified in the binding doc; the example uses
"ethernet", which is too unspecific, as there can be multiple ICSSG/PRUs, with each running a
separate Ethernet controller.

Existing Device trees use "icssgX-eth" for an Ethernet controller running on the ICSSG with label
"&icssgX", but labels are a source concept and don't exist in the compiled DTB by default.

I do have an idea for an alternative approach that does not need changes to the DT bindings: The PRU
Ethernet driver could detect that the referenced ti,prus are disabled and not just waiting to be
probed and then fail with ENODEV instead of EPROBE_DEFER.

Best regards,
Matthias


> 
> 
> 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/

Powered by blists - more mailing lists