netdev - Re: [PATCH net-next 09/13] dt-bindings: net: airoha: Add airoha,npu phandle property

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: <20250206-charbroil-armhole-078069994058@spud>
Date: Thu, 6 Feb 2025 18:24:40 +0000
From: Conor Dooley <conor@...nel.org>
To: Lorenzo Bianconi <lorenzo.bianconi@...hat.com>
Cc: Lorenzo Bianconi <lorenzo@...nel.org>,
	Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Felix Fietkau <nbd@....name>, Sean Wang <sean.wang@...iatek.com>,
	Matthias Brugger <matthias.bgg@...il.com>,
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
	Philipp Zabel <p.zabel@...gutronix.de>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>, netdev@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org,
	linux-mediatek@...ts.infradead.org, devicetree@...r.kernel.org,
	upstream@...oha.com
Subject: Re: [PATCH net-next 09/13] dt-bindings: net: airoha: Add airoha,npu
 phandle property

On Wed, Feb 05, 2025 at 09:52:14PM +0100, Lorenzo Bianconi wrote:
> > On Wed, Feb 05, 2025 at 08:33:13PM +0100, Lorenzo Bianconi wrote:
> > > > On Wed, Feb 05, 2025 at 07:21:28PM +0100, Lorenzo Bianconi wrote:
> > > > > Introduce the airoha,npu property for the npu syscon node available on
> > > > > EN7581 SoC. The airoha Network Processor Unit (NPU) is used to offload
> > > > > network traffic forwarded between Packet Switch Engine (PSE) ports.
> > > > > 
> > > > > Signed-off-by: Lorenzo Bianconi <lorenzo@...nel.org>
> > > > > ---
> > > > >  Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml | 8 ++++++++
> > > > >  1 file changed, 8 insertions(+)
> > > > > 
> > > > > diff --git a/Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml b/Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml
> > > > > index c578637c5826db4bf470a4d01ac6f3133976ae1a..6388afff64e990a20230b0c4e58534aa61f984da 100644
> > > > > --- a/Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml
> > > > > +++ b/Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml
> > > > > @@ -63,6 +63,12 @@ properties:
> > > > >    "#size-cells":
> > > > >      const: 0
> > > > >  
> > > > > +  airoha,npu:
> > > > > +    $ref: /schemas/types.yaml#/definitions/phandle
> > > > > +    description:
> > > > > +      Phandle to the syscon node used to configure the NPU module
> > > > > +      used for traffic offloading.
> > > > 
> > > > Why do you need a phandle for this, instead of a lookup by compatible?
> > > > Do you have multiple instances of this ethernet controller on the
> > > > device, that each need to look up a different npu?
> > > 
> > > actually not, but looking up via the compatible string has been naked by
> > > Krzysztof on a different series [0].
> > 
> > Hmm, I disagree with adding phandles that are not needed. I don't agree
> > that there's no reuse, if you can treat the phandle identically on a new
> > device, in all likelihood, that node should have a fallback to the
> > existing one.
> 
> honestly I do not have a strong opinion about it, I am fine both ways.
> 
> > 
> > That said, the bigger problem in what you link is the ABI break caused by
> > requiring the presence of a new node. I'd NAK that patch too.
> 
> there is no PCIe support in dts for this device upstream yet, so I guess there
> is no ABI break, right?

The binding and driver are upstream and therefore there could be users.
Whether or not the dts is upstream isn't what determines ABI break or not!
If the thing worked before adding the syscon in the PCIe patch you'd linked,
the syscon should remain optional - unless there really are no users
that will be impacted. If it didn't work without the syscon, then I
wonder why it was upstreamed like that at all.

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)