[PATCH v3 1/2] video: backlight: Support PWMs without a known period_ns

[PATCH v3 1/2] video: backlight: Support PWMs without a known period_ns

[Alper Nebi Yasak]

The PWM device provided by Chrome OS EC doesn't really support anything
other than setting a relative duty cycle. To support it as a backlight,
this patch makes the PWM period optional in the device tree and pretends
the valid brightness range is its period_ns.

Also adds a sandbox test for a PWM channel that has a fixed period,
checking that the resulting duty_cycle matches on a set_config() even if
the requested period_ns can't be set.

Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

Changes in v3:
- Add tag: "Reviewed-by: Simon Glass <sjg@chromium.org>"
- Adjust to match recently merged duty cycle calculation fix.

v2: https://patchwork.ozlabs.org/project/uboot/patch/20200926200851.4388-1-alpernebiyasak at gmail.com/

Changes in v2:
- Add sandbox test to pwm.c (using sandbox_pwm's channel 2)
- Add comments to pwm_set_config() and pwm_ops->set_config()

v1: https://patchwork.ozlabs.org/project/uboot/patch/20200923165231.18188-1-alpernebiyasak at gmail.com/

 drivers/pwm/sandbox_pwm.c     | 11 +++++++++--
 drivers/video/pwm_backlight.c | 20 ++++++++++++++------
 include/pwm.h                 |  8 ++++++++
 test/dm/pwm.c                 | 11 +++++++++++
 4 files changed, 42 insertions(+), 8 deletions(-)

diff --git a/drivers/pwm/sandbox_pwm.c b/drivers/pwm/sandbox_pwm.c
index 28988187e039..3fd2e78c909d 100644
--- a/drivers/pwm/sandbox_pwm.c
+++ b/drivers/pwm/sandbox_pwm.c
@@ -59,8 +59,15 @@ static int sandbox_pwm_set_config(struct udevice *dev, uint channel,
 	if (channel >= NUM_CHANNELS)
 		return -ENOSPC;
 	chan = &priv->chan[channel];
-	chan->period_ns = period_ns;
-	chan->duty_ns = duty_ns;
+
+	if (channel == 2) {
+		/* Pretend to have some fixed period */
+		chan->period_ns = 4096;
+		chan->duty_ns =  duty_ns * 4096 / period_ns;
+	} else {
+		chan->period_ns = period_ns;
+		chan->duty_ns = duty_ns;
+	}
 
 	return 0;
 }
diff --git a/drivers/video/pwm_backlight.c b/drivers/video/pwm_backlight.c
index 9519180ceb39..18b7a00b033c 100644
--- a/drivers/video/pwm_backlight.c
+++ b/drivers/video/pwm_backlight.c
@@ -62,10 +62,17 @@ static int set_pwm(struct pwm_backlight_priv *priv)
 	uint duty_cycle;
 	int ret;
 
-	duty_cycle = priv->period_ns * (priv->cur_level - priv->min_level) /
-		(priv->max_level - priv->min_level);
-	ret = pwm_set_config(priv->pwm, priv->channel, priv->period_ns,
-			     duty_cycle);
+	if (priv->period_ns) {
+		duty_cycle = priv->period_ns * (priv->cur_level - priv->min_level) /
+			(priv->max_level - priv->min_level);
+		ret = pwm_set_config(priv->pwm, priv->channel, priv->period_ns,
+				     duty_cycle);
+	} else {
+		/* PWM driver will internally scale these like the above. */
+		ret = pwm_set_config(priv->pwm, priv->channel,
+				     priv->max_level - priv->min_level,
+				     priv->cur_level - priv->min_level);
+	}
 	if (ret)
 		return log_ret(ret);
 
@@ -213,10 +220,11 @@ static int pwm_backlight_ofdata_to_platdata(struct udevice *dev)
 		log_debug("Cannot get PWM: ret=%d\n", ret);
 		return log_ret(ret);
 	}
-	if (args.args_count < 2)
+	if (args.args_count < 1)
 		return log_msg_ret("Not enough arguments to pwm\n", -EINVAL);
 	priv->channel = args.args[0];
-	priv->period_ns = args.args[1];
+	if (args.args_count > 1)
+		priv->period_ns = args.args[1];
 	if (args.args_count > 2)
 		priv->polarity = args.args[2];
 
diff --git a/include/pwm.h b/include/pwm.h
index 54ae3e64c310..97bda0856e6c 100644
--- a/include/pwm.h
+++ b/include/pwm.h
@@ -15,6 +15,10 @@ struct pwm_ops {
 	/**
 	 * set_config() - Set the PWM configuration
 	 *
+	 * Change both the PWM device's period and it's duty period if
+	 * possible. Otherwise, set an appropriate duty period that best
+	 * matches the given period_ns / duty_ns ratio for the device.
+	 *
 	 * @dev:	PWM device to update
 	 * @channel:	PWM channel to update
 	 * @period_ns:	PWM period in nanoseconds
@@ -49,6 +53,10 @@ struct pwm_ops {
 /**
  * pwm_set_config() - Set the PWM configuration
  *
+ * Change both the PWM device's period and it's duty period if
+ * possible. Otherwise, set an appropriate duty period that best
+ * matches the given period_ns / duty_ns ratio for the device.
+ *
  * @dev:	PWM device to update
  * @channel:	PWM channel to update
  * @period_ns:	PWM period in nanoseconds
diff --git a/test/dm/pwm.c b/test/dm/pwm.c
index 0de6dba1ba64..b624cf3d6558 100644
--- a/test/dm/pwm.c
+++ b/test/dm/pwm.c
@@ -6,6 +6,7 @@
 #include <common.h>
 #include <dm.h>
 #include <pwm.h>
+#include <asm/test.h>
 #include <dm/test.h>
 #include <test/test.h>
 #include <test/ut.h>
@@ -14,6 +15,10 @@
 static int dm_test_pwm_base(struct unit_test_state *uts)
 {
 	struct udevice *dev;
+	uint period_ns;
+	uint duty_ns;
+	bool enable;
+	bool polarity;
 
 	ut_assertok(uclass_get_device(UCLASS_PWM, 0, &dev));
 	ut_assertnonnull(dev);
@@ -24,6 +29,12 @@ static int dm_test_pwm_base(struct unit_test_state *uts)
 	ut_asserteq(-ENOSPC, pwm_set_enable(dev, 3, true));
 	ut_assertok(pwm_set_invert(dev, 0, true));
 
+	ut_assertok(pwm_set_config(dev, 2, 100, 50));
+	ut_assertok(sandbox_pwm_get_config(dev, 2, &period_ns, &duty_ns,
+					   &enable, &polarity));
+	ut_asserteq(period_ns, 4096);
+	ut_asserteq(duty_ns, 50 * 4096 / 100);
+
 	ut_assertok(uclass_get_device(UCLASS_PWM, 1, &dev));
 	ut_asserteq(-ENODEV, uclass_get_device(UCLASS_PWM, 2, &dev));
 
-- 
2.28.0

[PATCH v3 2/2] pwm: Add a driver for Chrome OS EC PWM

[Alper Nebi Yasak]

This PWM is used in rk3399-gru-bob and rk3399-gru-kevin to control
the display brightness. We can only change the duty cycle, so on
set_config() we just try to match the duty cycle that dividing duty_ns
by period_ns gives us. To disable, we set the duty cycle to zero while
keeping the old value for when we want to re-enable it.

The cros_ec_set_pwm_duty() function is taken from Depthcharge's
cros_ec_set_bl_pwm_duty() but modified to use the generic pwm type.
The driver itself is very loosely based on rk_pwm.c for the general pwm
driver structure.

The devicetree binding file is from Linux, before it was converted to
YAML at 5df5a577a6b4 ("dt-bindings: pwm: Convert google,cros-ec-pwm.txt
to YAML format") in their repo.

Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
I'm testing on a rk3399-gru-kevin with a lot of other patches to get it
and it's screen working. The actual tree I'm testing is available here:

    https://github.com/alpernebbi/u-boot/tree/rk3399-gru-kevin/wip
    (currently at commit 1c016682bf84b82488311d59adc81691de2a664f)

Changes in v3:
- Add tag: "Reviewed-by: Simon Glass <sjg@chromium.org>"

v2: https://patchwork.ozlabs.org/project/uboot/patch/20200926200851.4388-2-alpernebiyasak at gmail.com/

Changes in v2:
- Add device-tree-binding/pwm/cros-ec-pwm.txt adapted from Linux
- Return ec_command() errors correctly in cros_ec_set_pwm_duty()
- Add newlines before last returns in cros_ec_pwm.c

v1: https://patchwork.ozlabs.org/project/uboot/patch/20200923165231.18188-2-alpernebiyasak at gmail.com/

 doc/device-tree-bindings/pwm/cros-ec-pwm.txt | 23 ++++++
 drivers/misc/cros_ec.c                       | 17 ++++
 drivers/pwm/Kconfig                          |  9 +++
 drivers/pwm/Makefile                         |  1 +
 drivers/pwm/cros_ec_pwm.c                    | 84 ++++++++++++++++++++
 include/cros_ec.h                            | 13 +++
 6 files changed, 147 insertions(+)
 create mode 100644 doc/device-tree-bindings/pwm/cros-ec-pwm.txt
 create mode 100644 drivers/pwm/cros_ec_pwm.c

diff --git a/doc/device-tree-bindings/pwm/cros-ec-pwm.txt b/doc/device-tree-bindings/pwm/cros-ec-pwm.txt
new file mode 100644
index 000000000000..f198d088917b
--- /dev/null
+++ b/doc/device-tree-bindings/pwm/cros-ec-pwm.txt
@@ -0,0 +1,23 @@
+PWM controlled by ChromeOS EC
+
+Google's ChromeOS EC PWM is a simple PWM attached to the Embedded Controller
+(EC) and controlled via a host-command interface.
+
+An EC PWM node should be only found as a sub-node of the EC node (see
+doc/device-tree-bindings/misc/cros-ec.txt).
+
+Required properties:
+- compatible: Must contain "google,cros-ec-pwm"
+- #pwm-cells: Should be 1. The cell specifies the PWM index.
+
+Example:
+	cros-ec at 0 {
+		compatible = "google,cros-ec-spi";
+
+		...
+
+		cros_ec_pwm: ec-pwm {
+			compatible = "google,cros-ec-pwm";
+			#pwm-cells = <1>;
+		};
+	};
diff --git a/drivers/misc/cros_ec.c b/drivers/misc/cros_ec.c
index a5534b16673b..3be2dd6f14ac 100644
--- a/drivers/misc/cros_ec.c
+++ b/drivers/misc/cros_ec.c
@@ -1110,6 +1110,23 @@ int cros_ec_battery_cutoff(struct udevice *dev, uint8_t flags)
 	return 0;
 }
 
+int cros_ec_set_pwm_duty(struct udevice *dev, uint8_t index, uint16_t duty)
+{
+	struct ec_params_pwm_set_duty p;
+	int ret;
+
+	p.duty = duty;
+	p.pwm_type = EC_PWM_TYPE_GENERIC;
+	p.index = index;
+
+	ret = ec_command(dev, EC_CMD_PWM_SET_DUTY, 0, &p, sizeof(p),
+			 NULL, 0);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
 int cros_ec_set_ldo(struct udevice *dev, uint8_t index, uint8_t state)
 {
 	struct ec_params_ldo_set params;
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index b3bd5c6bb7ae..ef17330387bc 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -9,6 +9,15 @@ config DM_PWM
 	  frequency/period can be controlled along with the proportion of that
 	  time that the signal is high.
 
+config PWM_CROS_EC
+	bool "Enable support for the Chrome OS EC PWM"
+	depends on DM_PWM
+	help
+	  This PWM is found on several Chrome OS devices and controlled by
+	  the Chrome OS embedded controller. It may be used to control the
+	  screen brightness and/or the keyboard backlight depending on the
+	  device.
+
 config PWM_EXYNOS
 	bool "Enable support for the Exynos PWM"
 	depends on DM_PWM
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index f21ae7d76eeb..d7458e33a9a8 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -10,6 +10,7 @@
 
 obj-$(CONFIG_DM_PWM)		+= pwm-uclass.o
 
+obj-$(CONFIG_PWM_CROS_EC)	+= cros_ec_pwm.o
 obj-$(CONFIG_PWM_EXYNOS)	+= exynos_pwm.o
 obj-$(CONFIG_PWM_IMX)		+= pwm-imx.o pwm-imx-util.o
 obj-$(CONFIG_PWM_MESON)		+= pwm-meson.o
diff --git a/drivers/pwm/cros_ec_pwm.c b/drivers/pwm/cros_ec_pwm.c
new file mode 100644
index 000000000000..44f4105dfd54
--- /dev/null
+++ b/drivers/pwm/cros_ec_pwm.c
@@ -0,0 +1,84 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include <common.h>
+#include <cros_ec.h>
+#include <dm.h>
+#include <errno.h>
+#include <log.h>
+#include <pwm.h>
+
+struct cros_ec_pwm_priv {
+	bool enabled;
+	uint duty;
+};
+
+static int cros_ec_pwm_set_config(struct udevice *dev, uint channel,
+				  uint period_ns, uint duty_ns)
+{
+	struct cros_ec_pwm_priv *priv = dev_get_priv(dev);
+	uint duty;
+	int ret;
+
+	debug("%s: period_ns=%u, duty_ns=%u asked\n", __func__,
+	      period_ns, duty_ns);
+
+	/* No way to set the period, only a relative duty cycle */
+	duty = EC_PWM_MAX_DUTY * duty_ns / period_ns;
+	if (duty > EC_PWM_MAX_DUTY)
+		duty = EC_PWM_MAX_DUTY;
+
+	if (!priv->enabled) {
+		priv->duty = duty;
+		debug("%s: duty=%#x to-be-set\n", __func__, duty);
+		return 0;
+	}
+
+	ret = cros_ec_set_pwm_duty(dev->parent, channel, duty);
+	if (ret) {
+		debug("%s: duty=%#x failed\n", __func__, duty);
+		return ret;
+	}
+
+	priv->duty = duty;
+	debug("%s: duty=%#x set\n", __func__, duty);
+
+	return 0;
+}
+
+static int cros_ec_pwm_set_enable(struct udevice *dev, uint channel,
+				  bool enable)
+{
+	struct cros_ec_pwm_priv *priv = dev_get_priv(dev);
+	int ret;
+
+	ret = cros_ec_set_pwm_duty(dev->parent, channel,
+				   enable ? priv->duty : 0);
+	if (ret) {
+		debug("%s: enable=%d failed\n", __func__, enable);
+		return ret;
+	}
+
+	priv->enabled = enable;
+	debug("%s: enable=%d (duty=%#x) set\n", __func__,
+	      enable, priv->duty);
+
+	return 0;
+}
+
+static const struct pwm_ops cros_ec_pwm_ops = {
+	.set_config	= cros_ec_pwm_set_config,
+	.set_enable	= cros_ec_pwm_set_enable,
+};
+
+static const struct udevice_id cros_ec_pwm_ids[] = {
+	{ .compatible = "google,cros-ec-pwm" },
+	{ }
+};
+
+U_BOOT_DRIVER(cros_ec_pwm) = {
+	.name	= "cros_ec_pwm",
+	.id	= UCLASS_PWM,
+	.of_match = cros_ec_pwm_ids,
+	.ops	= &cros_ec_pwm_ops,
+	.priv_auto_alloc_size	= sizeof(struct cros_ec_pwm_priv),
+};
diff --git a/include/cros_ec.h b/include/cros_ec.h
index f4b9b7a5c26e..202e5c2c74a6 100644
--- a/include/cros_ec.h
+++ b/include/cros_ec.h
@@ -443,6 +443,19 @@ int cros_ec_efs_verify(struct udevice *dev, enum ec_flash_region region);
  */
 int cros_ec_battery_cutoff(struct udevice *dev, uint8_t flags);
 
+/**
+ * cros_ec_set_pwm_duty() - Set duty cycle of a generic pwm
+ *
+ * Note that duty value needs to be passed to the EC as a 16 bit number
+ * for increased precision.
+ *
+ * @param dev		CROS-EC device
+ * @param index		Index of the pwm
+ * @param duty		Desired duty cycle, in 0..EC_PWM_MAX_DUTY range.
+ * @return 0 if OK, -ve on error
+ */
+int cros_ec_set_pwm_duty(struct udevice *dev, uint8_t index, uint16_t duty);
+
 /**
  * cros_ec_read_limit_power() - Check if power is limited by batter/charger
  *
-- 
2.28.0

[PATCH v3 1/2] video: backlight: Support PWMs without a known period_ns

[Anatolij Gustschin]

On Thu, 22 Oct 2020 23:49:26 +0300
Alper Nebi Yasak alpernebiyasak at gmail.com wrote:
...
>  drivers/pwm/sandbox_pwm.c     | 11 +++++++++--
>  drivers/video/pwm_backlight.c | 20 ++++++++++++++------
>  include/pwm.h                 |  8 ++++++++
>  test/dm/pwm.c                 | 11 +++++++++++
>  4 files changed, 42 insertions(+), 8 deletions(-)

applied to u-boot-video/master, thanks!

--
Anatolij

[PATCH v3 2/2] pwm: Add a driver for Chrome OS EC PWM

[Anatolij Gustschin]

On Thu, 22 Oct 2020 23:49:27 +0300
Alper Nebi Yasak alpernebiyasak at gmail.com wrote:
...
>  doc/device-tree-bindings/pwm/cros-ec-pwm.txt | 23 ++++++
>  drivers/misc/cros_ec.c                       | 17 ++++
>  drivers/pwm/Kconfig                          |  9 +++
>  drivers/pwm/Makefile                         |  1 +
>  drivers/pwm/cros_ec_pwm.c                    | 84 ++++++++++++++++++++
>  include/cros_ec.h                            | 13 +++
>  6 files changed, 147 insertions(+)
>  create mode 100644 doc/device-tree-bindings/pwm/cros-ec-pwm.txt
>  create mode 100644 drivers/pwm/cros_ec_pwm.c

applied to u-boot-video/master, thanks!

--
Anatolij