linux-kernel - Re: [PATCH v5 2/7] pwm: pca9685: Support hardware readout

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: <YBRyG0vv3gRzygSB@workstation.tuxnet>
Date:   Fri, 29 Jan 2021 21:37:47 +0100
From:   Clemens Gruber <clemens.gruber@...ruber.com>
To:     Sven Van Asbroeck <thesven73@...il.com>
Cc:     Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>,
        Thierry Reding <thierry.reding@...il.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-pwm@...r.kernel.org
Subject: Re: [PATCH v5 2/7] pwm: pca9685: Support hardware readout

Hi Sven,

On Fri, Jan 29, 2021 at 01:05:14PM -0500, Sven Van Asbroeck wrote:
> Hi Clemens,
> 
> On Fri, Jan 29, 2021 at 11:31 AM Clemens Gruber
> <clemens.gruber@...ruber.com> wrote:
> >
> > Ok, so you suggest we extend our get_state logic to deal with cases
> > like the following:
> 
> Kind of. We can't control how other actors (bootloaders etc) program the
> chip. As far as I know, there are many, many different register settings that
> result in the same physical chip outputs. So if .probe() wants to preserve the
> existing chip settings, .get_state() has to be able to deal with every possible
> setting. Even invalid ones.

Is the driver really responsible for bootloaders that program the chip
with invalid values?
The chip comes out of PoR with sane default values. If the bootloader of
a user messes them up, isn't that a bootloader problem instead of a
Linux kernel driver problem?

> In addition, .apply() cannot make any assumptions as to which bits are
> already set/cleared on the chip. Including preserved, invalid settings.
> 
> This might get quite complex.
> 
> However if we reset the chip in .probe() to a known state (a normalized state,
> in the mathematical sense), then both .get_state() and .apply() become
> much simpler. because they only need to deal with known, normalized states.

Yes, I agree. This would however make it impossible to do a flicker-free
transition from bootloader to kernel, but that's not really a usecase I
have so I can live without it.

Another point in favor of resetting is that the driver already does it.
Removing the reset of the OFF register may break some boards who rely on
that behaviour.
My version only extended the reset to include the ON register.

> 
> In short, it's a tradeoff between code complexity, and user friendliness/
> features.
> 
> Sven

Thierry, Uwe, what's your take on this?

Thierry: Would you accept it if we continue to reset the registers in
.probe?

Thanks,
Clemens