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 <[email protected]> > > --- > > 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
signature.asc
Description: PGP signature
