From: Vladimir Oltean <olteanv@gmail.com>
To: Michael Walle <michael@walle.cc>
Cc: Heiner Kallweit <hkallweit1@gmail.com>,
Russell King <linux@armlinux.org.uk>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
Jose Abreu <Jose.Abreu@synopsys.com>,
Sergey Shtylyov <s.shtylyov@omp.ru>, Wei Fang <wei.fang@nxp.com>,
Shenwei Wang <shenwei.wang@nxp.com>,
Clark Wang <xiaoning.wang@nxp.com>,
NXP Linux Team <linux-imx@nxp.com>,
Sean Wang <sean.wang@mediatek.com>,
Landen Chao <Landen.Chao@mediatek.com>,
DENG Qingfang <dqfext@gmail.com>,
Florian Fainelli <f.fainelli@gmail.com>,
Matthias Brugger <matthias.bgg@gmail.com>,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-renesas-soc@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-mediatek@lists.infradead.org, Andrew Lunn <andrew@lunn.ch>,
Geert Uytterhoeven <geert+renesas@glider.be>
Subject: Re: [PATCH RFC net-next v2 11/12] net: dsa: Separate C22 and C45 MDIO bus transaction methods
Date: Tue, 3 Jan 2023 17:31:34 +0200 [thread overview]
Message-ID: <20230103153134.utalc6kw3l34dp4s@skbuf> (raw)
In-Reply-To: <20221227-v6-2-rc1-c45-seperation-v2-11-ddb37710e5a7@walle.cc> <20221227-v6-2-rc1-c45-seperation-v2-11-ddb37710e5a7@walle.cc>
On Wed, Dec 28, 2022 at 12:07:27AM +0100, Michael Walle wrote:
> From: Andrew Lunn <andrew@lunn.ch>
>
> By adding _c45 function pointers to the dsa_switch_op structure, the
> dsa core can register an MDIO bus with C45 accessors.
>
> The dsa-loop driver could in theory provide such accessors, since it
> just passed requests to the MDIO bus it is on, but it seems unlikely
> to be useful at the moment. It can however be added later.
>
> mt7530 does support C45, but its uses a mix of registering its MDIO
> bus and using the DSA core provided bus. This makes the change a bit
> more complex.
"using the DSA core provided bus" is a misrepresentation AFAICS.
Rather said, "providing its private MDIO bus to the DSA core too".
>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
> v2:
> - [al] Remove conditional c45, since all switches support c45
> - [al] Remove dsa core changes, they are not needed
> - [al] Add comment that DSA provided MDIO bus is C22 only.
> ---
> drivers/net/dsa/mt7530.c | 87 ++++++++++++++++++++++++------------------------
> drivers/net/dsa/mt7530.h | 15 ++++++---
> include/net/dsa.h | 2 +-
> 3 files changed, 56 insertions(+), 48 deletions(-)
This patch is not very coherent after the changes in v2.
There are really 2 distinct pieces:
1. a comment in include/net/dsa.h, which provides a justification for
why dsa_switch_ops :: {phy_read(), phy_write()} weren't split into
{phy_read(), phy_write()} and {phy_read_c45() and phy_write_c45()}.
2. a conversion of the mt7530 MDIO bus driver.
I would expect these to be distinct patches.
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 96086289aa9b..732c7bc261a9 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -858,7 +858,7 @@ struct dsa_switch_ops {
> u32 (*get_phy_flags)(struct dsa_switch *ds, int port);
>
> /*
> - * Access to the switch's PHY registers.
> + * Access to the switch's PHY registers. C22 only.
> */
> int (*phy_read)(struct dsa_switch *ds, int port, int regnum);
> int (*phy_write)(struct dsa_switch *ds, int port,
Let me try to untangle for you what these operations really do.
When they are present, DSA will allocate ds->slave_mii_bus on behalf of
the driver, and use these methods for MDIO access of internal PHYs.
The purpose of ds->slave_mii_bus is to offer a non-OF based
phy_connect() for old-style device trees where there is no phy-handle
between the user port fwnode and the internal PHY fwnode (normally
because the ethernet-phy isn't described in the device tree at all).
Like this:
ethernet-switch {
ethernet-ports {
port@1 {
reg = <1>;
};
};
};
So ds->slave_mii_bus is useful with or without the ds->ops->phy_read()
and ds->ops->phy_write() pointers, which is for example why mt7530
allocates its own MDIO bus with its own private methods (so it doesn't
populate ds->ops->phy_read()), but it also populates ds->slave_mii_bus
with its own bus.
Since clause 45 PHYs are identified by the "ethernet-phy-ieee802.3-c45"
compatible string (otherwise they are C22), then a PHY which is not
described in the device tree can only be C22. So this is why
ds->slave_mii_bus only deals with clause 22 methods, and the true reason
behind the comment above.
But actually this premise is no longer true since Luiz' commit
fe7324b93222 ("net: dsa: OF-ware slave_mii_bus"), which introduced the
strange concept of an "OF-aware helper for internal PHYs which are not
described in the device tree". After his patch, it is possible to have
something like this:
ethernet-switch {
ethernet-ports {
port@1 {
reg = <1>;
};
};
mdio {
ethernet-phy@1 {
compatible = "ethernet-phy-ieee802.3-c45"
reg = <1>;
};
};
};
As you can see, this is a clause 45 internal PHY which lacks a
phy-handle, so its bus must be put in ds->slave_mii_bus in order for
dsa_slave_phy_connect() to see it without that phy-handle (based on the
port number matching with the PHY number). After Luiz' patch, this kind
of device tree is possible, and it invalidates the assumption about
ds->slave_mii_bus only driving C22 PHYs.
>
> --
> 2.30.2
next prev parent reply other threads:[~2023-01-03 15:32 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-27 23:07 [PATCH RFC net-next v2 00/12] net: mdio: Start separating C22 and C45 Michael Walle
2022-12-27 23:07 ` [PATCH RFC net-next v2 01/12] net: mdio: Add dedicated C45 API to MDIO bus drivers Michael Walle
2022-12-27 23:07 ` [PATCH RFC net-next v2 02/12] net: pcs: pcs-xpcs: Use C45 MDIO API Michael Walle
2023-01-03 13:28 ` Vladimir Oltean
2022-12-27 23:07 ` [PATCH RFC net-next v2 03/12] net: mdio: mdiobus_register: update validation test Michael Walle
2023-01-03 10:13 ` Russell King (Oracle)
2023-01-03 10:21 ` Michael Walle
2023-01-03 22:19 ` Russell King (Oracle)
2023-01-09 12:35 ` Michael Walle
2023-01-09 12:41 ` Vladimir Oltean
2022-12-27 23:07 ` [PATCH RFC net-next v2 04/12] net: mdio: C22 is now optional, EOPNOTSUPP if not provided Michael Walle
2022-12-27 23:07 ` [PATCH RFC net-next v2 05/12] net: mdio: Move mdiobus_c45_addr() next to users Michael Walle
2022-12-27 23:07 ` [PATCH RFC net-next v2 06/12] net: mdio: mdio-bitbang: Separate C22 and C45 transactions Michael Walle
2023-01-03 13:15 ` Vladimir Oltean
2023-01-03 13:27 ` Michael Walle
2022-12-27 23:07 ` [PATCH RFC net-next v2 07/12] net: mdio: mvmdio: Convert XSMI bus to new API Michael Walle
2022-12-27 23:07 ` [PATCH RFC net-next v2 08/12] net: ethernet: freescale: xgmac: Separate C22 and C45 transactions for xgmac Michael Walle
2023-01-03 13:55 ` Vladimir Oltean
2022-12-27 23:07 ` [PATCH RFC net-next v2 09/12] net: ethernet: freescale: fec: " Michael Walle
2022-12-28 3:31 ` Wei Fang
2023-01-03 13:57 ` Vladimir Oltean
2022-12-27 23:07 ` [PATCH RFC net-next v2 10/12] net: mdio: add mdiobus_c45_read/write_nested helpers Michael Walle
2022-12-27 23:07 ` [PATCH RFC net-next v2 11/12] net: dsa: Separate C22 and C45 MDIO bus transaction methods Michael Walle
2023-01-03 15:31 ` Vladimir Oltean [this message]
2023-01-03 15:48 ` Andrew Lunn
2023-01-03 15:56 ` Vladimir Oltean
2023-01-16 7:51 ` Michael Walle
2023-01-16 19:28 ` Vladimir Oltean
2022-12-27 23:07 ` [PATCH RFC net-next v2 12/12] net: dsa: mv88e6xxx: Separate C22 and C45 transactions Michael Walle
2023-01-03 16:20 ` Vladimir Oltean
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=20230103153134.utalc6kw3l34dp4s@skbuf \
--to=olteanv@gmail.com \
--cc=Jose.Abreu@synopsys.com \
--cc=Landen.Chao@mediatek.com \
--cc=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=dqfext@gmail.com \
--cc=edumazet@google.com \
--cc=f.fainelli@gmail.com \
--cc=geert+renesas@glider.be \
--cc=hkallweit1@gmail.com \
--cc=kuba@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-imx@nxp.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=linux-renesas-soc@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=matthias.bgg@gmail.com \
--cc=michael@walle.cc \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=s.shtylyov@omp.ru \
--cc=sean.wang@mediatek.com \
--cc=shenwei.wang@nxp.com \
--cc=wei.fang@nxp.com \
--cc=xiaoning.wang@nxp.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).