[PATCH] sandbox: cros-ec: Add tests for the cros-ec-pwm driver

[Heinrich Schuchardt <xypron.glpk@gmx.de>]

Am 16. Mai 2021 17:41:44 MESZ schrieb Alper Nebi Yasak <alpernebiyasak@gmail.com>:
>This patch lets sandbox-cros-ec emulate a limited pwm device which has
>multiple channels but can only set a duty cycle for each, as the actual
>EC doesn't expose any functionality or information other than that.
>Mapping non-generic EC_PWM_TYPE_* values to these emulated pwm channels
>is not implemented as nothing in U-Boot uses these types.

This commit messages is full of of abbreviations which makes it unclear. Please, use human readable terms.

I guess you might be talking about

Chromium OS
Embedded Controller
Pulse Width Modulation

But I am not sure.

Best regards

Heinrich

>
>This emulated pwm is then used to test the cros-ec-pwm driver in
>sandbox. Adding the cros-ec-pwm node to the sandbox test device-tree
>unfortunately makes it the first pwm device, so this also touches some
>other tests to make sure they still use the sandbox pwm.
>
>Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
>---
>This depends on a small fix [1] for cros-ec-pwm which otherwise fails
>to
>build.
>
>[1]
>https://patchwork.ozlabs.org/project/uboot/patch/20210514134840.19380-1-alpernebiyasak at gmail.com/
>
> arch/sandbox/dts/test.dts          |  6 +++
> arch/sandbox/include/asm/test.h    | 10 +++++
> configs/sandbox64_defconfig        |  1 +
> configs/sandbox_defconfig          |  1 +
> configs/sandbox_flattree_defconfig |  1 +
> configs/sandbox_noinst_defconfig   |  1 +
> configs/sandbox_spl_defconfig      |  1 +
> drivers/misc/cros_ec_sandbox.c     | 47 +++++++++++++++++++++++
> test/cmd/pwm.c                     | 32 +++++++++++++++-
> test/dm/Makefile                   |  1 +
> test/dm/cros_ec_pwm.c              | 60 ++++++++++++++++++++++++++++++
> test/dm/panel.c                    |  2 +-
> test/dm/pwm.c                      |  6 ++-
> 13 files changed, 164 insertions(+), 5 deletions(-)
> create mode 100644 test/dm/cros_ec_pwm.c
>
>diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
>index 5ca3bc502a43..c684ff0b6db8 100644
>--- a/arch/sandbox/dts/test.dts
>+++ b/arch/sandbox/dts/test.dts
>@@ -139,6 +139,12 @@
> 				size = <0x10000>;
> 			};
> 		};
>+
>+		cros_ec_pwm: cros-ec-pwm {
>+			compatible = "google,cros-ec-pwm";
>+			#pwm-cells = <1>;
>+		};
>+
> 	};
> 
> 	dsi_host: dsi_host {
>diff --git a/arch/sandbox/include/asm/test.h
>b/arch/sandbox/include/asm/test.h
>index 1cb960ac240d..dab1a4ea01b3 100644
>--- a/arch/sandbox/include/asm/test.h
>+++ b/arch/sandbox/include/asm/test.h
>@@ -275,4 +275,14 @@ void sandbox_set_enable_memio(bool enable);
>  */
> void sandbox_cros_ec_set_test_flags(struct udevice *dev, uint flags);
> 
>+/**
>+ * sandbox_cros_ec_get_pwm_duty() - Get EC PWM config for testing
>purposes
>+ *
>+ * @dev: Device to check
>+ * @index: PWM channel index
>+ * @duty: Current duty cycle in 0..EC_PWM_MAX_DUTY range.
>+ * @return 0 if OK, -ENOSPC if the PWM number is invalid
>+ */
>+int sandbox_cros_ec_get_pwm_duty(struct udevice *dev, uint index, uint
>*duty);
>+
> #endif
>diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
>index 9a373bab6fe3..188ce05cfbf1 100644
>--- a/configs/sandbox64_defconfig
>+++ b/configs/sandbox64_defconfig
>@@ -186,6 +186,7 @@ CONFIG_REGULATOR_S5M8767=y
> CONFIG_DM_REGULATOR_SANDBOX=y
> CONFIG_REGULATOR_TPS65090=y
> CONFIG_DM_PWM=y
>+CONFIG_PWM_CROS_EC=y
> CONFIG_PWM_SANDBOX=y
> CONFIG_RAM=y
> CONFIG_REMOTEPROC_SANDBOX=y
>diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
>index bdbf714e2bd9..6c7a2f02718b 100644
>--- a/configs/sandbox_defconfig
>+++ b/configs/sandbox_defconfig
>@@ -223,6 +223,7 @@ CONFIG_DM_REGULATOR_SANDBOX=y
> CONFIG_REGULATOR_TPS65090=y
> CONFIG_DM_REGULATOR_SCMI=y
> CONFIG_DM_PWM=y
>+CONFIG_PWM_CROS_EC=y
> CONFIG_PWM_SANDBOX=y
> CONFIG_RAM=y
> CONFIG_REMOTEPROC_SANDBOX=y
>diff --git a/configs/sandbox_flattree_defconfig
>b/configs/sandbox_flattree_defconfig
>index 853c9440ea02..0844d6ec23eb 100644
>--- a/configs/sandbox_flattree_defconfig
>+++ b/configs/sandbox_flattree_defconfig
>@@ -163,6 +163,7 @@ CONFIG_REGULATOR_S5M8767=y
> CONFIG_DM_REGULATOR_SANDBOX=y
> CONFIG_REGULATOR_TPS65090=y
> CONFIG_DM_PWM=y
>+CONFIG_PWM_CROS_EC=y
> CONFIG_PWM_SANDBOX=y
> CONFIG_RAM=y
> CONFIG_REMOTEPROC_SANDBOX=y
>diff --git a/configs/sandbox_noinst_defconfig
>b/configs/sandbox_noinst_defconfig
>index c7fc98b5569a..629bde1f7ed6 100644
>--- a/configs/sandbox_noinst_defconfig
>+++ b/configs/sandbox_noinst_defconfig
>@@ -181,6 +181,7 @@ CONFIG_REGULATOR_S5M8767=y
> CONFIG_DM_REGULATOR_SANDBOX=y
> CONFIG_REGULATOR_TPS65090=y
> CONFIG_DM_PWM=y
>+CONFIG_PWM_CROS_EC=y
> CONFIG_PWM_SANDBOX=y
> CONFIG_RAM=y
> CONFIG_REMOTEPROC_SANDBOX=y
>diff --git a/configs/sandbox_spl_defconfig
>b/configs/sandbox_spl_defconfig
>index 87223a54d873..aa629e231753 100644
>--- a/configs/sandbox_spl_defconfig
>+++ b/configs/sandbox_spl_defconfig
>@@ -183,6 +183,7 @@ CONFIG_REGULATOR_S5M8767=y
> CONFIG_DM_REGULATOR_SANDBOX=y
> CONFIG_REGULATOR_TPS65090=y
> CONFIG_DM_PWM=y
>+CONFIG_PWM_CROS_EC=y
> CONFIG_PWM_SANDBOX=y
> CONFIG_RAM=y
> CONFIG_REMOTEPROC_SANDBOX=y
>diff --git a/drivers/misc/cros_ec_sandbox.c
>b/drivers/misc/cros_ec_sandbox.c
>index bc01df0904eb..db5e3b0f51a2 100644
>--- a/drivers/misc/cros_ec_sandbox.c
>+++ b/drivers/misc/cros_ec_sandbox.c
>@@ -64,6 +64,7 @@ struct ec_keymatrix_entry {
> 
> enum {
> 	VSTORE_SLOT_COUNT	= 4,
>+	PWM_CHANNEL_COUNT	= 4,
> };
> 
> struct vstore_slot {
>@@ -71,6 +72,10 @@ struct vstore_slot {
> 	u8 data[EC_VSTORE_SLOT_SIZE];
> };
> 
>+struct ec_pwm_channel {
>+	uint duty;	/* not ns, EC_PWM_MAX_DUTY = 100% */
>+};
>+
> /**
>  * struct ec_state - Information about the EC state
>  *
>@@ -85,6 +90,7 @@ struct vstore_slot {
>  * @recovery_req: Keyboard recovery requested
>  * @test_flags: Flags that control behaviour for tests
>  * @slot_locked: Locked vstore slots (mask)
>+ * @pwm: Information per PWM channel
>  */
> struct ec_state {
> 	u8 vbnv_context[EC_VBNV_BLOCK_SIZE_V2];
>@@ -98,6 +104,7 @@ struct ec_state {
> 	bool recovery_req;
> 	uint test_flags;
> 	struct vstore_slot slot[VSTORE_SLOT_COUNT];
>+	struct ec_pwm_channel pwm[PWM_CHANNEL_COUNT];
> } s_state, *g_state;
> 
> /**
>@@ -554,6 +561,33 @@ static int process_cmd(struct ec_state *ec,
> 		len = sizeof(*resp);
> 		break;
> 	}
>+	case EC_CMD_PWM_GET_DUTY: {
>+		const struct ec_params_pwm_get_duty *req = req_data;
>+		struct ec_response_pwm_get_duty *resp = resp_data;
>+		struct ec_pwm_channel *pwm;
>+
>+		if (req->pwm_type != EC_PWM_TYPE_GENERIC)
>+			return -EINVAL;
>+		if (req->index >= PWM_CHANNEL_COUNT)
>+			return -EINVAL;
>+		pwm = &ec->pwm[req->index];
>+		resp->duty = pwm->duty;
>+		len = sizeof(*resp);
>+		break;
>+	}
>+	case EC_CMD_PWM_SET_DUTY: {
>+		const struct ec_params_pwm_set_duty *req = req_data;
>+		struct ec_pwm_channel *pwm;
>+
>+		if (req->pwm_type != EC_PWM_TYPE_GENERIC)
>+			return -EINVAL;
>+		if (req->index >= PWM_CHANNEL_COUNT)
>+			return -EINVAL;
>+		pwm = &ec->pwm[req->index];
>+		pwm->duty = req->duty;
>+		len = 0;
>+		break;
>+	}
> 	default:
> 		printf("   ** Unknown EC command %#02x\n", req_hdr->command);
> 		return -1;
>@@ -619,6 +653,19 @@ void sandbox_cros_ec_set_test_flags(struct udevice
>*dev, uint flags)
> 	ec->test_flags = flags;
> }
> 
>+int sandbox_cros_ec_get_pwm_duty(struct udevice *dev, uint index, uint
>*duty)
>+{
>+	struct ec_state *ec = dev_get_priv(dev);
>+	struct ec_pwm_channel *pwm;
>+
>+	if (index >= PWM_CHANNEL_COUNT)
>+		return -ENOSPC;
>+	pwm = &ec->pwm[index];
>+	*duty = pwm->duty;
>+
>+	return 0;
>+}
>+
> int cros_ec_probe(struct udevice *dev)
> {
> 	struct ec_state *ec = dev_get_priv(dev);
>diff --git a/test/cmd/pwm.c b/test/cmd/pwm.c
>index 5343af83fa34..2fc0b5e40704 100644
>--- a/test/cmd/pwm.c
>+++ b/test/cmd/pwm.c
>@@ -18,16 +18,20 @@ static int dm_test_pwm_cmd(struct unit_test_state
>*uts)
> {
> 	struct udevice *dev;
> 
>+	/* cros-ec-pwm */
> 	ut_assertok(uclass_get_device(UCLASS_PWM, 0, &dev));
> 	ut_assertnonnull(dev);
> 
> 	ut_assertok(console_record_reset_enable());
> 
> 	/* pwm <invert> <pwm_dev_num> <channel> <polarity> */
>-	ut_assertok(run_command("pwm invert 0 0 1", 0));
>+	/* cros-ec-pwm doesn't support invert */
>+	ut_asserteq(1, run_command("pwm invert 0 0 1", 0));
>+	ut_assert_nextline("error(-38)")
> 	ut_assert_console_end();
> 
>-	ut_assertok(run_command("pwm invert 0 0 0", 0));
>+	ut_asserteq(1, run_command("pwm invert 0 0 0", 0));
>+	ut_assert_nextline("error(-38)")
> 	ut_assert_console_end();
> 
> 	/* pwm <config> <pwm_dev_num> <channel> <period_ns> <duty_ns> */
>@@ -41,6 +45,30 @@ static int dm_test_pwm_cmd(struct unit_test_state
>*uts)
> 	ut_assertok(run_command("pwm disable 0 0", 0));
> 	ut_assert_console_end();
> 
>+	/* sandbox-pwm */
>+	ut_assertok(uclass_get_device(UCLASS_PWM, 1, &dev));
>+	ut_assertnonnull(dev);
>+
>+	ut_assertok(console_record_reset_enable());
>+
>+	/* pwm <invert> <pwm_dev_num> <channel> <polarity> */
>+	ut_assertok(run_command("pwm invert 1 0 1", 0));
>+	ut_assert_console_end();
>+
>+	ut_assertok(run_command("pwm invert 1 0 0", 0));
>+	ut_assert_console_end();
>+
>+	/* pwm <config> <pwm_dev_num> <channel> <period_ns> <duty_ns> */
>+	ut_assertok(run_command("pwm config 1 0 10 50", 0));
>+	ut_assert_console_end();
>+
>+	/* pwm <enable/disable> <pwm_dev_num> <channel> */
>+	ut_assertok(run_command("pwm enable 1 0", 0));
>+	ut_assert_console_end();
>+
>+	ut_assertok(run_command("pwm disable 1 0", 0));
>+	ut_assert_console_end();
>+
> 	return 0;
> }
> 
>diff --git a/test/dm/Makefile b/test/dm/Makefile
>index c9644617a1fe..9ef9171a1cbc 100644
>--- a/test/dm/Makefile
>+++ b/test/dm/Makefile
>@@ -30,6 +30,7 @@ obj-$(CONFIG_DM_BOOTCOUNT) += bootcount.o
> obj-$(CONFIG_CLK) += clk.o clk_ccf.o
> obj-$(CONFIG_CPU) += cpu.o
> obj-$(CONFIG_CROS_EC) += cros_ec.o
>+obj-$(CONFIG_PWM_CROS_EC) += cros_ec_pwm.o
> obj-$(CONFIG_DEVRES) += devres.o
> obj-$(CONFIG_DMA) += dma.o
> obj-$(CONFIG_VIDEO_MIPI_DSI) += dsi_host.o
>diff --git a/test/dm/cros_ec_pwm.c b/test/dm/cros_ec_pwm.c
>new file mode 100644
>index 000000000000..f8d6e1e6c40f
>--- /dev/null
>+++ b/test/dm/cros_ec_pwm.c
>@@ -0,0 +1,60 @@
>+// SPDX-License-Identifier: GPL-2.0+
>+
>+#include <common.h>
>+#include <cros_ec.h>
>+#include <dm.h>
>+#include <pwm.h>
>+#include <asm/test.h>
>+#include <dm/test.h>
>+#include <test/test.h>
>+#include <test/ut.h>
>+
>+static int dm_test_cros_ec_pwm(struct unit_test_state *uts)
>+{
>+	struct udevice *pwm;
>+	struct udevice *ec;
>+	uint duty;
>+
>+	ut_assertok(uclass_get_device_by_name(UCLASS_PWM, "cros-ec-pwm",
>&pwm));
>+	ut_assertnonnull(pwm);
>+	ec = dev_get_parent(pwm);
>+	ut_assertnonnull(ec);
>+
>+	ut_assertok(pwm_set_config(pwm, 0, 100, 50));
>+	ut_assertok(pwm_set_enable(pwm, 0, true));
>+	ut_assertok(sandbox_cros_ec_get_pwm_duty(ec, 0, &duty));
>+	ut_asserteq(50 * EC_PWM_MAX_DUTY / 100, duty);
>+
>+	ut_assertok(pwm_set_config(pwm, 0, 15721, 2719));
>+	ut_assertok(pwm_set_enable(pwm, 0, true));
>+	ut_assertok(sandbox_cros_ec_get_pwm_duty(ec, 0, &duty));
>+	ut_asserteq(2719 * EC_PWM_MAX_DUTY / 15721, duty);
>+
>+	ut_assertok(pwm_set_enable(pwm, 0, false));
>+	ut_assertok(sandbox_cros_ec_get_pwm_duty(ec, 0, &duty));
>+	ut_asserteq(0, duty);
>+
>+	ut_assertok(pwm_set_enable(pwm, 0, true));
>+	ut_assertok(sandbox_cros_ec_get_pwm_duty(ec, 0, &duty));
>+	ut_asserteq(2719 * EC_PWM_MAX_DUTY / 15721, duty);
>+
>+	ut_assertok(pwm_set_config(pwm, 1, 1000, 0));
>+	ut_assertok(pwm_set_enable(pwm, 1, true));
>+	ut_assertok(sandbox_cros_ec_get_pwm_duty(ec, 1, &duty));
>+	ut_asserteq(0, duty);
>+
>+	ut_assertok(pwm_set_config(pwm, 2, 1000, 1024));
>+	ut_assertok(pwm_set_enable(pwm, 2, true));
>+	ut_assertok(sandbox_cros_ec_get_pwm_duty(ec, 2, &duty));
>+	ut_asserteq(EC_PWM_MAX_DUTY, duty);
>+
>+	ut_assertok(pwm_set_config(pwm, 3, EC_PWM_MAX_DUTY, 0xABCD));
>+	ut_assertok(pwm_set_enable(pwm, 3, true));
>+	ut_assertok(sandbox_cros_ec_get_pwm_duty(ec, 3, &duty));
>+	ut_asserteq(0xABCD, duty);
>+
>+	ut_asserteq(-EINVAL, pwm_set_enable(pwm, 4, true));
>+
>+	return 0;
>+}
>+DM_TEST(dm_test_cros_ec_pwm, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
>diff --git a/test/dm/panel.c b/test/dm/panel.c
>index 49f5ac7169d3..4d435a0d255c 100644
>--- a/test/dm/panel.c
>+++ b/test/dm/panel.c
>@@ -28,7 +28,7 @@ static int dm_test_panel(struct unit_test_state *uts)
> 	bool polarity;
> 
> 	ut_assertok(uclass_first_device_err(UCLASS_PANEL, &dev));
>-	ut_assertok(uclass_first_device_err(UCLASS_PWM, &pwm));
>+	ut_assertok(uclass_get_device_by_name(UCLASS_PWM, "pwm", &pwm));
> 	ut_assertok(uclass_get_device(UCLASS_GPIO, 1, &gpio));
> 	ut_assertok(regulator_get_by_platname("VDD_EMMC_1.8V", &reg));
> 	ut_assertok(sandbox_pwm_get_config(pwm, 0, &period_ns, &duty_ns,
>diff --git a/test/dm/pwm.c b/test/dm/pwm.c
>index b624cf3d6558..dff626c771ac 100644
>--- a/test/dm/pwm.c
>+++ b/test/dm/pwm.c
>@@ -20,7 +20,7 @@ static int dm_test_pwm_base(struct unit_test_state
>*uts)
> 	bool enable;
> 	bool polarity;
> 
>-	ut_assertok(uclass_get_device(UCLASS_PWM, 0, &dev));
>+	ut_assertok(uclass_get_device_by_name(UCLASS_PWM, "pwm", &dev));
> 	ut_assertnonnull(dev);
> 	ut_assertok(pwm_set_config(dev, 0, 100, 50));
> 	ut_assertok(pwm_set_enable(dev, 0, true));
>@@ -35,8 +35,10 @@ static int dm_test_pwm_base(struct unit_test_state
>*uts)
> 	ut_asserteq(period_ns, 4096);
> 	ut_asserteq(duty_ns, 50 * 4096 / 100);
> 
>+	ut_assertok(uclass_get_device(UCLASS_PWM, 0, &dev));
> 	ut_assertok(uclass_get_device(UCLASS_PWM, 1, &dev));
>-	ut_asserteq(-ENODEV, uclass_get_device(UCLASS_PWM, 2, &dev));
>+	ut_assertok(uclass_get_device(UCLASS_PWM, 2, &dev));
>+	ut_asserteq(-ENODEV, uclass_get_device(UCLASS_PWM, 3, &dev));
> 
> 	return 0;
> }