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

[Alexandru Marginean]

On 12/15/2019 3:16 PM, Vladimir Oltean wrote:
> 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
>> 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?
>>
> 
> My bad, phy_startup() is synchronous and waits for PHY AN to complete.
> So this is fine.
> 
>> Thanks,
>> -Vladimir

OK, thanks :)
Alex