2024-03-28 16:27:33

by Karel Balej

[permalink] [raw]
Subject: [REGRESSION] PWM vibrator does not probe with v6.9-rc1

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


2024-03-29 10:36:01

by Uwe Kleine-König

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

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 <[email protected]>
Fixes: 40ade0c2e794 ("pwm: Let the of_xlate callbacks accept references without period")
Signed-off-by: Uwe Kleine-König <[email protected]>
---
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


2024-03-29 11:21:37

by Karel Balej

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

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 <[email protected]>
> Fixes: 40ade0c2e794 ("pwm: Let the of_xlate callbacks accept references without period")
> Signed-off-by: Uwe Kleine-König <[email protected]>
> ---
> 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 <[email protected]>

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.

2024-03-29 15:04:53

by Uwe Kleine-König

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

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 <[email protected]>
> > Fixes: 40ade0c2e794 ("pwm: Let the of_xlate callbacks accept references without period")
> > Signed-off-by: Uwe Kleine-K?nig <[email protected]>
> > ---
> > 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 <[email protected]>

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/[email protected]

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


Attachments:
(No filename) (1.88 kB)
signature.asc (499.00 B)
Download all attachments
Subject: Re: [PATCH] pwm: Fix setting period with #pwm-cells = <1> and of_pwm_single_xlate()

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/[email protected]
>
> 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.