Received: by 2002:a05:6a10:17d3:0:0:0:0 with SMTP id hz19csp1774064pxb; Mon, 12 Apr 2021 06:30:52 -0700 (PDT) X-Google-Smtp-Source: ABdhPJz6Zz5OkkfZH7Zv+Xv0d55sjWj7/YM8UsDKjXYRIfIqE5T82IXZ9wEaJSacUBcqvuEMMYDP X-Received: by 2002:a63:d457:: with SMTP id i23mr26378391pgj.348.1618234252649; Mon, 12 Apr 2021 06:30:52 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1618234252; cv=none; d=google.com; s=arc-20160816; b=NRVYSFs5USzNgXxofe4J5zgSyYjGTcMGqp9mxwFHD1HLx0W4gzhUJCT/pkCr1Q5EBG HA/pNXUbhDqkeBe/rEdrENhywWHztQO23yjhuU9iEuqbiDIUV9HeM2K9mD9FQ+bMhRTl 6V1V2vqS/wTF7+fV7/qwZq5CAvD4wIiD58SLvCMHJbA/OLt1R9fTjYigjZVrVKpTAP6D ryySJPcgiSFQR1xz1QHHFIXHx9zunaMsEfsxIJZK7uPlheqlyR3ClVWwMvT1VznHceh4 LMAQNTsxrva1oY85oEuLHPsxsmlY9IvDAz9hMZZFzwlQYIrJ3DttYkKJkhMYDIZeHhHI XN+g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=yrlMdXt5C0n/+lWYvfdLDW8Lgl2oIdPpiujji5HTgXw=; b=wdeNXJA4licwlr8K4womcqy6rCTJ6tazy7x1GUsqVcz5R3PYlA28kBxKlSeQ4Lt5dA xaG4qDcSOY6WcrPeWDNAt/iOb5nqDiStYnmtLkDLXBhj8HLHWD17DVmqwrRUOe8VHk81 6TpTJwGcLHWL88GPoxoyjnEwyz6s7vCOH414MI+Xev/++sm6ZoGwNFu1mPAa9169q43i Hyl0vHAY5Rt6CKYjYRmUXlraGjocVyq/dKP6VcBMlyZE4/CLT9+6Fny6xggMkco+gP9a i0qym/YhdRQJqIgDCC2n4katd/3THUqFV6GqwQdHkZ5713pzMsnPUIIwkhPlBoP/yTSW z8Gg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@pqgruber.com header.s=mail header.b="Ma0T/F0f"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=NONE dis=NONE) header.from=pqgruber.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id t202si17226190pfc.234.2021.04.12.06.30.39; Mon, 12 Apr 2021 06:30:52 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@pqgruber.com header.s=mail header.b="Ma0T/F0f"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=NONE dis=NONE) header.from=pqgruber.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S241942AbhDLNaD (ORCPT + 99 others); Mon, 12 Apr 2021 09:30:03 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41842 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S241909AbhDLN3x (ORCPT ); Mon, 12 Apr 2021 09:29:53 -0400 Received: from mail.pqgruber.com (mail.pqgruber.com [IPv6:2a05:d014:575:f70b:4f2c:8f1d:40c4:b13e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D3C0BC06174A; Mon, 12 Apr 2021 06:29:33 -0700 (PDT) Received: from workstation.tuxnet (213-47-165-233.cable.dynamic.surfer.at [213.47.165.233]) by mail.pqgruber.com (Postfix) with ESMTPSA id 83BE3C725CF; Mon, 12 Apr 2021 15:29:31 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pqgruber.com; s=mail; t=1618234171; bh=yrlMdXt5C0n/+lWYvfdLDW8Lgl2oIdPpiujji5HTgXw=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Ma0T/F0fHFT/eWh/Pg7Deki1PbsbRzw4uIIFlBu6RPyPGguBBNSt47xE6IHK8SFFr 6v7hReImnHBmGsCR5Di2rQ5Zom2gkkaUAJGEYvwIz164POPoZ1jFFdBU6GtA456JTv uqAMpb9zmQJvt+M1d8gL9MUenikDeI/ycZCNmZNM= From: Clemens Gruber To: linux-pwm@vger.kernel.org Cc: Thierry Reding , Sven Van Asbroeck , =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= , devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Clemens Gruber Subject: [PATCH v8 7/8] pwm: pca9685: Restrict period change for enabled PWMs Date: Mon, 12 Apr 2021 15:27:44 +0200 Message-Id: <20210412132745.76609-7-clemens.gruber@pqgruber.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210412132745.76609-1-clemens.gruber@pqgruber.com> References: <20210412132745.76609-1-clemens.gruber@pqgruber.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Previously, the last used PWM channel could change the global prescale setting, even if other channels are already in use. Fix it by only allowing the first enabled PWM to change the global chip-wide prescale setting. If there is more than one channel in use, the prescale settings resulting from the chosen periods must match. GPIOs do not count as enabled PWMs as they are not using the prescaler and can't change it. Signed-off-by: Clemens Gruber --- Changes since v7: - As the HW readout always returns enabled, also set the pwms_enabled bit in request (except for the "all LEDs" channel) Changes since v6: - Only allow the first PWM that is enabled to change the prescaler, not the first one that uses the prescaler drivers/pwm/pwm-pca9685.c | 74 +++++++++++++++++++++++++++++++++------ 1 file changed, 64 insertions(+), 10 deletions(-) diff --git a/drivers/pwm/pwm-pca9685.c b/drivers/pwm/pwm-pca9685.c index 410b93b115dc..4583edd5e477 100644 --- a/drivers/pwm/pwm-pca9685.c +++ b/drivers/pwm/pwm-pca9685.c @@ -23,11 +23,11 @@ #include /* - * Because the PCA9685 has only one prescaler per chip, changing the period of - * one channel affects the period of all 16 PWM outputs! - * However, the ratio between each configured duty cycle and the chip-wide - * period remains constant, because the OFF time is set in proportion to the - * counter range. + * Because the PCA9685 has only one prescaler per chip, only the first channel + * that is enabled is allowed to change the prescale register. + * PWM channels requested afterwards must use a period that results in the same + * prescale setting as the one set by the first requested channel. + * GPIOs do not count as enabled PWMs as they are not using the prescaler. */ #define PCA9685_MODE1 0x00 @@ -78,8 +78,9 @@ struct pca9685 { struct pwm_chip chip; struct regmap *regmap; -#if IS_ENABLED(CONFIG_GPIOLIB) struct mutex lock; + DECLARE_BITMAP(pwms_enabled, PCA9685_MAXCHAN + 1); +#if IS_ENABLED(CONFIG_GPIOLIB) struct gpio_chip gpio; DECLARE_BITMAP(pwms_inuse, PCA9685_MAXCHAN + 1); #endif @@ -90,6 +91,22 @@ static inline struct pca9685 *to_pca(struct pwm_chip *chip) return container_of(chip, struct pca9685, chip); } +/* This function is supposed to be called with the lock mutex held */ +static bool pca9685_prescaler_can_change(struct pca9685 *pca, int channel) +{ + /* No PWM enabled: Change allowed */ + if (bitmap_empty(pca->pwms_enabled, PCA9685_MAXCHAN + 1)) + return true; + /* More than one PWM enabled: Change not allowed */ + if (bitmap_weight(pca->pwms_enabled, PCA9685_MAXCHAN + 1) > 1) + return false; + /* + * Only one PWM enabled: Change allowed if the PWM about to + * be changed is the one that is already enabled + */ + return test_bit(channel, pca->pwms_enabled); +} + /* Helper function to set the duty cycle ratio to duty/4096 (e.g. duty=2048 -> 50%) */ static void pca9685_pwm_set_duty(struct pca9685 *pca, int channel, unsigned int duty) { @@ -268,8 +285,6 @@ static int pca9685_pwm_gpio_probe(struct pca9685 *pca) { struct device *dev = pca->chip.dev; - mutex_init(&pca->lock); - pca->gpio.label = dev_name(dev); pca->gpio.parent = dev; pca->gpio.request = pca9685_pwm_gpio_request; @@ -313,8 +328,8 @@ static void pca9685_set_sleep_mode(struct pca9685 *pca, bool enable) } } -static int pca9685_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, - const struct pwm_state *state) +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 long long duty, prescale; @@ -337,6 +352,12 @@ static int pca9685_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, regmap_read(pca->regmap, PCA9685_PRESCALE, &val); if (prescale != val) { + if (!pca9685_prescaler_can_change(pca, pwm->hwpwm)) { + dev_err(chip->dev, + "pwm not changed: periods of enabled pwms must match!\n"); + return -EBUSY; + } + /* * Putting the chip briefly into SLEEP mode * at this point won't interfere with the @@ -359,6 +380,25 @@ static int pca9685_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, return 0; } +static int pca9685_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, + const struct pwm_state *state) +{ + struct pca9685 *pca = to_pca(chip); + int ret; + + mutex_lock(&pca->lock); + ret = __pca9685_pwm_apply(chip, pwm, state); + if (ret == 0) { + if (state->enabled) + set_bit(pwm->hwpwm, pca->pwms_enabled); + else + clear_bit(pwm->hwpwm, pca->pwms_enabled); + } + mutex_unlock(&pca->lock); + + return ret; +} + static void pca9685_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm, struct pwm_state *state) { @@ -400,6 +440,14 @@ static int pca9685_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm) if (pca9685_pwm_test_and_set_inuse(pca, pwm->hwpwm)) return -EBUSY; + + if (pwm->hwpwm < PCA9685_MAXCHAN) { + /* PWMs - except the "all LEDs" channel - default to enabled */ + mutex_lock(&pca->lock); + set_bit(pwm->hwpwm, pca->pwms_enabled); + mutex_unlock(&pca->lock); + } + pm_runtime_get_sync(chip->dev); return 0; @@ -409,7 +457,11 @@ static void pca9685_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm) { struct pca9685 *pca = to_pca(chip); + mutex_lock(&pca->lock); pca9685_pwm_set_duty(pca, pwm->hwpwm, 0); + clear_bit(pwm->hwpwm, pca->pwms_enabled); + mutex_unlock(&pca->lock); + pm_runtime_put(chip->dev); pca9685_pwm_clear_inuse(pca, pwm->hwpwm); } @@ -450,6 +502,8 @@ static int pca9685_pwm_probe(struct i2c_client *client, i2c_set_clientdata(client, pca); + mutex_init(&pca->lock); + regmap_read(pca->regmap, PCA9685_MODE2, ®); if (device_property_read_bool(&client->dev, "invert")) -- 2.31.1