[PATCH v1 0/2] Fix bridge gmii2rgmii
When taken in use U-Boot v2023.04 one our board's ethernet stop to work in U-Boot, which was working in v2022.01.
In v2022.01 the gmii2rgmii was called before PHY was created.
This patch change back the order and fix the problem. Also the ethernet-phy-id driver is taken in use in gmii2rgmii driver.
Tapio Reijonen (2): net: phy: Let gmiitorgmii converter create additional PHY net: phy: gmii2rgmii: Add support for phy ethernet id configuration
drivers/net/phy/phy.c | 8 ++++---- drivers/net/phy/xilinx_gmii2rgmii.c | 11 ++++++++--- 2 files changed, 12 insertions(+), 7 deletions(-)
Change the order to connect gmiitorgmii before PHY creation. The gmiitorgmii create additional in DTS configured PHY during it's configuration. This ensures, that converter sits between the MAC and the external phy MAC <==> GMII2RGMII <==> RGMII_PHY.
Fixes: commit a744a284e354 ("net: phy: Add support for ethernet-phy-id with gpio reset") Signed-off-by: Tapio Reijonen tapio.reijonen@vaisala.com ---
drivers/net/phy/phy.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index 716a1d46111..740533adeca 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -939,14 +939,14 @@ struct phy_device *phy_connect(struct mii_dev *bus, int addr, phydev = phy_device_create(bus, 0, PHY_NCSI_ID, false); #endif
-#ifdef CONFIG_PHY_ETHERNET_ID +#ifdef CONFIG_PHY_XILINX_GMII2RGMII if (!phydev) - phydev = phy_connect_phy_id(bus, dev, addr); + phydev = phy_connect_gmii2rgmii(bus, dev); #endif
-#ifdef CONFIG_PHY_XILINX_GMII2RGMII +#ifdef CONFIG_PHY_ETHERNET_ID if (!phydev) - phydev = phy_connect_gmii2rgmii(bus, dev); + phydev = phy_connect_phy_id(bus, dev, addr); #endif
if (!phydev)
Hi,
čt 16. 1. 2025 v 14:07 odesílatel Tapio Reijonen tapio.reijonen@vaisala.com napsal:
Change the order to connect gmiitorgmii before PHY creation. The gmiitorgmii create additional in DTS configured PHY during it's configuration. This ensures, that converter sits between the MAC and the external phy MAC <==> GMII2RGMII <==> RGMII_PHY.
Fixes: commit a744a284e354 ("net: phy: Add support for ethernet-phy-id with gpio reset")
From format. Look at link in 2/2.
Signed-off-by: Tapio Reijonen tapio.reijonen@vaisala.com
drivers/net/phy/phy.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index 716a1d46111..740533adeca 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -939,14 +939,14 @@ struct phy_device *phy_connect(struct mii_dev *bus, int addr, phydev = phy_device_create(bus, 0, PHY_NCSI_ID, false); #endif
-#ifdef CONFIG_PHY_ETHERNET_ID +#ifdef CONFIG_PHY_XILINX_GMII2RGMII if (!phydev)
phydev = phy_connect_phy_id(bus, dev, addr);
phydev = phy_connect_gmii2rgmii(bus, dev);#endif
-#ifdef CONFIG_PHY_XILINX_GMII2RGMII +#ifdef CONFIG_PHY_ETHERNET_ID if (!phydev)
phydev = phy_connect_gmii2rgmii(bus, dev);
phydev = phy_connect_phy_id(bus, dev, addr);#endif
if (!phydev)-- 2.39.5
I can't see any issue with this swap but I would like to check your DT description first. Can you please share your DT fragment which describes this?
Thanks, Michal
Hi Michal,
On 2025-01-21 15:14, Michal Simek wrote:
[You don't often get email from monstr@monstr.eu. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
Hi,
čt 16. 1. 2025 v 14:07 odesílatel Tapio Reijonen tapio.reijonen@vaisala.com napsal:
Change the order to connect gmiitorgmii before PHY creation. The gmiitorgmii create additional in DTS configured PHY during it's configuration. This ensures, that converter sits between the MAC and the external phy MAC <==> GMII2RGMII <==> RGMII_PHY.
Fixes: commit a744a284e354 ("net: phy: Add support for ethernet-phy-id with gpio reset")
From format. Look at link in 2/2.
Fixing format in next version.
Signed-off-by: Tapio Reijonen tapio.reijonen@vaisala.com
drivers/net/phy/phy.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index 716a1d46111..740533adeca 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -939,14 +939,14 @@ struct phy_device *phy_connect(struct mii_dev *bus, int addr, phydev = phy_device_create(bus, 0, PHY_NCSI_ID, false); #endif
-#ifdef CONFIG_PHY_ETHERNET_ID +#ifdef CONFIG_PHY_XILINX_GMII2RGMII if (!phydev)
phydev = phy_connect_phy_id(bus, dev, addr);
phydev = phy_connect_gmii2rgmii(bus, dev);#endif
-#ifdef CONFIG_PHY_XILINX_GMII2RGMII +#ifdef CONFIG_PHY_ETHERNET_ID if (!phydev)
phydev = phy_connect_gmii2rgmii(bus, dev);
phydev = phy_connect_phy_id(bus, dev, addr);#endif
if (!phydev)-- 2.39.5
I can't see any issue with this swap but I would like to check your DT description first. Can you please share your DT fragment which describes this?
Here is our board's DT fragment: &gem1 { status = "okay"; phy-handle = <&phy1>; phy-mode = "rgmii-id";
mdio { #address-cells = <1>; #size-cells = <0>; phy1: ethernet-phy@1 { reg = <1>; #phy-cells = <1>; compatible = "ethernet-phy-id2000.a231"; reset-assert-us = <10>; reset-deassert-us = <400>; reset-gpios = <&gpio0 54 GPIO_ACTIVE_LOW>; ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_00_NS>; ti,tx-internal-delay = <DP83867_RGMIIDCTL_2_50_NS>; ti,fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_4_B_NIB>; };
gmii_to_rgmii_0: gmiitorgmii@8 { reg = <8>; compatible = "xlnx,gmii-to-rgmii-1.0"; phy-handle = <&phy1>; }; }; };
Thanks, Tapio
Thanks, Michal
-- Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91 w: http://www.monstr.eu/ p: +42-0-721842854 Maintainer of Linux kernel - Xilinx Microblaze Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs
On 1/22/25 08:31, Tapio Reijonen wrote:
Hi Michal,
On 2025-01-21 15:14, Michal Simek wrote:
[You don't often get email from monstr@monstr.eu. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
Hi,
čt 16. 1. 2025 v 14:07 odesílatel Tapio Reijonen tapio.reijonen@vaisala.com napsal:
Change the order to connect gmiitorgmii before PHY creation. The gmiitorgmii create additional in DTS configured PHY during it's configuration. This ensures, that converter sits between the MAC and the external phy MAC <==> GMII2RGMII <==> RGMII_PHY.
Fixes: commit a744a284e354 ("net: phy: Add support for ethernet-phy-id with gpio reset")
From format. Look at link in 2/2.
Fixing format in next version.
Signed-off-by: Tapio Reijonen tapio.reijonen@vaisala.com
drivers/net/phy/phy.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index 716a1d46111..740533adeca 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -939,14 +939,14 @@ struct phy_device *phy_connect(struct mii_dev *bus, int addr, phydev = phy_device_create(bus, 0, PHY_NCSI_ID, false); #endif
-#ifdef CONFIG_PHY_ETHERNET_ID +#ifdef CONFIG_PHY_XILINX_GMII2RGMII if (!phydev)
phydev = phy_connect_phy_id(bus, dev, addr);
phydev = phy_connect_gmii2rgmii(bus, dev);-#ifdef CONFIG_PHY_XILINX_GMII2RGMII +#ifdef CONFIG_PHY_ETHERNET_ID if (!phydev)
phydev = phy_connect_gmii2rgmii(bus, dev);
phydev = phy_connect_phy_id(bus, dev, addr);#endif
if (!phydev)-- 2.39.5
I can't see any issue with this swap but I would like to check your DT description first. Can you please share your DT fragment which describes this?
Here is our board's DT fragment: &gem1 { status = "okay"; phy-handle = <&phy1>; phy-mode = "rgmii-id";
mdio { #address-cells = <1>; #size-cells = <0>; phy1: ethernet-phy@1 { reg = <1>; #phy-cells = <1>; compatible = "ethernet-phy-id2000.a231"; reset-assert-us = <10>; reset-deassert-us = <400>; reset-gpios = <&gpio0 54 GPIO_ACTIVE_LOW>; ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_00_NS>; ti,tx-internal-delay = <DP83867_RGMIIDCTL_2_50_NS>; ti,fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_4_B_NIB>; };
gmii_to_rgmii_0: gmiitorgmii@8 { reg = <8>; compatible = "xlnx,gmii-to-rgmii-1.0"; phy-handle = <&phy1>; };}; };
That's what I was expecting that you have and it is not your fault. I have been in touch with our team in connection to proper description of this situation and description above is unfortunately what Linux expects but it is not correct.
Description should be gem - phy_handle points to bridge and bridge point to phy
what you have above that gem and bridge are pointing to the same phy.
As far as I know there was no way in Linux kernel to describe it like this that's why dt binding for linux kernel is different. (And by purpose Linux dt binding is not describing this connection from IP).
It means I am fine with the patch above which let kernel binding to work but please describe this reason in commit message.
Thanks, Michal
When Kconfig PHY_ETHERNET_ID is set, create external PHY using via ethernet-phy-id driver to support using starpping resistors of the external PHY.
Fixes: commit a744a284e354 ("net: phy: Add support for ethernet-phy-id with gpio reset") Signed-off-by: Tapio Reijonen tapio.reijonen@vaisala.com ---
drivers/net/phy/xilinx_gmii2rgmii.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/net/phy/xilinx_gmii2rgmii.c b/drivers/net/phy/xilinx_gmii2rgmii.c index e44b7b75bd5..1d6d204b228 100644 --- a/drivers/net/phy/xilinx_gmii2rgmii.c +++ b/drivers/net/phy/xilinx_gmii2rgmii.c @@ -18,7 +18,7 @@ DECLARE_GLOBAL_DATA_PTR; static int xilinxgmiitorgmii_config(struct phy_device *phydev) { ofnode node = phy_get_ofnode(phydev); - struct phy_device *ext_phydev; + struct phy_device *ext_phydev = NULL; struct ofnode_phandle_args phandle; int ext_phyaddr = -1; int ret; @@ -40,8 +40,13 @@ static int xilinxgmiitorgmii_config(struct phy_device *phydev) return ret;
ext_phyaddr = ofnode_read_u32_default(phandle.node, "reg", -1); - ext_phydev = phy_find_by_mask(phydev->bus, - 1 << ext_phyaddr); + + if (IS_ENABLED(CONFIG_PHY_ETHERNET_ID)) + ext_phydev = phy_connect_phy_id(phydev->bus, phydev->dev, ext_phyaddr); + + if (!ext_phydev) + ext_phydev = phy_find_by_mask(phydev->bus, + 1 << ext_phyaddr); if (!ext_phydev) { printf("%s, No external phy device found\n", __func__); return -EINVAL;
On 1/16/25 06:43, Tapio Reijonen wrote:
When Kconfig PHY_ETHERNET_ID is set, create external PHY using via ethernet-phy-id driver to support using starpping resistors
typo.
of the external PHY.
Fixes: commit a744a284e354 ("net: phy: Add support for ethernet-phy-id with gpio reset")
Not correct format.
Please look at https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Docu...
Signed-off-by: Tapio Reijonen tapio.reijonen@vaisala.com
drivers/net/phy/xilinx_gmii2rgmii.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/net/phy/xilinx_gmii2rgmii.c b/drivers/net/phy/xilinx_gmii2rgmii.c index e44b7b75bd5..1d6d204b228 100644 --- a/drivers/net/phy/xilinx_gmii2rgmii.c +++ b/drivers/net/phy/xilinx_gmii2rgmii.c @@ -18,7 +18,7 @@ DECLARE_GLOBAL_DATA_PTR; static int xilinxgmiitorgmii_config(struct phy_device *phydev) { ofnode node = phy_get_ofnode(phydev);
- struct phy_device *ext_phydev;
- struct phy_device *ext_phydev = NULL; struct ofnode_phandle_args phandle; int ext_phyaddr = -1; int ret;
@@ -40,8 +40,13 @@ static int xilinxgmiitorgmii_config(struct phy_device *phydev) return ret;
ext_phyaddr = ofnode_read_u32_default(phandle.node, "reg", -1);
- ext_phydev = phy_find_by_mask(phydev->bus,
1 << ext_phyaddr);
- if (IS_ENABLED(CONFIG_PHY_ETHERNET_ID))
ext_phydev = phy_connect_phy_id(phydev->bus, phydev->dev, ext_phyaddr);
Can you please stay inside 80 chars per line?
- if (!ext_phydev)
ext_phydev = phy_find_by_mask(phydev->bus,1 << ext_phyaddr);
The rest looks good to me.
Thanks, Michal
Hi Michal,
On 2025-01-21 15:11, Michal Simek wrote:
On 1/16/25 06:43, Tapio Reijonen wrote:
When Kconfig PHY_ETHERNET_ID is set, create external PHY using via ethernet-phy-id driver to support using starpping resistors
typo.
starpping -> strapping
of the external PHY.
Fixes: commit a744a284e354 ("net: phy: Add support for ethernet-phy-id with gpio reset")
Not correct format.
Please look at https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Docu...
Fixing format in next version.
Signed-off-by: Tapio Reijonen tapio.reijonen@vaisala.com
drivers/net/phy/xilinx_gmii2rgmii.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/net/phy/xilinx_gmii2rgmii.c b/drivers/net/phy/xilinx_gmii2rgmii.c index e44b7b75bd5..1d6d204b228 100644 --- a/drivers/net/phy/xilinx_gmii2rgmii.c +++ b/drivers/net/phy/xilinx_gmii2rgmii.c @@ -18,7 +18,7 @@ DECLARE_GLOBAL_DATA_PTR; static int xilinxgmiitorgmii_config(struct phy_device *phydev) { ofnode node = phy_get_ofnode(phydev); - struct phy_device *ext_phydev; + struct phy_device *ext_phydev = NULL; struct ofnode_phandle_args phandle; int ext_phyaddr = -1; int ret; @@ -40,8 +40,13 @@ static int xilinxgmiitorgmii_config(struct phy_device *phydev) return ret; ext_phyaddr = ofnode_read_u32_default(phandle.node, "reg", -1); - ext_phydev = phy_find_by_mask(phydev->bus, - 1 << ext_phyaddr);
+ if (IS_ENABLED(CONFIG_PHY_ETHERNET_ID)) + ext_phydev = phy_connect_phy_id(phydev->bus, phydev->dev, ext_phyaddr);
Can you please stay inside 80 chars per line?
Fixing inside 80 chars per line.
+ if (!ext_phydev) + ext_phydev = phy_find_by_mask(phydev->bus, + 1 << ext_phyaddr); if (!ext_phydev) { printf("%s, No external phy device found\n", __func__); return -EINVAL;
The rest looks good to me.
Thanks, Tapio
Thanks, Michal
participants (3)
-
Michal Simek -
Michal Simek -
Tapio Reijonen