[PATCH 0/5] mx6cuboxi: add eth support for SoMs revision 1.9+
As of Revision 1.9 the SolidRun i.MX6 SoMs ship with a new PHY - an ADIN1300 at address 1. This patchset carries many small parts to facilitate proper operation of the ethernet port in U-Boot as well as Linux:
1. Fix a compile error in the recently phy driver 2. Add support for configuring the clock output pins to the phy driver 3. update device-tree with support for the neew phy 4. support runtime patching of device-tree for Linux: enables only the phy node detected during boot, to avoid warning messages during boot. 5. finally enable the phy driver in the cuboxi defconfig
Josua Mayer (5): phy: adin: remove broken support for adi,phy-mode-override phy: adin: add support for clock output ARM: dts: imx6qdl-sr-som: add support for alternate phy addresses mx6cuboxi: fixup dtb ethernet phy nodes before booting an OS mx6cuboxi: enable driver for adin1300 phy
arch/arm/dts/imx6qdl-sr-som.dtsi | 17 +++++- board/solidrun/mx6cuboxi/mx6cuboxi.c | 78 +++++++++++++++++++++++++++ configs/mx6cuboxi_defconfig | 2 + drivers/net/phy/adin.c | 80 ++++++++++++++++------------ 4 files changed, 141 insertions(+), 36 deletions(-)
The adin_get_phy_mode_override function does not compile, because it is missing both declaration and implementation of phy_get_interface_by_name.
Remove the whole function for now, since the missing implementation is not included in mainline Linux - and thus can not be copied.
Signed-off-by: Josua Mayer josua@solid-run.com --- drivers/net/phy/adin.c | 34 ---------------------------------- 1 file changed, 34 deletions(-)
diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c index cff841ab3d..2433e76fea 100644 --- a/drivers/net/phy/adin.c +++ b/drivers/net/phy/adin.c @@ -94,35 +94,6 @@ static u32 adin_get_reg_value(struct phy_device *phydev, return rc; }
-/** - * adin_get_phy_mode_override - Get phy-mode override for adin PHY - * - * The function gets phy-mode string from property 'adi,phy-mode-override' - * and return its index in phy_interface_strings table, or -1 in error case. - */ -int adin_get_phy_mode_override(struct phy_device *phydev) -{ - ofnode node = phy_get_ofnode(phydev); - const char *phy_mode_override; - const char *prop_phy_mode_override = "adi,phy-mode-override"; - int override_interface; - - phy_mode_override = ofnode_read_string(node, prop_phy_mode_override); - if (!phy_mode_override) - return -ENODEV; - - debug("%s: %s = '%s'\n", - __func__, prop_phy_mode_override, phy_mode_override); - - override_interface = phy_get_interface_by_name(phy_mode_override); - - if (override_interface < 0) - printf("%s: %s = '%s' is not valid\n", - __func__, prop_phy_mode_override, phy_mode_override); - - return override_interface; -} - static u16 adin_ext_read(struct phy_device *phydev, const u32 regnum) { u16 val; @@ -148,11 +119,6 @@ static int adin_config_rgmii_mode(struct phy_device *phydev) { u16 reg_val; u32 val; - int phy_mode_override = adin_get_phy_mode_override(phydev); - - if (phy_mode_override >= 0) { - phydev->interface = (phy_interface_t) phy_mode_override; - }
reg_val = adin_ext_read(phydev, ADIN1300_GE_RGMII_CFG);
Hi Josua,
On Sun, 2022-05-01 at 15:41 +0300, Josua Mayer wrote:
The adin_get_phy_mode_override function does not compile, because it is missing both declaration and implementation of phy_get_interface_by_name.
Remove the whole function for now, since the missing implementation is not included in mainline Linux - and thus can not be copied.
Signed-off-by: Josua Mayer josua@solid-run.com
drivers/net/phy/adin.c | 34 ---------------------------------- 1 file changed, 34 deletions(-)
diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c index cff841ab3d..2433e76fea 100644 --- a/drivers/net/phy/adin.c +++ b/drivers/net/phy/adin.c @@ -94,35 +94,6 @@ static u32 adin_get_reg_value(struct phy_device *phydev, return rc; } -/**
- adin_get_phy_mode_override - Get phy-mode override for adin PHY
- The function gets phy-mode string from property 'adi,phy-mode-
override'
- and return its index in phy_interface_strings table, or -1 in
error case.
- */
-int adin_get_phy_mode_override(struct phy_device *phydev) -{ - ofnode node = phy_get_ofnode(phydev); - const char *phy_mode_override; - const char *prop_phy_mode_override = "adi,phy-mode-override"; - int override_interface;
- phy_mode_override = ofnode_read_string(node, prop_phy_mode_override); - if (!phy_mode_override) - return -ENODEV;
- debug("%s: %s = '%s'\n", - __func__, prop_phy_mode_override, phy_mode_override);
- override_interface = phy_get_interface_by_name(phy_mode_override);
- if (override_interface < 0) - printf("%s: %s = '%s' is not valid\n", - __func__, prop_phy_mode_override, phy_mode_override);
- return override_interface; -}
static u16 adin_ext_read(struct phy_device *phydev, const u32 regnum) { u16 val; @@ -148,11 +119,6 @@ static int adin_config_rgmii_mode(struct phy_device *phydev) { u16 reg_val; u32 val; - int phy_mode_override = adin_get_phy_mode_override(phydev);
- if (phy_mode_override >= 0) { - phydev->interface = (phy_interface_t) phy_mode_override; - } reg_val = adin_ext_read(phydev, ADIN1300_GE_RGMII_CFG);
The patch that introduced adin_get_phy_mode_override was tested against the U-Boot master branch. Unfortunately, phy_get_interface_by_name was removed just a few days before by: https://github.com/u-boot/u-boot/commit/123ca114e07ecf28aa2538748d733e2b22d8...
Can we fix adin_get_phy_mode_override instead of removing it? We can implement part of ofnode_read_phy_mode. This needs to be tested, but for example:
diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c index cff841ab3d..6a05b9fd05 100644 --- a/drivers/net/phy/adin.c +++ b/drivers/net/phy/adin.c @@ -100,27 +100,28 @@ static u32 adin_get_reg_value(struct phy_device *phydev, * The function gets phy-mode string from property 'adi,phy-mode- override' * and return its index in phy_interface_strings table, or -1 in error case. */ -int adin_get_phy_mode_override(struct phy_device *phydev) +phy_interface_t adin_get_phy_mode_override(struct phy_device *phydev) { ofnode node = phy_get_ofnode(phydev); const char *phy_mode_override; const char *prop_phy_mode_override = "adi,phy-mode-override"; - int override_interface; + int i;
phy_mode_override = ofnode_read_string(node, prop_phy_mode_override); + if (!phy_mode_override) - return -ENODEV; + return PHY_INTERFACE_MODE_NA;
debug("%s: %s = '%s'\n", __func__, prop_phy_mode_override, phy_mode_override);
- override_interface = phy_get_interface_by_name(phy_mode_override); + for (i = 0; i < PHY_INTERFACE_MODE_MAX; i++) + if (!strcmp(phy_mode_override, phy_interface_strings[i])) + return i;
- if (override_interface < 0) - printf("%s: %s = '%s' is not valid\n", - __func__, prop_phy_mode_override, phy_mode_override); + debug("%s: Invalid PHY interface '%s'\n", __func__, phy_mode_override);
- return override_interface; + return PHY_INTERFACE_MODE_NA; }
static u16 adin_ext_read(struct phy_device *phydev, const u32 regnum) @@ -148,10 +149,10 @@ static int adin_config_rgmii_mode(struct phy_device *phydev) { u16 reg_val; u32 val; - int phy_mode_override = adin_get_phy_mode_override(phydev); + phy_interface_t phy_mode_override = adin_get_phy_mode_override(phydev);
- if (phy_mode_override >= 0) { - phydev->interface = (phy_interface_t) phy_mode_override; + if (phy_mode_override != PHY_INTERFACE_MODE_NA) { + phydev->interface = phy_mode_override; }
reg_val = adin_ext_read(phydev, ADIN1300_GE_RGMII_CFG);
Thanks, Nate
Hi Nate,
Am 02.05.22 um 16:25 schrieb Nate Drude:
Hi Josua,
On Sun, 2022-05-01 at 15:41 +0300, Josua Mayer wrote:
The adin_get_phy_mode_override function does not compile, because it is missing both declaration and implementation of phy_get_interface_by_name.
Remove the whole function for now, since the missing implementation is not included in mainline Linux - and thus can not be copied.
Signed-off-by: Josua Mayer josua@solid-run.com
drivers/net/phy/adin.c | 34 ---------------------------------- 1 file changed, 34 deletions(-)
diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c index cff841ab3d..2433e76fea 100644 --- a/drivers/net/phy/adin.c +++ b/drivers/net/phy/adin.c @@ -94,35 +94,6 @@ static u32 adin_get_reg_value(struct phy_device *phydev, return rc; }
-/**
- adin_get_phy_mode_override - Get phy-mode override for adin PHY
- The function gets phy-mode string from property 'adi,phy-mode-
override'
- and return its index in phy_interface_strings table, or -1 in
error case.
- */
-int adin_get_phy_mode_override(struct phy_device *phydev) -{ - ofnode node = phy_get_ofnode(phydev); - const char *phy_mode_override; - const char *prop_phy_mode_override = "adi,phy-mode-override"; - int override_interface;
- phy_mode_override = ofnode_read_string(node, prop_phy_mode_override); - if (!phy_mode_override) - return -ENODEV;
- debug("%s: %s = '%s'\n", - __func__, prop_phy_mode_override, phy_mode_override);
- override_interface = phy_get_interface_by_name(phy_mode_override);
- if (override_interface < 0) - printf("%s: %s = '%s' is not valid\n", - __func__, prop_phy_mode_override, phy_mode_override);
- return override_interface; -}
static u16 adin_ext_read(struct phy_device *phydev, const u32 regnum) { u16 val; @@ -148,11 +119,6 @@ static int adin_config_rgmii_mode(struct phy_device *phydev) { u16 reg_val; u32 val; - int phy_mode_override = adin_get_phy_mode_override(phydev);
- if (phy_mode_override >= 0) { - phydev->interface = (phy_interface_t) phy_mode_override; - }
reg_val = adin_ext_read(phydev, ADIN1300_GE_RGMII_CFG);
The patch that introduced adin_get_phy_mode_override was tested against the U-Boot master branch. Unfortunately, phy_get_interface_by_name was removed just a few days before by: https://github.com/u-boot/u-boot/commit/123ca114e07ecf28aa2538748d733e2b22d8...
:(
Can we fix adin_get_phy_mode_override instead of removing it?
I will take a look in the coming days and see if I understand how to fix this. I don't really mind if we fix it, or remove it - removing was the faster way getting my patch-set out of course ...
We can implement part of ofnode_read_phy_mode. This needs to be tested, but for example:
diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c index cff841ab3d..6a05b9fd05 100644 --- a/drivers/net/phy/adin.c +++ b/drivers/net/phy/adin.c @@ -100,27 +100,28 @@ static u32 adin_get_reg_value(struct phy_device *phydev,
- The function gets phy-mode string from property 'adi,phy-mode-
override'
- and return its index in phy_interface_strings table, or -1 in error
case. */ -int adin_get_phy_mode_override(struct phy_device *phydev) +phy_interface_t adin_get_phy_mode_override(struct phy_device *phydev) { ofnode node = phy_get_ofnode(phydev); const char *phy_mode_override; const char *prop_phy_mode_override = "adi,phy-mode-override";
int override_interface;
int i; phy_mode_override = ofnode_read_string(node,prop_phy_mode_override);
if (!phy_mode_override)
return -ENODEV;
return PHY_INTERFACE_MODE_NA; debug("%s: %s = '%s'\n", __func__, prop_phy_mode_override, phy_mode_override);
override_interface =phy_get_interface_by_name(phy_mode_override);
for (i = 0; i < PHY_INTERFACE_MODE_MAX; i++)if (!strcmp(phy_mode_override,phy_interface_strings[i]))
return i;
if (override_interface < 0)printf("%s: %s = '%s' is not valid\n",__func__, prop_phy_mode_override,phy_mode_override);
debug("%s: Invalid PHY interface '%s'\n", __func__,phy_mode_override);
return override_interface;
return PHY_INTERFACE_MODE_NA;}
static u16 adin_ext_read(struct phy_device *phydev, const u32 regnum)
@@ -148,10 +149,10 @@ static int adin_config_rgmii_mode(struct phy_device *phydev) { u16 reg_val; u32 val;
int phy_mode_override = adin_get_phy_mode_override(phydev);
phy_interface_t phy_mode_override =adin_get_phy_mode_override(phydev);
if (phy_mode_override >= 0) {phydev->interface = (phy_interface_t)phy_mode_override;
if (phy_mode_override != PHY_INTERFACE_MODE_NA) {phydev->interface = phy_mode_override; } reg_val = adin_ext_read(phydev, ADIN1300_GE_RGMII_CFG);Thanks, Nate
Hi Josua,
On Wed, 2022-05-04 at 11:51 +0300, Josua Mayer wrote:
Hi Nate,
Am 02.05.22 um 16:25 schrieb Nate Drude:
Hi Josua,
On Sun, 2022-05-01 at 15:41 +0300, Josua Mayer wrote:
The adin_get_phy_mode_override function does not compile, because it is missing both declaration and implementation of phy_get_interface_by_name.
Remove the whole function for now, since the missing implementation is not included in mainline Linux - and thus can not be copied.
Signed-off-by: Josua Mayer josua@solid-run.com
drivers/net/phy/adin.c | 34 ---------------------------------- 1 file changed, 34 deletions(-)
diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c index cff841ab3d..2433e76fea 100644 --- a/drivers/net/phy/adin.c +++ b/drivers/net/phy/adin.c @@ -94,35 +94,6 @@ static u32 adin_get_reg_value(struct phy_device *phydev, return rc; } -/**
- adin_get_phy_mode_override - Get phy-mode override for adin
PHY
- The function gets phy-mode string from property 'adi,phy-
mode- override'
- and return its index in phy_interface_strings table, or -1 in
error case.
- */
-int adin_get_phy_mode_override(struct phy_device *phydev) -{ - ofnode node = phy_get_ofnode(phydev); - const char *phy_mode_override; - const char *prop_phy_mode_override = "adi,phy-mode- override"; - int override_interface;
- phy_mode_override = ofnode_read_string(node, prop_phy_mode_override); - if (!phy_mode_override) - return -ENODEV;
- debug("%s: %s = '%s'\n", - __func__, prop_phy_mode_override, phy_mode_override);
- override_interface = phy_get_interface_by_name(phy_mode_override);
- if (override_interface < 0) - printf("%s: %s = '%s' is not valid\n", - __func__, prop_phy_mode_override, phy_mode_override);
- return override_interface; -}
static u16 adin_ext_read(struct phy_device *phydev, const u32 regnum) { u16 val; @@ -148,11 +119,6 @@ static int adin_config_rgmii_mode(struct phy_device *phydev) { u16 reg_val; u32 val; - int phy_mode_override = adin_get_phy_mode_override(phydev);
- if (phy_mode_override >= 0) { - phydev->interface = (phy_interface_t) phy_mode_override; - } reg_val = adin_ext_read(phydev, ADIN1300_GE_RGMII_CFG);
The patch that introduced adin_get_phy_mode_override was tested against the U-Boot master branch. Unfortunately, phy_get_interface_by_name was removed just a few days before by: https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com...
:(
Can we fix adin_get_phy_mode_override instead of removing it?
I will take a look in the coming days and see if I understand how to fix this. I don't really mind if we fix it, or remove it - removing was the faster
If you prefer, I can submit a separate patch to fix it. I'll wait to hear from you to avoid duplicated work.
way getting my patch-set out of course ...
We can implement part of ofnode_read_phy_mode. This needs to be tested, but for example:
diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c index cff841ab3d..6a05b9fd05 100644 --- a/drivers/net/phy/adin.c +++ b/drivers/net/phy/adin.c @@ -100,27 +100,28 @@ static u32 adin_get_reg_value(struct phy_device *phydev, * The function gets phy-mode string from property 'adi,phy-mode- override' * and return its index in phy_interface_strings table, or -1 in error case. */ -int adin_get_phy_mode_override(struct phy_device *phydev) +phy_interface_t adin_get_phy_mode_override(struct phy_device *phydev) { ofnode node = phy_get_ofnode(phydev); const char *phy_mode_override; const char *prop_phy_mode_override = "adi,phy-mode- override"; - int override_interface; + int i; phy_mode_override = ofnode_read_string(node, prop_phy_mode_override);
if (!phy_mode_override) - return -ENODEV; + return PHY_INTERFACE_MODE_NA; debug("%s: %s = '%s'\n", __func__, prop_phy_mode_override, phy_mode_override); - override_interface = phy_get_interface_by_name(phy_mode_override); + for (i = 0; i < PHY_INTERFACE_MODE_MAX; i++) + if (!strcmp(phy_mode_override, phy_interface_strings[i])) + return i; - if (override_interface < 0) - printf("%s: %s = '%s' is not valid\n", - __func__, prop_phy_mode_override, phy_mode_override); + debug("%s: Invalid PHY interface '%s'\n", __func__, phy_mode_override); - return override_interface; + return PHY_INTERFACE_MODE_NA; } static u16 adin_ext_read(struct phy_device *phydev, const u32 regnum) @@ -148,10 +149,10 @@ static int adin_config_rgmii_mode(struct phy_device *phydev) { u16 reg_val; u32 val; - int phy_mode_override = adin_get_phy_mode_override(phydev); + phy_interface_t phy_mode_override = adin_get_phy_mode_override(phydev); - if (phy_mode_override >= 0) { - phydev->interface = (phy_interface_t) phy_mode_override; + if (phy_mode_override != PHY_INTERFACE_MODE_NA) { + phydev->interface = phy_mode_override; } reg_val = adin_ext_read(phydev, ADIN1300_GE_RGMII_CFG);
Thanks, Nate
Thanks, Nate
\o/
Am 04.05.22 um 15:46 schrieb Nate Drude:
Hi Josua,
On Wed, 2022-05-04 at 11:51 +0300, Josua Mayer wrote:
Hi Nate,
Am 02.05.22 um 16:25 schrieb Nate Drude:
Hi Josua,
On Sun, 2022-05-01 at 15:41 +0300, Josua Mayer wrote:
The adin_get_phy_mode_override function does not compile, because it is missing both declaration and implementation of phy_get_interface_by_name.
Remove the whole function for now, since the missing implementation is not included in mainline Linux - and thus can not be copied.
Signed-off-by: Josua Mayer josua@solid-run.com
drivers/net/phy/adin.c | 34 ---------------------------------- 1 file changed, 34 deletions(-)
diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c index cff841ab3d..2433e76fea 100644 --- a/drivers/net/phy/adin.c +++ b/drivers/net/phy/adin.c @@ -94,35 +94,6 @@ static u32 adin_get_reg_value(struct phy_device *phydev, return rc; }
-/**
- adin_get_phy_mode_override - Get phy-mode override for adin
PHY
- The function gets phy-mode string from property 'adi,phy-
mode- override'
- and return its index in phy_interface_strings table, or -1 in
error case.
- */
-int adin_get_phy_mode_override(struct phy_device *phydev) -{ - ofnode node = phy_get_ofnode(phydev); - const char *phy_mode_override; - const char *prop_phy_mode_override = "adi,phy-mode- override"; - int override_interface;
- phy_mode_override = ofnode_read_string(node, prop_phy_mode_override); - if (!phy_mode_override) - return -ENODEV;
- debug("%s: %s = '%s'\n", - __func__, prop_phy_mode_override, phy_mode_override);
- override_interface = phy_get_interface_by_name(phy_mode_override);
- if (override_interface < 0) - printf("%s: %s = '%s' is not valid\n", - __func__, prop_phy_mode_override, phy_mode_override);
- return override_interface; -}
static u16 adin_ext_read(struct phy_device *phydev, const u32 regnum) { u16 val; @@ -148,11 +119,6 @@ static int adin_config_rgmii_mode(struct phy_device *phydev) { u16 reg_val; u32 val; - int phy_mode_override = adin_get_phy_mode_override(phydev);
- if (phy_mode_override >= 0) { - phydev->interface = (phy_interface_t) phy_mode_override; - }
reg_val = adin_ext_read(phydev, ADIN1300_GE_RGMII_CFG);
The patch that introduced adin_get_phy_mode_override was tested against the U-Boot master branch. Unfortunately, phy_get_interface_by_name was removed just a few days before by: https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com...
:(
Can we fix adin_get_phy_mode_override instead of removing it?
I will take a look in the coming days and see if I understand how to fix this. I don't really mind if we fix it, or remove it - removing was the faster
If you prefer, I can submit a separate patch to fix it. I'll wait to hear from you to avoid duplicated work.
I won't be able to look at this before Sunday, so feel free to work on it.
way getting my patch-set out of course ...
We can implement part of ofnode_read_phy_mode. This needs to be tested, but for example:
diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c index cff841ab3d..6a05b9fd05 100644 --- a/drivers/net/phy/adin.c +++ b/drivers/net/phy/adin.c @@ -100,27 +100,28 @@ static u32 adin_get_reg_value(struct phy_device *phydev, * The function gets phy-mode string from property 'adi,phy-mode- override' * and return its index in phy_interface_strings table, or -1 in error case. */ -int adin_get_phy_mode_override(struct phy_device *phydev) +phy_interface_t adin_get_phy_mode_override(struct phy_device *phydev) { ofnode node = phy_get_ofnode(phydev); const char *phy_mode_override; const char *prop_phy_mode_override = "adi,phy-mode- override"; - int override_interface; + int i;
phy_mode_override = ofnode_read_string(node, prop_phy_mode_override);
if (!phy_mode_override) - return -ENODEV; + return PHY_INTERFACE_MODE_NA;
debug("%s: %s = '%s'\n", __func__, prop_phy_mode_override, phy_mode_override);
- override_interface = phy_get_interface_by_name(phy_mode_override); + for (i = 0; i < PHY_INTERFACE_MODE_MAX; i++) + if (!strcmp(phy_mode_override, phy_interface_strings[i])) + return i;
- if (override_interface < 0) - printf("%s: %s = '%s' is not valid\n", - __func__, prop_phy_mode_override, phy_mode_override); + debug("%s: Invalid PHY interface '%s'\n", __func__, phy_mode_override);
- return override_interface; + return PHY_INTERFACE_MODE_NA; }
static u16 adin_ext_read(struct phy_device *phydev, const u32 regnum) @@ -148,10 +149,10 @@ static int adin_config_rgmii_mode(struct phy_device *phydev) { u16 reg_val; u32 val; - int phy_mode_override = adin_get_phy_mode_override(phydev); + phy_interface_t phy_mode_override = adin_get_phy_mode_override(phydev);
- if (phy_mode_override >= 0) { - phydev->interface = (phy_interface_t) phy_mode_override; + if (phy_mode_override != PHY_INTERFACE_MODE_NA) { + phydev->interface = phy_mode_override; }
reg_val = adin_ext_read(phydev, ADIN1300_GE_RGMII_CFG);
Thanks, Nate
Thanks, Nate
Hi Nate, Maintainers,
This patch is now superseded by Nate's [PATCH] phy: adin: fix broken support for adi,phy-mode-override submitted yesterday.
The remainder of this patchset cleanly applies on top of it though, so I won't roll v2.
Let me know if there is any further feedback on this series, or if it can be applied.
Am 05.05.22 um 18:30 schrieb Josua Mayer:
\o/
Am 04.05.22 um 15:46 schrieb Nate Drude:
Hi Josua,
On Wed, 2022-05-04 at 11:51 +0300, Josua Mayer wrote:
Hi Nate,
Am 02.05.22 um 16:25 schrieb Nate Drude:
Hi Josua,
On Sun, 2022-05-01 at 15:41 +0300, Josua Mayer wrote:
The adin_get_phy_mode_override function does not compile, because it is missing both declaration and implementation of phy_get_interface_by_name.
Remove the whole function for now, since the missing implementation is not included in mainline Linux - and thus can not be copied.
Signed-off-by: Josua Mayer josua@solid-run.com
drivers/net/phy/adin.c | 34 ---------------------------------- 1 file changed, 34 deletions(-)
diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c index cff841ab3d..2433e76fea 100644 --- a/drivers/net/phy/adin.c +++ b/drivers/net/phy/adin.c @@ -94,35 +94,6 @@ static u32 adin_get_reg_value(struct phy_device *phydev, return rc; } -/**
- adin_get_phy_mode_override - Get phy-mode override for adin
PHY
- The function gets phy-mode string from property 'adi,phy-
mode- override'
- and return its index in phy_interface_strings table, or -1 in
error case.
- */
-int adin_get_phy_mode_override(struct phy_device *phydev) -{ - ofnode node = phy_get_ofnode(phydev); - const char *phy_mode_override; - const char *prop_phy_mode_override = "adi,phy-mode- override"; - int override_interface;
- phy_mode_override = ofnode_read_string(node, prop_phy_mode_override); - if (!phy_mode_override) - return -ENODEV;
- debug("%s: %s = '%s'\n", - __func__, prop_phy_mode_override, phy_mode_override);
- override_interface = phy_get_interface_by_name(phy_mode_override);
- if (override_interface < 0) - printf("%s: %s = '%s' is not valid\n", - __func__, prop_phy_mode_override, phy_mode_override);
- return override_interface; -}
static u16 adin_ext_read(struct phy_device *phydev, const u32 regnum) { u16 val; @@ -148,11 +119,6 @@ static int adin_config_rgmii_mode(struct phy_device *phydev) { u16 reg_val; u32 val; - int phy_mode_override = adin_get_phy_mode_override(phydev);
- if (phy_mode_override >= 0) { - phydev->interface = (phy_interface_t) phy_mode_override; - } reg_val = adin_ext_read(phydev, ADIN1300_GE_RGMII_CFG);
The patch that introduced adin_get_phy_mode_override was tested against the U-Boot master branch. Unfortunately, phy_get_interface_by_name was removed just a few days before by: https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com...
:(
Can we fix adin_get_phy_mode_override instead of removing it?
I will take a look in the coming days and see if I understand how to fix this. I don't really mind if we fix it, or remove it - removing was the faster
If you prefer, I can submit a separate patch to fix it. I'll wait to hear from you to avoid duplicated work.
I won't be able to look at this before Sunday, so feel free to work on it.
way getting my patch-set out of course ...
We can implement part of ofnode_read_phy_mode. This needs to be tested, but for example:
diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c index cff841ab3d..6a05b9fd05 100644 --- a/drivers/net/phy/adin.c +++ b/drivers/net/phy/adin.c @@ -100,27 +100,28 @@ static u32 adin_get_reg_value(struct phy_device *phydev, * The function gets phy-mode string from property 'adi,phy-mode- override' * and return its index in phy_interface_strings table, or -1 in error case. */ -int adin_get_phy_mode_override(struct phy_device *phydev) +phy_interface_t adin_get_phy_mode_override(struct phy_device *phydev) { ofnode node = phy_get_ofnode(phydev); const char *phy_mode_override; const char *prop_phy_mode_override = "adi,phy-mode- override"; - int override_interface; + int i; phy_mode_override = ofnode_read_string(node, prop_phy_mode_override);
if (!phy_mode_override) - return -ENODEV; + return PHY_INTERFACE_MODE_NA; debug("%s: %s = '%s'\n", __func__, prop_phy_mode_override, phy_mode_override); - override_interface = phy_get_interface_by_name(phy_mode_override); + for (i = 0; i < PHY_INTERFACE_MODE_MAX; i++) + if (!strcmp(phy_mode_override, phy_interface_strings[i])) + return i; - if (override_interface < 0) - printf("%s: %s = '%s' is not valid\n", - __func__, prop_phy_mode_override, phy_mode_override); + debug("%s: Invalid PHY interface '%s'\n", __func__, phy_mode_override); - return override_interface; + return PHY_INTERFACE_MODE_NA; } static u16 adin_ext_read(struct phy_device *phydev, const u32 regnum) @@ -148,10 +149,10 @@ static int adin_config_rgmii_mode(struct phy_device *phydev) { u16 reg_val; u32 val; - int phy_mode_override = adin_get_phy_mode_override(phydev); + phy_interface_t phy_mode_override = adin_get_phy_mode_override(phydev); - if (phy_mode_override >= 0) { - phydev->interface = (phy_interface_t) phy_mode_override; + if (phy_mode_override != PHY_INTERFACE_MODE_NA) { + phydev->interface = phy_mode_override; } reg_val = adin_ext_read(phydev, ADIN1300_GE_RGMII_CFG);
Thanks, Nate
Thanks, Nate
- Josua Mayer
The ADIN1300 supports generating certain clocks on its GP_CLK pin, as well as providing the reference clock on CLK25_REF.
Add support for selecting the clock via device-tree properties.
This patch is based on just submitted patches to Linux [1] [2].
[1] https://patchwork.kernel.org/project/netdevbpf/patch/20220428082848.12191-2-... [2] https://patchwork.kernel.org/project/netdevbpf/patch/20220428082848.12191-4-...
Signed-off-by: Josua Mayer josua@solid-run.com --- drivers/net/phy/adin.c | 46 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+)
diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c index 2433e76fea..485430b128 100644 --- a/drivers/net/phy/adin.c +++ b/drivers/net/phy/adin.c @@ -4,6 +4,7 @@ * * Copyright 2019 Analog Devices Inc. * Copyright 2022 Variscite Ltd. + * Copyright 2022 Josua Mayer josua@solid-run.com */ #include <common.h> #include <phy.h> @@ -13,6 +14,16 @@ #define PHY_ID_ADIN1300 0x0283bc30 #define ADIN1300_EXT_REG_PTR 0x10 #define ADIN1300_EXT_REG_DATA 0x11 + +#define ADIN1300_GE_CLK_CFG_REG 0xff1f +#define ADIN1300_GE_CLK_CFG_MASK GENMASK(5, 0) +#define ADIN1300_GE_CLK_CFG_RCVR_125 BIT(5) +#define ADIN1300_GE_CLK_CFG_FREE_125 BIT(4) +#define ADIN1300_GE_CLK_CFG_REF_EN BIT(3) +#define ADIN1300_GE_CLK_CFG_HRT_RCVR BIT(2) +#define ADIN1300_GE_CLK_CFG_HRT_FREE BIT(1) +#define ADIN1300_GE_CLK_CFG_25 BIT(0) + #define ADIN1300_GE_RGMII_CFG 0xff23 #define ADIN1300_GE_RGMII_RX_MSK GENMASK(8, 6) #define ADIN1300_GE_RGMII_RX_SEL(x) \ @@ -115,6 +126,37 @@ static int adin_ext_write(struct phy_device *phydev, const u32 regnum, const u16 return phy_write(phydev, MDIO_DEVAD_NONE, ADIN1300_EXT_REG_DATA, val); }
+static int adin_config_clk_out(struct phy_device *phydev) +{ + ofnode node = phy_get_ofnode(phydev); + const char *val = NULL; + u8 sel = 0; + + val = ofnode_read_string(node, "adi,phy-output-clock"); + if (!val) { + /* property not present, do not enable GP_CLK pin */ + } else if (strcmp(val, "25mhz-reference") == 0) { + sel |= ADIN1300_GE_CLK_CFG_25; + } else if (strcmp(val, "125mhz-free-running") == 0) { + sel |= ADIN1300_GE_CLK_CFG_FREE_125; + } else if (strcmp(val, "125mhz-recovered") == 0) { + sel |= ADIN1300_GE_CLK_CFG_RCVR_125; + } else if (strcmp(val, "adaptive-free-running") == 0) { + sel |= ADIN1300_GE_CLK_CFG_HRT_FREE; + } else if (strcmp(val, "adaptive-recovered") == 0) { + sel |= ADIN1300_GE_CLK_CFG_HRT_RCVR; + } else { + pr_err("%s: invalid adi,phy-output-clock\n", __func__); + return -EINVAL; + } + + if (ofnode_read_bool(node, "adi,phy-output-reference-clock")) + sel |= ADIN1300_GE_CLK_CFG_REF_EN; + + return adin_ext_write(phydev, ADIN1300_GE_CLK_CFG_REG, + ADIN1300_GE_CLK_CFG_MASK & sel); +} + static int adin_config_rgmii_mode(struct phy_device *phydev) { u16 reg_val; @@ -168,6 +210,10 @@ static int adin1300_config(struct phy_device *phydev)
printf("ADIN1300 PHY detected at addr %d\n", phydev->addr);
+ ret = adin_config_clk_out(phydev); + if (ret < 0) + return ret; + ret = adin_config_rgmii_mode(phydev);
if (ret < 0)
The Cubox has an unstable phy address - which can appear at either address 0 (intended) or 4 (unintended).
SoM revision 1.9 has replaced the ar8035 phy with an adin1300, which will always appear at address 1.
Change the reg property of the phy node to the magic value 0xffffffff, which indicates to the generic phy driver that all addresses should be probed. That allows the same node (which is pinned by phy-handle) to match either the AR8035 PHY at both possible addresses, as well as the new one at address 1. Also add the new adi,phy-output-clock property for enabling the 125MHz clock used by the fec ethernet controller, as submitted to Linux [1].
Linux solves this problem differently: For the ar8035 phy it will probe both phy nodes in device-tree in order, and use the one that succeeds. For the new adin1300 it expects U-Boot to patch the status field in the DTB before booting
While at it also sync the reset-delay with the upstream Linux dtb.
[1] https://patchwork.kernel.org/project/netdevbpf/patch/20220428082848.12191-4-...
Signed-off-by: Josua Mayer josua@solid-run.com --- arch/arm/dts/imx6qdl-sr-som.dtsi | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/arch/arm/dts/imx6qdl-sr-som.dtsi b/arch/arm/dts/imx6qdl-sr-som.dtsi index b06577808f..c20bed2721 100644 --- a/arch/arm/dts/imx6qdl-sr-som.dtsi +++ b/arch/arm/dts/imx6qdl-sr-som.dtsi @@ -55,7 +55,13 @@ pinctrl-0 = <&pinctrl_microsom_enet_ar8035>; phy-handle = <&phy>; phy-mode = "rgmii-id"; - phy-reset-duration = <2>; + + /* + * The PHY seems to require a long-enough reset duration to avoid + * some rare issues where the PHY gets stuck in an inconsistent and + * non-functional state at boot-up. 10ms proved to be fine . + */ + phy-reset-duration = <10>; phy-reset-gpios = <&gpio4 15 GPIO_ACTIVE_LOW>; status = "okay";
@@ -64,8 +70,15 @@ #size-cells = <0>;
phy: ethernet-phy@0 { - reg = <0>; + /* + * The PHY can appear either: + * - AR8035: at address 0 or 4 + * - ADIN1300: at address 1 + * Actual address being detected at runtime. + */ + reg = <0xffffffff>; qca,clk-out-frequency = <125000000>; + adi,phy-output-clock = "125mhz-free-running"; }; }; };
SoM revision 1.9 has replaced the ar8035 phy address 0 with an adin1300 at address 1. Because early SoMs had a hardware flaw, the ar8035 can also appear at address 4 - making it a total of 3 phy nodes in the DTB.
To avoid confusing Linux with probe errors, fixup the dtb to only enable the phy node that is detected at runtime.
Signed-off-by: Josua Mayer josua@solid-run.com --- board/solidrun/mx6cuboxi/mx6cuboxi.c | 78 ++++++++++++++++++++++++++++ configs/mx6cuboxi_defconfig | 1 + 2 files changed, 79 insertions(+)
diff --git a/board/solidrun/mx6cuboxi/mx6cuboxi.c b/board/solidrun/mx6cuboxi/mx6cuboxi.c index 6207bf8253..42aa5cb63c 100644 --- a/board/solidrun/mx6cuboxi/mx6cuboxi.c +++ b/board/solidrun/mx6cuboxi/mx6cuboxi.c @@ -1,5 +1,7 @@ // SPDX-License-Identifier: GPL-2.0+ /* + * Copyright (C) 2022 Josua Mayer josua@solid-run.com + * * Copyright (C) 2015 Freescale Semiconductor, Inc. * * Author: Fabio Estevam fabio.estevam@freescale.com @@ -39,6 +41,8 @@ #include <spl.h> #include <usb.h> #include <usb/ehci-ci.h> +#include <netdev.h> +#include <phy.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -407,6 +411,80 @@ out: return 0; }
+static int find_ethernet_phy(void) +{ + struct mii_dev *bus = NULL; + struct phy_device *phydev = NULL; + int phy_addr = -ENOENT; + +#ifdef CONFIG_FEC_MXC + bus = fec_get_miibus(ENET_BASE_ADDR, -1); + if (!bus) + return -ENOENT; + + // scan address 0, 1, 4 + phydev = phy_find_by_mask(bus, 0b00010011); + if (!phydev) { + free(bus); + return -ENOENT; + } + pr_debug("%s: detected ethernet phy at address %d\n", __func__, phydev->addr); + phy_addr = phydev->addr; + + free(phydev); +#endif + + return phy_addr; +} + +#if defined(CONFIG_OF_LIBFDT) && defined(CONFIG_OF_BOARD_SETUP) +/* + * Configure the correct ethernet PHYs nodes in device-tree: + * - AR8035 at addresses 0 or 4: Cubox + * - AR8035 at address 0: HummingBoard, HummingBoard 2 + * - ADIN1300 at address 1: since SoM rev 1.9 + */ +int ft_board_setup(void *fdt, struct bd_info *bd) +{ + int node_phy0, node_phy1, node_phy4; + int ret, phy; + bool enable_phy0 = false, enable_phy1 = false, enable_phy4 = false; + + // detect phy + phy = find_ethernet_phy(); + if (phy == 0 || phy == 4) { + enable_phy0 = true; + switch (board_type()) { + case CUBOXI: + case UNKNOWN: + default: + enable_phy4 = true; + } + } else if (phy == 1) { + enable_phy1 = true; + } else { + pr_err("%s: couldn't detect ethernet phy, not patching dtb!\n", __func__); + return 0; + } + + // update all phy nodes status + node_phy0 = fdt_path_offset(fdt, "/soc/bus@2100000/ethernet@2188000/mdio/ethernet-phy@0"); + ret = fdt_setprop_string(fdt, node_phy0, "status", enable_phy0 ? "okay" : "disabled"); + if (ret < 0 && enable_phy0) + pr_err("%s: failed to enable ethernet phy at address 0 in dtb!\n", __func__); + node_phy1 = fdt_path_offset(fdt, "/soc/bus@2100000/ethernet@2188000/mdio/ethernet-phy@1"); + ret = fdt_setprop_string(fdt, node_phy1, "status", enable_phy1 ? "okay" : "disabled"); + if (ret < 0 && enable_phy1) + pr_err("%s: failed to enable ethernet phy at address 1 in dtb!\n", __func__); + node_phy4 = fdt_path_offset(fdt, "/soc/bus@2100000/ethernet@2188000/mdio/ethernet-phy@4"); + ret = fdt_setprop_string(fdt, node_phy4, "status", enable_phy4 ? "okay" : "disabled"); + if (ret < 0 && enable_phy4) + pr_err("%s: failed to enable ethernet phy at address 4 in dtb!\n", __func__); + + return 0; +} +#endif + /* Override the default implementation, DT model is not accurate */ int show_board_info(void) { diff --git a/configs/mx6cuboxi_defconfig b/configs/mx6cuboxi_defconfig index 1e2e332af9..d3ac8eeeba 100644 --- a/configs/mx6cuboxi_defconfig +++ b/configs/mx6cuboxi_defconfig @@ -22,6 +22,7 @@ CONFIG_CMD_HDMIDETECT=y CONFIG_AHCI=y CONFIG_DISTRO_DEFAULTS=y CONFIG_FIT=y +CONFIG_OF_BOARD_SETUP=y CONFIG_BOOTCOMMAND="run findfdt; run finduuid; run distro_bootcmd" CONFIG_USE_PREBOOT=y CONFIG_PREBOOT="if hdmidet; then usb start; setenv stdin serial,usbkbd; setenv stdout serial,vidconsole; setenv stderr serial,vidconsole; else setenv stdin serial; setenv stdout serial; setenv stderr serial; fi;"
Since SoMs revision 1.9 the ar8038 phy has been replaced by adin1300. Enable the driver so that the new SoMs have functional networking.
Signed-off-by: Josua Mayer josua@solid-run.com --- configs/mx6cuboxi_defconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/configs/mx6cuboxi_defconfig b/configs/mx6cuboxi_defconfig index d3ac8eeeba..46634a1727 100644 --- a/configs/mx6cuboxi_defconfig +++ b/configs/mx6cuboxi_defconfig @@ -55,6 +55,7 @@ CONFIG_DWC_AHSATA=y CONFIG_SPL_SYS_I2C_LEGACY=y CONFIG_FSL_USDHC=y CONFIG_PHYLIB=y +CONFIG_PHY_ADIN=y CONFIG_PHY_ATHEROS=y CONFIG_DM_ETH=y CONFIG_FEC_MXC=y
Greetings everybody,
The changes to the adin phy device-tree bindings and driver have been applied to netdev/net-next: https://patchwork.kernel.org/project/netdevbpf/cover/20220517085143.3749-1-j...
As there have been no further comments, I will send v2 soon including some changes the Linux maintainers asked for - and drop the first patch now that Nate Drude has submitted a fix: phy: adin: fix broken support for adi,phy-mode-override
sincerely Josua Mayer
Am 01.05.22 um 15:41 schrieb Josua Mayer:
As of Revision 1.9 the SolidRun i.MX6 SoMs ship with a new PHY - an ADIN1300 at address 1. This patchset carries many small parts to facilitate proper operation of the ethernet port in U-Boot as well as Linux:
- Fix a compile error in the recently phy driver
- Add support for configuring the clock output pins to the phy driver
- update device-tree with support for the neew phy
- support runtime patching of device-tree for Linux: enables only the phy node detected during boot, to avoid warning messages during boot.
- finally enable the phy driver in the cuboxi defconfig
Josua Mayer (5): phy: adin: remove broken support for adi,phy-mode-override phy: adin: add support for clock output ARM: dts: imx6qdl-sr-som: add support for alternate phy addresses mx6cuboxi: fixup dtb ethernet phy nodes before booting an OS mx6cuboxi: enable driver for adin1300 phy
arch/arm/dts/imx6qdl-sr-som.dtsi | 17 +++++- board/solidrun/mx6cuboxi/mx6cuboxi.c | 78 +++++++++++++++++++++++++++ configs/mx6cuboxi_defconfig | 2 + drivers/net/phy/adin.c | 80 ++++++++++++++++------------ 4 files changed, 141 insertions(+), 36 deletions(-)
participants (2)
-
Josua Mayer -
Nate Drude