Hello Simon,

On 05/10/2015 03:56 PM, Simon Glass wrote:

Hi Przemyslaw,

On 8 May 2015 at 10:20, Przemyslaw Marczak p.marczak@samsung.com wrote:

...

The cleanup includes:

• pmic.h - fix mistakes in a few comments
• pmic operations: value 'reg_count' - redefine as function call
• fix function name: pmic_bind_childs() -> pmic_bind_children()
• pmic_bind_children: increment child_info pointer if operation in loop fail

Signed-off-by: Przemyslaw Marczak p.marczak@samsung.com

drivers/power/pmic/pmic-uclass.c | 25 +++++++++---------------- include/power/pmic.h | 39 ++++++++++++++++++++------------------- 2 files changed, 29 insertions(+), 35 deletions(-)

Acked-by: Simon Glass sjg@chromium.org Tested on sandbox: Tested-by: Simon Glass sjg@chromium.org

Thank you for review and testing!

BTW in pmic_bind_children() you have something like this:

info = child_info; while (info->prefix) { ... if (ret) { debug(" - child binding error: %d\n", ret); info++; continue; }

I think this would be better:

for (info = child_info; info->prefix, info++)

and remove the three info++ expressions inside the loop.

Ah, that's right, I will fix that.

...

diff --git a/drivers/power/pmic/pmic-uclass.c b/drivers/power/pmic/pmic-uclass.c index d82d3da..6766bd5 100644 --- a/drivers/power/pmic/pmic-uclass.c +++ b/drivers/power/pmic/pmic-uclass.c @@ -27,8 +27,8 @@ static ulong str_get_num(const char *ptr, const char *maxptr) return simple_strtoul(ptr, NULL, 0); }

-int pmic_bind_childs(struct udevice *pmic, int offset,

•                const struct pmic_child_info *child_info)
  

+int pmic_bind_children(struct udevice *pmic, int offset,

•                  const struct pmic_child_info *child_info)
  

  { const struct pmic_child_info *info; const void *blob = gd->fdt_blob;

@@ -68,6 +68,7 @@ int pmic_bind_childs(struct udevice *pmic, int offset, if (!drv) { debug(" - driver: '%s' not found!\n", info->driver);

•                           info++;
                             continue;
                     }
  
  

@@ -77,6 +78,7 @@ int pmic_bind_childs(struct udevice *pmic, int offset, node, &child); if (ret) { debug(" - child binding error: %d\n", ret);

•                           info++;
                             continue;
                     }
  
  

@@ -110,16 +112,16 @@ int pmic_get(const char *name, struct udevice **devp) int pmic_reg_count(struct udevice *dev) { const struct dm_pmic_ops *ops = dev_get_driver_ops(dev);

•   if (!ops)
  

•   if (!ops || !ops->reg_count)
             return -ENOSYS;
  
  

•   return ops->reg_count;
  

•   return ops->reg_count(dev);
  

  }

  int pmic_read(struct udevice *dev, uint reg, uint8_t *buffer, int len) { const struct dm_pmic_ops *ops = dev_get_driver_ops(dev);

•   int ret;
  
     if (!buffer)
             return -EFAULT;
  

@@ -127,17 +129,12 @@ int pmic_read(struct udevice *dev, uint reg, uint8_t *buffer, int len) if (!ops || !ops->read) return -ENOSYS;

•   ret = ops->read(dev, reg, buffer, len);
  

•   if (ret)
  

•           return ret;
  

•   return 0;
  

•   return ops->read(dev, reg, buffer, len);
  

  }

  int pmic_write(struct udevice *dev, uint reg, const uint8_t *buffer, int len) { const struct dm_pmic_ops *ops = dev_get_driver_ops(dev);

•   int ret;
  
     if (!buffer)
             return -EFAULT;
  

@@ -145,11 +142,7 @@ int pmic_write(struct udevice *dev, uint reg, const uint8_t *buffer, int len) if (!ops || !ops->write) return -ENOSYS;

•   ret = ops->write(dev, reg, buffer, len);
  

•   if (ret)
  

•           return ret;
  

•   return 0;
  

•   return ops->write(dev, reg, buffer, len);
  

  }

  UCLASS_DRIVER(pmic) = {

diff --git a/include/power/pmic.h b/include/power/pmic.h index f7ae781..eb152ef 100644 --- a/include/power/pmic.h +++ b/include/power/pmic.h @@ -11,9 +11,9 @@ #ifndef __CORE_PMIC_H_ #define __CORE_PMIC_H_

-#include <linux/list.h> -#include <spi.h> #include <i2c.h> +#include <spi.h> +#include <linux/list.h> #include <power/power_chrg.h>

enum { PMIC_I2C, PMIC_SPI, PMIC_NONE}; @@ -90,10 +90,10 @@ struct pmic {

• U-Boot PMIC Framework
• =====================

• • UCLASS_PMIC - The is designed to provide an I/O interface for PMIC devices.

• • UCLASS_PMIC - This is designed to provide an I/O interface for PMIC devices.
  • For the multi-function PMIC devices, this can be used as parent I/O device

• • for each IC's interface. Then, each children uses its parent for read/write.

• • for each IC's interface. Then, each child uses its parent for read/write.
  • The driver model tree could look like this:

@@ -112,8 +112,8 @@ struct pmic {

• We can find two PMIC cases in boards design:
• • single I/O interface
• • multiple I/O interfaces

• • We bind single PMIC device for each interface, to provide an I/O as a parent,
• • of proper child devices. Each child usually implements a different function,

• • We bind a single PMIC device for each interface, to provide an I/O for
• • its child devices. And each child usually implements a different function,
  • controlled by the same interface.
  • The binding should be done automatically. If device tree nodes/subnodes are

@@ -131,7 +131,7 @@ struct pmic {

• Note:
• Each PMIC interface driver should use a different compatible string.

• • If each pmic child device driver need access the PMIC-specific registers,

• • If a PMIC child device driver needs access the PMIC-specific registers,
  • it need know only the register address and the access can be done through
  • the parent pmic driver. Like in the example:

@@ -141,10 +141,10 @@ struct pmic {

• | |_ dev: my_regulator (set value/etc..) (is child) - UCLASS_REGULATOR
• To ensure such device relationship, the pmic device driver should also bind

• • all its child devices, like in the example below. It should be done by call
• • the 'pmic_bind_childs()' - please refer to the description of this function
• • in this header file. This function, should be called in the driver's '.bind'
• • method.

• • all its child devices, like in the example below. It can be done by calling
• • the 'pmic_bind_children()' - please refer to the function description, which
• • can be found in this header file. This function, should be called inside the
• • driver's bind() method.
  • For the example driver, please refer the MAX77686 driver:
  • • 'drivers/power/pmic/max77686.c'

@@ -156,18 +156,19 @@ struct pmic {

• Should be implemented by UCLASS_PMIC device drivers. The standard
• device operations provides the I/O interface for it's childs.

• • @reg_count: devices register count

• • @reg_count: device's register count
  • @read: read 'len' bytes at "reg" and store it into the 'buffer'
  • @write: write 'len' bytes from the 'buffer' to the register at 'reg' address
  */ struct dm_pmic_ops {

•   int reg_count;
  

•   int (*reg_count)(struct udevice *dev);
     int (*read)(struct udevice *dev, uint reg, uint8_t *buffer, int len);
     int (*write)(struct udevice *dev, uint reg, const uint8_t *buffer,
                  int len);
  

  };

-/* enum pmic_op_type - used for various pmic devices operation calls, +/**

• • enum pmic_op_type - used for various pmic devices operation calls,
  • for reduce a number of lines with the same code for read/write or get/set.
  • @PMIC_OP_GET - get operation

@@ -181,7 +182,7 @@ enum pmic_op_type { /**

• struct pmic_child_info - basic device's child info for bind child nodes with
• the driver by the node name prefix and driver name. This is a helper struct

• • for function: pmic_bind_childs().

• • for function: pmic_bind_children().
  • @prefix - child node name prefix (or its name if is unique or single)
  • @driver - driver name for the sub-node with prefix

@@ -194,7 +195,7 @@ struct pmic_child_info { /* drivers/power/pmic-uclass.c */

/**

• • pmic_bind_childs() - bind drivers for given parent pmic, using child info

• • pmic_bind_children() - bind drivers for given parent pmic, using child info
  • found in 'child_info' array.
  • @pmic - pmic device - the parent of found child's

@@ -216,7 +217,7 @@ struct pmic_child_info {

• (pmic - bind automatically by compatible)
  

• compatible = "my_pmic";
  

• ...
  

• • (pmic's childs - bind by pmic_bind_childs())
    

• • (pmic's childs - bind by pmic_bind_children())
    

  • (nodes prefix: "ldo", driver: "my_regulator_ldo")
    

  • ldo1 { ... };
    

  • ldo2 { ... };
    

@@ -226,8 +227,8 @@ struct pmic_child_info {

• buck2 { ... };
  

• };

*/ -int pmic_bind_childs(struct udevice *pmic, int offset,

•                const struct pmic_child_info *child_info);
  

+int pmic_bind_children(struct udevice *pmic, int offset,

•                  const struct pmic_child_info *child_info);
  
  

  /**

  • pmic_get: get the pmic device using its name

-- 1.9.1

Best regards,

Hi Przemyslaw,

On 8 May 2015 at 10:20, Przemyslaw Marczak p.marczak@samsung.com wrote:

Signed-off-by: Przemyslaw Marczak p.marczak@samsung.com

board/samsung/odroid/odroid.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/board/samsung/odroid/odroid.c b/board/samsung/odroid/odroid.c index 29de325..32155f1 100644 --- a/board/samsung/odroid/odroid.c +++ b/board/samsung/odroid/odroid.c @@ -37,6 +37,7 @@ static const char *mmc_regulators[] = { "VDDQ_EMMC_1.8V", "VDDQ_EMMC_2.8V", "TFLASH_2.8V",

•   NULL,
  

};

void set_board_type(void) @@ -427,9 +428,7 @@ int exynos_init(void)

int exynos_power_init(void) {

•   int list_count = ARRAY_SIZE(mmc_regulators);
  

•   if (regulator_list_autoset(mmc_regulators, list_count, NULL, true))
  

•   if (regulator_list_autoset(mmc_regulators, NULL, true))
            error("Unable to init all mmc regulators");
  
    return 0;
  

@@ -441,7 +440,7 @@ static int s5pc210_phy_control(int on) struct udevice *dev; int ret;

•   ret = regulator_by_platname("VDD_UOTG_3.0V", &dev);
  

•   ret = regulator_get_by_platname("VDD_UOTG_3.0V", &dev);
    if (ret) {
            error("Regulator get error: %d", ret);
            return ret;
  

@@ -487,7 +486,7 @@ int board_usb_init(int index, enum usb_init_type init) /* Power off and on BUCK8 for LAN9730 */ debug("LAN9730 - Turning power buck 8 OFF and ON.\n");

•   ret = regulator_by_platname("VCC_P3V3_2.85V", &dev);
  

•   ret = regulator_get_by_platname("VCC_P3V3_2.85V", &dev);
    if (ret) {
            error("Regulator get error: %d", ret);
            return ret;
  

-- 1.9.1

This patch might need to be merged with the one that changes the function signature, to avoid build errors. Or you could move the CONFIG enable patch later in the stack perhaps?

Regards, Simon

Hello Simon,

On 05/10/2015 03:57 PM, Simon Glass wrote:

Hi Przemyslaw,

On 8 May 2015 at 10:20, Przemyslaw Marczak p.marczak@samsung.com wrote:

...

This change adds new file to sandbox driver model test environment. The file is: test/dm/power.c, and it includes tests for PMIC framework, which includes PMIC uclass and REGULATOR uclass.

All tests are based od Sandbox PMIC emulated device. Some test constants for this device are defined in the header: include/power/sandbox_pmic.h

PMIC tests includes:

• pmic get - tests, that pmic_get() returns the requested device
• pmic I/O - tests I/O by writing and reading some values to PMIC's registers and then compares, that the write/read values are equal.

The regulator tests includes:

• Regulator get by devname/platname
• Voltage set/get
• Current set/get
• Enable set/get
• Mode set/get
• Autoset
• List autoset

For the regulator 'get' test, the returned device pointers are compared, and their names are also compared to the requested one. Every other test, first sets the given attribute and next try to get it. The test pass, when the set/get values are equal.

Signed-off-by: Przemyslaw Marczak p.marczak@samsung.com

include/power/sandbox_pmic.h | 33 ++++ test/dm/Makefile | 2 + test/dm/power.c | 371 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 406 insertions(+) create mode 100644 test/dm/power.c

This looks great. I just have some minor nits below.

Thanks, that's a good information.

...

diff --git a/include/power/sandbox_pmic.h b/include/power/sandbox_pmic.h index f9bc10e..01004a0 100644 --- a/include/power/sandbox_pmic.h +++ b/include/power/sandbox_pmic.h @@ -186,4 +186,37 @@ enum { #define LDO2_UV_REG_DEFAULT LDO2_UV_TO_REG(OUT_LDO2_UV_DEFAULT) #define LDO2_OM_REG_DEFAULT OUT_OM_SET(OUT_LDO2_OM_DEFAULT)

+/* Test data for: test/dm/power.c */

+/* BUCK names */ +#define SANDBOX_BUCK1_DEVNAME "buck1" +#define SANDBOX_BUCK1_PLATNAME "SUPPLY_1.2V" +#define SANDBOX_BUCK2_DEVNAME "buck2" +#define SANDBOX_BUCK2_PLATNAME "SUPPLY_3.3V" +/* LDO names */ +#define SANDBOX_LDO1_DEVNAME "ldo1" +#define SANDBOX_LDO1_PLATNAME "VDD_EMMC_1.8V" +#define SANDBOX_LDO2_DEVNAME "ldo2" +#define SANDBOX_LDO2_PLATNAME "VDD_LCD_3.3V"

+/*

• • Expected regulators setup after call of:
• • • regulator_autoset()
• • • regulator_list_autoset()
• */

+/* BUCK1: for testing regulator_autoset() */ +#define SANDBOX_BUCK1_AUTOSET_EXPECTED_UV 1200000 +#define SANDBOX_BUCK1_AUTOSET_EXPECTED_UA 200000 +#define SANDBOX_BUCK1_AUTOSET_EXPECTED_ENABLE true

+/* LDO1/2 for testing regulator_list_autoset() */ +#define SANDBOX_LDO1_AUTOSET_EXPECTED_UV 1800000 +#define SANDBOX_LDO1_AUTOSET_EXPECTED_UA 100000 +#define SANDBOX_LDO1_AUTOSET_EXPECTED_ENABLE true

+#define SANDBOX_LDO2_AUTOSET_EXPECTED_UV OUT_LDO2_UV_DEFAULT +#define SANDBOX_LDO2_AUTOSET_EXPECTED_UA -ENOSYS +#define SANDBOX_LDO2_AUTOSET_EXPECTED_ENABLE false

• #endif

diff --git a/test/dm/Makefile b/test/dm/Makefile index fd9e29f..30df53d 100644 --- a/test/dm/Makefile +++ b/test/dm/Makefile @@ -24,4 +24,6 @@ obj-$(CONFIG_DM_PCI) += pci.o obj-$(CONFIG_DM_SPI_FLASH) += sf.o obj-$(CONFIG_DM_SPI) += spi.o obj-$(CONFIG_DM_USB) += usb.o +obj-$(CONFIG_DM_PMIC) += power.o +obj-$(CONFIG_DM_REGULATOR) += power.o endif diff --git a/test/dm/power.c b/test/dm/power.c new file mode 100644 index 0000000..8c607c6 --- /dev/null +++ b/test/dm/power.c @@ -0,0 +1,371 @@ +/*

• • Tests for the driver model pmic and regulator code
• • Copyright (c) 2015 Samsung Electronics
• • Przemyslaw Marczak p.marczak@samsung.com
• • SPDX-License-Identifier: GPL-2.0+
• */

+#include <common.h> +#include <errno.h> +#include <dm.h> +#include <fdtdec.h> +#include <malloc.h> +#include <dm/device-internal.h> +#include <dm/root.h> +#include <dm/ut.h> +#include <dm/util.h> +#include <dm/test.h> +#include <dm/uclass-internal.h> +#include <power/pmic.h> +#include <power/regulator.h> +#include <power/sandbox_pmic.h>

+DECLARE_GLOBAL_DATA_PTR;

+#ifdef CONFIG_DM_PMIC

How about creating pmic.c and then you can move the ifdef to the Makefile?

Ok, I can add this.

...

+/* Test PMIC get method */ +static int dm_test_power_pmic_get(struct dm_test_state *dms) +{

•   const char *name = "sandbox_pmic";
  

•   struct udevice *dev;
  

•   ut_assertok(pmic_get(name, &dev));
  

•   ut_assertnonnull(dev);
  

•   /* Check PMIC's name */
  

•   ut_asserteq_str(name, dev->name);
  

•   return 0;
  

+} +DM_TEST(dm_test_power_pmic_get, DM_TESTF_SCAN_FDT);

+/* Test PMIC I/O */ +static int dm_test_power_pmic_io(struct dm_test_state *dms) +{

•   const char *name = "sandbox_pmic";
  

•   uint8_t out_buffer, in_buffer;
  

•   struct udevice *dev;
  

•   int reg_count, i;
  

•   ut_assertok(pmic_get(name, &dev));
  

•   reg_count = pmic_reg_count(dev);
  

•   ut_asserteq(reg_count, SANDBOX_PMIC_REG_COUNT);
  

•   /*
  

•    * Test PMIC I/O - write and read a loop counter.
  

•    * usually we can't write to all PMIC's registers in the real hardware,
  

•    * but we can to the sandbox pmic.
  

•    */
  

•   for (i = 0; i < reg_count; i++) {
  

•           out_buffer = i;
  

•           ut_assertok(pmic_write(dev, i, &out_buffer, 1));
  

•           ut_assertok(pmic_read(dev, i, &in_buffer, 1));
  

•           ut_asserteq(out_buffer, in_buffer);
  

•   }
  

•   return 0;
  

+} +DM_TEST(dm_test_power_pmic_io, DM_TESTF_SCAN_FDT); +#endif /* CONFIG_DM_PMIC */

+#ifdef CONFIG_DM_REGULATOR

This could go in regulator.c

Yes.

...

+enum {

•   BUCK1,
  

•   BUCK2,
  

•   LDO1,
  

•   LDO2,
  

•   OUTPUT_COUNT,
  

+};

+enum {

•   DEVNAME = 0,
  

•   PLATNAME,
  

•   OUTPUT_NAME_COUNT,
  

+};

+static const char *regulator_names[OUTPUT_COUNT][OUTPUT_NAME_COUNT] = {

•   /* devname, platname */
  

•   { SANDBOX_BUCK1_DEVNAME, SANDBOX_BUCK1_PLATNAME },
  

•   { SANDBOX_BUCK2_DEVNAME, SANDBOX_BUCK2_PLATNAME },
  

•   { SANDBOX_LDO1_DEVNAME, SANDBOX_LDO1_PLATNAME},
  

•   { SANDBOX_LDO2_DEVNAME, SANDBOX_LDO2_PLATNAME},
  

+};

+/* Test regulator get method */ +static int dm_test_power_regulator_get(struct dm_test_state *dms) +{

•   struct dm_regulator_uclass_platdata *uc_pdata;
  

•   struct udevice *dev_by_devname;
  

•   struct udevice *dev_by_platname;
  

•   const char *devname;
  

•   const char *platname;
  

•   int i;
  

•   for (i = 0; i < OUTPUT_COUNT; i++) {
  

•           /*
  

•            * Do the test for each regulator's devname and platname,
  

•            * which are related to a single device.
  

•            */
  

•           devname = regulator_names[i][DEVNAME];
  

•           platname = regulator_names[i][PLATNAME];
  

•           /*
  

•            * Check, that regulator_get_by_devname() function, returns
  

•            * a device with the name equal to the requested one.
  

•            */
  

•           ut_assertok(regulator_get_by_devname(devname, &dev_by_devname));
  

•           ut_asserteq_str(devname, dev_by_devname->name);
  

•           /*
  

•            * Check, that regulator_get_by_platname() function, returns
  

•            * a device with the name equal to the requested one.
  

•            */
  

•           ut_assertok(regulator_get_by_platname(platname, &dev_by_platname));
  

•           ut_assert(uc_pdata = dev_get_uclass_platdata(dev_by_platname));
  

•           ut_asserteq_str(platname, uc_pdata->name);
  

•           /*
  

•            * Check, that the pointers returned by both get functions,
  

•            * points to the same regulator device.
  

•            */
  

•           ut_asserteq_ptr(dev_by_devname, dev_by_platname);
  

•   }
  

•   return 0;
  

+} +DM_TEST(dm_test_power_regulator_get, DM_TESTF_SCAN_FDT);

+/* Test regulator set and get Voltage method */ +static int dm_test_power_regulator_set_get_voltage(struct dm_test_state *dms) +{

•   struct dm_regulator_uclass_platdata *uc_pdata;
  

•   struct udevice *dev;
  

•   const char *platname;
  

•   int val_set, val_get;
  

•   /* Set and get Voltage of BUCK1 - set to 'min' constraint */
  

•   platname = regulator_names[BUCK1][PLATNAME];
  

•   ut_assertok(regulator_get_by_platname(platname, &dev));
  

•   ut_assert(uc_pdata = dev_get_uclass_platdata(dev));
  

uc_pdata = dev_get_uclass_platdata(dev) ut_assert(uc_pdata)

and below

Ok.

...

•   val_set = uc_pdata->min_uV;
  

•   ut_assertok(regulator_set_value(dev, val_set));
  

•   val_get = regulator_get_value(dev);
  

•   ut_assert(val_get >= 0);
  

•   ut_asserteq(val_set, val_get);
  

•   return 0;
  

+} +DM_TEST(dm_test_power_regulator_set_get_voltage, DM_TESTF_SCAN_FDT);

+/* Test regulator set and get Current method */ +static int dm_test_power_regulator_set_get_current(struct dm_test_state *dms) +{

•   struct dm_regulator_uclass_platdata *uc_pdata;
  

•   struct udevice *dev;
  

•   const char *platname;
  

•   int val_set, val_get;
  

•   /* Set and get the Current of LDO1 - set to 'min' constraint */
  

•   platname = regulator_names[LDO1][PLATNAME];
  

•   ut_assertok(regulator_get_by_platname(platname, &dev));
  

•   ut_assert(uc_pdata = dev_get_uclass_platdata(dev));
  

•   val_set = uc_pdata->min_uA;
  

•   ut_assertok(regulator_set_current(dev, val_set));
  

•   val_get = regulator_get_current(dev);
  

•   ut_assert(val_get >= 0);
  

•   ut_asserteq(val_set, val_get);
  

•   /* Check LDO2 current limit constraints - should be -ENODATA */
  

•   platname = regulator_names[LDO2][PLATNAME];
  

•   ut_assertok(regulator_get_by_platname(platname, &dev));
  

•   ut_assert(uc_pdata = dev_get_uclass_platdata(dev));
  

•   ut_asserteq(uc_pdata->min_uA, -ENODATA);
  

Expected value should come first.

Ok

...

•   ut_asserteq(uc_pdata->max_uA, -ENODATA);
  

•   /* Try set the Current of LDO2 - should return -ENOSYS */
  

•   ut_asserteq(regulator_set_current(dev, 0), -ENOSYS);
  

Nice test

...

•   return 0;
  

+} +DM_TEST(dm_test_power_regulator_set_get_current, DM_TESTF_SCAN_FDT);

+/* Test regulator set and get Enable method */ +static int dm_test_power_regulator_set_get_enable(struct dm_test_state *dms) +{

•   const char *platname;
  

•   struct udevice *dev;
  

•   bool val_set = true;
  

•   /* Set the Enable of LDO1 - default is disabled */
  

•   platname = regulator_names[LDO1][PLATNAME];
  

•   ut_assertok(regulator_get_by_platname(platname, &dev));
  

•   ut_assertok(regulator_set_enable(dev, val_set));
  

•   /* Get the Enable state of LDO1 and compare it with the requested one */
  

•   ut_asserteq(regulator_get_enable(dev), val_set);
  

•   return 0;
  

+} +DM_TEST(dm_test_power_regulator_set_get_enable, DM_TESTF_SCAN_FDT);

+/* Test regulator set and get mode method */ +static int dm_test_power_regulator_set_get_mode(struct dm_test_state *dms) +{

•   const char *platname;
  

•   struct udevice *dev;
  

•   int val_set = LDO_OM_SLEEP;
  

•   /* Set the mode id to LDO_OM_SLEEP of LDO1 - default is LDO_OM_OFF */
  

•   platname = regulator_names[LDO1][PLATNAME];
  

•   ut_assertok(regulator_get_by_platname(platname, &dev));
  

•   ut_assertok(regulator_set_mode(dev, val_set));
  

•   /* Get the mode id of LDO1 and compare it with the requested one */
  

•   ut_asserteq(regulator_get_mode(dev), val_set);
  

•   return 0;
  

+} +DM_TEST(dm_test_power_regulator_set_get_mode, DM_TESTF_SCAN_FDT);

+/* Test regulator autoset method */ +static int dm_test_power_regulator_autoset(struct dm_test_state *dms) +{

•   const char *platname;
  

•   struct udevice *dev, *dev_autoset;
  

•   /*
  

•    * Test the BUCK1 with fdt properties
  

•    * - min-microvolt = max-microvolt = 1200000
  

•    * - min-microamp = max-microamp = 200000
  

•    * - always-on = set
  

•    * - boot-on = not set
  

•    * Expected output state: uV=1200000; uA=200000; output enabled
  

•    */
  

•   platname = regulator_names[BUCK1][PLATNAME];
  

•   ut_assertok(regulator_autoset(platname, &dev_autoset, false));
  

•   /* Check, that the returned device is proper */
  

•   ut_assertok(regulator_get_by_platname(platname, &dev));
  

•   ut_asserteq_ptr(dev, dev_autoset);
  

•   /* Check the setup after autoset */
  

•   ut_asserteq(regulator_get_value(dev),
  

•               SANDBOX_BUCK1_AUTOSET_EXPECTED_UV);
  

Expected value should come first.

...

•   ut_asserteq(regulator_get_current(dev),
  

•               SANDBOX_BUCK1_AUTOSET_EXPECTED_UA);
  

•   ut_asserteq(regulator_get_enable(dev),
  

•               SANDBOX_BUCK1_AUTOSET_EXPECTED_ENABLE);
  

•   return 0;
  

+} +DM_TEST(dm_test_power_regulator_autoset, DM_TESTF_SCAN_FDT);

+/*

• • Struct setting: to keep the expected output settings.
• • @voltage: Voltage value [uV]
• • @current: Current value [uA]
• • @enable: output enable state: true/false
• */

+struct setting {

•   int voltage;
  

•   int current;
  

•   bool enable;
  

+};

+/*

• • platname_list: an array of regulator platform names.
• • For testing regulator_list_autoset() for outputs:
• • • LDO1
• • • LDO2
• */

+static const char *platname_list[] = {

•   SANDBOX_LDO1_PLATNAME,
  

•   SANDBOX_LDO2_PLATNAME,
  

•   NULL,
  

+};

+/*

• • expected_setting_list: an array of regulator output setting, expected after
• • call of the regulator_list_autoset() for the "platname_list" array.
• • For testing results of regulator_list_autoset() for outputs:
• • • LDO1
• • • LDO2
• • The settings are defined in: include/power/sandbox_pmic.h
• */

+static const struct setting expected_setting_list[] = {

•   [0] = { /* LDO1 */
  

•   .voltage = SANDBOX_LDO1_AUTOSET_EXPECTED_UV,
  

•   .current = SANDBOX_LDO1_AUTOSET_EXPECTED_UA,
  

•   .enable  = SANDBOX_LDO1_AUTOSET_EXPECTED_ENABLE,
  

•   },
  

•   [1] = { /* LDO2 */
  

•   .voltage = SANDBOX_LDO2_AUTOSET_EXPECTED_UV,
  

•   .current = SANDBOX_LDO2_AUTOSET_EXPECTED_UA,
  

•   .enable  = SANDBOX_LDO2_AUTOSET_EXPECTED_ENABLE,
  

•   },
  

+};

+static int list_count = ARRAY_SIZE(expected_setting_list);

+/* Test regulator list autoset method */ +static int dm_test_power_regulator_autoset_list(struct dm_test_state *dms) +{

•   struct udevice *dev_list[2], *dev;
  

•   int i;
  

•   /*
  

•    * Test the settings of the regulator list:
  

•    * LDO1 with fdt properties:
  

•    * - min-microvolt = max-microvolt = 1800000
  

•    * - min-microamp = max-microamp = 100000
  

•    * - always-on = not set
  

•    * - boot-on = set
  

•    * Expected output state: uV=1800000; uA=100000; output enabled
  

•    *
  

•    * LDO2 with fdt properties:
  

•    * - min-microvolt = max-microvolt = 3300000
  

•    * - always-on = not set
  

•    * - boot-on = not set
  

•    * Expected output state: uV=300000(default); output disabled(default)
  

•    * The expected settings are defined in: include/power/sandbox_pmic.h.
  

•    */
  

•   ut_assertok(regulator_list_autoset(platname_list, dev_list, false));
  

•   for (i = 0; i < list_count; i++) {
  

•           /* Check, that the returned device is non-NULL */
  

•           ut_assert(dev_list[i]);
  

•           /* Check, that the returned device is proper */
  

•           ut_assertok(regulator_get_by_platname(platname_list[i], &dev));
  

•           ut_asserteq_ptr(dev_list[i], dev);
  

•           /* Check, that regulator output Voltage value is as expected */
  

•           ut_asserteq(regulator_get_value(dev_list[i]),
  

•                       expected_setting_list[i].voltage);
  

Expected value should come first.

ok

...

•           /* Check, that regulator output Current value is as expected */
  

•           ut_asserteq(regulator_get_current(dev_list[i]),
  

•                       expected_setting_list[i].current);
  

•           /* Check, that regulator output Enable state is as expected */
  

•           ut_asserteq(regulator_get_enable(dev_list[i]),
  

•                       expected_setting_list[i].enable);
  

•   }
  

•   return 0;
  

+} +DM_TEST(dm_test_power_regulator_autoset_list, DM_TESTF_SCAN_FDT);

+#endif /* CONFIG_DM_REGULATOR */

1.9.1

Regards, Simon

Thank you,

Adding regulators subnode to fdt max77686 node, allows properly init regulators by the max77686 regulator driver. This enables the complete functionality of the regulator command.

Signed-off-by: Przemyslaw Marczak p.marczak@samsung.com Acked-by: Simon Glass sjg@chromium.org --- arch/arm/dts/exynos4412-odroid.dts | 253 +++++++++++++++++++++++++++++++++++++ 1 file changed, 253 insertions(+)

diff --git a/arch/arm/dts/exynos4412-odroid.dts b/arch/arm/dts/exynos4412-odroid.dts index 5507d9a..415dfea 100644 --- a/arch/arm/dts/exynos4412-odroid.dts +++ b/arch/arm/dts/exynos4412-odroid.dts @@ -40,6 +40,259 @@ interrupts = <7 0>; reg = <0x09 0 0>; #clock-cells = <1>; + + voltage-regulators { + ldo1_reg: ldo1 { + regulator-compatible = "LDO1"; + regulator-name = "VDD_ALIVE_1.0V"; + regulator-min-microvolt = <1000000>; + regulator-max-microvolt = <1000000>; + }; + + ldo2_reg: ldo2 { + regulator-compatible = "LDO2"; + regulator-name = "VDDQ_VM1M2_1.2V"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <1200000>; + }; + + ldo3_reg: ldo3 { + regulator-compatible = "LDO3"; + regulator-name = "VCC_1.8V_AP"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + }; + + ldo4_reg: ldo4 { + regulator-compatible = "LDO4"; + regulator-name = "VDDQ_MMC2_2.8V"; + regulator-min-microvolt = <2800000>; + regulator-max-microvolt = <2800000>; + }; + + ldo5_reg: ldo5 { + regulator-compatible = "LDO5"; + regulator-name = "VDDQ_MMC0/1/3_1.8V"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + }; + + ldo6_reg: ldo6 { + regulator-compatible = "LDO6"; + regulator-name = "VMPLL_1.0V"; + regulator-min-microvolt = <1100000>; + regulator-max-microvolt = <1100000>; + }; + + ldo7_reg: ldo7 { + regulator-compatible = "LDO7"; + regulator-name = "VPLL_1.1V"; + regulator-min-microvolt = <1100000>; + regulator-max-microvolt = <1100000>; + }; + + ldo8_reg: ldo8 { + regulator-compatible = "LDO8"; + regulator-name = "VDD_MIPI/HDMI_1.0V"; + regulator-min-microvolt = <1000000>; + regulator-max-microvolt = <1000000>; + }; + + ldo9_reg: ldo9 { + regulator-compatible = "LDO9"; + regulator-name = "nc"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + }; + + ldo10_reg: ldo10 { + regulator-compatible = "LDO10"; + regulator-name = "VDD_MIPI/HDMI_1.8V"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + }; + + ldo11_reg: ldo11 { + regulator-compatible = "LDO11"; + regulator-name = "VDD_ABB1_1.8V"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + }; + + ldo12_reg: ldo12 { + regulator-compatible = "LDO12"; + regulator-name = "VDD_UOTG_3.0V"; + regulator-min-microvolt = <3000000>; + regulator-max-microvolt = <3000000>; + }; + + ldo13_reg: ldo13 { + regulator-compatible = "LDO13"; + regulator-name = "VDD_C2C_1.8V"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + }; + + ldo14_reg: ldo14 { + regulator-compatible = "LDO14"; + regulator-name = "VDD_ABB02_1.8V"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + }; + + ldo15_reg: ldo15 { + regulator-compatible = "LDO15"; + regulator-name = "VDD_HSIC/OTG_1.0V"; + regulator-min-microvolt = <1000000>; + regulator-max-microvolt = <1000000>; + }; + + ldo16_reg: ldo16 { + regulator-compatible = "LDO16"; + regulator-name = "VDD_HSIC_1.8V"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + }; + + ldo17_reg: ldo17 { + regulator-compatible = "LDO17"; + regulator-name = "VDDQ_CAM_1.2V"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <1200000>; + }; + + ldo18_reg: ldo18 { + regulator-compatible = "LDO18"; + regulator-name = "nc"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + }; + + ldo19_reg: ldo19 { + regulator-compatible = "LDO19"; + regulator-name = "nc"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + }; + + ldo20_reg: ldo20 { + regulator-compatible = "LDO20"; + regulator-name = "VDDQ_EMMC_1.8V"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-always-on; + regulator-boot-on; + }; + + ldo21_reg: ldo21 { + regulator-compatible = "LDO21"; + regulator-name = "TFLASH_2.8V"; + regulator-min-microvolt = <2800000>; + regulator-max-microvolt = <2800000>; + regulator-always-on; + regulator-boot-on; + }; + + ldo22_reg: ldo22 { + regulator-compatible = "LDO22"; + regulator-name = "VDDQ_EMMC_2.8V"; + regulator-min-microvolt = <2800000>; + regulator-max-microvolt = <2800000>; + regulator-always-on; + regulator-boot-on; + }; + + ldo23_reg: ldo23 { + regulator-compatible = "LDO23"; + regulator-name = "nc"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + }; + + ldo24_reg: ldo24 { + regulator-compatible = "LDO24"; + regulator-name = "nc"; + regulator-min-microvolt = <3000000>; + regulator-max-microvolt = <3000000>; + }; + + ldo25_reg: ldo25 { + regulator-compatible = "LDO25"; + regulator-name = "VDDQ_LCD_3.0V"; + regulator-min-microvolt = <3000000>; + regulator-max-microvolt = <3000000>; + }; + + ldo26_reg: ldo26 { + regulator-compatible = "LDO26"; + regulator-name = "nc"; + regulator-min-microvolt = <3000000>; + regulator-max-microvolt = <3000000>; + }; + + buck1_reg: buck@1 { + regulator-compatible = "BUCK1"; + regulator-name = "VDD_MIF_1.0V"; + regulator-min-microvolt = <8500000>; + regulator-max-microvolt = <1100000>; + }; + + buck2_reg: buck@2 { + regulator-compatible = "BUCK2"; + regulator-name = "VDD_ARM_1.0V"; + regulator-min-microvolt = <850000>; + regulator-max-microvolt = <1500000>; + }; + + buck3_reg: buck3 { + regulator-compatible = "BUCK3"; + regulator-name = "VDD_INT_1.1V"; + regulator-min-microvolt = <850000>; + regulator-max-microvolt = <1150000>; + }; + + buck4_reg: buck4 { + regulator-compatible = "BUCK4"; + regulator-name = "VDD_G3D_1.0V"; + regulator-min-microvolt = <850000>; + regulator-max-microvolt = <1150000>; + }; + + buck5_reg: buck5 { + regulator-compatible = "BUCK5"; + regulator-name = "VDDQ_AP_1.2V"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <1200000>; + }; + + buck6_reg: buck6 { + regulator-compatible = "BUCK6"; + regulator-name = "VCC_INL1/7_1.35V"; + regulator-min-microvolt = <1350000>; + regulator-max-microvolt = <1350000>; + }; + + buck7_reg: buck7 { + regulator-compatible = "BUCK7"; + regulator-name = "VCC_INL2/3/5_2.0V"; + regulator-min-microvolt = <2000000>; + regulator-max-microvolt = <2000000>; + }; + + buck8_reg: buck8 { + regulator-compatible = "BUCK8"; + regulator-name = "VCC_P3V3_2.85V"; + regulator-min-microvolt = <2850000>; + regulator-max-microvolt = <3300000>; + }; + + buck9_reg: buck9 { + regulator-compatible = "BUCK9"; + regulator-name = "nc"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <1200000>; + }; + }; }; };

This change enables the configs required to init and setup max77686 regulator driver, using the new driver model pmic and regulator API. And also changes the old pmic framework calls to the new ones.

This commits enables: - CONFIG_ERRNO_STR - CONFIG_DM_PMIC - CONFIG_DM_PMIC_CMD - CONFIG_DM_PMIC_MAX77686 - CONFIG_DM_REGULATOR - CONFIG_DM_REGULATOR_CMD - CONFIG_DM_REGULATOR_MAX77686

And removes the unused: - CONFIG_DM_I2C_COMPAT - CONFIG_POWER - CONFIG_POWER_I2C - CONFIG_POWER_MAX77686

Signed-off-by: Przemyslaw Marczak p.marczak@samsung.com Acked-by: Simon Glass sjg@chromium.org --- Change V2: - split two commits for odroid into one --- board/samsung/common/misc.c | 1 + board/samsung/odroid/odroid.c | 77 +++++++++++++++++++++++++------------------ configs/odroid_defconfig | 8 ++++- include/configs/odroid.h | 5 --- 4 files changed, 53 insertions(+), 38 deletions(-)

diff --git a/board/samsung/common/misc.c b/board/samsung/common/misc.c index 1a77c82..f0d69d4 100644 --- a/board/samsung/common/misc.c +++ b/board/samsung/common/misc.c @@ -16,6 +16,7 @@ #include <asm/arch/cpu.h> #include <asm/gpio.h> #include <linux/input.h> +#include <dm.h> #include <power/pmic.h> #include <mmc.h>

diff --git a/board/samsung/odroid/odroid.c b/board/samsung/odroid/odroid.c index ae41c29..29de325 100644 --- a/board/samsung/odroid/odroid.c +++ b/board/samsung/odroid/odroid.c @@ -12,7 +12,9 @@ #include <asm/arch/gpio.h> #include <asm/gpio.h> #include <asm/arch/cpu.h> +#include <dm.h> #include <power/pmic.h> +#include <power/regulator.h> #include <power/max77686_pmic.h> #include <errno.h> #include <mmc.h> @@ -31,6 +33,12 @@ enum { ODROID_TYPES, };

+static const char *mmc_regulators[] = { + "VDDQ_EMMC_1.8V", + "VDDQ_EMMC_2.8V", + "TFLASH_2.8V", +}; + void set_board_type(void) { /* Set GPA1 pin 1 to HI - enable XCL205 output */ @@ -403,21 +411,6 @@ static void board_gpio_init(void) #endif }

-static int pmic_init_max77686(void) -{ - struct pmic *p = pmic_get("MAX77686_PMIC"); - - if (pmic_probe(p)) - return -ENODEV; - - /* Set LDO Voltage */ - max77686_set_ldo_voltage(p, 20, 1800000); /* LDO20 eMMC */ - max77686_set_ldo_voltage(p, 21, 2800000); /* LDO21 SD */ - max77686_set_ldo_voltage(p, 22, 2800000); /* LDO22 eMMC */ - - return 0; -} - int exynos_early_init_f(void) { board_clock_init(); @@ -434,8 +427,10 @@ int exynos_init(void)

int exynos_power_init(void) { - pmic_init(0); - pmic_init_max77686(); + int list_count = ARRAY_SIZE(mmc_regulators); + + if (regulator_list_autoset(mmc_regulators, list_count, NULL, true)) + error("Unable to init all mmc regulators");

return 0; } @@ -443,19 +438,20 @@ int exynos_power_init(void) #ifdef CONFIG_USB_GADGET static int s5pc210_phy_control(int on) { - struct pmic *p_pmic; + struct udevice *dev; + int ret;

- p_pmic = pmic_get("MAX77686_PMIC"); - if (!p_pmic) - return -ENODEV; - - if (pmic_probe(p_pmic)) - return -1; + ret = regulator_by_platname("VDD_UOTG_3.0V", &dev); + if (ret) { + error("Regulator get error: %d", ret); + return ret; + }

if (on) - return max77686_set_ldo_mode(p_pmic, 12, OPMODE_ON); + return regulator_set_mode(dev, OPMODE_ON); else - return max77686_set_ldo_mode(p_pmic, 12, OPMODE_LPM); + return regulator_set_mode(dev, OPMODE_LPM); + }

struct s3c_plat_otg_data s5pc210_otg_data = { @@ -472,7 +468,8 @@ struct s3c_plat_otg_data s5pc210_otg_data = { int board_usb_init(int index, enum usb_init_type init) { #ifdef CONFIG_CMD_USB - struct pmic *p_pmic; + struct udevice *dev; + int ret;

/* Set Ref freq 0 => 24MHz, 1 => 26MHz*/ /* Odroid Us have it at 24MHz, Odroid Xs at 26MHz */ @@ -490,14 +487,30 @@ int board_usb_init(int index, enum usb_init_type init) /* Power off and on BUCK8 for LAN9730 */ debug("LAN9730 - Turning power buck 8 OFF and ON.\n");

- p_pmic = pmic_get("MAX77686_PMIC"); - if (p_pmic && !pmic_probe(p_pmic)) { - max77686_set_buck_voltage(p_pmic, 8, 750000); - max77686_set_buck_voltage(p_pmic, 8, 3300000); + ret = regulator_by_platname("VCC_P3V3_2.85V", &dev); + if (ret) { + error("Regulator get error: %d", ret); + return ret; }

-#endif + ret = regulator_set_enable(dev, true); + if (ret) { + error("Regulator %s enable setting error: %d", dev->name, ret); + return ret; + }

+ ret = regulator_set_value(dev, 750000); + if (ret) { + error("Regulator %s value setting error: %d", dev->name, ret); + return ret; + } + + ret = regulator_set_value(dev, 3300000); + if (ret) { + error("Regulator %s value setting error: %d", dev->name, ret); + return ret; + } +#endif debug("USB_udc_probe\n"); return s3c_udc_probe(&s5pc210_otg_data); } diff --git a/configs/odroid_defconfig b/configs/odroid_defconfig index d32b5b5..6ca9586 100644 --- a/configs/odroid_defconfig +++ b/configs/odroid_defconfig @@ -4,5 +4,11 @@ CONFIG_TARGET_ODROID=y CONFIG_OF_CONTROL=y CONFIG_DEFAULT_DEVICE_TREE="exynos4412-odroid" CONFIG_DM_I2C=y -CONFIG_DM_I2C_COMPAT=y # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set +CONFIG_ERRNO_STR=y +CONFIG_DM_PMIC=y +CONFIG_CMD_PMIC=y +CONFIG_DM_PMIC_MAX77686=y +CONFIG_DM_REGULATOR=y +CONFIG_CMD_REGULATOR=y +CONFIG_DM_REGULATOR_MAX77686=y diff --git a/include/configs/odroid.h b/include/configs/odroid.h index 5ee0abe..3874baa 100644 --- a/include/configs/odroid.h +++ b/include/configs/odroid.h @@ -182,11 +182,6 @@ #define CONFIG_SYS_I2C_S3C24X0_SPEED 100000 #define CONFIG_SYS_I2C_S3C24X0_SLAVE 0

-/* POWER */ -#define CONFIG_POWER -#define CONFIG_POWER_I2C -#define CONFIG_POWER_MAX77686 - /* GPT */ #define CONFIG_RANDOM_UUID

The cleanup includes: - pmic.h - fix mistakes in a few comments - pmic operations: value 'reg_count' - redefine as function call - fix function name: pmic_bind_childs() -> pmic_bind_children() - pmic_bind_children: change the 'while' loop with the 'for' - add implementation of pmic_reg_count() method - pmic_bind_children() - update function call name - Kconfig: add new line at the end of file - Update MAX77686 driver code

Signed-off-by: Przemyslaw Marczak p.marczak@samsung.com Acked-by: Simon Glass sjg@chromium.org Tested on sandbox: Tested-by: Simon Glass sjg@chromium.org --- Changes V2: - cleanup of the pmic_bind_children() loop - split with: dm: pmic: max77686: update driver code --- drivers/power/pmic/Kconfig | 2 +- drivers/power/pmic/max77686.c | 15 ++++++++++----- drivers/power/pmic/pmic-uclass.c | 31 +++++++++---------------------- include/power/pmic.h | 39 ++++++++++++++++++++------------------- 4 files changed, 40 insertions(+), 47 deletions(-)

diff --git a/drivers/power/pmic/Kconfig b/drivers/power/pmic/Kconfig index af68783..d99d9e3 100644 --- a/drivers/power/pmic/Kconfig +++ b/drivers/power/pmic/Kconfig @@ -15,4 +15,4 @@ config DM_PMIC_MAX77686 depends on DM_PMIC ---help--- This config enables implementation of driver-model pmic uclass features - for PMIC MAX77686. The driver implements read/write operations. \ No newline at end of file + for PMIC MAX77686. The driver implements read/write operations. diff --git a/drivers/power/pmic/max77686.c b/drivers/power/pmic/max77686.c index e9503e2..3523b4a 100644 --- a/drivers/power/pmic/max77686.c +++ b/drivers/power/pmic/max77686.c @@ -16,12 +16,17 @@

DECLARE_GLOBAL_DATA_PTR;

-static const struct pmic_child_info pmic_childs_info[] = { +static const struct pmic_child_info pmic_children_info[] = { { .prefix = "ldo", .driver = MAX77686_LDO_DRIVER }, { .prefix = "buck", .driver = MAX77686_BUCK_DRIVER }, { }, };

+static int max77686_reg_count(struct udevice *dev) +{ + return MAX77686_NUM_OF_REGS; +} + static int max77686_write(struct udevice *dev, uint reg, const uint8_t *buff, int len) { @@ -47,7 +52,7 @@ static int max77686_bind(struct udevice *dev) { int regulators_node; const void *blob = gd->fdt_blob; - int childs; + int children;

regulators_node = fdt_subnode_offset(blob, dev->of_offset, "voltage-regulators"); @@ -59,8 +64,8 @@ static int max77686_bind(struct udevice *dev)

debug("%s: '%s' - found regulators subnode\n", __func__, dev->name);

- childs = pmic_bind_childs(dev, regulators_node, pmic_childs_info); - if (!childs) + children = pmic_bind_children(dev, regulators_node, pmic_children_info); + if (!children) debug("%s: %s - no child found\n", __func__, dev->name);

/* Always return success for this device */ @@ -68,7 +73,7 @@ static int max77686_bind(struct udevice *dev) }

static struct dm_pmic_ops max77686_ops = { - .reg_count = MAX77686_NUM_OF_REGS, + .reg_count = max77686_reg_count, .read = max77686_read, .write = max77686_write, }; diff --git a/drivers/power/pmic/pmic-uclass.c b/drivers/power/pmic/pmic-uclass.c index d82d3da..812ac13 100644 --- a/drivers/power/pmic/pmic-uclass.c +++ b/drivers/power/pmic/pmic-uclass.c @@ -27,8 +27,8 @@ static ulong str_get_num(const char *ptr, const char *maxptr) return simple_strtoul(ptr, NULL, 0); }

-int pmic_bind_childs(struct udevice *pmic, int offset, - const struct pmic_child_info *child_info) +int pmic_bind_children(struct udevice *pmic, int offset, + const struct pmic_child_info *child_info) { const struct pmic_child_info *info; const void *blob = gd->fdt_blob; @@ -53,14 +53,10 @@ int pmic_bind_childs(struct udevice *pmic, int offset, node);

child = NULL; - info = child_info; - while (info->prefix) { + for (info = child_info; info->prefix && info->driver; info++) { prefix_len = strlen(info->prefix); - if (strncasecmp(info->prefix, node_name, prefix_len) || - !info->driver) { - info++; + if (strncasecmp(info->prefix, node_name, prefix_len)) continue; - }

debug(" - compatible prefix: '%s'\n", info->prefix);

@@ -110,16 +106,16 @@ int pmic_get(const char *name, struct udevice **devp) int pmic_reg_count(struct udevice *dev) { const struct dm_pmic_ops *ops = dev_get_driver_ops(dev); - if (!ops) + + if (!ops || !ops->reg_count) return -ENOSYS;

- return ops->reg_count; + return ops->reg_count(dev); }

int pmic_read(struct udevice *dev, uint reg, uint8_t *buffer, int len) { const struct dm_pmic_ops *ops = dev_get_driver_ops(dev); - int ret;

if (!buffer) return -EFAULT; @@ -127,17 +123,12 @@ int pmic_read(struct udevice *dev, uint reg, uint8_t *buffer, int len) if (!ops || !ops->read) return -ENOSYS;

- ret = ops->read(dev, reg, buffer, len); - if (ret) - return ret; - - return 0; + return ops->read(dev, reg, buffer, len); }

int pmic_write(struct udevice *dev, uint reg, const uint8_t *buffer, int len) { const struct dm_pmic_ops *ops = dev_get_driver_ops(dev); - int ret;

if (!buffer) return -EFAULT; @@ -145,11 +136,7 @@ int pmic_write(struct udevice *dev, uint reg, const uint8_t *buffer, int len) if (!ops || !ops->write) return -ENOSYS;

- ret = ops->write(dev, reg, buffer, len); - if (ret) - return ret; - - return 0; + return ops->write(dev, reg, buffer, len); }

UCLASS_DRIVER(pmic) = { diff --git a/include/power/pmic.h b/include/power/pmic.h index f7ae781..eb152ef 100644 --- a/include/power/pmic.h +++ b/include/power/pmic.h @@ -11,9 +11,9 @@ #ifndef __CORE_PMIC_H_ #define __CORE_PMIC_H_

-#include <linux/list.h> -#include <spi.h> #include <i2c.h> +#include <spi.h> +#include <linux/list.h> #include <power/power_chrg.h>

enum { PMIC_I2C, PMIC_SPI, PMIC_NONE}; @@ -90,10 +90,10 @@ struct pmic { * U-Boot PMIC Framework * ===================== * - * UCLASS_PMIC - The is designed to provide an I/O interface for PMIC devices. + * UCLASS_PMIC - This is designed to provide an I/O interface for PMIC devices. * * For the multi-function PMIC devices, this can be used as parent I/O device - * for each IC's interface. Then, each children uses its parent for read/write. + * for each IC's interface. Then, each child uses its parent for read/write. * * The driver model tree could look like this: * @@ -112,8 +112,8 @@ struct pmic { * We can find two PMIC cases in boards design: * - single I/O interface * - multiple I/O interfaces - * We bind single PMIC device for each interface, to provide an I/O as a parent, - * of proper child devices. Each child usually implements a different function, + * We bind a single PMIC device for each interface, to provide an I/O for + * its child devices. And each child usually implements a different function, * controlled by the same interface. * * The binding should be done automatically. If device tree nodes/subnodes are @@ -131,7 +131,7 @@ struct pmic { * Note: * Each PMIC interface driver should use a different compatible string. * - * If each pmic child device driver need access the PMIC-specific registers, + * If a PMIC child device driver needs access the PMIC-specific registers, * it need know only the register address and the access can be done through * the parent pmic driver. Like in the example: * @@ -141,10 +141,10 @@ struct pmic { * | |_ dev: my_regulator (set value/etc..) (is child) - UCLASS_REGULATOR * * To ensure such device relationship, the pmic device driver should also bind - * all its child devices, like in the example below. It should be done by call - * the 'pmic_bind_childs()' - please refer to the description of this function - * in this header file. This function, should be called in the driver's '.bind' - * method. + * all its child devices, like in the example below. It can be done by calling + * the 'pmic_bind_children()' - please refer to the function description, which + * can be found in this header file. This function, should be called inside the + * driver's bind() method. * * For the example driver, please refer the MAX77686 driver: * - 'drivers/power/pmic/max77686.c' @@ -156,18 +156,19 @@ struct pmic { * Should be implemented by UCLASS_PMIC device drivers. The standard * device operations provides the I/O interface for it's childs. * - * @reg_count: devices register count + * @reg_count: device's register count * @read: read 'len' bytes at "reg" and store it into the 'buffer' * @write: write 'len' bytes from the 'buffer' to the register at 'reg' address */ struct dm_pmic_ops { - int reg_count; + int (*reg_count)(struct udevice *dev); int (*read)(struct udevice *dev, uint reg, uint8_t *buffer, int len); int (*write)(struct udevice *dev, uint reg, const uint8_t *buffer, int len); };

-/* enum pmic_op_type - used for various pmic devices operation calls, +/** + * enum pmic_op_type - used for various pmic devices operation calls, * for reduce a number of lines with the same code for read/write or get/set. * * @PMIC_OP_GET - get operation @@ -181,7 +182,7 @@ enum pmic_op_type { /** * struct pmic_child_info - basic device's child info for bind child nodes with * the driver by the node name prefix and driver name. This is a helper struct - * for function: pmic_bind_childs(). + * for function: pmic_bind_children(). * * @prefix - child node name prefix (or its name if is unique or single) * @driver - driver name for the sub-node with prefix @@ -194,7 +195,7 @@ struct pmic_child_info { /* drivers/power/pmic-uclass.c */

/** - * pmic_bind_childs() - bind drivers for given parent pmic, using child info + * pmic_bind_children() - bind drivers for given parent pmic, using child info * found in 'child_info' array. * * @pmic - pmic device - the parent of found child's @@ -216,7 +217,7 @@ struct pmic_child_info { * (pmic - bind automatically by compatible) * compatible = "my_pmic"; * ... - * (pmic's childs - bind by pmic_bind_childs()) + * (pmic's childs - bind by pmic_bind_children()) * (nodes prefix: "ldo", driver: "my_regulator_ldo") * ldo1 { ... }; * ldo2 { ... }; @@ -226,8 +227,8 @@ struct pmic_child_info { * buck2 { ... }; * }; */ -int pmic_bind_childs(struct udevice *pmic, int offset, - const struct pmic_child_info *child_info); +int pmic_bind_children(struct udevice *pmic, int offset, + const struct pmic_child_info *child_info);

/** * pmic_get: get the pmic device using its name

This cleanup includes: - remove of the preprocessor macros which pointed to long name functions - update of the names of some regulator uclass driver functions - cleanup of the function regulator_autoset() - reword of some comments of regulator uclass header file - regulator_get_by_platname: check error for uclass_find_* function calls - add function: regulator_name_is_unique - regulator post_bind(): check regulator name uniqueness - fix mistakes in: regulator/Kconfig - regulator.h: update comments - odroid u3: cleanup the regulator calls

Signed-off-by: Przemyslaw Marczak p.marczak@samsung.com Acked-by: Simon Glass sjg@chromium.org Tested on sandbox: Tested-by: Simon Glass sjg@chromium.org --- Changes V2: - odroid u3: cleanup the regulator calls --- board/samsung/odroid/odroid.c | 9 +-- drivers/power/regulator/Kconfig | 2 +- drivers/power/regulator/regulator-uclass.c | 104 +++++++++++++++++--------- include/power/regulator.h | 116 +++++++++++++++-------------- 4 files changed, 132 insertions(+), 99 deletions(-)

diff --git a/board/samsung/odroid/odroid.c b/board/samsung/odroid/odroid.c index 29de325..32155f1 100644 --- a/board/samsung/odroid/odroid.c +++ b/board/samsung/odroid/odroid.c @@ -37,6 +37,7 @@ static const char *mmc_regulators[] = { "VDDQ_EMMC_1.8V", "VDDQ_EMMC_2.8V", "TFLASH_2.8V", + NULL, };

void set_board_type(void) @@ -427,9 +428,7 @@ int exynos_init(void)

int exynos_power_init(void) { - int list_count = ARRAY_SIZE(mmc_regulators); - - if (regulator_list_autoset(mmc_regulators, list_count, NULL, true)) + if (regulator_list_autoset(mmc_regulators, NULL, true)) error("Unable to init all mmc regulators");

return 0; @@ -441,7 +440,7 @@ static int s5pc210_phy_control(int on) struct udevice *dev; int ret;

- ret = regulator_by_platname("VDD_UOTG_3.0V", &dev); + ret = regulator_get_by_platname("VDD_UOTG_3.0V", &dev); if (ret) { error("Regulator get error: %d", ret); return ret; @@ -487,7 +486,7 @@ int board_usb_init(int index, enum usb_init_type init) /* Power off and on BUCK8 for LAN9730 */ debug("LAN9730 - Turning power buck 8 OFF and ON.\n");

- ret = regulator_by_platname("VCC_P3V3_2.85V", &dev); + ret = regulator_get_by_platname("VCC_P3V3_2.85V", &dev); if (ret) { error("Regulator get error: %d", ret); return ret; diff --git a/drivers/power/regulator/Kconfig b/drivers/power/regulator/Kconfig index 54ce188..fd3cf35 100644 --- a/drivers/power/regulator/Kconfig +++ b/drivers/power/regulator/Kconfig @@ -14,7 +14,7 @@ config DM_REGULATOR It's important to call the device_bind() with the proper node offset, when binding the regulator devices. The pmic_bind_childs() can be used for this purpose if PMIC I/O driver is implemented or dm_scan_fdt_node() - otherwise. Detailed informations can be found in the header file. + otherwise. Detailed information can be found in the header file.

config DM_REGULATOR_MAX77686 bool "Enable Driver Model for REGULATOR MAX77686" diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c index 07ce286..31ffd44 100644 --- a/drivers/power/regulator/regulator-uclass.c +++ b/drivers/power/regulator/regulator-uclass.c @@ -108,16 +108,19 @@ int regulator_set_mode(struct udevice *dev, int mode) return ops->set_mode(dev, mode); }

-int regulator_by_platname(const char *plat_name, struct udevice **devp) +int regulator_get_by_platname(const char *plat_name, struct udevice **devp) { struct dm_regulator_uclass_platdata *uc_pdata; struct udevice *dev; + int ret;

*devp = NULL;

- for (uclass_find_first_device(UCLASS_REGULATOR, &dev); - dev; - uclass_find_next_device(&dev)) { + for (ret = uclass_find_first_device(UCLASS_REGULATOR, &dev); dev; + ret = uclass_find_next_device(&dev)) { + if (ret) + continue; + uc_pdata = dev_get_uclass_platdata(dev); if (!uc_pdata || strcmp(plat_name, uc_pdata->name)) continue; @@ -130,12 +133,12 @@ int regulator_by_platname(const char *plat_name, struct udevice **devp) return -ENODEV; }

-int regulator_by_devname(const char *devname, struct udevice **devp) +int regulator_get_by_devname(const char *devname, struct udevice **devp) { return uclass_get_device_by_name(UCLASS_REGULATOR, devname, devp); }

-static int setting_failed(int ret, bool verbose, const char *fmt, ...) +static int failed(int ret, bool verbose, const char *fmt, ...) { va_list args; char buf[64]; @@ -157,19 +160,18 @@ static int setting_failed(int ret, bool verbose, const char *fmt, ...) return ret; }

-int regulator_by_platname_autoset_and_enable(const char *platname, - struct udevice **devp, - bool verbose) +int regulator_autoset(const char *platname, + struct udevice **devp, + bool verbose) { struct dm_regulator_uclass_platdata *uc_pdata; struct udevice *dev; - bool v = verbose; int ret;

if (devp) *devp = NULL;

- ret = regulator_by_platname(platname, &dev); + ret = regulator_get_by_platname(platname, &dev); if (ret) { error("Can get the regulator: %s!", platname); return ret; @@ -181,7 +183,10 @@ int regulator_by_platname_autoset_and_enable(const char *platname, return -ENXIO; }

- if (v) + if (!uc_pdata->always_on && !uc_pdata->boot_on) + goto retdev; + + if (verbose) printf("%s@%s: ", dev->name, uc_pdata->name);

/* Those values are optional (-ENODATA if unset) */ @@ -189,7 +194,7 @@ int regulator_by_platname_autoset_and_enable(const char *platname, (uc_pdata->max_uV != -ENODATA) && (uc_pdata->min_uV == uc_pdata->max_uV)) { ret = regulator_set_value(dev, uc_pdata->min_uV); - if (setting_failed(ret, v, "set %d uV", uc_pdata->min_uV)) + if (failed(ret, verbose, "set %d uV", uc_pdata->min_uV)) goto exit; }

@@ -198,49 +203,69 @@ int regulator_by_platname_autoset_and_enable(const char *platname, (uc_pdata->max_uA != -ENODATA) && (uc_pdata->min_uA == uc_pdata->max_uA)) { ret = regulator_set_current(dev, uc_pdata->min_uA); - if (setting_failed(ret, v, "; set %d uA", uc_pdata->min_uA)) + if (failed(ret, verbose, "; set %d uA", uc_pdata->min_uA)) goto exit; }

- if (!uc_pdata->always_on && !uc_pdata->boot_on) - goto retdev; - ret = regulator_set_enable(dev, true); - if (setting_failed(ret, v, "; enabling", uc_pdata->min_uA)) + if (failed(ret, verbose, "; enabling", uc_pdata->min_uA)) goto exit;

retdev: if (devp) *devp = dev; exit: - if (v) + if (verbose) printf("\n"); + return ret; }

-int regulator_by_platname_list_autoset_and_enable(const char *list_platname[], - int list_entries, - struct udevice *list_devp[], - bool verbose) +int regulator_list_autoset(const char *list_platname[], + struct udevice *list_devp[], + bool verbose) { struct udevice *dev; - int i, ret, success = 0; + int error = 0, i = 0, ret;

- for (i = 0; i < list_entries; i++) { + while (list_platname[i]) { ret = regulator_autoset(list_platname[i], &dev, verbose); - if (!ret) - success++; + if (ret & !error) + error = ret; + + if (list_devp) + list_devp[i] = dev; + + i++; + } + + return error; +} + +static bool regulator_name_is_unique(struct udevice *check_dev, + const char *check_name) +{ + struct dm_regulator_uclass_platdata *uc_pdata; + struct udevice *dev; + int check_len = strlen(check_name); + int ret; + int len;

- if (!list_devp) + for (ret = uclass_find_first_device(UCLASS_REGULATOR, &dev); dev; + ret = uclass_find_next_device(&dev)) { + if (ret || dev == check_dev) continue;

- if (ret) - list_devp[i] = NULL; - else - list_devp[i] = dev; + uc_pdata = dev_get_uclass_platdata(dev); + len = strlen(uc_pdata->name); + if (len != check_len) + continue; + + if (!strcmp(uc_pdata->name, check_name)) + return false; }

- return (success != list_entries); + return true; }

static int regulator_post_bind(struct udevice *dev) @@ -248,20 +273,27 @@ static int regulator_post_bind(struct udevice *dev) struct dm_regulator_uclass_platdata *uc_pdata; int offset = dev->of_offset; const void *blob = gd->fdt_blob; + const char *property = "regulator-name";

uc_pdata = dev_get_uclass_platdata(dev); if (!uc_pdata) return -ENXIO;

/* Regulator's mandatory constraint */ - uc_pdata->name = fdt_getprop(blob, offset, "regulator-name", NULL); + uc_pdata->name = fdt_getprop(blob, offset, property, NULL); if (!uc_pdata->name) { debug("%s: dev: %s has no property 'regulator-name'\n", __func__, dev->name); - return -ENXIO; + return -EINVAL; }

- return 0; + if (regulator_name_is_unique(dev, uc_pdata->name)) + return 0; + + error(""%s" of dev: "%s", has nonunique value: "%s"", + property, dev->name, uc_pdata->name); + + return -EINVAL; }

static int regulator_pre_probe(struct udevice *dev) diff --git a/include/power/regulator.h b/include/power/regulator.h index 6916660..03a2cef 100644 --- a/include/power/regulator.h +++ b/include/power/regulator.h @@ -34,7 +34,7 @@ * regulator constraints, like in the example below: * * ldo1 { - * regulator-name = "VDD_MMC_1.8V"; (mandatory for bind) + * regulator-name = "VDD_MMC_1.8V"; (must be unique for proper bind) * regulator-min-microvolt = <1000000>; (optional) * regulator-max-microvolt = <1000000>; (optional) * regulator-min-microamp = <1000>; (optional) @@ -43,19 +43,22 @@ * regulator-boot-on; (optional) * }; * - * Please take a notice, that for the proper operation at least name constraint - * is needed, e.g. for call the device_by_platname(...). + * Note: For the proper operation, at least name constraint is needed, since + * it can be used when calling regulator_get_by_platname(). And the mandatory + * rule for this name is, that it must be globally unique for the single dts. * * Regulator bind: * For each regulator device, the device_bind() should be called with passed * device tree offset. This is required for this uclass's '.post_bind' method, - * which do the scan on the device node, for the 'regulator-name' constraint. + * which does the scan on the device node, for the 'regulator-name' constraint. * If the parent is not a PMIC device, and the child is not bind by function: * 'pmic_bind_childs()', then it's recommended to bind the device by call to * dm_scan_fdt_node() - this is usually done automatically for bus devices, * as a post bind method. + * + * Regulator get: * Having the device's name constraint, we can call regulator_by_platname(), - * to find interesting regulator. Before return, the regulator is probed, + * to find the required regulator. Before return, the regulator is probed, * and the rest of its constraints are put into the device's uclass platform * data, by the uclass regulator '.pre_probe' method. * @@ -201,8 +204,8 @@ struct dm_regulator_ops {

/** * The 'get/set_mode()' function calls should operate on a driver- - * specific mode definitions, which should be found in: - * field 'mode' of struct mode_desc. + * specific mode id definitions, which should be found in: + * field 'id' of struct dm_regulator_mode. * * get/set_mode - get/set operation mode of the given output number * @dev - regulator device @@ -211,7 +214,7 @@ struct dm_regulator_ops { * @return id/0 for get/set on success or negative errno if fail. * Note: * The field 'id' of struct type 'dm_regulator_mode', should be always - * positive number, since the negative is reserved for the error. + * a positive number, since the negative is reserved for the error. */ int (*get_mode)(struct udevice *dev); int (*set_mode)(struct udevice *dev, int mode_id); @@ -278,107 +281,106 @@ bool regulator_get_enable(struct udevice *dev); int regulator_set_enable(struct udevice *dev, bool enable);

/** - * regulator_get_mode: get mode of a given device regulator + * regulator_get_mode: get active operation mode id of a given regulator * * @dev - pointer to the regulator device - * @return - positive mode number on success or -errno val if fails + * @return - positive mode 'id' number on success or -errno val if fails * Note: - * The regulator driver should return one of defined, mode number rather, than - * the raw register value. The struct type 'mode_desc' provides a field 'mode' - * for this purpose and register_value for a raw register value. + * The device can provide an array of operating modes, which is type of struct + * dm_regulator_mode. Each mode has it's own 'id', which should be unique inside + * that array. By calling this function, the driver should return an active mode + * id of the given regulator device. */ int regulator_get_mode(struct udevice *dev);

/** - * regulator_set_mode: set given regulator mode + * regulator_set_mode: set the given regulator's, active mode id * - * @dev - pointer to the regulator device - * @mode - mode type (field 'mode' of struct mode_desc) - * @return - 0 on success or -errno value if fails + * @dev - pointer to the regulator device + * @mode_id - mode id to set ('id' field of struct type dm_regulator_mode) + * @return - 0 on success or -errno value if fails * Note: - * The regulator driver should take one of defined, mode number rather - * than a raw register value. The struct type 'regulator_mode_desc' has - * a mode field for this purpose and register_value for a raw register value. + * The device can provide an array of operating modes, which is type of struct + * dm_regulator_mode. Each mode has it's own 'id', which should be unique inside + * that array. By calling this function, the driver should set the active mode + * of a given regulator to given by "mode_id" argument. */ -int regulator_set_mode(struct udevice *dev, int mode); +int regulator_set_mode(struct udevice *dev, int mode_id);

/** - * regulator_by_platname_autoset_and_enable: setup the regulator given by - * its uclass's platform data '.name'. The setup depends on constraints found - * in device's uclass's platform data (struct dm_regulator_uclass_platdata): + * regulator_autoset: setup the regulator given by its uclass's platform data + * name field. The setup depends on constraints found in device's uclass's + * platform data (struct dm_regulator_uclass_platdata): + * - Enable - will set - if any of: 'always_on' or 'boot_on' is set to true, + * or if both are unset, then the function returns * - Voltage value - will set - if '.min_uV' and '.max_uV' values are equal * - Current limit - will set - if '.min_uA' and '.max_uA' values are equal - * - Enable - will set - if any of: '.always_on' or '.boot_on', is set to true * * The function returns on first encountered error. * * @platname - expected string for dm_regulator_uclass_platdata .name field - * @devp - returned pointer to the regulator device - if non-NULL passed - * @verbose - (true/false) print regulator setup info, or be quiet + * @devp - returned pointer to the regulator device - if non-NULL passed + * @verbose - (true/false) print regulator setup info, or be quiet * @return: 0 on success or negative value of errno. * * The returned 'regulator' device can be used with: * - regulator_get/set_* - * For shorter call name, the below macro regulator_autoset() can be used. */ -int regulator_by_platname_autoset_and_enable(const char *platname, - struct udevice **devp, - bool verbose); - -#define regulator_autoset(platname, devp, verbose) \ - regulator_by_platname_autoset_and_enable(platname, devp, verbose) +int regulator_autoset(const char *platname, + struct udevice **devp, + bool verbose);

/** - * regulator_by_platname_list_autoset_and_enable: setup the regulators given by - * list of its uclass's platform data '.name'. The setup depends on constraints - * found in device's uclass's platform data. The function loops with calls to: - * regulator_by_platname_autoset_and_enable() for each name of list. + * regulator_list_autoset: setup the regulators given by list of their uclass's + * platform data name field. The setup depends on constraints found in device's + * uclass's platform data. The function loops with calls to: + * regulator_autoset() for each name from the list. * * @list_platname - an array of expected strings for .name field of each * regulator's uclass platdata - * @list_entries - number of regulator's name list entries * @list_devp - an array of returned pointers to the successfully setup * regulator devices if non-NULL passed * @verbose - (true/false) print each regulator setup info, or be quiet - * @return 0 on successfully setup of all list entries or 1 otwerwise. + * @return 0 on successfully setup of all list entries, otherwise first error. * * The returned 'regulator' devices can be used with: * - regulator_get/set_* - * For shorter call name, the below macro regulator_list_autoset() can be used. + * + * Note: The list must ends with NULL entry, like in the "platname" list below: + * char *my_regulators[] = { + * "VCC_3.3V", + * "VCC_1.8V", + * NULL, + * }; */ -int regulator_by_platname_list_autoset_and_enable(const char *list_platname[], - int list_entries, - struct udevice *list_devp[], - bool verbose); - -#define regulator_list_autoset(namelist, entries, devlist, verbose) \ - regulator_by_platname_list_autoset_and_enable(namelist, entries, \ - devlist, verbose) +int regulator_list_autoset(const char *list_platname[], + struct udevice *list_devp[], + bool verbose);

/** - * regulator_by_devname: returns the pointer to the pmic regulator device. - * Search by name, found in regulator device's name. + * regulator_get_by_devname: returns the pointer to the pmic regulator device. + * Search by name, found in regulator device's name. * * @devname - expected string for 'dev->name' of regulator device * @devp - returned pointer to the regulator device * @return 0 on success or negative value of errno. * - * The returned 'regulator' device can be used with: + * The returned 'regulator' device is probed and can be used with: * - regulator_get/set_* */ -int regulator_by_devname(const char *devname, struct udevice **devp); +int regulator_get_by_devname(const char *devname, struct udevice **devp);

/** - * regulator_by_platname: returns the pointer to the pmic regulator device. - * Search by name, found in regulator uclass platdata. + * regulator_get_by_platname: returns the pointer to the pmic regulator device. + * Search by name, found in regulator uclass platdata. * * @platname - expected string for uc_pdata->name of regulator uclass platdata * @devp - returned pointer to the regulator device * @return 0 on success or negative value of errno. * - * The returned 'regulator' device can be used with: + * The returned 'regulator' device is probed and can be used with: * - regulator_get/set_* */ -int regulator_by_platname(const char *platname, struct udevice **devp); +int regulator_get_by_platname(const char *platname, struct udevice **devp);

#endif /* _INCLUDE_REGULATOR_H_ */

This commit cleanups the use of function: failed(). The new function name is: failure(), and it is used for print errno and the errno-related message only.

The second change is choosing PMIC device by it's name, instead of seq number. Thanks to this change, for set the current device, call of pmic_get() is enough.

Signed-off-by: Przemyslaw Marczak p.marczak@samsung.com Acked-by: Simon Glass sjg@chromium.org Tested on sandbox: Tested-by: Simon Glass sjg@chromium.org --- Changes V2: - none --- common/cmd_pmic.c | 131 +++++++++++++++++++++++------------------------------- 1 file changed, 55 insertions(+), 76 deletions(-)

diff --git a/common/cmd_pmic.c b/common/cmd_pmic.c index bd88d68..970767c 100644 --- a/common/cmd_pmic.c +++ b/common/cmd_pmic.c @@ -10,75 +10,41 @@ #include <dm/uclass-internal.h> #include <power/pmic.h>

-#define LIMIT_SEQ 3 -#define LIMIT_DEVNAME 20 +#define LIMIT_DEV 32 +#define LIMIT_PARENT 20

static struct udevice *currdev;

-static int failed(const char *getset, const char *thing, - const char *for_dev, int ret) +static int failure(int ret) { - printf("Can't %s %s %s.\nError: %d (%s)\n", getset, thing, for_dev, - ret, errno_str(ret)); - return CMD_RET_FAILURE; -} - -static int pmic_dev_get(bool list_only, int get_seq, struct udevice **devp) -{ - struct udevice *dev; - int ret; - - if (devp) - *devp = NULL; - - for (ret = uclass_first_device(UCLASS_PMIC, &dev); dev; - ret = uclass_next_device(&dev)) { - if (list_only) { - printf("|%*d | %-*.*s| %-*.*s| %s @ %d\n", - LIMIT_SEQ, dev->seq, - LIMIT_DEVNAME, LIMIT_DEVNAME, dev->name, - LIMIT_DEVNAME, LIMIT_DEVNAME, dev->parent->name, - dev_get_uclass_name(dev->parent), - dev->parent->seq); - continue; - } + printf("Error: %d (%s)\n", ret, errno_str(ret));

- if (dev->seq == get_seq) { - if (devp) - *devp = dev; - else - return -EINVAL; - - return 0; - } - } - - if (list_only) - return ret; - - return -ENODEV; + return CMD_RET_FAILURE; }

static int do_dev(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { - int seq, ret = -ENODEV; + char *name; + int ret = -ENODEV;

switch (argc) { case 2: - seq = simple_strtoul(argv[1], NULL, 0); - ret = uclass_get_device_by_seq(UCLASS_PMIC, seq, &currdev); - if (ret && (ret = pmic_dev_get(false, seq, &currdev))) - goto failed; + name = argv[1]; + ret = pmic_get(name, &currdev); + if (ret) { + printf("Can't get PMIC: %s!\n", name); + return failure(ret); + } case 1: - if (!currdev) - goto failed; + if (!currdev) { + printf("PMIC device is not set!\n\n"); + return CMD_RET_USAGE; + }

printf("dev: %d @ %s\n", currdev->seq, currdev->name); }

return CMD_RET_SUCCESS; -failed: - return failed("get", "the", "device", ret); }

static int do_list(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) @@ -86,18 +52,19 @@ static int do_list(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) struct udevice *dev; int ret;

- printf("|%*s | %-*.*s| %-*.*s| %s @ %s\n", - LIMIT_SEQ, "Seq", - LIMIT_DEVNAME, LIMIT_DEVNAME, "Name", - LIMIT_DEVNAME, LIMIT_DEVNAME, "Parent name", + printf("| %-*.*s| %-*.*s| %s @ %s\n", + LIMIT_DEV, LIMIT_DEV, "Name", + LIMIT_PARENT, LIMIT_PARENT, "Parent name", "Parent uclass", "seq");

for (ret = uclass_first_device(UCLASS_PMIC, &dev); dev; ret = uclass_next_device(&dev)) { - printf("|%*d | %-*.*s| %-*.*s| %s @ %d\n", - LIMIT_SEQ, dev->seq, - LIMIT_DEVNAME, LIMIT_DEVNAME, dev->name, - LIMIT_DEVNAME, LIMIT_DEVNAME, dev->parent->name, + if (ret) + continue; + + printf("| %-*.*s| %-*.*s| %s @ %d\n", + LIMIT_DEV, LIMIT_DEV, dev->name, + LIMIT_PARENT, LIMIT_PARENT, dev->parent->name, dev_get_uclass_name(dev->parent), dev->parent->seq); }

@@ -114,8 +81,10 @@ static int do_dump(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) uint reg; int ret;

- if (!currdev) - return failed("get", "current", "device", -ENODEV); + if (!currdev) { + printf("First, set the PMIC device!\n"); + return CMD_RET_USAGE; + }

dev = currdev;

@@ -123,8 +92,10 @@ static int do_dump(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])

for (reg = 0; reg < pmic_reg_count(dev); reg++) { ret = pmic_read(dev, reg, &value, 1); - if (ret) - return failed("read", dev->name, "register", ret); + if (ret) { + printf("Can't read register: %d\n", reg); + return failure(ret); + }

if (!(reg % 16)) printf("\n0x%02x: ", reg); @@ -143,8 +114,10 @@ static int do_read(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) uint8_t value; uint reg;

- if (!currdev) - return failed("get", "current", "device", -ENODEV); + if (!currdev) { + printf("First, set the PMIC device!\n"); + return CMD_RET_USAGE; + }

dev = currdev;

@@ -154,13 +127,15 @@ static int do_read(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) reg = simple_strtoul(argv[1], NULL, 0); regs = pmic_reg_count(dev); if (reg > regs) { - printf("Pmic max reg: %d\n", regs); - return failed("read", "given", "address", -EFAULT); + printf("PMIC max reg: %d\n", regs); + return failure(-EFAULT); }

ret = pmic_read(dev, reg, &value, 1); - if (ret) - return failed("read", dev->name, "register", ret); + if (ret) { + printf("Can't read PMIC register: %d!\n", reg); + return failure(ret); + }

printf("0x%02x: 0x%2.2x\n", reg, value);

@@ -174,8 +149,10 @@ static int do_write(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) uint8_t value; uint reg;

- if (!currdev) - return failed("get", "current", "device", -ENODEV); + if (!currdev) { + printf("First, set the PMIC device!\n"); + return CMD_RET_USAGE; + }

dev = currdev;

@@ -185,15 +162,17 @@ static int do_write(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) reg = simple_strtoul(argv[1], NULL, 0); regs = pmic_reg_count(dev); if (reg > regs) { - printf("Pmic max reg: %d\n", regs); - return failed("write", "given", "address", -EFAULT); + printf("PMIC max reg: %d\n", regs); + return failure(-EFAULT); }

value = simple_strtoul(argv[2], NULL, 0);

ret = pmic_write(dev, reg, &value, 1); - if (ret) - return failed("write", dev->name, "register", ret); + if (ret) { + printf("Can't write PMIC register: %d!\n", reg); + return failure(ret); + }

return CMD_RET_SUCCESS; } @@ -224,7 +203,7 @@ static int do_pmic(cmd_tbl_t *cmdtp, int flag, int argc, U_BOOT_CMD(pmic, CONFIG_SYS_MAXARGS, 1, do_pmic, " operations", "list - list pmic devices\n" - "pmic dev [id] - show or [set] operating pmic device\n" + "pmic dev [name] - show or [set] operating PMIC device\n" "pmic dump - dump registers\n" "pmic read address - read byte of register at address\n" "pmic write address - write byte to register at address\n"

This commit cleanups the regulator command. The first change, is adjusting "regulator dev" command to use "regulator-name" constraint, for setting the operating device. Thanks to this, the regulator_get() function is removed.

This also updates do_list() function, with loop over uclass_find_* function calls, to prevent probe of all listed regulators.

This also cleanups the printing in command.

Signed-off-by: Przemyslaw Marczak p.marczak@samsung.com Acked-by: Simon Glass sjg@chromium.org Tested on sandbox: Tested-by: Simon Glass sjg@chromium.org --- Changes V2: - none --- common/cmd_regulator.c | 239 +++++++++++++++++++++++++------------------------ 1 file changed, 122 insertions(+), 117 deletions(-)

diff --git a/common/cmd_regulator.c b/common/cmd_regulator.c index b1b9e87..6149d1e 100644 --- a/common/cmd_regulator.c +++ b/common/cmd_regulator.c @@ -10,98 +10,70 @@ #include <dm/uclass-internal.h> #include <power/regulator.h>

-#define LIMIT_SEQ 3 #define LIMIT_DEVNAME 20 -#define LIMIT_OFNAME 20 -#define LIMIT_INFO 16 +#define LIMIT_OFNAME 32 +#define LIMIT_INFO 18

static struct udevice *currdev;

-static int failed(const char *getset, const char *thing, - const char *for_dev, int ret) +static int failure(int ret) { - printf("Can't %s %s %s.\nError: %d (%s)\n", getset, thing, for_dev, - ret, errno_str(ret)); - return CMD_RET_FAILURE; -} - -static int regulator_get(bool list_only, int get_seq, struct udevice **devp) -{ - struct dm_regulator_uclass_platdata *uc_pdata; - struct udevice *dev; - int ret; - - if (devp) - *devp = NULL; - - for (ret = uclass_first_device(UCLASS_REGULATOR, &dev); dev; - ret = uclass_next_device(&dev)) { - if (list_only) { - uc_pdata = dev_get_uclass_platdata(dev); - printf("|%*d | %*.*s @ %-*.*s| %s @ %s\n", - LIMIT_SEQ, dev->seq, - LIMIT_DEVNAME, LIMIT_DEVNAME, dev->name, - LIMIT_OFNAME, LIMIT_OFNAME, uc_pdata->name, - dev->parent->name, - dev_get_uclass_name(dev->parent)); - continue; - } - - if (dev->seq == get_seq) { - if (devp) - *devp = dev; - else - return -EINVAL; - - return 0; - } - } - - if (list_only) - return ret; + printf("Error: %d (%s)\n", ret, errno_str(ret));

- return -ENODEV; + return CMD_RET_FAILURE; }

static int do_dev(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { struct dm_regulator_uclass_platdata *uc_pdata; - int seq, ret = -ENXIO; + const char *name; + int ret = -ENXIO;

switch (argc) { case 2: - seq = simple_strtoul(argv[1], NULL, 0); - ret = uclass_get_device_by_seq(UCLASS_REGULATOR, seq, &currdev); - if (ret && (ret = regulator_get(false, seq, &currdev))) - goto failed; + name = argv[1]; + ret = regulator_get_by_platname(name, &currdev); + if (ret) { + printf("Can't get the regulator: %s!\n", name); + return failure(ret); + } case 1: + if (!currdev) { + printf("Regulator device is not set!\n\n"); + return CMD_RET_USAGE; + } + uc_pdata = dev_get_uclass_platdata(currdev); - if (!uc_pdata) - goto failed; + if (!uc_pdata) { + printf("%s: no regulator platform data!\n", currdev->name); + return failure(ret); + }

- printf("dev: %d @ %s\n", currdev->seq, uc_pdata->name); + printf("dev: %s @ %s\n", uc_pdata->name, currdev->name); }

return CMD_RET_SUCCESS; -failed: - return failed("get", "the", "device", ret); }

-static int get_curr_dev_and_pl(struct udevice **devp, - struct dm_regulator_uclass_platdata **uc_pdata, - bool allow_type_fixed) +static int curr_dev_and_platdata(struct udevice **devp, + struct dm_regulator_uclass_platdata **uc_pdata, + bool allow_type_fixed) { *devp = NULL; *uc_pdata = NULL;

- if (!currdev) - return failed("get", "current", "device", -ENODEV); + if (!currdev) { + printf("First, set the regulator device!\n"); + return CMD_RET_FAILURE; + }

*devp = currdev;

*uc_pdata = dev_get_uclass_platdata(*devp); - if (!*uc_pdata) - return failed("get", "regulator", "platdata", -ENXIO); + if (!*uc_pdata) { + error("Regulator: %s - missing platform data!", currdev->name); + return CMD_RET_FAILURE; + }

if (!allow_type_fixed && (*uc_pdata)->type == REGULATOR_TYPE_FIXED) { printf("Operation not allowed for fixed regulator!\n"); @@ -113,19 +85,28 @@ static int get_curr_dev_and_pl(struct udevice **devp,

static int do_list(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { + struct dm_regulator_uclass_platdata *uc_pdata; + struct udevice *dev; int ret;

- printf("|%*s | %*.*s @ %-*.*s| %s @ %s\n", - LIMIT_SEQ, "Seq", - LIMIT_DEVNAME, LIMIT_DEVNAME, "Name", - LIMIT_OFNAME, LIMIT_OFNAME, "fdtname", - "Parent", "uclass"); + printf("| %-*.*s| %-*.*s| %s\n", + LIMIT_DEVNAME, LIMIT_DEVNAME, "Device", + LIMIT_OFNAME, LIMIT_OFNAME, "regulator-name", + "Parent");

- ret = regulator_get(true, 0, NULL); - if (ret) - return CMD_RET_FAILURE; + for (ret = uclass_find_first_device(UCLASS_REGULATOR, &dev); dev; + ret = uclass_find_next_device(&dev)) { + if (ret) + continue;

- return CMD_RET_SUCCESS; + uc_pdata = dev_get_uclass_platdata(dev); + printf("| %-*.*s| %-*.*s| %s\n", + LIMIT_DEVNAME, LIMIT_DEVNAME, dev->name, + LIMIT_OFNAME, LIMIT_OFNAME, uc_pdata->name, + dev->parent->name); + } + + return ret; }

static int constraint(const char *name, int val, const char *val_name) @@ -167,17 +148,18 @@ static int do_info(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) int ret; int i;

- ret = get_curr_dev_and_pl(&dev, &uc_pdata, true); + ret = curr_dev_and_platdata(&dev, &uc_pdata, true); if (ret) return ret;

parent_uc = dev_get_uclass_name(dev->parent);

- printf("Uclass regulator dev %d info:\n", dev->seq); - printf("%-*s %s @ %s\n%-*s %s\n%-*s %s\n%-*s\n", - LIMIT_INFO, "* parent:", dev->parent->name, parent_uc, - LIMIT_INFO, "* dev name:", dev->name, - LIMIT_INFO, "* fdt name:", uc_pdata->name, + printf("%s\n%-*s %s\n%-*s %s\n%-*s %s\n%-*s %s\n%-*s\n", + "Regulator info:", + LIMIT_INFO, "* regulator-name:", uc_pdata->name, + LIMIT_INFO, "* device name:", dev->name, + LIMIT_INFO, "* parent name:", dev->parent->name, + LIMIT_INFO, "* parent uclass:", parent_uc, LIMIT_INFO, "* constraints:");

constraint(" - min uV:", uc_pdata->min_uV, NULL); @@ -206,10 +188,12 @@ static int do_status(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) struct udevice *dev; bool enabled;

- ret = get_curr_dev_and_pl(&dev, &uc_pdata, true); + ret = curr_dev_and_platdata(&dev, &uc_pdata, true); if (ret) return ret;

+ printf("Regulator %s status:\n", uc_pdata->name); + enabled = regulator_get_enable(dev); constraint(" * enable:", enabled, enabled ? "true" : "false");

@@ -234,16 +218,19 @@ static int do_value(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) int force; int ret;

- ret = get_curr_dev_and_pl(&dev, &uc_pdata, argc == 1); + ret = curr_dev_and_platdata(&dev, &uc_pdata, argc == 1); if (ret) return ret;

if (argc == 1) { - value = regulator_get_value(dev); - if (value < 0) - return failed("get", uc_pdata->name, "voltage", value); + ret = regulator_get_value(dev); + if (ret < 0) { + printf("Regulator: %s - can't get the Voltage!\n", + uc_pdata->name); + return failure(ret); + }

- printf("%d uV\n", value); + printf("%d uV\n", ret); return CMD_RET_SUCCESS; }

@@ -259,8 +246,11 @@ static int do_value(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) }

ret = regulator_set_value(dev, value); - if (ret) - return failed("set", uc_pdata->name, "voltage value", ret); + if (ret) { + printf("Regulator: %s - can't set the Voltage!\n", + uc_pdata->name); + return failure(ret); + }

return CMD_RET_SUCCESS; } @@ -272,16 +262,19 @@ static int do_current(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) int current; int ret;

- ret = get_curr_dev_and_pl(&dev, &uc_pdata, argc == 1); + ret = curr_dev_and_platdata(&dev, &uc_pdata, argc == 1); if (ret) return ret;

if (argc == 1) { - current = regulator_get_current(dev); - if (current < 0) - return failed("get", uc_pdata->name, "current", current); + ret = regulator_get_current(dev); + if (ret < 0) { + printf("Regulator: %s - can't get the Current!\n", + uc_pdata->name); + return failure(ret); + }

- printf("%d uA\n", current); + printf("%d uA\n", ret); return CMD_RET_SUCCESS; }

@@ -292,8 +285,11 @@ static int do_current(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) }

ret = regulator_set_current(dev, current); - if (ret) - return failed("set", uc_pdata->name, "current value", ret); + if (ret) { + printf("Regulator: %s - can't set the Current!\n", + uc_pdata->name); + return failure(ret); + }

return CMD_RET_SUCCESS; } @@ -302,28 +298,33 @@ static int do_mode(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { struct udevice *dev; struct dm_regulator_uclass_platdata *uc_pdata; - int new_mode; int mode; int ret;

- ret = get_curr_dev_and_pl(&dev, &uc_pdata, false); + ret = curr_dev_and_platdata(&dev, &uc_pdata, false); if (ret) return ret;

if (argc == 1) { - mode = regulator_get_mode(dev); - if (mode < 0) - return failed("get", uc_pdata->name, "mode", mode); + ret = regulator_get_mode(dev); + if (ret < 0) { + printf("Regulator: %s - can't get the operation mode!\n", + uc_pdata->name); + return failure(ret); + }

- printf("mode id: %d\n", mode); + printf("mode id: %d\n", ret); return CMD_RET_SUCCESS; }

- new_mode = simple_strtoul(argv[1], NULL, 0); + mode = simple_strtoul(argv[1], NULL, 0);

- ret = regulator_set_mode(dev, new_mode); - if (ret) - return failed("set", uc_pdata->name, "mode", ret); + ret = regulator_set_mode(dev, mode); + if (ret) { + printf("Regulator: %s - can't set the operation mode!\n", + uc_pdata->name); + return failure(ret); + }

return CMD_RET_SUCCESS; } @@ -334,13 +335,15 @@ static int do_enable(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) struct dm_regulator_uclass_platdata *uc_pdata; int ret;

- ret = get_curr_dev_and_pl(&dev, &uc_pdata, true); + ret = curr_dev_and_platdata(&dev, &uc_pdata, true); if (ret) return ret;

ret = regulator_set_enable(dev, true); - if (ret) - return failed("enable", "regulator", uc_pdata->name, ret); + if (ret) { + printf("Regulator: %s - can't enable!\n", uc_pdata->name); + return failure(ret); + }

return CMD_RET_SUCCESS; } @@ -351,13 +354,15 @@ static int do_disable(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) struct dm_regulator_uclass_platdata *uc_pdata; int ret;

- ret = get_curr_dev_and_pl(&dev, &uc_pdata, true); + ret = curr_dev_and_platdata(&dev, &uc_pdata, true); if (ret) return ret;

ret = regulator_set_enable(dev, false); - if (ret) - return failed("disable", "regulator", uc_pdata->name, ret); + if (ret) { + printf("Regulator: %s - can't disable!\n", uc_pdata->name); + return failure(ret); + }

return CMD_RET_SUCCESS; } @@ -391,13 +396,13 @@ static int do_regulator(cmd_tbl_t *cmdtp, int flag, int argc,

U_BOOT_CMD(regulator, CONFIG_SYS_MAXARGS, 1, do_regulator, "uclass operations", - "list - list UCLASS regulator devices\n" - "regulator dev [id] - show or [set] operating regulator device\n" - "regulator [info] - print constraints info\n" - "regulator [status] - print operating status\n" - "regulator [value] [-f] - print/[set] voltage value [uV] (force)\n" - "regulator [current] - print/[set] current value [uA]\n" - "regulator [mode_id] - print/[set] operating mode id\n" - "regulator [enable] - enable the regulator output\n" - "regulator [disable] - disable the regulator output\n" + "list - list UCLASS regulator devices\n" + "regulator dev [regulator-name] - show/[set] operating regulator device\n" + "regulator info - print constraints info\n" + "regulator status - print operating status\n" + "regulator value [val] [-f] - print/[set] voltage value [uV] (force)\n" + "regulator current [val] - print/[set] current value [uA]\n" + "regulator mode [id] - print/[set] operating mode id\n" + "regulator enable - enable the regulator output\n" + "regulator disable - disable the regulator output\n" );

This commit cleanups the PMIC framework documentation.

Signed-off-by: Przemyslaw Marczak p.marczak@samsung.com Acked-by: Simon Glass sjg@chromium.org Tested on sandbox: Tested-by: Simon Glass sjg@chromium.org --- Changes V2: - none --- doc/driver-model/pmic-framework.txt | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/doc/driver-model/pmic-framework.txt b/doc/driver-model/pmic-framework.txt index cc82236..95b1a66 100644 --- a/doc/driver-model/pmic-framework.txt +++ b/doc/driver-model/pmic-framework.txt @@ -79,7 +79,7 @@ use pmic read/write directly.

3. Pmic uclass ============== -The basic informations: +The basic information: * Uclass: 'UCLASS_PMIC' * Header: 'include/power/pmic.h' * Core: 'drivers/power/pmic/pmic-uclass.c' @@ -88,14 +88,13 @@ The basic informations: config: 'CONFIG_CMD_PMIC' * Example: 'drivers/power/pmic/max77686.c'

-This is still under the construction. So for the API description, please refer -to the header file. +For detailed API description, please refer to the header file.

As an example of the pmic driver, please refer to the MAX77686 driver.

-Please pay attention for the driver's '.bind' method. Exactly the function call: -'pmic_bind_childs()', which is used to bind the regulators by using the array of -regulator's node, compatible prefixes. +Please pay attention for the driver's bind() method. Exactly the function call: +'pmic_bind_children()', which is used to bind the regulators by using the array +of regulator's node, compatible prefixes.

The 'pmic; command also supports the new API. So the pmic command can be enabled by adding CONFIG_CMD_PMIC. @@ -108,11 +107,11 @@ The new pmic command allows to: This command can use only UCLASS_PMIC devices, since this uclass is designed for pmic I/O operations only.

-For more informations, please refer to the file: 'common/cmd_pmic.c'. +For more information, please refer to the core file.

4. Regulator uclass =================== -The basic informations: +The basic information: * Uclass: 'UCLASS_REGULATOR' * Header: 'include/power/regulator.h' * Core: 'drivers/power/regulator/regulator-uclass.c' @@ -125,8 +124,7 @@ The basic informations: * Example: 'drivers/power/regulator/fixed.c' config" 'CONFIG_DM_REGULATOR_FIXED'

-This is still under the construction. So for the API description, please refer -to the header file. +For detailed API description, please refer to the header file.

For the example regulator driver, please refer to the MAX77686 regulator driver, but this driver can't operate without pmic's example driver, which provides an @@ -139,4 +137,4 @@ The 'regulator' command also supports the new API. The command allow: - choose the current device (like the mmc command) - do all regulator-specific operations

-For more informations, please refer to the file: 'common/cmd_regulator.c' +For more information, please refer to the command file.

The function get_emul() in sandbox i2c bus driver, always returns first child as i2c emul device. This may only work for i2c devices with a single child, which is an only i2c emul device.

In case when i2c device has more than one child (e.g. PMIC), and one is i2c emul, then the function should search it by check uclass id for each child. This patch add this change to the get_emul().

Signed-off-by: Przemyslaw Marczak p.marczak@samsung.com Cc: Simon Glass sjg@chromium.org Acked-by: Simon Glass sjg@chromium.org Tested on sandbox: Tested-by: Simon Glass sjg@chromium.org --- Changes V2: - none --- drivers/i2c/sandbox_i2c.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/sandbox_i2c.c b/drivers/i2c/sandbox_i2c.c index d6adc0f..d4b543d 100644 --- a/drivers/i2c/sandbox_i2c.c +++ b/drivers/i2c/sandbox_i2c.c @@ -26,6 +26,7 @@ static int get_emul(struct udevice *dev, struct udevice **devp, struct dm_i2c_ops **opsp) { struct dm_i2c_chip *plat; + struct udevice *child; int ret;

*devp = NULL; @@ -37,9 +38,22 @@ static int get_emul(struct udevice *dev, struct udevice **devp, if (ret) return ret;

- ret = device_get_child(dev, 0, &plat->emul); - if (ret) - return ret; + for (device_find_first_child(dev, &child); child; + device_find_next_child(&child)) { + if (device_get_uclass_id(child) != UCLASS_I2C_EMUL) + continue; + + ret = device_probe(child); + if (ret) + return ret; + + break; + } + + if (child) + plat->emul = child; + else + return -ENODEV; } *devp = plat->emul; *opsp = i2c_get_ops(plat->emul);

This commit adds emulation of sandbox PMIC device, which includes: - PMIC I2C emulation driver - PMIC I/O driver (UCLASS_PMIC) - PMIC regulator driver (UCLASS_REGULATOR)

The sandbox PMIC has 12 significant registers and 4 as padding to 16 bytes, which allows using 'i2c md' command with the default count (16).

The sandbox PMIC provides regulators: - 2x BUCK - 2x LDO

Each, with adjustable output: - Enable state - Voltage - Current limit (LDO1/BUCK1 only) - Operation mode (different for BUCK and LDO)

Each attribute has it's own register, beside the enable state, which depends on operation mode.

The header file: sandbox_pmic.h includes PMIC's default register values, which are set on i2c pmic emul driver's probe() method.

Signed-off-by: Przemyslaw Marczak p.marczak@samsung.com Acked-by: Simon Glass sjg@chromium.org Tested on sandbox: Tested-by: Simon Glass sjg@chromium.org --- Changes V2: - pmic emul driver: get the default register settings from fdt - cleanup of sandbox_pmic.h --- doc/device-tree-bindings/pmic/sandbox.txt | 35 +++ doc/device-tree-bindings/regulator/sandbox.txt | 45 ++++ drivers/power/pmic/Kconfig | 25 ++ drivers/power/pmic/Makefile | 3 +- drivers/power/pmic/i2c_pmic_emul.c | 142 ++++++++++ drivers/power/pmic/sandbox.c | 79 ++++++ drivers/power/regulator/Kconfig | 30 +++ drivers/power/regulator/Makefile | 1 + drivers/power/regulator/sandbox.c | 355 +++++++++++++++++++++++++ include/power/sandbox_pmic.h | 105 ++++++++ 10 files changed, 819 insertions(+), 1 deletion(-) create mode 100644 doc/device-tree-bindings/pmic/sandbox.txt create mode 100644 doc/device-tree-bindings/regulator/sandbox.txt create mode 100644 drivers/power/pmic/i2c_pmic_emul.c create mode 100644 drivers/power/pmic/sandbox.c create mode 100644 drivers/power/regulator/sandbox.c create mode 100644 include/power/sandbox_pmic.h

diff --git a/doc/device-tree-bindings/pmic/sandbox.txt b/doc/device-tree-bindings/pmic/sandbox.txt new file mode 100644 index 0000000..d84c977 --- /dev/null +++ b/doc/device-tree-bindings/pmic/sandbox.txt @@ -0,0 +1,35 @@ +Sandbox pmic + +This device uses two drivers: +- drivers/power/pmic/sandbox.c (for parent device) +- drivers/power/regulator/sandbox.c (for child regulators) + +This file describes the binding info for the PMIC driver. + +To bind the regulators, please read the regulator binding info: +- doc/device-tree-bindings/regulator/sandbox.txt + +Required PMIC node properties: +- compatible: "sandbox,pmic" +- reg = 0x40 + +Required PMIC's "emul" subnode, with property: +- compatible: "sandbox,i2c-pmic" + +With the above properties, the pmic device can be used for read/write only. +To bind each regulator, the optional regulator subnodes should exists. + +Optional subnodes: +- ldo/buck subnodes of each device's regulator (see regulator binding info) + +Example: + +sandbox_pmic { + compatible = "sandbox,pmic"; + reg = <0x40>; + + /* Mandatory for I/O */ + emul { + compatible = "sandbox,i2c-pmic"; + }; +}; diff --git a/doc/device-tree-bindings/regulator/sandbox.txt b/doc/device-tree-bindings/regulator/sandbox.txt new file mode 100644 index 0000000..d70494c --- /dev/null +++ b/doc/device-tree-bindings/regulator/sandbox.txt @@ -0,0 +1,45 @@ +Sandbox, PMIC regulators + +This device uses two drivers: +- drivers/power/pmic/sandbox.c (as parent I/O device) +- drivers/power/regulator/sandbox.c (for child regulators) + +This file describes the binding info for the REGULATOR driver. + +First, please read the binding info for the PMIC: +- doc/device-tree-bindings/pmic/sandbox.txt + +Required subnodes: +- ldoN { }; +- buckN { }; + +The sandbox PMIC can support: ldo1, ldo2, buck1, buck2. + +For each PMIC's regulator subnode, there is one required property: +- regulator-name: used for regulator uclass platform data '.name' + +Optional: +- regulator-min-microvolt: minimum allowed Voltage to set +- regulator-max-microvolt: minimum allowed Voltage to set +- regulator-min-microamps: minimum allowed Current limit to set (LDO1/BUCK1) +- regulator-max-microamps: minimum allowed Current limit to set (LDO1/BUCK1) +- regulator-always-on: regulator should be never disabled +- regulator-boot-on: regulator should be enabled by the bootloader + +Example PMIC's regulator subnodes: + +ldo1 { + regulator-name = "VDD_1.0V"; + regulator-min-microvolt = <1000000>; + regulator-max-microvolt = <1200000>; + regulator-min-microamps = <100000>; + regulator-max-microamps = <400000>; + regulator-always-on; +}; + +buck2 { + regulator-name = "VDD_1.8V"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-boot-on; +}; diff --git a/drivers/power/pmic/Kconfig b/drivers/power/pmic/Kconfig index d99d9e3..164f421 100644 --- a/drivers/power/pmic/Kconfig +++ b/drivers/power/pmic/Kconfig @@ -16,3 +16,28 @@ config DM_PMIC_MAX77686 ---help--- This config enables implementation of driver-model pmic uclass features for PMIC MAX77686. The driver implements read/write operations. + +config DM_PMIC_SANDBOX + bool "Enable Driver Model for emulated Sandbox PMIC " + depends on DM_PMIC + ---help--- + Enable the driver for Sandbox PMIC emulation. The emulated PMIC device + depends on two drivers: + - sandbox PMIC I/O driver - implements dm pmic operations + - sandbox PMIC i2c emul driver - emulates the PMIC's I2C transmission + + A detailed information can be found in header: '<power/sandbox_pmic.h>' + + The Sandbox PMIC info: + * I/O interface: + - I2C chip address: 0x40 + - first register address: 0x0 + - register count: 0x10 + * Adjustable outputs: + - 2x LDO + - 2x BUCK + - Each, with a different operating conditions (header). + * Reset values: + - set by i2c emul driver's probe() (defaults in header) + + Driver binding info: doc/device-tree-bindings/pmic/sandbox.txt diff --git a/drivers/power/pmic/Makefile b/drivers/power/pmic/Makefile index 8cb993d..ae86f04 100644 --- a/drivers/power/pmic/Makefile +++ b/drivers/power/pmic/Makefile @@ -6,12 +6,13 @@ #

obj-$(CONFIG_DM_PMIC) += pmic-uclass.o +obj-$(CONFIG_DM_PMIC_MAX77686) += max77686.o +obj-$(CONFIG_DM_PMIC_SANDBOX) += sandbox.o i2c_pmic_emul.o obj-$(CONFIG_POWER_LTC3676) += pmic_ltc3676.o obj-$(CONFIG_POWER_MAX8998) += pmic_max8998.o obj-$(CONFIG_POWER_MAX8997) += pmic_max8997.o obj-$(CONFIG_POWER_MUIC_MAX8997) += muic_max8997.o obj-$(CONFIG_POWER_MAX77686) += pmic_max77686.o -obj-$(CONFIG_DM_PMIC_MAX77686) += max77686.o obj-$(CONFIG_POWER_PFUZE100) += pmic_pfuze100.o obj-$(CONFIG_POWER_TPS65090_I2C) += pmic_tps65090.o obj-$(CONFIG_POWER_TPS65090_EC) += pmic_tps65090_ec.o diff --git a/drivers/power/pmic/i2c_pmic_emul.c b/drivers/power/pmic/i2c_pmic_emul.c new file mode 100644 index 0000000..aeab5c9 --- /dev/null +++ b/drivers/power/pmic/i2c_pmic_emul.c @@ -0,0 +1,142 @@ +/* + * Copyright (C) 2015 Samsung Electronics + * Przemyslaw Marczak p.marczak@samsung.com + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <fdtdec.h> +#include <errno.h> +#include <dm.h> +#include <i2c.h> +#include <power/pmic.h> +#include <power/sandbox_pmic.h> + +DECLARE_GLOBAL_DATA_PTR; + +/** + * struct sandbox_i2c_pmic_plat_data - platform data for the PMIC + * + * @rw_reg: PMICs register of the chip I/O transaction + * @reg: PMICs registers array + */ +struct sandbox_i2c_pmic_plat_data { + u8 rw_reg; + u8 reg[SANDBOX_PMIC_REG_COUNT]; +}; + +static int sandbox_i2c_pmic_read_data(struct udevice *emul, uchar chip, + uchar *buffer, int len) +{ + struct sandbox_i2c_pmic_plat_data *plat = dev_get_platdata(emul); + + if (plat->rw_reg + len > SANDBOX_PMIC_REG_COUNT) { + error("Request exceeds PMIC register range! Max register: %#x", + SANDBOX_PMIC_REG_COUNT); + return -EFAULT; + } + + debug("Read PMIC: %#x at register: %#x count: %d\n", + (unsigned)chip & 0xff, plat->rw_reg, len); + + memcpy(buffer, &plat->reg[plat->rw_reg], len); + + return 0; +} + +static int sandbox_i2c_pmic_write_data(struct udevice *emul, uchar chip, + uchar *buffer, int len, + bool next_is_read) +{ + struct sandbox_i2c_pmic_plat_data *plat = dev_get_platdata(emul); + + /* Probe only */ + if (!len) + return 0; + + /* Set PMIC register for I/O */ + plat->rw_reg = *buffer; + + debug("Write PMIC: %#x at register: %#x count: %d\n", + (unsigned)chip & 0xff, plat->rw_reg, len); + + /* For read operation, set (write) only chip reg */ + if (next_is_read) + return 0; + + buffer++; + len--; + + if (plat->rw_reg + len > SANDBOX_PMIC_REG_COUNT) { + error("Request exceeds PMIC register range! Max register: %#x", + SANDBOX_PMIC_REG_COUNT); + } + + memcpy(&plat->reg[plat->rw_reg], buffer, len); + + return 0; +} + +static int sandbox_i2c_pmic_xfer(struct udevice *emul, struct i2c_msg *msg, + int nmsgs) +{ + int ret = 0; + + for (; nmsgs > 0; nmsgs--, msg++) { + bool next_is_read = nmsgs > 1 && (msg[1].flags & I2C_M_RD); + if (msg->flags & I2C_M_RD) { + ret = sandbox_i2c_pmic_read_data(emul, msg->addr, + msg->buf, msg->len); + } else { + ret = sandbox_i2c_pmic_write_data(emul, msg->addr, + msg->buf, msg->len, + next_is_read); + } + + if (ret) + break; + } + + return ret; +} + +static int sandbox_i2c_pmic_ofdata_to_platdata(struct udevice *emul) +{ + struct sandbox_i2c_pmic_plat_data *plat = dev_get_platdata(emul); + const u8 *reg_defaults; + + debug("%s:%d Setting PMIC default registers\n", __func__, __LINE__); + + reg_defaults = fdtdec_locate_byte_array(gd->fdt_blob, emul->of_offset, + "reg-defaults", + SANDBOX_PMIC_REG_COUNT); + + if (!reg_defaults) { + error("Property "reg-defaults" not found for device: %s!", + emul->name); + return -EINVAL; + } + + memcpy(&plat->reg, reg_defaults, SANDBOX_PMIC_REG_COUNT); + + return 0; +} + +struct dm_i2c_ops sandbox_i2c_pmic_emul_ops = { + .xfer = sandbox_i2c_pmic_xfer, +}; + +static const struct udevice_id sandbox_i2c_pmic_ids[] = { + { .compatible = "sandbox,i2c-pmic" }, + { } +}; + +U_BOOT_DRIVER(sandbox_i2c_pmic_emul) = { + .name = "sandbox_i2c_pmic_emul", + .id = UCLASS_I2C_EMUL, + .of_match = sandbox_i2c_pmic_ids, + .ofdata_to_platdata = sandbox_i2c_pmic_ofdata_to_platdata, + .platdata_auto_alloc_size = sizeof(struct sandbox_i2c_pmic_plat_data), + .ops = &sandbox_i2c_pmic_emul_ops, +}; diff --git a/drivers/power/pmic/sandbox.c b/drivers/power/pmic/sandbox.c new file mode 100644 index 0000000..3e56acd --- /dev/null +++ b/drivers/power/pmic/sandbox.c @@ -0,0 +1,79 @@ +/* + * Copyright (C) 2015 Samsung Electronics + * Przemyslaw Marczak p.marczak@samsung.com + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <fdtdec.h> +#include <errno.h> +#include <dm.h> +#include <i2c.h> +#include <power/pmic.h> +#include <power/regulator.h> +#include <power/sandbox_pmic.h> + +DECLARE_GLOBAL_DATA_PTR; + +static const struct pmic_child_info pmic_children_info[] = { + { .prefix = SANDBOX_OF_LDO_PREFIX, .driver = SANDBOX_LDO_DRIVER }, + { .prefix = SANDBOX_OF_BUCK_PREFIX, .driver = SANDBOX_BUCK_DRIVER }, + { }, +}; + +static int sandbox_pmic_reg_count(struct udevice *dev) +{ + return SANDBOX_PMIC_REG_COUNT; +} + +static int sandbox_pmic_write(struct udevice *dev, uint reg, + const uint8_t *buff, int len) +{ + if (dm_i2c_write(dev, reg, buff, len)) { + error("write error to device: %p register: %#x!", dev, reg); + return -EIO; + } + + return 0; +} + +static int sandbox_pmic_read(struct udevice *dev, uint reg, + uint8_t *buff, int len) +{ + if (dm_i2c_read(dev, reg, buff, len)) { + error("read error from device: %p register: %#x!", dev, reg); + return -EIO; + } + + return 0; +} + +static int sandbox_pmic_bind(struct udevice *dev) +{ + if (!pmic_bind_children(dev, dev->of_offset, pmic_children_info)) + error("%s:%d PMIC: %s - no child found!", __func__, __LINE__, + dev->name); + + /* Always return success for this device - allows for PMIC I/O */ + return 0; +} + +static struct dm_pmic_ops sandbox_pmic_ops = { + .reg_count = sandbox_pmic_reg_count, + .read = sandbox_pmic_read, + .write = sandbox_pmic_write, +}; + +static const struct udevice_id sandbox_pmic_ids[] = { + { .compatible = "sandbox,pmic" }, + { } +}; + +U_BOOT_DRIVER(sandbox_pmic) = { + .name = "sandbox_pmic", + .id = UCLASS_PMIC, + .of_match = sandbox_pmic_ids, + .bind = sandbox_pmic_bind, + .ops = &sandbox_pmic_ops, +}; diff --git a/drivers/power/regulator/Kconfig b/drivers/power/regulator/Kconfig index fd3cf35..6289b83 100644 --- a/drivers/power/regulator/Kconfig +++ b/drivers/power/regulator/Kconfig @@ -31,3 +31,33 @@ config DM_REGULATOR_FIXED This config enables implementation of driver-model regulator uclass features for fixed value regulators. The driver implements get/set api for enable and get only for voltage value. + +config DM_REGULATOR_SANDBOX + bool "Enable Driver Model for Sandbox PMIC regulator" + depends on DM_REGULATOR && DM_PMIC_SANDBOX + ---help--- + Enable the regulator driver for emulated Sandbox PMIC. + The emulated PMIC device depends on two drivers: + - sandbox PMIC I/O driver - implements dm pmic operations + - sandbox PMIC regulator driver - implements dm regulator operations + - sandbox PMIC i2c emul driver - emulates the PMIC's I2C transmission + + The regulator driver provides uclass operations for sandbox PMIC's + regulators. The driver implements get/set api for: voltage, current, + operation mode and enable state. + The driver supports LDO and BUCK regulators. + + The Sandbox PMIC info: + * I/O interface: + - I2C chip address: 0x40 + - first register address: 0x0 + - register count: 0x10 + * Adjustable outputs: + - 2x LDO + - 2x BUCK + - Each, with a different operating conditions (header). + * Reset values: + - set by i2c emul driver's probe() (defaults in header) + + A detailed information can be found in header: '<power/sandbox_pmic.h>' + Binding info: 'doc/device-tree-bindings/pmic/max77686.txt' diff --git a/drivers/power/regulator/Makefile b/drivers/power/regulator/Makefile index cc8326d..96aa624 100644 --- a/drivers/power/regulator/Makefile +++ b/drivers/power/regulator/Makefile @@ -8,3 +8,4 @@ obj-$(CONFIG_DM_REGULATOR) += regulator-uclass.o obj-$(CONFIG_DM_REGULATOR_MAX77686) += max77686.o obj-$(CONFIG_DM_REGULATOR_FIXED) += fixed.o +obj-$(CONFIG_DM_REGULATOR_SANDBOX) += sandbox.o diff --git a/drivers/power/regulator/sandbox.c b/drivers/power/regulator/sandbox.c new file mode 100644 index 0000000..2cca579 --- /dev/null +++ b/drivers/power/regulator/sandbox.c @@ -0,0 +1,355 @@ +/* + * Copyright (C) 2015 Samsung Electronics + * Przemyslaw Marczak p.marczak@samsung.com + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <fdtdec.h> +#include <errno.h> +#include <dm.h> +#include <i2c.h> +#include <power/pmic.h> +#include <power/regulator.h> +#include <power/sandbox_pmic.h> + +DECLARE_GLOBAL_DATA_PTR; + +#define MODE(_id, _val, _name) [_id] = { \ + .id = _id, \ + .register_value = _val, \ + .name = _name, \ +} + +#define RANGE(_min, _max, _step) { \ + .min = _min, \ + .max = _max, \ + .step = _step, \ +} + +/* + * struct output_range - helper structure type to define the range of output + * operating values (current/voltage), limited by the PMIC IC design. + * + * @min - minimum value + * @max - maximum value + * @step - step value +*/ +struct output_range { + int min; + int max; + int step; +}; + +/* BUCK: 1,2 - voltage range */ +static struct output_range buck_voltage_range[] = { + RANGE(OUT_BUCK1_UV_MIN, OUT_BUCK1_UV_MAX, OUT_BUCK1_UV_STEP), + RANGE(OUT_BUCK2_UV_MIN, OUT_BUCK2_UV_MAX, OUT_BUCK2_UV_STEP), +}; + +/* BUCK: 1 - current range */ +static struct output_range buck_current_range[] = { + RANGE(OUT_BUCK1_UA_MIN, OUT_BUCK1_UA_MAX, OUT_BUCK1_UA_STEP), +}; + +/* BUCK operating modes */ +static struct dm_regulator_mode sandbox_buck_modes[] = { + MODE(BUCK_OM_OFF, OM2REG(BUCK_OM_OFF), "OFF"), + MODE(BUCK_OM_ON, OM2REG(BUCK_OM_ON), "ON"), + MODE(BUCK_OM_PWM, OM2REG(BUCK_OM_PWM), "PWM"), +}; + +/* LDO: 1,2 - voltage range */ +static struct output_range ldo_voltage_range[] = { + RANGE(OUT_LDO1_UV_MIN, OUT_LDO1_UV_MAX, OUT_LDO1_UV_STEP), + RANGE(OUT_LDO2_UV_MIN, OUT_LDO2_UV_MAX, OUT_LDO2_UV_STEP), +}; + +/* LDO: 1 - current range */ +static struct output_range ldo_current_range[] = { + RANGE(OUT_LDO1_UA_MIN, OUT_LDO1_UA_MAX, OUT_LDO1_UA_STEP), +}; + +/* LDO operating modes */ +static struct dm_regulator_mode sandbox_ldo_modes[] = { + MODE(LDO_OM_OFF, OM2REG(LDO_OM_OFF), "OFF"), + MODE(LDO_OM_ON, OM2REG(LDO_OM_ON), "ON"), + MODE(LDO_OM_SLEEP, OM2REG(LDO_OM_SLEEP), "SLEEP"), + MODE(LDO_OM_STANDBY, OM2REG(LDO_OM_STANDBY), "STANDBY"), +}; + +int out_get_value(struct udevice *dev, int output_count, int reg_type, + struct output_range *range) +{ + uint8_t reg_val; + uint reg; + int ret; + + if (dev->driver_data > output_count) { + error("Unknown regulator number: %lu for PMIC %s!", + dev->driver_data, dev->name); + return -EINVAL; + } + + reg = (dev->driver_data - 1) * OUT_REG_COUNT + reg_type; + ret = pmic_read(dev->parent, reg, &reg_val, 1); + if (ret) { + error("PMIC read failed: %d\n", ret); + return ret; + } + + ret = REG2VAL(range[dev->driver_data - 1].min, + range[dev->driver_data - 1].step, + reg_val); + + return ret; +} + +static int out_set_value(struct udevice *dev, int output_count, int reg_type, + struct output_range *range, int value) +{ + uint8_t reg_val; + uint reg; + int ret; + int max_value; + + if (dev->driver_data > output_count) { + error("Unknown regulator number: %lu for PMIC %s!", + dev->driver_data, dev->name); + return -EINVAL; + } + + max_value = range[dev->driver_data - 1].max; + if (value > max_value) { + error("Wrong value for %s: %lu. Max is: %d.", + dev->name, dev->driver_data, max_value); + return -EINVAL; + } + + reg_val = VAL2REG(range[dev->driver_data - 1].min, + range[dev->driver_data - 1].step, + value); + + reg = (dev->driver_data - 1) * OUT_REG_COUNT + reg_type; + ret = pmic_write(dev->parent, reg, &reg_val, 1); + if (ret) { + error("PMIC write failed: %d\n", ret); + return ret; + } + + return 0; +} + +static int out_get_mode(struct udevice *dev) +{ + struct dm_regulator_uclass_platdata *uc_pdata; + uint8_t reg_val; + uint reg; + int ret; + int i; + + uc_pdata = dev_get_uclass_platdata(dev); + + reg = (dev->driver_data - 1) * OUT_REG_COUNT + OUT_REG_OM; + ret = pmic_read(dev->parent, reg, &reg_val, 1); + if (ret) { + error("PMIC read failed: %d\n", ret); + return ret; + } + + for (i = 0; i < uc_pdata->mode_count; i++) { + if (reg_val == uc_pdata->mode[i].register_value) + return uc_pdata->mode[i].id; + } + + error("Unknown operation mode for %s!", dev->name); + return -EINVAL; +} + +static int out_set_mode(struct udevice *dev, int mode) +{ + struct dm_regulator_uclass_platdata *uc_pdata; + int reg_val = -1; + uint reg; + int ret; + int i; + + uc_pdata = dev_get_uclass_platdata(dev); + + if (mode >= uc_pdata->mode_count) + return -EINVAL; + + for (i = 0; i < uc_pdata->mode_count; i++) { + if (mode == uc_pdata->mode[i].id) { + reg_val = uc_pdata->mode[i].register_value; + break; + } + } + + if (reg_val == -1) { + error("Unknown operation mode for %s!", dev->name); + return -EINVAL; + } + + reg = (dev->driver_data - 1) * OUT_REG_COUNT + OUT_REG_OM; + ret = pmic_write(dev->parent, reg, (uint8_t *)&reg_val, 1); + if (ret) { + error("PMIC write failed: %d\n", ret); + return ret; + } + + return 0; +} + +static int buck_get_voltage(struct udevice *dev) +{ + return out_get_value(dev, SANDBOX_BUCK_COUNT, OUT_REG_UV, + buck_voltage_range); +} + +static int buck_set_voltage(struct udevice *dev, int uV) +{ + return out_set_value(dev, SANDBOX_BUCK_COUNT, OUT_REG_UV, + buck_voltage_range, uV); +} + +static int buck_get_current(struct udevice *dev) +{ + /* BUCK2 - unsupported */ + if (dev->driver_data == 2) + return -ENOSYS; + + return out_get_value(dev, SANDBOX_BUCK_COUNT, OUT_REG_UA, + buck_current_range); +} + +static int buck_set_current(struct udevice *dev, int uA) +{ + /* BUCK2 - unsupported */ + if (dev->driver_data == 2) + return -ENOSYS; + + return out_set_value(dev, SANDBOX_BUCK_COUNT, OUT_REG_UA, + buck_current_range, uA); +} + +static bool buck_get_enable(struct udevice *dev) +{ + if (out_get_mode(dev) == BUCK_OM_OFF) + return false; + + return true; +} + +static int buck_set_enable(struct udevice *dev, bool enable) +{ + return out_set_mode(dev, enable ? BUCK_OM_ON : BUCK_OM_OFF); +} + +static int sandbox_buck_probe(struct udevice *dev) +{ + struct dm_regulator_uclass_platdata *uc_pdata; + + uc_pdata = dev_get_uclass_platdata(dev); + + uc_pdata->type = REGULATOR_TYPE_BUCK; + uc_pdata->mode = sandbox_buck_modes; + uc_pdata->mode_count = ARRAY_SIZE(sandbox_buck_modes); + + return 0; +} + +static const struct dm_regulator_ops sandbox_buck_ops = { + .get_value = buck_get_voltage, + .set_value = buck_set_voltage, + .get_current = buck_get_current, + .set_current = buck_set_current, + .get_enable = buck_get_enable, + .set_enable = buck_set_enable, + .get_mode = out_get_mode, + .set_mode = out_set_mode, +}; + +U_BOOT_DRIVER(sandbox_buck) = { + .name = SANDBOX_BUCK_DRIVER, + .id = UCLASS_REGULATOR, + .ops = &sandbox_buck_ops, + .probe = sandbox_buck_probe, +}; + +static int ldo_get_voltage(struct udevice *dev) +{ + return out_get_value(dev, SANDBOX_LDO_COUNT, OUT_REG_UV, + ldo_voltage_range); +} + +static int ldo_set_voltage(struct udevice *dev, int uV) +{ + return out_set_value(dev, SANDBOX_LDO_COUNT, OUT_REG_UV, + ldo_voltage_range, uV); +} + +static int ldo_get_current(struct udevice *dev) +{ + /* LDO2 - unsupported */ + if (dev->driver_data == 2) + return -ENOSYS; + + return out_get_value(dev, SANDBOX_LDO_COUNT, OUT_REG_UA, + ldo_current_range); +} + +static int ldo_set_current(struct udevice *dev, int uA) +{ + /* LDO2 - unsupported */ + if (dev->driver_data == 2) + return -ENOSYS; + + return out_set_value(dev, SANDBOX_LDO_COUNT, OUT_REG_UA, + ldo_current_range, uA); +} + +static bool ldo_get_enable(struct udevice *dev) +{ + if (out_get_mode(dev) == LDO_OM_OFF) + return false; + + return true; +} + +static int ldo_set_enable(struct udevice *dev, bool enable) +{ + return out_set_mode(dev, enable ? LDO_OM_ON : LDO_OM_OFF); +} + +static int sandbox_ldo_probe(struct udevice *dev) +{ + struct dm_regulator_uclass_platdata *uc_pdata; + + uc_pdata = dev_get_uclass_platdata(dev); + + uc_pdata->type = REGULATOR_TYPE_LDO; + uc_pdata->mode = sandbox_ldo_modes; + uc_pdata->mode_count = ARRAY_SIZE(sandbox_ldo_modes); + + return 0; +} + +static const struct dm_regulator_ops sandbox_ldo_ops = { + .get_value = ldo_get_voltage, + .set_value = ldo_set_voltage, + .get_current = ldo_get_current, + .set_current = ldo_set_current, + .get_enable = ldo_get_enable, + .set_enable = ldo_set_enable, + .get_mode = out_get_mode, + .set_mode = out_set_mode, +}; + +U_BOOT_DRIVER(sandbox_ldo) = { + .name = SANDBOX_LDO_DRIVER, + .id = UCLASS_REGULATOR, + .ops = &sandbox_ldo_ops, + .probe = sandbox_ldo_probe, +}; diff --git a/include/power/sandbox_pmic.h b/include/power/sandbox_pmic.h new file mode 100644 index 0000000..f317c3a --- /dev/null +++ b/include/power/sandbox_pmic.h @@ -0,0 +1,105 @@ +/* + * Copyright (C) 2015 Samsung Electronics + * Przemyslaw Marczak p.marczak@samsung.com + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#ifndef _SANDBOX_PMIC_H_ +#define _SANDBOX_PMIC_H_ + +#define SANDBOX_LDO_DRIVER "sandbox_ldo" +#define SANDBOX_OF_LDO_PREFIX "ldo" +#define SANDBOX_BUCK_DRIVER "sandbox_buck" +#define SANDBOX_OF_BUCK_PREFIX "buck" + +#define SANDBOX_BUCK_COUNT 2 +#define SANDBOX_LDO_COUNT 2 +/* + * Sandbox PMIC registers: + * We have only 12 significant registers, but we alloc 16 for padding. + */ +enum { + SANDBOX_PMIC_REG_BUCK1_UV = 0, + SANDBOX_PMIC_REG_BUCK1_UA, + SANDBOX_PMIC_REG_BUCK1_OM, + + SANDBOX_PMIC_REG_BUCK2_UV, + SANDBOX_PMIC_REG_BUCK2_UA, + SANDBOX_PMIC_REG_BUCK2_OM, + + SANDBOX_PMIC_REG_LDO_OFFSET, + SANDBOX_PMIC_REG_LDO1_UV = SANDBOX_PMIC_REG_LDO_OFFSET, + SANDBOX_PMIC_REG_LDO1_UA, + SANDBOX_PMIC_REG_LDO1_OM, + + SANDBOX_PMIC_REG_LDO2_UV, + SANDBOX_PMIC_REG_LDO2_UA, + SANDBOX_PMIC_REG_LDO2_OM, + + SANDBOX_PMIC_REG_COUNT = 16, +}; + +/* Register offset for output: micro Volts, micro Amps, Operation Mode */ +enum { + OUT_REG_UV = 0, + OUT_REG_UA, + OUT_REG_OM, + OUT_REG_COUNT, +}; + +/* Buck operation modes */ +enum { + BUCK_OM_OFF = 0, + BUCK_OM_ON, + BUCK_OM_PWM, + BUCK_OM_COUNT, +}; + +/* Ldo operation modes */ +enum { + LDO_OM_OFF = 0, + LDO_OM_ON, + LDO_OM_SLEEP, + LDO_OM_STANDBY, + LDO_OM_COUNT, +}; + +/* BUCK1 Voltage: min: 0.8V, step: 25mV, max 2.4V */ +#define OUT_BUCK1_UV_MIN 800000 +#define OUT_BUCK1_UV_MAX 2400000 +#define OUT_BUCK1_UV_STEP 25000 + +/* BUCK1 Amperage: min: 150mA, step: 25mA, max: 250mA */ +#define OUT_BUCK1_UA_MIN 150000 +#define OUT_BUCK1_UA_MAX 250000 +#define OUT_BUCK1_UA_STEP 25000 + +/* BUCK2 Voltage: min: 0.75V, step: 50mV, max 3.95V */ +#define OUT_BUCK2_UV_MIN 750000 +#define OUT_BUCK2_UV_MAX 3950000 +#define OUT_BUCK2_UV_STEP 50000 + +/* LDO1 Voltage: min: 0.8V, step: 25mV, max 2.4V */ +#define OUT_LDO1_UV_MIN 800000 +#define OUT_LDO1_UV_MAX 2400000 +#define OUT_LDO1_UV_STEP 25000 + +/* LDO1 Amperage: min: 100mA, step: 50mA, max: 200mA */ +#define OUT_LDO1_UA_MIN 100000 +#define OUT_LDO1_UA_MAX 200000 +#define OUT_LDO1_UA_STEP 50000 + +/* LDO2 Voltage: min: 0.75V, step: 50mV, max 3.95V */ +#define OUT_LDO2_UV_MIN 750000 +#define OUT_LDO2_UV_MAX 3950000 +#define OUT_LDO2_UV_STEP 50000 + +/* register <-> value conversion */ +#define REG2VAL(min, step, reg) ((min) + ((step) * (reg))) +#define VAL2REG(min, step, val) (((val) - (min)) / (step)) + +/* Operation mode id -> register value conversion */ +#define OM2REG(x) (x) + +#endif

This change adds new file to sandbox driver model test environment. The file is: test/dm/power.c, and it includes tests for PMIC framework, which includes PMIC uclass and REGULATOR uclass.

All tests are based od Sandbox PMIC emulated device. Some test constants for this device are defined in the header: include/power/sandbox_pmic.h

PMIC tests includes: - pmic get - tests, that pmic_get() returns the requested device - pmic I/O - tests I/O by writing and reading some values to PMIC's registers and then compares, that the write/read values are equal.

The regulator tests includes: - Regulator get by devname/platname - Voltage set/get - Current set/get - Enable set/get - Mode set/get - Autoset - List autoset

For the regulator 'get' test, the returned device pointers are compared, and their names are also compared to the requested one. Every other test, first sets the given attribute and next try to get it. The test pass, when the set/get values are equal.

Signed-off-by: Przemyslaw Marczak p.marczak@samsung.com Acked-by: Simon Glass sjg@chromium.org Tested on sandbox: Tested-by: Simon Glass sjg@chromium.org --- Changes V2: - test: dm: split power.c into pmic.c and regulator.c - regulator tests: cleanup --- include/power/sandbox_pmic.h | 33 +++++ test/dm/Makefile | 2 + test/dm/pmic.c | 69 +++++++++ test/dm/regulator.c | 325 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 429 insertions(+) create mode 100644 test/dm/pmic.c create mode 100644 test/dm/regulator.c

diff --git a/include/power/sandbox_pmic.h b/include/power/sandbox_pmic.h index f317c3a..ae14292 100644 --- a/include/power/sandbox_pmic.h +++ b/include/power/sandbox_pmic.h @@ -102,4 +102,37 @@ enum { /* Operation mode id -> register value conversion */ #define OM2REG(x) (x)

+/* Test data for: test/dm/power.c */ + +/* BUCK names */ +#define SANDBOX_BUCK1_DEVNAME "buck1" +#define SANDBOX_BUCK1_PLATNAME "SUPPLY_1.2V" +#define SANDBOX_BUCK2_DEVNAME "buck2" +#define SANDBOX_BUCK2_PLATNAME "SUPPLY_3.3V" +/* LDO names */ +#define SANDBOX_LDO1_DEVNAME "ldo1" +#define SANDBOX_LDO1_PLATNAME "VDD_EMMC_1.8V" +#define SANDBOX_LDO2_DEVNAME "ldo2" +#define SANDBOX_LDO2_PLATNAME "VDD_LCD_3.3V" + +/* + * Expected regulators setup after call of: + * - regulator_autoset() + * - regulator_list_autoset() + */ + +/* BUCK1: for testing regulator_autoset() */ +#define SANDBOX_BUCK1_AUTOSET_EXPECTED_UV 1200000 +#define SANDBOX_BUCK1_AUTOSET_EXPECTED_UA 200000 +#define SANDBOX_BUCK1_AUTOSET_EXPECTED_ENABLE true + +/* LDO1/2 for testing regulator_list_autoset() */ +#define SANDBOX_LDO1_AUTOSET_EXPECTED_UV 1800000 +#define SANDBOX_LDO1_AUTOSET_EXPECTED_UA 100000 +#define SANDBOX_LDO1_AUTOSET_EXPECTED_ENABLE true + +#define SANDBOX_LDO2_AUTOSET_EXPECTED_UV 3000000 +#define SANDBOX_LDO2_AUTOSET_EXPECTED_UA -ENOSYS +#define SANDBOX_LDO2_AUTOSET_EXPECTED_ENABLE false + #endif diff --git a/test/dm/Makefile b/test/dm/Makefile index fd9e29f..444b24e 100644 --- a/test/dm/Makefile +++ b/test/dm/Makefile @@ -24,4 +24,6 @@ obj-$(CONFIG_DM_PCI) += pci.o obj-$(CONFIG_DM_SPI_FLASH) += sf.o obj-$(CONFIG_DM_SPI) += spi.o obj-$(CONFIG_DM_USB) += usb.o +obj-$(CONFIG_DM_PMIC) += pmic.o +obj-$(CONFIG_DM_REGULATOR) += regulator.o endif diff --git a/test/dm/pmic.c b/test/dm/pmic.c new file mode 100644 index 0000000..e9c904c --- /dev/null +++ b/test/dm/pmic.c @@ -0,0 +1,69 @@ +/* + * Tests for the driver model pmic API + * + * Copyright (c) 2015 Samsung Electronics + * Przemyslaw Marczak p.marczak@samsung.com + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <errno.h> +#include <dm.h> +#include <fdtdec.h> +#include <malloc.h> +#include <dm/device-internal.h> +#include <dm/root.h> +#include <dm/ut.h> +#include <dm/util.h> +#include <dm/test.h> +#include <dm/uclass-internal.h> +#include <power/pmic.h> +#include <power/sandbox_pmic.h> + +DECLARE_GLOBAL_DATA_PTR; + +/* Test PMIC get method */ +static int dm_test_power_pmic_get(struct dm_test_state *dms) +{ + const char *name = "sandbox_pmic"; + struct udevice *dev; + + ut_assertok(pmic_get(name, &dev)); + ut_assertnonnull(dev); + + /* Check PMIC's name */ + ut_asserteq_str(name, dev->name); + + return 0; +} +DM_TEST(dm_test_power_pmic_get, DM_TESTF_SCAN_FDT); + +/* Test PMIC I/O */ +static int dm_test_power_pmic_io(struct dm_test_state *dms) +{ + const char *name = "sandbox_pmic"; + uint8_t out_buffer, in_buffer; + struct udevice *dev; + int reg_count, i; + + ut_assertok(pmic_get(name, &dev)); + + reg_count = pmic_reg_count(dev); + ut_asserteq(reg_count, SANDBOX_PMIC_REG_COUNT); + + /* + * Test PMIC I/O - write and read a loop counter. + * usually we can't write to all PMIC's registers in the real hardware, + * but we can to the sandbox pmic. + */ + for (i = 0; i < reg_count; i++) { + out_buffer = i; + ut_assertok(pmic_write(dev, i, &out_buffer, 1)); + ut_assertok(pmic_read(dev, i, &in_buffer, 1)); + ut_asserteq(out_buffer, in_buffer); + } + + return 0; +} +DM_TEST(dm_test_power_pmic_io, DM_TESTF_SCAN_FDT); diff --git a/test/dm/regulator.c b/test/dm/regulator.c new file mode 100644 index 0000000..c4f14bd --- /dev/null +++ b/test/dm/regulator.c @@ -0,0 +1,325 @@ +/* + * Tests for the driver model regulator API + * + * Copyright (c) 2015 Samsung Electronics + * Przemyslaw Marczak p.marczak@samsung.com + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <errno.h> +#include <dm.h> +#include <fdtdec.h> +#include <malloc.h> +#include <dm/device-internal.h> +#include <dm/root.h> +#include <dm/ut.h> +#include <dm/util.h> +#include <dm/test.h> +#include <dm/uclass-internal.h> +#include <power/pmic.h> +#include <power/regulator.h> +#include <power/sandbox_pmic.h> + +DECLARE_GLOBAL_DATA_PTR; + +enum { + BUCK1, + BUCK2, + LDO1, + LDO2, + OUTPUT_COUNT, +}; + +enum { + DEVNAME = 0, + PLATNAME, + OUTPUT_NAME_COUNT, +}; + +static const char *regulator_names[OUTPUT_COUNT][OUTPUT_NAME_COUNT] = { + /* devname, platname */ + { SANDBOX_BUCK1_DEVNAME, SANDBOX_BUCK1_PLATNAME }, + { SANDBOX_BUCK2_DEVNAME, SANDBOX_BUCK2_PLATNAME }, + { SANDBOX_LDO1_DEVNAME, SANDBOX_LDO1_PLATNAME}, + { SANDBOX_LDO2_DEVNAME, SANDBOX_LDO2_PLATNAME}, +}; + +/* Test regulator get method */ +static int dm_test_power_regulator_get(struct dm_test_state *dms) +{ + struct dm_regulator_uclass_platdata *uc_pdata; + struct udevice *dev_by_devname; + struct udevice *dev_by_platname; + const char *devname; + const char *platname; + int i; + + for (i = 0; i < OUTPUT_COUNT; i++) { + /* + * Do the test for each regulator's devname and platname, + * which are related to a single device. + */ + devname = regulator_names[i][DEVNAME]; + platname = regulator_names[i][PLATNAME]; + + /* + * Check, that regulator_get_by_devname() function, returns + * a device with the name equal to the requested one. + */ + ut_assertok(regulator_get_by_devname(devname, &dev_by_devname)); + ut_asserteq_str(devname, dev_by_devname->name); + + /* + * Check, that regulator_get_by_platname() function, returns + * a device with the name equal to the requested one. + */ + ut_assertok(regulator_get_by_platname(platname, &dev_by_platname)); + uc_pdata = dev_get_uclass_platdata(dev_by_platname); + ut_assert(uc_pdata); + ut_asserteq_str(platname, uc_pdata->name); + + /* + * Check, that the pointers returned by both get functions, + * points to the same regulator device. + */ + ut_asserteq_ptr(dev_by_devname, dev_by_platname); + } + + return 0; +} +DM_TEST(dm_test_power_regulator_get, DM_TESTF_SCAN_FDT); + +/* Test regulator set and get Voltage method */ +static int dm_test_power_regulator_set_get_voltage(struct dm_test_state *dms) +{ + struct dm_regulator_uclass_platdata *uc_pdata; + struct udevice *dev; + const char *platname; + int val_set, val_get; + + /* Set and get Voltage of BUCK1 - set to 'min' constraint */ + platname = regulator_names[BUCK1][PLATNAME]; + ut_assertok(regulator_get_by_platname(platname, &dev)); + + uc_pdata = dev_get_uclass_platdata(dev); + ut_assert(uc_pdata); + + val_set = uc_pdata->min_uV; + ut_assertok(regulator_set_value(dev, val_set)); + + val_get = regulator_get_value(dev); + ut_assert(val_get >= 0); + + ut_asserteq(val_set, val_get); + + return 0; +} +DM_TEST(dm_test_power_regulator_set_get_voltage, DM_TESTF_SCAN_FDT); + +/* Test regulator set and get Current method */ +static int dm_test_power_regulator_set_get_current(struct dm_test_state *dms) +{ + struct dm_regulator_uclass_platdata *uc_pdata; + struct udevice *dev; + const char *platname; + int val_set, val_get; + + /* Set and get the Current of LDO1 - set to 'min' constraint */ + platname = regulator_names[LDO1][PLATNAME]; + ut_assertok(regulator_get_by_platname(platname, &dev)); + + uc_pdata = dev_get_uclass_platdata(dev); + ut_assert(uc_pdata); + + val_set = uc_pdata->min_uA; + ut_assertok(regulator_set_current(dev, val_set)); + + val_get = regulator_get_current(dev); + ut_assert(val_get >= 0); + + ut_asserteq(val_set, val_get); + + /* Check LDO2 current limit constraints - should be -ENODATA */ + platname = regulator_names[LDO2][PLATNAME]; + ut_assertok(regulator_get_by_platname(platname, &dev)); + + uc_pdata = dev_get_uclass_platdata(dev); + ut_assert(uc_pdata); + ut_asserteq(-ENODATA, uc_pdata->min_uA); + ut_asserteq(-ENODATA, uc_pdata->max_uA); + + /* Try set the Current of LDO2 - should return -ENOSYS */ + ut_asserteq(-ENOSYS, regulator_set_current(dev, 0)); + + return 0; +} +DM_TEST(dm_test_power_regulator_set_get_current, DM_TESTF_SCAN_FDT); + +/* Test regulator set and get Enable method */ +static int dm_test_power_regulator_set_get_enable(struct dm_test_state *dms) +{ + const char *platname; + struct udevice *dev; + bool val_set = true; + + /* Set the Enable of LDO1 - default is disabled */ + platname = regulator_names[LDO1][PLATNAME]; + ut_assertok(regulator_get_by_platname(platname, &dev)); + ut_assertok(regulator_set_enable(dev, val_set)); + + /* Get the Enable state of LDO1 and compare it with the requested one */ + ut_asserteq(regulator_get_enable(dev), val_set); + + return 0; +} +DM_TEST(dm_test_power_regulator_set_get_enable, DM_TESTF_SCAN_FDT); + +/* Test regulator set and get mode method */ +static int dm_test_power_regulator_set_get_mode(struct dm_test_state *dms) +{ + const char *platname; + struct udevice *dev; + int val_set = LDO_OM_SLEEP; + + /* Set the mode id to LDO_OM_SLEEP of LDO1 - default is LDO_OM_OFF */ + platname = regulator_names[LDO1][PLATNAME]; + ut_assertok(regulator_get_by_platname(platname, &dev)); + ut_assertok(regulator_set_mode(dev, val_set)); + + /* Get the mode id of LDO1 and compare it with the requested one */ + ut_asserteq(regulator_get_mode(dev), val_set); + + return 0; +} +DM_TEST(dm_test_power_regulator_set_get_mode, DM_TESTF_SCAN_FDT); + +/* Test regulator autoset method */ +static int dm_test_power_regulator_autoset(struct dm_test_state *dms) +{ + const char *platname; + struct udevice *dev, *dev_autoset; + + /* + * Test the BUCK1 with fdt properties + * - min-microvolt = max-microvolt = 1200000 + * - min-microamp = max-microamp = 200000 + * - always-on = set + * - boot-on = not set + * Expected output state: uV=1200000; uA=200000; output enabled + */ + platname = regulator_names[BUCK1][PLATNAME]; + ut_assertok(regulator_autoset(platname, &dev_autoset, false)); + + /* Check, that the returned device is proper */ + ut_assertok(regulator_get_by_platname(platname, &dev)); + ut_asserteq_ptr(dev, dev_autoset); + + /* Check the setup after autoset */ + ut_asserteq(regulator_get_value(dev), + SANDBOX_BUCK1_AUTOSET_EXPECTED_UV); + ut_asserteq(regulator_get_current(dev), + SANDBOX_BUCK1_AUTOSET_EXPECTED_UA); + ut_asserteq(regulator_get_enable(dev), + SANDBOX_BUCK1_AUTOSET_EXPECTED_ENABLE); + + return 0; +} +DM_TEST(dm_test_power_regulator_autoset, DM_TESTF_SCAN_FDT); + +/* + * Struct setting: to keep the expected output settings. + * @voltage: Voltage value [uV] + * @current: Current value [uA] + * @enable: output enable state: true/false + */ +struct setting { + int voltage; + int current; + bool enable; +}; + +/* + * platname_list: an array of regulator platform names. + * For testing regulator_list_autoset() for outputs: + * - LDO1 + * - LDO2 + */ +static const char *platname_list[] = { + SANDBOX_LDO1_PLATNAME, + SANDBOX_LDO2_PLATNAME, + NULL, +}; + +/* + * expected_setting_list: an array of regulator output setting, expected after + * call of the regulator_list_autoset() for the "platname_list" array. + * For testing results of regulator_list_autoset() for outputs: + * - LDO1 + * - LDO2 + * The settings are defined in: include/power/sandbox_pmic.h + */ +static const struct setting expected_setting_list[] = { + [0] = { /* LDO1 */ + .voltage = SANDBOX_LDO1_AUTOSET_EXPECTED_UV, + .current = SANDBOX_LDO1_AUTOSET_EXPECTED_UA, + .enable = SANDBOX_LDO1_AUTOSET_EXPECTED_ENABLE, + }, + [1] = { /* LDO2 */ + .voltage = SANDBOX_LDO2_AUTOSET_EXPECTED_UV, + .current = SANDBOX_LDO2_AUTOSET_EXPECTED_UA, + .enable = SANDBOX_LDO2_AUTOSET_EXPECTED_ENABLE, + }, +}; + +static int list_count = ARRAY_SIZE(expected_setting_list); + +/* Test regulator list autoset method */ +static int dm_test_power_regulator_autoset_list(struct dm_test_state *dms) +{ + struct udevice *dev_list[2], *dev; + int i; + + /* + * Test the settings of the regulator list: + * LDO1 with fdt properties: + * - min-microvolt = max-microvolt = 1800000 + * - min-microamp = max-microamp = 100000 + * - always-on = not set + * - boot-on = set + * Expected output state: uV=1800000; uA=100000; output enabled + * + * LDO2 with fdt properties: + * - min-microvolt = max-microvolt = 3300000 + * - always-on = not set + * - boot-on = not set + * Expected output state: uV=300000(default); output disabled(default) + * The expected settings are defined in: include/power/sandbox_pmic.h. + */ + ut_assertok(regulator_list_autoset(platname_list, dev_list, false)); + + for (i = 0; i < list_count; i++) { + /* Check, that the returned device is non-NULL */ + ut_assert(dev_list[i]); + + /* Check, that the returned device is proper */ + ut_assertok(regulator_get_by_platname(platname_list[i], &dev)); + ut_asserteq_ptr(dev_list[i], dev); + + /* Check, that regulator output Voltage value is as expected */ + ut_asserteq(regulator_get_value(dev_list[i]), + expected_setting_list[i].voltage); + + /* Check, that regulator output Current value is as expected */ + ut_asserteq(regulator_get_current(dev_list[i]), + expected_setting_list[i].current); + + /* Check, that regulator output Enable state is as expected */ + ut_asserteq(regulator_get_enable(dev_list[i]), + expected_setting_list[i].enable); + } + + return 0; +} +DM_TEST(dm_test_power_regulator_autoset_list, DM_TESTF_SCAN_FDT);

The file test.dts from driver model test directory, was compiled by call dtc in script: test/dm/test-dm.sh.

This doesn't allow for including of dtsi files and using of C preprocessor routines in this dts file.

Since the mentioned script builds U-Boot before tests, then moving the test.dts file into sandbox dts directory is reasonable.

Signed-off-by: Przemyslaw Marczak p.marczak@samsung.com Acked-by: Simon Glass sjg@chromium.org Tested on sandbox: Tested-by: Simon Glass sjg@chromium.org --- Changes V2: - new commit --- arch/sandbox/dts/Makefile | 1 + arch/sandbox/dts/test.dts | 230 ++++++++++++++++++++++++++++++++++++++++++++++ test/dm/.gitignore | 1 - test/dm/test-dm.sh | 3 +- test/dm/test-main.c | 3 +- test/dm/test.dts | 230 ---------------------------------------------- 6 files changed, 233 insertions(+), 235 deletions(-) create mode 100644 arch/sandbox/dts/test.dts delete mode 100644 test/dm/.gitignore delete mode 100644 test/dm/test.dts

diff --git a/arch/sandbox/dts/Makefile b/arch/sandbox/dts/Makefile index a4c980b..562a078 100644 --- a/arch/sandbox/dts/Makefile +++ b/arch/sandbox/dts/Makefile @@ -1,4 +1,5 @@ dtb-$(CONFIG_SANDBOX) += sandbox.dtb +dtb-$(CONFIG_DM_TEST) += test.dtb

targets += $(dtb-y)

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts new file mode 100644 index 0000000..d0c40be --- /dev/null +++ b/arch/sandbox/dts/test.dts @@ -0,0 +1,230 @@ +/dts-v1/; + +/ { + model = "sandbox"; + compatible = "sandbox"; + #address-cells = <1>; + #size-cells = <0>; + + aliases { + console = &uart0; + i2c0 = "/i2c@0"; + spi0 = "/spi@0"; + pci0 = &pci; + testfdt6 = "/e-test"; + testbus3 = "/some-bus"; + testfdt0 = "/some-bus/c-test@0"; + testfdt1 = "/some-bus/c-test@1"; + testfdt3 = "/b-test"; + testfdt5 = "/some-bus/c-test@5"; + testfdt8 = "/a-test"; + eth0 = "/eth@10002000"; + eth5 = &eth_5; + usb0 = &usb_0; + usb1 = &usb_1; + usb2 = &usb_2; + }; + + uart0: serial { + compatible = "sandbox,serial"; + u-boot,dm-pre-reloc; + }; + + a-test { + reg = <0>; + compatible = "denx,u-boot-fdt-test"; + ping-expect = <0>; + ping-add = <0>; + u-boot,dm-pre-reloc; + test-gpios = <&gpio_a 1>, <&gpio_a 4>, <&gpio_b 5 0 3 2 1>, + <0>, <&gpio_a 12>; + test2-gpios = <&gpio_a 1>, <&gpio_a 4>, <&gpio_b 6 1 3 2 1>, + <&gpio_b 7 2 3 2 1>, <&gpio_b 8 4 3 2 1>, + <&gpio_b 9 0xc 3 2 1>; + }; + + junk { + reg = <1>; + compatible = "not,compatible"; + }; + + no-compatible { + reg = <2>; + }; + + b-test { + reg = <3>; + compatible = "denx,u-boot-fdt-test"; + ping-expect = <3>; + ping-add = <3>; + }; + + some-bus { + #address-cells = <1>; + #size-cells = <0>; + compatible = "denx,u-boot-test-bus"; + reg = <3>; + ping-expect = <4>; + ping-add = <4>; + c-test@5 { + compatible = "denx,u-boot-fdt-test"; + reg = <5>; + ping-expect = <5>; + ping-add = <5>; + }; + c-test@0 { + compatible = "denx,u-boot-fdt-test"; + reg = <0>; + ping-expect = <6>; + ping-add = <6>; + }; + c-test@1 { + compatible = "denx,u-boot-fdt-test"; + reg = <1>; + ping-expect = <7>; + ping-add = <7>; + }; + }; + + d-test { + reg = <3>; + ping-expect = <6>; + ping-add = <6>; + compatible = "google,another-fdt-test"; + }; + + e-test { + reg = <3>; + ping-expect = <6>; + ping-add = <6>; + compatible = "google,another-fdt-test"; + }; + + f-test { + compatible = "denx,u-boot-fdt-test"; + }; + + g-test { + compatible = "denx,u-boot-fdt-test"; + }; + + gpio_a: base-gpios { + compatible = "sandbox,gpio"; + gpio-controller; + #gpio-cells = <1>; + gpio-bank-name = "a"; + num-gpios = <20>; + }; + + gpio_b: extra-gpios { + compatible = "sandbox,gpio"; + gpio-controller; + #gpio-cells = <5>; + gpio-bank-name = "b"; + num-gpios = <10>; + }; + + i2c@0 { + #address-cells = <1>; + #size-cells = <0>; + reg = <0>; + compatible = "sandbox,i2c"; + clock-frequency = <100000>; + eeprom@2c { + reg = <0x2c>; + compatible = "i2c-eeprom"; + emul { + compatible = "sandbox,i2c-eeprom"; + sandbox,filename = "i2c.bin"; + sandbox,size = <256>; + }; + }; + }; + + pci: pci-controller { + compatible = "sandbox,pci"; + device_type = "pci"; + #address-cells = <3>; + #size-cells = <2>; + ranges = <0x02000000 0 0x10000000 0x10000000 0 0x2000 + 0x01000000 0 0x20000000 0x20000000 0 0x2000>; + pci@1f,0 { + compatible = "pci-generic"; + reg = <0xf800 0 0 0 0>; + emul@1f,0 { + compatible = "sandbox,swap-case"; + }; + }; + }; + + spi@0 { + #address-cells = <1>; + #size-cells = <0>; + reg = <0>; + compatible = "sandbox,spi"; + cs-gpios = <0>, <&gpio_a 0>; + spi.bin@0 { + reg = <0>; + compatible = "spansion,m25p16", "spi-flash"; + spi-max-frequency = <40000000>; + sandbox,filename = "spi.bin"; + }; + }; + + eth@10002000 { + compatible = "sandbox,eth"; + reg = <0x10002000 0x1000>; + fake-host-hwaddr = <0x00 0x00 0x66 0x44 0x22 0x00>; + }; + + eth_5: eth@10003000 { + compatible = "sandbox,eth"; + reg = <0x10003000 0x1000>; + fake-host-hwaddr = <0x00 0x00 0x66 0x44 0x22 0x11>; + }; + + eth@10004000 { + compatible = "sandbox,eth"; + reg = <0x10004000 0x1000>; + fake-host-hwaddr = <0x00 0x00 0x66 0x44 0x22 0x22>; + }; + + usb_0: usb@0 { + compatible = "sandbox,usb"; + status = "disabled"; + hub { + compatible = "sandbox,usb-hub"; + #address-cells = <1>; + #size-cells = <0>; + flash-stick { + reg = <0>; + compatible = "sandbox,usb-flash"; + }; + }; + }; + + usb_1: usb@1 { + compatible = "sandbox,usb"; + hub { + compatible = "usb-hub"; + usb,device-class = <9>; + hub-emul { + compatible = "sandbox,usb-hub"; + #address-cells = <1>; + #size-cells = <0>; + flash-stick { + reg = <0>; + compatible = "sandbox,usb-flash"; + sandbox,filepath = "testflash.bin"; + }; + + }; + }; + }; + + usb_2: usb@2 { + compatible = "sandbox,usb"; + status = "disabled"; + }; + +}; diff --git a/test/dm/.gitignore b/test/dm/.gitignore deleted file mode 100644 index b741b8a..0000000 --- a/test/dm/.gitignore +++ /dev/null @@ -1 +0,0 @@ -/test.dtb diff --git a/test/dm/test-dm.sh b/test/dm/test-dm.sh index 6158f68..5c47ffd 100755 --- a/test/dm/test-dm.sh +++ b/test/dm/test-dm.sh @@ -6,12 +6,11 @@ die() { }

NUM_CPUS=$(cat /proc/cpuinfo |grep -c processor) -dtc -I dts -O dtb test/dm/test.dts -o test/dm/test.dtb make O=sandbox sandbox_config || die "Cannot configure U-Boot" make O=sandbox -s -j${NUM_CPUS} || die "Cannot build U-Boot" dd if=/dev/zero of=spi.bin bs=1M count=2 echo -n "this is a test" > testflash.bin dd if=/dev/zero bs=1M count=4 >>testflash.bin -./sandbox/u-boot -d test/dm/test.dtb -c "dm test" +./sandbox/u-boot -d ./sandbox/arch/sandbox/dts/test.dtb -c "dm test" rm spi.bin rm testflash.bin diff --git a/test/dm/test-main.c b/test/dm/test-main.c index a47bb37..7348f69 100644 --- a/test/dm/test-main.c +++ b/test/dm/test-main.c @@ -78,8 +78,7 @@ int dm_test_main(const char *test_name) */ if (!gd->fdt_blob || fdt_next_node(gd->fdt_blob, 0, NULL) < 0) { puts("Please run with test device tree:\n" - " dtc -I dts -O dtb test/dm/test.dts -o test/dm/test.dtb\n" - " ./u-boot -d test/dm/test.dtb\n"); + " ./u-boot -d arch/sandbox/dts/test.dtb\n"); ut_assert(gd->fdt_blob); }

diff --git a/test/dm/test.dts b/test/dm/test.dts deleted file mode 100644 index d0c40be..0000000 --- a/test/dm/test.dts +++ /dev/null @@ -1,230 +0,0 @@ -/dts-v1/; - -/ { - model = "sandbox"; - compatible = "sandbox"; - #address-cells = <1>; - #size-cells = <0>; - - aliases { - console = &uart0; - i2c0 = "/i2c@0"; - spi0 = "/spi@0"; - pci0 = &pci; - testfdt6 = "/e-test"; - testbus3 = "/some-bus"; - testfdt0 = "/some-bus/c-test@0"; - testfdt1 = "/some-bus/c-test@1"; - testfdt3 = "/b-test"; - testfdt5 = "/some-bus/c-test@5"; - testfdt8 = "/a-test"; - eth0 = "/eth@10002000"; - eth5 = &eth_5; - usb0 = &usb_0; - usb1 = &usb_1; - usb2 = &usb_2; - }; - - uart0: serial { - compatible = "sandbox,serial"; - u-boot,dm-pre-reloc; - }; - - a-test { - reg = <0>; - compatible = "denx,u-boot-fdt-test"; - ping-expect = <0>; - ping-add = <0>; - u-boot,dm-pre-reloc; - test-gpios = <&gpio_a 1>, <&gpio_a 4>, <&gpio_b 5 0 3 2 1>, - <0>, <&gpio_a 12>; - test2-gpios = <&gpio_a 1>, <&gpio_a 4>, <&gpio_b 6 1 3 2 1>, - <&gpio_b 7 2 3 2 1>, <&gpio_b 8 4 3 2 1>, - <&gpio_b 9 0xc 3 2 1>; - }; - - junk { - reg = <1>; - compatible = "not,compatible"; - }; - - no-compatible { - reg = <2>; - }; - - b-test { - reg = <3>; - compatible = "denx,u-boot-fdt-test"; - ping-expect = <3>; - ping-add = <3>; - }; - - some-bus { - #address-cells = <1>; - #size-cells = <0>; - compatible = "denx,u-boot-test-bus"; - reg = <3>; - ping-expect = <4>; - ping-add = <4>; - c-test@5 { - compatible = "denx,u-boot-fdt-test"; - reg = <5>; - ping-expect = <5>; - ping-add = <5>; - }; - c-test@0 { - compatible = "denx,u-boot-fdt-test"; - reg = <0>; - ping-expect = <6>; - ping-add = <6>; - }; - c-test@1 { - compatible = "denx,u-boot-fdt-test"; - reg = <1>; - ping-expect = <7>; - ping-add = <7>; - }; - }; - - d-test { - reg = <3>; - ping-expect = <6>; - ping-add = <6>; - compatible = "google,another-fdt-test"; - }; - - e-test { - reg = <3>; - ping-expect = <6>; - ping-add = <6>; - compatible = "google,another-fdt-test"; - }; - - f-test { - compatible = "denx,u-boot-fdt-test"; - }; - - g-test { - compatible = "denx,u-boot-fdt-test"; - }; - - gpio_a: base-gpios { - compatible = "sandbox,gpio"; - gpio-controller; - #gpio-cells = <1>; - gpio-bank-name = "a"; - num-gpios = <20>; - }; - - gpio_b: extra-gpios { - compatible = "sandbox,gpio"; - gpio-controller; - #gpio-cells = <5>; - gpio-bank-name = "b"; - num-gpios = <10>; - }; - - i2c@0 { - #address-cells = <1>; - #size-cells = <0>; - reg = <0>; - compatible = "sandbox,i2c"; - clock-frequency = <100000>; - eeprom@2c { - reg = <0x2c>; - compatible = "i2c-eeprom"; - emul { - compatible = "sandbox,i2c-eeprom"; - sandbox,filename = "i2c.bin"; - sandbox,size = <256>; - }; - }; - }; - - pci: pci-controller { - compatible = "sandbox,pci"; - device_type = "pci"; - #address-cells = <3>; - #size-cells = <2>; - ranges = <0x02000000 0 0x10000000 0x10000000 0 0x2000 - 0x01000000 0 0x20000000 0x20000000 0 0x2000>; - pci@1f,0 { - compatible = "pci-generic"; - reg = <0xf800 0 0 0 0>; - emul@1f,0 { - compatible = "sandbox,swap-case"; - }; - }; - }; - - spi@0 { - #address-cells = <1>; - #size-cells = <0>; - reg = <0>; - compatible = "sandbox,spi"; - cs-gpios = <0>, <&gpio_a 0>; - spi.bin@0 { - reg = <0>; - compatible = "spansion,m25p16", "spi-flash"; - spi-max-frequency = <40000000>; - sandbox,filename = "spi.bin"; - }; - }; - - eth@10002000 { - compatible = "sandbox,eth"; - reg = <0x10002000 0x1000>; - fake-host-hwaddr = <0x00 0x00 0x66 0x44 0x22 0x00>; - }; - - eth_5: eth@10003000 { - compatible = "sandbox,eth"; - reg = <0x10003000 0x1000>; - fake-host-hwaddr = <0x00 0x00 0x66 0x44 0x22 0x11>; - }; - - eth@10004000 { - compatible = "sandbox,eth"; - reg = <0x10004000 0x1000>; - fake-host-hwaddr = <0x00 0x00 0x66 0x44 0x22 0x22>; - }; - - usb_0: usb@0 { - compatible = "sandbox,usb"; - status = "disabled"; - hub { - compatible = "sandbox,usb-hub"; - #address-cells = <1>; - #size-cells = <0>; - flash-stick { - reg = <0>; - compatible = "sandbox,usb-flash"; - }; - }; - }; - - usb_1: usb@1 { - compatible = "sandbox,usb"; - hub { - compatible = "usb-hub"; - usb,device-class = <9>; - hub-emul { - compatible = "sandbox,usb-hub"; - #address-cells = <1>; - #size-cells = <0>; - flash-stick { - reg = <0>; - compatible = "sandbox,usb-flash"; - sandbox,filepath = "testflash.bin"; - }; - - }; - }; - }; - - usb_2: usb@2 { - compatible = "sandbox,usb"; - status = "disabled"; - }; - -};