On Thu, Mar 31, 2022 at 10:01 AM Vladimir Oltean vladimir.oltean@nxp.com wrote:
On Tue, Mar 29, 2022 at 03:52:38PM -0700, Tim Harvey wrote:
Add support for DM_MDIO by registering a UCLASS_MDIO driver and attempting to use it. This is necessary if wanting to use a DSA driver for example hanging off of the FEC MAC.
Care is taken to fallback to non DM_MDIO as several boards define DM_MDIO without having the proper device-tree configuration necessary such as an mdio subnode, a phy-mode prop, and either a valid phy-handle prop or fixed-phy subnode.
Signed-off-by: Tim Harvey tharvey@gateworks.com
drivers/net/fec_mxc.c | 113 ++++++++++++++++++++++++++++++++++++++---- drivers/net/fec_mxc.h | 1 + 2 files changed, 104 insertions(+), 10 deletions(-)
diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c index e8ebef09032a..374ef9d67d5e 100644 --- a/drivers/net/fec_mxc.c +++ b/drivers/net/fec_mxc.c @@ -30,6 +30,8 @@ #include <asm/arch/imx-regs.h> #include <asm/mach-imx/sys_proto.h> #include <asm-generic/gpio.h> +#include <dm/device_compat.h> +#include <dm/lists.h>
#include "fec_mxc.h" #include <eth_phy.h> @@ -1025,6 +1027,80 @@ struct mii_dev *fec_get_miibus(ulong base_addr, int dev_id) return bus; }
+#ifdef CONFIG_DM_MDIO +struct dm_fec_mdio_priv {
struct ethernet_regs *regs;+};
+static int dm_fec_mdio_read(struct udevice *dev, int addr, int devad, int reg) +{
struct dm_fec_mdio_priv *priv = dev_get_priv(dev);return fec_mdio_read(priv->regs, addr, reg);+}
+static int dm_fec_mdio_write(struct udevice *dev, int addr, int devad, int reg, u16 data) +{
struct dm_fec_mdio_priv *priv = dev_get_priv(dev);return fec_mdio_write(priv->regs, addr, reg, data);+}
+static const struct mdio_ops dm_fec_mdio_ops = {
.read = dm_fec_mdio_read,.write = dm_fec_mdio_write,+};
+static int dm_fec_mdio_probe(struct udevice *dev) +{
struct dm_fec_mdio_priv *priv = dev_get_priv(dev);priv->regs = (struct ethernet_regs *)ofnode_get_addr(dev_ofnode(dev->parent));return 0;+}
+U_BOOT_DRIVER(fec_mdio) = {
.name = "fec_mdio",.id = UCLASS_MDIO,.probe = dm_fec_mdio_probe,.ops = &dm_fec_mdio_ops,.priv_auto = sizeof(struct dm_fec_mdio_priv),+};
+static int dm_fec_bind_mdio(struct udevice *dev) +{
struct udevice *mdiodev;const char *name;ofnode mdio;int ret;/* for a UCLASS_MDIO driver we need to bind and probe manually* for an internal MDIO bus that has no dt compatible of its own*/ofnode_for_each_subnode(mdio, dev_ofnode(dev)) {name = ofnode_get_name(mdio);if (strcmp(name, "mdio"))continue;ret = device_bind_driver_to_node(dev, "fec_mdio",name, mdio, &mdiodev);if (ret)return ret;/* need to probe it as there is no compatible to do so */ret = uclass_get_device_by_ofnode(UCLASS_MDIO, mdio, &mdiodev);if (!ret)return 0;break;}return ret;+} +#endif
On which branch does this apply? The context above fecmxc_read_rom_hwaddr() is different in the branches I've checked: https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/net/fec_mxc.c#L12... https://source.denx.de/u-boot/custodians/u-boot-net/-/blob/next/drivers/net/...
Sorry, I should have specified in my cover letter that this is on top of next which removes the non dm-eth support from the driver which is quite the cleanup.
Also, after more testing I believe the dm-mdio driver code 'above' is correct but the business of trying to use it and fallback 'below' is wrong and needs work. It doesn't break current users of fec_mxc from what I can tell but it also doesn't properly connect the dm_mdio driver.
static int fecmxc_read_rom_hwaddr(struct udevice *dev) { struct fec_priv *priv = dev_get_priv(dev); @@ -1088,7 +1164,7 @@ static int device_get_phy_addr(struct fec_priv *priv, struct udevice *dev)
static int fec_phy_init(struct fec_priv *priv, struct udevice *dev) {
struct phy_device *phydev;
struct phy_device *phydev = NULL; int addr; addr = device_get_phy_addr(priv, dev);@@ -1096,7 +1172,12 @@ static int fec_phy_init(struct fec_priv *priv, struct udevice *dev) addr = CONFIG_FEC_MXC_PHYADDR; #endif
phydev = phy_connect(priv->bus, addr, dev, priv->interface);+#ifdef CONFIG_DM_MDIO
if (priv->dm_mdio)phydev = dm_eth_phy_connect(dev);+#endif
if (!phydev)phydev = phy_connect(priv->bus, addr, dev, priv->interface); if (!phydev) return -ENODEV;@@ -1227,11 +1308,19 @@ static int fecmxc_probe(struct udevice *dev)
priv->dev_id = dev_seq(dev);+#ifdef CONFIG_DM_MDIO
ret = dm_fec_bind_mdio(dev);if (!ret) {ret = fec_phy_init(priv, dev);if (!ret)priv->dm_mdio = true;}+#endif #ifdef CONFIG_DM_ETH_PHY bus = eth_phy_get_mdio_bus(dev); #endif
if (!bus) {
if (!bus && !priv->dm_mdio) { dm_mii_bus = false;#ifdef CONFIG_FEC_MXC_MDIO_BASE bus = fec_get_miibus((ulong)CONFIG_FEC_MXC_MDIO_BASE, @@ -1240,7 +1329,7 @@ static int fecmxc_probe(struct udevice *dev) bus = fec_get_miibus((ulong)priv->eth, dev_seq(dev)); #endif }
if (!bus) {
if (!bus && !priv->dm_mdio) { ret = -ENOMEM; goto err_mii; }@@ -1271,14 +1360,16 @@ static int fecmxc_probe(struct udevice *dev) break; }
ret = fec_phy_init(priv, dev);if (ret)goto err_phy;
if (!priv->dm_mdio) {ret = fec_phy_init(priv, dev);if (ret)goto err_phy;} return 0;err_phy:
if (!dm_mii_bus) {
if (!dm_mii_bus && !priv->dm_mdio) { mdio_unregister(bus); free(bus); }@@ -1294,8 +1385,10 @@ static int fecmxc_remove(struct udevice *dev)
free(priv->phydev); fec_free_descs(priv);
mdio_unregister(priv->bus);mdio_free(priv->bus);
if (priv->bus) {mdio_unregister(priv->bus);mdio_free(priv->bus);}#ifdef CONFIG_DM_REGULATOR if (priv->phy_supply) diff --git a/drivers/net/fec_mxc.h b/drivers/net/fec_mxc.h index 48faa33d66ec..c297880ccc54 100644 --- a/drivers/net/fec_mxc.h +++ b/drivers/net/fec_mxc.h @@ -248,6 +248,7 @@ struct fec_priv { uint8_t *tdb_ptr; int dev_id; struct mii_dev *bus;
bool dm_mdio;#ifdef CONFIG_PHYLIB struct phy_device *phydev; ofnode phy_of_node; -- 2.17.1
What I'm trying to accomplish here vs just simply using dm_fec_bind_mdio(dev) and dm_eth_phy_connect(dev) is to provide a fallback for the many users of fec_mxc that do not have a dt that works with dm-mdio yet still have defconfig's that enable DM_MDIO. Most of the users would not currently have an mdio subnode in the fec node, nor have the required phy-mode 'and' phy-handle prop or fixed-link subnode which would cause the dm_eth_phy_connect to fail.
Best Regards,
Tim