Re: [U-Boot] [PATCH v4 04/16] dm: pmic: add implementation of driver model pmic uclass

Hi Przemyslaw,

On 20 April 2015 at 12:07, Przemyslaw Marczak p.marczak@samsung.com wrote:

This commit introduces the PMIC uclass implementation. It allows providing the basic I/O interface for PMIC devices. For the multi-function PMIC devices, this can be used as I/O parent device, for each IC's interface. Then, each PMIC particular function can be provided by the child device's operations, and the child devices will use its parent for read/write by the common API.

Core files:

• 'include/power/pmic.h'
• 'drivers/power/pmic/pmic-uclass.c'

The old pmic framework is still kept and is independent.

For more detailed informations, please look into the header file.

Changes:

• new uclass-id: UCLASS_PMIC
• new config: CONFIG_DM_PMIC

Signed-off-by: Przemyslaw Marczak p.marczak@samsung.com

Changes V2:

• pmic uclass: adjust uclass code to the mainline changes
• pmic uclass: remove pmic_i2c and pmic_spi
• pmic uclass: modify pmic_platdata
• pmic uclass: add pmic_if_* functions
• pmic uclass: remove pmic_init_dm()
• pmic uclass: cleanup
• pmic.h: define pmic ops structure (read/write operations)
• pmic.h: add comments to functions

Changes V3:

• pmic-uclass.c and pmic.h: -- remove pmic_if_* functions -- add new function pmic_child_node_scan()
• add Kconfig entry

Changes V4:

• move drivers/power/pmic-uclass.c to drivers/power/pmic/pmic-uclass.c
• move DM_PMIC Kconfig entry: drivers/power/Kconfig to drivers/power/pmic/Kconfig
• drivers/power/Kconfig: Add menu "Power" and include pmic Kconfig
• Kconfig: provide only the general information about the PMIC
• pmic-uclass.c: add pmic_bind_childs()
• pmic-uclass.c: add debug
• pmic-uclass.c: cleanup includes
• pmic-uclass.c: remove pmic_get_uclass_ops() and use of dev_get_driver_ops()
• pmic-uclass.c: use of uclass_get_device_by_name()
• pmic-uclass.c: add str_get_num() - for get number from string
• include/power/pmic.h - start comments rewording
• power/pmic.h: comments update

---

drivers/power/Kconfig | 6 ++ drivers/power/pmic/Kconfig | 11 +++ drivers/power/pmic/Makefile | 1 + drivers/power/pmic/pmic-uclass.c | 158 ++++++++++++++++++++++++++++++++ include/dm/uclass-id.h | 3 + include/power/pmic.h | 189 +++++++++++++++++++++++++++++++++++++++ 6 files changed, 368 insertions(+) create mode 100644 drivers/power/pmic/Kconfig create mode 100644 drivers/power/pmic/pmic-uclass.c

Acked-by: Simon Glass sjg@chromium.org

I have a few nits below - perhaps they can be targeted in a follow-up patch or two? I'd like to merge this soon and it is not worth holding up the series for nits.

diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig index f8f0239..d03626e 100644 --- a/drivers/power/Kconfig +++ b/drivers/power/Kconfig @@ -1,3 +1,7 @@ +menu "Power"

+source "drivers/power/pmic/Kconfig"

config AXP221_POWER boolean "axp221 / axp223 pmic support" depends on MACH_SUN6I || MACH_SUN8I @@ -73,3 +77,5 @@ config AXP221_ELDO3_VOLT disable eldo3. On some A31(s) tablets it might be used to supply 1.2V for the SSD2828 chip (converter of parallel LCD interface into MIPI DSI).

+endmenu diff --git a/drivers/power/pmic/Kconfig b/drivers/power/pmic/Kconfig new file mode 100644 index 0000000..d06d632 --- /dev/null +++ b/drivers/power/pmic/Kconfig @@ -0,0 +1,11 @@ +config DM_PMIC

•   bool "Enable Driver Model for PMIC drivers (UCLASS_PMIC)"
  

•   depends on DM
  

•   ---help---
  

•   This config enables the driver-model PMIC support.
  

•   UCLASS_PMIC - designed to provide an I/O interface for PMIC devices.
  

•   For the multi-function PMIC devices, this can be used as parent I/O
  

•   device for each IC's interface. Then, each children uses its parent
  

•   for read/write. For detailed description, please refer to the files:
  

•   - 'drivers/power/pmic/pmic-uclass.c'
  

•   - 'include/power/pmic.h'
  

diff --git a/drivers/power/pmic/Makefile b/drivers/power/pmic/Makefile index 985cfdb..594f620 100644 --- a/drivers/power/pmic/Makefile +++ b/drivers/power/pmic/Makefile @@ -5,6 +5,7 @@ # SPDX-License-Identifier: GPL-2.0+ #

+obj-$(CONFIG_DM_PMIC) += pmic-uclass.o obj-$(CONFIG_POWER_LTC3676) += pmic_ltc3676.o obj-$(CONFIG_POWER_MAX8998) += pmic_max8998.o obj-$(CONFIG_POWER_MAX8997) += pmic_max8997.o diff --git a/drivers/power/pmic/pmic-uclass.c b/drivers/power/pmic/pmic-uclass.c new file mode 100644 index 0000000..d82d3da --- /dev/null +++ b/drivers/power/pmic/pmic-uclass.c @@ -0,0 +1,158 @@ +/*

• • Copyright (C) 2014-2015 Samsung Electronics
• • Przemyslaw Marczak p.marczak@samsung.com
• • SPDX-License-Identifier: GPL-2.0+
• */

+#include <common.h> +#include <fdtdec.h> +#include <errno.h> +#include <dm.h> +#include <dm/lists.h> +#include <dm/device-internal.h> +#include <dm/uclass-internal.h> +#include <power/pmic.h> +#include <linux/ctype.h>

+DECLARE_GLOBAL_DATA_PTR;

+static ulong str_get_num(const char *ptr, const char *maxptr) +{

•   if (!ptr || !maxptr)
  

•           return 0;
  

•   while (!isdigit(*ptr) && ptr++ < maxptr);
  

•   return simple_strtoul(ptr, NULL, 0);
  

+}

+int pmic_bind_childs(struct udevice *pmic, int offset,

•                const struct pmic_child_info *child_info)
  

+{

•   const struct pmic_child_info *info;
  

•   const void *blob = gd->fdt_blob;
  

•   struct driver *drv;
  

•   struct udevice *child;
  

•   const char *node_name;
  

•   int node_name_len;
  

•   int bind_count = 0;
  

•   int node;
  

•   int prefix_len;
  

•   int ret;
  

•   debug("%s for '%s' at node offset: %d\n", __func__, pmic->name,
  

•         pmic->of_offset);
  

•   for (node = fdt_first_subnode(blob, offset);
  

•        node > 0;
  

•        node = fdt_next_subnode(blob, node)) {
  

•           node_name = fdt_get_name(blob, node, &node_name_len);
  

•           debug("* Found child node: '%s' at offset:%d\n", node_name,
  

•                                                            node);
  

•           child = NULL;
  

•           info = child_info;
  

•           while (info->prefix) {
  

•                   prefix_len = strlen(info->prefix);
  

•                   if (strncasecmp(info->prefix, node_name, prefix_len) ||
  

•                       !info->driver) {
  

•                           info++;
  

•                           continue;
  

•                   }
  

•                   debug("  - compatible prefix: '%s'\n", info->prefix);
  

•                   drv = lists_driver_lookup_name(info->driver);
  

•                   if (!drv) {
  

•                           debug("  - driver: '%s' not found!\n",
  

•                                 info->driver);
  

•                           continue;
  

•                   }
  

•                   debug("  - found child driver: '%s'\n", drv->name);
  

•                   ret = device_bind(pmic, drv, node_name, NULL,
  

•                                     node, &child);
  

•                   if (ret) {
  

•                           debug("  - child binding error: %d\n", ret);
  

•                           continue;
  

•                   }
  

•                   debug("  - bound child device: '%s'\n", child->name);
  

•                   child->driver_data = str_get_num(node_name +
  

•                                                    prefix_len,
  

•                                                    node_name +
  

•                                                    node_name_len);
  

•                   debug("  - set 'child->driver_data': %lu\n",
  

•                         child->driver_data);
  

•                   break;
  

•           }
  

•           if (child)
  

•                   bind_count++;
  

•           else
  

•                   debug("  - compatible prefix not found\n");
  

•   }
  

•   debug("Bound: %d childs for PMIC: '%s'\n", bind_count, pmic->name);
  

•   return bind_count;
  

+}

+int pmic_get(const char *name, struct udevice **devp) +{

•   return uclass_get_device_by_name(UCLASS_PMIC, name, devp);
  

+}

+int pmic_reg_count(struct udevice *dev) +{

•   const struct dm_pmic_ops *ops = dev_get_driver_ops(dev);
  

blank line here

•   if (!ops)
  

•           return -ENOSYS;
  

•   return ops->reg_count;
  

+}

+int pmic_read(struct udevice *dev, uint reg, uint8_t *buffer, int len) +{

•   const struct dm_pmic_ops *ops = dev_get_driver_ops(dev);
  

•   int ret;
  

•   if (!buffer)
  

•           return -EFAULT;
  

•   if (!ops || !ops->read)
  

•           return -ENOSYS;
  

•   ret = ops->read(dev, reg, buffer, len);
  

•   if (ret)
  

•           return ret;
  

•   return 0;
  

+}

+int pmic_write(struct udevice *dev, uint reg, const uint8_t *buffer, int len) +{

•   const struct dm_pmic_ops *ops = dev_get_driver_ops(dev);
  

•   int ret;
  

•   if (!buffer)
  

•           return -EFAULT;
  

•   if (!ops || !ops->write)
  

•           return -ENOSYS;
  

•   ret = ops->write(dev, reg, buffer, len);
  

•   if (ret)
  

•           return ret;
  

•   return 0;
  

+}

+UCLASS_DRIVER(pmic) = {

•   .id             = UCLASS_PMIC,
  

•   .name           = "pmic",
  

+}; diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index fddfd35..23b3eb9 100644 --- a/include/dm/uclass-id.h +++ b/include/dm/uclass-id.h @@ -46,6 +46,9 @@ enum uclass_id { UCLASS_USB_DEV_GENERIC, /* USB generic device */ UCLASS_MASS_STORAGE, /* Mass storage device */

•   /* Power Management */
  

•   UCLASS_PMIC,            /* PMIC I/O device */
  

•   UCLASS_COUNT,
    UCLASS_INVALID = -1,
  

}; diff --git a/include/power/pmic.h b/include/power/pmic.h index afbc5aa..f7ae781 100644 --- a/include/power/pmic.h +++ b/include/power/pmic.h @@ -1,4 +1,7 @@ /*

• • Copyright (C) 2014-2015 Samsung Electronics
• • Przemyslaw Marczak p.marczak@samsung.com
• • Copyright (C) 2011-2012 Samsung Electronics
  • Lukasz Majewski l.majewski@samsung.com

@@ -9,10 +12,13 @@ #define __CORE_PMIC_H_

#include <linux/list.h> +#include <spi.h> #include <i2c.h> #include <power/power_chrg.h>

At some point can you update the ordering of this lot? I think it should be:

i2c.g spi.h linux/ power/

enum { PMIC_I2C, PMIC_SPI, PMIC_NONE};

+#ifdef CONFIG_POWER enum { I2C_PMIC, I2C_NUM, }; enum { PMIC_READ, PMIC_WRITE, }; enum { PMIC_SENSOR_BYTE_ORDER_LITTLE, PMIC_SENSOR_BYTE_ORDER_BIG, }; @@ -77,7 +83,189 @@ struct pmic { struct pmic *parent; struct list_head list; }; +#endif /* CONFIG_POWER */

+#ifdef CONFIG_DM_PMIC +/**

• • U-Boot PMIC Framework
• • =====================
• • UCLASS_PMIC - The is designed to provide an I/O interface for PMIC devices.

This is designed

• • For the multi-function PMIC devices, this can be used as parent I/O device
• • for each IC's interface. Then, each children uses its parent for read/write.

each child

• • The driver model tree could look like this:
• *_ root device
• • |_ BUS 0 device (e.g. I2C0) - UCLASS_I2C/SPI/...
• • | |_ PMIC device (READ/WRITE ops) - UCLASS_PMIC
• • | |_ REGULATOR device (ldo/buck/... ops) - UCLASS_REGULATOR
• • | |_ CHARGER device (charger ops) - UCLASS_CHARGER (in the future)
• • | |_ MUIC device (microUSB connector ops) - UCLASS_MUIC (in the future)
• • | |_ ...
• • |
• • |_ BUS 1 device (e.g. I2C1) - UCLASS_I2C/SPI/...
• • |_ PMIC device (READ/WRITE ops) - UCLASS_PMIC

• • |_ RTC device (rtc ops)                 - UCLASS_RTC     (in the future)
    

• • We can find two PMIC cases in boards design:
• • • single I/O interface
• • • multiple I/O interfaces
• • We bind single PMIC device for each interface, to provide an I/O as a parent,

a single

• • of proper child devices. Each child usually implements a different function,
• • controlled by the same interface.
• • The binding should be done automatically. If device tree nodes/subnodes are
• • proper defined, then:
• • |_ the ROOT driver will bind the device for I2C/SPI node:
• • |_ the I2C/SPI driver should bind a device for pmic node:

• • |_ the PMIC driver should bind devices for its childs:
    

• •   |_ regulator (child)
    

• •   |_ charger   (child)
    

• •   |_ other     (child)
    

• • The same for other device nodes, for multi-interface PMIC.
• • Note:
• • Each PMIC interface driver should use a different compatible string.
• • If each pmic child device driver need access the PMIC-specific registers,

If a pmic child device driver needs

• • it need know only the register address and the access can be done through
• • the parent pmic driver. Like in the example:
• *_ root driver
• • |_ dev: bus I2C0 - UCLASS_I2C
• • | |_ dev: my_pmic (read/write) (is parent) - UCLASS_PMIC
• • | |_ dev: my_regulator (set value/etc..) (is child) - UCLASS_REGULATOR
• • To ensure such device relationship, the pmic device driver should also bind
• • all its child devices, like in the example below. It should be done by call

by calling pmic_bind_childs()

(which you should rename to pmic_bind_children() I think)

• • the 'pmic_bind_childs()' - please refer to the description of this function
• • in this header file. This function, should be called in the driver's '.bind'
• • method.
• • For the example driver, please refer the MAX77686 driver:
• • • 'drivers/power/pmic/max77686.c'
• */

+/**

• • struct dm_pmic_ops - PMIC device I/O interface
• • Should be implemented by UCLASS_PMIC device drivers. The standard
• • device operations provides the I/O interface for it's childs.
• • @reg_count: devices register count
• • @read: read 'len' bytes at "reg" and store it into the 'buffer'
• • @write: write 'len' bytes from the 'buffer' to the register at 'reg' address
• */

+struct dm_pmic_ops {

•   int reg_count;
  

This should not be in ops as it is not an operation. Perhaps add a function for it, or put it in the uclass platform data?

•   int (*read)(struct udevice *dev, uint reg, uint8_t *buffer, int len);
  

•   int (*write)(struct udevice *dev, uint reg, const uint8_t *buffer,
  

•                int len);
  

+};

+/* enum pmic_op_type - used for various pmic devices operation calls,

/** * enum pmic_op_type -

• • for reduce a number of lines with the same code for read/write or get/set.
• • @PMIC_OP_GET - get operation
• • @PMIC_OP_SET - set operation

+*/ +enum pmic_op_type {

•   PMIC_OP_GET,
  

•   PMIC_OP_SET,
  

+};

+/**

• • struct pmic_child_info - basic device's child info for bind child nodes with
• • the driver by the node name prefix and driver name. This is a helper struct
• • for function: pmic_bind_childs().
• • @prefix - child node name prefix (or its name if is unique or single)
• • @driver - driver name for the sub-node with prefix
• */

+struct pmic_child_info {

•   const char *prefix;
  

•   const char *driver;
  

+};

+/* drivers/power/pmic-uclass.c */

+/**

• • pmic_bind_childs() - bind drivers for given parent pmic, using child info
• • found in 'child_info' array.
• • @pmic - pmic device - the parent of found child's
• • @child_info - N-childs info array
• • @return a positive number of childs, or 0 if no child found (error)
• • Note: For N-childs the child_info array should have N+1 entries and the last
• • entry prefix should be NULL - the same as for drivers compatible.
• • For example, a single prefix info (N=1):
• • static const struct pmic_child_info bind_info[] = {

• • { .prefix = "ldo", .driver = "ldo_driver" },
    

• • { },
    

• • };
• • This function is useful for regulator sub-nodes:
• • my_regulator@0xa {

• • reg = <0xa>;
    

• • (pmic - bind automatically by compatible)
    

• • compatible = "my_pmic";
    

• • ...
    

• • (pmic's childs - bind by pmic_bind_childs())
    

• • (nodes prefix: "ldo", driver: "my_regulator_ldo")
    

• • ldo1 { ... };
    

• • ldo2 { ... };
    

• • (nodes prefix: "buck", driver: "my_regulator_buck")
    

• • buck1 { ... };
    

• • buck2 { ... };
    

• • };
• */

+int pmic_bind_childs(struct udevice *pmic, int offset,

•                const struct pmic_child_info *child_info);
  

pmic_bind_children

+/**

• • pmic_get: get the pmic device using its name
• • @name - device name
• • @devp - returned pointer to the pmic device
• • @return 0 on success or negative value of errno.
• • The returned devp device can be used with pmic_read/write calls
• */

+int pmic_get(const char *name, struct udevice **devp);

+/**

• • pmic_reg_count: get the pmic register count
• • The required pmic device can be obtained by 'pmic_get()'
• • @dev - pointer to the UCLASS_PMIC device
• • @return register count value on success or negative value of errno.
• */

+int pmic_reg_count(struct udevice *dev);

+/**

• • pmic_read/write: read/write to the UCLASS_PMIC device
• • The required pmic device can be obtained by 'pmic_get()'
• • @pmic - pointer to the UCLASS_PMIC device
• • @reg - device register offset
• • @buffer - pointer to read/write buffer
• • @len - byte count for read/write
• • @return 0 on success or negative value of errno.
• */

+int pmic_read(struct udevice *dev, uint reg, uint8_t *buffer, int len); +int pmic_write(struct udevice *dev, uint reg, const uint8_t *buffer, int len); +#endif /* CONFIG_DM_PMIC */

+#ifdef CONFIG_POWER int pmic_init(unsigned char bus); int power_init_board(void); int pmic_dialog_init(unsigned char bus); @@ -88,6 +276,7 @@ int pmic_probe(struct pmic *p); int pmic_reg_read(struct pmic *p, u32 reg, u32 *val); int pmic_reg_write(struct pmic *p, u32 reg, u32 val); int pmic_set_output(struct pmic *p, u32 reg, int ldo, int on); +#endif

#define pmic_i2c_addr (p->hw.i2c.addr)

#define pmic_i2c_tx_num (p->hw.i2c.tx_num)

1.9.1

Regards, Simon