Re: [PATCH v3] pwm: imx27: workaround of the pwm output bug

[Uwe Kleine-König]

Hello Stefan,

On Wed, Jan 24, 2024 at 12:48:21PM +0100, Stefan Wahren wrote:
> Am 03.01.24 um 13:20 schrieb Francesco Dolcini:
> > Hello Pratik,
> > 
> > On Wed, Jan 03, 2024 at 04:32:00PM +0530, pratikmanvar09@xxxxxxxxx wrote:
> > > From: Clark Wang <xiaoning.wang@xxxxxxx>
> > > 
> > > This fixes the pwm output bug when decrease the duty cycle.
> > > This is a limited workaround for the PWM IP issue TKT0577206.
> > > 
> > > Root cause:
> > > When the SAR FIFO is empty, the new write value will be directly applied
> > > to SAR even the current period is not over.
> > > If the new SAR value is less than the old one, and the counter is
> > > greater than the new SAR value, the current period will not filp the
> > > level. This will result in a pulse with a duty cycle of 100%.
> > > 
> > > Workaround:
> > > Add an old value SAR write before updating the new duty cycle to SAR.
> > > This will keep the new value is always in a not empty fifo, and can be
> > > wait to update after a period finished.
> > > 
> > > Limitation:
> > > This workaround can only solve this issue when the PWM period is longer
> > > than 2us(or <500KHz).
> > > 
> > > Reviewed-by: Jun Li <jun.li@xxxxxxx>
> > > Signed-off-by: Clark Wang <xiaoning.wang@xxxxxxx>
> > > Link: https://github.com/nxp-imx/linux-imx/commit/16181cc4eee61d87cbaba0e5a479990507816317
> > > Tested-by: Pratik Manvar <pratik.manvar@xxxxxxx>
> > > Signed-off-by: Pratik Manvar <pratik.manvar@xxxxxxx>
> > A very similar patch was already send in 2021 [1], did it had review
> > comments not addressed? Please have a look.
> > 
> > In general please refrain from sending a new patch version every other
> > day, while every Linux kernel subsystem has different rules and a
> > difference pace of development, in this specific case sending a v3 just
> > adding your signed-off-by without allowing a little bit of time to wait
> > for more feedback is just not sane.
> > 
> > [1] https://lore.kernel.org/all/?q=dfn%3Adrivers%2Fpwm%2Fpwm-imx27.c+AND+b%3A%22Clark+Wang%22
> thank you, this is very helpful. Unfortunately i don't have the
> knowledge and resources to continue this work.
> 
> @Uwe It seems that you were able to reproduce this issue. Is it possible
> to trigger this via sysfs and some kind of script?

When I read through the linked thread I was surprised to read that I
could reproduce it. My best guess is that I did that using a long period
and an LED connected to the PWM's output to make the effect easily visible.
Something like: Configure for

	.duty_cycle = 1s, .period = 5s

and change that to

	.duty_cycle = 0s, .period = 5s

(e.g. using sysfs) when the output got high.

Best regards
Uwe

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

Attachment: signature.asc
Description: PGP signature