Skip to content

Commit

Permalink
pwm: pca9685: Fix GPIO-only operation
Browse files Browse the repository at this point in the history 
GPIO-only driver operation never clears the SLEEP bit, which can cause
the GPIOs to become unusable.

Example:
1. user requests first PWM  ->      driver clears SLEEP bit
2. user frees last PWM      ->      driver sets SLEEP bit
3. user requests GPIO
4. user switches GPIO on    ->      output does not turn on
                                    because SLEEP bit is set

Prevent this behaviour by letting the runtime PM framework control the
SLEEP bit. This will put the chip to SLEEP if no PWMs/GPIOs are exported
or in use.

Fixes: bccec89 ("Allow any of the 16 PWMs to be used as a GPIO")
Reported-by: Sven Van Asbroeck <TheSven73@googlemail.com>
Signed-off-by: Sven Van Asbroeck <TheSven73@googlemail.com>
Suggested-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
  • Loading branch information
Sven Van Asbroeck authored and Thierry Reding committed Apr 13, 2017
1 parent 6db249d commit c40c461
Showing 1 changed file with 79 additions and 33 deletions.
112 changes: 79 additions & 33 deletions drivers/pwm/pwm-pca9685.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include <linux/regmap.h>
#include <linux/slab.h>
#include <linux/delay.h>
#include <linux/pm_runtime.h>

/*
* Because the PCA9685 has only one prescaler per chip, changing the period of
Expand Down Expand Up @@ -79,7 +80,6 @@
struct pca9685 {
struct pwm_chip chip;
struct regmap *regmap;
int active_cnt;
int duty_ns;
int period_ns;
#if IS_ENABLED(CONFIG_GPIOLIB)
Expand Down Expand Up @@ -111,20 +111,10 @@ static int pca9685_pwm_gpio_request(struct gpio_chip *gpio, unsigned int offset)
pwm_set_chip_data(pwm, (void *)1);

mutex_unlock(&pca->lock);
pm_runtime_get_sync(pca->chip.dev);
return 0;
}

static void pca9685_pwm_gpio_free(struct gpio_chip *gpio, unsigned int offset)
{
struct pca9685 *pca = gpiochip_get_data(gpio);
struct pwm_device *pwm;

mutex_lock(&pca->lock);
pwm = &pca->chip.pwms[offset];
pwm_set_chip_data(pwm, NULL);
mutex_unlock(&pca->lock);
}

static bool pca9685_pwm_is_gpio(struct pca9685 *pca, struct pwm_device *pwm)
{
bool is_gpio = false;
Expand Down Expand Up @@ -177,6 +167,19 @@ static void pca9685_pwm_gpio_set(struct gpio_chip *gpio, unsigned int offset,
regmap_write(pca->regmap, LED_N_ON_H(pwm->hwpwm), on);
}

static void pca9685_pwm_gpio_free(struct gpio_chip *gpio, unsigned int offset)
{
struct pca9685 *pca = gpiochip_get_data(gpio);
struct pwm_device *pwm;

pca9685_pwm_gpio_set(gpio, offset, 0);
pm_runtime_put(pca->chip.dev);
mutex_lock(&pca->lock);
pwm = &pca->chip.pwms[offset];
pwm_set_chip_data(pwm, NULL);
mutex_unlock(&pca->lock);
}

static int pca9685_pwm_gpio_get_direction(struct gpio_chip *chip,
unsigned int offset)
{
Expand Down Expand Up @@ -238,6 +241,16 @@ static inline int pca9685_pwm_gpio_probe(struct pca9685 *pca)
}
#endif

static void pca9685_set_sleep_mode(struct pca9685 *pca, int sleep)
{
regmap_update_bits(pca->regmap, PCA9685_MODE1,
MODE1_SLEEP, sleep ? MODE1_SLEEP : 0);
if (!sleep) {
/* Wait 500us for the oscillator to be back up */
udelay(500);
}
}

static int pca9685_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
int duty_ns, int period_ns)
{
Expand All @@ -252,19 +265,20 @@ static int pca9685_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,

if (prescale >= PCA9685_PRESCALE_MIN &&
prescale <= PCA9685_PRESCALE_MAX) {
/*
* putting the chip briefly into SLEEP mode
* at this point won't interfere with the
* pm_runtime framework, because the pm_runtime
* state is guaranteed active here.
*/
/* Put chip into sleep mode */
regmap_update_bits(pca->regmap, PCA9685_MODE1,
MODE1_SLEEP, MODE1_SLEEP);
pca9685_set_sleep_mode(pca, 1);

/* Change the chip-wide output frequency */
regmap_write(pca->regmap, PCA9685_PRESCALE, prescale);

/* Wake the chip up */
regmap_update_bits(pca->regmap, PCA9685_MODE1,
MODE1_SLEEP, 0x0);

/* Wait 500us for the oscillator to be back up */
udelay(500);
pca9685_set_sleep_mode(pca, 0);

pca->period_ns = period_ns;
} else {
Expand Down Expand Up @@ -406,21 +420,15 @@ static int pca9685_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)

if (pca9685_pwm_is_gpio(pca, pwm))
return -EBUSY;

if (pca->active_cnt++ == 0)
return regmap_update_bits(pca->regmap, PCA9685_MODE1,
MODE1_SLEEP, 0x0);
pm_runtime_get_sync(chip->dev);

return 0;
}

static void pca9685_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
{
struct pca9685 *pca = to_pca(chip);

if (--pca->active_cnt == 0)
regmap_update_bits(pca->regmap, PCA9685_MODE1, MODE1_SLEEP,
MODE1_SLEEP);
pca9685_pwm_disable(chip, pwm);
pm_runtime_put(chip->dev);
}

static const struct pwm_ops pca9685_pwm_ops = {
Expand Down Expand Up @@ -492,22 +500,54 @@ static int pca9685_pwm_probe(struct i2c_client *client,
return ret;

ret = pca9685_pwm_gpio_probe(pca);
if (ret < 0)
if (ret < 0) {
pwmchip_remove(&pca->chip);
return ret;
}

/* the chip comes out of power-up in the active state */
pm_runtime_set_active(&client->dev);
/*
* enable will put the chip into suspend, which is what we
* want as all outputs are disabled at this point
*/
pm_runtime_enable(&client->dev);

return ret;
return 0;
}

static int pca9685_pwm_remove(struct i2c_client *client)
{
struct pca9685 *pca = i2c_get_clientdata(client);
int ret;

regmap_update_bits(pca->regmap, PCA9685_MODE1, MODE1_SLEEP,
MODE1_SLEEP);
ret = pwmchip_remove(&pca->chip);
if (ret)
return ret;
pm_runtime_disable(&client->dev);
return 0;
}

return pwmchip_remove(&pca->chip);
#ifdef CONFIG_PM
static int pca9685_pwm_runtime_suspend(struct device *dev)
{
struct i2c_client *client = to_i2c_client(dev);
struct pca9685 *pca = i2c_get_clientdata(client);

pca9685_set_sleep_mode(pca, 1);
return 0;
}

static int pca9685_pwm_runtime_resume(struct device *dev)
{
struct i2c_client *client = to_i2c_client(dev);
struct pca9685 *pca = i2c_get_clientdata(client);

pca9685_set_sleep_mode(pca, 0);
return 0;
}
#endif

static const struct i2c_device_id pca9685_id[] = {
{ "pca9685", 0 },
{ /* sentinel */ },
Expand All @@ -530,11 +570,17 @@ static const struct of_device_id pca9685_dt_ids[] = {
MODULE_DEVICE_TABLE(of, pca9685_dt_ids);
#endif

static const struct dev_pm_ops pca9685_pwm_pm = {
SET_RUNTIME_PM_OPS(pca9685_pwm_runtime_suspend,
pca9685_pwm_runtime_resume, NULL)
};

static struct i2c_driver pca9685_i2c_driver = {
.driver = {
.name = "pca9685-pwm",
.acpi_match_table = ACPI_PTR(pca9685_acpi_ids),
.of_match_table = of_match_ptr(pca9685_dt_ids),
.pm = &pca9685_pwm_pm,
},
.probe = pca9685_pwm_probe,
.remove = pca9685_pwm_remove,
Expand Down

0 comments on commit c40c461

Please sign in to comment.