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

[Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>]

On 08/10/2022 01:10, Vladimir Oltean wrote:
> On Wed, Oct 05, 2022 at 10:09:06AM +0200, Krzysztof Kozlowski wrote:
>>> The /spi/soc@0 node actually has a compatible of "mscc,vsc7512" which
>>> Colin did not show in the example (it is not "simple-bus"). It is covered
>>> by Documentation/devicetree/bindings/mfd/mscc,ocelot.yaml. Still waiting
>>> for a better suggestion for how to name the mfd container node.
>>
>> Then still the /spi node does not seem related. If I understand
>> correctly, your device described in this bindings is a child of soc@0.
>> Sounds fine. How that soc@0 is connected to the parent - via SPI or
>> whatever - is not related to this binding, is it? It is related to the
>> soc binding, but not here.
> 
> It's an example, it's meant to be informative. It is the first DSA
> driver of its kind. When everybody else ATM puts the ethernet-switch node
> under the &spi controller node, this puts it under &spi/soc@<chip-select>/,
> for reasons that have to do with scalability. If the examples aren't a
> good place to make this more obvious, I don't know why we don't just
> tell people to RTFD.

It still does not help me to understand why do you need that &spi. The
device is part of the soc@CS and that's it. Where the soc@ is located,
does not matter for this device, right? The example shows usage of this
device, not of the soc@CS. Bindings for soc@CS should show how to use it
inside spi etc.


> 
>>> Unrelated to your "existing soc example" (the VSC9953), but relevant and
>>> you may want to share your opinion on this:
>>>
>>> The same hardware present in the VSC7514 SoC can also be driven by an
>>> integrated MIPS processor, and in that case, it is indeed expected that
>>> the same dt-bindings cover both the /soc and the /spi/soc@0/ relative
>>> positioning of their OF node. This is true for simpler peripherals like
>>> "mscc,ocelot-miim", "mscc,ocelot-pinctrl", "mscc,ocelot-sgpio". However
>>> it is not true for the main switching IP of the SoC itself.
>>>
>>> When driven by a switchdev driver, by the internal MIPS processor (the
>>> DMA engine is what is used for packet I/O), the switching IP follows the
>>> Documentation/devicetree/bindings/net/mscc,vsc7514-switch.yaml binding
>>> document.
>>>
>>> When driven by a DSA driver (external processor, host frames are
>>> redirected through an Ethernet port instead of DMA controller),
>>> the switching IP follows the Documentation/devicetree/bindings/net/dsa/mscc,ocelot.yaml
>>> document.
>>>
>>> The switching IP is special in this regard because the hardware is not
>>> used in the same way. The DSA dt-binding also needs the 'ethernet'
>>> phandle to be present in a port node. The different placement of the
>>> bindings according to the use case of the hardware is a bit awkward, but
>>> is a direct consequence of the separation between DSA and pure switchdev
>>> drivers that has existed thus far (and the fact that DSA has its own
>>> folder in the dt-bindings, with common properties in dsa.yaml and
>>> dsa-port.yaml etc). It is relatively uncommon for a switching IP to have
>>> provisioning to be used in both modes, and for Linux to support both
>>> modes (using different drivers), yet this is what we have here.
>>
>> Is there a question here to me? What shall I do with this paragraph? You
>> know, I do not have a problem of lack of material to read...
> 
> For mscc,vsc7514-switch we have a switchdev driver. For mscc,vsc7512-switch,
> Colin is working on a DSA driver. Their dt-bindings currently live in
> different folders. The mscc,vsc7514-switch can also be used together
> with a DSA driver, and support for that will inevitably be added. When
> it will, how and where do you recommend the dt-bindings should be added?

The bindings should in general describe the hardware, not the Linux
drivers. I assume there is only one VSC7514 device, so there should be
only one binding file. If bindings are correct, then this one hardware
description can be used by two different driver implementations. That's
said, for practical reasons entirely different implementations might
require different bindings, but this should be rather exception
requiring serious reasons.

> In net/dsa/mscc,ocelot.yaml, together with the other switches used in
> DSA mode, or in net/mscc,vsc7514-switch.yaml, because its compatible
> string already exists there? We can't have a compatible string present
> in multiple schemas, right?

You can, if bindings are the same... but then why would you have the
same bindings in two files? Which leads to solution: have only one
binding file.

If bindings are entirely different (and not compatible to each other),
you cannot have same compatible in two different places... and this
leads to paragraph before - there should be only one binding, thus only
one place to document the compatible.

> 
> This matters because it has implications upon what Colin should do with
> the mscc,vsc7512-switch. If your answer to my question is "add $ref: dsa.yaml#
> to net/mscc,vsc7514-switch.yaml", then I don't see why we wouldn't do
> that now, and wait until the vsc7514 to make that move anyway.

Best regards,
Krzysztof