[PATCH 0/5] mx6cuboxi: add eth support for SoMs revision 1.9+

[PATCH 0/5] mx6cuboxi: add eth support for SoMs revision 1.9+

[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:

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(-)

-- 
2.34.1

[PATCH 1/5] phy: adin: remove broken support for adi, phy-mode-override

[Josua Mayer]

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);
 
-- 
2.34.1

[PATCH 2/5] phy: adin: add support for clock output

[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-josua@solid-run.com/
[2] https://patchwork.kernel.org/project/netdevbpf/patch/20220428082848.12191-4-josua@solid-run.com/

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)
-- 
2.34.1

[PATCH 3/5] ARM: dts: imx6qdl-sr-som: add support for alternate phy addresses

[Josua Mayer]

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-josua@solid-run.com/

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";
 		};
 	};
 };
-- 
2.34.1

[PATCH 4/5] mx6cuboxi: fixup dtb ethernet phy nodes before booting an OS

[Josua Mayer]

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;"
-- 
2.34.1

[PATCH 5/5] mx6cuboxi: enable driver for adin1300 phy

[Josua Mayer]

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
-- 
2.34.1

Re: [PATCH 1/5] phy: adin: remove broken support for adi,phy-mode-override

[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/123ca114e07ecf28aa2538748d733e2b22d8b8b5

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

Re: [PATCH 1/5] phy: adin: remove broken support for adi,phy-mode-override

[Josua Mayer]

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/123ca114e07ecf28aa2538748d733e2b22d8b8b5
:(
> 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

Re: [PATCH 1/5] phy: adin: remove broken support for adi,phy-mode-override

[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%2Fu-boot%2Fu-boot%2Fcommit%2F123ca114e07ecf28aa2538748d733e2b22d8b8b5&amp;data=05%7C01%7CNate.D%40variscite.com%7C467af650626f4902ae2b08da2dab6225%7C399ae6ac38f44ef094a8440b0ad581de%7C1%7C0%7C637872511359995609%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=Ok3YOO8rheEZuRa%2Fn04NZf37TsPQ7hzArUojFZ4QDUg%3D&amp;reserved=0
> :(
> > 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

Re: [PATCH 1/5] phy: adin: remove broken support for adi,phy-mode-override

[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%2Fu-boot%2Fu-boot%2Fcommit%2F123ca114e07ecf28aa2538748d733e2b22d8b8b5&amp;data=05%7C01%7CNate.D%40variscite.com%7C467af650626f4902ae2b08da2dab6225%7C399ae6ac38f44ef094a8440b0ad581de%7C1%7C0%7C637872511359995609%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=Ok3YOO8rheEZuRa%2Fn04NZf37TsPQ7hzArUojFZ4QDUg%3D&amp;reserved=0
>> :(
>>> 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
>

Re: [PATCH 1/5] phy: adin: remove broken support for adi,phy-mode-override

[Josua Mayer]

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%2Fu-boot%2Fu-boot%2Fcommit%2F123ca114e07ecf28aa2538748d733e2b22d8b8b5&amp;data=05%7C01%7CNate.D%40variscite.com%7C467af650626f4902ae2b08da2dab6225%7C399ae6ac38f44ef094a8440b0ad581de%7C1%7C0%7C637872511359995609%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=Ok3YOO8rheEZuRa%2Fn04NZf37TsPQ7hzArUojFZ4QDUg%3D&amp;reserved=0 
>>>>
>>> :(
>>>> 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

Re: [PATCH 0/5] mx6cuboxi: add eth support for SoMs revision 1.9+

[Josua Mayer]

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-josua@solid-run.com/

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:
> 
> 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(-)
>