lkml.org 
[lkml]   [2021]   [Oct]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v4 net-next 05/10] net: dsa: microchip: add DSA support for microchip lan937x
From
Date
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.


\
 
 \ /
  Last update: 2021-10-08 20:17    [W:0.076 / U:1.304 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site