diff mbox series

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

Message ID 20200923165231.18188-2-alpernebiyasak@gmail.com
State Superseded
Delegated to: Anatolij Gustschin
Headers show
Series [1/2] video: backlight: Support PWMs without a known period_ns | expand

Commit Message

Alper Nebi Yasak Sept. 23, 2020, 4:52 p.m. UTC
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.

Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
---
I'm testing on a rk3399-gru-kevin with a lot of other patches to get it
(and it's screen) barely working, but using something like the following
in a rk_board_late_init() added to gru.c gets the backlight to change:

    struct *udevice panel;
    uclass_first_device_err(UCLASS_PANEL, &panel);
    panel_enable_backlight(panel);
    panel_set_backlight(panel, 50);
    panel_set_backlight(panel, 100);

The actual tree I'm testing and how I'm getting it to chainload from the
vendor firmware (see commit message there) is available here:

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

 drivers/misc/cros_ec.c    | 14 +++++++
 drivers/pwm/Kconfig       |  9 +++++
 drivers/pwm/Makefile      |  1 +
 drivers/pwm/cros_ec_pwm.c | 82 +++++++++++++++++++++++++++++++++++++++
 include/cros_ec.h         | 13 +++++++
 5 files changed, 119 insertions(+)
 create mode 100644 drivers/pwm/cros_ec_pwm.c

Comments

Simon Glass Sept. 24, 2020, 4:08 p.m. UTC | #1
Hi Alper,

On Wed, 23 Sep 2020 at 10:52, Alper Nebi Yasak <alpernebiyasak@gmail.com> wrote:
>
> 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.
>
> Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
> ---
> I'm testing on a rk3399-gru-kevin with a lot of other patches to get it
> (and it's screen) barely working, but using something like the following
> in a rk_board_late_init() added to gru.c gets the backlight to change:
>
>     struct *udevice panel;
>     uclass_first_device_err(UCLASS_PANEL, &panel);
>     panel_enable_backlight(panel);
>     panel_set_backlight(panel, 50);
>     panel_set_backlight(panel, 100);
>
> The actual tree I'm testing and how I'm getting it to chainload from the
> vendor firmware (see commit message there) is available here:
>
>     https://github.com/alpernebbi/u-boot/tree/rk3399-gru-kevin/wip
>     (currently at commit 2cdb2cc4fb503dd5f6958e8f30406293e90b835)
>
>  drivers/misc/cros_ec.c    | 14 +++++++
>  drivers/pwm/Kconfig       |  9 +++++
>  drivers/pwm/Makefile      |  1 +
>  drivers/pwm/cros_ec_pwm.c | 82 +++++++++++++++++++++++++++++++++++++++
>  include/cros_ec.h         | 13 +++++++
>  5 files changed, 119 insertions(+)
>  create mode 100644 drivers/pwm/cros_ec_pwm.c

This looks good, but please add a devicetree binding file. Is this the
same as is used in Linux?

>
> diff --git a/drivers/misc/cros_ec.c b/drivers/misc/cros_ec.c
> index a5534b1667..a9a1eb8a25 100644
> --- a/drivers/misc/cros_ec.c
> +++ b/drivers/misc/cros_ec.c
> @@ -1110,6 +1110,20 @@ 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;
> +
> +       p.duty = duty;
> +       p.pwm_type = EC_PWM_TYPE_GENERIC;
> +       p.index = index;
> +
> +       if (ec_command(dev, EC_CMD_PWM_SET_DUTY, 0, &p, sizeof(p),
> +                      NULL, 0) < 0)
> +               return -1;

Please return the error you got. Also -1 is -EPERM which is not what
you mean here.

> +       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 61eb468cde..73b0e5ed1e 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 0f4e84b04d..7af13538de 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_MTK)          += pwm-mtk.o
> diff --git a/drivers/pwm/cros_ec_pwm.c b/drivers/pwm/cros_ec_pwm.c
> new file mode 100644
> index 0000000000..781f1a1eae
> --- /dev/null
> +++ b/drivers/pwm/cros_ec_pwm.c
> @@ -0,0 +1,82 @@
> +// 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);

blank line before last return in function. Same below

> +       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 f4b9b7a5c2..202e5c2c74 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
>

Regards,
Simon
Alper Nebi Yasak Sept. 25, 2020, 4:11 p.m. UTC | #2
On 24/09/2020 19:08, Simon Glass wrote:
> Hi Alper,
> 
> On Wed, 23 Sep 2020 at 10:52, Alper Nebi Yasak <alpernebiyasak@gmail.com> wrote:
>>
>> 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.
>>
>> Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
>> ---
>> I'm testing on a rk3399-gru-kevin with a lot of other patches to get it
>> (and it's screen) barely working, but using something like the following
>> in a rk_board_late_init() added to gru.c gets the backlight to change:
>>
>>     struct *udevice panel;
>>     uclass_first_device_err(UCLASS_PANEL, &panel);
>>     panel_enable_backlight(panel);
>>     panel_set_backlight(panel, 50);
>>     panel_set_backlight(panel, 100);
>>
>> The actual tree I'm testing and how I'm getting it to chainload from the
>> vendor firmware (see commit message there) is available here:
>>
>>     https://github.com/alpernebbi/u-boot/tree/rk3399-gru-kevin/wip
>>     (currently at commit 2cdb2cc4fb503dd5f6958e8f30406293e90b835)
>>
>>  drivers/misc/cros_ec.c    | 14 +++++++
>>  drivers/pwm/Kconfig       |  9 +++++
>>  drivers/pwm/Makefile      |  1 +
>>  drivers/pwm/cros_ec_pwm.c | 82 +++++++++++++++++++++++++++++++++++++++
>>  include/cros_ec.h         | 13 +++++++
>>  5 files changed, 119 insertions(+)
>>  create mode 100644 drivers/pwm/cros_ec_pwm.c
> 
> This looks good, but please add a devicetree binding file. Is this the
> same as is used in Linux?

The binding is the same as in Linux. The binding documentation file
there has been migrated to yaml, but a txt version was available as
well. I'll adapt the txt as there doesn't seem to be any yaml files in
U-Boot doc/device-tree-bindings/**.

>>
>> diff --git a/drivers/misc/cros_ec.c b/drivers/misc/cros_ec.c
>> index a5534b1667..a9a1eb8a25 100644
>> --- a/drivers/misc/cros_ec.c
>> +++ b/drivers/misc/cros_ec.c
>> @@ -1110,6 +1110,20 @@ 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;
>> +
>> +       p.duty = duty;
>> +       p.pwm_type = EC_PWM_TYPE_GENERIC;
>> +       p.index = index;
>> +
>> +       if (ec_command(dev, EC_CMD_PWM_SET_DUTY, 0, &p, sizeof(p),
>> +                      NULL, 0) < 0)
>> +               return -1;
> 
> Please return the error you got. Also -1 is -EPERM which is not what
> you mean here.

OK. I didn't mean -EPERM indeed, but there are still a number of other
functions in the file that do it similar to this, as far as I can tell.

>> +       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 61eb468cde..73b0e5ed1e 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 0f4e84b04d..7af13538de 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_MTK)          += pwm-mtk.o
>> diff --git a/drivers/pwm/cros_ec_pwm.c b/drivers/pwm/cros_ec_pwm.c
>> new file mode 100644
>> index 0000000000..781f1a1eae
>> --- /dev/null
>> +++ b/drivers/pwm/cros_ec_pwm.c
>> @@ -0,0 +1,82 @@
>> +// 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);
> 
> blank line before last return in function. Same below

OK.

>> +       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 f4b9b7a5c2..202e5c2c74 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
>>
> 
> Regards,
> Simon
>
diff mbox series

Patch

diff --git a/drivers/misc/cros_ec.c b/drivers/misc/cros_ec.c
index a5534b1667..a9a1eb8a25 100644
--- a/drivers/misc/cros_ec.c
+++ b/drivers/misc/cros_ec.c
@@ -1110,6 +1110,20 @@  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;
+
+	p.duty = duty;
+	p.pwm_type = EC_PWM_TYPE_GENERIC;
+	p.index = index;
+
+	if (ec_command(dev, EC_CMD_PWM_SET_DUTY, 0, &p, sizeof(p),
+		       NULL, 0) < 0)
+		return -1;
+	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 61eb468cde..73b0e5ed1e 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 0f4e84b04d..7af13538de 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_MTK)		+= pwm-mtk.o
diff --git a/drivers/pwm/cros_ec_pwm.c b/drivers/pwm/cros_ec_pwm.c
new file mode 100644
index 0000000000..781f1a1eae
--- /dev/null
+++ b/drivers/pwm/cros_ec_pwm.c
@@ -0,0 +1,82 @@ 
+// 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 f4b9b7a5c2..202e5c2c74 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
  *