On 09/04/2017 21:27, Simon Glass wrote:
Hi,
On 7 April 2017 at 05:42, Jean-Jacques Hiblot <jjhib...@ti.com> 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 <jjhib...@ti.com>
---
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
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).
+ 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.
OK
+
+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 <jjhib...@ti.com>
+ *
+ * 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?
OK
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <generic-phy.h>
+
+#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 :-)
indeed
+
+ 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' ?
OK
+ .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 <jjhib...@ti.com>
+ *
+ * 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).
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.
+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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
