From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Glass Date: Sun, 9 Apr 2017 13:27:33 -0600 Subject: [U-Boot] [PATCH v2 03/10] drivers: phy: add generic PHY framework In-Reply-To: <1491565329-20267-4-git-send-email-jjhiblot@ti.com> References: <1491565329-20267-1-git-send-email-jjhiblot@ti.com> <1491565329-20267-4-git-send-email-jjhiblot@ti.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi, On 7 April 2017 at 05:42, Jean-Jacques Hiblot wrote: > The PHY framework provides a set of APIs to control a PHY. This API is > derived from the linux version of the generic PHY framework. > Currently the API supports init(), deinit(), power_on, power_off() and > reset(). The framework provides a way to get a reference to a phy from the > device-tree. > > Signed-off-by: Jean-Jacques Hiblot > --- > Makefile | 1 + > drivers/Kconfig | 2 ++ > drivers/Makefile | 1 + > drivers/phy/Kconfig | 22 ++++++++++++++ > drivers/phy/Makefile | 5 ++++ > drivers/phy/phy-uclass.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++ > include/dm/uclass-id.h | 1 + > include/generic-phy.h | 38 ++++++++++++++++++++++++ > 8 files changed, 147 insertions(+) > create mode 100644 drivers/phy/Kconfig > create mode 100644 drivers/phy/Makefile > create mode 100644 drivers/phy/phy-uclass.c > create mode 100644 include/generic-phy.h Can you please create a sandbox driver and a test? > > diff --git a/Makefile b/Makefile > index 2638acf..06454ce 100644 > --- a/Makefile > +++ b/Makefile > @@ -650,6 +650,7 @@ libs-y += fs/ > libs-y += net/ > libs-y += disk/ > libs-y += drivers/ > +libs-y += drivers/phy/ Could this go in drivers/Makefile? > libs-y += drivers/dma/ > libs-y += drivers/gpio/ > libs-y += drivers/i2c/ > diff --git a/drivers/Kconfig b/drivers/Kconfig > index 0e5d97d..a90ceca 100644 > --- a/drivers/Kconfig > +++ b/drivers/Kconfig > @@ -88,6 +88,8 @@ source "drivers/video/Kconfig" > > source "drivers/watchdog/Kconfig" > > +source "drivers/phy/Kconfig" > + > config PHYS_TO_BUS > bool "Custom physical to bus address mapping" > help > diff --git a/drivers/Makefile b/drivers/Makefile > index 5d8baa5..4656509 100644 > --- a/drivers/Makefile > +++ b/drivers/Makefile > @@ -47,6 +47,7 @@ obj-$(CONFIG_OMAP_USB_PHY) += usb/phy/ > obj-$(CONFIG_SPL_SATA_SUPPORT) += block/ > obj-$(CONFIG_SPL_USB_HOST_SUPPORT) += block/ > obj-$(CONFIG_SPL_MMC_SUPPORT) += block/ > +obj-$(CONFIG_SPL_GENERIC_PHY) += phy/ > endif > > ifdef CONFIG_TPL_BUILD > diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig > new file mode 100644 > index 0000000..b6fed9e > --- /dev/null > +++ b/drivers/phy/Kconfig > @@ -0,0 +1,22 @@ > + > +menu "PHY Subsystem" > + > +config GENERIC_PHY Just a question: do you think we need the word GENERIC in this config? I'm OK with it, but wonder if CONFIG_PHY would be enough? > + bool "PHY Core" > + depends on DM > + help > + Generic PHY support. > + > + This framework is designed to provide a generic interface for PHY > + devices. Could you given a few examples of PHY devices and the types of operations you can perform on them. > + > +config SPL_GENERIC_PHY > + bool "PHY Core in SPL" > + depends on DM > + help > + Generic PHY support in SPL. > + > + This framework is designed to provide a generic interface for PHY > + devices. > + > +endmenu > diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile > new file mode 100644 > index 0000000..ccd15ed > --- /dev/null > +++ b/drivers/phy/Makefile > @@ -0,0 +1,5 @@ > +obj-$(CONFIG_$(SPL_)GENERIC_PHY) += phy-uclass.o > + > +ifeq ($(CONFIG_SPL_BUILD)$(CONFIG_TPL_BUILD),) > +obj-y += marvell/ > +endif > diff --git a/drivers/phy/phy-uclass.c b/drivers/phy/phy-uclass.c > new file mode 100644 > index 0000000..4d1584d > --- /dev/null > +++ b/drivers/phy/phy-uclass.c > @@ -0,0 +1,77 @@ > +/* > + * Copyright (C) 2017 Texas Instruments Incorporated - http://www.ti.com/ > + * Written by Jean-Jacques Hiblot > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. SPDX? > + */ > + > +#include > +#include > +#include > + > +#define get_ops(dev) ((struct generic_phy_ops *)(dev)->driver->ops) > + > +#define generic_phy_to_dev(x) ((struct udevice *)(x)) > +#define dev_to_generic_phy(x) ((struct generic_phy *)(x)) > + > +struct generic_phy *dm_generic_phy_get(struct udevice *dev, const char *string) > +{ > + struct udevice *generic_phy_dev; dev is a shorter name :-) > + > + int rc = uclass_get_device_by_phandle(UCLASS_PHY, dev, > + string, &generic_phy_dev); > + if (rc) { > + error("unable to find generic_phy device %d\n", rc); > + return ERR_PTR(rc); > + } > + return dev_to_generic_phy(generic_phy_dev); > +} > + > +int generic_phy_init(struct generic_phy *generic_phy) > +{ > + struct udevice *dev = generic_phy_to_dev(generic_phy); > + struct generic_phy_ops *ops = get_ops(dev); > + > + return (ops && ops->init) ? ops->init(dev) : 0; > +} > + > +int generic_phy_reset(struct generic_phy *generic_phy) > +{ > + struct udevice *dev = generic_phy_to_dev(generic_phy); > + struct generic_phy_ops *ops = get_ops(dev); > + > + return (ops && ops->reset) ? ops->reset(dev) : 0; > +} > + > +int generic_phy_exit(struct generic_phy *generic_phy) > +{ > + struct udevice *dev = generic_phy_to_dev(generic_phy); > + struct generic_phy_ops *ops = get_ops(dev); > + > + return (ops && ops->exit) ? ops->exit(dev) : 0; > +} > + > +int generic_phy_power_on(struct generic_phy *generic_phy) > +{ > + struct udevice *dev = generic_phy_to_dev(generic_phy); > + struct generic_phy_ops *ops = get_ops(dev); > + > + return (ops && ops->power_on) ? ops->power_on(dev) : 0; > +} > + > +int generic_phy_power_off(struct generic_phy *generic_phy) > +{ > + struct udevice *dev = generic_phy_to_dev(generic_phy); > + struct generic_phy_ops *ops = get_ops(dev); > + > + return (ops && ops->power_off) ? ops->power_off(dev) : 0; > +} > + Drop 2 extra blank ilnes > + > + > +UCLASS_DRIVER(simple_generic_phy) = { remove the word 'simple' ? > + .id = UCLASS_PHY, > + .name = "generic_phy", > +}; > diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h > index 8c92d0b..9d34a32 100644 > --- a/include/dm/uclass-id.h > +++ b/include/dm/uclass-id.h > @@ -83,6 +83,7 @@ enum uclass_id { > UCLASS_VIDEO, /* Video or LCD device */ > UCLASS_VIDEO_BRIDGE, /* Video bridge, e.g. DisplayPort to LVDS */ > UCLASS_VIDEO_CONSOLE, /* Text console driver for video device */ > + UCLASS_PHY, /* generic PHY device */ > > UCLASS_COUNT, > UCLASS_INVALID = -1, > diff --git a/include/generic-phy.h b/include/generic-phy.h > new file mode 100644 > index 0000000..f02e9ce > --- /dev/null > +++ b/include/generic-phy.h > @@ -0,0 +1,38 @@ > +/* > + * Copyright (C) 2017 Texas Instruments Incorporated - http://www.ti.com/ > + * Written by Jean-Jacques Hiblot > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + */ > + > +#ifndef __GENERIC_PHY_H > +#define __GENERIC_PHY_H > + > +struct generic_phy; > +/* > + * struct generic_phy_ops - set of function pointers for phy operations > + * @init: operation to be performed for initializing phy > + * @exit: operation to be performed while exiting > + * @power_on: powering on the phy > + * @power_off: powering off the phy Need to mention that these are all optional (from what I can tell above). > + */ > +struct generic_phy_ops { > + int (*init)(struct udevice *phy); > + int (*exit)(struct udevice *phy); > + int (*reset)(struct udevice *phy); > + int (*power_on)(struct udevice *phy); > + int (*power_off)(struct udevice *phy); > +}; > + > + > +int generic_phy_init(struct generic_phy *phy); Why do these not use struct udevice? > +int generic_phy_reset(struct generic_phy *phy); > +int generic_phy_exit(struct generic_phy *phy); > +int generic_phy_power_on(struct generic_phy *phy); > +int generic_phy_power_off(struct generic_phy *phy); > + > +struct generic_phy *dm_generic_phy_get(struct udevice *dev, const char *string); > + > +#endif /*__GENERIC_PHY_H */ > -- > 1.9.1 > Regards, Simon