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

Simon Glass sjg at chromium.org
Wed Apr 22 18:30:01 CEST 2015


Hi Przemyslaw,

On 20 April 2015 at 12:07, Przemyslaw Marczak <p.marczak at 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 at 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 at 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 at 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 at samsung.com>
> + *
>   *  Copyright (C) 2011-2012 Samsung Electronics
>   *  Lukasz Majewski <l.majewski at 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 at 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


More information about the U-Boot mailing list