dsa driver for mv88e61xx
Tim Harvey
tharvey at gateworks.com
Fri Mar 11 17:41:48 CET 2022
On Thu, Mar 10, 2022 at 3:18 PM Vladimir Oltean <olteanv at gmail.com> wrote:
>
> 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?
>
Yes, the FEC MAC's internal MDIO lacks its own compatible string so I
can in the fec driver bind the MDIO driver to the mdio node and probe
it via uclass_get_device_by_ofnode(UCLASS_MDIO, node, &mdiodev).
Now the UCLASS_MDIO driver is probed and it works with boards that
have MAC->PHY (non-fixed PHY's) but for a MAC<->MAC fixed-link
phy_connect calls phy_connect_fixed with a null bus and does not scan
the mdio bus and the dsa driver still does not probe. I'm still back
to not understanding how to get the dsa switch driver to probe when
it's a child of an mdio node.
Maybe the answer is similar to your above answer in that I need to
make the fec driver continue to probe all children in the mdio bus
node. I feel like this is quite a burden on the mac driver and that
functionality should be handled in uclass-mdio somehow? I'm not sure
if there is a push to convert MDIO drivers to UCLASS_MDIO. It seems
like there is only a handful compared to all the drivers that call
mdio_register so perhaps this issue just hasn't come up yet. Are we
sure that in order to use UCLASS_DSA you must have a UCLASS_MDIO mdio
bus?
Best Regards,
Tim
> >
> > &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