linux-kernel - Re: [PATCH 2/2] pwm: pca9685: fix prescaler initialization

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: <20170118142533.GA17640@archie.localdomain>
Date:   Wed, 18 Jan 2017 15:25:33 +0100
From:   Clemens Gruber <clemens.gruber@...ruber.com>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Thierry Reding <thierry.reding@...il.com>
Cc:     linux-pwm@...r.kernel.org, linux-kernel@...r.kernel.org,
        Florian Vaussard <florian.vaussard@...g-vd.ch>,
        Mika Westerberg <mika.westerberg@...ux.intel.com>
Subject: Re: [PATCH 2/2] pwm: pca9685: fix prescaler initialization

On Wed, Jan 18, 2017 at 04:01:58PM +0200, Andy Shevchenko wrote:
> On Wed, 2017-01-18 at 14:53 +0100, Clemens Gruber wrote:
> > Yes, but the period could be different, maybe modified in the
> > bootloader
> > or at a previous boot without hardware reset in between. (We do not
> > send
> > a SWRST to the chip, so the period register could be different)
> 
> It's fragile to rely on some external settings, right? Wouldn't be
> better to leave device in a known state after ->probe()?

Yes, that's what this patch tries to solve by verifying that the
external setting (the prescale register) is set to its hardware default
value of 0x1E (corresponding to a period of 1/200 Hz).
If it is not 0x1E, the driver will reconfigure the prescaler according
to the desired period at the time of the next configuration.

> 
> > Until now, we assumed it is always 1/200 Hz and skipped the lengthy
> > prescale configuration (put chip into sleep mode, set prescaler, go
> > out
> > of sleep mode, udelay for 0.5ms until the oscillator is back up) if
> > the
> > user wants a period of 1/200 Hz.
> > 
> > With this patch, we check if it is in fact set to the hardware
> > default.
> > If not, we set pca->period_ns to 0 which leads to changing the
> > prescaler
> > in the next call to pca9685_pwm_config.
> 
> And this contradicts, for my opinion, to the logic you referred in the
> first paragraph. If you would like to use preset values, you need to
> read and recalculate period correctly.

I don't want to use preset values. I wanted to be sure that we do not
skip the prescaler configuration if the prescale register was modified.
We should only skip it, if it is already at 0x1E.

Thanks,
Clemens