diff mbox series

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

Message ID 20200923165231.18188-1-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
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(-)

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:
>
> 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
Alper Nebi Yasak Sept. 25, 2020, 3:55 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:
>>
>> 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)
diff mbox series

Patch

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];