[U-Boot] [PATCH 12/12] test: dm: add sandbox PMIC framework tests
Przemyslaw Marczak
p.marczak at samsung.com
Mon May 11 09:36:22 CEST 2015
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 at 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 at 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 at 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
More information about the U-Boot
mailing list