pwm: Fix setting period with #pwm-cells = <1> and of_pwm_single_xlate()

[PATCH] pwm: Fix setting period with #pwm-cells = <1> and of_pwm_single_xlate()

Posted by Uwe Kleine-König 1 year, 10 months ago

For drivers making use of of_pwm_single_xlate() (i.e. those that don't
pass a hwpwm index) and also don't pass flags, setting period was
wrongly skipped. This affects the pwm-pxa and ti-sn65dsi86 drivers.

Reported-by: Karel Balej <balejk@matfyz.cz>
Fixes: 40ade0c2e794 ("pwm: Let the of_xlate callbacks accept references without period")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/pwm/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 54a62879fffa..ee3ef3f44bc5 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -512,7 +512,7 @@ of_pwm_single_xlate(struct pwm_chip *chip, const struct of_phandle_args *args)
 	if (IS_ERR(pwm))
 		return pwm;
 
-	if (args->args_count > 1)
+	if (args->args_count > 0)
 		pwm->args.period = args->args[0];
 
 	pwm->args.polarity = PWM_POLARITY_NORMAL;
-- 
2.43.0

Re: [PATCH] pwm: Fix setting period with #pwm-cells = <1> and of_pwm_single_xlate()

Posted by Karel Balej 1 year, 10 months ago

Uwe Kleine-König, 2024-03-29T11:35:40+01:00:
> For drivers making use of of_pwm_single_xlate() (i.e. those that don't
> pass a hwpwm index) and also don't pass flags, setting period was
> wrongly skipped. This affects the pwm-pxa and ti-sn65dsi86 drivers.
>
> Reported-by: Karel Balej <balejk@matfyz.cz>
> Fixes: 40ade0c2e794 ("pwm: Let the of_xlate callbacks accept references without period")
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/pwm/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
> index 54a62879fffa..ee3ef3f44bc5 100644
> --- a/drivers/pwm/core.c
> +++ b/drivers/pwm/core.c
> @@ -512,7 +512,7 @@ of_pwm_single_xlate(struct pwm_chip *chip, const struct of_phandle_args *args)
>  	if (IS_ERR(pwm))
>  		return pwm;
>  
> -	if (args->args_count > 1)
> +	if (args->args_count > 0)
>  		pwm->args.period = args->args[0];
>  
>  	pwm->args.polarity = PWM_POLARITY_NORMAL;
> -- 
> 2.43.0

Thank you, this fixes the issue for me.

Tested-by: Karel Balej <balejk@matfyz.cz>

Just a nit: I am not sure if perhaps this being part of the report
thread is sufficient, but generally there should probably also be a
Closes: trailer for regzbot to automatically mark the report as resolved
among other reasons.

Best regards,
K. B.

Re: [PATCH] pwm: Fix setting period with #pwm-cells = <1> and of_pwm_single_xlate()

Posted by Uwe Kleine-König 1 year, 10 months ago

Hello Karel,

On Fri, Mar 29, 2024 at 12:21:15PM +0100, Karel Balej wrote:
> Uwe Kleine-König, 2024-03-29T11:35:40+01:00:
> > For drivers making use of of_pwm_single_xlate() (i.e. those that don't
> > pass a hwpwm index) and also don't pass flags, setting period was
> > wrongly skipped. This affects the pwm-pxa and ti-sn65dsi86 drivers.
> >
> > Reported-by: Karel Balej <balejk@matfyz.cz>
> > Fixes: 40ade0c2e794 ("pwm: Let the of_xlate callbacks accept references without period")
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> >  drivers/pwm/core.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
> > index 54a62879fffa..ee3ef3f44bc5 100644
> > --- a/drivers/pwm/core.c
> > +++ b/drivers/pwm/core.c
> > @@ -512,7 +512,7 @@ of_pwm_single_xlate(struct pwm_chip *chip, const struct of_phandle_args *args)
> >  	if (IS_ERR(pwm))
> >  		return pwm;
> >  
> > -	if (args->args_count > 1)
> > +	if (args->args_count > 0)
> >  		pwm->args.period = args->args[0];
> >  
> >  	pwm->args.polarity = PWM_POLARITY_NORMAL;
> > -- 
> > 2.43.0
> 
> Thank you, this fixes the issue for me.
> 
> Tested-by: Karel Balej <balejk@matfyz.cz>

Great, thanks for your report and test.

> Just a nit: I am not sure if perhaps this being part of the report
> thread is sufficient, but generally there should probably also be a
> Closes: trailer for regzbot to automatically mark the report as resolved
> among other reasons.

I applied this patch and added

Link: https://lore.kernel.org/r/D05IVTPYH35N.2CLDG6LSILRSN@matfyz.cz

to the Signoff area which should be good enough to make the regzbot
recognize this as the matching fix.

Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Re: [PATCH] pwm: Fix setting period with #pwm-cells = <1> and of_pwm_single_xlate()

Posted by Linux regression tracking (Thorsten Leemhuis) 1 year, 10 months ago

On 29.03.24 14:24, Uwe Kleine-König wrote:
> On Fri, Mar 29, 2024 at 12:21:15PM +0100, Karel Balej wrote:
>> Just a nit: I am not sure if perhaps this being part of the report
>> thread is sufficient, but generally there should probably also be a
>> Closes: trailer for regzbot to automatically mark the report as resolved
>> among other reasons.
> 
> I applied this patch and added
> 
> Link: https://lore.kernel.org/r/D05IVTPYH35N.2CLDG6LSILRSN@matfyz.cz
> 
> to the Signoff area which should be good enough to make the regzbot
> recognize this as the matching fix.

Thx for that. FWIW, those tags are not only for regzbot: they are older,
as Linus wants them for good reasons[1]; that's why the docs also tell
people to place them[2] for many years now. But a lot of developer are
either not aware or ignore that.

Ciao, Thorsten

[1] for details, see:
https://lore.kernel.org/all/CAHk-=wjMmSZzMJ3Xnskdg4+GGz=5p5p+GSYyFBTh0f-DgvdBWg@mail.gmail.com/
https://lore.kernel.org/all/CAHk-=wgs38ZrfPvy=nOwVkVzjpM3VFU1zobP37Fwd_h9iAD5JQ@mail.gmail.com/
https://lore.kernel.org/all/CAHk-=wjxzafG-=J8oT30s7upn4RhBs6TX-uVFZ5rME+L5_DoJA@mail.gmail.com/

[2] see Documentation/process/submitting-patches.rst
(http://docs.kernel.org/process/submitting-patches.html) and
Documentation/process/5.Posting.rst
(https://docs.kernel.org/process/5.Posting.html)

--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.