diff mbox series

[1/2] realtek/rtl839x: respect phy-is-integrated property

Message ID 20240426224008.3400172-1-stijn@linux-ipv6.be
State Accepted, archived
Delegated to: Stijn Tintel
Headers show
Series [1/2] realtek/rtl839x: respect phy-is-integrated property | expand

Commit Message

Stijn Tintel April 26, 2024, 10:40 p.m. UTC
Respect the phy-is-integrated property on ethernet-phy nodes.

There are RTL8393M switches where the PHYs at address 48 and 49 are
provided by an external RTL8214FC. Hardcoding them to use the internal
SerDes makes it impossible to use the ports connected to such an
external PHY. Respect the phy-is-integrated property on ethernet-phy
nodes as a first step to support such ports.

The potential impact for this should be limited to RTL8393 based
switches, and looking at the commit messages and device tree files of
the supported switches based on this SoC, the SFP and/or combo ports are
either not working (D-Link DGS-1210-52, Netgear GS750E, TP-Link
SG2452P/T1600G-52PS), use PHYs at a different address (Panasonic
SwitchM48EG PN28480K), or already have the phy-is-integrated property
set on the PHYs at address 48 and 49.

Signed-off-by: Stijn Tintel <stijn@linux-ipv6.be>
---
 .../realtek/files-5.15/drivers/net/ethernet/rtl838x_eth.c     | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Daniel Golle April 26, 2024, 11:06 p.m. UTC | #1
Hi Stijn,

On Sat, Apr 27, 2024 at 01:40:07AM +0300, stijn@linux-ipv6.be wrote:
> Respect the phy-is-integrated property on ethernet-phy nodes.
> 
> There are RTL8393M switches where the PHYs at address 48 and 49 are
> provided by an external RTL8214FC. Hardcoding them to use the internal
> SerDes makes it impossible to use the ports connected to such an
> external PHY. Respect the phy-is-integrated property on ethernet-phy
> nodes as a first step to support such ports.

Yes, we should get rid of all mentions of port addresses in the
Ethernet driver. Thank you for taking care of that.

Sidenote: Those aren't actual (MDIO bus) PHY addresses. RealTek switches
got an internal mapping which is setup by the driver which map the
addresses of the actual PHYs on actual physical MDIO (aka SMI) busses to
the ports of the switch. MDIO bus uses 5-bit address, hence an address
greater than 31 is nonsense and should rather be called a port address
or a port-mapped PHY address. Transparently exposing the actual physical
busses and having the driver reverse the augmentation of the addresses
using the (hardware) mapping between switch ports and PHYs would
probably be the best, and allow for the use of generic PHY (and PHY
package) drivers instead of a lot of RealTek-specific hacks. I started
working on that some time ago and may get back to it if I find the time.


> 
> The potential impact for this should be limited to RTL8393 based
> switches, and looking at the commit messages and device tree files of
> the supported switches based on this SoC, the SFP and/or combo ports are
> either not working (D-Link DGS-1210-52, Netgear GS750E, TP-Link
> SG2452P/T1600G-52PS), use PHYs at a different address (Panasonic
> SwitchM48EG PN28480K), or already have the phy-is-integrated property
> set on the PHYs at address 48 and 49.
> 
> Signed-off-by: Stijn Tintel <stijn@linux-ipv6.be>

Acked-by: Daniel Golle <daniel@makrotopia.org>

> ---
>  .../realtek/files-5.15/drivers/net/ethernet/rtl838x_eth.c     | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/linux/realtek/files-5.15/drivers/net/ethernet/rtl838x_eth.c b/target/linux/realtek/files-5.15/drivers/net/ethernet/rtl838x_eth.c
> index 54e592aeaa..71e7937336 100644
> --- a/target/linux/realtek/files-5.15/drivers/net/ethernet/rtl838x_eth.c
> +++ b/target/linux/realtek/files-5.15/drivers/net/ethernet/rtl838x_eth.c
> @@ -1658,7 +1658,7 @@ static int rtl839x_mdio_read_paged(struct mii_bus *bus, int mii_id, u16 page, in
>  	int err;
>  	struct rtl838x_eth_priv *priv = bus->priv;
>  
> -	if (mii_id >= 48 && mii_id <= 49 && priv->id == 0x8393)
> +	if (priv->phy_is_internal[mii_id])
>  		return rtl839x_read_sds_phy(mii_id, regnum);
>  
>  	if (regnum & (MII_ADDR_C45 | MII_ADDR_C22_MMD)) {
> @@ -1797,7 +1797,7 @@ static int rtl839x_mdio_write_paged(struct mii_bus *bus, int mii_id, u16 page,
>  	struct rtl838x_eth_priv *priv = bus->priv;
>  	int err;
>  
> -	if (mii_id >= 48 && mii_id <= 49 && priv->id == 0x8393)
> +	if (priv->phy_is_internal[mii_id])
>  		return rtl839x_write_sds_phy(mii_id, regnum, value);
>  
>  	if (regnum & (MII_ADDR_C45 | MII_ADDR_C22_MMD)) {
> -- 
> 2.43.2
> 
> 
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Andreas Böhler May 7, 2024, 12:34 p.m. UTC | #2
The sender domain has a DMARC Reject/Quarantine policy which disallows
sending mailing list messages using the original "From" header.

To mitigate this problem, the original message has been wrapped
automatically by the mailing list software.
Hi,

On 27/04/2024 00:40, stijn@linux-ipv6.be wrote:
> Respect the phy-is-integrated property on ethernet-phy nodes.
>
> There are RTL8393M switches where the PHYs at address 48 and 49 are
> provided by an external RTL8214FC. Hardcoding them to use the internal
> SerDes makes it impossible to use the ports connected to such an
> external PHY. Respect the phy-is-integrated property on ethernet-phy
> nodes as a first step to support such ports.
>
> The potential impact for this should be limited to RTL8393 based
> switches, and looking at the commit messages and device tree files of
> the supported switches based on this SoC, the SFP and/or combo ports are
> either not working (D-Link DGS-1210-52, Netgear GS750E, TP-Link
> SG2452P/T1600G-52PS), use PHYs at a different address (Panasonic
> SwitchM48EG PN28480K), or already have the phy-is-integrated property
> set on the PHYs at address 48 and 49.

Some time ago, Jan Hoffmann has been working on support for the HPE 
1920-48G switch. He came up with a very similar patch and also with 
patches for the RTL8214FC. I'm using his version for quite some time now 
on my HPE switch.

Here is his commit: 
https://github.com/janh/openwrt/commit/ee5d8a17b540a3a9aa7b666a679b479903799d10

Unfortunately, there are so many different patches floating around, 
scattered across different forks, it's hard to know who has done what.

Andreas
diff mbox series

Patch

diff --git a/target/linux/realtek/files-5.15/drivers/net/ethernet/rtl838x_eth.c b/target/linux/realtek/files-5.15/drivers/net/ethernet/rtl838x_eth.c
index 54e592aeaa..71e7937336 100644
--- a/target/linux/realtek/files-5.15/drivers/net/ethernet/rtl838x_eth.c
+++ b/target/linux/realtek/files-5.15/drivers/net/ethernet/rtl838x_eth.c
@@ -1658,7 +1658,7 @@  static int rtl839x_mdio_read_paged(struct mii_bus *bus, int mii_id, u16 page, in
 	int err;
 	struct rtl838x_eth_priv *priv = bus->priv;
 
-	if (mii_id >= 48 && mii_id <= 49 && priv->id == 0x8393)
+	if (priv->phy_is_internal[mii_id])
 		return rtl839x_read_sds_phy(mii_id, regnum);
 
 	if (regnum & (MII_ADDR_C45 | MII_ADDR_C22_MMD)) {
@@ -1797,7 +1797,7 @@  static int rtl839x_mdio_write_paged(struct mii_bus *bus, int mii_id, u16 page,
 	struct rtl838x_eth_priv *priv = bus->priv;
 	int err;
 
-	if (mii_id >= 48 && mii_id <= 49 && priv->id == 0x8393)
+	if (priv->phy_is_internal[mii_id])
 		return rtl839x_write_sds_phy(mii_id, regnum, value);
 
 	if (regnum & (MII_ADDR_C45 | MII_ADDR_C22_MMD)) {