Re: [U-Boot] [PATCH v2 03/12] dm: pmic: add implementation of driver model pmic uclass

Hi Przemyslaw,

On 3 March 2015 at 09:24, Przemyslaw Marczak p.marczak@samsung.com wrote:

This is an introduction to driver-model multi uclass PMIC support. It starts with UCLASS_PMIC - a common PMIC devices uclass type to provide device read/write operations only.

Beside two basic operations the pmic platform data is introduced, which provides basic informations about the pmic device I/O interface and is shared with all childs (and should also for childs new uclass types in the future).

Usually PMIC devices provides various functionalities with single or multiple I/O interfaces. Using this new framework and new uclass types introduced in the future, it can be handle like this:

_ root device | |_ BUS 0 device (e.g. I2C0) - UCLASS_I2C/SPI/... | |_ PMIC device 1 (read/write ops) - UCLASS_PMIC | |_ REGULATOR device (ldo/buck/... ops) - UCLASS_REGULATOR | |_ CHARGER device (charger ops) - UCLASS_CHARGER (in the future) | |_ MUIC device (microUSB con ops) - UCLASS_MUIC (in the future) | |_ ... | |_ BUS 1 device (e.g. I2C1) - UCLASS_I2C/SPI/... |_ PMIC device 2 (read/write ops) - UCLASS_PMIC |_ RTC device (rtc ops) - UCLASS_MUIC (in the future)

For each PMIC device interface, new UCLASS_PMIC device is bind with proper pmic driver, and it's child devices provides some specified operations.

All new definitions can be found in file:

• 'include/power/pmic.h'

Uclass file:

• pmic-uclass.c - provides a common code for UCLASS_PMIC device drivers

The old pmic framework is still kept and is independent.

Changes:

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

New pmic api is documented in: doc/README.power-framework-dm

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

---

drivers/power/Makefile | 1 + drivers/power/pmic-uclass.c | 191 +++++++++++++++++++++++++++++++ include/dm/uclass-id.h | 3 + include/power/pmic.h | 265 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 460 insertions(+) create mode 100644 drivers/power/pmic-uclass.c

This should have a Kconfig file.

diff --git a/drivers/power/Makefile b/drivers/power/Makefile index 2145652..5c9a189 100644 --- a/drivers/power/Makefile +++ b/drivers/power/Makefile @@ -21,3 +21,4 @@ obj-$(CONFIG_DIALOG_POWER) += power_dialog.o obj-$(CONFIG_POWER_FSL) += power_fsl.o obj-$(CONFIG_POWER_I2C) += power_i2c.o obj-$(CONFIG_POWER_SPI) += power_spi.o +obj-$(CONFIG_DM_PMIC) += pmic-uclass.o diff --git a/drivers/power/pmic-uclass.c b/drivers/power/pmic-uclass.c new file mode 100644 index 0000000..309463e --- /dev/null +++ b/drivers/power/pmic-uclass.c @@ -0,0 +1,191 @@ +/*

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

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

+DECLARE_GLOBAL_DATA_PTR;

+static char * const pmic_interfaces[] = {

•   "I2C",
  

•   "SPI",
  

•   "--",
  

+};

+const char *pmic_if_str(struct udevice *pmic) +{

•   int if_types = ARRAY_SIZE(pmic_interfaces);
  

•   int if_type;
  

•   if_type = pmic_if_type(pmic);
  

•   if (if_type < 0 || if_type >= if_types)
  

•           return pmic_interfaces[if_types - 1];
  

•   return pmic_interfaces[if_type];
  

+}

+int pmic_if_type(struct udevice *pmic) +{

•   struct pmic_platdata *pl = pmic->platdata;
  

•   return pl->if_type;
  

+}

+int pmic_if_bus_num(struct udevice *pmic) +{

•   struct pmic_platdata *pl = pmic->platdata;
  

•   return pl->if_bus_num;
  

+}

+int pmic_if_addr_cs(struct udevice *pmic) +{

•   struct pmic_platdata *pl = pmic->platdata;
  

•   return pl->if_addr_cs;
  

+}

+int pmic_if_max_offset(struct udevice *pmic) +{

•   struct pmic_platdata *pl = pmic->platdata;
  

•   return pl->if_max_offset;
  

+}

+int pmic_read(struct udevice *pmic, unsigned reg, unsigned char *val) +{

•   const struct dm_pmic_ops *ops;
  

•   ops = pmic_get_uclass_ops(pmic, UCLASS_PMIC);
  

•   if (!ops)
  

•           return -ENODEV;
  

•   if (!ops->read)
  

•           return -EPERM;
  

•   if (ops->read(pmic, reg, val))
  

•           return -EIO;
  

•   return 0;
  

+}

+int pmic_write(struct udevice *pmic, unsigned reg, unsigned char val) +{

•   const struct dm_pmic_ops *ops;
  

•   ops = pmic_get_uclass_ops(pmic, UCLASS_PMIC);
  

•   if (!ops)
  

•           return -ENODEV;
  

•   if (!ops->write)
  

•           return -EPERM;
  

•   if (ops->write(pmic, reg, val))
  

•           return -EIO;
  

•   return 0;
  

+}

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

•   struct udevice *dev;
  

•   struct uclass *uc;
  

•   int ret;
  

•   ret = uclass_get(UCLASS_PMIC, &uc);
  

•   if (ret) {
  

•           error("PMIC uclass not initialized!");
  

•           return ret;
  

•   }
  

•   uclass_foreach_dev(dev, uc) {
  

•           if (!dev)
  

•                   continue;
  

•           if (!strncmp(name, dev->name, strlen(name))) {
  

•                   ret = device_probe(dev);
  

•                   if (ret)
  

•                           error("Dev: %s probe failed", dev->name);
  

•                   *pmic = dev;
  

•                   return ret;
  

•           }
  

•   }
  

•   *pmic = NULL;
  

•   return -EINVAL;
  

+}

+int pmic_i2c_get(int bus, int addr, struct udevice **pmic) +{

•   struct udevice *dev;
  

•   int ret;
  

•   ret = i2c_get_chip_for_busnum(bus, addr, 1, &dev);
  

•   if (ret) {
  

•           error("Chip: %d on bus %d not found!", addr, bus);
  

•           *pmic = NULL;
  

•           return ret;
  

•   }
  

•   *pmic = dev;
  

•   return 0;
  

+}

+int pmic_spi_get(int bus, int cs, struct udevice **pmic) +{

•   struct udevice *busp, *devp;
  

•   int ret;
  

•   ret = spi_find_bus_and_cs(bus, cs, &busp, &devp);
  

•   if (ret) {
  

•           error("Chip: %d on bus %d not found!", cs, bus);
  

•           *pmic = NULL;
  

•           return ret;
  

•   }
  

•   *pmic = devp;
  

•   return 0;
  

+}

+int pmic_io_dev(struct udevice *pmic, struct udevice **pmic_io) +{

•   struct pmic_platdata *pl;
  

•   int ret;
  

•   pl = dev_get_platdata(pmic);
  

•   if (!pl) {
  

•           error("pmic: %s platdata not found!", pmic->name);
  

•           return -EINVAL;
  

•   }
  

•   switch (pl->if_type) {
  

•   case PMIC_I2C:
  

•           ret = pmic_i2c_get(pl->if_bus_num, pl->if_addr_cs, pmic_io);
  

•           break;
  

•   case PMIC_SPI:
  

•           ret = pmic_spi_get(pl->if_bus_num, pl->if_addr_cs, pmic_io);
  

•           break;
  

•   default:
  

•           error("Unsupported interface for: %s", pmic->name);
  

•           ret = -ENXIO;
  

•   }
  

•   return ret;
  

+}

+UCLASS_DRIVER(pmic) = {

•   .id             = UCLASS_PMIC,
  

•   .name           = "pmic",
  

+}; diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index 91bb90d..ff360ac 100644 --- a/include/dm/uclass-id.h +++ b/include/dm/uclass-id.h @@ -35,6 +35,9 @@ enum uclass_id { UCLASS_I2C_EEPROM, /* I2C EEPROM device */ UCLASS_MOD_EXP, /* RSA Mod Exp device */

•   /* PMIC uclass and PMIC related uclasses */
  

PMIC-related

•   UCLASS_PMIC,
  

•   UCLASS_COUNT,
    UCLASS_INVALID = -1,
  

}; diff --git a/include/power/pmic.h b/include/power/pmic.h index afbc5aa..94171ac 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>

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,265 @@ struct pmic { struct pmic *parent; struct list_head list; }; +#endif /* CONFIG_POWER */

+#ifdef CONFIG_DM_PMIC +/**

• • Driver model pmic framework.
• • The PMIC_UCLASS uclass is designed to provide a common I/O
• • interface for pmic child devices of various uclass types.
• • Usually PMIC devices provides more than one functionality,
• • which means that we should implement uclass operations for
• • each functionality - one driver per uclass.
• • And usually PMIC devices provide those various functionalities
• • by one or more interfaces. And this could looks 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_MUIC (in the future)
    

• • Two PMIC types are possible:
• • • single I/O interface
• • Then UCLASS_PMIC device should be a parent of all pmic devices, where each
• • is usually different uclass type, but need to access the same interface
• • • multiple I/O interfaces
• • For each interface the UCLASS_PMIC device should be a parent of only those
• • devices (different uclass types), which needs to access the specified
• • interface.
• • For each case binding should be done automatically. If only 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 bus nodes, if pmic uses more then one interface.
• • Note:
• • Each PMIC interface driver should use different compatible string.
• • If each pmic child device driver need access the pmic specified registers,
• • it need to know only the register address and the access is 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 this example:
• • my_pmic.c - pmic driver:
• • int my_pmic_bind(struct udevice *my_pmic)
• • {

• •     // A list of other drivers for this pmic
    

• •     char *my_pmic_drv_list = {"my_regulator", "my_charger", "my_rtc"};
    

• •     struct udevice *my_pmic_child_dev[3];
    

• • ...

• •     for (i=0; i < ARRAY_SIZE(my_pmic_drv_list); i++) {
    

• •             // Look for child device driver
    

• •             drv_my_reg = lists_driver_lookup_name(my_pmic_drv_list[i]);
    

• •             // If driver found, then bind the new device to it
    

• •             if (drv_my_reg)
    

• •                     ret = device_bind(my_pmic, my_pmic_drv_list[i],
    

• •                                       my_pmic->name, my_pmic->platdata,
    

• •                                       my_pmic->of_offset,
    

• •                                       my_pmic_child_dev[i]);
    

• •             ...
    

• •     }
    

Can we not automatically do this with dm_scan_fdt_node() as in spi_post_bind(), etc.?

• • ...
• • }
• • my_regulator.c - regulator driver (child of pmic I/O driver):
• • int my_regulator_set_value(struct udevice *dev, int type, int num, int *val)
• • {
• • ...

• •     some_val = ...;
    

• •     // dev->parent == my_pmic
    

• •     pmic_write(dev->parent, some_reg, some_val);
    

• • ...
• • }
• • In this example pmic driver is always a parent for other pmic devices,
• • because the first argument of device_bind() call is the parent device,
• • and here it is 'my_pmic'.
• • For all childs the device->platdata is the same, and provide an info about
• • the same interface - it's used for getting the devices by interface address
• • and uclass types.
• */

+/**

• • struct pmic_platdata - PMIC chip interface info
• • This is required for pmic command purposes and should be shared
• • by pmic device and it's childs as the same platdata pointer.
• • @if_type: one of: PMIC_I2C, PMIC_SPI, PMIC_NONE
• • @if_bus_num: usually alias seq of i2c/spi bus device
• • @if_addr_cd: i2c/spi chip addr or cs
• • @if_max_offset: max register offset within the chip
• */

+struct pmic_platdata {

•   int if_type;
  

•   int if_bus_num;
  

•   int if_addr_cs;
  

•   int if_max_offset;
  

+};

Can we drop the if_ prefix and use an enum for if_type?

For bus_num, this will be the parent of the pmic. There should be no need to store it.

For addr_cs, there should be no need to store this. The pmic will be either on an i2c or spi bus - both of these provide the address information in dev_get_parent_platdata().

if_max_offset should be in the PMIC platdata but not that of its children. So I don't think they should all use the same platdata.

+/**

• • 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.
• • @read: called for read "reg" value into "val" through "pmic" parent
• • @write: called for write "val" into "reg" through the "pmic" parent
• */

+struct dm_pmic_ops {

•   int (*read)(struct udevice *pmic, unsigned reg, unsigned char *val);
  

Can any pmics support reading 16 bits or more?

•   int (*write)(struct udevice *pmic, unsigned reg, unsigned char val);
  

+};

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

Comment style

• • 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,
  

+};

+/**

• • pmic_get_uclass_ops - inline function for getting device uclass operations
• • @dev - device to check
• • @uclass_id - device uclass id
• • Returns:
• • void pointer to device operations or NULL pointer on error
• */

+static __inline__ const void *pmic_get_uclass_ops(struct udevice *dev,

•                                             int uclass_id)
  

enum

+{

•   const void *ops;
  

•   if (!dev)
  

•           return NULL;
  

•   if (dev->driver->id != uclass_id)
  

•           return NULL;
  

•   ops = dev->driver->ops;
  

•   if (!ops)
  

•           return NULL;
  

•   return ops;
  

+}

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

+/**

• • pmic_..._get: looking for the pmic device using the specified arguments:
• • NAME:
• • @name - device name (useful for single specific names)
• • or SPI/I2C interface:
• • @bus - device I2C/SPI bus number
• • @addr_cs - device specific address for I2C or chip select for SPI,

• •         on the given bus number
    

• • Returns:
• • @pmic - returned pointer to the pmic device
• • Returns: 0 on success or negative value of errno.
• • The returned pmic device can be used with:
• • • pmic_read/write calls
• • • pmic_if_* calls
• */

+int pmic_get(char *name, struct udevice **pmic); +int pmic_i2c_get(int bus, int addr, struct udevice **pmic); +int pmic_spi_get(int bus, int cs, struct udevice **pmic);

These last two should be internal to the pmic framework I think. Clients should not care.

Also we should reference SPI and I2C devices with a struct udevice, not a bus/addr or bus/cs pair.

+/**

• • pmic_io_dev: returns given pmic/regulator, uclass pmic I/O udevice
• • @pmic - pointer to pmic_io child device
• • Returns:
• • @pmic_io - pointer to the I/O chip device
• • This is used for pmic command, and also can be used if
• • needs raw read/write operations for uclass regulator device.
• • Example of use:
• • pmic_io_dev(regulator, &pmic_io);
• • pmic_write(pmic_io, 0x0, 0x1);
• • This base on pmic_platdata info, so for the given pmic i2c/spi chip,
• • as '@pmic', it should return the same chip, if platdata is filled.
• */

+int pmic_io_dev(struct udevice *pmic, struct udevice **pmic_io);

+/**

• • pmic_if_* functions: returns one of given pmic interface attribute:
• • • type - one of enum { PMIC_I2C, PMIC_SPI, PMIC_NONE}
• • • bus number - usually i2c/spi bus seq number
• • • addres for i2c or cs for spi - basaed on dm_i2c/spi_chip
• • • max offset - device internal address range
• • • string - "I2C"/"SPI"/"--"
• • @pmic - pointer to the pmic device
• • Returns: one of listed attribute on success, or a negative value of errno.
• */

+int pmic_if_type(struct udevice *pmic); +int pmic_if_bus_num(struct udevice *pmic); +int pmic_if_addr_cs(struct udevice *pmic); +int pmic_if_max_offset(struct udevice *pmic); +const char *pmic_if_str(struct udevice *pmic);

I hope that none of these is needed in fact.

+/**

• • pmic_read/write: read/write to the UCLASS_PMIC device
• • The required pmic device can be obtained by:
• • • pmic_i2c_get()
• • • pmic_spi_get()
• • • pmic_io_dev() - for pmic command purposes
• • @pmic - pointer to the UCLASS_PMIC device
• • @reg - device register offset
• • @val - value for write or its pointer to read
• • Returns: offset value on success or negative value of errno.

s/offset/register/ ?

• */

+int pmic_read(struct udevice *pmic, unsigned reg, unsigned char *val); +int pmic_write(struct udevice *pmic, unsigned reg, unsigned char val); +#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 +352,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