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

[Tim Harvey]

On Fri, Jun 24, 2022 at 5:13 PM Vladimir Oltean <vladimir.oltean at nxp.com> wrote:
>
> 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.

Ok, I'll attempt to go that route. I'm out of the office next week so
expect a v4 sometime after that.

Best Regards,

Tim