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

Hi Alex,

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