netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Lunn <andrew@lunn.ch>
To: Michael Walle <michael@walle.cc>
Cc: "Russell King (Oracle)" <linux@armlinux.org.uk>,
	"Yisen Zhuang" <yisen.zhuang@huawei.com>,
	"Salil Mehta" <salil.mehta@huawei.com>,
	"David S . Miller" <davem@davemloft.net>,
	"Jakub Kicinski" <kuba@kernel.org>,
	"Paolo Abeni" <pabeni@redhat.com>,
	"Florian Fainelli" <f.fainelli@gmail.com>,
	"Broadcom internal kernel review list"
	<bcm-kernel-feedback-list@broadcom.com>,
	"Heiner Kallweit" <hkallweit1@gmail.com>,
	"Marek Behún" <kabel@kernel.org>, "Xu Liang" <lxu@maxlinear.com>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next 0/5] net: phy: C45-over-C22 access
Date: Tue, 24 Jan 2023 02:44:01 +0100	[thread overview]
Message-ID: <Y8834YKbZAoLyMQJ@lunn.ch> (raw)
In-Reply-To: <05e044a5f308ad81919d28a5b2dfdd42@walle.cc>

On Tue, Jan 24, 2023 at 01:35:48AM +0100, Michael Walle wrote:
> >       - const: ethernet-phy-ieee802.3-c45
> >         description: PHYs that implement IEEE802.3 clause 45
> > 
> > But it is not clear what that actually means. Does it mean it has c45
> > registers, or does it mean it supports C45 bus transactions?
> 
> PHYs which support C45 access but don't have C45 registers aren't
> a thing I presume - or doesn't make any sense, right?
> 
> PHYs which have C45 registers but don't support C45 access exists.
> e.g. the EEE registers of C22 PHYs. But they are C22 PHYs.

I wonder if there are any C22 PHYs which only allow access to EEE via
C45 transactions. That would be pretty broken...

To some extent, i'm still looking at everything and trying to decide
if the current definitions/documentation make it clear if means C45
transfers or registers. And the documentation in DT is ambiguous, but
as you point out, it probably means registers, not transactions.

> So I'd say if you have compatible = "ethernet-phy-ieee802.3-c45",
> it is a PHY with C45 registers _and_ which are accessible by
> C45 transactions. But they might or might not support C22 access.
> But I think thats pretty irrelevant because you always do C45 if
> you can. You cannot do C45 if:
>  (1) the mdio bus doesn't support C45
>  (2) you have a broken C22 phy on the mdio bus
> 
> In both cases, if the PHY doesn't support C22-over-C45 you are
> screwed. Therefore, if you have either (1) or (2) we should always
> fall back to C22-over-C45.
> 
> > If we have that compatible, we could probe first using C45 and if that
> > fails, or is not supported by the bus master, probe using C45 over
> > C22. That seems safe. For Michael use case, the results of
> > mdiobus_prevent_c45_scan(bus) needs keeping as a property of bus, so
> > we know not to perform the C45 scan, and go direct to C45 over C22.
> 
> So you are talking about DT, in which case there is no auto probing.
> See phy_mask in the dt code. Only PHYs in the device tree are probed.
> (unless of course there is no reg property.. then there is some
> obscure auto scanning). So if you want a C45 PHY you'd have to
> have that compatible in any case.
> 
> Btw. I still don't know how you can get a C45 PHY instance without
> a device tree, except if there is a C45 only bus or the PHY doesn't
> respond on C22 ids. Maybe I'm missing something here..

In the DT case, you are probably correct. But there are a number of
MDIO busses which don't come from DT. Those are typically PCIe or USB
devices. Those do get scanned, and my recent changes should mean they
first get scanned using C22 and then C45. DSA switches also typically
don't have a MDIO node in DT, it is assumed there is a 1:1 mapping
between port number and address on the MDIO bus. But as you said, it
would require that they don't respond to C22, or the bus master does
not support C22, which does actually exist from Marvell at least.

In the none DT case, we probably cannot easily do anything about
C22-over-C45, because as Russell pointed out, it is potentially a
destructive process doing a scan. We want some indication we do expect
a PHY to be there. And "ethernet-phy-ieee802.3-c45" would do that.

	Andrew

  reply	other threads:[~2023-01-24  1:44 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-20 22:40 [PATCH net-next 0/5] net: phy: C45-over-C22 access Michael Walle
2023-01-20 22:40 ` [PATCH net-next 1/5] net: phy: add error checks in mmd_phy_indirect() and export it Michael Walle
2023-01-23 15:34   ` Andrew Lunn
2023-01-20 22:40 ` [PATCH net-next 2/5] net: phy: support indirect c45 access in get_phy_c45_ids() Michael Walle
2023-01-20 22:40 ` [PATCH net-next 3/5] net: phy: add support for C45-over-C22 transfers Michael Walle
2023-01-20 22:40 ` [PATCH net-next 4/5] phy: net: introduce phy_promote_to_c45() Michael Walle
2023-01-20 23:20   ` Russell King (Oracle)
2023-01-20 23:27     ` Michael Walle
2023-01-20 22:40 ` [PATCH net-next 5/5] net: phy: mxl-gpy: remove unneeded ops Michael Walle
2023-01-23 18:03 ` [PATCH net-next 0/5] net: phy: C45-over-C22 access Andrew Lunn
2023-01-23 18:47   ` Russell King (Oracle)
2023-01-23 20:05     ` Andrew Lunn
2023-01-24  0:35       ` Michael Walle
2023-01-24  1:44         ` Andrew Lunn [this message]
2023-01-24 14:41     ` Michael Walle
2023-01-24 21:03       ` Andrew Lunn
2023-01-24 21:20         ` Michael Walle
2023-01-25 13:52           ` Andrew Lunn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y8834YKbZAoLyMQJ@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=davem@davemloft.net \
    --cc=f.fainelli@gmail.com \
    --cc=hkallweit1@gmail.com \
    --cc=kabel@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=lxu@maxlinear.com \
    --cc=michael@walle.cc \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=salil.mehta@huawei.com \
    --cc=yisen.zhuang@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).