[U-Boot] [PATCH] net: introduce MDIO DM class for MDIO devices
Alex Marginean
alexm.osslist at gmail.com
Sat Jun 1 18:44:11 UTC 2019
On 6/1/2019 9:42 PM, Joe Hershberger wrote:
> Hi Bin,
>
> Thanks for the review. I shouldn't have tried to look at this while
> falling asleep in the airport. :(
>
> -Joe
I guess I should have double checked the comments and the other things
too, sorry for that..
I'll send a v2 on Monday.
Thank you!
Alex
>
> On Sat, Jun 1, 2019 at 12:16 PM Bin Meng <bmeng.cn at gmail.com> 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/*)
>>
>>> 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
>>
>>> + *
>>> + * @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
>>
>>> + *
>>> + * @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?
>>
>>> + 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.
>>
>>> +{
>>> + 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"?
>>
>>> + .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
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> https://lists.denx.de/listinfo/u-boot
More information about the U-Boot
mailing list