Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 18 Jul 2023 22:46:45 +0100
From:   "Russell King (Oracle)" <linux@armlinux.org.uk>
To:     Andrew Lunn <andrew@lunn.ch>
Cc:     Michael Walle <mwalle@kernel.org>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Yisen Zhuang <yisen.zhuang@huawei.com>,
        Salil Mehta <salil.mehta@huawei.com>,
        Florian Fainelli <florian.fainelli@broadcom.com>,
        Broadcom internal kernel review list 
        <bcm-kernel-feedback-list@broadcom.com>,
        Marek =?iso-8859-1?Q?Beh=FAn?= <kabel@kernel.org>,
        Xu Liang <lxu@maxlinear.com>, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Simon Horman <simon.horman@corigine.com>
Subject: Re: [PATCH net-next v3 03/11] net: phy: replace is_c45 with
 phy_accces_mode
Message-ID: <ZLcIRYb4Ak+nW557@shell.armlinux.org.uk>
References: <20230620-feature-c45-over-c22-v3-0-9eb37edf7be0@kernel.org>
 <20230620-feature-c45-over-c22-v3-3-9eb37edf7be0@kernel.org>
 <509889a3-f633-40b0-8349-9ef378818cc7@lunn.ch>
 <ZLbRTLRbHW/Xt2hL@shell.armlinux.org.uk>
 <6afc205e-b525-44df-b81b-ef7cfdfb6680@lunn.ch>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <6afc205e-b525-44df-b81b-ef7cfdfb6680@lunn.ch>
Sender: Russell King (Oracle) <linux@armlinux.org.uk>
Precedence: bulk

On Tue, Jul 18, 2023 at 09:18:51PM +0200, Andrew Lunn wrote:
> On Tue, Jul 18, 2023 at 06:52:12PM +0100, Russell King (Oracle) wrote:
> > On Tue, Jul 18, 2023 at 07:40:49PM +0200, Andrew Lunn wrote:
> > > >  static inline bool phy_has_c45_registers(struct phy_device *phydev)
> > > >  {
> > > > -	return phydev->is_c45;
> > > > +	return phydev->access_mode != PHY_ACCESS_C22;
> > > >  }
> > > 
> > > So this is making me wounder if we have a clean separation between
> > > register spaces and access methods.
> > > 
> > > Should there be a phy_has_c22_registers() ?
> > 
> > Yes, I've been wondering that. I've recently heard about a Realtek PHY
> > which is supported by our realtek driver, but appears on a SFP that
> > can only do C45 accesses. However, the realtek driver is written to
> > use C22 accesses to this PHY - and the PHY supports both. So currently
> > it doesn't work.
> >
> > That's just an additional data point for thinking about this, I haven't
> > formulated a solution to it yet.
> 
> That kind of sounds like two drivers. Or two drivers in one .c
> file. Do you know what C45 IDs it has? Same as the C22? If it is
> different, each could have its own struct phy_driver.

Why would it be two drivers?

The PHY in question is RTL8221B-VB-CG, and it has the same ID in both
the C22 and C45 registers, since the realtek driver attempts to drive
the device, but fails because __phy_read() errors out (because there's
no C22 bus access.)

If one looks at:

https://www.realtek.com/en/products/connected-media-ics/item/rtl8221b-i-vb-cg

it states "Supports clause 22 and Clause 45 MDC/MDIO management
interface".

which to me suggests it can be driven via clause 22 MDIO cycles on
the bus as a clause 22 PHY and using indirect access to clause 45
registers, or only via clause 45 MDIO cycles.

In the case of this particular SFP, it apparently uses the Rollball
access method which is C45-only.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!