Hi Alex, On Tue, 3 Dec 2019 at 18:18, Alex Marginean <alexandru.margin...@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 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