Re: [U-Boot] [PATCH v4 2/7] drivers: phy: add generic PHY framework

Hi Jean-Jacques,

On 21 April 2017 at 04:08, Jean-Jacques Hiblot jjhiblot@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 jjhiblot@ti.com

changes since v3:

• make it possible for a phy device to handle multiple ports. The consequence is the intruction of struct phy which is is used as a parameter to all the functions of the API.
• Add generic_phy_get_by_index() and generic_phy_get_by_name() to get a reference to a phy port. Removed dm_generic_phy_get().
• Improved the documentation

drivers/Kconfig | 2 + drivers/Makefile | 2 + drivers/phy/Kconfig | 36 ++++++++ drivers/phy/Makefile | 2 + drivers/phy/phy-uclass.c | 139 +++++++++++++++++++++++++++++ include/dm/uclass-id.h | 1 + include/generic-phy.h | 224 +++++++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 406 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

Reviewed-by: Simon Glass sjg@chromium.org

With some nits below.

This is a really nice piece of work and a great addition to U-Boot.

diff --git a/drivers/Kconfig b/drivers/Kconfig index 3e6bbac..1fe5baf 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -90,6 +90,8 @@ source "drivers/video/Kconfig"

source "drivers/watchdog/Kconfig"

+source "drivers/phy/Kconfig"

Can you please move this up so we retail the alphabetical order?

config PHYS_TO_BUS bool "Custom physical to bus address mapping" help diff --git a/drivers/Makefile b/drivers/Makefile index 5d8baa5..c2700a0 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -5,9 +5,11 @@ obj-$(CONFIG_$(SPL_TPL_)DM) += core/ obj-$(CONFIG_$(SPL_)CLK) += clk/ obj-$(CONFIG_$(SPL_)LED) += led/ +obj-$(CONFIG_$(SPL_)PHY) += phy/ obj-$(CONFIG_$(SPL_)PINCTRL) += pinctrl/ obj-$(CONFIG_$(SPL_)RAM) += ram/

Drop this extra blank line

ifdef CONFIG_SPL_BUILD

obj-$(CONFIG_SPL_CPU_SUPPORT) += cpu/ diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig new file mode 100644 index 0000000..0a74920 --- /dev/null +++ b/drivers/phy/Kconfig @@ -0,0 +1,36 @@

+menu "PHY Subsystem"

+config PHY

•   bool "PHY Core"
  

•   depends on DM
  

•   help
  

•     PHY support.
  

•     This framework is designed to provide a generic interface for PHY
  

•     devices. PHY devices are dedicated hardware that handle the physical
  

•     layer of the protocols in the OSI model.
  

•     PHYs are commonly used for high speed interfaces such as Serial-ATA
  

•     or PCI express.
  

•     The API provides functions to initialize/deinitialize the
  

•     PHY, power on/off the PHY, and reset the PHY. It's meant to be as
  

•     compatible as possible with the equivalent framework found in the
  

•     linux kernel.
  

+config SPL_PHY

•   bool "PHY Core in SPL"
  

•   depends on DM
  

•   help
  

•     PHY support in SPL.
  

•     This framework is designed to provide a generic interface for PHY
  

•     devices. PHY devices are dedicated hardware that handle the physical
  

•     layer of the protocols (https://en.wikipedia.org/wiki/OSI_model).
  

•     PHYs are commonly used for high speed interfaces such as Serial-ATA
  

•     or PCI express.
  

•     The API provides functions to initialize/deinitialize the
  

•     PHY, power on/off the PHY, and reset the PHY. It's meant to be as
  

•     compatible as possible with the equivalent framework found in the
  

•     linux kernel.
  

+endmenu diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile new file mode 100644 index 0000000..186f07d --- /dev/null +++ b/drivers/phy/Makefile @@ -0,0 +1,2 @@ +obj-$(CONFIG_$(SPL_)PHY) += phy-uclass.o

Extra blank line here. Also can you please add your copyright and SPDX license to the Makefile?

diff --git a/drivers/phy/phy-uclass.c b/drivers/phy/phy-uclass.c new file mode 100644 index 0000000..0d8bef7 --- /dev/null +++ b/drivers/phy/phy-uclass.c @@ -0,0 +1,139 @@ +/*

• • Copyright (C) 2017 Texas Instruments Incorporated - http://www.ti.com/
• • Written by Jean-Jacques Hiblot jjhiblot@ti.com
• • SPDX-License-Identifier: GPL-2.0+
• */

+#include <common.h> +#include <dm.h> +#include <generic-phy.h>

+DECLARE_GLOBAL_DATA_PTR;

+static inline struct phy_ops *phy_dev_ops(struct udevice *dev) +{

•   return (struct phy_ops *)dev->driver->ops;
  

+}

+static int generic_phy_xlate_offs_flags(struct phy *phy,

•                            struct fdtdec_phandle_args *args)
  

+{

•   debug("%s(phy=%p)\n", __func__, phy);
  

•   if (args->args_count > 1) {
  

•           debug("Invaild args_count: %d\n", args->args_count);
  

•           return -EINVAL;
  

•   }
  

•   if (args->args_count)
  

•           phy->id = args->args[0];
  

•   else
  

•           phy->id = 0;
  

•   return 0;
  

+}

+int generic_phy_get_by_index(struct udevice *dev, int index,

•                        struct phy *phy)
  

+{

•   struct fdtdec_phandle_args args;
  

•   struct phy_ops *ops;
  

•   int ret;
  

•   struct udevice *phydev;
  

•   debug("%s(dev=%p, index=%d, phy=%p)\n", __func__, dev, index, phy);
  

•   assert(phy);
  

•   ret = fdtdec_parse_phandle_with_args(gd->fdt_blob, dev_of_offset(dev),
  

•                                        "phys", "#phy-cells", 0, index,
  

•                                        &args);
  

•   if (ret) {
  

•           debug("%s: fdtdec_parse_phandle_with_args failed: err=%d\n",
  

•                 __func__, ret);
  

•           return ret;
  

•   }
  

•   ret = uclass_get_device_by_of_offset(UCLASS_PHY, args.node, &phydev);
  

•   if (ret) {
  

•           debug("%s: uclass_get_device_by_of_offset failed: err=%d\n",
  

•                 __func__, ret);
  

•           return ret;
  

•   }
  

•   phy->dev = phydev;
  

•   ops = phy_dev_ops(phydev);
  

•   if (ops->of_xlate)
  

•           ret = ops->of_xlate(phy, &args);
  

•   else
  

•           ret = generic_phy_xlate_offs_flags(phy, &args);
  

•   if (ret) {
  

•           debug("of_xlate() failed: %d\n", ret);
  

•           goto err;
  

•   }
  

•   return 0;
  

+err:

•   return ret;
  

+}

+int generic_phy_get_by_name(struct udevice *dev, const char *phy_name,

•                       struct phy *phy)
  

+{

•   int index;
  

•   debug("%s(dev=%p, name=%s, phy=%p)\n", __func__, dev, phy_name, phy);
  

•   index = fdt_stringlist_search(gd->fdt_blob, dev_of_offset(dev),
  

•                                 "phy-names", phy_name);
  

•   if (index < 0) {
  

•           debug("fdt_stringlist_search() failed: %d\n", index);
  

•           return index;
  

•   }
  

•   return generic_phy_get_by_index(dev, index, phy);
  

+}

+int generic_phy_init(struct phy *phy) +{

•   struct phy_ops const *ops = phy_dev_ops(phy->dev);
  

•   return ops->init ? ops->init(phy) : 0;
  

+}

+int generic_phy_reset(struct phy *phy) +{

•   struct phy_ops const *ops = phy_dev_ops(phy->dev);
  

•   return ops->reset ? ops->reset(phy) : 0;
  

+}

+int generic_phy_exit(struct phy *phy) +{

•   struct phy_ops const *ops = phy_dev_ops(phy->dev);
  

•   return ops->exit ? ops->exit(phy) : 0;
  

+}

+int generic_phy_power_on(struct phy *phy) +{

•   struct phy_ops const *ops = phy_dev_ops(phy->dev);
  

•   return ops->power_on ? ops->power_on(phy) : 0;
  

+}

+int generic_phy_power_off(struct phy *phy) +{

•   struct phy_ops const *ops = phy_dev_ops(phy->dev);
  

•   return ops->power_off ? ops->power_off(phy) : 0;
  

+}

+UCLASS_DRIVER(phy) = {

•   .id             = UCLASS_PHY,
  

•   .name           = "phy",
  

+}; diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index 8c92d0b..996e94f 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,             /* Physical Layer (PHY) device */
  

Please put this before PINCONFIG

    UCLASS_COUNT,
    UCLASS_INVALID = -1,

diff --git a/include/generic-phy.h b/include/generic-phy.h new file mode 100644 index 0000000..d8cf0c9 --- /dev/null +++ b/include/generic-phy.h @@ -0,0 +1,224 @@ +/*

• • Copyright (C) 2017 Texas Instruments Incorporated - http://www.ti.com/
• • Written by Jean-Jacques Hiblot jjhiblot@ti.com
• • SPDX-License-Identifier: GPL-2.0+
• */

+#ifndef __GENERIC_PHY_H +#define __GENERIC_PHY_H

+/**

• • struct phy - A handle to (allowing control of) a single phy port.
• • Clients provide storage for phy handles. The content of the structure is
• • managed solely by the PHY API and PHY drivers. A phy struct is
• • initialized by "get"ing the phy struct. The phy struct is passed to all
• • other phy APIs to identify which PHY port to operate upon.
• • @dev: The device which implements the PHY port.
• • @id: The PHY ID within the provider.
• */

+struct phy {

•   struct udevice *dev;
  

•   unsigned long id;
  

+};

+/*

• • struct udevice_ops - set of function pointers for phy operations
• • @init: operation to be performed for initializing phy (optional)
• • @exit: operation to be performed while exiting (optional)
• • @reset: reset the phy (optional).
• • @power_on: powering on the phy (optional)
• • @power_off: powering off the phy (optional)
• */

+struct phy_ops {

•   /**
  

•    * of_xlate - Translate a client's device-tree (OF) phy specifier.
  

•    *
  

•    * The PHY core calls this function as the first step in implementing
  

•    * a client's generic_phy_get_by_*() call.
  

•    *
  

•    * If this function pointer is set to NULL, the PHY core will use a
  

•    * default implementation, which assumes #phy-cells = <0> or
  

•    * #phy-cells = <1>, and in the later case that the DT cell
  

•    * contains a simple integer PHY port ID.
  

•    *
  

•    * @phy:        The phy struct to hold the translation result.
  

•    * @args:       The phy specifier values from device tree.
  

•    * @return 0 if OK, or a negative error code.
  

•    */
  

•   int     (*of_xlate)(struct phy *phy,
  

•                   struct fdtdec_phandle_args *args);
  

•   /**
  

•    * init - initialize the hardware.
  

•    *
  

•    * Hardware intialization should not be done in during probe() but
  

•    * should be implemented in this init() function. It could be starting
  

•    * PLL, taking a controller out of reset, routing, etc. This function
  

•    * is typically called only once per PHY port.
  

•    * If power_on() is not implemented, it must power up the phy.
  

•    *
  

•    * @phy:        the PHY port to initialize
  

•    * @return 0 if OK, or a negative error code.
  

•    */
  

•   int     (*init)(struct phy *phy);
  

•   /**
  

•   * exit - de-initialize the PHY device
  

•   *
  

•   * Hardware de-intialization should be done here. Every step done in
  

•   * init() should be undone here.
  

•   * This could be used to suspend the phy to reduce power consumption or
  

•   * to put the phy in a known condition before booting the OS (though it
  

•   * is NOT called automatically before booting the OS)
  

•   * If power_off() is not implemented, it must power down the phy.
  

•   *
  

•   * @phy: PHY port to be de-initialized
  

•   * @return 0 if OK, or a negative error code
  

•   */
  

•   int     (*exit)(struct phy *phy);
  

•   /**
  

•   * reset - resets a PHY device without shutting down
  

•   *
  

•   * @phy: PHY port to be reset
  

•   *
  

•   * During runtime, the PHY may need to be reset in order to
  

•   * re-establish connection etc without being shut down or exit.
  

•   *
  

•   * @return 0 if OK, or a negative error code
  

•   */
  

•   int     (*reset)(struct phy *phy);
  

•   /**
  

•   * power_on - power on a PHY device
  

•   *
  

•   * @phy: PHY port to be powered on
  

•   *
  

•   * During runtime, the PHY may need to be powered on or off several
  

•   * times. This function is used to power on the PHY. It relies on the
  

•   * setup done in init(). If init() is not implemented, it must take care
  

•   * of setting up the context (PLLs, ...)
  

•   *
  

•   * @return 0 if OK, or a negative error code
  

•   */
  

•   int     (*power_on)(struct phy *phy);
  

•   /**
  

•   * power_off - power off a PHY device
  

•   *
  

•   * @phy: PHY port to be powered off
  

•   *
  

•   * During runtime, the PHY may need to be powered on or off several
  

•   * times. This function is used to power off the PHY. Except if
  

•   * init()/deinit() are not implemented, it must not de-initialize
  

•   * everything.
  

•   *
  

•   * @return 0 if OK, or a negative error code
  

•   */
  

•   int     (*power_off)(struct phy *phy);
  

+};

+/**

• • generic_phy_init() - initialize the PHY port
• • @phy: the PHY port to initialize
• • @return 0 if OK, or a negative error code
• */

+int generic_phy_init(struct phy *phy);

+/**

• • generic_phy_init() - de-initialize the PHY device
• • @phy: PHY port to be de-initialized
• • @return 0 if OK, or a negative error code
• */

+int generic_phy_exit(struct phy *phy);

+/**

• • generic_phy_reset() - resets a PHY device without shutting down
• • @phy: PHY port to be reset
• *@return 0 if OK, or a negative error code
• */

+int generic_phy_reset(struct phy *phy);

+/**

• • generic_phy_power_on() - power on a PHY device
• • @phy: PHY port to be powered on
• • @return 0 if OK, or a negative error code
• */

+int generic_phy_power_on(struct phy *phy);

+/**

• • generic_phy_power_off() - power off a PHY device
• • @phy: PHY port to be powered off
• • @return 0 if OK, or a negative error code
• */

+int generic_phy_power_off(struct phy *phy);

+/**

• • generic_phy_get_by_index() - Get a PHY device by integer index.
• • @user: the client device
• • @index: The index in the list of available PHYs
• • @phy: A pointer to the PHY port
• • This looks up a PHY device for a client device based on its position in the
• • list of the possible PHYs.
• • example:
• • usb1: usb_otg_ss@xxx {

• •   compatible = "xxx";
    

• •   reg = <xxx>;
    

• • .
• • .
• • phys = <&usb2_phy>, <&usb3_phy>;
• • .
• • .
• • };
• • the USB2 phy can be accessed by passing index '0' and the USB3 phy can
• • be accessed by passing index '1'
• • @return 0 if OK, or a negative error code
• */

+int generic_phy_get_by_index(struct udevice *user, int index,

•                        struct phy *phy);
  

+/**

• • generic_phy_get_by_name() - Get a PHY device by its name.
• • @user: the client device
• • @phy_name: The name of the PHY in the list of possible PHYs
• • @phy: A pointer to the PHY port
• • This looks up a PHY device for a client device in the
• • list of the possible PHYs based on its name.
• • example:
• • usb1: usb_otg_ss@xxx {

• •   compatible = "xxx";
    

• •   reg = <xxx>;
    

• • .
• • .
• • phys = <&usb2_phy>, <&usb3_phy>;
• • phy-names = "usb2phy", "usb3phy";
• • .
• • .
• • };
• • the USB3 phy can be accessed using "usb3phy", and USB2 by using "usb2phy"
• • @return 0 if OK, or a negative error code
• */

+int generic_phy_get_by_name(struct udevice *user, const char *phy_name,

•                       struct phy *phy);
  

+#endif /*__GENERIC_PHY_H */

1.9.1

Regards, Simon