[PATCH v3 4/6] drivers: net: add Felix DSA switch driver

[Vladimir Oltean]

On Sun, 15 Dec 2019 at 14:53, Vladimir Oltean <olteanv at gmail.com> wrote:
>
> Hi Alex,
>
> On Tue, 3 Dec 2019 at 18:18, Alex Marginean <alexandru.marginean at nxp.com> wrote:
> > +static int felix_port_enable(struct udevice *dev, int port,
> > +                            struct phy_device *phy)
> > +{
> > +       struct felix_priv *priv = dev_get_priv(dev);
> > +       void *base = priv->regs_base;
> > +
> > +       out_le32(base + FELIX_GMII_MAC_ENA_CFG(port),
> > +                FELIX_GMII_MAX_ENA_CFG_TX | FELIX_GMII_MAX_ENA_CFG_RX);
> > +
> > +       out_le32(base + FELIX_QSYS_SYSTEM_SW_PORT_MODE(port),
> > +                FELIX_QSYS_SYSTEM_SW_PORT_ENA |
> > +                FELIX_QSYS_SYSTEM_SW_PORT_LOSSY |
> > +                FELIX_QSYS_SYSTEM_SW_PORT_SCH(1));
> > +
> > +       if (phy)
> > +               phy_startup(phy);
> > +       return 0;
> > +}
> > +
> > +static void felix_port_disable(struct udevice *dev, int port,
> > +                              struct phy_device *phy)
> > +{
> > +       struct felix_priv *priv = dev_get_priv(dev);
> > +       void *base = priv->regs_base;
> > +
> > +       out_le32(base + FELIX_GMII_MAC_ENA_CFG(port), 0);
> > +
> > +       out_le32(base + FELIX_QSYS_SYSTEM_SW_PORT_MODE(port),
> > +                FELIX_QSYS_SYSTEM_SW_PORT_LOSSY |
> > +                FELIX_QSYS_SYSTEM_SW_PORT_SCH(1));
> > +
> > +       /*
> > +        * we don't call phy_shutdown here to avoind waiting next time we use
> > +        * the port, but the downside is that remote side will think we're
> > +        * actively processing traffic although we are not.
> > +        */
> > +}
> > --
> > 2.17.1
> >
>
> What is the correct general procedure here, is it to call phy_startup
> so late (felix_port_enable)? I'm trying to take this driver as an
> example for sja1105, which has RGMII so PCS and no autonomous in-band

RGMII, so no* PCS

> AN like felix does. On this switch, it is too late to do phy_startup
> now. Instead, I would need to look at phy->speed which should have
> been settled by now, and reprogram my MAC with it.
> My question is: don't you think phy_startup() and phy_shutdown()
> belong in the DSA uclass code?
>
> Thanks,
> -Vladimir