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.

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

+{

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

+{

•   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@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@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.

+#endif

#define pmic_i2c_addr (p->hw.i2c.addr)

#define pmic_i2c_tx_num (p->hw.i2c.tx_num)

1.9.1

Regards, Simon

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@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@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

Hi,

On 20 October 2014 09:44, Przemyslaw Marczak p.marczak@samsung.com wrote:

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@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.

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@samsung.com
• • Copyright (C) 2011 Samsung Electronics
• • Lukasz Majewski l.majewski@samsung.com
• • (C) Copyright 2010
• • Stefano Babic, DENX Software Engineering, sbabic@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@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

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

Is it good idea?