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

[Maxime Ripard]

On Thu, Jul 20, 2023 at 06:47:43PM +0300, Roger Quadros wrote:
> 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.

Will do

> > +	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");

Good idea, thanks :)

Maxime
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20230721/27586481/attachment.sig>