Re: DSA

["Wyse, Chris" <cwyse@canoga.com>]

Subject: Re: DSA

> On Mon, 30 Apr 2018 14:50:30 +0200, Andrew Lunn wrote:
>
> > On Fri, Apr 27, 2018 at 06:10:55PM +0000, Dave Richards wrote:
> > > Hello,
> > >
> > > I am building a prototype for a new product based on a Lanner,
Inc. embedded PC.
> > > It is an Intel Celeron-based system with two host I210 GbE chips
connected to 2
> > > MV88E6172 chips (one NIC to one switch).  Everything appears to
show up
> > > hardware-wise.  My question is, what is the next step?  How does
DSA know which
> > > NICs are intended to be masters?  Is this supposed to be auto-
detected or is this
> > > knowledge supposed to be communicated explicitly.  Reading
through the DSA driver
> > > code I see that there is a check of the OF property list for the
device for a
> > > "label"/"cpu" property/value pair that needs to be present.  Who
sets this and when?
> >
> > Hi Dave
> >
> > Since you are on Intel, you don't have simple access to Device
> > tree. So you need to use platform data instead. Or possibly start
> > hacking on ACPI support for DSA. For the moment, i would suggest
> > platform data.
> >
> > I'm also working on a similar setup, intel CPU connected to an
> > MV88E6532. I have some work in progress code i can share with you,
> > which i want to submit for inclusion to mainline in the next few
> > weeks.  This adds platform data support to the mv88e6xxx driver,
and
> > will give you an idea how you link the MAC to the switch.
> >
> > What MDIO bus do you connect the switches to? The i210 MDIO bus? If
> > so, this is going to cause you a problem. The igb driver ignores
the
> > Linux MDIO and PHY code, and does it all itself. DSA assumes the
> > switch can be accessed using Linux standard MDIO interfaces. So you
> > have going to have to hack on the igb driver to make it use
standard
> > MDIO.
> >
> > Andrew
>
> > > Take a look at arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi
> > >
> > > &pcie {
> > >         pinctrl-names = "default";
> > >         pinctrl-0 = <&pinctrl_pcie>;
> > >         reset-gpio = <&gpio7 12 GPIO_ACTIVE_LOW>;
> > >         status = "okay";
> > >
> > >         host@0 {
> > >                 reg = <0 0 0 0 0>;
> > >
> > >                 #address-cells = <3>;
> > >                 #size-cells = <2>;
> > >
> > >                 i210: i210@0 {
> > >                         reg = <0 0 0 0 0>;
> > >                 };
> > >         };
> > > };
> > >
> > I'll look at this, but one thing I see initially is that there are
> > references to other nodes that are not present in our device tree
> > overlay.  The overlay solely supports the IP modules in the
FPGA.  Both
> > of our PCIe buses are handled via the ACPI table.  I'm not sure how
to
> > handle something that already has an ACPI node.
>
> Overlay is also possibly too late.

The current boot sequence is below:
1. i210 driver loads
2. Device Tree overlay installed (in addition to ACPI)
   - Includes DSA switch and ports, but master port was incorrectly
specified as the EMACLite IP module, which had a DT node
   - No DT node for the i210
3. MFD driver reads DT overlay and instantiates supporting devices and
the DSA driver

My thought was to create a DT entry for the i210 in the overlay, even
though the driver is already loaded via ACPI.  The DT overlay node for
the i210 would provide any needed information to the DSA driver.  It
would be essentially a reference to the already loaded driver.

If I could do that, the sequence above would remain the same.  The only
difference would be the additional DT overlay node for the i210 and
referenced devices.  I'm not an ACPI or DT expert, so I'm not sure what
types of issues I'd run into with this approach.

> Maybe. I guess you need the DT
> available at the time the PCIe controller probes the bus. The core
> PCIe code then pokes around in the DT and finds the node which
> corresponds to the device on the bus. You might be able to work
around
> this with pci hotplugging?

I think hot-plugging would work, but I'm not sure I see the need, since
the DSA driver would not be instantiated until the MFD driver is
loaded.

> Load the overlay, and then trigger a hot
> unplug/plug of the i210 via files in /sys? The relevant bit of code
is
> pci_set_of_node() which appears to get called independent of ACPI or
> DT.

So I think you're saying that the DT node for the i210 should work fine
if I emulate a PCI hotplug event.  Won't there be an issue since there
are ACPI nodes for the i210 and referenced devices?  I could unload the
drivers, then issue the hotplug event.

>
> Otherwise you need to go the platform driver route. What works for
mv88e6xxx is
>
> 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;
>         }
>
> On this device, there is a bit-banging MDIO driver. The MDIO core has
> the needed code to associate the mdio_board_info to the bus, such
that
> after the bus probes, it adds a platform device on that bus, the
> switch. The mv88e6xxx gets the dsa_mv88e6xxx_pdata, containing the
> name of the Ethernet interface. You can probably do something similar
> in your MFD for the FPGA.

I think I need to see what you did for that.  That might be the right
approach for our purposes.  I could add a DT overlay node with the name
of the interface.  We're in a controlled environment - the name won't
change.  The DSA driver (modified) could read the i210 DT node,
retrieve the device name, then call dev_get_by_name().

>
> This a bit fragile. systemd can come in and rename your interface
from
> eth0 to enp1s0, and then dev_get_by_name(). The advantage of DT is
> that the name does not matter, you point directly at the device.
>
> The other problem with this is you don't have a DT representation of
> the switch, making it hard to use phylink for the SFPs, etc. So
> getting overlays working would be best.

Hmm...  Overlays are working except for the missing master port node.
I think I would use the same DT representation I have now, except to
reference the new i210 node as the master.



________________________________

Canoga Perkins
20600 Prairie Street
Chatsworth, CA 91311
(818) 718-6300

This e-mail and any attached document(s) is confidential and is intended only for the review of the party to whom it is addressed. If you have received this transmission in error, please notify the sender immediately and discard the original message and any attachment(s).