[PATCH v3 8/8] board: gw_ventana: enable MV88E61XX DSA support

[Vladimir Oltean]

On Fri, Jun 24, 2022 at 04:16:34PM -0700, Tim Harvey wrote:
> On Fri, Jun 24, 2022 at 3:25 AM Vladimir Oltean <vladimir.oltean at nxp.com> wrote:
> >
> > On Mon, May 23, 2022 at 11:25:49AM -0700, Tim Harvey wrote:
> > > Add MV88E61XX DSA support:
> > >  - update dt: U-Boot dsa driver requires different device-tree syntax
> > >    than the linux driver in order to link the dsa ports to the mdio bus.
> > >  - update defconfig
> > >  - replace mv88e61xx_hw_reset weak override with board_phy_config support
> > >    for mv88e61xx configuration that is outside the scope of the DSA driver
> > >
> > > Signed-off-by: Tim Harvey <tharvey at gateworks.com>
> > > ---
> > > v3:
> > >  - move mdio's mdio at 0 subnode
> > > v2: no changes
> > > ---
> > >  arch/arm/dts/imx6qdl-gw5904.dtsi        | 41 ++++++++++++++++++++
> > >  board/gateworks/gw_ventana/gw_ventana.c | 50 +++++++++----------------
> > >  configs/gwventana_gw5904_defconfig      |  7 ++--
> > >  3 files changed, 62 insertions(+), 36 deletions(-)
> > >
> > > diff --git a/arch/arm/dts/imx6qdl-gw5904.dtsi b/arch/arm/dts/imx6qdl-gw5904.dtsi
> > > index 286c7a9924c2..1b2f70d1ccb2 100644
> > > --- a/arch/arm/dts/imx6qdl-gw5904.dtsi
> > > +++ b/arch/arm/dts/imx6qdl-gw5904.dtsi
> > > @@ -219,6 +219,33 @@
> > >                       compatible = "marvell,mv88e6085";
> > >                       reg = <0>;
> > >
> > > +                     mdios {
> > > +                             #address-cells = <1>;
> > > +                             #size-cells = <0>;
> > > +
> > > +                             mdio at 0 {
> >
> > If you are going to follow this new model with a dedicated "mdios" subnode,
> > I've consulted with Andrew Lunn and Florian Fainelli and there shouldn't
> > be a problem to later make Linux accept this alternate binding format.
> > But in that case, please match this OF node by a dedicated compatible
> > string, like "marvell,mv88e6xxx-mdio-internal", so that there will be a
> > way to differentiate this from the existing "marvell,mv88e6xxx-mdio-external"
> > when support for that is added in U-Boot.
> >
> > Alternatively, to repeat myself, you can always follow the de-facto
> > bindings for Linux mv88e6xxx which have:
> >
> >                 switch0: switch0 at 0 {
> >                         compatible = "marvell,mv88e6190";
> >
> >                         ports {
> >                                 #address-cells = <1>;
> >                                 #size-cells = <0>;
> >
> >                                 ...
> >                         };
> >
> >                         mdio { // internal
> >                                 #address-cells = <1>;
> >                                 #size-cells = <0>;
> >
> >                                 ...
> >                         };
> >
> >                         mdio1 {
> >                                 compatible = "marvell,mv88e6xxx-mdio-external";
> >                                 #address-cells = <1>;
> >                                 #size-cells = <0>;
> >
> >                                 ...
> >                         };
> >                 };
> >
> 
> Documentation/devicetree/bindings/net/dsa/marvell.txt shows en example
> with just one child node under the internal mdio node:
> 
>                         mdio {
>                                 #address-cells = <1>;
>                                 #size-cells = <0>;
>                                 switch1phy0: switch1phy0 at 0 {
>                                         reg = <0>;
>                                         interrupt-parent = <&switch0>;
>                                         interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
>                                 };
>                         };
> 
> Am I to assume I can add additional nodes there for the other ports
> such as the following?
> 
>                         mdio {
>                                 #address-cells = <1>;
>                                 #size-cells = <0>;
> 
>                                 switch1phy0: switch1phy0 at 0 {
>                                         reg = <0>;
>                                 };
> 
>                                 switch1phy1: switch1phy1 at 1 {
>                                         reg = <1>;
>                                 };
> 
>                                 switch1phy2: switch1phy2 at 2 {
>                                         reg = <2>;
>                                 };
> 
>                                 switch1phy3: switch1phy3 at 3 {
>                                         reg = <3>;
>                                 };
> 
>                                 ...
>                     };

Sure, but name those PHY nodes "ethernet-phy at N" rather than "switchMphyN",
as Documentation/devicetree/bindings/net/ethernet-phy.yaml requires.
Many mistakes were made in writing mv88e6xxx device trees, we don't need
to follow each and every one of them, only the important ones.