linux-kernel - Re: (EXT) Re: [PATCH 1/4] pwm: pca9685: remove unused duty

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: <CAGngYiUinGeppRdnsWvX4DBLVcHFBSGWamUKdTC+rTY3kpFWcQ@mail.gmail.com>
Date:   Fri, 3 Apr 2020 19:47:58 -0400
From:   Sven Van Asbroeck <thesven73@...il.com>
To:     Matthias Schiffer <matthias.schiffer@...tq-group.com>
Cc:     Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>,
        Thierry Reding <thierry.reding@...il.com>,
        linux-pwm@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Clemens Gruber <clemens.gruber@...ruber.com>
Subject: Re: (EXT) Re: [PATCH 1/4] pwm: pca9685: remove unused duty_cycle
 struct element

On Wed, Feb 26, 2020 at 12:04 PM Matthias Schiffer
<matthias.schiffer@...tq-group.com> wrote:
>
> >  - Is this racy somehow (i.e. can it happen that when going from
> >    duty_cycle/period = 1000/5000 to duty_cycle/period = 4000/10000
> > the
> >    output is 1000/10000 (or 4000/5000) for one cycle)?
>
> It currently is racy. It should be possible to fix that either by
> updating all 4 registers in a single I2C write, or by using the "update
> on ACK" mode which requires all 4 registers to be updated before the
> new setting is applied (I'm not sure if this mode would require using a
> single I2C write as well though).

Matthias, did you verify experimentally that changing the period is racy?

Looking at the datasheet and driver code, it shouldn't be. This is because
the OFF time is set as a proportion of the counter range. When the period
changes from 5000 to 10000, then 5000*20%/5000 and 10000*20%/10000
will result in the same 20% ratio (disregarding rounding errors).

This is documented at the beginning of the driver:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pwm/pwm-pca9685.c?h=v5.6#n25

Should we move that comment to pwm_config(), so future versions of
ourselves won't overlook it?