netdev - Re: [PATCH v4 net-next 05/10] net: dsa: microchip: add DSA support for microchip lan937x

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]

Message-ID: <0c4264ca6f859b6ce53e59f1565563f8dc29a2c6.camel@microchip.com>
Date:   Fri, 8 Oct 2021 23:47:23 +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 v4 net-next 05/10] net: dsa: microchip: add DSA support
 for microchip lan937x

On Thu, 2021-10-07 at 23:00 +0300, Vladimir Oltean wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> 
> On Thu, Oct 07, 2021 at 08:41:55PM +0530, Prasanna Vengateshan wrote:
> > +static int lan937x_mdio_register(struct dsa_switch *ds)
> > 
> > +
> > +     ret = of_mdiobus_register(ds->slave_mii_bus, mdio_np);
> 
> Please use devm_of_mdiobus_register if you're going to use
> devm_mdiobus_alloc, or no devres at all.
> https://patchwork.kernel.org/project/netdevbpf/patch/20210920214209.1733768-3-vladimir.oltean@nxp.com/

Sure, Will change it to devm_of_mdiobus_register.

> > 
> > +
> > +                     /* Check if the device tree have specific interface
> > +                      * setting otherwise read & assign from XMII register
> > +                      * for host port interface
> > +                      */
> > +                     interface = lan937x_get_interface(dev, i);
> 
> What does the CPU port have so special that you override it here?
> Again some compatibility with out-of-tree DT bindings?
> > 
> 
Device strapping method cannot be used since the phy-mode is expected
to be present in the DT. So above assignment have to be removed along
with lan937x_get_interface function.


> > +
> > +     /* maximum delay is 4ns */
> > +     if (val > 4000)
> > +             val = 4000;
> 
> These bindings are new. Given that you also document their min and max
> values, why don't you just error out on out-of-range values instead of
> silently doing what you think is going to be fine?

Sure, i think the driver can also notify rx/tx internal delay applied
message using dev_info.