On Tue, Nov 24, 2020 at 10:51:23PM -0500, Sven Van Asbroeck wrote:
> On Tue, Nov 24, 2020 at 1:10 PM Clemens Gruber
> <clemens.gru...@pqgruber.com> wrote:
> >
> > The PCA9685 supports staggered LED output ON times to minimize current
> > surges and reduce EMI.
> > When this new option is enabled, the ON times of each channel are
> > delayed by channel number x counter range / 16, which avoids asserting
> > all enabled outputs at the same counter value while still maintaining
> > the configured duty cycle of each output.
> >
> > Signed-off-by: Clemens Gruber <clemens.gru...@pqgruber.com>
> > ---
> >
> > Changes since v1:
> > - Rebased
> >
> > drivers/pwm/pwm-pca9685.c | 35 ++++++++++++++++++++++++++++++++++-
> > 1 file changed, 34 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/pwm/pwm-pca9685.c b/drivers/pwm/pwm-pca9685.c
> > index 2b82b35ff0de..37c80bc8edcf 100644
> > --- a/drivers/pwm/pwm-pca9685.c
> > +++ b/drivers/pwm/pwm-pca9685.c
> > @@ -75,6 +75,7 @@ struct pca9685 {
> > struct pwm_chip chip;
> > struct regmap *regmap;
> > int prescale;
> > + bool staggered_outputs;
> > #if IS_ENABLED(CONFIG_GPIOLIB)
> > struct mutex lock;
> > struct gpio_chip gpio;
> > @@ -329,7 +330,7 @@ static int pca9685_pwm_apply(struct pwm_chip *chip,
> > struct pwm_device *pwm,
> > {
> > struct pca9685 *pca = to_pca(chip);
> > unsigned long long duty, prescale;
> > - unsigned int reg;
> > + unsigned int on, off, reg;
> >
> > if (state->polarity != PWM_POLARITY_NORMAL)
> > return -EOPNOTSUPP;
> > @@ -375,6 +376,32 @@ static int pca9685_pwm_apply(struct pwm_chip *chip,
> > struct pwm_device *pwm,
> > duty = (PCA9685_COUNTER_RANGE - 1) * state->duty_cycle;
> > duty = DIV_ROUND_UP_ULL(duty, state->period);
> >
> > + if (pca->staggered_outputs) {
> > + if (pwm->hwpwm < PCA9685_MAXCHAN) {
> > + /*
> > + * To reduce EMI, the ON times of each channel are
> > + * spread out evenly within the counter range, while
> > + * still maintaining the configured duty cycle
> > + */
> > + on = pwm->hwpwm * PCA9685_COUNTER_RANGE /
> > + PCA9685_MAXCHAN;
> > + off = (on + duty) % PCA9685_COUNTER_RANGE;
>
> Will pwm_get_state() still return the correct duty cycle in case
> of staggered outputs?
No, but it should. I will improve the get_state function in the next
version to cover all cases.
Thanks for your input. This would have been a mess without your feedback
;)
>
> > + regmap_write(pca->regmap, LED_N_ON_L(pwm->hwpwm),
> > + on & 0xff);
> > + regmap_write(pca->regmap, LED_N_ON_H(pwm->hwpwm),
> > + (on >> 8) & 0xf);
> > + regmap_write(pca->regmap, LED_N_OFF_L(pwm->hwpwm),
> > + off & 0xff);
> > + regmap_write(pca->regmap, LED_N_OFF_H(pwm->hwpwm),
> > + (off >> 8) & 0xf);
> > + return 0;
> > + }
> > +
> > + /* No staggering possible if "all LEDs" channel is used */
> > + regmap_write(pca->regmap, PCA9685_ALL_LED_ON_L, 0);
> > + regmap_write(pca->regmap, PCA9685_ALL_LED_ON_H, 0);
> > + }
> > +
> > if (pwm->hwpwm >= PCA9685_MAXCHAN)
> > reg = PCA9685_ALL_LED_OFF_L;
> > else
> > @@ -470,6 +497,9 @@ static int pca9685_pwm_probe(struct i2c_client *client,
> >
> > regmap_write(pca->regmap, PCA9685_MODE2, reg);
> >
> > + pca->staggered_outputs = device_property_read_bool(
> > + &client->dev, "staggered-outputs");
> > +
> > /* Disable all LED ALLCALL and SUBx addresses to avoid bus
> > collisions */
> > regmap_read(pca->regmap, PCA9685_MODE1, ®);
> > reg &= ~(MODE1_ALLCALL | MODE1_SUB1 | MODE1_SUB2 | MODE1_SUB3);
> > @@ -478,6 +508,9 @@ static int pca9685_pwm_probe(struct i2c_client *client,
> > /* Reset OFF registers to HW default (only full OFF bit is set) */
> > regmap_write(pca->regmap, PCA9685_ALL_LED_OFF_L, 0);
> > regmap_write(pca->regmap, PCA9685_ALL_LED_OFF_H, LED_FULL);
> > + /* Reset ON registers to HW default */
> > + regmap_write(pca->regmap, PCA9685_ALL_LED_ON_L, 0);
> > + regmap_write(pca->regmap, PCA9685_ALL_LED_ON_H, 0);
> >
> > pca->chip.ops = &pca9685_pwm_ops;
> > /* Add an extra channel for ALL_LED */
> > --
> > 2.29.2
> >
