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

Simon Glass
2015-05-10T13:57:14Z

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.

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

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

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

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

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

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