Re: [PATCH v8 net-next 06/10] net: dsa: microchip: add support for phylink management

[Prasanna Vengateshan]

On Mon, 2022-02-07 at 19:27 +0200, Vladimir Oltean wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> 
> On Mon, Feb 07, 2022 at 10:52:00PM +0530, Prasanna Vengateshan wrote:
> > ased on interface later */
> > +     data8 &= ~PORT_MII_SEL_M;
> > +
> > +     /* configure MAC based on interface */
> > +     switch (interface) {
> > +     case PHY_INTERFACE_MODE_MII:
> > +             lan937x_config_gbit(dev, false, &data8);
> > +             data8 |= PORT_MII_SEL;
> > +             break;
> > +     case PHY_INTERFACE_MODE_RMII:
> > +             lan937x_config_gbit(dev, false, &data8);
> > +             data8 |= PORT_RMII_SEL;
> > +             break;
> > +     case PHY_INTERFACE_MODE_RGMII:
> > +             lan937x_config_gbit(dev, true, &data8);
> > +             data8 |= PORT_RGMII_SEL;
> > +             break;
> > +     case PHY_INTERFACE_MODE_RGMII_ID:
> > +     case PHY_INTERFACE_MODE_RGMII_TXID:
> > +     case PHY_INTERFACE_MODE_RGMII_RXID:
> > +             lan937x_config_gbit(dev, true, &data8);
> > +             data8 |= PORT_RGMII_SEL;
> > +
> > +             /* Apply rgmii internal delay for the mac */
> > +             lan937x_apply_rgmii_delay(dev, port, interface, data8);
> 
> I think the agreement from previous discussions was to apply RGMII delay
> _exclusively_ based on the 'rx-internal-delay-ps' and 'tx-internal-delay-ps'
> properties, at least for new drivers with no legacy. You are omitting to
> apply delays in phy-mode = "rgmii", which contradicts that agreement.
> I think you should treat all 4 RGMII cases the same, and remove the
> interface checks from lan937x_apply_rgmii_delay.

Thanks for the feedback. Yes, you are right. Regardless of the phy-mode, mac
should apply its delay from the device tree. I will change both of the places in
the next revision.

Prasanna V
>