netdev - Re: [PATCH net-next] dt-bindings: net: marvell,orion-mdio: Drop "reg" sizes schema

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]

Date: Fri, 15 Dec 2023 11:18:22 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Rob Herring <robh@...nel.org>
Cc: Conor Dooley <conor@...nel.org>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
	Conor Dooley <conor+dt@...nel.org>, netdev@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next] dt-bindings: net: marvell,orion-mdio: Drop
 "reg" sizes schema

On Thu, Dec 14, 2023 at 12:12:42PM -0600, Rob Herring wrote:
> On Thu, Dec 14, 2023 at 10:23 AM Conor Dooley <conor@...nel.org> wrote:
> >
> > On Wed, Dec 13, 2023 at 05:24:55PM -0600, Rob Herring wrote:
> > > Defining the size of register regions is not really in scope of what
> > > bindings need to cover. The schema for this is also not completely correct
> > > as a reg entry can be variable number of cells for the address and size,
> > > but the schema assumes 1 cell.
> > >
> > > Signed-off-by: Rob Herring <robh@...nel.org>
> >
> > Does this not also remove restrictions on what the number in the reg
> > entry is actually allowed to be?
> 
> Yes, that's what I mean with the first sentence. We don't do this
> anywhere else with the exception of some I2C devices with fixed
> addresses. Keying off of the interrupt property also seems
> questionable. If the register size is different, that should be a
> different compatible.

Reading the code, it appears the hardware always supported interrupts,
however the first version of the driver never used them. It seems like
some DT blobs had the register space cover just the needed registers
for polling, and excluded the interrupt control register. When
interrupt support was added, all in-tree DT files were updated with
the extended register space, but to allow backwards compatibility, the
driver checks the length of the register space and will not enable
interrupts if its too small.

I'm guessing that since the hardware did not change, a new compatible
was not used when adding interrupt support. And the yaml is there to
help when old out of tree .dts files are merged into the tree and have
the old register space.

This is and old driver, and its usage of DT is from long before many
of the current best practices where determined, or yaml was even an
idea. So i'm not surprised it has a few odd quirks.

I don't see a reason not to remove these constraints, as i said, the
driver should do the right thing if the register space it too small
and YAML does not warn about it.

      Andrew