[U-Boot] [PATCH 12/12] test: dm: add sandbox PMIC framework tests

[Przemyslaw Marczak <p.marczak@samsung.com>]

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,
-- 
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com