diff mbox

[v2,01/16] drivers: pwm: core: use a single of xlate function

Message ID 6441df76-fb4a-ce00-1019-f7ff9143b75e@microchip.com (mailing list archive)
State New, archived
Headers show

Commit Message

Claudiu Beznea Jan. 15, 2018, 8:41 a.m. UTC
Hi Boris,

Thanks for your review. See below my answers.

On 12.01.2018 20:35, Brian Norris wrote:
> Hi,
> 
> On Fri, Jan 12, 2018 at 04:22:48PM +0200, Claudiu Beznea wrote:
>> Remove of_pwm_simple_xlate() and of_pwm_xlate_with_flags() functions
>> and add of_pwm_xlate() which is used in all cases no mather if the OF
>> bindings are with PWM flags or not. This should not affect the old
>> behavior since the xlate will be based on #pwm-cells property of the
>> PWM controller. Based on #pwm-cells property the xlate will consider
>> the flags or not. This will permit the addition of other inputs to OF
>> xlate by just adding proper code at the end of of_pwm_xlate() and a new
>> input to enum pwm_args_xlate_options. With this changes there will be
>> no need to fill of_xlate and of_pwm_n_cells of struct pwm_chip from
>> the drivers probe methods. References in drives to references to of_xlate
>> and of_pwm_n_cells were removed. Drivers which used private of_xlate
>> functions switched to the generic of_pwm_xlate() function which fits
>> for it but with little changes in device trees (these drivers translated
>> differently the "pwms" bindings; the "pwms" bindings now are generic to
>> all drivers and all drivers should provide them in the format described
>> in pwm documentation).
>>
>> Cc: Thierry Reding <thierry.reding@gmail.com>
>> Cc: Mike Dunn <mikedunn@newsguy.com>
>> Cc: Brian Norris <briannorris@chromium.org>
>> Cc: Alexander Shiyan <shc_work@mail.ru>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>> ---
>>
>> This patch (and the next 7) could be applied independetly by this series, if
>> any, but I choosed to have it here since it makes easy the PWM modes parsing.
>> If you feel it could be independently of this series I could send a new version.
>>
>> Also, Thierry, Mike, Brian, Shiyan, please take an extra look over pwm-pxa.c,
>> pwm-cros-ec.c and pwm-clps711x.c since these were moved to use the generic
>> pwms (minimum 2 pwm-cells).
>>
>>  drivers/pwm/core.c             | 56 +++++++++++-------------------------------
>>  drivers/pwm/pwm-atmel-hlcdc.c  |  2 --
>>  drivers/pwm/pwm-atmel-tcb.c    |  2 --
>>  drivers/pwm/pwm-atmel.c        |  6 -----
>>  drivers/pwm/pwm-bcm-iproc.c    |  2 --
>>  drivers/pwm/pwm-bcm-kona.c     |  2 --
>>  drivers/pwm/pwm-bcm2835.c      |  2 --
>>  drivers/pwm/pwm-berlin.c       |  2 --
>>  drivers/pwm/pwm-clps711x.c     | 11 ---------
>>  drivers/pwm/pwm-cros-ec.c      | 20 ---------------
> 
> For pwm-cros-ec.c:
> 
> Nacked-by: Brian Norris <briannorris@chromium.org>
> 
> This is a fiat change of the documented binding, which breaks the RK3399
> Kevin board. That's not how we do device tree.
> 
> You can extend the binding if you want, so you can represent the period
> in the device tree if you'd like (though the value won't mean anything;
> it can't be changed by the kernel), but don't break existing device
> trees.

That wasn't the idea, I wasn't intended to break something. The idea was
to have a generic device tree parsing function since all the drivers,
except pwm-pxa.c, pwm-cros-ec.c and pwm-clps711x.c, uses the same function
to parse DT bindings. And I think, these 3 drivers could use this way of
parsing, which is not something new, is what all the current PWM drivers
uses (except pwm-pxa.c, pwm-cros-ec.c and pwm-clps711x.c). It is true
I have no RK3399 board to run any tests.

pwm-cross-ec.c it is true that it's period cannot be changed. It is fixed, as
I saw in the driver, at EC_PWM_MAX_DUTY=0xffff. The driver itself won't apply
any PWM state if the period is different from EC_PWM_MAX_DUTY.
For this driver, the PWM bindings were changed (I did a grep by "google,cros-ec-pwm"
and located only:
arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
files) and changed the bindings in this series, as follows, patch 7 from this series:

The code that was removed requests a PWM, the one that was set in the bindings, and
then set pwm->args.period:
-static struct pwm_device *
-cros_ec_pwm_xlate(struct pwm_chip *pc, const struct of_phandle_args *args)
-{
-	struct pwm_device *pwm;
-
-	if (args->args[0] >= pc->npwm)
-		return ERR_PTR(-EINVAL);
-
-	pwm = pwm_request_from_chip(pc, args->args[0], NULL);
-	if (IS_ERR(pwm))
-		return pwm;
-
-	/* The EC won't let us change the period */
-	pwm->args.period = EC_PWM_MAX_DUTY;
-
-	return pwm;
-}

The old flow is as follows:
of_pwm_get() -> cros_ec_pwm_xlate() { request chip and set constant period }

The new flow uses of_pwm_xlate():
of_pwm_get() -> of_pwm_xlate() -> { parse PWM args: channel number, period, flags +
request PWM chip + set pwm->args; }

This path is only used at DT parsing.

In case of PWM channel requested by PWM backlight driver it looks good to me
with the changes in rk3399-gru-kevin.dts (please correct me if I'm wrong).

Since this driver accepts only EC_PWM_MAX_DUTY period maybe the documentation should
be updated regarding this value?

Please, let me know what you think!

Thanks,
Claudiu
> 
>>  drivers/pwm/pwm-fsl-ftm.c      |  2 --
>>  drivers/pwm/pwm-hibvt.c        |  2 --
>>  drivers/pwm/pwm-imx.c          |  8 ------
>>  drivers/pwm/pwm-lpc18xx-sct.c  |  2 --
>>  drivers/pwm/pwm-meson.c        |  2 --
>>  drivers/pwm/pwm-omap-dmtimer.c |  2 --
>>  drivers/pwm/pwm-pxa.c          | 19 --------------
>>  drivers/pwm/pwm-renesas-tpu.c  |  2 --
>>  drivers/pwm/pwm-rockchip.c     |  5 ----
>>  drivers/pwm/pwm-samsung.c      |  3 ---
>>  drivers/pwm/pwm-sun4i.c        |  2 --
>>  drivers/pwm/pwm-tiecap.c       |  2 --
>>  drivers/pwm/pwm-tiehrpwm.c     |  2 --
>>  drivers/pwm/pwm-vt8500.c       |  2 --
>>  drivers/pwm/pwm-zx.c           |  2 --
>>  include/linux/pwm.h            | 23 ++++++++++-------
>>  26 files changed, 29 insertions(+), 156 deletions(-)
>>
> ...
> 
>> diff --git a/drivers/pwm/pwm-cros-ec.c b/drivers/pwm/pwm-cros-ec.c
>> index 9c13694eaa24..692298693768 100644
>> --- a/drivers/pwm/pwm-cros-ec.c
>> +++ b/drivers/pwm/pwm-cros-ec.c
>> @@ -133,24 +133,6 @@ static void cros_ec_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
>>  	state->duty_cycle = ret;
>>  }
>>  
>> -static struct pwm_device *
>> -cros_ec_pwm_xlate(struct pwm_chip *pc, const struct of_phandle_args *args)
>> -{
>> -	struct pwm_device *pwm;
>> -
>> -	if (args->args[0] >= pc->npwm)
>> -		return ERR_PTR(-EINVAL);
>> -
>> -	pwm = pwm_request_from_chip(pc, args->args[0], NULL);
>> -	if (IS_ERR(pwm))
>> -		return pwm;
>> -
>> -	/* The EC won't let us change the period */
>> -	pwm->args.period = EC_PWM_MAX_DUTY;
>> -
>> -	return pwm;
>> -}
>> -
>>  static const struct pwm_ops cros_ec_pwm_ops = {
>>  	.get_state	= cros_ec_pwm_get_state,
>>  	.apply		= cros_ec_pwm_apply,
>> @@ -207,8 +189,6 @@ static int cros_ec_pwm_probe(struct platform_device *pdev)
>>  	/* PWM chip */
>>  	chip->dev = dev;
>>  	chip->ops = &cros_ec_pwm_ops;
>> -	chip->of_xlate = cros_ec_pwm_xlate;
>> -	chip->of_pwm_n_cells = 1;
>>  	chip->base = -1;
>>  	ret = cros_ec_num_pwms(ec);
>>  	if (ret < 0) {
> 
> ...
> 
> Brian
>

Comments

Claudiu Beznea Jan. 15, 2018, 12:43 p.m. UTC | #1
On 15.01.2018 10:41, Claudiu Beznea wrote:
> Hi Boris,
s/Boris/Brian

> 
> Thanks for your review. See below my answers.
> 
> On 12.01.2018 20:35, Brian Norris wrote:
>> Hi,
>>
>> On Fri, Jan 12, 2018 at 04:22:48PM +0200, Claudiu Beznea wrote:
>>> Remove of_pwm_simple_xlate() and of_pwm_xlate_with_flags() functions
>>> and add of_pwm_xlate() which is used in all cases no mather if the OF
>>> bindings are with PWM flags or not. This should not affect the old
>>> behavior since the xlate will be based on #pwm-cells property of the
>>> PWM controller. Based on #pwm-cells property the xlate will consider
>>> the flags or not. This will permit the addition of other inputs to OF
>>> xlate by just adding proper code at the end of of_pwm_xlate() and a new
>>> input to enum pwm_args_xlate_options. With this changes there will be
>>> no need to fill of_xlate and of_pwm_n_cells of struct pwm_chip from
>>> the drivers probe methods. References in drives to references to of_xlate
>>> and of_pwm_n_cells were removed. Drivers which used private of_xlate
>>> functions switched to the generic of_pwm_xlate() function which fits
>>> for it but with little changes in device trees (these drivers translated
>>> differently the "pwms" bindings; the "pwms" bindings now are generic to
>>> all drivers and all drivers should provide them in the format described
>>> in pwm documentation).
>>>
>>> Cc: Thierry Reding <thierry.reding@gmail.com>
>>> Cc: Mike Dunn <mikedunn@newsguy.com>
>>> Cc: Brian Norris <briannorris@chromium.org>
>>> Cc: Alexander Shiyan <shc_work@mail.ru>
>>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>>> ---
>>>
>>> This patch (and the next 7) could be applied independetly by this series, if
>>> any, but I choosed to have it here since it makes easy the PWM modes parsing.
>>> If you feel it could be independently of this series I could send a new version.
>>>
>>> Also, Thierry, Mike, Brian, Shiyan, please take an extra look over pwm-pxa.c,
>>> pwm-cros-ec.c and pwm-clps711x.c since these were moved to use the generic
>>> pwms (minimum 2 pwm-cells).
>>>
>>>  drivers/pwm/core.c             | 56 +++++++++++-------------------------------
>>>  drivers/pwm/pwm-atmel-hlcdc.c  |  2 --
>>>  drivers/pwm/pwm-atmel-tcb.c    |  2 --
>>>  drivers/pwm/pwm-atmel.c        |  6 -----
>>>  drivers/pwm/pwm-bcm-iproc.c    |  2 --
>>>  drivers/pwm/pwm-bcm-kona.c     |  2 --
>>>  drivers/pwm/pwm-bcm2835.c      |  2 --
>>>  drivers/pwm/pwm-berlin.c       |  2 --
>>>  drivers/pwm/pwm-clps711x.c     | 11 ---------
>>>  drivers/pwm/pwm-cros-ec.c      | 20 ---------------
>>
>> For pwm-cros-ec.c:
>>
>> Nacked-by: Brian Norris <briannorris@chromium.org>
>>
>> This is a fiat change of the documented binding, which breaks the RK3399
>> Kevin board. That's not how we do device tree.
>>
>> You can extend the binding if you want, so you can represent the period
>> in the device tree if you'd like (though the value won't mean anything;
>> it can't be changed by the kernel), but don't break existing device
>> trees.
> 
> That wasn't the idea, I wasn't intended to break something. The idea was
> to have a generic device tree parsing function since all the drivers,
> except pwm-pxa.c, pwm-cros-ec.c and pwm-clps711x.c, uses the same function
> to parse DT bindings. And I think, these 3 drivers could use this way of
> parsing, which is not something new, is what all the current PWM drivers
> uses (except pwm-pxa.c, pwm-cros-ec.c and pwm-clps711x.c). It is true
> I have no RK3399 board to run any tests.
> 
> pwm-cross-ec.c it is true that it's period cannot be changed. It is fixed, as
> I saw in the driver, at EC_PWM_MAX_DUTY=0xffff. The driver itself won't apply
> any PWM state if the period is different from EC_PWM_MAX_DUTY.
> For this driver, the PWM bindings were changed (I did a grep by "google,cros-ec-pwm"
> and located only:
> arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
> arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
> files) and changed the bindings in this series, as follows, patch 7 from this series:
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
> index 0384e3121f18..0c790ec387eb 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
> @@ -77,7 +77,7 @@
>  
>         backlight: backlight {
>                 compatible = "pwm-backlight";
> -               pwms = <&cros_ec_pwm 1>;
> +               pwms = <&cros_ec_pwm 1 65535>;
>                 brightness-levels = <0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16
>                                      17 18 19 20 21 22 23 24 25 26 27 28 29 30
>                                      31 32 33 34 35 36 37 38 39 40 41 42 43 44
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
> index 5772c52fbfd3..aa377f9ae6ad 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
> @@ -853,7 +853,7 @@ ap_i2c_audio: &i2c8 {
>  
>                 cros_ec_pwm: ec-pwm {
>                         compatible = "google,cros-ec-pwm";
> -                       #pwm-cells = <1>;
> +                       #pwm-cells = <2>;
>                 };
>         };
> };
> 
> The code that was removed requests a PWM, the one that was set in the bindings, and
> then set pwm->args.period:
> -static struct pwm_device *
> -cros_ec_pwm_xlate(struct pwm_chip *pc, const struct of_phandle_args *args)
> -{
> -	struct pwm_device *pwm;
> -
> -	if (args->args[0] >= pc->npwm)
> -		return ERR_PTR(-EINVAL);
> -
> -	pwm = pwm_request_from_chip(pc, args->args[0], NULL);
> -	if (IS_ERR(pwm))
> -		return pwm;
> -
> -	/* The EC won't let us change the period */
> -	pwm->args.period = EC_PWM_MAX_DUTY;
> -
> -	return pwm;
> -}
> 
> The old flow is as follows:
> of_pwm_get() -> cros_ec_pwm_xlate() { request chip and set constant period }
> 
> The new flow uses of_pwm_xlate():
> of_pwm_get() -> of_pwm_xlate() -> { parse PWM args: channel number, period, flags +
> request PWM chip + set pwm->args; }
> 
> This path is only used at DT parsing.
> 
> In case of PWM channel requested by PWM backlight driver it looks good to me
> with the changes in rk3399-gru-kevin.dts (please correct me if I'm wrong).
> 
> Since this driver accepts only EC_PWM_MAX_DUTY period maybe the documentation should
> be updated regarding this value?
> 
> Please, let me know what you think!
> 
> Thanks,
> Claudiu
>>
>>>  drivers/pwm/pwm-fsl-ftm.c      |  2 --
>>>  drivers/pwm/pwm-hibvt.c        |  2 --
>>>  drivers/pwm/pwm-imx.c          |  8 ------
>>>  drivers/pwm/pwm-lpc18xx-sct.c  |  2 --
>>>  drivers/pwm/pwm-meson.c        |  2 --
>>>  drivers/pwm/pwm-omap-dmtimer.c |  2 --
>>>  drivers/pwm/pwm-pxa.c          | 19 --------------
>>>  drivers/pwm/pwm-renesas-tpu.c  |  2 --
>>>  drivers/pwm/pwm-rockchip.c     |  5 ----
>>>  drivers/pwm/pwm-samsung.c      |  3 ---
>>>  drivers/pwm/pwm-sun4i.c        |  2 --
>>>  drivers/pwm/pwm-tiecap.c       |  2 --
>>>  drivers/pwm/pwm-tiehrpwm.c     |  2 --
>>>  drivers/pwm/pwm-vt8500.c       |  2 --
>>>  drivers/pwm/pwm-zx.c           |  2 --
>>>  include/linux/pwm.h            | 23 ++++++++++-------
>>>  26 files changed, 29 insertions(+), 156 deletions(-)
>>>
>> ...
>>
>>> diff --git a/drivers/pwm/pwm-cros-ec.c b/drivers/pwm/pwm-cros-ec.c
>>> index 9c13694eaa24..692298693768 100644
>>> --- a/drivers/pwm/pwm-cros-ec.c
>>> +++ b/drivers/pwm/pwm-cros-ec.c
>>> @@ -133,24 +133,6 @@ static void cros_ec_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
>>>  	state->duty_cycle = ret;
>>>  }
>>>  
>>> -static struct pwm_device *
>>> -cros_ec_pwm_xlate(struct pwm_chip *pc, const struct of_phandle_args *args)
>>> -{
>>> -	struct pwm_device *pwm;
>>> -
>>> -	if (args->args[0] >= pc->npwm)
>>> -		return ERR_PTR(-EINVAL);
>>> -
>>> -	pwm = pwm_request_from_chip(pc, args->args[0], NULL);
>>> -	if (IS_ERR(pwm))
>>> -		return pwm;
>>> -
>>> -	/* The EC won't let us change the period */
>>> -	pwm->args.period = EC_PWM_MAX_DUTY;
>>> -
>>> -	return pwm;
>>> -}
>>> -
>>>  static const struct pwm_ops cros_ec_pwm_ops = {
>>>  	.get_state	= cros_ec_pwm_get_state,
>>>  	.apply		= cros_ec_pwm_apply,
>>> @@ -207,8 +189,6 @@ static int cros_ec_pwm_probe(struct platform_device *pdev)
>>>  	/* PWM chip */
>>>  	chip->dev = dev;
>>>  	chip->ops = &cros_ec_pwm_ops;
>>> -	chip->of_xlate = cros_ec_pwm_xlate;
>>> -	chip->of_pwm_n_cells = 1;
>>>  	chip->base = -1;
>>>  	ret = cros_ec_num_pwms(ec);
>>>  	if (ret < 0) {
>>
>> ...
>>
>> Brian
>>
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
Andy Shevchenko Jan. 15, 2018, 8:27 p.m. UTC | #2
On Mon, Jan 15, 2018 at 10:41 AM, Claudiu Beznea
<Claudiu.Beznea@microchip.com> wrote:

> For this driver, the PWM bindings were changed (I did a grep by "google,cros-ec-pwm"
> and located only:
> arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
> arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
> files) and changed the bindings in this series, as follows, patch 7 from this series:
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
> index 0384e3121f18..0c790ec387eb 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
> @@ -77,7 +77,7 @@
>
>         backlight: backlight {
>                 compatible = "pwm-backlight";
> -               pwms = <&cros_ec_pwm 1>;
> +               pwms = <&cros_ec_pwm 1 65535>;

This shows an breakage for user. The old PWM device tree sources or
binaries should work independently on what changes you did to kernel.
Claudiu Beznea Jan. 16, 2018, 8:24 a.m. UTC | #3
On 15.01.2018 22:27, Andy Shevchenko wrote:
> On Mon, Jan 15, 2018 at 10:41 AM, Claudiu Beznea
> <Claudiu.Beznea@microchip.com> wrote:
> 
>> For this driver, the PWM bindings were changed (I did a grep by "google,cros-ec-pwm"
>> and located only:
>> arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
>> arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
>> files) and changed the bindings in this series, as follows, patch 7 from this series:
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
>> index 0384e3121f18..0c790ec387eb 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
>> @@ -77,7 +77,7 @@
>>
>>         backlight: backlight {
>>                 compatible = "pwm-backlight";
>> -               pwms = <&cros_ec_pwm 1>;
>> +               pwms = <&cros_ec_pwm 1 65535>;
> 
> This shows an breakage for user.
As long as pwm-cells=2 the OF hooks will read PWM channel and PWM period
(e.g. in this case, PWM channel=1, PWM period=65532)
I don't see a breakage here. Please explain me further.

The old PWM device tree sources or
> binaries should work independently on what changes you did to kernel.
Please explain me further. From this I understand, as a general rule,
that the device tree binaries from, e.g. 3 years ago, should be
compatible with, e.g. the current version of kernel?

Thanks,
Claudiu
>
Brian Norris Jan. 17, 2018, 11:14 p.m. UTC | #4
On Tue, Jan 16, 2018 at 10:24:59AM +0200, Claudiu Beznea wrote:
> Please explain me further. From this I understand, as a general rule,
> that the device tree binaries from, e.g. 3 years ago, should be
> compatible with, e.g. the current version of kernel?

Yes.

https://www.kernel.org/doc/Documentation/devicetree/bindings/ABI.txt
Claudiu Beznea Jan. 18, 2018, 9:11 a.m. UTC | #5
On 18.01.2018 01:14, Brian Norris wrote:
> On Tue, Jan 16, 2018 at 10:24:59AM +0200, Claudiu Beznea wrote:
>> Please explain me further. From this I understand, as a general rule,
>> that the device tree binaries from, e.g. 3 years ago, should be
>> compatible with, e.g. the current version of kernel?
> 
> Yes.
> 
> https://www.kernel.org/doc/Documentation/devicetree/bindings/ABI.txt
> 
I agree, I found that after some research.

Thanks,
Claudiu
diff mbox

Patch

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
index 0384e3121f18..0c790ec387eb 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
@@ -77,7 +77,7 @@ 
 
        backlight: backlight {
                compatible = "pwm-backlight";
-               pwms = <&cros_ec_pwm 1>;
+               pwms = <&cros_ec_pwm 1 65535>;
                brightness-levels = <0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16
                                     17 18 19 20 21 22 23 24 25 26 27 28 29 30
                                     31 32 33 34 35 36 37 38 39 40 41 42 43 44
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
index 5772c52fbfd3..aa377f9ae6ad 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
@@ -853,7 +853,7 @@  ap_i2c_audio: &i2c8 {
 
                cros_ec_pwm: ec-pwm {
                        compatible = "google,cros-ec-pwm";
-                       #pwm-cells = <1>;
+                       #pwm-cells = <2>;
                };
        };
};