On Wed, Dec 16, 2020 at 11:02:03PM -0500, Sven Van Asbroeck wrote: > Hi Clemens, minor nit below. > > On Wed, Dec 16, 2020 at 7:53 AM Clemens Gruber > <clemens.gru...@pqgruber.com> wrote: > > > > Reset the prescale and ON/OFF registers to their POR default state in > > the probe function. Otherwise, the PWMs could still be active after a > > watchdog reset and reboot, etc. > > > > Signed-off-by: Clemens Gruber <clemens.gru...@pqgruber.com> > > --- > > drivers/pwm/pwm-pca9685.c | 9 +++++++-- > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/pwm/pwm-pca9685.c b/drivers/pwm/pwm-pca9685.c > > index 7b14447f3c05..38aadaf50996 100644 > > --- a/drivers/pwm/pwm-pca9685.c > > +++ b/drivers/pwm/pwm-pca9685.c > > @@ -47,6 +47,7 @@ > > #define PCA9685_ALL_LED_OFF_H 0xFD > > #define PCA9685_PRESCALE 0xFE > > > > +#define PCA9685_PRESCALE_DEF 0x1E /* => default frequency of ~200 Hz > > */ > > #define PCA9685_PRESCALE_MIN 0x03 /* => max. frequency of 1526 Hz */ > > #define PCA9685_PRESCALE_MAX 0xFF /* => min. frequency of 24 Hz */ > > > > @@ -446,9 +447,11 @@ static int pca9685_pwm_probe(struct i2c_client *client, > > reg &= ~(MODE1_ALLCALL | MODE1_SUB1 | MODE1_SUB2 | MODE1_SUB3); > > regmap_write(pca->regmap, PCA9685_MODE1, reg); > > > > - /* Clear all "full off" bits */ > > + /* Reset ON/OFF registers to HW defaults (only full OFF bit is set) > > */ > > + regmap_write(pca->regmap, PCA9685_ALL_LED_ON_L, 0); > > + regmap_write(pca->regmap, PCA9685_ALL_LED_ON_H, 0); > > regmap_write(pca->regmap, PCA9685_ALL_LED_OFF_L, 0); > > - regmap_write(pca->regmap, PCA9685_ALL_LED_OFF_H, 0); > > + regmap_write(pca->regmap, PCA9685_ALL_LED_OFF_H, LED_FULL); > > > > pca->chip.ops = &pca9685_pwm_ops; > > /* Add an extra channel for ALL_LED */ > > @@ -470,8 +473,10 @@ static int pca9685_pwm_probe(struct i2c_client *client, > > /* > > * The chip comes out of power-up in the sleep state, > > * but force it to sleep in case it was woken up before > > + * and set the default prescale value > > */ > > pca9685_set_sleep_mode(pca, true); > > + regmap_write(pca->regmap, PCA9685_PRESCALE, PCA9685_PRESCALE_DEF); > > pm_runtime_set_suspended(&client->dev); > > pm_runtime_enable(&client->dev); > > Consider making it clearer that prescale can only be touched when the chip is > in sleep mode. Suggestion: > > /* set the default prescale value - chip _must_ be in sleep mode */ > regmap_write(pca->regmap, PCA9685_PRESCALE, PCA9685_PRESCALE_DEF);
Good point, thanks. > > > > > -- > > 2.29.2 > >
