On Thu, May 19, 2022 at 09:15:00AM -0700, Colin Foster wrote:

Hi Vladimir,

On Thu, May 19, 2022 at 02:44:41PM +0000, Vladimir Oltean wrote:

quoted

Hi Colin,

On Sat, May 14, 2022 at 03:00:10PM -0700, Colin Foster wrote:

quoted

On Mon, May 09, 2022 at 05:13:05PM +0000, Vladimir Oltean wrote:

quoted

Hi Colin,

On Sun, May 08, 2022 at 11:52:57AM -0700, Colin Foster wrote:

quoted

		mdio0: mdio0@0 {

This is going to be interesting. Some drivers with multiple MDIO buses
create an "mdios" container with #address-cells = <1> and put the MDIO
bus nodes under that. Others create an "mdio" node and an "mdio0" node
(and no address for either of them).

The problem with the latter approach is that
Documentation/devicetree/bindings/net/mdio.yaml does not accept the
"mdio0"/"mdio1" node name for an MDIO bus.

I'm starting this implementation. Yep - it is interesting.

A quick grep for "mdios" only shows one hit:
arch/arm64/boot/dts/freescale/fsl-lx2160a-bluebox3.dts

While that has an mdios field (two, actually), each only has one mdio
bus, and they all seem to get parsed / registered through
sja1105_mdiobus_.*_register.


Is this change correct (I have a feeling it isn't):

ocelot-chip@0 {
    #address-cells = <1>;
    #size-cells = <0>;

    ...

    mdio0: mdio@0 {
        reg=<0>;
        ...
    };

    mdio1: mdio@1 {
        reg = <1>;
        ...
    };
    ...
};

When I run this with MFD's (use,)of_reg, things work as I'd expect. But
I don't directly have the option to use an "mdios" container here
because MFD runs "for_each_child_of_node" doesn't dig into
mdios->mdio0...

Sorry for the delayed response. I think you can avoid creating an
"mdios" container node, but you need to provide some "reg" values based
on which the MDIO controllers can be distinguished. What is your convention
for "reg" values of MFD cells? Maybe pass the base address/size of this
device's regmap as the "reg", even if the driver itself won't use it?

No worries. Everyone is busy.

Right now it looks like this:

}, {
    .name = "ocelot-miim0",
    .of_compatible = "mscc,ocelot-miim",
    .of_reg = 0,
    .use_of_reg = true,
    .num_resources = ARRAY_SIZE(vsc7512_miim0_resources),
    .resources = vsc7512_miim0_resources,
}, {
    .name = "ocelot-miim1",
    .of_compatible = "mscc,ocelot-miim",
    .num_resources = ARRAY_SIZE(vsc7512_miim1_resources),
    .of_reg = 1,
    .use_of_reg = true,
    .resources = vsc7512_miim1_resources,
}, {

"0" and "1" being somewhat arbitrary... although they are named as such
in the datasheet.


So you're thinking it might look more like:

.of_reg = vsc7512_miim0_resources[0].start,

and the device tree would be:

mdio0: mdio@0x7107009c {
    reg = <0x7107009c>;
};

Yeah, this is what I was thinking.

I could see that making sense. The main thing I don't like is applying
the address-cells to every peripheral in the switch. It seems incorrect
to have:

switch {
    address-cells = <1>;
    mdio0: mdio@7107009c {
        reg = <0x7107009c>;
    };
    gpio: pinctrl {
        /* No reg parameter */
    };
};

That's what I currently have. To my surprise it actually doesn't throw
any warnings, which I would've expected.

I tried mangling some device trees and indeed it looks like dtc won't
warn, but I still think it's invalid to mix node address conventions
with the same #address-cells. Maybe if that wasn't the case things would
be easier.

I could see either 0/1 or the actual base addresses making sense.
Whichever you'd suggest.

The idea with putting the actual base addresses was that you could then
do that for all cells, like the pinctrl node, too, and they'd have a
coherent meaning.

I've got another day or two to button things up, so it looks like I
missed the boat for this release. This should be ready to go on day 1
after the window.