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

Hello Maxime,

Thank you for the patch.

On 24/07/23 19:27, Maxime Ripard wrote:

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

LGTM.

Reviewed-by: Siddharth Vadapalli s-vadapalli@ti.com

---

drivers/net/ti/Kconfig | 1 + drivers/net/ti/am65-cpsw-nuss.c | 60 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 61 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..51a8167d14a9 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,62 @@ 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 0;
  

• /*

• * The MDIO controller is represented in the DT binding by a
  

• * subnode of the MAC controller.
  

• *
  

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

• * 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 +917,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,

+};