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

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

[Maxime Ripard]

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.org

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.org

---
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,
-- 
Maxime Ripard <mripard@kernel.org>

[PATCH v3 1/3] net: ti: am65-cpsw-nuss: Enforce pinctrl state on the MDIO child node

[Maxime Ripard]

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,
+};

-- 
2.41.0

[PATCH v3 2/3] fixup! arm: dts: k3-am62: Bump dtsi from linux v6.5-rc1

[Maxime Ripard]

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 {

-- 
2.41.0

[PATCH v3 3/3] fixup! net: ti: am65-cpsw-nuss: Enforce pinctrl state on the MDIO child node

[Maxime Ripard]

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

-- 
2.41.0

Re: [PATCH v3 3/3] fixup! net: ti: am65-cpsw-nuss: Enforce pinctrl state on the MDIO child node

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 250 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]