Date:   Tue, 7 Mar 2023 11:29:31 +0100
From:   Lukas Wunner <lukas@wunner.de>
To:     Grant Grundler <grundler@chromium.org>
Cc:     Florian Fainelli <f.fainelli@gmail.com>,
        Oleksij Rempel <linux@rempel-privat.de>,
        Pavel Skripkin <paskripkin@gmail.com>,
        Anton Lundin <glance@acc.umu.se>,
        Eizan Miyamoto <eizan@chromium.org>,
        Jakub Kicinski <kuba@kernel.org>,
        netdev <netdev@vger.kernel.org>,
        "David S . Miller" <davem@davemloft.net>,
        LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] net: asix: fix modprobe "sysfs: cannot create duplicate
 filename"
Message-ID: <20230307102931.GA25631@wunner.de>
References: <20230307005028.2065800-1-grundler@chromium.org>
 <84094771-7f98-0d8d-fe79-7c22e15a602d@gmail.com>
 <CANEJEGvM_xLrSSjrgKLh_xP+BrFFT+afDQhG8BOdgHPf7eR4gQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CANEJEGvM_xLrSSjrgKLh_xP+BrFFT+afDQhG8BOdgHPf7eR4gQ@mail.gmail.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

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@gmail.com> 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.

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.

Thanks,

Lukas