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@xxxxxxxxxxxxxx > - "pwm: crc: Use consistent variable naming for driver data" > https://lore.kernel.org/linux-pwm/20231130074133.969806-1-u.kleine-koenig@xxxxxxxxxxxxxx > - Two leds/qcom-lpg patches > https://lore.kernel.org/linux-leds/20231126095230.683204-1-u.kleine-koenig@xxxxxxxxxxxxxx > 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@xxxxxxxxxxxxxx > > 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
From 72ea79887d96850f9ccc832ce52b907ca276c940 Mon Sep 17 00:00:00 2001 From: Thierry Reding <thierry.reding@xxxxxxxxx> 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@xxxxxxxxx> --- 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
Attachment:
signature.asc
Description: PGP signature