linux-kernel - Re: [PATCH v4 1/4] pwm: pca9685: Switch to atomic API

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:   Thu, 10 Dec 2020 16:37:01 -0500
From:   Sven Van Asbroeck <thesven73@...il.com>
To:     Clemens Gruber <clemens.gruber@...ruber.com>
Cc:     Thierry Reding <thierry.reding@...il.com>,
        Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>, linux-pwm@...r.kernel.org,
        Lee Jones <lee.jones@...aro.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Mika Westerberg <mika.westerberg@...ux.intel.com>,
        David Jander <david@...tonic.nl>
Subject: Re: [PATCH v4 1/4] pwm: pca9685: Switch to atomic API

On Thu, Dec 10, 2020 at 3:54 PM Clemens Gruber
<clemens.gruber@...ruber.com> wrote:
>
> After reading your reasoning in this mail and rethinking the whole
> situation, I do no longer have any objections to the more complex
> solution. (Allowing 0% and 100% duty cycle channels with any period)
>
> I first thought it would be too confusing to block users from changing a
> duty cycle when in reality the period is the problem.
> However, if we log an error message, explaining that the periods have to
> match if duty cycles > 0 and < 100% are used, I think it's OK.
>
> Uwe, Sven: Do you have any objections?

No objections, as long as the "fully flexible" solution doesn't get too
complex. Complex code is hard to maintain and extend, but obviously that
decision is for Thierry or Uwe to make.

Thinking this through a little bit, I don't think the "fully flexible"
solution has to be that complex. Keeping track of prescale-inuse may
have to involve a bitmask and not a counter, such as here:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pwm/pwm-pca9685.c?h=v5.10-rc7#n81

That way, channels can do set_bit() to mark that they use the prescaler,
clear_bit() when giving up the prescaler, and if (bitmap_weight() <= 1) to
check if they can make a prescale change.

But this is theory - the only way to find out what's best, is to actually
write the code...

> If not, I will prepare a v5 next week.

Looking forward to it, thank you !!