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;
Date:   Sat, 8 Oct 2022 01:48:08 +0300
From:   Vladimir Oltean <olteanv@gmail.com>
To:     Colin Foster <colin.foster@in-advantage.com>
Cc:     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>
Subject: Re: [PATCH v3 net-next 12/14] dt-bindings: net: dsa: ocelot: add
 ocelot-ext documentation
Message-ID: <20221007224808.dgksesjbiptwmqj7@skbuf>
References: <20220926002928.2744638-1-colin.foster@in-advantage.com>
 <20220926002928.2744638-13-colin.foster@in-advantage.com>
 <20220927202600.hy5dr2s6j4jnmfpg@skbuf>
 <YzN3P6NaDhjA1Qrk@colin-ia-desktop>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YzN3P6NaDhjA1Qrk@colin-ia-desktop>
Precedence: bulk

On Tue, Sep 27, 2022 at 03:20:47PM -0700, Colin Foster wrote:
> > The mfd driver can use these resources or can choose to ignore them, but
> > I don't see a reason why the dt-bindings should diverge from vsc7514,
> > its closest cousin.
> 
> This one I can answer. (from November 2021). Also I'm not saying that my
> interpretation is correct. Historically when there are things up for
> interpretation, I choose the incorrect path. (case in point... the other
> part of this email)
> 
> https://patchwork.kernel.org/project/netdevbpf/patch/20211125201301.3748513-4-colin.foster@in-advantage.com/#24620755
> 
> '''
> The thing with putting the targets in the device tree is that you're
> inflicting yourself unnecessary pain. Take a look at
> Documentation/devicetree/bindings/net/mscc-ocelot.txt, and notice that
> they mark the "ptp" target as optional because it wasn't needed when
> they first published the device tree, and now they need to maintain
> compatibility with those old blobs. To me that is one of the sillier
> reasons why you would not support PTP, because you don't know where your
> registers are. And that document is not even up to date, it hasn't been
> updated when VCAP ES0, IS1, IS2 were added. I don't think that Horatiu
> even bothered to maintain backwards compatibility when he initially
> added tc-flower offload for VCAP IS2, and as a result, I did not bother
> either when extending it for the S0 and S1 targets. At some point
> afterwards, the Microchip people even stopped complaining and just went
> along with it. (the story is pretty much told from memory, I'm sorry if
> I mixed up some facts). It's pretty messy, and that's what you get for
> creating these micro-maps of registers spread through the guts of the
> SoC and then a separate reg-name for each. When we worked on the device
> tree for LS1028A and then T1040, it was very much a conscious decision
> for the driver to have a single, big register map and split it up pretty
> much in whichever way it wants to. In fact I think we wouldn't be
> having the discussion about how to split things right now if we didn't
> have that flexibility.
> '''
> 
> I'm happy to go any way. The two that make the most sense might be:
> 
> micro-maps to make the VSC7512 "switch" portion match the VSC7514. The
> ethernet switch portion might still have to ignore these...
> 
> A 'mega-map' that would also be ignored by the switch. It would be less
> arbitrary than the <0 0> that I went with. Maybe something like
> <0x70000000 0x02000000> to at least point to some valid region.

A mega-map for the switch makes a lot more sense to me, if feasible
(it should not overlap with the regions of any other peripherals).
Something isn't quite right to me in having 20 reg-names for a single
device tree node, and I still stand for just describing the whole range
and letting the driver split it up according to its needs. I don't know
why this approach wasn't chosen for the ocelot switch and I did not have
the patience to map out the addresses that the peripherals use in the
Microchip SoCs relative to each other, so see if what I'm proposing is
possible.

But on the other hand this also needs to be balanced with the fact that
one day, someone might come along with a mscc,vsc7514-switch that's SPI
controlled, and expect that the dt-bindings for it in DSA mode expect
the same reg-names that they do in switchdev mode. Or maybe they
wouldn't expect that, I don't know. In any case, for NXP switches I
didn't have a compatibility issue with switchdev-mode Ocelot to concern
myself with, so I went with what made the most sense.