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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 9 Feb 2022 16:31:08 +0530
From:   Prasanna Vengateshan <prasanna.vengateshan@...rochip.com>
To:     Vladimir Oltean <olteanv@...il.com>
CC:     <andrew@...n.ch>, <netdev@...r.kernel.org>, <robh+dt@...nel.org>,
        <UNGLinuxDriver@...rochip.com>, <woojung.huh@...rochip.com>,
        <hkallweit1@...il.com>, <linux@...linux.org.uk>,
        <davem@...emloft.net>, <kuba@...nel.org>,
        <linux-kernel@...r.kernel.org>, <vivien.didelot@...il.com>,
        <f.fainelli@...il.com>, <devicetree@...r.kernel.org>
Subject: Re: [PATCH v8 net-next 06/10] net: dsa: microchip: add support for
 phylink management

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
>