From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Glass Date: Fri, 14 Apr 2017 04:36:26 -0600 Subject: [U-Boot] [PATCH v2 03/10] drivers: phy: add generic PHY framework In-Reply-To: <3a7735db-fa6f-f130-4e8f-a35412fc7086@ti.com> References: <1491565329-20267-1-git-send-email-jjhiblot@ti.com> <1491565329-20267-4-git-send-email-jjhiblot@ti.com> <3a7735db-fa6f-f130-4e8f-a35412fc7086@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 Jean-Jacques, On 13 April 2017 at 08:17, Jean-Jacques Hiblot wrote: > > > On 09/04/2017 21:27, Simon Glass wrote: >> >> 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? > > Sure. I'll add something. It'll be pretty basic though Basic is fine. It needs to create a device or two and call some methods. >> >> >>> 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? > > OK > >> >>> 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? > > GENERIC_PHY is the name of the config option in the kernel and the functions > are also prefixed with generic_phy_. > BTW the functions in linux are not prefixed with generic_phy_ but only phy_, > but in the case of u-boot a lot of phy_xxx() functions already exist and > are not necessarily static (like phy_reset() for ther ethernet phy). OK. [..] >>> + */ >>> +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? > > It's quite easy for the PHY driver to get its internal data structure from > the struct udevice*. > I could also have passed struct generic_phy * but it adds another > translation that I don't think is necessary. I'd like to change that for consistency. A uclass API is supposed to take a struct udevice * rather than anything else. It is confusing if one uclass does this differently. The translation is cheap and some users will have a struct udevice * readily available. > > >> >>> +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 >> > Regards, Simon