[PATCH v4 000/115] pwm: Fix lifetime issues for pwm_chips
Thierry Reding
thierry.reding at gmail.com
Fri Dec 8 07:41:39 PST 2023
On Wed, Dec 06, 2023 at 12:43:14PM +0100, Uwe Kleine-König wrote:
> Hello,
>
> This series is based on Thierry's for-next.
>
> It starts with some cleanups that were all sent out separately already:
>
> - "pwm: Reduce number of pointer dereferences in pwm_device_request()"
> https://lore.kernel.org/linux-pwm/20231130072105.966848-1-u.kleine-koenig@pengutronix.de
> - "pwm: crc: Use consistent variable naming for driver data"
> https://lore.kernel.org/linux-pwm/20231130074133.969806-1-u.kleine-koenig@pengutronix.de
> - Two leds/qcom-lpg patches
> https://lore.kernel.org/linux-leds/20231126095230.683204-1-u.kleine-koenig@pengutronix.de
> Lee already claimed to have taken the series already, but it's not yet in
> next.
> - "leds: qcom-lpg: Introduce a wrapper for getting driver data from a pwm chip"
> https://lore.kernel.org/linux-leds/20231124215208.616551-3-u.kleine-koenig@pengutronix.de
>
> In the following patches I changed:
>
> - "pwm: cros-ec: Change prototype of helper to prepare further changes" +
> This was simplified in response to feedback by Tzung-Bi Shih
> - Make pwmchip_priv() static (and don't export it), let drivers use a new
> pwmchip_get_drvdata() instead.
> - For drm/ti-sn65dsi86.c and leds/qcom-lpg make use of
> pwmchip_set_drvdata() which makes the conversion to
> devm_pwmchip_alloc() much prettier.
> - Some cleanups here and there
> - Add review tags received in v3
> I kept all tags even though the pwmchip_alloc() patches changed
> slightly. Most of the time that's only
> s/pwmchip_priv/pwmchip_get_drvdata/ though. Still, if you object,
> just tell me. (This affects Paul Cercueil on patch #68, Conor Dooley
> on patch #76 and Greg for patch #109.)
>
> I kept the pwmchip_alloc() + pwmchip_register() approach despite Bart
> not liking it. To balance that out I don't like Bart's alternative
> approach. There are no technically relevant differences between the two
> approaches and no benchmarks that show either of the two to be better
> than the other. Conceptually the design ideas around pwmchip_alloc() +
> pwmchip_register() are used in several other subsystems, so it's a
> proven way to do things.
[Trimming the recipients, keeping only Bart and the mailing lists.]
I do think there are technically relevant differences. For one, the
better we isolate the internal data structure, the easier this becomes
to manage. I'm attaching a patch that I've prototyped which should
basically get us to somewhere around patch 110 of this series but with
something like 1/8th of the changes. It doesn't need every driver to
change and (mostly) decouples driver code from the core code.
Now, I know that you think this is all bad because it's not a single
allocation, but I much prefer the end result because it's got the driver
and internals much more cleanly separated. Going forward I think it
would be easier to apply all the ref-counting on top of that because we
only need to keep the PWM framework-internal data structure alive after
a PWM chip has gone away.
Thierry
-------------- next part --------------
From 72ea79887d96850f9ccc832ce52b907ca276c940 Mon Sep 17 00:00:00 2001
From: Thierry Reding <thierry.reding at gmail.com>
Date: Tue, 28 Nov 2023 15:42:39 +0100
Subject: [PATCH] pwm: Isolate internal data into a separate structure
In order to prepare for proper reference counting of PWM chips and PWM
devices, move the internal data from the public PWM chip to a private
PWM chip structure. This ensures that the data that the subsystem core
may need to reference later on can stick around beyond the lifetime of
the driver-private data.
Signed-off-by: Thierry Reding <thierry.reding at gmail.com>
---
drivers/pwm/core.c | 185 +++++++++++++++++++++-------------
drivers/pwm/internal.h | 38 +++++++
drivers/pwm/pwm-atmel-hlcdc.c | 8 +-
drivers/pwm/pwm-fsl-ftm.c | 6 +-
drivers/pwm/pwm-lpc18xx-sct.c | 4 +-
drivers/pwm/pwm-lpss.c | 14 +--
drivers/pwm/pwm-pca9685.c | 6 +-
drivers/pwm/pwm-samsung.c | 6 +-
drivers/pwm/pwm-sifive.c | 4 +-
drivers/pwm/pwm-stm32-lp.c | 6 +-
drivers/pwm/pwm-stm32.c | 6 +-
drivers/pwm/pwm-tiecap.c | 6 +-
drivers/pwm/pwm-tiehrpwm.c | 6 +-
drivers/pwm/sysfs.c | 48 ++++-----
include/linux/pwm.h | 26 +----
15 files changed, 228 insertions(+), 141 deletions(-)
create mode 100644 drivers/pwm/internal.h
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index f60b715abe62..54d57dec6dce 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -24,17 +24,19 @@
#define CREATE_TRACE_POINTS
#include <trace/events/pwm.h>
+#include "internal.h"
+
static DEFINE_MUTEX(pwm_lookup_lock);
static LIST_HEAD(pwm_lookup_list);
-/* protects access to pwmchip_idr */
+/* protects access to pwm_chips */
static DEFINE_MUTEX(pwm_lock);
-static DEFINE_IDR(pwmchip_idr);
+static DEFINE_IDR(pwm_chips);
static struct pwm_chip *pwmchip_find_by_name(const char *name)
{
- struct pwm_chip *chip;
+ struct pwm_chip_private *priv;
unsigned long id, tmp;
if (!name)
@@ -42,12 +44,12 @@ static struct pwm_chip *pwmchip_find_by_name(const char *name)
mutex_lock(&pwm_lock);
- idr_for_each_entry_ul(&pwmchip_idr, chip, tmp, id) {
- const char *chip_name = dev_name(chip->dev);
+ idr_for_each_entry_ul(&pwm_chips, priv, tmp, id) {
+ const char *chip_name = dev_name(priv->chip->dev);
if (chip_name && strcmp(chip_name, name) == 0) {
mutex_unlock(&pwm_lock);
- return chip;
+ return priv->chip;
}
}
@@ -58,23 +60,24 @@ static struct pwm_chip *pwmchip_find_by_name(const char *name)
static int pwm_device_request(struct pwm_device *pwm, const char *label)
{
+ struct pwm_chip *chip = pwm->priv->chip;
int err;
if (test_bit(PWMF_REQUESTED, &pwm->flags))
return -EBUSY;
- if (!try_module_get(pwm->chip->owner))
+ if (!try_module_get(pwm->priv->owner))
return -ENODEV;
- if (pwm->chip->ops->request) {
- err = pwm->chip->ops->request(pwm->chip, pwm);
+ if (chip->ops->request) {
+ err = chip->ops->request(chip, pwm);
if (err) {
- module_put(pwm->chip->owner);
+ module_put(pwm->priv->owner);
return err;
}
}
- if (pwm->chip->ops->get_state) {
+ if (chip->ops->get_state) {
/*
* Zero-initialize state because most drivers are unaware of
* .usage_power. The other members of state are supposed to be
@@ -84,7 +87,7 @@ static int pwm_device_request(struct pwm_device *pwm, const char *label)
*/
struct pwm_state state = { 0, };
- err = pwm->chip->ops->get_state(pwm->chip, pwm, &state);
+ err = chip->ops->get_state(chip, pwm, &state);
trace_pwm_get(pwm, &state, err);
if (!err)
@@ -196,6 +199,64 @@ static bool pwm_ops_check(const struct pwm_chip *chip)
return true;
}
+static struct pwm_chip_private *pwmchip_alloc(struct pwm_chip *chip,
+ struct module *owner)
+{
+ struct pwm_chip_private *priv;
+ struct pwm_device *pwm;
+ unsigned int i;
+ int err;
+
+ priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return ERR_PTR(-ENOMEM);
+
+ priv->pwms = kcalloc(chip->npwm, sizeof(*pwm), GFP_KERNEL);
+ if (!priv->pwms) {
+ err = -ENOMEM;
+ goto free;
+ }
+
+ priv->owner = owner;
+ priv->chip = chip;
+
+ for (i = 0; i < chip->npwm; i++) {
+ struct pwm_device *pwm = &priv->pwms[i];
+
+ pwm->priv = priv;
+ pwm->hwpwm = i;
+ }
+
+ mutex_lock(&pwm_lock);
+
+ err = idr_alloc(&pwm_chips, priv, 0, 0, GFP_KERNEL);
+ if (err < 0) {
+ mutex_unlock(&pwm_lock);
+ goto free;
+ }
+
+ mutex_unlock(&pwm_lock);
+
+ priv->id = err;
+
+ return priv;
+
+free:
+ kfree(priv->pwms);
+ kfree(priv);
+ return ERR_PTR(err);
+}
+
+static void pwmchip_free(struct pwm_chip_private *priv)
+{
+ mutex_lock(&pwm_lock);
+ idr_remove(&pwm_chips, priv->id);
+ mutex_unlock(&pwm_lock);
+
+ kfree(priv->pwms);
+ kfree(priv);
+}
+
/**
* __pwmchip_add() - register a new PWM chip
* @chip: the PWM chip to add
@@ -208,8 +269,7 @@ static bool pwm_ops_check(const struct pwm_chip *chip)
*/
int __pwmchip_add(struct pwm_chip *chip, struct module *owner)
{
- unsigned int i;
- int ret;
+ struct pwm_chip_private *priv;
if (!chip || !chip->dev || !chip->ops || !chip->npwm)
return -EINVAL;
@@ -217,36 +277,16 @@ int __pwmchip_add(struct pwm_chip *chip, struct module *owner)
if (!pwm_ops_check(chip))
return -EINVAL;
- chip->owner = owner;
-
- chip->pwms = kcalloc(chip->npwm, sizeof(*chip->pwms), GFP_KERNEL);
- if (!chip->pwms)
+ priv = pwmchip_alloc(chip, owner);
+ if (!priv)
return -ENOMEM;
- mutex_lock(&pwm_lock);
-
- ret = idr_alloc(&pwmchip_idr, chip, 0, 0, GFP_KERNEL);
- if (ret < 0) {
- mutex_unlock(&pwm_lock);
- kfree(chip->pwms);
- return ret;
- }
-
- chip->id = ret;
-
- for (i = 0; i < chip->npwm; i++) {
- struct pwm_device *pwm = &chip->pwms[i];
-
- pwm->chip = chip;
- pwm->hwpwm = i;
- }
-
- mutex_unlock(&pwm_lock);
+ chip->priv = priv;
if (IS_ENABLED(CONFIG_OF))
of_pwmchip_add(chip);
- pwmchip_sysfs_export(chip);
+ pwmchip_sysfs_export(priv);
return 0;
}
@@ -260,18 +300,14 @@ EXPORT_SYMBOL_GPL(__pwmchip_add);
*/
void pwmchip_remove(struct pwm_chip *chip)
{
- pwmchip_sysfs_unexport(chip);
-
- if (IS_ENABLED(CONFIG_OF))
- of_pwmchip_remove(chip);
-
mutex_lock(&pwm_lock);
- idr_remove(&pwmchip_idr, chip->id);
+ pwmchip_sysfs_unexport(chip->priv);
- mutex_unlock(&pwm_lock);
+ if (IS_ENABLED(CONFIG_OF))
+ of_pwmchip_remove(chip);
- kfree(chip->pwms);
+ pwmchip_free(chip->priv);
}
EXPORT_SYMBOL_GPL(pwmchip_remove);
@@ -315,7 +351,7 @@ struct pwm_device *pwm_request_from_chip(struct pwm_chip *chip,
return ERR_PTR(-EINVAL);
mutex_lock(&pwm_lock);
- pwm = &chip->pwms[index];
+ pwm = &chip->priv->pwms[index];
err = pwm_device_request(pwm, label);
if (err < 0)
@@ -329,9 +365,9 @@ EXPORT_SYMBOL_GPL(pwm_request_from_chip);
static void pwm_apply_state_debug(struct pwm_device *pwm,
const struct pwm_state *state)
{
- struct pwm_state *last = &pwm->last;
- struct pwm_chip *chip = pwm->chip;
+ struct pwm_chip *chip = pwm->priv->chip;
struct pwm_state s1 = { 0 }, s2 = { 0 };
+ struct pwm_state *last = &pwm->last;
int err;
if (!IS_ENABLED(CONFIG_PWM_DEBUG))
@@ -439,7 +475,6 @@ static void pwm_apply_state_debug(struct pwm_device *pwm,
*/
int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state)
{
- struct pwm_chip *chip;
int err;
/*
@@ -455,8 +490,6 @@ int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state)
state->duty_cycle > state->period)
return -EINVAL;
- chip = pwm->chip;
-
if (state->period == pwm->state.period &&
state->duty_cycle == pwm->state.duty_cycle &&
state->polarity == pwm->state.polarity &&
@@ -464,7 +497,7 @@ int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state)
state->usage_power == pwm->state.usage_power)
return 0;
- err = chip->ops->apply(chip, pwm, state);
+ err = pwm->priv->chip->ops->apply(pwm->priv->chip, pwm, state);
trace_pwm_apply(pwm, state, err);
if (err)
return err;
@@ -492,16 +525,19 @@ EXPORT_SYMBOL_GPL(pwm_apply_state);
int pwm_capture(struct pwm_device *pwm, struct pwm_capture *result,
unsigned long timeout)
{
+ struct pwm_chip *chip;
int err;
- if (!pwm || !pwm->chip->ops)
+ if (!pwm)
return -EINVAL;
- if (!pwm->chip->ops->capture)
+ chip = pwm->priv->chip;
+
+ if (!chip || !chip->ops || !chip->ops->capture)
return -ENOSYS;
mutex_lock(&pwm_lock);
- err = pwm->chip->ops->capture(pwm->chip, pwm, result, timeout);
+ err = chip->ops->capture(chip, pwm, result, timeout);
mutex_unlock(&pwm_lock);
return err;
@@ -566,16 +602,19 @@ EXPORT_SYMBOL_GPL(pwm_adjust_config);
static struct pwm_chip *fwnode_to_pwmchip(struct fwnode_handle *fwnode)
{
- struct pwm_chip *chip;
+ struct pwm_chip_private *priv;
unsigned long id, tmp;
mutex_lock(&pwm_lock);
- idr_for_each_entry_ul(&pwmchip_idr, chip, tmp, id)
+ idr_for_each_entry_ul(&pwm_chips, priv, tmp, id) {
+ struct pwm_chip *chip = priv->chip;
+
if (chip->dev && device_match_fwnode(chip->dev, fwnode)) {
mutex_unlock(&pwm_lock);
return chip;
}
+ }
mutex_unlock(&pwm_lock);
@@ -585,6 +624,7 @@ static struct pwm_chip *fwnode_to_pwmchip(struct fwnode_handle *fwnode)
static struct device_link *pwm_device_link_add(struct device *dev,
struct pwm_device *pwm)
{
+ struct pwm_chip *chip = pwm->priv->chip;
struct device_link *dl;
if (!dev) {
@@ -593,15 +633,15 @@ static struct device_link *pwm_device_link_add(struct device *dev,
* impact the PM sequence ordering: the PWM supplier may get
* suspended before the consumer.
*/
- dev_warn(pwm->chip->dev,
+ dev_warn(chip->dev,
"No consumer device specified to create a link to\n");
return NULL;
}
- dl = device_link_add(dev, pwm->chip->dev, DL_FLAG_AUTOREMOVE_CONSUMER);
+ dl = device_link_add(dev, chip->dev, DL_FLAG_AUTOREMOVE_CONSUMER);
if (!dl) {
dev_err(dev, "failed to create device link to %s\n",
- dev_name(pwm->chip->dev));
+ dev_name(chip->dev));
return ERR_PTR(-EINVAL);
}
@@ -918,12 +958,12 @@ void pwm_put(struct pwm_device *pwm)
goto out;
}
- if (pwm->chip->ops->free)
- pwm->chip->ops->free(pwm->chip, pwm);
+ if (pwm->priv->chip->ops->free)
+ pwm->priv->chip->ops->free(pwm->priv->chip, pwm);
pwm->label = NULL;
- module_put(pwm->chip->owner);
+ module_put(pwm->priv->owner);
out:
mutex_unlock(&pwm_lock);
}
@@ -997,12 +1037,12 @@ struct pwm_device *devm_fwnode_pwm_get(struct device *dev,
EXPORT_SYMBOL_GPL(devm_fwnode_pwm_get);
#ifdef CONFIG_DEBUG_FS
-static void pwm_dbg_show(struct pwm_chip *chip, struct seq_file *s)
+static void pwm_dbg_show(struct pwm_chip_private *priv, struct seq_file *s)
{
unsigned int i;
- for (i = 0; i < chip->npwm; i++) {
- struct pwm_device *pwm = &chip->pwms[i];
+ for (i = 0; i < priv->chip->npwm; i++) {
+ struct pwm_device *pwm = &priv->pwms[i];
struct pwm_state state;
pwm_get_state(pwm, &state);
@@ -1035,7 +1075,7 @@ static void *pwm_seq_start(struct seq_file *s, loff_t *pos)
mutex_lock(&pwm_lock);
s->private = "";
- ret = idr_get_next_ul(&pwmchip_idr, &id);
+ ret = idr_get_next_ul(&pwm_chips, &id);
*pos = id;
return ret;
}
@@ -1047,7 +1087,7 @@ static void *pwm_seq_next(struct seq_file *s, void *v, loff_t *pos)
s->private = "\n";
- ret = idr_get_next_ul(&pwmchip_idr, &id);
+ ret = idr_get_next_ul(&pwm_chips, &id);
*pos = id;
return ret;
}
@@ -1059,15 +1099,16 @@ static void pwm_seq_stop(struct seq_file *s, void *v)
static int pwm_seq_show(struct seq_file *s, void *v)
{
- struct pwm_chip *chip = v;
+ struct pwm_chip_private *priv = v;
+ struct pwm_chip *chip = priv->chip;
seq_printf(s, "%s%d: %s/%s, %d PWM device%s\n",
- (char *)s->private, chip->id,
+ (char *)s->private, priv->id,
chip->dev->bus ? chip->dev->bus->name : "no-bus",
dev_name(chip->dev), chip->npwm,
(chip->npwm != 1) ? "s" : "");
- pwm_dbg_show(chip, s);
+ pwm_dbg_show(priv, s);
return 0;
}
diff --git a/drivers/pwm/internal.h b/drivers/pwm/internal.h
new file mode 100644
index 000000000000..44fffd3b6744
--- /dev/null
+++ b/drivers/pwm/internal.h
@@ -0,0 +1,38 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef PWM_INTERNAL_H
+#define PWM_INTERNAL_H
+
+#include <linux/list.h>
+
+struct pwm_chip;
+struct pwm_device;
+
+/**
+ * struct pwm_chip_private - subsystem-private PWM chip data
+ * @chip: driver-private PWM chip data
+ * @owner: module providing the chip
+ * @pwms: array of PWM devices allocated by the framework
+ * @base: number of first PWM controlled by this chip
+ */
+struct pwm_chip_private {
+ struct pwm_chip *chip;
+ struct module *owner;
+ struct pwm_device *pwms;
+ unsigned int id;
+};
+
+#ifdef CONFIG_PWM_SYSFS
+void pwmchip_sysfs_export(struct pwm_chip_private *priv);
+void pwmchip_sysfs_unexport(struct pwm_chip_private *priv);
+#else
+static inline void pwmchip_sysfs_export(struct pwm_chip_private *priv)
+{
+}
+
+static inline void pwmchip_sysfs_unexport(struct pwm_chip_private *priv)
+{
+}
+#endif /* CONFIG_PWM_SYSFS */
+
+#endif /* PWM_INTERNAL_H */
diff --git a/drivers/pwm/pwm-atmel-hlcdc.c b/drivers/pwm/pwm-atmel-hlcdc.c
index 3f2c5031a3ba..6bbbfaa582c1 100644
--- a/drivers/pwm/pwm-atmel-hlcdc.c
+++ b/drivers/pwm/pwm-atmel-hlcdc.c
@@ -15,6 +15,8 @@
#include <linux/pwm.h>
#include <linux/regmap.h>
+#include "internal.h"
+
#define ATMEL_HLCDC_PWMCVAL_MASK GENMASK(15, 8)
#define ATMEL_HLCDC_PWMCVAL(x) (((x) << 8) & ATMEL_HLCDC_PWMCVAL_MASK)
#define ATMEL_HLCDC_PWMPOL BIT(4)
@@ -185,7 +187,7 @@ static int atmel_hlcdc_pwm_suspend(struct device *dev)
struct atmel_hlcdc_pwm *atmel = dev_get_drvdata(dev);
/* Keep the periph clock enabled if the PWM is still running. */
- if (pwm_is_enabled(&atmel->chip.pwms[0]))
+ if (pwm_is_enabled(&atmel->chip.priv->pwms[0]))
clk_disable_unprepare(atmel->hlcdc->periph_clk);
return 0;
@@ -197,7 +199,7 @@ static int atmel_hlcdc_pwm_resume(struct device *dev)
struct pwm_state state;
int ret;
- pwm_get_state(&atmel->chip.pwms[0], &state);
+ pwm_get_state(&atmel->chip.priv->pwms[0], &state);
/* Re-enable the periph clock it was stopped during suspend. */
if (!state.enabled) {
@@ -206,7 +208,7 @@ static int atmel_hlcdc_pwm_resume(struct device *dev)
return ret;
}
- return atmel_hlcdc_pwm_apply(&atmel->chip, &atmel->chip.pwms[0],
+ return atmel_hlcdc_pwm_apply(&atmel->chip, &atmel->chip.priv->pwms[0],
&state);
}
diff --git a/drivers/pwm/pwm-fsl-ftm.c b/drivers/pwm/pwm-fsl-ftm.c
index d1b6d1aa4773..72467d620a90 100644
--- a/drivers/pwm/pwm-fsl-ftm.c
+++ b/drivers/pwm/pwm-fsl-ftm.c
@@ -19,6 +19,8 @@
#include <linux/slab.h>
#include <linux/fsl/ftm.h>
+#include "internal.h"
+
#define FTM_SC_CLK(c) (((c) + 1) << FTM_SC_CLK_MASK_SHIFT)
enum fsl_pwm_clk {
@@ -468,7 +470,7 @@ static int fsl_pwm_suspend(struct device *dev)
regcache_mark_dirty(fpc->regmap);
for (i = 0; i < fpc->chip.npwm; i++) {
- struct pwm_device *pwm = &fpc->chip.pwms[i];
+ struct pwm_device *pwm = &fpc->chip.priv->pwms[i];
if (!test_bit(PWMF_REQUESTED, &pwm->flags))
continue;
@@ -491,7 +493,7 @@ static int fsl_pwm_resume(struct device *dev)
int i;
for (i = 0; i < fpc->chip.npwm; i++) {
- struct pwm_device *pwm = &fpc->chip.pwms[i];
+ struct pwm_device *pwm = &fpc->chip.priv->pwms[i];
if (!test_bit(PWMF_REQUESTED, &pwm->flags))
continue;
diff --git a/drivers/pwm/pwm-lpc18xx-sct.c b/drivers/pwm/pwm-lpc18xx-sct.c
index b3d4a955aa31..80f89ad2bc8f 100644
--- a/drivers/pwm/pwm-lpc18xx-sct.c
+++ b/drivers/pwm/pwm-lpc18xx-sct.c
@@ -27,6 +27,8 @@
#include <linux/platform_device.h>
#include <linux/pwm.h>
+#include "internal.h"
+
/* LPC18xx SCT registers */
#define LPC18XX_PWM_CONFIG 0x000
#define LPC18XX_PWM_CONFIG_UNIFY BIT(0)
@@ -224,7 +226,7 @@ static int lpc18xx_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
!lpc18xx_pwm->period_ns) {
lpc18xx_pwm->period_ns = period_ns;
for (i = 0; i < chip->npwm; i++)
- pwm_set_period(&chip->pwms[i], period_ns);
+ pwm_set_period(&chip->priv->pwms[i], period_ns);
lpc18xx_pwm_config_period(chip, period_ns);
}
diff --git a/drivers/pwm/pwm-lpss.c b/drivers/pwm/pwm-lpss.c
index a6ea3ce7e019..085d3c17f96b 100644
--- a/drivers/pwm/pwm-lpss.c
+++ b/drivers/pwm/pwm-lpss.c
@@ -19,6 +19,8 @@
#include <linux/pm_runtime.h>
#include <linux/time.h>
+#include "internal.h"
+
#define DEFAULT_SYMBOL_NAMESPACE PWM_LPSS
#include "pwm-lpss.h"
@@ -73,21 +75,21 @@ static inline struct pwm_lpss_chip *to_lpwm(struct pwm_chip *chip)
static inline u32 pwm_lpss_read(const struct pwm_device *pwm)
{
- struct pwm_lpss_chip *lpwm = to_lpwm(pwm->chip);
+ struct pwm_lpss_chip *lpwm = to_lpwm(pwm->priv->chip);
return readl(lpwm->regs + pwm->hwpwm * PWM_SIZE + PWM);
}
static inline void pwm_lpss_write(const struct pwm_device *pwm, u32 value)
{
- struct pwm_lpss_chip *lpwm = to_lpwm(pwm->chip);
+ struct pwm_lpss_chip *lpwm = to_lpwm(pwm->priv->chip);
writel(value, lpwm->regs + pwm->hwpwm * PWM_SIZE + PWM);
}
static int pwm_lpss_wait_for_update(struct pwm_device *pwm)
{
- struct pwm_lpss_chip *lpwm = to_lpwm(pwm->chip);
+ struct pwm_lpss_chip *lpwm = to_lpwm(pwm->priv->chip);
const void __iomem *addr = lpwm->regs + pwm->hwpwm * PWM_SIZE + PWM;
const unsigned int ms = 500 * USEC_PER_MSEC;
u32 val;
@@ -106,7 +108,7 @@ static int pwm_lpss_wait_for_update(struct pwm_device *pwm)
*/
err = readl_poll_timeout(addr, val, !(val & PWM_SW_UPDATE), 40, ms);
if (err)
- dev_err(pwm->chip->dev, "PWM_SW_UPDATE was not cleared\n");
+ dev_err(pwm->priv->chip->dev, "PWM_SW_UPDATE was not cleared\n");
return err;
}
@@ -114,7 +116,7 @@ static int pwm_lpss_wait_for_update(struct pwm_device *pwm)
static inline int pwm_lpss_is_updating(struct pwm_device *pwm)
{
if (pwm_lpss_read(pwm) & PWM_SW_UPDATE) {
- dev_err(pwm->chip->dev, "PWM_SW_UPDATE is still set, skipping update\n");
+ dev_err(pwm->priv->chip->dev, "PWM_SW_UPDATE is still set, skipping update\n");
return -EBUSY;
}
@@ -278,7 +280,7 @@ struct pwm_lpss_chip *devm_pwm_lpss_probe(struct device *dev, void __iomem *base
}
for (i = 0; i < lpwm->info->npwm; i++) {
- ctrl = pwm_lpss_read(&lpwm->chip.pwms[i]);
+ ctrl = pwm_lpss_read(&lpwm->chip.priv->pwms[i]);
if (ctrl & PWM_ENABLE)
pm_runtime_get(dev);
}
diff --git a/drivers/pwm/pwm-pca9685.c b/drivers/pwm/pwm-pca9685.c
index e79b1de8c4d8..d82cca90dba9 100644
--- a/drivers/pwm/pwm-pca9685.c
+++ b/drivers/pwm/pwm-pca9685.c
@@ -22,6 +22,8 @@
#include <linux/pm_runtime.h>
#include <linux/bitmap.h>
+#include "internal.h"
+
/*
* Because the PCA9685 has only one prescaler per chip, only the first channel
* that is enabled is allowed to change the prescale register.
@@ -134,7 +136,7 @@ static int pca9685_write_reg(struct pca9685 *pca, unsigned int reg, unsigned int
/* 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)
{
- struct pwm_device *pwm = &pca->chip.pwms[channel];
+ struct pwm_device *pwm = &pca->chip.priv->pwms[channel];
unsigned int on, off;
if (duty == 0) {
@@ -173,7 +175,7 @@ static void pca9685_pwm_set_duty(struct pca9685 *pca, int channel, unsigned int
static unsigned int pca9685_pwm_get_duty(struct pca9685 *pca, int channel)
{
- struct pwm_device *pwm = &pca->chip.pwms[channel];
+ struct pwm_device *pwm = &pca->chip.priv->pwms[channel];
unsigned int off = 0, on = 0, val = 0;
if (WARN_ON(channel >= PCA9685_MAXCHAN)) {
diff --git a/drivers/pwm/pwm-samsung.c b/drivers/pwm/pwm-samsung.c
index 6e77302f7368..e1baede405bc 100644
--- a/drivers/pwm/pwm-samsung.c
+++ b/drivers/pwm/pwm-samsung.c
@@ -26,6 +26,8 @@
/* For struct samsung_timer_variant and samsung_pwm_lock. */
#include <clocksource/samsung_pwm.h>
+#include "internal.h"
+
#define REG_TCFG0 0x00
#define REG_TCFG1 0x04
#define REG_TCON 0x08
@@ -627,10 +629,10 @@ static int pwm_samsung_resume(struct device *dev)
unsigned int i;
for (i = 0; i < SAMSUNG_PWM_NUM; i++) {
- struct pwm_device *pwm = &chip->pwms[i];
+ struct pwm_device *pwm = &chip->priv->pwms[i];
struct samsung_pwm_channel *chan = &our_chip->channel[i];
- if (!test_bit(PWMF_REQUESTED, &pwm->flags))
+ if (WARN_ON(!pwm) || !test_bit(PWMF_REQUESTED, &pwm->flags))
continue;
if (our_chip->variant.output_mask & BIT(i))
diff --git a/drivers/pwm/pwm-sifive.c b/drivers/pwm/pwm-sifive.c
index 089e50bdbbf0..e1dccaf33398 100644
--- a/drivers/pwm/pwm-sifive.c
+++ b/drivers/pwm/pwm-sifive.c
@@ -20,6 +20,8 @@
#include <linux/slab.h>
#include <linux/bitfield.h>
+#include "internal.h"
+
/* Register offsets */
#define PWM_SIFIVE_PWMCFG 0x0
#define PWM_SIFIVE_PWMCOUNT 0x8
@@ -322,7 +324,7 @@ static void pwm_sifive_remove(struct platform_device *dev)
clk_notifier_unregister(ddata->clk, &ddata->notifier);
for (ch = 0; ch < ddata->chip.npwm; ch++) {
- pwm = &ddata->chip.pwms[ch];
+ pwm = &ddata->chip.priv->pwms[ch];
if (pwm->state.enabled)
clk_disable(ddata->clk);
}
diff --git a/drivers/pwm/pwm-stm32-lp.c b/drivers/pwm/pwm-stm32-lp.c
index 439068f3eca1..cb32caf5368a 100644
--- a/drivers/pwm/pwm-stm32-lp.c
+++ b/drivers/pwm/pwm-stm32-lp.c
@@ -17,6 +17,8 @@
#include <linux/platform_device.h>
#include <linux/pwm.h>
+#include "internal.h"
+
struct stm32_pwm_lp {
struct pwm_chip chip;
struct clk *clk;
@@ -223,10 +225,10 @@ static int stm32_pwm_lp_suspend(struct device *dev)
struct stm32_pwm_lp *priv = dev_get_drvdata(dev);
struct pwm_state state;
- pwm_get_state(&priv->chip.pwms[0], &state);
+ pwm_get_state(&priv->chip.priv->pwms[0], &state);
if (state.enabled) {
dev_err(dev, "The consumer didn't stop us (%s)\n",
- priv->chip.pwms[0].label);
+ priv->chip.priv->pwms[0].label);
return -EBUSY;
}
diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c
index 5f10cba492ec..81933df80768 100644
--- a/drivers/pwm/pwm-stm32.c
+++ b/drivers/pwm/pwm-stm32.c
@@ -16,6 +16,8 @@
#include <linux/platform_device.h>
#include <linux/pwm.h>
+#include "internal.h"
+
#define CCMR_CHANNEL_SHIFT 8
#define CCMR_CHANNEL_MASK 0xFF
#define MAX_BREAKINPUT 2
@@ -127,7 +129,7 @@ static int stm32_pwm_raw_capture(struct stm32_pwm *priv, struct pwm_device *pwm,
*raw_prd = priv->max_arr - priv->capture[0] + priv->capture[2];
/* Duty cycle capture requires at least two capture units */
- if (pwm->chip->npwm < 2)
+ if (priv->chip.npwm < 2)
*raw_dty = 0;
else if (priv->capture[0] <= priv->capture[3])
*raw_dty = priv->capture[3] - priv->capture[0];
@@ -682,7 +684,7 @@ static int stm32_pwm_suspend(struct device *dev)
mask = TIM_CCER_CC1E << (i * 4);
if (ccer & mask) {
dev_err(dev, "PWM %u still in use by consumer %s\n",
- i, priv->chip.pwms[i].label);
+ i, priv->chip.priv->pwms[i].label);
return -EBUSY;
}
}
diff --git a/drivers/pwm/pwm-tiecap.c b/drivers/pwm/pwm-tiecap.c
index d974f4414ac9..f089257051d1 100644
--- a/drivers/pwm/pwm-tiecap.c
+++ b/drivers/pwm/pwm-tiecap.c
@@ -14,6 +14,8 @@
#include <linux/pwm.h>
#include <linux/of.h>
+#include "internal.h"
+
/* ECAP registers and bits definitions */
#define CAP1 0x08
#define CAP2 0x0C
@@ -288,7 +290,7 @@ static void ecap_pwm_restore_context(struct ecap_pwm_chip *pc)
static int ecap_pwm_suspend(struct device *dev)
{
struct ecap_pwm_chip *pc = dev_get_drvdata(dev);
- struct pwm_device *pwm = pc->chip.pwms;
+ struct pwm_device *pwm = pc->chip.priv->pwms;
ecap_pwm_save_context(pc);
@@ -302,7 +304,7 @@ static int ecap_pwm_suspend(struct device *dev)
static int ecap_pwm_resume(struct device *dev)
{
struct ecap_pwm_chip *pc = dev_get_drvdata(dev);
- struct pwm_device *pwm = pc->chip.pwms;
+ struct pwm_device *pwm = pc->chip.priv->pwms;
/* Enable explicitly if PWM was running */
if (pwm_is_enabled(pwm))
diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
index af231fa74fa9..0d137100b040 100644
--- a/drivers/pwm/pwm-tiehrpwm.c
+++ b/drivers/pwm/pwm-tiehrpwm.c
@@ -14,6 +14,8 @@
#include <linux/pm_runtime.h>
#include <linux/of.h>
+#include "internal.h"
+
/* EHRPWM registers and bits definitions */
/* Time base module registers */
@@ -557,7 +559,7 @@ static int ehrpwm_pwm_suspend(struct device *dev)
ehrpwm_pwm_save_context(pc);
for (i = 0; i < pc->chip.npwm; i++) {
- struct pwm_device *pwm = &pc->chip.pwms[i];
+ struct pwm_device *pwm = &pc->chip.priv->pwms[i];
if (!pwm_is_enabled(pwm))
continue;
@@ -575,7 +577,7 @@ static int ehrpwm_pwm_resume(struct device *dev)
unsigned int i;
for (i = 0; i < pc->chip.npwm; i++) {
- struct pwm_device *pwm = &pc->chip.pwms[i];
+ struct pwm_device *pwm = &pc->chip.priv->pwms[i];
if (!pwm_is_enabled(pwm))
continue;
diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c
index 4edb994fa2e1..653df20afe1c 100644
--- a/drivers/pwm/sysfs.c
+++ b/drivers/pwm/sysfs.c
@@ -14,6 +14,8 @@
#include <linux/kdev_t.h>
#include <linux/pwm.h>
+#include "internal.h"
+
struct pwm_export {
struct device child;
struct pwm_device *pwm;
@@ -311,7 +313,7 @@ static ssize_t export_store(struct device *parent,
struct device_attribute *attr,
const char *buf, size_t len)
{
- struct pwm_chip *chip = dev_get_drvdata(parent);
+ struct pwm_chip_private *priv = dev_get_drvdata(parent);
struct pwm_device *pwm;
unsigned int hwpwm;
int ret;
@@ -320,10 +322,10 @@ static ssize_t export_store(struct device *parent,
if (ret < 0)
return ret;
- if (hwpwm >= chip->npwm)
+ if (hwpwm >= priv->chip->npwm)
return -ENODEV;
- pwm = pwm_request_from_chip(chip, hwpwm, "sysfs");
+ pwm = pwm_request_from_chip(priv->chip, hwpwm, "sysfs");
if (IS_ERR(pwm))
return PTR_ERR(pwm);
@@ -339,7 +341,7 @@ static ssize_t unexport_store(struct device *parent,
struct device_attribute *attr,
const char *buf, size_t len)
{
- struct pwm_chip *chip = dev_get_drvdata(parent);
+ struct pwm_chip_private *priv = dev_get_drvdata(parent);
unsigned int hwpwm;
int ret;
@@ -347,10 +349,10 @@ static ssize_t unexport_store(struct device *parent,
if (ret < 0)
return ret;
- if (hwpwm >= chip->npwm)
+ if (hwpwm >= priv->chip->npwm)
return -ENODEV;
- ret = pwm_unexport_child(parent, &chip->pwms[hwpwm]);
+ ret = pwm_unexport_child(parent, &priv->pwms[hwpwm]);
return ret ? : len;
}
@@ -359,9 +361,9 @@ static DEVICE_ATTR_WO(unexport);
static ssize_t npwm_show(struct device *parent, struct device_attribute *attr,
char *buf)
{
- const struct pwm_chip *chip = dev_get_drvdata(parent);
+ const struct pwm_chip_private *priv = dev_get_drvdata(parent);
- return sysfs_emit(buf, "%u\n", chip->npwm);
+ return sysfs_emit(buf, "%u\n", priv->chip->npwm);
}
static DEVICE_ATTR_RO(npwm);
@@ -411,12 +413,12 @@ static int pwm_class_apply_state(struct pwm_export *export,
static int pwm_class_resume_npwm(struct device *parent, unsigned int npwm)
{
- struct pwm_chip *chip = dev_get_drvdata(parent);
+ struct pwm_chip_private *priv = dev_get_drvdata(parent);
unsigned int i;
int ret = 0;
for (i = 0; i < npwm; i++) {
- struct pwm_device *pwm = &chip->pwms[i];
+ struct pwm_device *pwm = &priv->pwms[i];
struct pwm_state state;
struct pwm_export *export;
@@ -442,12 +444,12 @@ static int pwm_class_resume_npwm(struct device *parent, unsigned int npwm)
static int pwm_class_suspend(struct device *parent)
{
- struct pwm_chip *chip = dev_get_drvdata(parent);
+ struct pwm_chip_private *priv = dev_get_drvdata(parent);
unsigned int i;
int ret = 0;
- for (i = 0; i < chip->npwm; i++) {
- struct pwm_device *pwm = &chip->pwms[i];
+ for (i = 0; i < priv->chip->npwm; i++) {
+ struct pwm_device *pwm = &priv->pwms[i];
struct pwm_state state;
struct pwm_export *export;
@@ -483,9 +485,9 @@ static int pwm_class_suspend(struct device *parent)
static int pwm_class_resume(struct device *parent)
{
- struct pwm_chip *chip = dev_get_drvdata(parent);
+ struct pwm_chip_private *priv = dev_get_drvdata(parent);
- return pwm_class_resume_npwm(parent, chip->npwm);
+ return pwm_class_resume_npwm(parent, priv->chip->npwm);
}
static DEFINE_SIMPLE_DEV_PM_OPS(pwm_class_pm_ops, pwm_class_suspend, pwm_class_resume);
@@ -501,7 +503,7 @@ static int pwmchip_sysfs_match(struct device *parent, const void *data)
return dev_get_drvdata(parent) == data;
}
-void pwmchip_sysfs_export(struct pwm_chip *chip)
+void pwmchip_sysfs_export(struct pwm_chip_private *priv)
{
struct device *parent;
@@ -509,26 +511,26 @@ void pwmchip_sysfs_export(struct pwm_chip *chip)
* If device_create() fails the pwm_chip is still usable by
* the kernel it's just not exported.
*/
- parent = device_create(&pwm_class, chip->dev, MKDEV(0, 0), chip,
- "pwmchip%d", chip->id);
+ parent = device_create(&pwm_class, priv->chip->dev, MKDEV(0, 0), priv,
+ "pwmchip%d", priv->id);
if (IS_ERR(parent)) {
- dev_warn(chip->dev,
+ dev_warn(priv->chip->dev,
"device_create failed for pwm_chip sysfs export\n");
}
}
-void pwmchip_sysfs_unexport(struct pwm_chip *chip)
+void pwmchip_sysfs_unexport(struct pwm_chip_private *priv)
{
struct device *parent;
unsigned int i;
- parent = class_find_device(&pwm_class, NULL, chip,
+ parent = class_find_device(&pwm_class, NULL, priv,
pwmchip_sysfs_match);
if (!parent)
return;
- for (i = 0; i < chip->npwm; i++) {
- struct pwm_device *pwm = &chip->pwms[i];
+ for (i = 0; i < priv->chip->npwm; i++) {
+ struct pwm_device *pwm = &priv->pwms[i];
if (test_bit(PWMF_EXPORTED, &pwm->flags))
pwm_unexport_child(parent, pwm);
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index f87655c06c82..5c478cd592d7 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -6,6 +6,7 @@
#include <linux/mutex.h>
#include <linux/of.h>
+struct pwm_chip_private;
struct pwm_chip;
/**
@@ -79,11 +80,12 @@ struct pwm_device {
const char *label;
unsigned long flags;
unsigned int hwpwm;
- struct pwm_chip *chip;
struct pwm_args args;
struct pwm_state state;
struct pwm_state last;
+
+ struct pwm_chip_private *priv;
};
/**
@@ -280,26 +282,21 @@ struct pwm_ops {
* struct pwm_chip - abstract a PWM controller
* @dev: device providing the PWMs
* @ops: callbacks for this PWM controller
- * @owner: module providing this chip
- * @id: unique number of this PWM chip
* @npwm: number of PWMs controlled by this chip
* @of_xlate: request a PWM device given a device tree PWM specifier
* @of_pwm_n_cells: number of cells expected in the device tree PWM specifier
- * @pwms: array of PWM devices allocated by the framework
+ * @priv: subsystem internal chip data
*/
struct pwm_chip {
struct device *dev;
const struct pwm_ops *ops;
- struct module *owner;
- unsigned int id;
unsigned int npwm;
struct pwm_device * (*of_xlate)(struct pwm_chip *chip,
const struct of_phandle_args *args);
unsigned int of_pwm_n_cells;
- /* only used internally by the PWM framework */
- struct pwm_device *pwms;
+ struct pwm_chip_private *priv;
};
#if IS_ENABLED(CONFIG_PWM)
@@ -564,17 +561,4 @@ static inline void pwm_remove_table(struct pwm_lookup *table, size_t num)
}
#endif
-#ifdef CONFIG_PWM_SYSFS
-void pwmchip_sysfs_export(struct pwm_chip *chip);
-void pwmchip_sysfs_unexport(struct pwm_chip *chip);
-#else
-static inline void pwmchip_sysfs_export(struct pwm_chip *chip)
-{
-}
-
-static inline void pwmchip_sysfs_unexport(struct pwm_chip *chip)
-{
-}
-#endif /* CONFIG_PWM_SYSFS */
-
#endif /* __LINUX_PWM_H */
--
2.43.0
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-rockchip/attachments/20231208/625347ce/attachment-0001.sig>
More information about the Linux-rockchip
mailing list