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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <w34gpyikqc6oz5zjoan3ws4ckw5dlzjytddfw4df26dyflx3uk@itw6b34z5luv>
Date: Thu, 25 Jan 2024 22:28:20 +0100
From: Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
To: Stefan Wahren <wahrenst@....net>
Cc: Francesco Dolcini <francesco@...cini.it>, pratikmanvar09@...il.com, 
	linux-pwm@...r.kernel.org, shawnguo@...nel.org, s.hauer@...gutronix.de, 
	pratik.manvar@....com, linux-kernel@...r.kernel.org, thierry.reding@...il.com, 
	xiaoning.wang@....com, linux-imx@....com, kernel@...gutronix.de, 
	oe-kbuild-all@...ts.linux.dev, festevam@...il.com, linux-arm-kernel@...ts.infradead.org, 
	jun.li@....com
Subject: Re: [PATCH v3] pwm: imx27: workaround of the pwm output bug

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@...il.com wrote:
> > > From: Clark Wang <xiaoning.wang@....com>
> > > 
> > > 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@....com>
> > > Signed-off-by: Clark Wang <xiaoning.wang@....com>
> > > Link: https://github.com/nxp-imx/linux-imx/commit/16181cc4eee61d87cbaba0e5a479990507816317
> > > Tested-by: Pratik Manvar <pratik.manvar@....com>
> > > Signed-off-by: Pratik Manvar <pratik.manvar@....com>
> > 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/ |

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)