linux-kernel - Re: [RFC PATCH v1] regulator: pwm-regulator: Fix continuous get

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]

Date: Fri, 22 Dec 2023 01:17:44 +0100
From: Martin Blumenstingl <martin.blumenstingl@...glemail.com>
To: Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
Cc: lgirdwood@...il.com, broonie@...nel.org, linux-pwm@...r.kernel.org, 
	linux-amlogic@...ts.infradead.org, linux-kernel@...r.kernel.org, 
	Thierry Reding <thierry.reding@...il.com>, Heiner Kallweit <hkallweit1@...il.com>, 
	Dmitry Rokosov <ddrokosov@...rdevices.ru>
Subject: Re: [RFC PATCH v1] regulator: pwm-regulator: Fix continuous
 get_voltage for disabled PWM

Hi Uwe,

On Thu, Dec 21, 2023 at 11:03 PM Uwe Kleine-König
<u.kleine-koenig@...gutronix.de> wrote:
[...]
> Note this isn't save in general. You're implicitly assuming that a
> disabled PWM runs with the minimal supported duty_cycle. Most disabled
> PWMs yield the inactive level (which corresponds to a 0% relative duty
> cycle). But there are exceptions.
Good catch - thank you!

[...]
> Without claiming to understand all implications, I'd say
> pwm_regulator_get_voltage should signal to the caller when the
> duty_cycle isn't contained in [min(max_uV_duty, min_uV_duty),
> max(max_uV_duty, min_uV_duty)].
It seems like there's -ENOTRECOVERABLE that we can signal to the caller.
This makes the following message appear in my kernel log:
  VDDEE: Setting 1100000-1140000uV
Which means that pwm_regulator_set_voltage() is called which will then
pick the minimum voltage.

To make this work I will need to update meson8b-odroidc1.dts (which
isn't a problem, I just want to point it out as it's mandatory for
that solution. also I will send that in a separate patch).

See my attached patch (which replaces the initial patch I sent, it's
not meant to be applied on top).
One question that I still have is whether we are allowed to just
enable the PWM output within pwm_regulator_set_voltage().
Doing so is actually mandatory, otherwise we end up in an infinite
loop of not being able to read the voltage, then sets the minimum
voltage (but leaves the PWM output disabled) which then means that it
can't read back the voltage which means it tries to set the minimum
voltage ....

Best regards,
Martin

View attachment "odroidc1-vddee-fix-to-discuss.diff" of type "text/x-patch" (1688 bytes)