Received-SPF: pass (google.com: domain of linux-kernel+bounces-28347-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Precedence: bulk
MIME-Version: 1.0
References: <20240116193542.711482-1-tmenninger@purestorage.com> <04d22048-737a-4281-a43f-b125ebe0c896@lunn.ch>
In-Reply-To: <04d22048-737a-4281-a43f-b125ebe0c896@lunn.ch>
From: Tim Menninger <tmenninger@purestorage.com>
Date: Tue, 16 Jan 2024 14:24:55 -0800
Message-ID: <CAO-L_44YVi0HDk4gC9QijMZrYNGoKtfH7qsXOwtDwM4VrFRDHw@mail.gmail.com>
Subject: Re: [PATCH] net: dsa: mv88e6xxx: Make *_c45 callbacks agree with
 phy_*_c45 callbacks
To: Andrew Lunn <andrew@lunn.ch>
Cc: f.fainelli@gmail.com, olteanv@gmail.com, davem@davemloft.net, 
	edumazet@google.com, kuba@kernel.org, pabeni@redhat.com, 
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Tue, Jan 16, 2024 at 11:59=E2=80=AFAM Andrew Lunn <andrew@lunn.ch> wrote=
:
>
> On Tue, Jan 16, 2024 at 07:35:42PM +0000, Tim Menninger wrote:
> > Set the read_c45 callback in the mii_bus struct in mv88e6xxx only if th=
ere
> > is a non-NULL phy_read_c45 callback on the chip mv88e6xxx_ops. Similarl=
y
> > for write_c45 and phy_write_c45.
> >
> > In commit 743a19e38d02 ("net: dsa: mv88e6xxx: Separate C22 and C45 tran=
sactions")
> > the MDIO bus driver split its API to separate C22 and C45 transfers.
> >
> > In commit 1a136ca2e089 ("net: mdio: scan bus based on bus capabilities =
for C22 and C45")
> > we do a C45 mdio bus scan based on existence of the read_c45 callback
> > rather than checking MDIO bus capabilities then in
> > commit da099a7fb13d ("net: phy: Remove probe_capabilities") we remove t=
he
> > probe_capabilities from the mii_bus struct.
> >
> > The combination of the above results in a scenario (e.g. mv88e6185)
> > where we take a non-NULL read_c45 callback on the mii_bus struct to mea=
n
> > we can perform a C45 read and proceed with a C45 MDIO bus scan. The sca=
n
> > encounters a NULL phy_read_c45 callback in the mv88e6xxx_ops which impl=
ies
> > we can NOT perform a C45 read and fails with EOPNOTSUPP. The read_c45
> > callback should be NULL if phy_read_c45 is NULL, and similarly for
> > write_c45 and phy_write_c45.
>
> Hi Tim
>
> What does phylib do with the return of -EOPNOTSUPP? I've not tested
> it, but i would expect it just keeps going with the scan? It treats it
> as if there is no device there? And since it never accesses the
> hardware, this should be fast?
>
> Or is my assumption wrong? Do you see the EPOPNOTSUPP getting reported
> back to user space, and the probe failing?
>
>      Andrew
>

Hi Andrew,

It bubbles up as EIO (the translation happens in get_phy_c45_ids when
get_phy_c45_devs_in_pkg fails) and ultimately causes the probe to fail.

The EIO causes the scan to stop and fail immediately - the way I read
mdiobus_scan_bus_c45, only ENODEV is permissible.

From logs:
$ dmesg | grep mv88e6
[   12.951149] mv88e6085 ixgbe-mdio-0000:05:00.0:00: switch 0x1a70
detected: Marvell 88E6185, revision 2
[   13.272812] mv88e6085 ixgbe-mdio-0000:05:00.0:00: Cannot register
MDIO bus (-5)
[   13.401140] mv88e6085: probe of ixgbe-mdio-0000:05:00.0:00 failed
with error -5
[   13.413105] mv88e6085 ixgbe-mdio-0000:05:00.1:00: switch 0x1a70
detected: Marvell 88E6185, revision 2
[   13.730227] mv88e6085 ixgbe-mdio-0000:05:00.1:00: Cannot register
MDIO bus (-5)
[   13.858336] mv88e6085: probe of ixgbe-mdio-0000:05:00.1:00 failed
with error -5

Tim