[U-Boot] [PATCH v2 03/12] dm: pmic: add implementation of driver model pmic uclass
Przemyslaw Marczak
p.marczak at samsung.com
Wed Mar 25 17:08:12 CET 2015
Hello Simon,
On 03/06/2015 03:11 PM, Simon Glass wrote:
> Hi Przemyslaw,
>
> On 3 March 2015 at 09:24, Przemyslaw Marczak <p.marczak at 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 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
>> ---
>> 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 at 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 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>
>>
>> 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.?
>
This was solved partially in V3, in which I added the common function to
bind childs by checking compatible string in the property given by arg.
Looking at few dts files, we can found that some boards uses
"compatible", and some uses "regulator-compatible" for the compatible
property name of the regulators. I think, that this is not bad.
>> + * ...
>> + * }
>> + *
>> + * 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.
>
Right, this feature was not pretty, and is removed in V3.
>> +
>> +/**
>> + * 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?
>
Shame on me for missing this. The new declarations are fixed with the
'*buffer' and 'len'.
>> + 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.
>
After rework, the V3 doesn't use this.
>> +
>> +/**
>> + * 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
>
Thank you for comments.
Best regards,
--
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com
More information about the U-Boot
mailing list