[U-Boot] [PATCH] net: introduce MDIO DM class for MDIO devices
Alex Marginean
alexm.osslist at gmail.com
Sat Jun 1 18:41:41 UTC 2019
Hi Bin,
On 6/1/2019 8:16 PM, Bin Meng wrote:
> Hi Alex,
>
> On Sat, Jun 1, 2019 at 12:27 AM Alex Marginean <alexm.osslist at gmail.com> wrote:
>>
>> Adds UCLASS_MDIO DM class supporting MDIO buses that are probed as
>> stand-alone devices. Useful in particular for systems that support
>> DM_ETH and have a stand-alone MDIO hardware block shared by multiple
>> Ethernet interfaces.
>>
>
> Please add a test case for testing mdio (see test/dm/*)
OK, will look into it.
>
>> Signed-off-by: Alex Marginean <alexm.osslist at gmail.com>
>> ---
>> cmd/mdio.c | 5 ++
>> drivers/net/Kconfig | 13 +++++
>> include/dm/uclass-id.h | 1 +
>> include/miiphy.h | 50 ++++++++++++++++++++
>> net/Makefile | 1 +
>> net/mdio-uclass.c | 105 +++++++++++++++++++++++++++++++++++++++++
>> 6 files changed, 175 insertions(+)
>> create mode 100644 net/mdio-uclass.c
>>
>> diff --git a/cmd/mdio.c b/cmd/mdio.c
>> index 5e219f699d..a6fa9266d0 100644
>> --- a/cmd/mdio.c
>> +++ b/cmd/mdio.c
>> @@ -203,6 +203,11 @@ static int do_mdio(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>> if (argc < 2)
>> return CMD_RET_USAGE;
>>
>> +#ifdef CONFIG_DM_MDIO
>> + /* probe DM MII device before any operation so they are all accesible */
>> + dm_mdio_probe_devices();
>> +#endif
>> +
>> /*
>> * We use the last specified parameters, unless new ones are
>> * entered.
>> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
>> index e6a4fdf30e..6fba5a84dd 100644
>> --- a/drivers/net/Kconfig
>> +++ b/drivers/net/Kconfig
>> @@ -11,6 +11,19 @@ config DM_ETH
>> This is currently implemented in net/eth-uclass.c
>> Look in include/net.h for details.
>>
>> +config DM_MDIO
>> + bool "Enable Driver Model for MDIO devices"
>> + depends on DM_ETH && PHYLIB
>> + help
>> + Enable driver model for MDIO devices
>> +
>> + Adds UCLASS_MDIO DM class supporting MDIO buses that are probed as
>> + stand-alone devices. Useful in particular for systems that support
>> + DM_ETH and have a stand-alone MDIO hardware block shared by multiple
>> + Ethernet interfaces.
>> + This is currently implemented in net/mdio-uclass.c
>> + Look in include/miiphy.h for details.
>> +
>> menuconfig NETDEVICES
>> bool "Network device support"
>> depends on NET
>> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
>> index 09e0ad5391..90667e62cf 100644
>> --- a/include/dm/uclass-id.h
>> +++ b/include/dm/uclass-id.h
>> @@ -58,6 +58,7 @@ enum uclass_id {
>> UCLASS_LPC, /* x86 'low pin count' interface */
>> UCLASS_MAILBOX, /* Mailbox controller */
>> UCLASS_MASS_STORAGE, /* Mass storage device */
>> + UCLASS_MDIO, /* MDIO bus */
>> UCLASS_MISC, /* Miscellaneous device */
>> UCLASS_MMC, /* SD / MMC card or chip */
>> UCLASS_MOD_EXP, /* RSA Mod Exp device */
>> diff --git a/include/miiphy.h b/include/miiphy.h
>> index f11763affd..455feacd33 100644
>> --- a/include/miiphy.h
>> +++ b/include/miiphy.h
>> @@ -118,4 +118,54 @@ int bb_miiphy_write(struct mii_dev *miidev, int addr, int devad, int reg,
>> #define ESTATUS_1000XF 0x8000
>> #define ESTATUS_1000XH 0x4000
>>
>> +#ifdef CONFIG_DM_MDIO
>> +
>> +/**
>> + * struct mii_pdata - Platform data for MDIO buses
>
> wrong name: mii_pdata
Oops, I had this renamed and refactored and I seem to missed the
comments entirely. And a few other things as you pointed out below.
>
>> + *
>> + * @mii_bus: Supporting MII legacy bus
>> + * @priv_pdata: device specific platdata
>
> there is no such field called priv_pdata.
>
>> + */
>> +struct mdio_perdev_priv {
>> + struct mii_dev *mii_bus;
>> +};
>> +
>> +/**
>> + * struct mii_ops - MDIO bus operations
>
> struct mdio_ops
Thanks for pointing these out, I'll send a v2 with the fixes.
>
>> + *
>> + * @read: Read from a PHY register
>> + * @write: Write to a PHY register
>> + * @reset: Reset the MDIO bus, NULL if not supported
>> + */
>> +struct mdio_ops {
>> + int (*read)(struct udevice *mdio_dev, int addr, int devad, int reg);
>> + int (*write)(struct udevice *mdio_dev, int addr, int devad, int reg,
>> + u16 val);
>> + int (*reset)(struct udevice *mdio_dev);
>> +};
>> +
>> +#define mdio_get_ops(dev) ((struct mdio_ops *)(dev)->driver->ops)
>> +
>> +/**
>> + * mii_dm_probe_devices - Call probe on all MII devices, currently used for
>
> wrong name "mii_dm_probe_devices". MII devices -> MDIO devices?
>
>> + * MDIO console commands.
>> + */
>> +void dm_mdio_probe_devices(void);
>> +
>> +/**
>> + * dm_mdio_phy_connect - Wrapper over phy_connect for DM MDIO
>> + *
>> + * @dev: mdio dev
>> + * @addr: PHY address on MDIO bus
>> + * @ethdev: ethernet device to connect to the PHY
>> + * @interface: MAC-PHY protocol
>> + *
>> + * @return pointer to phy_device, or 0 on error
>> + */
>> +struct phy_device *dm_mdio_phy_connect(struct udevice *dev, int addr,
>> + struct udevice *ethdev,
>> + phy_interface_t interface);
>> +
>> +#endif
>> +
>> #endif
>> diff --git a/net/Makefile b/net/Makefile
>> index ce36362168..6251ff3991 100644
>> --- a/net/Makefile
>> +++ b/net/Makefile
>> @@ -15,6 +15,7 @@ obj-$(CONFIG_NET) += eth-uclass.o
>> else
>> obj-$(CONFIG_NET) += eth_legacy.o
>> endif
>> +obj-$(CONFIG_DM_MDIO) += mdio-uclass.o
>> obj-$(CONFIG_NET) += eth_common.o
>> obj-$(CONFIG_CMD_LINK_LOCAL) += link_local.o
>> obj-$(CONFIG_NET) += net.o
>> diff --git a/net/mdio-uclass.c b/net/mdio-uclass.c
>> new file mode 100644
>> index 0000000000..6ae7b4ecfd
>> --- /dev/null
>> +++ b/net/mdio-uclass.c
>> @@ -0,0 +1,105 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * (C) Copyright 2019
>> + * Alex Marginean, NXP
>> + */
>> +
>> +#include <common.h>
>> +#include <dm.h>
>> +#include <miiphy.h>
>> +#include <dm/device-internal.h>
>> +#include <dm/uclass-internal.h>
>> +
>> +void dm_mdio_probe_devices(void)
>> +{
>> + struct udevice *it;
>> + struct uclass *uc;
>> +
>> + uclass_get(UCLASS_MDIO, &uc);
>> + uclass_foreach_dev(it, uc) {
>> + device_probe(it);
>> + }
>> +}
>> +
>> +static int dm_mdio_post_bind(struct udevice *dev)
>> +{
>> + if (strchr(dev->name, ' ')) {
>
> Is such check a must have? If yes, could you please add some comments?
These names may be used in mdio command and I'm guessing the command
gets confused if there are spaces, I'll put a comment here.
>
>> + debug("\nError: MDIO device name \"%s\" has a space!\n",
>> + dev->name);
>> + return -EINVAL;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int dm_mdio_read(struct mii_dev *mii_bus, int addr, int devad, int reg)
>
> This does not look correct. Normally the dm_ functions should have
> udevice * as its first argument.
These are not exported through dm api, instead they are registered to
legacy mii API. I should probably rename the function and drop dm_,
I'll add comments to these functions to so people don't get confused.
>
>> +{
>> + struct udevice *dev = mii_bus->priv;
>> +
>> + return mdio_get_ops(dev)->read(dev, addr, devad, reg);
>> +}
>> +
>> +static int dm_mdio_write(struct mii_dev *mii_bus, int addr, int devad, int reg,
>
> ditto
>
>> + u16 val)
>> +{
>> + struct udevice *dev = mii_bus->priv;
>> +
>> + return mdio_get_ops(dev)->write(dev, addr, devad, reg, val);
>> +}
>> +
>> +static int dm_mdio_reset(struct mii_dev *mii_bus)
>
> ditto
>
>> +{
>> + struct udevice *dev = mii_bus->priv;
>> +
>> + if (mdio_get_ops(dev)->reset)
>> + return mdio_get_ops(dev)->reset(dev);
>> + else
>> + return 0;
>> +}
>> +
>> +static int dm_mdio_post_probe(struct udevice *dev)
>> +{
>> + struct mdio_perdev_priv *pdata = dev_get_uclass_priv(dev);
>> +
>> + pdata->mii_bus = mdio_alloc();
>> + pdata->mii_bus->read = dm_mdio_read;
>> + pdata->mii_bus->write = dm_mdio_write;
>> + pdata->mii_bus->reset = dm_mdio_reset;
>> + pdata->mii_bus->priv = dev;
>> + strncpy(pdata->mii_bus->name, dev->name, MDIO_NAME_LEN);
>> +
>> + return mdio_register(pdata->mii_bus);
>> +}
>> +
>> +static int dm_mdio_pre_remove(struct udevice *dev)
>> +{
>> + struct mdio_perdev_priv *pdata = dev_get_uclass_priv(dev);
>> + struct mdio_ops *ops = mdio_get_ops(dev);
>> +
>> + if (ops->reset)
>> + ops->reset(dev);
>> + mdio_free(pdata->mii_bus);
>> +
>> + return 0;
>> +}
>> +
>> +struct phy_device *dm_mdio_phy_connect(struct udevice *dev, int addr,
>> + struct udevice *ethdev,
>> + phy_interface_t interface)
>> +{
>> + struct mdio_perdev_priv *pdata = dev_get_uclass_priv(dev);
>> +
>> + if (device_probe(dev))
>> + return 0;
>> +
>> + return phy_connect(pdata->mii_bus, addr, ethdev, interface);
>> +}
>> +
>> +UCLASS_DRIVER(mii) = {
>
> UCLASS_DRIVER(mdio)?
>
>> + .id = UCLASS_MDIO,
>> + .name = "mii",
>
> "mdio"?
Yes, you're right, it should be "mdio".
>
>> + .post_bind = dm_mdio_post_bind,
>> + .post_probe = dm_mdio_post_probe,
>> + .pre_remove = dm_mdio_pre_remove,
>> + .per_device_auto_alloc_size = sizeof(struct mdio_perdev_priv),
>> +};
>> --
>
> Regards,
> Bin
>
Thank you, Bin!
Alex
More information about the U-Boot
mailing list