linux-kernel - Re: [PATCH v10 0/3] Change PWM-controlled LED pin active mode and algorithm

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: <zneb3qwgf52zitcbq4wz76shnmhwfkabbsts3sussjpc5s5tsz@uneaxdfp4m2f>
Date: Tue, 21 Jan 2025 08:47:42 +0100
From: Uwe Kleine-König <u.kleine-koenig@...libre.com>
To: Nylon Chen <nylon.chen@...ive.com>
Cc: linux-kernel@...r.kernel.org, linux-pwm@...r.kernel.org, 
	devicetree@...r.kernel.org, Paul Walmsley <paul.walmsley@...ive.com>, 
	Palmer Dabbelt <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>, 
	Samuel Holland <samuel.holland@...ive.com>, linux-riscv@...ts.infradead.org
Subject: Re: [PATCH v10 0/3] Change PWM-controlled LED pin active mode and
 algorithm

Hello,

On Sun, Jan 19, 2025 at 03:03:16PM +0800, Nylon Chen wrote:
> I ran some basic tests by changing the period and duty cycle in both
> decreasing and increasing sequences (see the script below).

What is clk_get_rate(ddata->clk) for you?

> # Backward testing for period (decreasing)
> echo "Testing period backward..."
> 
> seq 15000 -1 5000 | while read p; do
> 
>    echo $p > /sys/class/pwm/pwmchip1/pwm0/period
> 
>    echo "Testing period: $p"
> 
> done
> 
> 
> # Forward testing for period (increasing)
> echo "Testing period forward..."
> 
> seq 5000 1 15000 | while read p; do
> 
>    echo $p > /sys/class/pwm/pwmchip1/pwm0/period
> 
>    echo "Testing period: $p"
> 
> done
> 
> 
> # Backward testing for duty cycle (decreasing)
> echo "Testing duty cycle backward..."
> 
> for duty in $(seq 10000 -1 0); do
> 
>    echo $duty > /sys/class/pwm/pwmchip1/pwm0/duty_cycle
> 
>    echo "Testing duty cycle: $duty"
> 
> done
> 
> 
> # Forward testing for duty cycle (increasing)
> 
> echo "Testing duty cycle forward..."
> 
> for duty in $(seq 0 1 10000); do
> 
>    echo $duty > /sys/class/pwm/pwmchip1/pwm0/duty_cycle
> 
>    echo "Testing duty cycle: $duty"
> 
> done
> 
> 
> 
> In these particular tests, I didn’t see any functional difference or
> unexpected behavior whether I used DIV64_U64_ROUND_CLOSEST() or
> DIV64_U64_ROUND_UP.
> Of course, there’s a chance my tests haven’t covered every scenario,
> so there could still be edge cases I missed.

Just to be sure: You have PWM_DEBUG enabled?

> From what I understand, your main concern is to ensure we never end up
> with a duty cycle that’s smaller than what the user requested, which a
> round-up approach would help guarantee. If you still recommend making
> that change to achieve the desired behavior, I’m happy to update the
> code accordingly(CLOSEST->UP).

No, .apply should round down and so to ensure that

	pwm_get_state(mypwm, &state);
	pwm_apply(mypwm, &state);

doesn't modify the hardware setting, .get_state has to round up.

Best regards
Uwe

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