Power(full) framework based on Driver Model

From: Przemyslaw Marczak <[email protected]>

The functions error's numbers are standarized - but the error messages are not.

The errors are often handled with unclear error messages, so why not use an errno standarized messages.

Advantages:
- This could decrease the binary size.

Quoted 10 lines

From: Przemyslaw Marczak <[email protected]>

The functions error's numbers are standarized - but the error messages are not.

The errors are often handled with unclear error messages, so why not use an errno standarized messages.

Advantages:
- This could decrease the binary size.

Having an array of string ptrs adds some extra space needs. Each str needs a ptr and that ptr needs relocation, 8 bytes on 32 bits

If you want to save space do this instead
static const char const errno_message[] =
"Success\0Operation not permitted\0No such file or directory" etc.
Then count "\0" to find the error msg.

Jocke

Hello Joakim,

Quoted 15 lines · Joakim Tjernlund

From: Przemyslaw Marczak <[email protected]>

Quoted 10 lines

The functions error's numbers are standarized - but the error messages are not.

The errors are often handled with unclear error messages, so why not use an errno standarized messages.

Advantages:
- This could decrease the binary size.

Having an array of string ptrs adds some extra space needs. Each str needs a ptr and that ptr needs relocation, 8 bytes on 32 bits

Quoted 8 lines

If you want to save space do this instead
static const char const errno_message[] =
"Success\0Operation not permitted\0No such file or directory" etc.
Then count "\0" to find the error msg.

Jocke

Is this really a problem to add some array with the pointers?

You are right, this array requires some additional space, but this is not the main reason of introducing this function. This can be enabled optional, so maybe for the less memory and slower devices this shouldn't be used - but in the other way, we see many text messages in the code that could be replaced with the one from that array.
So, which is better?

This helps me sometimes, so I added this as some extra feature.

Hello Joakim,

Quoted 19 lines · Joakim Tjernlund

Quoted 10 lines

From: Przemyslaw Marczak <[email protected]>

The functions error's numbers are standarized - but the error messages are not.

The errors are often handled with unclear error messages, so why not use an errno standarized messages.

Advantages:
- This could decrease the binary size.

Having an array of string ptrs adds some extra space needs. Each str needs a ptr and that ptr needs relocation, 8 bytes on 32 bits

If you want to save space do this instead
static const char const errno_message[] =
"Success\0Operation not permitted\0No such file or directory" etc.
Then count "\0" to find the error msg.

Quoted 2 lines

Jocke

Is this really a problem to add some array with the pointers?

You are right, this array requires some additional space, but this is not the main reason of introducing this function. This can be enabled optional, so maybe for the less memory and slower devices this shouldn't

be used - but in the other way, we see many text messages in the code that could be replaced with the one from that array.
So, which is better?

This helps me sometimes, so I added this as some extra feature.

Przemyslaw Marczak <[email protected]> wrote on 2014/10/09 18:23:54:

Quoted 26 lines

Hello Joakim,

Quoted 21 lines · Joakim Tjernlund

Quoted 10 lines

From: Przemyslaw Marczak <[email protected]>

The functions error's numbers are standarized - but the error messages are not.

The errors are often handled with unclear error messages, so why not use an errno standarized messages.

Advantages:
- This could decrease the binary size.

Having an array of string ptrs adds some extra space needs. Each str needs a ptr and that ptr needs relocation, 8 bytes on 32 bits

If you want to save space do this instead
static const char const errno_message[] =
"Success\0Operation not permitted\0No such file or directory" etc.
Then count "\0" to find the error msg.

Jocke

Quoted 1 line

Is this really a problem to add some array with the pointers?

Probably not, I only mentioned this because you claimed it could reduces size
as if that was important to you.

Jocke

Quoted 4 lines

You are right, this array requires some additional space, but this is not the main reason of introducing this function. This can be enabled optional, so maybe for the less memory and slower devices this shouldn't

Quoted 5 lines

be used - but in the other way, we see many text messages in the code that could be replaced with the one from that array.
So, which is better?

This helps me sometimes, so I added this as some extra feature.

The functions error's numbers are standarized - but the error messages are not.

The errors are often handled with unclear error messages, so why not use an errno standarized messages.

Advantages:
- This could decrease the binary size.
- Appended with a detailed information,
the error message will be clear.

This commit introduces new function:
- const char *errno_to_str(int errno)

The functions returns a pointer to the errno corresponding text message:
- if errno is null or positive number - a pointer to "Success" message
- if errno is negative - a pointer to errno related message

Signed-off-by Przemyslaw Marczak <[email protected]>

Quoted 18 lines · Przemyslaw Marczak

The functions error's numbers are standarized - but the error messages are not.

The errors are often handled with unclear error messages, so why not use an errno standarized messages.

Advantages:
- This could decrease the binary size.
- Appended with a detailed information,
the error message will be clear.

This commit introduces new function:
- const char *errno_to_str(int errno)

The functions returns a pointer to the errno corresponding text message:
- if errno is null or positive number - a pointer to "Success" message
- if errno is negative - a pointer to errno related message

Quoted 1 line

Signed-off-by Przemyslaw Marczak <[email protected]>

In and of itself,

Reviewed-by Tom Rini <[email protected]>

Quoted 22 lines · Tom Rini

Quoted 19 lines · Przemyslaw Marczak

The functions error's numbers are standarized - but the error messages are not.

The errors are often handled with unclear error messages, so why not use an errno standarized messages.

Advantages:
- This could decrease the binary size.
- Appended with a detailed information,
the error message will be clear.

This commit introduces new function:
- const char *errno_to_str(int errno)

The functions returns a pointer to the errno corresponding text message:
- if errno is null or positive number - a pointer to "Success" message
- if errno is negative - a pointer to errno related message

Signed-off-by Przemyslaw Marczak <[email protected]>

Quoted 3 lines

In and of itself,

Reviewed-by Tom Rini <[email protected]>

Applied to u-boot-dm, thanks!

- Simon

Hello Simon,

Quoted 31 lines · Simon Glass

Quoted 27 lines · Tom Rini

Quoted 11 lines · Przemyslaw Marczak

The functions error's numbers are standarized - but the error messages are not.

The errors are often handled with unclear error messages, so why not use an errno standarized messages.

Advantages:
- This could decrease the binary size.
- Appended with a detailed information,
the error message will be clear.

Quoted 8 lines

This commit introduces new function:
- const char *errno_to_str(int errno)

The functions returns a pointer to the errno corresponding text message:
- if errno is null or positive number - a pointer to "Success" message
- if errno is negative - a pointer to errno related message

Signed-off-by Przemyslaw Marczak <[email protected]>

In and of itself,

Reviewed-by Tom Rini <[email protected]>

Quoted 4 lines

Applied to u-boot-dm, thanks!

- Simon

Thank, maybe today or tomorrow I will send the I2C for exynos and next I can continue work on pmic.

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 <[email protected]>

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

drivers/power/Makefile +3 -0

index dc64e4d..8def501 100644
@@ -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

drivers/power/pmic-uclass.c new file +45 -0

new file mode 100644
index 0000000..5e8494b
@@ -0,0 +1,255 @@
+/*
+ * Copyright (C) 2014 Samsung Electronics
+ * Przemyslaw Marczak <[email protected]>
+ *
+ * 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

drivers/power/pmic-uclass.c new file +54 -0

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

drivers/power/pmic-uclass.c new file +74 -0

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

drivers/power/pmic-uclass.c new file +52 -0

+{
+       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",
+};

drivers/power/pmic_i2c.c new file +136 -0

new file mode 100644
index 0000000..350d375
@@ -0,0 +1,136 @@
+/*
+ * Copyright (C) 2014 Samsung Electronics
+ * Przemyslaw Marczak <[email protected]>
+ *
+ * Copyright (C) 2011 Samsung Electronics
+ * Lukasz Majewski <[email protected]>
+ *
+ * (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;
+}

drivers/power/pmic_spi.c new file +137 -0

new file mode 100644
index 0000000..7851adf
@@ -0,0 +1,137 @@
+/*
+ * Copyright (C) 2014 Samsung Electronics
+ * Przemyslaw Marczak <[email protected]>
+ *
+ * Copyright (C) 2011 Samsung Electronics
+ * Lukasz Majewski <[email protected]>
+ *
+ * (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;
+}

include/dm/uclass-id.h +3 -0

index e3e9296..e6d9d39 100644
@@ -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,
 };

include/power/pmic.h +17 -0

index afbc5aa..7114650 100644
@@ -1,4 +1,7 @@
 /*
+ *  Copyright (C) 2014 Samsung Electronics
+ *  Przemyslaw Marczak <[email protected]>
+ *
  *  Copyright (C) 2011-2012 Samsung Electronics
  *  Lukasz Majewski <[email protected]>
  *

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

include/power/pmic.h +36 -0

+};
+
+/* 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);

include/power/pmic.h +1 -1

+#endif
 
 #define pmic_i2c_addr (p->hw.i2c.addr)
 #define pmic_i2c_tx_num (p->hw.i2c.tx_num)
--

1.9.1

Hi,

Quoted 97 lines · Przemyslaw Marczak

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 <[email protected]>

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

drivers/power/Makefile +3 -0

index dc64e4d..8def501 100644
@@ -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

drivers/power/pmic-uclass.c new file +45 -0

new file mode 100644
index 0000000..5e8494b
@@ -0,0 +1,255 @@
+/*
+ * Copyright (C) 2014 Samsung Electronics
+ * Przemyslaw Marczak <[email protected]>
+ *
+ * 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

drivers/power/pmic-uclass.c line 55

Quoted 10 lines

drivers/power/pmic-uclass.c new file +10 -0

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

Quoted 59 lines

drivers/power/pmic-uclass.c new file +59 -0

+       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:

drivers/power/pmic-uclass.c line 119

Quoted 5 lines

drivers/power/pmic-uclass.c new file +5 -0

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

Quoted 81 lines

drivers/power/pmic-uclass.c new file +81 -0

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

drivers/power/pmic-uclass.c line 203

Quoted 3 lines

drivers/power/pmic-uclass.c new file +3 -0

+}
+
+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.

Quoted 391 lines

drivers/power/pmic-uclass.c new file +52 -0

+{
+       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",
+};

drivers/power/pmic_i2c.c new file +136 -0

new file mode 100644
index 0000000..350d375
@@ -0,0 +1,136 @@
+/*
+ * Copyright (C) 2014 Samsung Electronics
+ * Przemyslaw Marczak <[email protected]>
+ *
+ * Copyright (C) 2011 Samsung Electronics
+ * Lukasz Majewski <[email protected]>
+ *
+ * (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;
+}

drivers/power/pmic_spi.c new file +137 -0

new file mode 100644
index 0000000..7851adf
@@ -0,0 +1,137 @@
+/*
+ * Copyright (C) 2014 Samsung Electronics
+ * Przemyslaw Marczak <[email protected]>
+ *
+ * Copyright (C) 2011 Samsung Electronics
+ * Lukasz Majewski <[email protected]>
+ *
+ * (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;
+}

include/dm/uclass-id.h +3 -0

index e3e9296..e6d9d39 100644
@@ -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,
  };

include/power/pmic.h +17 -0

index afbc5aa..7114650 100644
@@ -1,4 +1,7 @@
  /*
+ *  Copyright (C) 2014 Samsung Electronics
+ *  Przemyslaw Marczak <[email protected]>
+ *
   *  Copyright (C) 2011-2012 Samsung Electronics
   *  Lukasz Majewski <[email protected]>
   *

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

include/power/pmic.h struct pmic { · line 107

Quoted 10 lines

include/power/pmic.h +10 -0

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

Quoted 37 lines

include/power/pmic.h +36 -0

+};
+
+/* 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);

include/power/pmic.h int pmic_probe(struct pmic *p); · line 153

Quoted 6 lines

include/power/pmic.h +0 -0

  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.

Quoted 7 lines

include/power/pmic.h +1 -1

+#endif
 
  #define pmic_i2c_addr (p->hw.i2c.addr)
  #define pmic_i2c_tx_num (p->hw.i2c.tx_num)
--

1.9.1

Regards,
Simon

Hello Simon,

Quoted 121 lines · Simon Glass

Hi,

Quoted 100 lines · Przemyslaw Marczak

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 <[email protected]>

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

drivers/power/Makefile +3 -0

index dc64e4d..8def501 100644
@@ -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

drivers/power/pmic-uclass.c new file +27 -0

new file mode 100644
index 0000000..5e8494b
@@ -0,0 +1,255 @@
+/*
+ * Copyright (C) 2014 Samsung Electronics
+ * Przemyslaw Marczak <[email protected]>
+ *
+ * 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",

drivers/power/pmic-uclass.c new file +18 -0

+                      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

drivers/power/pmic-uclass.c line 55

Quoted 10 lines

drivers/power/pmic-uclass.c new file +10 -0

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

Quoted 3 lines

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?

Quoted 63 lines

Quoted 63 lines

drivers/power/pmic-uclass.c new file +61 -0

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

Quoted 9 lines

Quoted 4 lines

drivers/power/pmic-uclass.c new file +3 -0

+}
+
+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.

Quoted 84 lines

Quoted 84 lines

drivers/power/pmic-uclass.c new file +82 -0

+{
+       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;
+}

Quoted 9 lines

drivers/power/pmic-uclass.c

Quoted 2 lines

drivers/power/pmic-uclass.c new file +2 -0

+
+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?

Quoted 454 lines

Quoted 403 lines

drivers/power/pmic-uclass.c new file +23 -0

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

drivers/power/pmic-uclass.c new file +29 -0

+               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",
+};

drivers/power/pmic_i2c.c new file +63 -0

new file mode 100644
index 0000000..350d375
@@ -0,0 +1,136 @@
+/*
+ * Copyright (C) 2014 Samsung Electronics
+ * Przemyslaw Marczak <[email protected]>
+ *
+ * Copyright (C) 2011 Samsung Electronics
+ * Lukasz Majewski <[email protected]>
+ *
+ * (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);

drivers/power/pmic_i2c.c new file +48 -0

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

drivers/power/pmic_i2c.c new file +25 -0

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

drivers/power/pmic_spi.c new file +49 -0

new file mode 100644
index 0000000..7851adf
@@ -0,0 +1,137 @@
+/*
+ * Copyright (C) 2014 Samsung Electronics
+ * Przemyslaw Marczak <[email protected]>
+ *
+ * Copyright (C) 2011 Samsung Electronics
+ * Lukasz Majewski <[email protected]>
+ *
+ * (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,

drivers/power/pmic_spi.c new file +88 -0

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

include/dm/uclass-id.h +3 -0

index e3e9296..e6d9d39 100644
@@ -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,
  };

include/power/pmic.h +8 -0

index afbc5aa..7114650 100644
@@ -1,4 +1,7 @@
  /*
+ *  Copyright (C) 2014 Samsung Electronics
+ *  Przemyslaw Marczak <[email protected]>
+ *
   *  Copyright (C) 2011-2012 Samsung Electronics
   *  Lukasz Majewski <[email protected]>
   *

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

include/power/pmic.h struct pmic { · line 107

Quoted 7 lines

include/power/pmic.h +7 -0

+*/
+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.

Quoted 39 lines

include/power/pmic.h +18 -0

+};
+
+/* 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);

include/power/pmic.h +18 -0

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

Quoted 9 lines

include/power/pmic.h int pmic_probe(struct pmic *p); · line 153

Quoted 5 lines

  int pmic_dialog_init(unsigned char bus);

include/power/pmic.h +0 -0

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

Quoted 8 lines

Quoted 7 lines

include/power/pmic.h +1 -1

+#endif
 
  #define pmic_i2c_addr (p->hw.i2c.addr)
  #define pmic_i2c_tx_num (p->hw.i2c.tx_num)
--

1.9.1

Quoted 3 lines

Regards,
Simon

Thank you for the fast review :)

Hi,

Quoted 129 lines · Przemyslaw Marczak

Hello Simon,

Quoted 121 lines · Simon Glass

Hi,

Quoted 100 lines · Przemyslaw Marczak

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 <[email protected]>

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

drivers/power/Makefile +3 -0

index dc64e4d..8def501 100644
@@ -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

drivers/power/pmic-uclass.c new file +27 -0

new file mode 100644
index 0000000..5e8494b
@@ -0,0 +1,255 @@
+/*
+ * Copyright (C) 2014 Samsung Electronics
+ * Przemyslaw Marczak <[email protected]>
+ *
+ * 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",

drivers/power/pmic-uclass.c new file +18 -0

+                      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

drivers/power/pmic-uclass.c line 55

Quoted 10 lines

drivers/power/pmic-uclass.c new file +10 -0

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

Quoted 3 lines

For now this seems fine though. I think the 'return -ENOSYS' can just go at the end of the function and appear once.

Quoted 8 lines

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.

Quoted 72 lines

Quoted 63 lines

Quoted 63 lines

drivers/power/pmic-uclass.c new file +61 -0

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

Quoted 9 lines

Quoted 4 lines

drivers/power/pmic-uclass.c new file +3 -0

+}
+
+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.

Quoted 3 lines

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

Quoted 93 lines

Quoted 84 lines

Quoted 84 lines

drivers/power/pmic-uclass.c new file +82 -0

+{
+       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;
+}

Quoted 9 lines

drivers/power/pmic-uclass.c

Quoted 2 lines

drivers/power/pmic-uclass.c new file +2 -0

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

Quoted 7 lines

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.

Quoted 476 lines

Quoted 454 lines

Quoted 403 lines

drivers/power/pmic-uclass.c new file +23 -0

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

drivers/power/pmic-uclass.c new file +29 -0

+               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",
+};

drivers/power/pmic_i2c.c new file +63 -0

new file mode 100644
index 0000000..350d375
@@ -0,0 +1,136 @@
+/*
+ * Copyright (C) 2014 Samsung Electronics
+ * Przemyslaw Marczak <[email protected]>
+ *
+ * Copyright (C) 2011 Samsung Electronics
+ * Lukasz Majewski <[email protected]>
+ *
+ * (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);

drivers/power/pmic_i2c.c new file +48 -0

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

drivers/power/pmic_i2c.c new file +25 -0

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

drivers/power/pmic_spi.c new file +49 -0

new file mode 100644
index 0000000..7851adf
@@ -0,0 +1,137 @@
+/*
+ * Copyright (C) 2014 Samsung Electronics
+ * Przemyslaw Marczak <[email protected]>
+ *
+ * Copyright (C) 2011 Samsung Electronics
+ * Lukasz Majewski <[email protected]>
+ *
+ * (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,

drivers/power/pmic_spi.c new file +88 -0

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

include/dm/uclass-id.h +3 -0

index e3e9296..e6d9d39 100644
@@ -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,
   };

include/power/pmic.h +8 -0

index afbc5aa..7114650 100644
@@ -1,4 +1,7 @@
   /*
+ *  Copyright (C) 2014 Samsung Electronics
+ *  Przemyslaw Marczak <[email protected]>
+ *
    *  Copyright (C) 2011-2012 Samsung Electronics
    *  Lukasz Majewski <[email protected]>
    *

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

include/power/pmic.h struct pmic { · line 107

Quoted 7 lines

include/power/pmic.h +7 -0

+*/
+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.

Quoted 39 lines

include/power/pmic.h +18 -0

+};
+
+/* 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);

include/power/pmic.h +18 -0

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

Quoted 9 lines

include/power/pmic.h int pmic_probe(struct pmic *p); · line 153

Quoted 5 lines

   int pmic_dialog_init(unsigned char bus);

include/power/pmic.h +0 -0

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

Quoted 11 lines

Quoted 7 lines

include/power/pmic.h +1 -1

+#endif
 
   #define pmic_i2c_addr (p->hw.i2c.addr)
   #define pmic_i2c_tx_num (p->hw.i2c.tx_num)
--

1.9.1

Regards,
Simon