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

Simon Glass
2014-10-10T23:18:42Z

Hi,

On 10 October 2014 07:32, Przemyslaw Marczak <p.marczak@samsung.com> wrote:
> Hello Simon,
>
>
> On 10/10/2014 05:17 AM, Simon Glass wrote:
>>
>> Hi,
>>
>> On 8 October 2014 14:48, Przemyslaw Marczak <p.marczak@samsung.com> wrote:
>>>
>>> This is an introduction to driver-model multi class PMIC support.
>>> It starts with UCLASS_PMIC - a common PMIC class type for I/O, which
>>> doesn't need to implement any specific operations and features beside
>>> the platform data, which is the 'struct pmic_platdata' defined in file:
>>> - 'include/power/pmic.h'
>>>
>>> New files:
>>> - pmic-uclass.c - provides a common code for UCLASS_PMIC drivers
>>> - pmic_i2c.c - provides dm interface for i2c pmic drivers
>>> - pmic_spi.c - provides dm interface for spi pmic drivers
>>>
>>> Those files are based on a current PMIC framework files and code.
>>> The new files are introduced to keep code readability and allow to
>>> make an easy drivers migration. The old pmic framework is still kept
>>> and full independent.
>>>
>>> Changes:
>>> - new uclass-id: UCLASS_PMIC,
>>> - new configs: CONFIG_DM_PMIC; CONFIG_DM_PMIC_SPI; CONFIG_DM_PMIC_I2C,
>>>
>>> New pmic api is documented in: doc/README.power-framework-dm
>>>
>>> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
>>> ---
>>> drivers/power/Makefile | 3 +
>>> drivers/power/pmic-uclass.c | 255
>>> ++++++++++++++++++++++++++++++++++++++++++++
>>> drivers/power/pmic_i2c.c | 136 +++++++++++++++++++++++
>>> drivers/power/pmic_spi.c | 137 ++++++++++++++++++++++++
>>> include/dm/uclass-id.h | 3 +
>>> include/power/pmic.h | 64 +++++++++++
>>> 6 files changed, 598 insertions(+)
>>> create mode 100644 drivers/power/pmic-uclass.c
>>> create mode 100644 drivers/power/pmic_i2c.c
>>> create mode 100644 drivers/power/pmic_spi.c
>>>
>>> diff --git a/drivers/power/Makefile b/drivers/power/Makefile
>>> index dc64e4d..8def501 100644
>>> --- a/drivers/power/Makefile
>>> +++ b/drivers/power/Makefile
>>> @@ -19,3 +19,6 @@ 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_SPI) += pmic_spi.o
>>> +obj-$(CONFIG_DM_PMIC_I2C) += pmic_i2c.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..5e8494b
>>> --- /dev/null
>>> +++ b/drivers/power/pmic-uclass.c
>>> @@ -0,0 +1,255 @@
>>> +/*
>>> + * Copyright (C) 2014 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 <power/pmic.h>
>>> +#include <dm/device-internal.h>
>>> +#include <dm/uclass-internal.h>
>>> +#include <dm/root.h>
>>> +#include <dm/lists.h>
>>> +#include <i2c.h>
>>> +#include <compiler.h>
>>> +#include <errno.h>
>>> +
>>> +DECLARE_GLOBAL_DATA_PTR;
>>> +
>>> +int pmic_check_reg(struct pmic_platdata *p, unsigned reg)
>>> +{
>>> + if (!p)
>>> + return -ENODEV;
>>> +
>>> + if (reg >= p->regs_num) {
>>> + error("<reg num> = %d is invalid. Should be less than
>>> %d",
>>> + reg, p->regs_num);
>>> + return -EINVAL;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +int pmic_reg_write(struct udevice *dev, unsigned reg, unsigned val)
>>> +{
>>> + struct pmic_platdata *p;
>>> +
>>> + p = dev_get_platdata(dev);
>>> + if (!p)
>>> + return -EPERM;
>>> +
>>> + switch (p->interface) {
>>> + case PMIC_I2C:
>>> +#ifdef CONFIG_DM_PMIC_I2C
>>> + return pmic_i2c_reg_write(dev, reg, val);
>>> +#else
>>> + return -ENOSYS;
>>> +#endif
>>> + case PMIC_SPI:
>>> +#ifdef CONFIG_DM_PMIC_SPI
>>> + return pmic_spi_reg_write(dev, reg, val);
>>> +#else
>>> + return -ENOSYS;
>>> +#endif
>>
>>
>> Perhaps one day we should add another uclass which is some sort of
>> register cache. It could be implemented by I2C and SPI drivers (or
>> better perhaps the I2C and SPI uclasses could provide a register cache
>> uclass device for their main when requested).
>>
>> For now this seems fine though. I think the 'return -ENOSYS' can just
>> go at the end of the function and appear once.
>>
>
> Why do we need the registers cache?
> This maybe is good for a many read operations at a time in the system,
> but actually I think, that it is not required for the bootloader purposes.
>
> If we write some data - sequential, then we probably would like to check it
> in the same time, e.g. update some range of I2C registers.
>
> I don't know the Chromebook design, but how it could be useful for you?

We don't need a register cache - I was just thinking of that from the
kernel. But it feels like an interface like:

int reg_read(int reg, uint32_t *value)
int reg_write(int reg, uint32_t value)
int reg_write_range(int reg_start, int reg_count, uint32_t value[])

might be useful - it could be implemented for both I2C and SPI.

>
>
>>> + default:
>>> + return -ENODEV;
>>> + }
>>> +}
>>> +
>>> +int pmic_reg_read(struct udevice *dev, unsigned reg, unsigned *val)
>>> +{
>>> + struct pmic_platdata *p;
>>> +
>>> + p = dev_get_platdata(dev);
>>> + if (!p)
>>> + return -EPERM;
>>> +
>>> + switch (p->interface) {
>>> + case PMIC_I2C:
>>> +#ifdef CONFIG_DM_PMIC_I2C
>>> + return pmic_i2c_reg_read(dev, reg, val);
>>> +#else
>>> + return -ENOSYS;
>>> +#endif
>>> + case PMIC_SPI:
>>> +#ifdef CONFIG_DM_PMIC_SPI
>>> + return pmic_spi_reg_read(dev, reg, val);
>>> +#else
>>> + return -ENOSYS;
>>> +#endif
>>> + default:
>>> + return -ENODEV;
>>> + }
>>> +}
>>> +
>>> +int pmic_probe(struct udevice *dev, struct spi_slave *spi_slave)
>>> +{
>>> + struct pmic_platdata *p;
>>> +
>>> + p = dev_get_platdata(dev);
>>> + if (!p)
>>> + return -EPERM;
>>> +
>>> + switch (p->interface) {
>>> + case PMIC_I2C:
>>> +#ifdef CONFIG_DM_PMIC_I2C
>>> + return pmic_i2c_probe(dev);
>>> +#else
>>> + return -ENOSYS;
>>> +#endif
>>> + case PMIC_SPI:
>>> + if (!spi_slave)
>>> + return -EINVAL;
>>> +#ifdef CONFIG_DM_PMIC_SPI
>>> + spi_slave = pmic_spi_probe(dev);
>>> + if (!spi_slave)
>>> + return -EIO;
>>> +
>>> + return 0;
>>> +#else
>>> + return -ENOSYS;
>>> +#endif
>>> + default:
>>> + return -ENODEV;
>>> + }
>>> +}
>>> +
>>> +struct udevice *pmic_get_by_interface(int uclass_id, int bus, int
>>> addr_cs)
>>
>>
>> This is an interesting function! Again we can probably improve things
>> when we have an i2c uclass.
>>
> Thanks! But even if we have i2c uclass, then we also should know how to
> recognize each device, e.g. in the board code. So the interface and address
> will probably no change.

I'm thinking that one day you could do something like:

// Find the regmap associated with the device
reg_dev = device_get_child(UCLASS_REGMAP, dev);

// Call the regmap uclass to access registers whether it is I2C or SPI
regmap_write(reg_dev, value);

>
>
>>> +{
>>> + struct pmic_platdata *pl;
>>> + struct udevice *dev;
>>> + struct uclass *uc;
>>> + int ret;
>>> +
>>> + if (uclass_id < 0 || uclass_id >= UCLASS_COUNT) {
>>> + error("Bad uclass id.\n");
>>> + return NULL;
>>> + }
>>> +
>>> + ret = uclass_get(uclass_id, &uc);
>>> + if (ret) {
>>> + error("PMIC uclass: %d not initialized!\n", uclass_id);
>>> + return NULL;
>>> + }
>>> +
>>> + uclass_foreach_dev(dev, uc) {
>>> + if (!dev || !dev->platdata)
>>> + continue;
>>> +
>>> + pl = dev_get_platdata(dev);
>>> +
>>> + if (pl->bus != bus)
>>> + continue;
>>> +
>>> + switch (pl->interface) {
>>> + case PMIC_I2C:
>>> + if (addr_cs != pl->hw.i2c.addr)
>>> + continue;
>>> + break;
>>> + case PMIC_SPI:
>>> + if (addr_cs != pl->hw.spi.cs)
>>> + continue;
>>> + break;
>>> + default:
>>> + error("Unsupported interface of: %s", dev->name);
>>> + return NULL;
>>> + }
>>> +
>>> + ret = device_probe(dev);
>>> + if (ret) {
>>> + error("Dev: %s probe failed", dev->name);
>>> + return NULL;
>>> + }
>>> + return dev;
>>> + }
>>> +
>>> + return NULL;
>>> +}
>>> +
>>> +struct udevice *pmic_get_by_name(int uclass_id, char *name)
>>> +{
>>> + struct udevice *dev;
>>> + struct uclass *uc;
>>> + int ret;
>>> +
>>> + if (uclass_id < 0 || uclass_id >= UCLASS_COUNT) {
>>> + error("Bad uclass id.\n");
>>> + return NULL;
>>> + }
>>> +
>>> + ret = uclass_get(uclass_id, &uc);
>>> + if (ret) {
>>> + error("PMIC uclass: %d not initialized!", uclass_id);
>>> + return NULL;
>>> + }
>>> +
>>> + 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);
>>> + return dev;
>>> + }
>>> + }
>>> +
>>> + return NULL;
>>> +}
>>> +
>>> +int pmic_init_dm(void)
>>
>>
>> I don't understand why you need this function at all. Driver model
>> should find the pmics automatically. Is it because we don't have an
>> i2c uclass? In that case I think it would be better to limit this hack
>> to I2C. For SPI it should work correctly without this function.
>>
> Currently, it shouldn't and yes - it isn't because of the I2C uclass
> missing. So no one check the I2C nodes - and its child subnodes.
> Actually, this can be easy fixed - just don't add the "pmic" alias to the
> dts. But I will also change fix the code.
>
> And by the way - is there any check in the code, which protects for the
> device bind more than once?

No there is no such check at present.

>
>
>>> +{
>>> + const void *blob = gd->fdt_blob;
>>> + const struct fdt_property *prop;
>>> + struct udevice *dev = NULL;
>>> + const char *path;
>>> + const char *alias;
>>> + int alias_node, node, offset, ret = 0;
>>> + int alias_len;
>>> + int len;
>>> +
>>> + alias = "pmic";
>>> + alias_len = strlen(alias);
>>> +
>>> + alias_node = fdt_path_offset(blob, "/aliases");
>>> + offset = fdt_first_property_offset(blob, alias_node);
>>> +
>>> + if (offset < 0) {
>>> + error("Alias node not found.");
>>> + return -ENODEV;
>>> + }
>>> +
>>> + offset = fdt_first_property_offset(blob, alias_node);
>>> + for (; offset > 0; offset = fdt_next_property_offset(blob,
>>> offset)) {
>>> + prop = fdt_get_property_by_offset(blob, offset, &len);
>>> + if (!len)
>>> + continue;
>>> +
>>> + path = fdt_string(blob, fdt32_to_cpu(prop->nameoff));
>>> +
>>> + if (!strncmp(alias, path, alias_len))
>>> + node = fdt_path_offset(blob, prop->data);
>>> + else
>>> + node = 0;
>>> +
>>> + if (node <= 0)
>>> + continue;
>>> +
>>> + ret = lists_bind_fdt(gd->dm_root, blob, node, &dev);
>>> + if (ret < 0)
>>> + continue;
>>> +
>>> + if (device_probe(dev))
>>> + error("Device: %s, probe error", dev->name);
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +UCLASS_DRIVER(pmic) = {
>>> + .id = UCLASS_PMIC,
>>> + .name = "pmic",
>>> +};
>>> diff --git a/drivers/power/pmic_i2c.c b/drivers/power/pmic_i2c.c
>>> new file mode 100644
>>> index 0000000..350d375
>>> --- /dev/null
>>> +++ b/drivers/power/pmic_i2c.c
>>> @@ -0,0 +1,136 @@
>>> +/*
>>> + * Copyright (C) 2014 Samsung Electronics
>>> + * Przemyslaw Marczak <p.marczak@samsung.com>
>>> + *
>>> + * Copyright (C) 2011 Samsung Electronics
>>> + * Lukasz Majewski <l.majewski@samsung.com>
>>> + *
>>> + * (C) Copyright 2010
>>> + * Stefano Babic, DENX Software Engineering, sbabic at denx.de
>>> + *
>>> + * (C) Copyright 2008-2009 Freescale Semiconductor, Inc.
>>> + *
>>> + * SPDX-License-Identifier: GPL-2.0+
>>> + */
>>> +
>>> +#include <common.h>
>>> +#include <linux/types.h>
>>> +#include <power/pmic.h>
>>> +#include <errno.h>
>>> +#include <dm.h>
>>> +#include <i2c.h>
>>> +
>>> +int pmic_i2c_reg_write(struct udevice *dev, unsigned reg, unsigned val)
>>> +{
>>> + struct pmic_platdata *p;
>>> + unsigned char buf[4] = { 0 };
>>> +
>>> + if (!dev)
>>> + return -ENODEV;
>>> +
>>> + p = dev->platdata;
>>> +
>>> + if (pmic_check_reg(p, reg))
>>> + return -EINVAL;
>>> +
>>> + I2C_SET_BUS(p->bus);
>>> +
>>> + switch (pmic_i2c_tx_num) {
>>> + case 3:
>>> + if (p->byte_order == PMIC_SENSOR_BYTE_ORDER_BIG) {
>>> + buf[2] = (cpu_to_le32(val) >> 16) & 0xff;
>>> + buf[1] = (cpu_to_le32(val) >> 8) & 0xff;
>>> + buf[0] = cpu_to_le32(val) & 0xff;
>>> + } else {
>>> + buf[0] = (cpu_to_le32(val) >> 16) & 0xff;
>>> + buf[1] = (cpu_to_le32(val) >> 8) & 0xff;
>>> + buf[2] = cpu_to_le32(val) & 0xff;
>>> + }
>>> + break;
>>> + case 2:
>>> + if (p->byte_order == PMIC_SENSOR_BYTE_ORDER_BIG) {
>>> + buf[1] = (cpu_to_le32(val) >> 8) & 0xff;
>>> + buf[0] = cpu_to_le32(val) & 0xff;
>>> + } else {
>>> + buf[0] = (cpu_to_le32(val) >> 8) & 0xff;
>>> + buf[1] = cpu_to_le32(val) & 0xff;
>>> + }
>>> + break;
>>> + case 1:
>>> + buf[0] = cpu_to_le32(val) & 0xff;
>>> + break;
>>> + default:
>>> + printf("%s: invalid tx_num: %d", __func__,
>>> pmic_i2c_tx_num);
>>> + return -1;
>>> + }
>>> +
>>> + if (i2c_write(pmic_i2c_addr, reg, 1, buf, pmic_i2c_tx_num))
>>> + return -1;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +int pmic_i2c_reg_read(struct udevice *dev, unsigned reg, unsigned *val)
>>> +{
>>> + struct pmic_platdata *p;
>>> + unsigned char buf[4] = { 0 };
>>> + u32 ret_val = 0;
>>> +
>>> + if (!dev)
>>> + return -ENODEV;
>>> +
>>> + p = dev->platdata;
>>> +
>>> + if (pmic_check_reg(p, reg))
>>> + return -EINVAL;
>>> +
>>> + I2C_SET_BUS(p->bus);
>>> +
>>> + if (i2c_read(pmic_i2c_addr, reg, 1, buf, pmic_i2c_tx_num))
>>> + return -1;
>>> +
>>> + switch (pmic_i2c_tx_num) {
>>> + case 3:
>>> + if (p->byte_order == PMIC_SENSOR_BYTE_ORDER_BIG)
>>> + ret_val = le32_to_cpu(buf[2] << 16
>>> + | buf[1] << 8 | buf[0]);
>>> + else
>>> + ret_val = le32_to_cpu(buf[0] << 16 |
>>> + buf[1] << 8 | buf[2]);
>>> + break;
>>> + case 2:
>>> + if (p->byte_order == PMIC_SENSOR_BYTE_ORDER_BIG)
>>> + ret_val = le32_to_cpu(buf[1] << 8 | buf[0]);
>>> + else
>>> + ret_val = le32_to_cpu(buf[0] << 8 | buf[1]);
>>> + break;
>>> + case 1:
>>> + ret_val = le32_to_cpu(buf[0]);
>>> + break;
>>> + default:
>>> + printf("%s: invalid tx_num: %d", __func__,
>>> pmic_i2c_tx_num);
>>> + return -1;
>>> + }
>>> + memcpy(val, &ret_val, sizeof(ret_val));
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +int pmic_i2c_probe(struct udevice *dev)
>>> +{
>>> + struct pmic_platdata *p;
>>> +
>>> + if (!dev)
>>> + return -ENODEV;
>>> +
>>> + p = dev->platdata;
>>> +
>>> + i2c_set_bus_num(p->bus);
>>> + debug("Bus: %d PMIC:%s probed!\n", p->bus, dev->name);
>>> + if (i2c_probe(pmic_i2c_addr)) {
>>> + printf("Can't find PMIC:%s\n", dev->name);
>>> + return -1;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> diff --git a/drivers/power/pmic_spi.c b/drivers/power/pmic_spi.c
>>> new file mode 100644
>>> index 0000000..7851adf
>>> --- /dev/null
>>> +++ b/drivers/power/pmic_spi.c
>>> @@ -0,0 +1,137 @@
>>> +/*
>>> + * Copyright (C) 2014 Samsung Electronics
>>> + * Przemyslaw Marczak <p.marczak@samsung.com>
>>> + *
>>> + * Copyright (C) 2011 Samsung Electronics
>>> + * Lukasz Majewski <l.majewski@samsung.com>
>>> + *
>>> + * (C) Copyright 2010
>>> + * Stefano Babic, DENX Software Engineering, sbabic at denx.de
>>> + *
>>> + * (C) Copyright 2008-2009 Freescale Semiconductor, Inc.
>>> + *
>>> + * SPDX-License-Identifier: GPL-2.0+
>>> + */
>>> +
>>> +#include <common.h>
>>> +#include <linux/types.h>
>>> +#include <power/pmic.h>
>>> +#include <errno.h>
>>> +#include <dm.h>
>>> +#include <spi.h>
>>> +
>>> +static struct spi_slave *slave;
>>> +
>>> +void pmic_spi_free(struct spi_slave *slave)
>>> +{
>>> + if (slave)
>>> + spi_free_slave(slave);
>>> +}
>>> +
>>> +struct spi_slave *pmic_spi_probe(struct udevice *dev)
>>> +{
>>> + struct pmic_platdata *p;
>>> +
>>> + if (!dev)
>>> + return NULL;
>>> +
>>> + p = dev->platdata;
>>> +
>>> + if (pmic_check_reg(p, reg))
>>> + return NULL;
>>> +
>>> + return spi_setup_slave(p->bus,
>>> + p->hw.spi.cs,
>>> + p->hw.spi.clk,
>>> + p->hw.spi.mode);
>>> +}
>>> +
>>> +static int pmic_spi_reg(struct udevice *dev, unsigned reg, unsigned
>>> *val,
>>> + int write)
>>> +{
>>> + struct pmic_platdata *p;
>>> + u32 pmic_tx, pmic_rx;
>>> + u32 tmp;
>>> +
>>> + if (!dev)
>>> + return -EINVAL;
>>> +
>>> + p = dev->platdata;
>>> +
>>> + if (pmic_check_reg(p, reg))
>>> + return -EFAULT;
>>> +
>>> + if (!slave) {
>>> + slave = pmic_spi_probe(p);
>>> +
>>> + if (!slave)
>>> + return -ENODEV;
>>> + }
>>> +
>>> + if (pmic_check_reg(p, reg))
>>> + return -EFAULT;
>>> +
>>> + if (spi_claim_bus(slave))
>>> + return -EIO;
>>> +
>>> + pmic_tx = p->hw.spi.prepare_tx(reg, val, write);
>>> +
>>> + tmp = cpu_to_be32(pmic_tx);
>>> +
>>> + if (spi_xfer(slave, pmic_spi_bitlen, &tmp, &pmic_rx,
>>> + pmic_spi_flags)) {
>>> + spi_release_bus(slave);
>>> + return -EIO;
>>> + }
>>> +
>>> + if (write) {
>>> + pmic_tx = p->hw.spi.prepare_tx(reg, val, 0);
>>> + tmp = cpu_to_be32(pmic_tx);
>>> + if (spi_xfer(slave, pmic_spi_bitlen, &tmp, &pmic_rx,
>>> + pmic_spi_flags)) {
>>> + spi_release_bus(slave);
>>> + return -EIO;
>>> + }
>>> + }
>>> +
>>> + spi_release_bus(slave);
>>> + *val = cpu_to_be32(pmic_rx);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +int pmic_spi_reg_write(struct udevice *dev, unsigned reg, unsigned val)
>>> +{
>>> + struct pmic_platdata *p;
>>> +
>>> + if (!dev)
>>> + return -EINVAL;
>>> +
>>> + p = dev->platdata;
>>> +
>>> + if (pmic_check_reg(p, reg))
>>> + return -EFAULT;
>>> +
>>> + if (pmic_spi_reg(p, reg, &val, 1))
>>> + return -1;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +int pmic_spi_reg_read(struct udevice *dev, unsigned reg, unsigned *val)
>>> +{
>>> + struct pmic_platdata *p;
>>> +
>>> + if (!dev)
>>> + return -EINVAL;
>>> +
>>> + p = dev->platdata;
>>> +
>>> + if (pmic_check_reg(p, reg))
>>> + return -EFAULT;
>>> +
>>> + if (pmic_spi_reg(p, reg, val, 0))
>>> + return -1;
>>> +
>>> + return 0;
>>> +}
>>> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
>>> index e3e9296..e6d9d39 100644
>>> --- a/include/dm/uclass-id.h
>>> +++ b/include/dm/uclass-id.h
>>> @@ -29,6 +29,9 @@ enum uclass_id {
>>> UCLASS_SPI_FLASH, /* SPI flash */
>>> UCLASS_CROS_EC, /* Chrome OS EC */
>>>
>>> + /* PMIC uclass and PMIC related uclasses */
>>> + UCLASS_PMIC,
>>> +
>>> UCLASS_COUNT,
>>> UCLASS_INVALID = -1,
>>> };
>>> diff --git a/include/power/pmic.h b/include/power/pmic.h
>>> index afbc5aa..7114650 100644
>>> --- a/include/power/pmic.h
>>> +++ b/include/power/pmic.h
>>> @@ -1,4 +1,7 @@
>>> /*
>>> + * Copyright (C) 2014 Samsung Electronics
>>> + * Przemyslaw Marczak <p.marczak@samsung.com>
>>> + *
>>> * Copyright (C) 2011-2012 Samsung Electronics
>>> * Lukasz Majewski <l.majewski@samsung.com>
>>> *
>>> @@ -8,9 +11,12 @@
>>> #ifndef __CORE_PMIC_H_
>>> #define __CORE_PMIC_H_
>>>
>>> +#include <dm.h>
>>> #include <linux/list.h>
>>> +#include <spi.h>
>>> #include <i2c.h>
>>> #include <power/power_chrg.h>
>>> +#include <power/regulator.h>
>>>
>>> enum { PMIC_I2C, PMIC_SPI, PMIC_NONE};
>>> enum { I2C_PMIC, I2C_NUM, };
>>> @@ -78,6 +84,63 @@ struct pmic {
>>> struct list_head list;
>>> };
>>>
>>> +#ifdef CONFIG_DM_PMIC
>>> +/* struct pmic_platdata - a standard descriptor for pmic device, which
>>> holds
>>> + * an informations about interface. It is common for all pmic devices.
>>> + *
>>> + * Note:
>>> + * Interface fields are the same as in: struct pmic.
>>> + * Note: struct pmic will be removed in the future after drivers
>>> migration
>>> + *
>>> + * @bus - a physical bus on which device is connected
>>> + * @interface - an interface type, one of enum: PMIC_I2C, PMIC_SPI,
>>> PMIC_NONE
>>> + * @byte_order - one of enum: PMIC_SENSOR_BYTE_ORDER*_LITTLE/_BIG
>>> + * @regs_num - number of device registers
>>> + * @hw - one of union structure: p_i2c or p_spi
>>> + * based on @interface field
>>> +*/
>>> +struct pmic_platdata {
>>> + int bus;
>>> + int interface;
>>> + int byte_order;
>>> + int regs_num;
>>> + union hw hw;
>>
>>
>> If we have a 'register cache' uclass (later, once i2c is done) then
>> this could just be a device.
>>
>>> +};
>>> +
>>> +/* enum pmic_op_type - used for various pmic devices operation calls,
>>> + * for decrease a number of functions 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,
>>> +};
>>> +
>>> +/* drivers/power/pmic-uclass.c */
>>> +int power_init_dm(void);
>>> +struct udevice *pmic_get_by_name(int uclass_id, char *name);
>>> +struct udevice *pmic_get_by_interface(int uclass_id, int bus, int
>>> addr_cs);
>>> +const char *pmic_if_str(int interface);
>>> +int pmic_check_reg(struct pmic_platdata *p, unsigned reg);
>>> +int pmic_reg_write(struct udevice *dev, unsigned reg, unsigned val);
>>> +int pmic_reg_read(struct udevice *dev, unsigned reg, unsigned *val);
>>> +int pmic_probe(struct udevice *dev, struct spi_slave *spi_slave);
>>> +
>>> +/* drivers/power/pmic_i2c.c */
>>> +int pmic_i2c_reg_write(struct udevice *dev, unsigned reg, unsigned val);
>>> +int pmic_i2c_reg_read(struct udevice *dev, unsigned reg, unsigned *val);
>>> +int pmic_i2c_probe(struct udevice *dev);
>>> +
>>> +/* drivers/power/pmic_spi.c */
>>> +int pmic_spi_reg_write(struct udevice *dev, unsigned reg, unsigned val);
>>> +int pmic_spi_reg_read(struct udevice *dev, unsigned reg, unsigned *val);
>>> +struct spi_slave *pmic_spi_probe(struct udevice *dev);
>>> +#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 +151,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);
>>
>>
>> These should have comments.
>>
> Ok, I will add it.
>
>>> +#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 the fast review :)

Very interested to see this development.

Regards,
Simon