Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <c9ce1d83-d1ca-4640-bba2-724e18e6e56b@linaro.org>
Date:   Sun, 9 Oct 2022 12:14:22 -0400
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.3.1
Subject: Re: [PATCH v3 net-next 12/14] dt-bindings: net: dsa: ocelot: add
 ocelot-ext documentation
Content-Language: en-US
To:     Vladimir Oltean <olteanv@gmail.com>
Cc:     Colin Foster <colin.foster@in-advantage.com>,
        linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
        devicetree@vger.kernel.org, netdev@vger.kernel.org,
        Russell King <linux@armlinux.org.uk>,
        Linus Walleij <linus.walleij@linaro.org>,
        UNGLinuxDriver@microchip.com,
        Alexandre Belloni <alexandre.belloni@bootlin.com>,
        Claudiu Manoil <claudiu.manoil@nxp.com>,
        Lee Jones <lee@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Rob Herring <robh+dt@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Eric Dumazet <edumazet@google.com>,
        "David S. Miller" <davem@davemloft.net>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Vivien Didelot <vivien.didelot@gmail.com>,
        Andrew Lunn <andrew@lunn.ch>
References: <20220926002928.2744638-1-colin.foster@in-advantage.com>
 <20220926002928.2744638-13-colin.foster@in-advantage.com>
 <ec63b5aa-3dec-3c27-e987-25e36b1632ba@linaro.org>
 <YzzLCYHmTcrHbZcH@colin-ia-desktop>
 <455e31be-dc87-39b3-c7fe-22384959c556@linaro.org>
 <Yz2mSOXf68S16Xg/@colin-ia-desktop>
 <28b4d9f9-f41a-deca-aa61-26fb65dcc873@linaro.org>
 <20221008000014.vs2m3vei5la2r2nd@skbuf>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <20221008000014.vs2m3vei5la2r2nd@skbuf>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 08/10/2022 02:00, Vladimir Oltean wrote:
> 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?

By IO I meant MMIO (or similar) which resides in reg (thus in unit
address). The SPI devices have only chip-select as reg, AFAIR.

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

So probably I misunderstood here. If this is I2C or SPI device, then of
course reg and unit address do not represent registers.

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

So what is @710700c0? It's not chip-select, but MMIO or some other bus
(specific to the device), right?

The mscc,ocelot.yaml has a soc@0 SPI device. Children of soc@0 use unit
addresses/reg meaningful for that soc@0.

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

Yeah, at least to my limited knowledge of this hardware.


> 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"? 

Describe the hardware properties in terms of it fit in to the whole
system - so some inputs (clocks, GPIOs), outputs (interrupts),
characteristics of a device (e.g. clock provider -> clock cells) and
properties configuring hardware per specific implementation.

But mostly this argument "describe hardware" should be understood like:
do not describe software (Linux drivers) and software policies (driver
choices)...

> Verilog/VHDL is what the hardware description that's
> independent of software implementation is, good luck parsing that.

Best regards,
Krzysztof