linux-kernel - Re: [PATCH v6 2/2] pwm: add support for NXPs high-side switch MC33XS2410

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] [day] [month] [year] [list]

Message-ID: <nmlnzinyynah3wqgszk7p3ojxdtekeiftmpkusuyqj2yhymd5x@iudiukib25ea>
Date: Wed, 6 Nov 2024 09:13:29 +0100
From: Uwe Kleine-König <u.kleine-koenig@...libre.com>
To: Dimitri Fedrau <dima.fedrau@...il.com>
Cc: Rob Herring <robh+dt@...nel.org>, 
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>, Conor Dooley <conor+dt@...nel.org>, linux-pwm@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6 2/2] pwm: add support for NXPs high-side switch
 MC33XS2410

On Mon, Nov 04, 2024 at 02:07:53PM +0100, Dimitri Fedrau wrote:
> Am Mon, Nov 04, 2024 at 09:52:51AM +0100 schrieb Uwe Kleine-König:
> > `echo 0 > /sys/class/pwm/pwmchip3/pwm0/duty_cycle` should result in
> > MC33XS2410_PWM_CTRL3 having MC33XS2410_PWM_CTRL3_EN(pwm->hwpwm) cleared.
> > When mc33xs2410_pwm_get_state() is called then it returns state->enabled
> > = false and in that case the above mentioned warning doesn't trigger.
>
> Yes, as you explained. But the warning is shown.
> 
> > Where is the misunderstanding?
> 
> if (state->enabled && state->duty_cycle < s2.duty_cycle)
> 	dev_warn(pwmchip_parent(chip),
> 		".apply is supposed to round down duty_cycle (requested: %llu/%llu, applied: %llu/%llu)\n",
> 		state->duty_cycle, state->period,
> 		s2.duty_cycle, s2.period);
> 
> state has previously applied settings and is parameter of pwm_apply_debug,
> in that case s2=s1, and s1 is returned by get_state:
> 
> state->enabled=true
> state->duty_cycle=0
> s2.enabled=false
> s2.duty_cycle=1908
> 
> Due to the code the warning should be raised. If it shouldn't the check
> should be different, something like if (state->enabled && s2.enabled &&
> ...)

This should be fixed with https://lore.kernel.org/linux-pwm/20241105153521.1001864-2-u.kleine-koenig@baylibre.com/T/#u
Test feedback and review very welcome.

Best regards
Uwe

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