On Fri, Aug 2, 2024 at 4:08 PM Andrew Lunn <andrew@xxxxxxx> wrote:
>
> > Management of integrated ethernet switch on this SoC is not handled by
> > the PCIe interface.
>
> MDIO? SPI? I2C?
>
The device uses SPI interface. The switch has internal ARM M7 for
controller firmware.
> > +#define XGMAC_PCIE_MISC_MII_CTRL 0x4
> > +#define XGMAC_PCIE_MISC_MII_CTRL_VALUE 0x7
>
> Could you replace these magic values with actual definitions. What
> does 7 mean?
>
Thanks, I will fix the macros.
> > +#define XGMAC_PCIE_MISC_PCIESS_CTRL 0x8
> > +#define XGMAC_PCIE_MISC_PCIESS_CTRL_VALUE 0x200
>
> > +static int num_instances;
>
> > + /* This device is directly attached to the switch chip internal to the
> > + * SoC using XGMII interface. Since no MDIO is present, register
> > + * fixed-link software_node to create phylink.
> > + */
> > + if (num_instances == 0) {
> > + ret = software_node_register_node_group(fixed_link_node_group);
> > + if (ret) {
> > + dev_err(&pdev->dev,
> > + "%s: failed to register software node\n",
> > + __func__);
> > + return ret;
> > + }
> > + }
> > + num_instances++;
>
> So all the instances of the MAC share one fixed link? That is pretty
> unusual. In DT, each would have its own. Have you reviewed the
> implications of this?
>
Our thinking was that since the software node is only used for static
node data to populate phylink config, a per device node is not
required.
We tested with multiple devices and repeated PCI remove/rescan operations.
However, It does make sense to be consistent with the DT usage model.
We will add the per device node entry in the next patch update.
> Andrew
