[PATCH v4 1/2] net: ti: am65-cpsw-nuss: Enforce pinctrl state on the MDIO child node
mripard at kernel.org
mripard at kernel.org
Fri Jul 28 15:52:55 CEST 2023
On Fri, Jul 28, 2023 at 01:34:38PM +0000, Marcel Ziswiler wrote:
> Hi Maxime
>
> Just a minor nitpick in the comments below.
>
> On Mon, 2023-07-24 at 15:57 +0200, 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 at kernel.org>
> > ---
> > 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
>
> Does that for us?
>
> > + * 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,
> > +};
>
> However, so far I could not make this work for our use-case [1]. It just keeps crashing. Any ideas?
>
> [snip]
>
> Model: Toradex 0076 Verdin AM62 Quad 2GB WB IT V1.0A
> Serial#: 15037380
> Carrier: Toradex Dahlia V1.1A, Serial# 10763237
> am65_cpsw_nuss ethernet at 8000000: K3 CPSW: nuss_ver: 0x6BA01103 cpsw_ver: 0x6BA81
> 103 ale_ver: 0x00290105 Ports:2 mdio_freq:1000000
> Setting variant to wifi
> Net:
> Warning: ethernet at 8000000port@1 MAC addresses don't match:
> Address in ROM is 1c:63:49:22:5f:f9
> Address in environment is 00:14:2d:e5:73:c4
> eth0: ethernet at 8000000port@1 [PRIME]Could not get PHY for ethernet at 8000000port@1
> : addr 7
> am65_cpsw_nuss_port ethernet at 8000000port@2: phy_connect() failed
>
> Hit any key to stop autoboot: 0
>
> Verdin AM62 # dhcp
> ti-udma dma-controller at 485c0000: k3_dmaring Ring probed rings:150, sci-dev-id:30
> ti-udma dma-controller at 485c0000: dma-ring-reset-quirk: disabled
> am65_cpsw_nuss_port ethernet at 8000000port@1: K3 CPSW: rflow_id_base: 19
> ethernet at 8000000port@1 Waiting for PHY auto negotiation to complete......... TIMEOUT !
> am65_cpsw_nuss_port ethernet at 8000000port@1: phy_startup failed
> am65_cpsw_nuss_port ethernet at 8000000port@1: am65_cpsw_start end error
> "Synchronous Abort" handler, esr 0x96000010, far 0x8000f04
> elr: 00000000808503ac lr : 000000008084ce1c (reloc)
> elr: 000000009bf533ac lr : 000000009bf4fe1c
> x0 : 0000000008000f00 x1 : 0000000000000007
> x2 : 00000000ffffffff x3 : 0000000000000002
> x4 : 000000009bf53388 x5 : 0000000000011d28
> x6 : 0000000000007578 x7 : 0000000099f10570
> x8 : 0000000000000002 x9 : 0000000000000007
> x10: 0000000000000002 x11: 0000000000007578
> x12: 0000000099eeea98 x13: 0000000099eef030
> x14: 0000000000000000 x15: 0000000099eee834
> x16: 000000009bf5d040 x17: 0000000000000000
> x18: 0000000099f00d70 x19: 0000000099eeead4
> x20: 0000000099f10590 x21: 0000000000000007
> x22: 00000000ffffffff x23: 0000000000000001
> x24: 0000000000000080 x25: 0000000000000000
> x26: 00000000ffffffff x27: 000000001fffffff
> x28: 0000000000000000 x29: 0000000099eeea30
>
> Code: 2a0103e9 2a0303e8 910003fd f94000e0 (b9400400)
> Resetting CPU ...
>
> resetting ...
>
> [1] https://lore.kernel.org/all/20230715074050.941051-1-marcel@ziswiler.com
It looks like this series is fairly outdated and won't reasonably work
with these patches. Do you have a branch where you pulled our patches?
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/20230728/c10a6deb/attachment.sig>
More information about the U-Boot
mailing list