[REGRESSION] PWM vibrator does not probe with v6.9-rc1

[REGRESSION] PWM vibrator does not probe with v6.9-rc1

[Karel Balej]

Uwe,

I am working on bringing the mainline Linux to my old smartphone. Most
of the changes are not yet in-tree.

The phone has a PWM vibrator for which the corresponding input driver
(pwm-vibrator) is used. The driver used for the PWM is pwm-pxa (or
pxa25x-pwm).

The DT nodes look like this

[...]
	pwm: pwm@1ac00 {
		compatible = "marvell,pxa250-pwm";
		reg = <0x1ac00 0x10>;
		#pwm-cells = <1>;
		clocks = <&apbc PXA1908_CLK_PWM3>;
	};
[...]
	vibrator {
		compatible = "pwm-vibrator";
		pwm-names = "enable";
		pwms = <&pwm 100000>;
		enable-gpios = <&gpio 20 GPIO_ACTIVE_HIGH>;
		pinctrl-names = "default";
		pinctrl-0 = <&vibrator_pins>;
	};
[...]

The vibrator worked fine with v6.8-rc6 but after I rebased to v6.9-rc1,
it no longer probes printing

	[  +0.000118] pwm-vibrator vibrator: failed to apply initial PWM state: -22

to dmesg.

I have bisected the problem to 40ade0c2e794 ("pwm: Let the of_xlate
callbacks accept references without period").

Looking at the commit and adjacent history, I don't believe this problem
is caused by this still being an out-of-tree DT, nonetheless, if it
proves to be the case, then I apologize for false alarm.

Would you please take a look?

Thank you and kind regards,
K. B.

#regzbot introduced: 40ade0c2e794

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

[Uwe Kleine-König]

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()

[Karel Balej]

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()

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1927 bytes --]

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/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Linux regression tracking (Thorsten Leemhuis)]

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.