Hi Ken, On 6 June 2018 at 18:08, <m...@marvell.com> wrote: > From: Ken Ma <m...@marvell.com> > > Add a uclass which provides access to MDIO busses and includes > operations required by MDIO. > The implementation is based on the existing mii/phy/mdio data > structures and APIs. > This patch also adds evice tree binding for MDIO bus. > > Signed-off-by: Ken Ma <m...@marvell.com> > --- > > Changes in v2: None > > MAINTAINERS | 1 + > doc/device-tree-bindings/mdio/mdio-bus.txt | 54 +++++++++++++ > drivers/Kconfig | 2 + > drivers/Makefile | 1 + > drivers/mdio/Kconfig | 18 +++++ > drivers/mdio/Makefile | 6 ++ > drivers/mdio/mdio-uclass.c | 119 > +++++++++++++++++++++++++++++ > include/dm/uclass-id.h | 1 + > include/mdio.h | 62 +++++++++++++++ > 9 files changed, 264 insertions(+) > create mode 100644 doc/device-tree-bindings/mdio/mdio-bus.txt > create mode 100644 drivers/mdio/Kconfig > create mode 100644 drivers/mdio/Makefile > create mode 100644 drivers/mdio/mdio-uclass.c > create mode 100644 include/mdio.h
When adding a new uclass, please add a sandbox driver and a test in test/dm/mdio.c > +menu "MDIO Support" > + > +config DM_MDIO I don't see any CONFIG_MDIO option, so you can just make this MDIO. The DM_ prefix is for things that are being converted to driver model, but still have legacy code. > + bool "Enable Driver Model for MDIO drivers" > + depends on DM > + help > + Enable driver model for MDIO access. What does MDIO stand for? You should use the full name in help at least once. > + Drivers provide methods to management data > + Input/Output. > + MDIO uclass provides interfaces to get mdio > + udevice or mii bus from its child phy node or > + an ethernet udevice which the phy belongs to. > + > +endmenu > diff --git a/drivers/mdio/Makefile b/drivers/mdio/Makefile > new file mode 100644 > index 0000000..9b290c0 > --- /dev/null > +++ b/drivers/mdio/Makefile > @@ -0,0 +1,6 @@ > +# SPDX-License-Identifier: GPL-2.0+ > +# > +# Copyright (C) 2018 Marvell International Ltd. > +# Author: Ken Ma<m...@marvell.com> > + > +obj-$(CONFIG_DM_MDIO) += mdio-uclass.o > diff --git a/drivers/mdio/mdio-uclass.c b/drivers/mdio/mdio-uclass.c > new file mode 100644 > index 0000000..251776b > --- /dev/null > +++ b/drivers/mdio/mdio-uclass.c > @@ -0,0 +1,119 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright (C) 2018 Marvell International Ltd. > + * Author: Ken Ma<m...@marvell.com> > + */ > + > +#include <common.h> > +#include <fdtdec.h> > +#include <errno.h> > +#include <dm.h> > +#include <dm/uclass.h> > +#include <dm/uclass-internal.h> > +#include <miiphy.h> > +#include <mdio.h> > + > +DECLARE_GLOBAL_DATA_PTR; > + > +int mdio_mii_bus_get(struct udevice *mdio_dev, struct mii_dev **bus) > +{ > + *bus = *(struct mii_dev **)dev_get_uclass_platdata(mdio_dev); > + > + return 0; > +} > + > +int mdio_device_get_from_phy(int phy_node, struct udevice **devp) Please can you use livetree functions, like dev_read_...() and ofnode_..() instead the flat-tree functions? > +{ > + int mdio_off; > + > + mdio_off = fdt_parent_offset(gd->fdt_blob, phy_node); > + return uclass_get_device_by_of_offset(UCLASS_MDIO, mdio_off, > + devp); > +} > + [..] > +static int mdio_uclass_pre_probe(struct udevice *dev) > +{ > + struct mii_dev **pbus = dev_get_uclass_platdata(dev); > + struct mii_dev *bus; > + const char *name; > + > + bus = mdio_alloc(); > + if (!bus) { > + printf("Failed to allocate MDIO bus @%p\n", > + devfdt_get_addr_ptr(dev)); debug() > + return -1; Should return a real error, like -ENOMEM But can you not put struct mii_dev in the private data, to avoid the alloc? If not, you need a remove() method to free this data. > + } > + > + name = fdt_getprop(gd->fdt_blob, dev_of_offset(dev), > + "mdio-name", NULL); > + if (name) > + strncpy(bus->name, name, MDIO_NAME_LEN); > + *pbus = bus; > + > + return 0; > +} > + > +static int mdio_uclass_post_probe(struct udevice *dev) > +{ > + struct mii_dev **pbus = dev_get_uclass_platdata(dev); > + > + return mdio_register(*pbus); > +} > + > +UCLASS_DRIVER(mdio) = { > + .id = UCLASS_MDIO, > + .name = "mdio", > + .pre_probe = mdio_uclass_pre_probe, > + .post_probe = mdio_uclass_post_probe, > + .per_device_platdata_auto_alloc_size = sizeof(struct mii_dev *), > +}; > diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h > index d7f9df3..170a0cc 100644 > --- a/include/dm/uclass-id.h > +++ b/include/dm/uclass-id.h > @@ -49,6 +49,7 @@ enum uclass_id { > UCLASS_LPC, /* x86 'low pin count' interface */ > UCLASS_MAILBOX, /* Mailbox controller */ > UCLASS_MASS_STORAGE, /* Mass storage device */ > + UCLASS_MDIO, /* MDIO device */ Spell out MDIO if room > UCLASS_MISC, /* Miscellaneous device */ > UCLASS_MMC, /* SD / MMC card or chip */ > UCLASS_MOD_EXP, /* RSA Mod Exp device */ > diff --git a/include/mdio.h b/include/mdio.h > new file mode 100644 > index 0000000..50458b1 > --- /dev/null > +++ b/include/mdio.h > @@ -0,0 +1,62 @@ > +/* SPDX-License-Identifier: GPL-2.0+ */ > +/* > + * Copyright (C) 2018 Marvell International Ltd. > + * Author: Ken Ma<m...@marvell.com> > + */ > + > +#ifndef _MDIO_H_ > +#define _MDIO_H_ > + > +#include <dm.h> /* Because we dereference struct udevice here */ > +#include <phy.h> > + > +/** > + * mdio_mii_bus_get() - Get mii bus from mdio udevice > + * > + * @mdio_dev: mdio udevice > + * @bus: mii bus returns mii bus Also please can you add a 'p' suffix on pointers that are used to return values. struct mii_dev **busp That is consistent with how things are done in the driver-model code. > + * @returns 0 on success, error code otherwise. > + */ > +int mdio_mii_bus_get(struct udevice *mdio_dev, struct mii_dev **bus); > + > +/** > + * mdio_device_get_from_phy() - Get the mdio udevice which the phy belongs to > + * > + * @phy_node: phy node offset Should use ofnode I think, same below > + * @devp: mdio udevice > + * @returns 0 on success, error code otherwise. > + */ > +int mdio_device_get_from_phy(int phy_node, struct udevice **devp); > + > +/** > + * mdio_mii_bus_get_from_phy() - Get the mii bus which the phy belongs to > + * > + * @phy_node: phy node offset > + * @bus: mii bus > + * @returns 0 on success, error code otherwise. > + */ > +int mdio_mii_bus_get_from_phy(int phy_node, struct mii_dev **bus); > + > +/** > + * mdio_device_get_from_eth() - When there is a phy reference of "phy = > <&...>" > + * under an ethernet udevice fdt node, this function can > + * get the mdio udevice which the phy belongs to > + * > + * @dev: the ethernet udevice which contains the phy reference > + * @devp: mdio udevice Yes devp is a good name. > + * @returns 0 on success, error code otherwise. > + */ > +int mdio_device_get_from_eth(struct udevice *eth, struct udevice **devp); > + > +/** > + * mdio_mii_bus_get_from_eth() - When there is a phy reference of > + * "phy = <&...>" under an ethernet udevice fdt node, > this > + * function can get the mii bus which the phy belongs to > + * > + * @eth: the ethernet udevice which contains the phy reference > + * @bus: mii bus > + * @returns 0 on success, error code otherwise. > + */ > +int mdio_mii_bus_get_from_eth(struct udevice *eth, struct mii_dev **bus); > + > +#endif /* _MDIO_H_ */ > -- > 1.9.1 > Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot