dsa driver for mv88e61xx
Vladimir Oltean
olteanv at gmail.com
Fri Mar 11 00:18:55 CET 2022
On Thu, Mar 10, 2022 at 02:35:21PM -0800, Tim Harvey wrote:
> On Thu, Mar 10, 2022 at 8:50 AM Vladimir Oltean <olteanv at gmail.com> wrote:
> >
> > Hello Tim,
> >
> > On Thu, Mar 10, 2022 at 08:16:13AM -0800, Tim Harvey wrote:
> > > Greetings,
> > >
> > > I wanted to take a stab at adding dsa support for the mv88e61xx which
> > > currently has a driver in drivers/net/phy [1]. The board I have
> > > available to me is the gw5904 which has a mv88e6085 with the upstream
> > > port connected to the IMX6 FEC MAC over RGMII. This currently works
> > > with the existing mv88e61xx driver with the following defined in
> > > gwventana_gw5904_defconfig:
> > >
> > > CONFIG_MV88E61XX_SWITCH=y
> > > CONFIG_MV88E61XX_CPU_PORT=5
> > > CONFIG_MV88E61XX_PHY_PORTS=0xf
> > > CONFIG_MV88E61XX_FIXED_PORTS=0x0
> > >
> > > The device-tree is arch/arm/dts/imx6qdl-gw5904.dtsi [2] which I
> > > believe is proper and works in Linux with the Linux driver in
> > > drivers/net/dsa/mv88e6xxx [3].
> > >
> > > &fec {
> > > pinctrl-names = "default";
> > > pinctrl-0 = <&pinctrl_enet>;
> > > phy-mode = "rgmii-id";
> > > phy-reset-gpios = <&gpio1 30 GPIO_ACTIVE_LOW>;
> > > phy-reset-duration = <10>;
> > > phy-reset-post-delay = <100>;
> > > status = "okay";
> > >
> > > fixed-link {
> > > speed = <1000>;
> > > full-duplex;
> > > };
> > >
> > > mdio {
> > > #address-cells = <1>;
> > > #size-cells = <0>;
> > >
> > > switch at 0 {
> > > compatible = "marvell,mv88e6085";
> > > reg = <0>;
> > >
> > > ports {
> > > #address-cells = <1>;
> > > #size-cells = <0>;
> > >
> > > port at 0 {
> > > reg = <0>;
> > > label = "lan4";
> > > };
> > >
> > > port at 1 {
> > > reg = <1>;
> > > label = "lan3";
> > > };
> > >
> > > port at 2 {
> > > reg = <2>;
> > > label = "lan2";
> > > };
> > >
> > > port at 3 {
> > > reg = <3>;
> > > label = "lan1";
> > > };
> > >
> > > port at 5 {
> > > reg = <5>;
> > > label = "cpu";
> > > ethernet = <&fec>;
> > > };
> > > };
> > > };
> > > };
> > > };
> > >
> > > My motivation for doing this is to be able to drop
> > > gwventana_gw5904_defconfig as it is the same defconfig as
> > > gwventana_emmc_defconfig with the switch added and with get_phy_id
> > > overridden by the current mv88e61xx driver that config won't work with
> > > boards that lack the switch.
> > >
> > > My first approach was to just put a #if !defined(CONFIG_DM_DSA) around
> > > mv88e61xx get_phy_id and add a skeleton driver with an of_match of
> > > compatible = "marvell,mv88e6085" but the driver does not probe with
> > > the above dt fragment.
> > >
> > > Any ideas why the driver won't probe and advise on how I should
> > > proceed with this? I'm not clear yet if I can just modify the existing
> > > driver or if I should create a new one.
> > >
> > > Best Regards,
> > >
> > > Tim
> > > [1] https://elixir.bootlin.com/u-boot/latest/source/drivers/net/phy/mv88e61xx.c
> > > [2] https://elixir.bootlin.com/u-boot/latest/source/arch/arm/dts/imx6qdl-gw5904.dtsi
> > > [3] https://elixir.bootlin.com/linux/latest/source/drivers/net/dsa/mv88e6xxx/chip.c
> >
> > I spent about 3 minutes looking at mv88e61xx.c, so take what I'm about
> > to say with a grain of salt.
> >
> > The biggest issue with reusing that code seems to be that it uses struct
> > phy_device throughout. A DM_DSA driver would need to work around a
> > struct udevice. I'd probably create a new driver, make it easy for
> > others to use, then delete old driver. I see there are 5 occurrences of
> > CONFIG_MV88E61XX_PHY_PORTS in defconfigs, all added within the past 4
> > years. One is yours. Could have been a lot worse.
> >
> > As to why your driver is not probing. I think the fec_mxc parent MDIO
> > bus must be a udevice as well, registered using DM_MDIO?
>
> Vladimir,
>
> Ok, I see that even though fec_mxc is DM_ETH it doesn't use DM_MDIO. I
> suppose my ksz9477 dsa driver probed because that switch was hanging
> off I2C and DM_I2C was in use as opposed to this time the switch is
> hanging off of MDIO.
>
> With changes to fec_mxc to use DM_MDIO/UCLASS_MDIO for the its mdio
> bus, I see my UCLASS_MDIO bus getting probed and registering the bus
> but the call to dm_eth_phy_connect(fec) fails.
>
> For the situation I have where the fec_mxc provides the MDIO bus
> connected to the switch as well as the MAC that's connected via rgmii
> to the upstream port I believe I need a 'phy-handle' in my fec node vs
> a 'fixed-link' subnode otherwise dm_eth_connect_phy_handle() won't
> probe my mdio device. But then I find that dm_eth_phy_connect(fec)
> calls dm_eth_connect_phy_handle() which probes my UCLASS_MDIO but the
> call to dm_mdio_phy_connect() fails and my UCLASS_DSA device never
> gets probed.
>
> So I guess I'm confused if I should be using fixed-link or not and if
> so, how does the UCLASS_MDIO device get probed?
You should definitely use a fixed-link and not a phy-handle to describe
the MAC-to-MAC connection between the FEC and the switch's CPU port.
If the MDIO bus udevice cannot get created/bound to a driver in lack of
a compatible string, maybe you need to force a call to
device_bind_driver_to_node() from the FEC driver for its "mdio" subnode?
>
> &fec {
> pinctrl-names = "default"
> #if 1 // required for dm_eth_connect_phy_handle to probe the UCLASS_MDIO device
> phy-handle = "switch";
> #endif
> phy-mode = "rgmii-id";
> status = "okay";
>
> #if 0 // if present phy_connect is called immediate in
> dm_eth_connect_phy_handle and the UCLASS_MDIO device is not probed
> fixed-link {
> speed = <1000>;
> full-duplex;
> };
> #endif
>
> mdio {
> #address-cells = <1>;
> #size-cells = <0>;
>
> switch: switch at 0 {
> compatible = "marvell,mv88e6085";
> reg = <0>;
>
> ports {
> #address-cells = <1>;
> #size-cells = <0>;
>
> port at 0 {
> reg = <0>;
> label = "lan4";
> phy-mode = "internal";
> };
>
> ...
> port at 5 {
> reg = <5>;
> label = "cpu";
> ethernet = <&fec>;
> phy-mode = "rgmii-id";
>
> fixed-link {
> speed = <1000>;
> full-duplex;
> };
> };
> };
> };
> };
> };
>
> Best regards,
>
> Tim
More information about the U-Boot
mailing list