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

[Roger Quadros]

On 20/07/2023 12:55, 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 device trees since Linux 6.5-rc1 which will
> put some pinctrl states on the MDIO device tree node.
> 
> Let's rework the driver a bit to fetch the pinctrl state from the MDIO
> node and apply it.
> 
> Signed-off-by: Maxime Ripard <mripard at kernel.org>
> ---
>  drivers/net/ti/am65-cpsw-nuss.c | 49 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
> 
> diff --git a/drivers/net/ti/am65-cpsw-nuss.c b/drivers/net/ti/am65-cpsw-nuss.c
> index f674b0baa359..2d579bf4af2c 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>
> @@ -696,6 +697,50 @@ out:
>  	return ret;
>  }
>  
> +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_setup_mdio(struct udevice *dev)
> +{
> +	ofnode mdio;
> +	int ret;
> +
> +	mdio = am65_cpsw_find_mdio(dev_ofnode(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.
> +	 */
Please mention this as a HACK: that needs to go away when davinci_mdio
and this driver properly supports MDIO infrastructure.

> +	ret = pinctrl_select_state_by_ofnode(dev, mdio, "default");

Instead of modifying the API, can we have a dummy driver for the "ti,davinci_mdio"
device that registers as class UCLASS_MDIO but does nothing else?

Then you can drop patch 1 and instead do

	ret = uclass_get_device_by_ofnode(UCLASS_MDIO, node, &mdio_dev);
        if (!ret)
		pinctrl_select_state(mdio_dev, "default");

	
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
>  static int am65_cpsw_probe_nuss(struct udevice *dev)
>  {
>  	struct am65_cpsw_common *cpsw_common = dev_get_priv(dev);
> @@ -728,6 +773,10 @@ static int am65_cpsw_probe_nuss(struct udevice *dev)
>  				AM65_CPSW_CPSW_NU_ALE_BASE;
>  	cpsw_common->mdio_base = cpsw_common->ss_base + AM65_CPSW_MDIO_BASE;
>  
> +	ret = am65_cpsw_setup_mdio(dev);
> +	if (ret)
> +		goto out;
> +
>  	ports_np = dev_read_subnode(dev, "ethernet-ports");
>  	if (!ofnode_valid(ports_np)) {
>  		ret = -ENOENT;
> 

-- 
cheers,
-roger