Received-SPF: pass (google.com: domain of linux-kernel+bounces-28118-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Tue, 16 Jan 2024 20:59:28 +0100
From: Andrew Lunn <andrew@lunn.ch>
To: Tim Menninger <tmenninger@purestorage.com>
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
Subject: Re: [PATCH] net: dsa: mv88e6xxx: Make *_c45 callbacks agree with
 phy_*_c45 callbacks
Message-ID: <04d22048-737a-4281-a43f-b125ebe0c896@lunn.ch>
References: <20240116193542.711482-1-tmenninger@purestorage.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240116193542.711482-1-tmenninger@purestorage.com>

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 there
> is a non-NULL phy_read_c45 callback on the chip mv88e6xxx_ops. Similarly
> for write_c45 and phy_write_c45.
> 
> In commit 743a19e38d02 ("net: dsa: mv88e6xxx: Separate C22 and C45 transactions")
> 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 the
> 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 mean
> we can perform a C45 read and proceed with a C45 MDIO bus scan. The scan
> encounters a NULL phy_read_c45 callback in the mv88e6xxx_ops which implies
> 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