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 <mrip...@kernel.org>
LGTM.
Reviewed-by: Siddharth Vadapalli <s-vadapa...@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,
> +};
>
--
Regards,
Siddharth.
