DSA and Marvell Switch Drivers

DSA and Marvell Switch Drivers

[Moltmann, Tobias]

Hello,

I would first rewrite my questions and provide some information's and add some of Andrews answer to it as well.
As I learned it would be helpful to address the "netdev list" to get (hopefully) even more help. 
So please feel free to provide some feedback, thank you!

The issue we face comes along with an Kernel upgrade from 4.4.xxx to now 5.10.xx. It is all industrial-based hardware, so 
no classical PC or something. On the hardware we have an x86 CPU, an IGB -> Marvell Switch (Marvell 88E6176) ->
and two PHY's connected there.

Very roughly the way it worked/run with the 4.4 Kernel:

- mv88e6xxx_init() called - registered the driver
- IGB driver loaded, started probing
- within the probing we set up an MDIO bus (name: igb MDIO, id: 0000:01:00.0_mii)
- the libphy does a first scan with no result due to some other missing stuff - at this point it is ok
- also within the igb probing we set up an dsa_platform_data struct and run a platform_device_register()
- this triggers the DSA driver
- mv88e6352_probe() is called our Marvell switch is detected
- a new DSA slave MDIO bus is been brought up automatically
- mdiobus_scan() there register our two PHY devices 
- everything is working :)


Now in the 5.10 Kernel, as there are the rewritten DSA/Marvell drivers included, the upper "stuff" is not working anymore.
So far so good... Due to the fact, that the DSA/Marvell drivers aren't platform driver anymore the trigger with the platform_device_register()
has no effect any more. I suggest we need to end up in the new mv88e6xxx_probe() function in drivers/net/dsa/mv88e6xxx/chip.c.
As there is all the probing, register of DSA stuff included. 
I added some "printf" in the module init parts in the chip.c file as well as in the /net/dsa/dsa.c - and I do see them in the
Kernel logfile after startup. Everything else I added as output, does not show up.

Andrew already pointed out, that you now have to set up (as example) the following:

static struct dsa_mv88e6xxx_pdata dsa_mv88e6xxx_pdata = {
        .cd = {
                .port_names[0] = NULL,
                .port_names[1] = "cpu",
                .port_names[2] = "red",
                .port_names[3] = "blue",
                .port_names[4] = "green",
                .port_names[5] = NULL,
                .port_names[6] = NULL,
                .port_names[7] = NULL,
                .port_names[8] = "waic0",
        },
        .compatible = "marvell,mv88e6190",
        .enabled_ports = BIT(1) | BIT(2) | BIT(3) | BIT(4) | BIT(8),
        .eeprom_len = 65536,
};

static const struct mdio_board_info bdinfo = {
        .bus_id = "gpio-0",
        .modalias = "mv88e6085",
        .mdio_addr = 0,
        .platform_data = &dsa_mv88e6xxx_pdata, };

        dsa_mv88e6xxx_pdata.netdev = dev_get_by_name(&init_net, "eth0");
        if (!dsa_mv88e6xxx_pdata.netdev) {
                dev_err(dev, "Error finding Ethernet device\n");
                return -ENODEV;
        }

        err = mdiobus_register_board_info(&bdinfo, 1);
        if (err) {
                dev_err(dev, "Error setting up MDIO board info\n");
                goto out;
        }

I did a bunch of tries with this information's, put in our .bus_id, changed the port_names accordingly, switched the board register up and down etc. 
but it still does not succeed ☹ 
As Andrew also mentioned there is no mainline example and I'm also a bit confused that I haven't found (might be limitated to my person)
any example somewhere out there which will put some light in our darkness...

So I would appreciate any help from you guys.
You can also contact me directly - feel free if you think it a more optimized way. 
I'll share my solution/information's if it works someday ...

Kind regards,
Tobias

Re: DSA and Marvell Switch Drivers

[Florian Fainelli]

On 2/16/22 12:55 AM, Moltmann, Tobias wrote:
> Hello,
> 
> I would first rewrite my questions and provide some information's and add some of Andrews answer to it as well.
> As I learned it would be helpful to address the "netdev list" to get (hopefully) even more help. 
> So please feel free to provide some feedback, thank you!
> 
> The issue we face comes along with an Kernel upgrade from 4.4.xxx to now 5.10.xx. It is all industrial-based hardware, so 
> no classical PC or something. On the hardware we have an x86 CPU, an IGB -> Marvell Switch (Marvell 88E6176) ->
> and two PHY's connected there.
> 
> Very roughly the way it worked/run with the 4.4 Kernel:
> 
> - mv88e6xxx_init() called - registered the driver
> - IGB driver loaded, started probing
> - within the probing we set up an MDIO bus (name: igb MDIO, id: 0000:01:00.0_mii)
> - the libphy does a first scan with no result due to some other missing stuff - at this point it is ok
> - also within the igb probing we set up an dsa_platform_data struct and run a platform_device_register()
> - this triggers the DSA driver
> - mv88e6352_probe() is called our Marvell switch is detected
> - a new DSA slave MDIO bus is been brought up automatically
> - mdiobus_scan() there register our two PHY devices 
> - everything is working :)
> 
> 
> Now in the 5.10 Kernel, as there are the rewritten DSA/Marvell drivers included, the upper "stuff" is not working anymore.
> So far so good... Due to the fact, that the DSA/Marvell drivers aren't platform driver anymore the trigger with the platform_device_register()
> has no effect any more. I suggest we need to end up in the new mv88e6xxx_probe() function in drivers/net/dsa/mv88e6xxx/chip.c.
> As there is all the probing, register of DSA stuff included. 
> I added some "printf" in the module init parts in the chip.c file as well as in the /net/dsa/dsa.c - and I do see them in the
> Kernel logfile after startup. Everything else I added as output, does not show up.
> 
> Andrew already pointed out, that you now have to set up (as example) the following:
> 
> static struct dsa_mv88e6xxx_pdata dsa_mv88e6xxx_pdata = {
>         .cd = {
>                 .port_names[0] = NULL,
>                 .port_names[1] = "cpu",
>                 .port_names[2] = "red",
>                 .port_names[3] = "blue",
>                 .port_names[4] = "green",
>                 .port_names[5] = NULL,
>                 .port_names[6] = NULL,
>                 .port_names[7] = NULL,
>                 .port_names[8] = "waic0",
>         },
>         .compatible = "marvell,mv88e6190",
>         .enabled_ports = BIT(1) | BIT(2) | BIT(3) | BIT(4) | BIT(8),
>         .eeprom_len = 65536,
> };
> 
> static const struct mdio_board_info bdinfo = {
>         .bus_id = "gpio-0",
>         .modalias = "mv88e6085",
>         .mdio_addr = 0,
>         .platform_data = &dsa_mv88e6xxx_pdata, };
> 
>         dsa_mv88e6xxx_pdata.netdev = dev_get_by_name(&init_net, "eth0");
>         if (!dsa_mv88e6xxx_pdata.netdev) {
>                 dev_err(dev, "Error finding Ethernet device\n");
>                 return -ENODEV;
>         }
> 
>         err = mdiobus_register_board_info(&bdinfo, 1);
>         if (err) {
>                 dev_err(dev, "Error setting up MDIO board info\n");
>                 goto out;
>         }
> 
> I did a bunch of tries with this information's, put in our .bus_id, changed the port_names accordingly, switched the board register up and down etc. 
> but it still does not succeed ☹ 
> As Andrew also mentioned there is no mainline example and I'm also a bit confused that I haven't found (might be limitated to my person)
> any example somewhere out there which will put some light in our darkness...
> 
> So I would appreciate any help from you guys.
> You can also contact me directly - feel free if you think it a more optimized way. 
> I'll share my solution/information's if it works someday ...

There is a working example of registering a dsa platform data based
switch under drivers/net/dsa/dsa_loop_bdinfo.c can you try to get
inspiration from that one and see where the registration fails?
-- 
Florian

Re: DSA and Marvell Switch Drivers

[Andrew Lunn]

On Wed, Feb 16, 2022 at 08:55:17AM +0000, Moltmann, Tobias wrote:
> Hello,
> 
> I would first rewrite my questions and provide some information's and add some of Andrews answer to it as well.
> As I learned it would be helpful to address the "netdev list" to get (hopefully) even more help. 
> So please feel free to provide some feedback, thank you!
> 
> The issue we face comes along with an Kernel upgrade from 4.4.xxx to now 5.10.xx. It is all industrial-based hardware, so 
> no classical PC or something. On the hardware we have an x86 CPU, an IGB -> Marvell Switch (Marvell 88E6176) ->
> and two PHY's connected there.
> 
> Very roughly the way it worked/run with the 4.4 Kernel:
> 
> - mv88e6xxx_init() called - registered the driver
> - IGB driver loaded, started probing
> - within the probing we set up an MDIO bus (name: igb MDIO, id: 0000:01:00.0_mii)

So you have some out of tree patches for this? As far as i remember,
IGB does not make use of the MDIO framework.

> - the libphy does a first scan with no result due to some other missing stuff - at this point it is ok
> - also within the igb probing we set up an dsa_platform_data struct and run a platform_device_register()
> - this triggers the DSA driver
> - mv88e6352_probe() is called our Marvell switch is detected
> - a new DSA slave MDIO bus is been brought up automatically
> - mdiobus_scan() there register our two PHY devices 
> - everything is working :)

You can find a full example here, for a slights out of date kernel,

https://github.com/lunn/linux/blob/v5.15.5-zii-rap/drivers/platform/x86/zii-rap.c

This board also uses the igb, since the Ethernet is an i210. However,
it does not hack the driver to export the MDIO bus, it makes use of a
couple of GPIOs, and the bit banging MDIO bus driver. But that should
not make too big a difference.

The basic flow is, that once this platform driver loads, it calls
zii_rap_marvell_switch(). That tries to get the 'eth0' device. There
are some not so nice assumptions here, that this platform driver loads
after the igb.

It then adds a GPIO lookup table so you can find the GPIO being used
for the switch interrupt. The IRQ GPIO is then got, and converted into
an interrupt number. The switch actually uses level interrupts, but on
this platform only edge interrupts are supported. So we setup falling
edge and hope for the best.

The IRQ number is then added to the switch platform data.

The switch platform data is then registered to the MDIO core, via
bdinfo, against a bus name we know the MDIO bus will be given,
"gpio-0".

The bit banging MDIO bus is then setup and added.

When the MDIO bus is registered with the MDIO core, it will look at
the name, find the bdinfo structure uses the same name, and use that
to instantiate a device on the MDIO bus. That device will cause the
mv88e6xxx driver to probe, and setup a switch on the bus.

You should be able to follow the same basic scheme. Just don't add the
big banging MDIO bus, use the name you know your hacked igb driver
will use when it registers its MDIO bus. This board uses a 6390, which
needs a different compatible string to what you need. You need
"marvell,mv88e6085" for your switch.

	  Andrew