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

Przemyslaw Marczak p.marczak at samsung.com
Mon Oct 20 17:44:52 CEST 2014


Hello Simon,

I missed some of your comments.

On 10/11/2014 01:18 AM, Simon Glass wrote:
> Hi,
>
> On 10 October 2014 07:32, Przemyslaw Marczak <p.marczak at 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 at 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 at 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 at 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.
>

Ok, now I see.

>>
>>
>>>> +       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);
>

This could be an another interesting possible feature of dm.

>>
>>
>>>> +{
>>>> +       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 at samsung.com>
>>>> + *
>>>> + * Copyright (C) 2011 Samsung Electronics
>>>> + * Lukasz Majewski <l.majewski at 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 at samsung.com>
>>>> + *
>>>> + * Copyright (C) 2011 Samsung Electronics
>>>> + * Lukasz Majewski <l.majewski at 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 at samsung.com>
>>>> + *
>>>>     *  Copyright (C) 2011-2012 Samsung Electronics
>>>>     *  Lukasz Majewski <l.majewski at 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
>

Thank you again.
I'm going to check the i2c-working tree and maybe rebase the dm-pmic 
onto it.

Is it good idea?

Best Regards,
-- 
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com


More information about the U-Boot mailing list