From patchwork Mon Dec 7 19:36:27 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Clemens Gruber X-Patchwork-Id: 1412185 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=23.128.96.18; helo=vger.kernel.org; envelope-from=linux-pwm-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=pass (p=quarantine dis=none) header.from=pqgruber.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=pqgruber.com header.i=@pqgruber.com header.a=rsa-sha256 header.s=mail header.b=og579p8n; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by ozlabs.org (Postfix) with ESMTP id 4CqYWG05vGz9sTX for ; Tue, 8 Dec 2020 06:38:02 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725822AbgLGThp (ORCPT ); Mon, 7 Dec 2020 14:37:45 -0500 Received: from mail.pqgruber.com ([52.59.78.55]:35184 "EHLO mail.pqgruber.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725799AbgLGThp (ORCPT ); Mon, 7 Dec 2020 14:37:45 -0500 Received: from workstation.tuxnet (213-47-165-233.cable.dynamic.surfer.at [213.47.165.233]) by mail.pqgruber.com (Postfix) with ESMTPSA id 61BD9C89267; Mon, 7 Dec 2020 20:37:00 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pqgruber.com; s=mail; t=1607369820; bh=/QZhrHYMZrj0RL9tX/lahMSkpVtbjJX6kABlunaKdCA=; h=From:To:Cc:Subject:Date:From; b=og579p8nhoSBmi7AnYyaB+okYvPkM04ZS6PKL17rZE06LhP2OaYLZzJgj1d8bebM0 +7uBeGzD26QsFWgNX43GssRuIoKjZ4kqwaaCZ/OlM3nkyhzksGZnzHIfNI8FeNbDyM d6OoBRNMH8y7ow5V8V316jnlzGu0xHN+P7dh2uG4= From: Clemens Gruber To: linux-pwm@vger.kernel.org Cc: Thierry Reding , u.kleine-koenig@pengutronix.de, Lee Jones , linux-kernel@vger.kernel.org, Sven Van Asbroeck , Mika Westerberg , David Jander , Clemens Gruber Subject: [PATCH v4 1/4] pwm: pca9685: Switch to atomic API Date: Mon, 7 Dec 2020 20:36:27 +0100 Message-Id: <20201207193629.493241-1-clemens.gruber@pqgruber.com> X-Mailer: git-send-email 2.29.2 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-pwm@vger.kernel.org The switch to the atomic API goes hand in hand with a few fixes to previously experienced issues: - The duty cycle is no longer lost after disable/enable (previously the OFF registers were cleared in disable and the user was required to call config to restore the duty cycle settings) - If one sets a period resulting in the same prescale register value, the sleep and write to the register is now skipped - The prescale register is now set to the default value in probe. On systems without CONFIG_PM, the chip is woken up at probe time. The hardware readout may return slightly different values than those that were set in apply due to the limited range of possible prescale and counter register values. If one channel is reconfigured with new duty cycle and period, the others will keep the same relative duty cycle to period ratio as they had before, even though the per-chip / global frequency changed. (The PCA9685 has only one prescaler!) Note that although the datasheet mentions 200 Hz as default frequency when using the internal 25 MHz oscillator, the calculated period from the default prescaler register setting of 30 is 5079040ns. Signed-off-by: Clemens Gruber --- Changes since v3: - Refactoring: Extracted common functions - Read prescale register value instead of caching it - Return all zeros and disabled for "all LEDs" channel state - Improved duty calculation / mapping to 0..4096 Changes since v2: - Always set default prescale value in probe - Simplified probe code - Inlined functions with one callsite Changes since v1: - Fixed a logic error - Impoved PM runtime handling and fixed !CONFIG_PM - Write default prescale reg value if invalid in probe - Reuse full_off/_on functions throughout driver - Use cached prescale value whenever possible drivers/pwm/pwm-pca9685.c | 335 ++++++++++++++++++++------------------ 1 file changed, 175 insertions(+), 160 deletions(-) diff --git a/drivers/pwm/pwm-pca9685.c b/drivers/pwm/pwm-pca9685.c index 4a55dc18656c..0425e0bcb81e 100644 --- a/drivers/pwm/pwm-pca9685.c +++ b/drivers/pwm/pwm-pca9685.c @@ -47,11 +47,11 @@ #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 */ #define PCA9685_COUNTER_RANGE 4096 -#define PCA9685_DEFAULT_PERIOD 5000000 /* Default period_ns = 1/200 Hz */ #define PCA9685_OSC_CLOCK_MHZ 25 /* Internal oscillator with 25 MHz */ #define PCA9685_NUMREGS 0xFF @@ -74,7 +74,6 @@ struct pca9685 { struct pwm_chip chip; struct regmap *regmap; - int period_ns; #if IS_ENABLED(CONFIG_GPIOLIB) struct mutex lock; struct gpio_chip gpio; @@ -87,6 +86,81 @@ static inline struct pca9685 *to_pca(struct pwm_chip *chip) return container_of(chip, struct pca9685, chip); } +static void pca9685_pwm_set_full_off(struct pca9685 *pca, int index, bool enable) +{ + unsigned int val = enable ? LED_FULL : 0; + + /* Note: The full OFF bit has the highest precedence */ + + if (index >= PCA9685_MAXCHAN) { + regmap_write(pca->regmap, PCA9685_ALL_LED_OFF_H, val); + return; + } + regmap_update_bits(pca->regmap, LED_N_OFF_H(index), LED_FULL, val); +} + +static bool pca9685_pwm_is_full_off(struct pca9685 *pca, int index) +{ + unsigned int val = 0; + + if (index >= PCA9685_MAXCHAN) + return false; + + regmap_read(pca->regmap, LED_N_OFF_H(index), &val); + return val & LED_FULL; +} + +static void pca9685_pwm_set_full_on(struct pca9685 *pca, int index, bool enable) +{ + unsigned int val = enable ? LED_FULL : 0; + + if (index >= PCA9685_MAXCHAN) { + regmap_write(pca->regmap, PCA9685_ALL_LED_ON_H, val); + return; + } + regmap_update_bits(pca->regmap, LED_N_ON_H(index), LED_FULL, val); +} + +static bool pca9685_pwm_is_full_on(struct pca9685 *pca, int index) +{ + unsigned int val = 0; + + if (index >= PCA9685_MAXCHAN) + return false; + + regmap_read(pca->regmap, LED_N_ON_H(index), &val); + return val & LED_FULL; +} + +static void pca9685_pwm_set_off_time(struct pca9685 *pca, int index, unsigned int off) +{ + int reg; + + /* Note: This function also clears the full OFF bit */ + + reg = index >= PCA9685_MAXCHAN ? + PCA9685_ALL_LED_OFF_L : LED_N_OFF_L(index); + regmap_write(pca->regmap, reg, off & 0xff); + + reg = index >= PCA9685_MAXCHAN ? + PCA9685_ALL_LED_OFF_H : LED_N_OFF_H(index); + regmap_write(pca->regmap, reg, (off >> 8) & 0xf); +} + +static unsigned int pca9685_pwm_off_time(struct pca9685 *pca, int index) +{ + unsigned int off, val = 0; + + if (index >= PCA9685_MAXCHAN) + return 0; + + regmap_read(pca->regmap, LED_N_OFF_H(index), &val); + off = (val & 0xf) << 8; + + regmap_read(pca->regmap, LED_N_OFF_L(index), &val); + return off | (val & 0xff); +} + #if IS_ENABLED(CONFIG_GPIOLIB) static bool pca9685_pwm_test_and_set_inuse(struct pca9685 *pca, int pwm_idx) { @@ -138,34 +212,31 @@ static int pca9685_pwm_gpio_request(struct gpio_chip *gpio, unsigned int offset) static int pca9685_pwm_gpio_get(struct gpio_chip *gpio, unsigned int offset) { struct pca9685 *pca = gpiochip_get_data(gpio); - struct pwm_device *pwm = &pca->chip.pwms[offset]; - unsigned int value; - - regmap_read(pca->regmap, LED_N_ON_H(pwm->hwpwm), &value); - return value & LED_FULL; + return !pca9685_pwm_is_full_off(pca, offset); } static void pca9685_pwm_gpio_set(struct gpio_chip *gpio, unsigned int offset, int value) { struct pca9685 *pca = gpiochip_get_data(gpio); - struct pwm_device *pwm = &pca->chip.pwms[offset]; - unsigned int on = value ? LED_FULL : 0; - - /* Clear both OFF registers */ - regmap_write(pca->regmap, LED_N_OFF_L(pwm->hwpwm), 0); - regmap_write(pca->regmap, LED_N_OFF_H(pwm->hwpwm), 0); - /* Set the full ON bit */ - regmap_write(pca->regmap, LED_N_ON_H(pwm->hwpwm), on); + if (value) { + pca9685_pwm_set_full_on(pca, offset, true); + /* Clear full OFF bit last */ + pca9685_pwm_set_full_off(pca, offset, false); + } else { + /* Set full OFF bit first */ + pca9685_pwm_set_full_off(pca, offset, true); + pca9685_pwm_set_full_on(pca, offset, false); + } } static void pca9685_pwm_gpio_free(struct gpio_chip *gpio, unsigned int offset) { struct pca9685 *pca = gpiochip_get_data(gpio); - pca9685_pwm_gpio_set(gpio, offset, 0); + pca9685_pwm_set_full_off(pca, offset, true); pm_runtime_put(pca->chip.dev); pca9685_pwm_clear_inuse(pca, offset); } @@ -246,165 +317,98 @@ static void pca9685_set_sleep_mode(struct pca9685 *pca, bool enable) } } -static int pca9685_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm, - int duty_ns, int period_ns) +static void pca9685_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm, + struct pwm_state *state) { struct pca9685 *pca = to_pca(chip); unsigned long long duty; - unsigned int reg; - int prescale; - - if (period_ns != pca->period_ns) { - prescale = DIV_ROUND_CLOSEST(PCA9685_OSC_CLOCK_MHZ * period_ns, - PCA9685_COUNTER_RANGE * 1000) - 1; - - 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 */ - pca9685_set_sleep_mode(pca, true); - - /* Change the chip-wide output frequency */ - regmap_write(pca->regmap, PCA9685_PRESCALE, prescale); - - /* Wake the chip up */ - pca9685_set_sleep_mode(pca, false); - - pca->period_ns = period_ns; - } else { - dev_err(chip->dev, - "prescaler not set: period out of bounds!\n"); - return -EINVAL; - } - } + unsigned int val = 0; - if (duty_ns < 1) { - if (pwm->hwpwm >= PCA9685_MAXCHAN) - reg = PCA9685_ALL_LED_OFF_H; - else - reg = LED_N_OFF_H(pwm->hwpwm); + /* Calculate (chip-wide) period from prescale value */ + regmap_read(pca->regmap, PCA9685_PRESCALE, &val); + state->period = (PCA9685_COUNTER_RANGE * 1000 / PCA9685_OSC_CLOCK_MHZ) * + (val + 1); - regmap_write(pca->regmap, reg, LED_FULL); + /* The (per-channel) polarity is fixed */ + state->polarity = PWM_POLARITY_NORMAL; - return 0; + if (pwm->hwpwm >= PCA9685_MAXCHAN) { + /* + * The "all LEDs" channel does not support HW readout + * Return 0 and disabled for backwards compatibility + */ + state->duty_cycle = 0; + state->enabled = false; + return; } - if (duty_ns == period_ns) { - /* Clear both OFF registers */ - if (pwm->hwpwm >= PCA9685_MAXCHAN) - reg = PCA9685_ALL_LED_OFF_L; - else - reg = LED_N_OFF_L(pwm->hwpwm); - - regmap_write(pca->regmap, reg, 0x0); - - if (pwm->hwpwm >= PCA9685_MAXCHAN) - reg = PCA9685_ALL_LED_OFF_H; - else - reg = LED_N_OFF_H(pwm->hwpwm); - - regmap_write(pca->regmap, reg, 0x0); - - /* Set the full ON bit */ - if (pwm->hwpwm >= PCA9685_MAXCHAN) - reg = PCA9685_ALL_LED_ON_H; - else - reg = LED_N_ON_H(pwm->hwpwm); + state->enabled = !pca9685_pwm_is_full_off(pca, pwm->hwpwm); - regmap_write(pca->regmap, reg, LED_FULL); - - return 0; + if (state->enabled && pca9685_pwm_is_full_on(pca, pwm->hwpwm)) { + state->duty_cycle = state->period; + return; } - duty = PCA9685_COUNTER_RANGE * (unsigned long long)duty_ns; - duty = DIV_ROUND_UP_ULL(duty, period_ns); - - if (pwm->hwpwm >= PCA9685_MAXCHAN) - reg = PCA9685_ALL_LED_OFF_L; - else - reg = LED_N_OFF_L(pwm->hwpwm); - - regmap_write(pca->regmap, reg, (int)duty & 0xff); - - if (pwm->hwpwm >= PCA9685_MAXCHAN) - reg = PCA9685_ALL_LED_OFF_H; - else - reg = LED_N_OFF_H(pwm->hwpwm); - - regmap_write(pca->regmap, reg, ((int)duty >> 8) & 0xf); - - /* Clear the full ON bit, otherwise the set OFF time has no effect */ - if (pwm->hwpwm >= PCA9685_MAXCHAN) - reg = PCA9685_ALL_LED_ON_H; - else - reg = LED_N_ON_H(pwm->hwpwm); - - regmap_write(pca->regmap, reg, 0); - - return 0; + duty = pca9685_pwm_off_time(pca, pwm->hwpwm) * state->period; + state->duty_cycle = duty / PCA9685_COUNTER_RANGE; } -static int pca9685_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm) +static int pca9685_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, + const struct pwm_state *state) { struct pca9685 *pca = to_pca(chip); - unsigned int reg; - - /* - * The PWM subsystem does not support a pre-delay. - * So, set the ON-timeout to 0 - */ - if (pwm->hwpwm >= PCA9685_MAXCHAN) - reg = PCA9685_ALL_LED_ON_L; - else - reg = LED_N_ON_L(pwm->hwpwm); + unsigned long long duty, prescale; + unsigned int val = 0; - regmap_write(pca->regmap, reg, 0); + if (state->polarity != PWM_POLARITY_NORMAL) + return -EOPNOTSUPP; - if (pwm->hwpwm >= PCA9685_MAXCHAN) - reg = PCA9685_ALL_LED_ON_H; - else - reg = LED_N_ON_H(pwm->hwpwm); - - regmap_write(pca->regmap, reg, 0); - - /* - * Clear the full-off bit. - * It has precedence over the others and must be off. - */ - if (pwm->hwpwm >= PCA9685_MAXCHAN) - reg = PCA9685_ALL_LED_OFF_H; - else - reg = LED_N_OFF_H(pwm->hwpwm); + prescale = DIV_ROUND_CLOSEST_ULL(PCA9685_OSC_CLOCK_MHZ * state->period, + PCA9685_COUNTER_RANGE * 1000) - 1; + if (prescale < PCA9685_PRESCALE_MIN || prescale > PCA9685_PRESCALE_MAX) { + dev_err(chip->dev, "prescaler not set: period out of bounds!\n"); + return -EINVAL; + } - regmap_update_bits(pca->regmap, reg, LED_FULL, 0x0); + regmap_read(pca->regmap, PCA9685_PRESCALE, &val); + if (prescale != val) { + /* + * 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 */ + pca9685_set_sleep_mode(pca, true); - return 0; -} + /* Change the chip-wide output frequency */ + regmap_write(pca->regmap, PCA9685_PRESCALE, (int)prescale); -static void pca9685_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm) -{ - struct pca9685 *pca = to_pca(chip); - unsigned int reg; + /* Wake the chip up */ + pca9685_set_sleep_mode(pca, false); + } - if (pwm->hwpwm >= PCA9685_MAXCHAN) - reg = PCA9685_ALL_LED_OFF_H; - else - reg = LED_N_OFF_H(pwm->hwpwm); + duty = PCA9685_COUNTER_RANGE * state->duty_cycle; + duty = DIV_ROUND_CLOSEST_ULL(duty, state->period); - regmap_write(pca->regmap, reg, LED_FULL); + if (!state->enabled || duty < 1) { + /* Set full OFF bit first */ + pca9685_pwm_set_full_off(pca, pwm->hwpwm, true); + pca9685_pwm_set_full_on(pca, pwm->hwpwm, false); + return 0; + } - /* Clear the LED_OFF counter. */ - if (pwm->hwpwm >= PCA9685_MAXCHAN) - reg = PCA9685_ALL_LED_OFF_L; - else - reg = LED_N_OFF_L(pwm->hwpwm); + if (duty == PCA9685_COUNTER_RANGE) { + pca9685_pwm_set_full_on(pca, pwm->hwpwm, true); + /* Clear full OFF bit last */ + pca9685_pwm_set_full_off(pca, pwm->hwpwm, false); + return 0; + } - regmap_write(pca->regmap, reg, 0x0); + pca9685_pwm_set_off_time(pca, pwm->hwpwm, duty); + /* Clear full ON bit after OFF time was set */ + pca9685_pwm_set_full_on(pca, pwm->hwpwm, false); + return 0; } static int pca9685_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm) @@ -422,15 +426,14 @@ static void pca9685_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm) { struct pca9685 *pca = to_pca(chip); - pca9685_pwm_disable(chip, pwm); + pca9685_pwm_set_full_off(pca, pwm->hwpwm, true); pm_runtime_put(chip->dev); pca9685_pwm_clear_inuse(pca, pwm->hwpwm); } static const struct pwm_ops pca9685_pwm_ops = { - .enable = pca9685_pwm_enable, - .disable = pca9685_pwm_disable, - .config = pca9685_pwm_config, + .get_state = pca9685_pwm_get_state, + .apply = pca9685_pwm_apply, .request = pca9685_pwm_request, .free = pca9685_pwm_free, .owner = THIS_MODULE, @@ -461,7 +464,6 @@ static int pca9685_pwm_probe(struct i2c_client *client, ret); return ret; } - pca->period_ns = PCA9685_DEFAULT_PERIOD; i2c_set_clientdata(client, pca); @@ -505,14 +507,20 @@ static int pca9685_pwm_probe(struct i2c_client *client, 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 + * Always initialize with default prescale, but chip must be + * in sleep mode while changing prescaler. */ + 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); + if (!IS_ENABLED(CONFIG_PM)) { + /* Wake the chip up on non-PM environments */ + pca9685_set_sleep_mode(pca, false); + } + return 0; } @@ -524,7 +532,14 @@ static int pca9685_pwm_remove(struct i2c_client *client) ret = pwmchip_remove(&pca->chip); if (ret) return ret; + pm_runtime_disable(&client->dev); + + if (!IS_ENABLED(CONFIG_PM)) { + /* Put chip in sleep state on non-PM environments */ + pca9685_set_sleep_mode(pca, true); + } + return 0; } From patchwork Mon Dec 7 19:36:28 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Clemens Gruber X-Patchwork-Id: 1412186 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=23.128.96.18; helo=vger.kernel.org; envelope-from=linux-pwm-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=pass (p=quarantine dis=none) header.from=pqgruber.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=pqgruber.com header.i=@pqgruber.com header.a=rsa-sha256 header.s=mail header.b=rh71cuWL; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by ozlabs.org (Postfix) with ESMTP id 4CqYWH0Z0tz9sWC for ; Tue, 8 Dec 2020 06:38:03 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726024AbgLGThx (ORCPT ); Mon, 7 Dec 2020 14:37:53 -0500 Received: from mail.pqgruber.com ([52.59.78.55]:35196 "EHLO mail.pqgruber.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725799AbgLGThw (ORCPT ); Mon, 7 Dec 2020 14:37:52 -0500 Received: from workstation.tuxnet (213-47-165-233.cable.dynamic.surfer.at [213.47.165.233]) by mail.pqgruber.com (Postfix) with ESMTPSA id 921A6C8926C; Mon, 7 Dec 2020 20:37:10 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pqgruber.com; s=mail; t=1607369830; bh=ShuICKLZBVI5ErEI1/41Y2BY3Y2fl8W0JkwcIOrl/JY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=rh71cuWLJ6yoKVtlaSv2+4mPyT///9Jkn48WuhA97VyHaY03Ad0yGYSGBstew26R7 i8QbKNHls2M7hrHEEA264ZBqz6Z3nka/Hsz5Z8qm4EKdyCyl9fC/YiYb/Ypu+Q7lB6 e/c/cVtQ9vhPUNQoS1XQKbSUBpzOoJkT25Ew+Ppo= From: Clemens Gruber To: linux-pwm@vger.kernel.org Cc: Thierry Reding , u.kleine-koenig@pengutronix.de, Lee Jones , linux-kernel@vger.kernel.org, Sven Van Asbroeck , Mika Westerberg , David Jander , Clemens Gruber Subject: [PATCH v4 2/4] pwm: pca9685: Set full OFF bits in probe Date: Mon, 7 Dec 2020 20:36:28 +0100 Message-Id: <20201207193629.493241-2-clemens.gruber@pqgruber.com> X-Mailer: git-send-email 2.29.2 In-Reply-To: <20201207193629.493241-1-clemens.gruber@pqgruber.com> References: <20201207193629.493241-1-clemens.gruber@pqgruber.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-pwm@vger.kernel.org The full OFF bits are set by default in the PCA9685 LEDn_OFF_H registers at POR. LEDn_ON_L/H and LEDn_OFF_L default to 0. The datasheet states that LEDn_OFF and LEDn_ON should never be both set to the same values. This patch removes the clearing of the full OFF bit in the probe function. We still clear the rest of the OFF regs but now we set the full OFF bit to avoid having both OFF and ON registers set to 0 and start up in a safe default state. Signed-off-by: Clemens Gruber --- Changes since v1: - Rebased drivers/pwm/pwm-pca9685.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/pwm/pwm-pca9685.c b/drivers/pwm/pwm-pca9685.c index 0425e0bcb81e..fdce5fb9f41e 100644 --- a/drivers/pwm/pwm-pca9685.c +++ b/drivers/pwm/pwm-pca9685.c @@ -486,9 +486,9 @@ 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 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, 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 */ From patchwork Mon Dec 7 19:36:29 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Clemens Gruber X-Patchwork-Id: 1412187 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=23.128.96.18; helo=vger.kernel.org; envelope-from=linux-pwm-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=pass (p=quarantine dis=none) header.from=pqgruber.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=pqgruber.com header.i=@pqgruber.com header.a=rsa-sha256 header.s=mail header.b=ubKs4K5g; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by ozlabs.org (Postfix) with ESMTP id 4CqYWH2fb2z9sWF for ; Tue, 8 Dec 2020 06:38:03 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725905AbgLGTiC (ORCPT ); Mon, 7 Dec 2020 14:38:02 -0500 Received: from mail.pqgruber.com ([52.59.78.55]:35210 "EHLO mail.pqgruber.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725799AbgLGTiC (ORCPT ); Mon, 7 Dec 2020 14:38:02 -0500 Received: from workstation.tuxnet (213-47-165-233.cable.dynamic.surfer.at [213.47.165.233]) by mail.pqgruber.com (Postfix) with ESMTPSA id 42E4AC8926D; Mon, 7 Dec 2020 20:37:19 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pqgruber.com; s=mail; t=1607369839; bh=2aTuh/OTF7qUnaEWBSyPsz9linBNYAjPZccuYISJWAU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=ubKs4K5gJ2uzt6LYg981PCKHSJ0ebOy/DJlB5HLOuujnm6oPqiOXOpf8lbW1bHF+W kKGwD4uMpEtCFqHjVpVO780MREUdSH5gWLKXJ465Gtuepte8lDEbYFrkCF+68F6Kvv Dd3gju+CVw/sytn/yh6W01qZvKY6E1SR7+h1uFR8= From: Clemens Gruber To: linux-pwm@vger.kernel.org Cc: Thierry Reding , u.kleine-koenig@pengutronix.de, Lee Jones , linux-kernel@vger.kernel.org, Sven Van Asbroeck , Mika Westerberg , David Jander , Clemens Gruber Subject: [PATCH v4 3/4] pwm: pca9685: Support staggered output ON times Date: Mon, 7 Dec 2020 20:36:29 +0100 Message-Id: <20201207193629.493241-3-clemens.gruber@pqgruber.com> X-Mailer: git-send-email 2.29.2 In-Reply-To: <20201207193629.493241-1-clemens.gruber@pqgruber.com> References: <20201207193629.493241-1-clemens.gruber@pqgruber.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-pwm@vger.kernel.org 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 --- Changes since v3: - Refactoring: Extracted common functions - Fixed error in .get_state - Added vendor prefix to DT property Changes since v1: - Rebased drivers/pwm/pwm-pca9685.c | 72 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 69 insertions(+), 3 deletions(-) diff --git a/drivers/pwm/pwm-pca9685.c b/drivers/pwm/pwm-pca9685.c index fdce5fb9f41e..2697138ee95a 100644 --- a/drivers/pwm/pwm-pca9685.c +++ b/drivers/pwm/pwm-pca9685.c @@ -74,6 +74,7 @@ struct pca9685 { struct pwm_chip chip; struct regmap *regmap; + bool staggered_outputs; #if IS_ENABLED(CONFIG_GPIOLIB) struct mutex lock; struct gpio_chip gpio; @@ -161,6 +162,35 @@ static unsigned int pca9685_pwm_off_time(struct pca9685 *pca, int index) return off | (val & 0xff); } +static void pca9685_pwm_set_on_time(struct pca9685 *pca, int index, unsigned int on) +{ + int reg; + + /* Note: This function also clears the full ON bit */ + + reg = index >= PCA9685_MAXCHAN ? + PCA9685_ALL_LED_ON_L : LED_N_ON_L(index); + regmap_write(pca->regmap, reg, on & 0xff); + + reg = index >= PCA9685_MAXCHAN ? + PCA9685_ALL_LED_ON_H : LED_N_ON_H(index); + regmap_write(pca->regmap, reg, (on >> 8) & 0xf); +} + +static unsigned int pca9685_pwm_on_time(struct pca9685 *pca, int index) +{ + unsigned int on, val = 0; + + if (index >= PCA9685_MAXCHAN) + return 0; + + regmap_read(pca->regmap, LED_N_ON_H(index), &val); + on = (val & 0xf) << 8; + + regmap_read(pca->regmap, LED_N_ON_L(index), &val); + return on | (val & 0xff); +} + #if IS_ENABLED(CONFIG_GPIOLIB) static bool pca9685_pwm_test_and_set_inuse(struct pca9685 *pca, int pwm_idx) { @@ -322,7 +352,7 @@ static void pca9685_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm, { struct pca9685 *pca = to_pca(chip); unsigned long long duty; - unsigned int val = 0; + unsigned int on, off, val = 0; /* Calculate (chip-wide) period from prescale value */ regmap_read(pca->regmap, PCA9685_PRESCALE, &val); @@ -349,7 +379,19 @@ static void pca9685_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm, return; } - duty = pca9685_pwm_off_time(pca, pwm->hwpwm) * state->period; + off = pca9685_pwm_off_time(pca, pwm->hwpwm); + + if (pca->staggered_outputs) { + on = pca9685_pwm_on_time(pca, pwm->hwpwm); + + if (off >= on) + duty = off - on; + else + duty = off + PCA9685_COUNTER_RANGE - on; + } else + duty = off; + + duty *= state->period; state->duty_cycle = duty / PCA9685_COUNTER_RANGE; } @@ -358,7 +400,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 val = 0; + unsigned int on, off, val = 0; if (state->polarity != PWM_POLARITY_NORMAL) return -EOPNOTSUPP; @@ -405,6 +447,24 @@ static int pca9685_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, return 0; } + 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; + pca9685_pwm_set_on_time(pca, pwm->hwpwm, on); + pca9685_pwm_set_off_time(pca, pwm->hwpwm, off); + return 0; + } + + /* No staggering possible if "all LEDs" channel is used */ + pca9685_pwm_set_on_time(pca, PCA9685_MAXCHAN, 0); + } + pca9685_pwm_set_off_time(pca, pwm->hwpwm, duty); /* Clear full ON bit after OFF time was set */ pca9685_pwm_set_full_on(pca, pwm->hwpwm, false); @@ -481,6 +541,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, "nxp,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); @@ -489,6 +552,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 */ From patchwork Mon Dec 7 19:38:10 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Clemens Gruber X-Patchwork-Id: 1412188 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=23.128.96.18; helo=vger.kernel.org; envelope-from=linux-pwm-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=pass (p=quarantine dis=none) header.from=pqgruber.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=pqgruber.com header.i=@pqgruber.com header.a=rsa-sha256 header.s=mail header.b=Qcqki5qC; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by ozlabs.org (Postfix) with ESMTP id 4CqYXs3H4tz9sTX for ; Tue, 8 Dec 2020 06:39:25 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725915AbgLGTjJ (ORCPT ); Mon, 7 Dec 2020 14:39:09 -0500 Received: from mail.pqgruber.com ([52.59.78.55]:35246 "EHLO mail.pqgruber.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725853AbgLGTjJ (ORCPT ); Mon, 7 Dec 2020 14:39:09 -0500 Received: from workstation.tuxnet (213-47-165-233.cable.dynamic.surfer.at [213.47.165.233]) by mail.pqgruber.com (Postfix) with ESMTPSA id 15614C89267; Mon, 7 Dec 2020 20:38:27 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pqgruber.com; s=mail; t=1607369907; bh=Xyh8Wni+Ue0ny3rZRboSXRdZfKHzl+fSzSIiwkbye9I=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Qcqki5qCyt/3MvhYyz2CoN0t4Sxs6mIMbI1swceOl7TACVxFV+udkmffJrjLck0wj szTX29sumpitTmRvjMXD2bbvcMz9pTiICpeK2Wg+25brVaB0eix34sN5zTyhnCPprH qvwkG7EzCd7bKv2GuUt0yv2jvoHSj+NO7a727JHU= From: Clemens Gruber To: linux-pwm@vger.kernel.org Cc: Thierry Reding , u.kleine-koenig@pengutronix.de, Lee Jones , linux-kernel@vger.kernel.org, Clemens Gruber Subject: [PATCH v4 4/4] dt-bindings: pwm: pca9685: Add nxp,staggered-outputs property Date: Mon, 7 Dec 2020 20:38:10 +0100 Message-Id: <20201207193810.493299-1-clemens.gruber@pqgruber.com> X-Mailer: git-send-email 2.29.2 In-Reply-To: <20201207193629.493241-1-clemens.gruber@pqgruber.com> References: <20201207193629.493241-1-clemens.gruber@pqgruber.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-pwm@vger.kernel.org The pca9685 driver supports a new nxp,staggered-outputs property for reduced current surges and EMI. This adds documentation for the new DT property. Signed-off-by: Clemens Gruber --- Changes since v1: - Added vendor prefix Documentation/devicetree/bindings/pwm/nxp,pca9685-pwm.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/pwm/nxp,pca9685-pwm.txt b/Documentation/devicetree/bindings/pwm/nxp,pca9685-pwm.txt index f21b55c95738..fafe954369dc 100644 --- a/Documentation/devicetree/bindings/pwm/nxp,pca9685-pwm.txt +++ b/Documentation/devicetree/bindings/pwm/nxp,pca9685-pwm.txt @@ -12,6 +12,8 @@ Optional properties: - invert (bool): boolean to enable inverted logic - open-drain (bool): boolean to configure outputs with open-drain structure; if omitted use totem-pole structure + - nxp,staggered-outputs (bool): boolean to enable staggered output ON times to + minimize current surges and EMI Example: