* [PATCH 1/2] video: backlight: Support PWMs without a known period_ns
@ 2020-09-23 16:52 Alper Nebi Yasak
2020-09-23 16:52 ` [PATCH 2/2] pwm: Add a driver for Chrome OS EC PWM Alper Nebi Yasak
2020-09-24 16:08 ` [PATCH 1/2] video: backlight: Support PWMs without a known period_ns Simon Glass
0 siblings, 2 replies; 6+ messages in thread
From: Alper Nebi Yasak @ 2020-09-23 16:52 UTC (permalink / raw)
To: u-boot
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.
Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
---
drivers/video/pwm_backlight.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/video/pwm_backlight.c b/drivers/video/pwm_backlight.c
index 468a5703bd..4c2a307462 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 + 1);
- 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 + 1);
+ 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 + 1,
+ 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];
--
2.28.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] pwm: Add a driver for Chrome OS EC PWM
2020-09-23 16:52 [PATCH 1/2] video: backlight: Support PWMs without a known period_ns Alper Nebi Yasak
@ 2020-09-23 16:52 ` Alper Nebi Yasak
2020-09-24 16:08 ` Simon Glass
2020-09-24 16:08 ` [PATCH 1/2] video: backlight: Support PWMs without a known period_ns Simon Glass
1 sibling, 1 reply; 6+ messages in thread
From: Alper Nebi Yasak @ 2020-09-23 16:52 UTC (permalink / raw)
To: u-boot
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
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
*
--
2.28.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 1/2] video: backlight: Support PWMs without a known period_ns
2020-09-23 16:52 [PATCH 1/2] video: backlight: Support PWMs without a known period_ns Alper Nebi Yasak
2020-09-23 16:52 ` [PATCH 2/2] pwm: Add a driver for Chrome OS EC PWM Alper Nebi Yasak
@ 2020-09-24 16:08 ` Simon Glass
2020-09-25 15:55 ` Alper Nebi Yasak
1 sibling, 1 reply; 6+ messages in thread
From: Simon Glass @ 2020-09-24 16:08 UTC (permalink / raw)
To: u-boot
Hi Alper,
On Wed, 23 Sep 2020 at 10:52, Alper Nebi Yasak <alpernebiyasak@gmail.com> wrote:
>
> 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.
>
> Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
> ---
>
> drivers/video/pwm_backlight.c | 20 ++++++++++++++------
> 1 file changed, 14 insertions(+), 6 deletions(-)
This looks OK.
But please update the comment for pwm_set_config() to describe the
param updates and add a sandbox test to pwm.c
Does this affect the devicetree binding?
Regards,
Simon
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] pwm: Add a driver for Chrome OS EC PWM
2020-09-23 16:52 ` [PATCH 2/2] pwm: Add a driver for Chrome OS EC PWM Alper Nebi Yasak
@ 2020-09-24 16:08 ` Simon Glass
2020-09-25 16:11 ` Alper Nebi Yasak
0 siblings, 1 reply; 6+ messages in thread
From: Simon Glass @ 2020-09-24 16:08 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] video: backlight: Support PWMs without a known period_ns
2020-09-24 16:08 ` [PATCH 1/2] video: backlight: Support PWMs without a known period_ns Simon Glass
@ 2020-09-25 15:55 ` Alper Nebi Yasak
0 siblings, 0 replies; 6+ messages in thread
From: Alper Nebi Yasak @ 2020-09-25 15:55 UTC (permalink / raw)
To: u-boot
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:
>>
>> 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.
>>
>> Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
>> ---
>>
>> drivers/video/pwm_backlight.c | 20 ++++++++++++++------
>> 1 file changed, 14 insertions(+), 6 deletions(-)
>
> This looks OK.
>
> But please update the comment for pwm_set_config() to describe the
> param updates and add a sandbox test to pwm.c
OK. I'm also adding the same comment to pwm_ops->set_config() as it has
the same change in its meaning.
> Does this affect the devicetree binding?
I don't think it does, or more precisely I'm making the backlight driver
support the following where I think the bindings themselves were already
valid:
pwm: pwm {
#pwm-cells = <1>;
[...]
};
backlight: backlight {
compatible = "pwm-backlight";
pwms = <&pwm 0>;
[...]
};
(These with &cros-ec-pwm already exist in rk3399-gru-bob.dts and
rk3399-gru-kevin.dts)
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] pwm: Add a driver for Chrome OS EC PWM
2020-09-24 16:08 ` Simon Glass
@ 2020-09-25 16:11 ` Alper Nebi Yasak
0 siblings, 0 replies; 6+ messages in thread
From: Alper Nebi Yasak @ 2020-09-25 16:11 UTC (permalink / raw)
To: u-boot
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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-09-25 16:11 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-23 16:52 [PATCH 1/2] video: backlight: Support PWMs without a known period_ns Alper Nebi Yasak
2020-09-23 16:52 ` [PATCH 2/2] pwm: Add a driver for Chrome OS EC PWM Alper Nebi Yasak
2020-09-24 16:08 ` Simon Glass
2020-09-25 16:11 ` Alper Nebi Yasak
2020-09-24 16:08 ` [PATCH 1/2] video: backlight: Support PWMs without a known period_ns Simon Glass
2020-09-25 15:55 ` Alper Nebi Yasak
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.