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

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Wed, 2017-01-18 at 15:25 +0100, Clemens Gruber wrote:
> 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.

Yes, and my question is what is possible go wrong if you just enforce
prescaler to be 1/200Hz?

> > > 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.

Same question here.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy