linux-kernel - Re: [PATCH v5 1/1] pwm: imx27: workaround of the pwm output bug when decrease the duty cycle

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 for Android: free password hash cracker in your pocket

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

Message-ID: <0c69242f-7ce6-463a-b704-a5c0b94389cf@denx.de>
Date: Thu, 12 Sep 2024 02:22:15 +0200
From: Marek Vasut <marex@...x.de>
To: Frank Li <Frank.li@....com>
Cc: conor+dt@...nel.org, devicetree@...r.kernel.org, festevam@...il.com,
 francesco@...cini.it, imx@...ts.linux.dev, jun.li@....com,
 kernel@...gutronix.de, krzk+dt@...nel.org,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
 linux-pwm@...r.kernel.org, p.zabel@...gutronix.de, pratikmanvar09@...il.com,
 robh@...nel.org, s.hauer@...gutronix.de, shawnguo@...nel.org,
 ukleinek@...nel.org, xiaoning.wang@....com
Subject: Re: [PATCH v5 1/1] pwm: imx27: workaround of the pwm output bug when
 decrease the duty cycle

On 9/11/24 11:55 PM, Frank Li wrote:

Hi,

>>> This only resolves the issue when the PWM period is longer than 2us
>>> (or <500KHz) because write register is not quick enough when PWM period is
>>> very short.
>>
>> You did mention the IPS bus is slow. Do I understand it correctly that the
>> IPS bus write takes about 1us ? Because of the PWM consumes a sample every
>> 2us and we need to write 2 samples to avoid FIFO underrun, then to safely
>> write those 2 samples, we need to be able to write 1 sample per 1 us into
>> the FIFO ?
> 
> The above time is just estimated, which variance at difference platform and
> impact by other IPs. If there are pending write/read from GPIO, PWM write
> have to wait for GPIO's write finish. It actually depend on IPS bus's
> loading.
> 
> <500Khz is very less possiblity that write slower than PWM's consumes.
> 
>>
>> Also, would writing more samples help with such "fast" use cases ?
>> Something like this:
>>
>> if (clkrate > 500000) {
>>    // This usleep() could use some further improvement, e.g. calculate
>>    // precise delay for the FIFO to get empty based on PWM clkrate
>>    usleep(2 * 5); // wait 2us for each of the 4 samples in FIFO and a bit
>>    // Now the FIFO is surely empty, write all four FIFO slots
>>    writel_relaxed(imx->duty_cycle, imx->mmio_base + MX3_PWMSAR);
>>    writel_relaxed(imx->duty_cycle, imx->mmio_base + MX3_PWMSAR);
>>    writel_relaxed(imx->duty_cycle, imx->mmio_base + MX3_PWMSAR);
>>    writel_relaxed(imx->duty_cycle, imx->mmio_base + MX3_PWMSAR);
> 
> It can help at some possiblity, but still have problem if > 1Mhz, write
> will always less than consume.
> 
> If errata happen, only 1 cycle is full high. I think it is quite less
> impact at such high frequency.
> 
> We found this problem by observe a screen backlight flick when change
> ducty_cycle. I think we try fix it after a real user visible impact happen.

Indeed, I observed similar problem.

> Put code here can reduce some possiblity at certain freq range, but may
> miss-leading user the problem fixed when > 500k.

You already have good code comments, maybe expanding one would help 
clarify this issue cannot really be fully fixed with current hardware.

I think the multi-write for > 500 kHz could further improve the patch, 
but let's wait for input from others, let's see what they think.