On Sat, Dec 05, 2020 at 12:18:25PM +0100, Johannes Pointner wrote:
> Hello Uwe,
> 
> On Fri, Dec 4, 2020 at 12:48 PM Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> wrote:
> >
> > Hello Johannes,
> >
> > On Thu, Dec 03, 2020 at 01:00:56PM +0100, Johannes Pointner wrote:
> > > I just tested 5.10-rc6 with a imx6dl-board and found an issue
> > > regarding the pwm-backlight.
> > > Using 5.10 at about level 67 I got a new maximum and with level 68
> > > it's restarting at about level 1.
> > > This was working properly for me with kernel 5.4.
> >
> > Reverting only the last hunk helps already I assume? I starred at the
> > patch for some time now and don't see a relevant change.
> Yes, that's right. If I just revert the calculation in the function
> pwm_imx27_apply it works again.
> >
> > Can you please enable PWM_DEBUG and TRACING in the kernel configuration
> > and then do:
> >
> >         echo 1 > /sys/kernel/debug/tracing/events/pwm/enable
> >
> > reproduce a wrong setting (the less you do other than that the easier it
> > will be to analyse the trace) and then send me the contents of
> >
> >         /sys/kernel/debug/tracing/trace
> >
> I attached the trace log. I also added a trace where I
> reverted(trace_revert.log) the commit.
> I did for both logs the following sequence of commands:
> root# echo 1 > /sys/kernel/debug/tracing/events/pwm/enable
> root# echo 0 > /sys/class/backlight/backlight/brightness
> root# echo 1 > /sys/class/backlight/backlight/brightness
> root# echo 50 > /sys/class/backlight/backlight/brightness
> root# echo 67 > /sys/class/backlight/backlight/brightness
> root# echo 68 > /sys/class/backlight/backlight/brightness
> 
> > ? Also please lookup the frequency of the per clk (grep for "pwm" in
> > /sys/kernel/debug/clk/clk_summary).
> root# grep pwm  /sys/kernel/debug/clk/clk_summary
>                               pwm4       1        1        0
> 66000000          0     0  50000

Ah, I understood. The problem is:

	do_div(c, NSEC_PER_SEC * prescale);

with (in your case) prescale = 6. This make the second argument
1000000000 * 6 = 6000000000 which doesn't fit into an u32 and so is
discarded to something considerably smaller.

A quick and dirty fix would be:

diff --git a/drivers/pwm/pwm-imx27.c b/drivers/pwm/pwm-imx27.c
index c50d453552bd..e7449edb6dc1 100644
--- a/drivers/pwm/pwm-imx27.c
+++ b/drivers/pwm/pwm-imx27.c
@@ -235,7 +235,8 @@ static int pwm_imx27_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 
 	period_cycles /= prescale;
 	c = clkrate * state->duty_cycle;
-	do_div(c, NSEC_PER_SEC * prescale);
+	do_div(c, NSEC_PER_SEC);
+	do_div(c, prescale);
 	duty_cycles = c;
 
 	/*


but that's ugly. I'll think about how to properly fix this.

Thanks for your report
Uwe

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