On Wed, May 18, 2022 at 7:41 AM Vladimir Oltean <vladimir.olt...@nxp.com> wrote:
>
> On Wed, May 11, 2022 at 05:20:03PM -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 <thar...@gateworks.com>
> > ---
> > v2: no changes
> > ---
> > arch/arm/dts/imx6qdl-gw5904.dtsi | 35 +++++++++++++++++
> > board/gateworks/gw_ventana/gw_ventana.c | 50 +++++++++----------------
> > configs/gwventana_gw5904_defconfig | 7 ++--
> > 3 files changed, 56 insertions(+), 36 deletions(-)
> >
> > diff --git a/arch/arm/dts/imx6qdl-gw5904.dtsi
> > b/arch/arm/dts/imx6qdl-gw5904.dtsi
> > index 286c7a9924c2..63590a2debc7 100644
> > --- a/arch/arm/dts/imx6qdl-gw5904.dtsi
> > +++ b/arch/arm/dts/imx6qdl-gw5904.dtsi
> > @@ -219,6 +219,27 @@
> > compatible = "marvell,mv88e6085";
> > reg = <0>;
> >
> > + mdios {
>
> I'm thinking either "mdios" is a container node for other "mdio" nodes,
> or you have the "mdio" node directly. Are you sure you want this
> structure? We might have problems if we pass this DT as-is up to Linux.
> Whereas the support for an "mdios" subnode could be upstreamed, I think.
> The current DT bindings document says:
>
> - mdio : Container of PHY and devices on the switches MDIO
> bus.
> - mdio? : Container of PHYs and devices on the external MDIO
> bus. The node must contains a compatible string of
> "marvell,mv88e6xxx-mdio-external"
>
> You have an "mdios" which theoretically matches "mdio?" but only
> unintendedly so, since it is the internal MDIO bus and not the external
> one.
>
> Easiest way out right now is to not introduce something which isn't
> parsed by the current Linux driver. So the internal MDIO bus would be
> named "mdio", and the external one would be named whatever, and would be
> identified by compatible string.
>
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + sw_phy0: ethernet-phy@0 {
> > + reg = <0x0>;
> > + };
> > +
> > + sw_phy1: ethernet-phy@1 {
> > + reg = <0x1>;
> > + };
> > +
> > + sw_phy2: ethernet-phy@2 {
> > + reg = <0x2>;
> > + };
> > +
> > + sw_phy3: ethernet-phy@3 {
> > + reg = <0x3>;
> > + };
> > + };
> > +
> > ports {
> > #address-cells = <1>;
> > #size-cells = <0>;
Vladimir,
I'm not sure I follow your suggestion correctly. I agree this is not
ideal and not at all what I wanted. The dt prior to this patch is what
exists and works in Linux but U-Boot dsa (more correctly DM_MDIO
dm_eth_phy_connect()) requires the phy-mode and phy-handle in the
ports and I'm not clear how to define the mdio's for those to point to
here.
Recall I did the same thing for gw7901 in commit 1cb87b929e1e ("arm:
dts: imx8mm-venice-gw7901.dts: fix dsa switch configuration") which I
didn't feel was correct either as the previous dt there is what is in
Linux and works.
Can you give me an example?
Best Regards,
Tim
