[PATCH v3 0/3] net: ti: am65-cpsw-nuss: Fix DT binding handling of pinctrl

Hi,
This series is based on: https://lore.kernel.org/all/20230713072019.3153871-1-nm@ti.com/ https://lore.kernel.org/u-boot/20230614222853.574427-1-dannenberg@ti.com/ https://lore.kernel.org/u-boot/20230722193151.117345-1-rogerq@kernel.org/
It fixes the issue of Linux booting from the DT embedded by U-boot. The main issue there is that U-Boot doesn't handle the MDIO child node that might have resources attached to it.
Thus, any pinctrl configuration that could be attached to the MDIO child node is effectively ignored. Unfortunately, starting with 6.5-rc1, Linux does just that.
This was solved by duplicating the pinctrl configuration onto the MAC device node. Unfortunately, this doesn't work for Linux since now it has two devices competing for the same pins.
Let me know what you think, Maxime
Signed-off-by: Maxime Ripard mripard@kernel.org --- Changes in v3: - Rebase on top of latest Roger series - Link to v2: https://lore.kernel.org/r/20230721-ti-mdio-pinmux-v2-0-4bb443e09ac0@kernel.o...
Changes in v2: - Drop the pinctrl API change in favour of a dummy driver - Link to v1: https://lore.kernel.org/r/20230720-ti-mdio-pinmux-v1-0-0bd3bd1cf759@kernel.o...
--- Maxime Ripard (3): net: ti: am65-cpsw-nuss: Enforce pinctrl state on the MDIO child node fixup! arm: dts: k3-am62: Bump dtsi from linux v6.5-rc1 fixup! net: ti: am65-cpsw-nuss: Enforce pinctrl state on the MDIO child node
arch/arm/dts/k3-am625-sk-u-boot.dtsi | 1 - drivers/net/ti/Kconfig | 1 + drivers/net/ti/am65-cpsw-nuss.c | 60 ++++++++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 1 deletion(-) --- base-commit: db7126c87c44c45c448c68925d366510f7a09b11 change-id: 20230720-ti-mdio-pinmux-a12525dba973
Best regards,

The binding represents the MDIO controller as a child device tree node of the MAC device tree node.
The U-Boot driver mostly ignores that child device tree node and just hardcodes the resources it uses to support both the MAC and MDIO in a single driver.
However, some resources like pinctrl muxing states are thus ignored. This has been a problem with some device trees that will put some pinctrl states on the MDIO device tree node, like the SK-AM62 Device Tree does.
Let's rework the driver a bit to create a dummy MDIO driver that we will then get during our initialization to force the core to select the right muxing.
Signed-off-by: Maxime Ripard mripard@kernel.org --- drivers/net/ti/Kconfig | 1 + drivers/net/ti/am65-cpsw-nuss.c | 67 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+)
diff --git a/drivers/net/ti/Kconfig b/drivers/net/ti/Kconfig index d9f1c019a885..02660e4fbb44 100644 --- a/drivers/net/ti/Kconfig +++ b/drivers/net/ti/Kconfig @@ -41,6 +41,7 @@ endchoice config TI_AM65_CPSW_NUSS bool "TI K3 AM65x MCU CPSW Nuss Ethernet controller driver" depends on ARCH_K3 + imply DM_MDIO imply MISC_INIT_R imply MISC imply SYSCON diff --git a/drivers/net/ti/am65-cpsw-nuss.c b/drivers/net/ti/am65-cpsw-nuss.c index ce52106e5238..29a4284a9b70 100644 --- a/drivers/net/ti/am65-cpsw-nuss.c +++ b/drivers/net/ti/am65-cpsw-nuss.c @@ -15,6 +15,7 @@ #include <dm.h> #include <dm/device_compat.h> #include <dm/lists.h> +#include <dm/pinctrl.h> #include <dma-uclass.h> #include <dm/of_access.h> #include <miiphy.h> @@ -605,14 +606,69 @@ static const struct soc_attr k3_mdio_soc_data[] = { { /* sentinel */ }, };
+static ofnode am65_cpsw_find_mdio(ofnode parent) +{ + ofnode node; + + ofnode_for_each_subnode(node, parent) + if (ofnode_device_is_compatible(node, "ti,cpsw-mdio")) + return node; + + return ofnode_null(); +} + +static int am65_cpsw_mdio_setup(struct udevice *dev) +{ + struct am65_cpsw_priv *priv = dev_get_priv(dev); + struct am65_cpsw_common *cpsw_common = priv->cpsw_common; + struct udevice *mdio_dev; + ofnode mdio; + int ret; + + mdio = am65_cpsw_find_mdio(dev_ofnode(cpsw_common->dev)); + if (!ofnode_valid(mdio)) + return -ENODEV; + + /* + * The MDIO controller is represented in the DT binding by a + * subnode of the MAC controller. + * + * Our driver largely ignores that and supports MDIO by + * hardcoding the register offsets. + * + * However, some resources (clocks, pinctrl) might be tied to + * the MDIO node, and thus ignored. + * + * Clocks are not a concern at the moment since it's shared + * between the MAC and MDIO, so the driver handles both at the + * same time. + * + * However, we do need to make sure the pins states tied to the + * MDIO node are configured properly. Fortunately, the core DM + * does that for use when we get a device, so we can work around + * that whole issue by just requesting a dummy MDIO driver to + * probe, and our pins will get muxed. + */ + ret = uclass_get_device_by_ofnode(UCLASS_MDIO, mdio, &mdio_dev); + if (ret) + return ret; + + return 0; +} + static int am65_cpsw_mdio_init(struct udevice *dev) { struct am65_cpsw_priv *priv = dev_get_priv(dev); struct am65_cpsw_common *cpsw_common = priv->cpsw_common; + int ret;
if (!priv->has_phy || cpsw_common->bus) return 0;
+ ret = am65_cpsw_mdio_setup(dev); + if (ret) + return ret; + cpsw_common->bus = cpsw_mdio_init(dev->name, cpsw_common->mdio_base, cpsw_common->bus_freq, @@ -868,3 +924,14 @@ U_BOOT_DRIVER(am65_cpsw_nuss_port) = { .plat_auto = sizeof(struct eth_pdata), .flags = DM_FLAG_ALLOC_PRIV_DMA | DM_FLAG_OS_PREPARE, }; + +static const struct udevice_id am65_cpsw_mdio_ids[] = { + { .compatible = "ti,cpsw-mdio" }, + { } +}; + +U_BOOT_DRIVER(am65_cpsw_mdio) = { + .name = "am65_cpsw_mdio", + .id = UCLASS_MDIO, + .of_match = am65_cpsw_mdio_ids, +};

The MDIO pinctrl nodes can't be duplicated between the child and device, because if we ever boot Linux with our DT it will try to attach that pinctrl configuration to both the MAC and MDIO devices, which will result in failure to probe.
Signed-off-by: Maxime Ripard mripard@kernel.org --- arch/arm/dts/k3-am625-sk-u-boot.dtsi | 1 - 1 file changed, 1 deletion(-)
diff --git a/arch/arm/dts/k3-am625-sk-u-boot.dtsi b/arch/arm/dts/k3-am625-sk-u-boot.dtsi index a3f207baa304..c1685bc9ca39 100644 --- a/arch/arm/dts/k3-am625-sk-u-boot.dtsi +++ b/arch/arm/dts/k3-am625-sk-u-boot.dtsi @@ -120,7 +120,6 @@
&cpsw3g { bootph-pre-ram; - pinctrl-0 = <&main_mdio1_pins_default &main_rgmii1_pins_default>; };
&cpsw_port1 {

--- drivers/net/ti/am65-cpsw-nuss.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ti/am65-cpsw-nuss.c b/drivers/net/ti/am65-cpsw-nuss.c index 29a4284a9b70..51a8167d14a9 100644 --- a/drivers/net/ti/am65-cpsw-nuss.c +++ b/drivers/net/ti/am65-cpsw-nuss.c @@ -627,21 +627,14 @@ static int am65_cpsw_mdio_setup(struct udevice *dev)
mdio = am65_cpsw_find_mdio(dev_ofnode(cpsw_common->dev)); if (!ofnode_valid(mdio)) - return -ENODEV; + return 0;
/* * The MDIO controller is represented in the DT binding by a * subnode of the MAC controller. * - * Our driver largely ignores that and supports MDIO by - * hardcoding the register offsets. - * - * However, some resources (clocks, pinctrl) might be tied to - * the MDIO node, and thus ignored. - * - * Clocks are not a concern at the moment since it's shared - * between the MAC and MDIO, so the driver handles both at the - * same time. + * We don't have a DM driver for the MDIO device yet, and thus any + * pinctrl setting on its node will be ignored. * * However, we do need to make sure the pins states tied to the * MDIO node are configured properly. Fortunately, the core DM

Hi,
On Mon, Jul 24, 2023 at 03:52:54PM +0200, Maxime Ripard wrote:
drivers/net/ti/am65-cpsw-nuss.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-)
Sorry, it should have been squashed. I'll send a v4 shortly.
Maxime
participants (1)
-
Maxime Ripard