Re: [PATCH v3 net-next 12/14] dt-bindings: net: dsa: ocelot: add ocelot-ext documentation

[Vladimir Oltean]

On Wed, Oct 05, 2022 at 06:09:59PM +0200, Krzysztof Kozlowski wrote:
> >> I don't understand how your answer relates to "reg=<0 0>;". How is it
> >> going to become 0x71010000 if there is no other reg/ranges set in parent
> >> nodes. The node has only one IO address, but you say the switch has 20
> >> addresses...
> >>
> >> Are we talking about same hardware?
> > 
> > Yes. The switch driver for both the VSC7512 and VSC7514 use up to ~20 regmaps
> > depending on what capabilities it is to have. In the 7514 they are all
> > memory-mapped from the device tree. While the 7512 does need these
> > regmaps, they are managed by the MFD, not the device tree. So there
> > isn't a _need_ for them to be here, since at the end of the day they're
> > ignored.
> > 
> > The "reg=<0 0>;" was my attempt to indicate that they are ignored, but I
> > understand that isn't desired. So moving forward I'll add all the
> > regmaps back into the device tree.
> 
> You need to describe the hardware. If hardware has IO address space, how
> does it matter that some driver needs or needs not something?

What do you mean by IO address space exactly? It is a SPI device with registers.
Does that constitute an IO address space to you?

The driver need matters because you don't usually see DT nodes of SPI,
I2C, MDIO devices describing the address space of their registers, and
having child nodes with unit addresses in that address space. Only when
those devices are so complex that the need arises to identify smaller
building blocks is when you will end up needing that. And this is an
implementation detail which shapes how the dt-bindings will look like.

> You mentioned that address space is mapped to regmaps. Regmap is Linux
> specific implementation detail, so this does not answer at all about
> hardware.
>
> On the other hand, if your DTS design requires this is a child of
> something else and by itself it does not have address space, it would be
> understandable to skip unit address entirely... but so far it is still
> confusing, especially that you use arguments related to implementation
> to justify the DTS.

If Colin skips the unit address entirely, then how could he distinguish
between the otherwise identical MDIO controllers mdio@7107009c and
mdio@710700c0 from Documentation/devicetree/bindings/mfd/mscc,ocelot.yaml?
The ethernet-switch node added here is on the same hierarchical level
with the MDIO controller nodes, so it must have a unit address just like
them.

But I don't support Colin's choice of "reg=<0 0>;" either. A choice must
be made between 2 options:
- mapping all 20 regions of the SPI address space into "reg" values
- mapping a single region from the smallest until the largest address of
  those 20, and hope nothing overlaps with some other peripheral, or
  worse, that this region will never need to be expanded to the left.

What information do you need to provide some best practices that can be
applied here and are more useful than "you need to describe the
hardware"? Verilog/VHDL is what the hardware description that's
independent of software implementation is, good luck parsing that.