Re: [PATCH] net: asix: fix modprobe "sysfs: cannot create duplicate filename"

From: Grant Grundler
Date: Tue Mar 07 2023 - 13:19:46 EST


On Tue, Mar 7, 2023 at 2:29 AM Lukas Wunner <lukas@xxxxxxxxx> wrote:
>
> On Mon, Mar 06, 2023 at 10:10:09PM -0800, Grant Grundler wrote:
> > On Mon, Mar 6, 2023 at 7:46???PM Florian Fainelli <f.fainelli@xxxxxxxxx> wrote:
> > > On 3/6/2023 4:50 PM, Grant Grundler wrote:
> > > > + priv->phydev = mdiobus_get_phy(priv->mdio, priv->phy_addr);
> > > > + if (priv->phydev)
> > > > + return 0;
> > >
> > > This was in ax88772_init_phy() before, why is this being moved here now?
> >
> > Because other drivers I looked at (e.g. tg3 and r8169) do all the mdiobus_*
> > calls in one function and I wanted to have some "symmetry"
> > with ax88772_release_mdio() function I added below.
>
> I'd suggest moving this cleanup to a separate commit so that you keep
> the fix itself as small as possible and thus minimize the potential of
> introducing regressions in stable kernels that will receive the fix.

Ok - will do.

> Also, per convention please use the if-clause to catch the error case,
> not the success case. It doesn't matter if you need two or three more
> lines, readability is more important IMO.

Sure. No problem. The code I wrote seemed easier to read but I'm
familiar with the convention and have no objection to that.

I'll add the Fixes: tag as well (thanks for confirming Oleksij!)

cheers,
grant

>
> Thanks,
>
> Lukas