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@xxxxxxx/ 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.
