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

From: Andy Shevchenko
Date: Wed Jan 18 2017 - 06:11:54 EST


On Wed, 2017-01-18 at 11:57 +0100, Thierry Reding wrote:
> On Tue, Dec 13, 2016 at 04:52:51PM +0100, Clemens Gruber wrote:
> > Until now, we assumed that the period is the hardware default of
> > 1/200Hz
> > at probe time, but if the period was changed and the user reboots,
> > this
> > assumption is wrong.
> >
> > Solution: Check if the prescaler is set to the hardware default. If
> > not,
> > reprogram the prescaler at first configuration.

AFAIU the PWM is off until user space or kernel user enables it
explicitly after reboot. Is it correct?

In such case we may just enforce default period to be one we are
expecting.

> >
> > Cc: <stable@xxxxxxxxxxxxxxx> # v4.3+
> > Signed-off-by: Clemens Gruber <clemens.gruber@xxxxxxxxxxxx>
> > ---
> > Âdrivers/pwm/pwm-pca9685.c | 10 ++++++++--
> > Â1 file changed, 8 insertions(+), 2 deletions(-)
>
> Cc'ing Mika and Andy and quoting verbatim for review.
>
> Thierry
>
> > diff --git a/drivers/pwm/pwm-pca9685.c b/drivers/pwm/pwm-pca9685.c
> > index 01a6a83..efc657e 100644
> > --- a/drivers/pwm/pwm-pca9685.c
> > +++ b/drivers/pwm/pwm-pca9685.c
> > @@ -55,6 +55,7 @@
> > Â#define PCA9685_PRESCALE 0xFE
> > Â
> > Â#define PCA9685_PRESCALE_MIN 0x03 /* => max.
> > frequency of 1526 Hz */
> > +#define PCA9685_PRESCALE_DEF 0x1E /* => default
> > frequency of 200 Hz */
> > Â#define PCA9685_PRESCALE_MAX 0xFF /* => min.
> > frequency of 24 Hz */
> > Â
> > Â#define PCA9685_COUNTER_RANGE 4096
> > @@ -289,8 +290,8 @@ static int pca9685_pwm_probe(struct i2c_client
> > *client,
> > Â const struct i2c_device_id *id)
> > Â{
> > Â struct pca9685 *pca;
> > + int prescale, mode2;
> > Â int ret;
> > - int mode2;
> > Â
> > Â pca = devm_kzalloc(&client->dev, sizeof(*pca), GFP_KERNEL);
> > Â if (!pca)
> > @@ -304,10 +305,15 @@ static int pca9685_pwm_probe(struct i2c_client
> > *client,
> > Â return ret;
> > Â }
> > Â pca->duty_ns = 0;
> > - pca->period_ns = PCA9685_DEFAULT_PERIOD;
> > Â
> > Â i2c_set_clientdata(client, pca);
> > Â
> > + regmap_read(pca->regmap, PCA9685_PRESCALE, &prescale);
> > + if (prescale == PCA9685_PRESCALE_DEF)
> > + pca->period_ns = PCA9685_DEFAULT_PERIOD;
> > + else
> > + pca->period_ns = 0;
> > +
> > Â regmap_read(pca->regmap, PCA9685_MODE2, &mode2);
> > Â
> > Â if (device_property_read_bool(&client->dev, "invert"))
> > --Â
> > 2.10.2
> >

--
Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Intel Finland Oy