Re: Possible bug into DSA2 code.

[Andrew Lunn <andrew@lunn.ch>]

On Wed, Feb 20, 2019 at 08:54:01AM +0100, Rodolfo Giometti wrote:
> On 19/02/2019 01:03, Andrew Lunn wrote:
> >>diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> >>index 54a5b660640a..bb46ebbb2bb8 100644
> >>--- a/drivers/net/dsa/mv88e6xxx/chip.c
> >>+++ b/drivers/net/dsa/mv88e6xxx/chip.c
> >>@@ -2399,7 +2399,7 @@ static int mv88e6xxx_setup(struct dsa_switch *ds)
> >>         int i;
> >>
> >>         chip->ds = ds;
> >>-       ds->slave_mii_bus = mv88e6xxx_default_mdio_bus(chip);
> >>+       ds->slave_mii_bus = NULL;
> >>
> >>         mutex_lock(&chip->reg_lock);
> >>
> >>Is that right?
> >
> >Hi Rodolfo
> >
> >Humm, that needs testing. There are two used combinations you need to
> >test:
> >
> >No MDIO node in device tree, e.g.
> >arch/arm/boot/dts/kirkwood-dir665.dts
> >
> >MDIO node in device tree, e.g:
> >arch/arm/boot/dts/vf610-zii-dev-rev-b.dts
> 
> I'm sorry but I haven't such boards... :'(

Hi Rodolfo

I know. But you should be able to modify the device tree of the boards
you are using to follow these patterns, and test them.

> 
> >There is a third combination which will appear soon. There is only the
> >external mdio bus in device tree:
> >
> >                                 port@9 {
> >                                         reg = <9>;
> >                                         label = "eth_cu_1000_2";
> >                                         phy-handle = <&phy9>;
> >                                         phy-mode = "sgmii";
> >                                         managed = "in-band-status";
> >                                 };
> >                         };
> >
> >                         mdio1 {
> >                                 compatible = "marvell,mv88e6xxx-mdio-external";
> >                                 #address-cells = <1>;
> >                                 #size-cells = <0>;
> >
> >                                 phy9: phy9@0 {
> >                                         compatible = "ethernet-phy-ieee802.3-c45";
> >                                         pinctrl-0 = <&pinctrl_gpio_phy9>;
> >                                         pinctrl-names = "default";
> >                                         interrupt-parent = <&gpio2>;
> >                                         interrupts = <30 IRQ_TYPE_LEVEL_LOW>;
> >                                         reg = <0>;
> >                                 };
> >                         };
> >
> >Here port 9 uses the external MDIO bus and all the other ports
> >implicitly make use of the internal MDIO bus.
> 
> Mmm... maybe should code into drivers/net/dsa/mv88e6xxx use private data to
> hold used mdio busses instead of ds->slave_mii_bus pointer?

I suspend that might break some of these different use cases.

Either there needs to be a well argued analysis, or some testing of
these setups.

      Andrew