Re: [PATCH v2] pwm: Ensure for legacy drivers that pwm->state stays consistent

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Uwe,

On Wed, Jun 30, 2021 at 8:48 AM Uwe Kleine-König
<u.kleine-koenig@xxxxxxxxxxxxxx> wrote:
> On Tue, Jun 29, 2021 at 09:44:38PM +0200, Geert Uytterhoeven wrote:
> > On Sat, 1 May 2021, Uwe Kleine-König wrote:
> > > Without this change it can happen that if changing the polarity succeeded
> > > but changing duty_cycle and period failed pwm->state contains a mixture
> > > between the old and the requested state.
> > >
> > > So remember the initial state before starting to modify the configuration
> > > and restore it when one of the required callback fails.
> > >
> > > Compared to the previous implementation .disable() (if necessary) is called
> > > earlier to prevent a glitch.
> > >
> > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
> >
> > Thanks for your patch, which is now commit d7bff84fe7ed8c3b ("pwm:
> > Ensure for legacy drivers that pwm->state stays consistent") in
> > pwm/for-next.
> >
> > This commit broke the backlight on the Atmark Techno Armadillo 800 EVA
> > board (arch/arm/boot/dts/r8a7740-armadillo800eva.dts), which now shows a
> > black screen.  Reverting the commit fixes the problem.
> >
> > Do you have an idea what is wrong, and how to fix it?
>
> I starred at the patch for some time now and couldn't find a problem.
> Looking at drivers/pwm/pwm-renesas-tpu.c I don't see something obvious.
> (The .set_polarity callback is faulty as I doesn't commit the request to
> hardware, but that shouldn't matter here.)
>
> I guess the first request the backlight driver emits is
>
>         .period = 33333,
>         .duty_cycle = 33333,
>         .enabled = true,
>         .polarity = PWM_POLARITY_INVERSED,
>
> which should result into the following driver calls (with and without
> the breaking commit):
>
>         tpu_pwm_set_polarity(chip, pwm, PWM_POLARITY_INVERSED);
>         tpu_pwm_config(chip, pwm, 33333, 33333);
>         tpu_pwm_enable(chip, pwm);
>
> Can you confirm that?

tpu_pwm_config() is no longer called:

     renesas-tpu-pwm e6600000.pwm: tpu_pwm_set_polarity:334: channel
2, polarity = 1
    -renesas-tpu-pwm e6600000.pwm: tpu_pwm_config:257: channel = 2,
duty_ns = 0, period_ns = 33333
    -renesas-tpu-pwm e6600000.pwm: tpu_pwm_config:257: channel = 2,
duty_ns = 33333, period_ns = 33333
     renesas-tpu-pwm e6600000.pwm: tpu_pwm_enable:346: channel 2

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds



[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux