From 940052bcbcd50081244f995e3cad89eaa2cc1c04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Mon, 18 Mar 2024 17:09:49 +0100 Subject: [PATCH 01/34] hwmon: (aspeed-g6-pwm-tacho): Make use of pwmchip_parent() accessor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit struct pwm_chip::dev is about to change. To not have to touch this driver in the same commit as struct pwm_chip::dev, use the accessor function provided for exactly this purpose. Acked-by: Guenter Roeck Link: https://lore.kernel.org/r/5c6a11dd10cd29e0f7bfaa1fdef145523347cb58.1710777536.git.u.kleine-koenig@pengutronix.de Signed-off-by: Uwe Kleine-König --- drivers/hwmon/aspeed-g6-pwm-tach.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/hwmon/aspeed-g6-pwm-tach.c b/drivers/hwmon/aspeed-g6-pwm-tach.c index 597b3b019d49..64def5e4cdf6 100644 --- a/drivers/hwmon/aspeed-g6-pwm-tach.c +++ b/drivers/hwmon/aspeed-g6-pwm-tach.c @@ -195,7 +195,7 @@ static int aspeed_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, expect_period = div64_u64(ULLONG_MAX, (u64)priv->clk_rate); expect_period = min(expect_period, state->period); - dev_dbg(chip->dev, "expect period: %lldns, duty_cycle: %lldns", + dev_dbg(pwmchip_parent(chip), "expect period: %lldns, duty_cycle: %lldns", expect_period, state->duty_cycle); /* * Pick the smallest value for div_h so that div_l can be the biggest @@ -218,12 +218,12 @@ static int aspeed_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, if (div_l > 255) div_l = 255; - dev_dbg(chip->dev, "clk source: %ld div_h %lld, div_l : %lld\n", + dev_dbg(pwmchip_parent(chip), "clk source: %ld div_h %lld, div_l : %lld\n", priv->clk_rate, div_h, div_l); /* duty_pt = duty_cycle * (PERIOD + 1) / period */ duty_pt = div64_u64(state->duty_cycle * priv->clk_rate, (u64)NSEC_PER_SEC * (div_l + 1) << div_h); - dev_dbg(chip->dev, "duty_cycle = %lld, duty_pt = %d\n", + dev_dbg(pwmchip_parent(chip), "duty_cycle = %lld, duty_pt = %d\n", state->duty_cycle, duty_pt); /* From 79dedfadb79e527ca4dc6f3727dace96e3333f82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Mon, 18 Mar 2024 17:09:50 +0100 Subject: [PATCH 02/34] hwmon: (aspeed-g6-pwm-tacho): Make use of devm_pwmchip_alloc() function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This prepares the aspeed-g6-pwm-tacho driver to further changes of the pwm core outlined in the commit introducing devm_pwmchip_alloc(). There is no intended semantical change and the driver should behave as before. Acked-by: Guenter Roeck Link: https://lore.kernel.org/r/e95e41eea5a138ae206c9116ba3cb1d9e0178284.1710777536.git.u.kleine-koenig@pengutronix.de Signed-off-by: Uwe Kleine-König --- drivers/hwmon/aspeed-g6-pwm-tach.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/drivers/hwmon/aspeed-g6-pwm-tach.c b/drivers/hwmon/aspeed-g6-pwm-tach.c index 64def5e4cdf6..332c02216668 100644 --- a/drivers/hwmon/aspeed-g6-pwm-tach.c +++ b/drivers/hwmon/aspeed-g6-pwm-tach.c @@ -136,7 +136,6 @@ struct aspeed_pwm_tach_data { struct clk *clk; struct reset_control *reset; unsigned long clk_rate; - struct pwm_chip chip; bool tach_present[TACH_ASPEED_NR_TACHS]; u32 tach_divisor; }; @@ -144,7 +143,7 @@ struct aspeed_pwm_tach_data { static inline struct aspeed_pwm_tach_data * aspeed_pwm_chip_to_data(struct pwm_chip *chip) { - return container_of(chip, struct aspeed_pwm_tach_data, chip); + return pwmchip_get_drvdata(chip); } static int aspeed_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm, @@ -459,6 +458,7 @@ static int aspeed_pwm_tach_probe(struct platform_device *pdev) int ret; struct device_node *child; struct aspeed_pwm_tach_data *priv; + struct pwm_chip *chip; priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); if (!priv) @@ -487,11 +487,14 @@ static int aspeed_pwm_tach_probe(struct platform_device *pdev) if (ret) return ret; - priv->chip.dev = dev; - priv->chip.ops = &aspeed_pwm_ops; - priv->chip.npwm = PWM_ASPEED_NR_PWMS; + chip = devm_pwmchip_alloc(dev, PWM_ASPEED_NR_PWMS, 0); + if (IS_ERR(chip)) + return PTR_ERR(chip); - ret = devm_pwmchip_add(dev, &priv->chip); + pwmchip_set_drvdata(chip, priv); + chip->ops = &aspeed_pwm_ops; + + ret = devm_pwmchip_add(dev, chip); if (ret) return dev_err_probe(dev, ret, "Failed to add PWM chip\n"); From 05947224ff469bf17b3791fd009bc27ce5151997 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Wed, 14 Feb 2024 10:33:28 +0100 Subject: [PATCH 03/34] pwm: Ensure that pwm_chips are allocated using pwmchip_alloc() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Memory holding a struct device must not be freed before the reference count drops to zero. So a struct pwm_chip must not live in memory freed by a driver on unbind. All in-tree drivers were fixed accordingly, but as out-of-tree drivers, that were not adapted, still compile fine, catch these in pwmchip_add(). Link: https://lore.kernel.org/r/35f5b229c98f78b2f6ce2397259a4a936be477c0.1707900770.git.u.kleine-koenig@pengutronix.de Signed-off-by: Uwe Kleine-König --- drivers/pwm/core.c | 10 ++++++++++ include/linux/pwm.h | 2 ++ 2 files changed, 12 insertions(+) diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c index 403525cc1783..d3f475eca97f 100644 --- a/drivers/pwm/core.c +++ b/drivers/pwm/core.c @@ -481,6 +481,7 @@ struct pwm_chip *pwmchip_alloc(struct device *parent, unsigned int npwm, size_t chip->dev = parent; chip->npwm = npwm; + chip->uses_pwmchip_alloc = true; pwmchip_set_drvdata(chip, pwmchip_priv(chip)); @@ -561,6 +562,15 @@ int __pwmchip_add(struct pwm_chip *chip, struct module *owner) if (!chip || !pwmchip_parent(chip) || !chip->ops || !chip->npwm) return -EINVAL; + /* + * a struct pwm_chip must be allocated using (devm_)pwmchip_alloc, + * otherwise the embedded struct device might disappear too early + * resulting in memory corruption. + * Catch drivers that were not converted appropriately. + */ + if (!chip->uses_pwmchip_alloc) + return -EINVAL; + if (!pwm_ops_check(chip)) return -EINVAL; diff --git a/include/linux/pwm.h b/include/linux/pwm.h index 4a6568dfdf3f..94a642a88817 100644 --- a/include/linux/pwm.h +++ b/include/linux/pwm.h @@ -272,6 +272,7 @@ struct pwm_ops { * @npwm: number of PWMs controlled by this chip * @of_xlate: request a PWM device given a device tree PWM specifier * @atomic: can the driver's ->apply() be called in atomic context + * @uses_pwmchip_alloc: signals if pwmchip_allow was used to allocate this chip * @driver_data: Private pointer for driver specific info * @pwms: array of PWM devices allocated by the framework */ @@ -287,6 +288,7 @@ struct pwm_chip { bool atomic; /* only used internally by the PWM framework */ + bool uses_pwmchip_alloc; void *driver_data; struct pwm_device *pwms; }; From 4bda9700a55447dfcf33b219de1cf5d7904fdd4f Mon Sep 17 00:00:00 2001 From: Varshini Rajendran Date: Fri, 23 Feb 2024 22:56:19 +0530 Subject: [PATCH 04/34] dt-bindings: pwm: at91: Add sam9x7 compatible strings list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add compatible strings list for SAM9X7. Signed-off-by: Varshini Rajendran Acked-by: Conor Dooley Link: https://lore.kernel.org/r/20240223172619.672262-1-varshini.rajendran@microchip.com Signed-off-by: Uwe Kleine-König --- Documentation/devicetree/bindings/pwm/atmel,at91sam-pwm.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/devicetree/bindings/pwm/atmel,at91sam-pwm.yaml b/Documentation/devicetree/bindings/pwm/atmel,at91sam-pwm.yaml index d84268b59784..96cd6f3c3546 100644 --- a/Documentation/devicetree/bindings/pwm/atmel,at91sam-pwm.yaml +++ b/Documentation/devicetree/bindings/pwm/atmel,at91sam-pwm.yaml @@ -25,6 +25,9 @@ properties: - items: - const: microchip,sama7g5-pwm - const: atmel,sama5d2-pwm + - items: + - const: microchip,sam9x7-pwm + - const: microchip,sam9x60-pwm reg: maxItems: 1 From 5bb0b194aeee5d5da6881232f4e9989b35957c25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Sun, 10 Mar 2024 12:00:54 +0100 Subject: [PATCH 05/34] pwm: sti: Simplify probe function using devm functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of of_clk_get_by_name() use devm_clk_get_prepared() which has several advantages: - Combines getting the clock and a call to clk_prepare(). The latter can be dropped from sti_pwm_probe() accordingly. - Cares for calling clk_put() which is missing in both probe's error path and the remove function. - Cares for calling clk_unprepare() which can be dropped from the error paths and the remove function. (Note that not all error path got this right.) With additionally using devm_pwmchip_add() instead of pwmchip_add() the remove callback can be dropped completely. With it the last user of platform_get_drvdata() goes away and so platform_set_drvdata() can be dropped from the probe function, too. Fixes: 378fe115d19d ("pwm: sti: Add new driver for ST's PWM IP") Link: https://lore.kernel.org/r/81f0e1d173652f435afda6719adaed1922fe059a.1710068192.git.u.kleine-koenig@pengutronix.de Signed-off-by: Uwe Kleine-König --- drivers/pwm/pwm-sti.c | 39 +++------------------------------------ 1 file changed, 3 insertions(+), 36 deletions(-) diff --git a/drivers/pwm/pwm-sti.c b/drivers/pwm/pwm-sti.c index 39d80da0e14a..f07b1126e7a8 100644 --- a/drivers/pwm/pwm-sti.c +++ b/drivers/pwm/pwm-sti.c @@ -624,32 +624,20 @@ static int sti_pwm_probe(struct platform_device *pdev) return ret; if (cdata->pwm_num_devs) { - pc->pwm_clk = of_clk_get_by_name(dev->of_node, "pwm"); + pc->pwm_clk = devm_clk_get_prepared(dev, "pwm"); if (IS_ERR(pc->pwm_clk)) { dev_err(dev, "failed to get PWM clock\n"); return PTR_ERR(pc->pwm_clk); } - - ret = clk_prepare(pc->pwm_clk); - if (ret) { - dev_err(dev, "failed to prepare clock\n"); - return ret; - } } if (cdata->cpt_num_devs) { - pc->cpt_clk = of_clk_get_by_name(dev->of_node, "capture"); + pc->cpt_clk = devm_clk_get_prepared(dev, "capture"); if (IS_ERR(pc->cpt_clk)) { dev_err(dev, "failed to get PWM capture clock\n"); return PTR_ERR(pc->cpt_clk); } - ret = clk_prepare(pc->cpt_clk); - if (ret) { - dev_err(dev, "failed to prepare clock\n"); - return ret; - } - cdata->ddata = devm_kzalloc(dev, cdata->cpt_num_devs * sizeof(*cdata->ddata), GFP_KERNEL); if (!cdata->ddata) return -ENOMEM; @@ -664,27 +652,7 @@ static int sti_pwm_probe(struct platform_device *pdev) mutex_init(&ddata->lock); } - ret = pwmchip_add(chip); - if (ret < 0) { - clk_unprepare(pc->pwm_clk); - clk_unprepare(pc->cpt_clk); - return ret; - } - - platform_set_drvdata(pdev, chip); - - return 0; -} - -static void sti_pwm_remove(struct platform_device *pdev) -{ - struct pwm_chip *chip = platform_get_drvdata(pdev); - struct sti_pwm_chip *pc = to_sti_pwmchip(chip); - - pwmchip_remove(chip); - - clk_unprepare(pc->pwm_clk); - clk_unprepare(pc->cpt_clk); + return devm_pwmchip_add(dev, chip); } static const struct of_device_id sti_pwm_of_match[] = { @@ -699,7 +667,6 @@ static struct platform_driver sti_pwm_driver = { .of_match_table = sti_pwm_of_match, }, .probe = sti_pwm_probe, - .remove_new = sti_pwm_remove, }; module_platform_driver(sti_pwm_driver); From 3025c9c669bac6c193a8f5fc2570f705b675eb40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Sun, 10 Mar 2024 12:00:55 +0100 Subject: [PATCH 06/34] pwm: sti: Improve error reporting using dev_err_probe() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This has the advantage of handling EPROBE_DEFER correctly and being more compact. This change also introduces an error message for a few error paths that lacked an error indicator before. Also sti_pwm_probe_dt() is renamed to sti_pwm_probe_regmap() to better fit what it actually does. Link: https://lore.kernel.org/r/8e540733ab882f2b8873712faf85c4f0cb48133a.1710068192.git.u.kleine-koenig@pengutronix.de Signed-off-by: Uwe Kleine-König --- drivers/pwm/pwm-sti.c | 41 ++++++++++++++++++++--------------------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/drivers/pwm/pwm-sti.c b/drivers/pwm/pwm-sti.c index f07b1126e7a8..919d25023e5a 100644 --- a/drivers/pwm/pwm-sti.c +++ b/drivers/pwm/pwm-sti.c @@ -502,7 +502,7 @@ static irqreturn_t sti_pwm_interrupt(int irq, void *data) return ret; } -static int sti_pwm_probe_dt(struct sti_pwm_chip *pc) +static int sti_pwm_probe_regmap(struct sti_pwm_chip *pc) { struct device *dev = pc->dev; const struct reg_field *reg_fields; @@ -570,10 +570,8 @@ static int sti_pwm_probe(struct platform_device *pdev) if (!ret) cpt_num_devs = num_devs; - if (!pwm_num_devs && !cpt_num_devs) { - dev_err(dev, "No channels configured\n"); - return -EINVAL; - } + if (!pwm_num_devs && !cpt_num_devs) + return dev_err_probe(dev, -EINVAL, "No channels configured\n"); chip = devm_pwmchip_alloc(dev, max(pwm_num_devs, cpt_num_devs), sizeof(*pc)); if (IS_ERR(chip)) @@ -591,7 +589,8 @@ static int sti_pwm_probe(struct platform_device *pdev) pc->regmap = devm_regmap_init_mmio(dev, pc->mmio, &sti_pwm_regmap_config); if (IS_ERR(pc->regmap)) - return PTR_ERR(pc->regmap); + return dev_err_probe(dev, PTR_ERR(pc->regmap), + "Failed to initialize regmap\n"); irq = platform_get_irq(pdev, 0); if (irq < 0) @@ -599,10 +598,8 @@ static int sti_pwm_probe(struct platform_device *pdev) ret = devm_request_irq(&pdev->dev, irq, sti_pwm_interrupt, 0, pdev->name, pc); - if (ret < 0) { - dev_err(&pdev->dev, "Failed to request IRQ\n"); - return ret; - } + if (ret < 0) + dev_err_probe(&pdev->dev, ret, "Failed to request IRQ\n"); /* * Setup PWM data with default values: some values could be replaced @@ -619,24 +616,22 @@ static int sti_pwm_probe(struct platform_device *pdev) pc->en_count = 0; mutex_init(&pc->sti_pwm_lock); - ret = sti_pwm_probe_dt(pc); + ret = sti_pwm_probe_regmap(pc); if (ret) - return ret; + return dev_err_probe(dev, ret, "Failed to initialize regmap fields\n"); if (cdata->pwm_num_devs) { pc->pwm_clk = devm_clk_get_prepared(dev, "pwm"); - if (IS_ERR(pc->pwm_clk)) { - dev_err(dev, "failed to get PWM clock\n"); - return PTR_ERR(pc->pwm_clk); - } + if (IS_ERR(pc->pwm_clk)) + return dev_err_probe(dev, PTR_ERR(pc->pwm_clk), + "failed to get PWM clock\n"); } if (cdata->cpt_num_devs) { pc->cpt_clk = devm_clk_get_prepared(dev, "capture"); - if (IS_ERR(pc->cpt_clk)) { - dev_err(dev, "failed to get PWM capture clock\n"); - return PTR_ERR(pc->cpt_clk); - } + if (IS_ERR(pc->cpt_clk)) + return dev_err_probe(dev, PTR_ERR(pc->cpt_clk), + "failed to get PWM capture clock\n"); cdata->ddata = devm_kzalloc(dev, cdata->cpt_num_devs * sizeof(*cdata->ddata), GFP_KERNEL); if (!cdata->ddata) @@ -652,7 +647,11 @@ static int sti_pwm_probe(struct platform_device *pdev) mutex_init(&ddata->lock); } - return devm_pwmchip_add(dev, chip); + ret = devm_pwmchip_add(dev, chip); + if (ret) + return dev_err_probe(dev, ret, "Failed to register pwm chip\n"); + + return 0; } static const struct of_device_id sti_pwm_of_match[] = { From 354bf751339082161a9911022c45831992026f1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Sun, 10 Mar 2024 12:00:56 +0100 Subject: [PATCH 07/34] pwm: sti: Drop member from driver data that only carries a constant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The .regfield member of struct sti_pwm_compat_data only holds a pointer to the global array sti_pwm_regfields. Replace the few usages by directly using this array and drop the member, saving a bit of memory and a few pointer dereferences. Link: https://lore.kernel.org/r/7ddb76ef49fd84a07713b46c65374cb51f3b4ac0.1710068192.git.u.kleine-koenig@pengutronix.de Signed-off-by: Uwe Kleine-König --- drivers/pwm/pwm-sti.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/drivers/pwm/pwm-sti.c b/drivers/pwm/pwm-sti.c index 919d25023e5a..fa86590a92a6 100644 --- a/drivers/pwm/pwm-sti.c +++ b/drivers/pwm/pwm-sti.c @@ -74,7 +74,6 @@ struct sti_cpt_ddata { }; struct sti_pwm_compat_data { - const struct reg_field *reg_fields; unsigned int pwm_num_devs; unsigned int cpt_num_devs; unsigned int max_pwm_cnt; @@ -505,38 +504,34 @@ static irqreturn_t sti_pwm_interrupt(int irq, void *data) static int sti_pwm_probe_regmap(struct sti_pwm_chip *pc) { struct device *dev = pc->dev; - const struct reg_field *reg_fields; - struct sti_pwm_compat_data *cdata = pc->cdata; - - reg_fields = cdata->reg_fields; pc->prescale_low = devm_regmap_field_alloc(dev, pc->regmap, - reg_fields[PWMCLK_PRESCALE_LOW]); + sti_pwm_regfields[PWMCLK_PRESCALE_LOW]); if (IS_ERR(pc->prescale_low)) return PTR_ERR(pc->prescale_low); pc->prescale_high = devm_regmap_field_alloc(dev, pc->regmap, - reg_fields[PWMCLK_PRESCALE_HIGH]); + sti_pwm_regfields[PWMCLK_PRESCALE_HIGH]); if (IS_ERR(pc->prescale_high)) return PTR_ERR(pc->prescale_high); pc->pwm_out_en = devm_regmap_field_alloc(dev, pc->regmap, - reg_fields[PWM_OUT_EN]); + sti_pwm_regfields[PWM_OUT_EN]); if (IS_ERR(pc->pwm_out_en)) return PTR_ERR(pc->pwm_out_en); pc->pwm_cpt_en = devm_regmap_field_alloc(dev, pc->regmap, - reg_fields[PWM_CPT_EN]); + sti_pwm_regfields[PWM_CPT_EN]); if (IS_ERR(pc->pwm_cpt_en)) return PTR_ERR(pc->pwm_cpt_en); pc->pwm_cpt_int_en = devm_regmap_field_alloc(dev, pc->regmap, - reg_fields[PWM_CPT_INT_EN]); + sti_pwm_regfields[PWM_CPT_INT_EN]); if (IS_ERR(pc->pwm_cpt_int_en)) return PTR_ERR(pc->pwm_cpt_int_en); pc->pwm_cpt_int_stat = devm_regmap_field_alloc(dev, pc->regmap, - reg_fields[PWM_CPT_INT_STAT]); + sti_pwm_regfields[PWM_CPT_INT_STAT]); if (PTR_ERR_OR_ZERO(pc->pwm_cpt_int_stat)) return PTR_ERR(pc->pwm_cpt_int_stat); @@ -605,7 +600,6 @@ static int sti_pwm_probe(struct platform_device *pdev) * Setup PWM data with default values: some values could be replaced * with specific ones provided from Device Tree. */ - cdata->reg_fields = sti_pwm_regfields; cdata->max_prescale = 0xff; cdata->max_pwm_cnt = 255; cdata->pwm_num_devs = pwm_num_devs; From 9e287e0c5fc7b0b926382a6db0f97c3f34a03640 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Sun, 10 Mar 2024 12:00:57 +0100 Subject: [PATCH 08/34] pwm: sti: Maintain all per-chip driver data in a single struct MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of (arbitrarily?) spliting the per-chip driver data into two structures, put everything into struct sti_pwm_chip. This reduces memory management overhead and a few pointer indirections. Link: https://lore.kernel.org/r/2788a421ec838ee8f63e76a78b04e1d48b49f959.1710068192.git.u.kleine-koenig@pengutronix.de Signed-off-by: Uwe Kleine-König --- drivers/pwm/pwm-sti.c | 58 ++++++++++++++++--------------------------- 1 file changed, 22 insertions(+), 36 deletions(-) diff --git a/drivers/pwm/pwm-sti.c b/drivers/pwm/pwm-sti.c index fa86590a92a6..e8fdf96d8cc4 100644 --- a/drivers/pwm/pwm-sti.c +++ b/drivers/pwm/pwm-sti.c @@ -73,20 +73,16 @@ struct sti_cpt_ddata { wait_queue_head_t wait; }; -struct sti_pwm_compat_data { - unsigned int pwm_num_devs; - unsigned int cpt_num_devs; - unsigned int max_pwm_cnt; - unsigned int max_prescale; - struct sti_cpt_ddata *ddata; -}; - struct sti_pwm_chip { struct device *dev; struct clk *pwm_clk; struct clk *cpt_clk; struct regmap *regmap; - struct sti_pwm_compat_data *cdata; + unsigned int pwm_num_devs; + unsigned int cpt_num_devs; + unsigned int max_pwm_cnt; + unsigned int max_prescale; + struct sti_cpt_ddata *ddata; struct regmap_field *prescale_low; struct regmap_field *prescale_high; struct regmap_field *pwm_out_en; @@ -121,7 +117,6 @@ static inline struct sti_pwm_chip *to_sti_pwmchip(struct pwm_chip *chip) static int sti_pwm_get_prescale(struct sti_pwm_chip *pc, unsigned long period, unsigned int *prescale) { - struct sti_pwm_compat_data *cdata = pc->cdata; unsigned long clk_rate; unsigned long value; unsigned int ps; @@ -136,13 +131,13 @@ static int sti_pwm_get_prescale(struct sti_pwm_chip *pc, unsigned long period, * prescale = ((period_ns * clk_rate) / (10^9 * (max_pwm_cnt + 1)) - 1 */ value = NSEC_PER_SEC / clk_rate; - value *= cdata->max_pwm_cnt + 1; + value *= pc->max_pwm_cnt + 1; if (period % value) return -EINVAL; ps = period / value - 1; - if (ps > cdata->max_prescale) + if (ps > pc->max_prescale) return -EINVAL; *prescale = ps; @@ -163,7 +158,6 @@ static int sti_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm, int duty_ns, int period_ns) { struct sti_pwm_chip *pc = to_sti_pwmchip(chip); - struct sti_pwm_compat_data *cdata = pc->cdata; unsigned int ncfg, value, prescale = 0; struct pwm_device *cur = pc->cur; struct device *dev = pc->dev; @@ -223,7 +217,7 @@ static int sti_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm, * PWM pulse = (max_pwm_count + 1) local cycles, * that is continuous pulse: signal never goes low. */ - value = cdata->max_pwm_cnt * duty_ns / period_ns; + value = pc->max_pwm_cnt * duty_ns / period_ns; ret = regmap_write(pc->regmap, PWM_OUT_VAL(pwm->hwpwm), value); if (ret) @@ -312,14 +306,13 @@ static int sti_pwm_capture(struct pwm_chip *chip, struct pwm_device *pwm, struct pwm_capture *result, unsigned long timeout) { struct sti_pwm_chip *pc = to_sti_pwmchip(chip); - struct sti_pwm_compat_data *cdata = pc->cdata; - struct sti_cpt_ddata *ddata = &cdata->ddata[pwm->hwpwm]; + struct sti_cpt_ddata *ddata = &pc->ddata[pwm->hwpwm]; struct device *dev = pc->dev; unsigned int effective_ticks; unsigned long long high, low; int ret; - if (pwm->hwpwm >= cdata->cpt_num_devs) { + if (pwm->hwpwm >= pc->cpt_num_devs) { dev_err(dev, "device %u is not valid\n", pwm->hwpwm); return -EINVAL; } @@ -394,11 +387,10 @@ static int sti_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, const struct pwm_state *state) { struct sti_pwm_chip *pc = to_sti_pwmchip(chip); - struct sti_pwm_compat_data *cdata = pc->cdata; struct device *dev = pc->dev; int err; - if (pwm->hwpwm >= cdata->pwm_num_devs) { + if (pwm->hwpwm >= pc->pwm_num_devs) { dev_err(dev, "device %u is not valid for pwm mode\n", pwm->hwpwm); return -EINVAL; @@ -447,7 +439,7 @@ static irqreturn_t sti_pwm_interrupt(int irq, void *data) while (cpt_int_stat) { devicenum = ffs(cpt_int_stat) - 1; - ddata = &pc->cdata->ddata[devicenum]; + ddata = &pc->ddata[devicenum]; /* * Capture input: @@ -551,7 +543,6 @@ static int sti_pwm_probe(struct platform_device *pdev) u32 num_devs; unsigned int pwm_num_devs = 0; unsigned int cpt_num_devs = 0; - struct sti_pwm_compat_data *cdata; struct pwm_chip *chip; struct sti_pwm_chip *pc; unsigned int i; @@ -573,10 +564,6 @@ static int sti_pwm_probe(struct platform_device *pdev) return PTR_ERR(chip); pc = to_sti_pwmchip(chip); - cdata = devm_kzalloc(dev, sizeof(*cdata), GFP_KERNEL); - if (!cdata) - return -ENOMEM; - pc->mmio = devm_platform_ioremap_resource(pdev, 0); if (IS_ERR(pc->mmio)) return PTR_ERR(pc->mmio); @@ -600,12 +587,11 @@ static int sti_pwm_probe(struct platform_device *pdev) * Setup PWM data with default values: some values could be replaced * with specific ones provided from Device Tree. */ - cdata->max_prescale = 0xff; - cdata->max_pwm_cnt = 255; - cdata->pwm_num_devs = pwm_num_devs; - cdata->cpt_num_devs = cpt_num_devs; + pc->max_prescale = 0xff; + pc->max_pwm_cnt = 255; + pc->pwm_num_devs = pwm_num_devs; + pc->cpt_num_devs = cpt_num_devs; - pc->cdata = cdata; pc->dev = dev; pc->en_count = 0; mutex_init(&pc->sti_pwm_lock); @@ -614,28 +600,28 @@ static int sti_pwm_probe(struct platform_device *pdev) if (ret) return dev_err_probe(dev, ret, "Failed to initialize regmap fields\n"); - if (cdata->pwm_num_devs) { + if (pc->pwm_num_devs) { pc->pwm_clk = devm_clk_get_prepared(dev, "pwm"); if (IS_ERR(pc->pwm_clk)) return dev_err_probe(dev, PTR_ERR(pc->pwm_clk), "failed to get PWM clock\n"); } - if (cdata->cpt_num_devs) { + if (pc->cpt_num_devs) { pc->cpt_clk = devm_clk_get_prepared(dev, "capture"); if (IS_ERR(pc->cpt_clk)) return dev_err_probe(dev, PTR_ERR(pc->cpt_clk), "failed to get PWM capture clock\n"); - cdata->ddata = devm_kzalloc(dev, cdata->cpt_num_devs * sizeof(*cdata->ddata), GFP_KERNEL); - if (!cdata->ddata) + pc->ddata = devm_kzalloc(dev, pc->cpt_num_devs * sizeof(*pc->ddata), GFP_KERNEL); + if (!pc->ddata) return -ENOMEM; } chip->ops = &sti_pwm_ops; - for (i = 0; i < cdata->cpt_num_devs; i++) { - struct sti_cpt_ddata *ddata = &cdata->ddata[i]; + for (i = 0; i < pc->cpt_num_devs; i++) { + struct sti_cpt_ddata *ddata = &pc->ddata[i]; init_waitqueue_head(&ddata->wait); mutex_init(&ddata->lock); From c0143f68919e6e36a4fa8816ddb49d266f3b21de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Sun, 10 Mar 2024 12:00:58 +0100 Subject: [PATCH 09/34] pwm: sti: Use devm_kcalloc() instead of calculating the size for devm_kzalloc() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Using a multiplication to calculate the size of an allocation isn't recommended in case the mulitplication overflows. While the chance this happens is low, preventing such an error is easy enough; so do that. Link: https://lore.kernel.org/r/17062aef42e6677629a056e25c6916d8b6eaedeb.1710068192.git.u.kleine-koenig@pengutronix.de Signed-off-by: Uwe Kleine-König --- drivers/pwm/pwm-sti.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/pwm/pwm-sti.c b/drivers/pwm/pwm-sti.c index e8fdf96d8cc4..7a7d1c622a17 100644 --- a/drivers/pwm/pwm-sti.c +++ b/drivers/pwm/pwm-sti.c @@ -613,7 +613,8 @@ static int sti_pwm_probe(struct platform_device *pdev) return dev_err_probe(dev, PTR_ERR(pc->cpt_clk), "failed to get PWM capture clock\n"); - pc->ddata = devm_kzalloc(dev, pc->cpt_num_devs * sizeof(*pc->ddata), GFP_KERNEL); + pc->ddata = devm_kcalloc(dev, pc->cpt_num_devs, + sizeof(*pc->ddata), GFP_KERNEL); if (!pc->ddata) return -ENOMEM; } From 7db42aa2b629de0a103f603f41c5b0929c66ccda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Sun, 10 Mar 2024 12:00:59 +0100 Subject: [PATCH 10/34] pwm: sti: Prefer local variable over pointer dereference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While the compiler probably optimizes out the pointer dereference, using the local variable holding the same value also benefits the human reader. So simplify accordingly. Also move a loop over all capture lines into the capture if block to group the operations related to capturing in .probe(). Link: https://lore.kernel.org/r/a7a81f3838f7ed7f4d6dbee3d646989cc265f676.1710068192.git.u.kleine-koenig@pengutronix.de Signed-off-by: Uwe Kleine-König --- drivers/pwm/pwm-sti.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/pwm/pwm-sti.c b/drivers/pwm/pwm-sti.c index 7a7d1c622a17..396b52672ce0 100644 --- a/drivers/pwm/pwm-sti.c +++ b/drivers/pwm/pwm-sti.c @@ -600,34 +600,34 @@ static int sti_pwm_probe(struct platform_device *pdev) if (ret) return dev_err_probe(dev, ret, "Failed to initialize regmap fields\n"); - if (pc->pwm_num_devs) { + if (pwm_num_devs) { pc->pwm_clk = devm_clk_get_prepared(dev, "pwm"); if (IS_ERR(pc->pwm_clk)) return dev_err_probe(dev, PTR_ERR(pc->pwm_clk), "failed to get PWM clock\n"); } - if (pc->cpt_num_devs) { + if (cpt_num_devs) { pc->cpt_clk = devm_clk_get_prepared(dev, "capture"); if (IS_ERR(pc->cpt_clk)) return dev_err_probe(dev, PTR_ERR(pc->cpt_clk), "failed to get PWM capture clock\n"); - pc->ddata = devm_kcalloc(dev, pc->cpt_num_devs, + pc->ddata = devm_kcalloc(dev, cpt_num_devs, sizeof(*pc->ddata), GFP_KERNEL); if (!pc->ddata) return -ENOMEM; + + for (i = 0; i < cpt_num_devs; i++) { + struct sti_cpt_ddata *ddata = &pc->ddata[i]; + + init_waitqueue_head(&ddata->wait); + mutex_init(&ddata->lock); + } } chip->ops = &sti_pwm_ops; - for (i = 0; i < pc->cpt_num_devs; i++) { - struct sti_cpt_ddata *ddata = &pc->ddata[i]; - - init_waitqueue_head(&ddata->wait); - mutex_init(&ddata->lock); - } - ret = devm_pwmchip_add(dev, chip); if (ret) return dev_err_probe(dev, ret, "Failed to register pwm chip\n"); From b40ac0e176bf2c83c78cf72fd64a42be2f9b9638 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Sun, 17 Mar 2024 11:40:33 +0100 Subject: [PATCH 11/34] pwm: Give some sysfs related variables and functions better names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The code handling the sysfs API uses "child" and "parent" to refer to the devices corresponding to a struct pwm or a struct pwm_chip respectively. Other parts of the pwm core use "parent" to refer to the parent device of a struct pwm_chip. So rename "child" to "pwm_dev" and "parent" to "pwmchip_dev" which better explains the semantic. Also two functions are changed to match the new names: child_to_pwm_export() -> pwmexport_from_dev() child_to_pwm_device() -> pwm_from_dev() (which have the additional advantage to start with "pwm" which gives the right scope). Additionally introduce a wrapper for dev_get_drvdata() to convert a pwmchip_dev to the respective pwm_chip. Link: https://lore.kernel.org/r/9cc05aceeae2f06ecb850bccb15ba821e768c183.1710670958.git.u.kleine-koenig@pengutronix.de Signed-off-by: Uwe Kleine-König --- drivers/pwm/sysfs.c | 161 +++++++++++++++++++++++--------------------- 1 file changed, 83 insertions(+), 78 deletions(-) diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c index 3f434a771fb5..1a1b1f6dd98f 100644 --- a/drivers/pwm/sysfs.c +++ b/drivers/pwm/sysfs.c @@ -15,29 +15,34 @@ #include struct pwm_export { - struct device child; + struct device pwm_dev; struct pwm_device *pwm; struct mutex lock; struct pwm_state suspend; }; -static struct pwm_export *child_to_pwm_export(struct device *child) +static inline struct pwm_chip *pwmchip_from_dev(struct device *pwmchip_dev) { - return container_of(child, struct pwm_export, child); + return dev_get_drvdata(pwmchip_dev); } -static struct pwm_device *child_to_pwm_device(struct device *child) +static inline struct pwm_export *pwmexport_from_dev(struct device *pwm_dev) { - struct pwm_export *export = child_to_pwm_export(child); + return container_of(pwm_dev, struct pwm_export, pwm_dev); +} + +static inline struct pwm_device *pwm_from_dev(struct device *pwm_dev) +{ + struct pwm_export *export = pwmexport_from_dev(pwm_dev); return export->pwm; } -static ssize_t period_show(struct device *child, +static ssize_t period_show(struct device *pwm_dev, struct device_attribute *attr, char *buf) { - const struct pwm_device *pwm = child_to_pwm_device(child); + const struct pwm_device *pwm = pwm_from_dev(pwm_dev); struct pwm_state state; pwm_get_state(pwm, &state); @@ -45,11 +50,11 @@ static ssize_t period_show(struct device *child, return sysfs_emit(buf, "%llu\n", state.period); } -static ssize_t period_store(struct device *child, +static ssize_t period_store(struct device *pwm_dev, struct device_attribute *attr, const char *buf, size_t size) { - struct pwm_export *export = child_to_pwm_export(child); + struct pwm_export *export = pwmexport_from_dev(pwm_dev); struct pwm_device *pwm = export->pwm; struct pwm_state state; u64 val; @@ -68,11 +73,11 @@ static ssize_t period_store(struct device *child, return ret ? : size; } -static ssize_t duty_cycle_show(struct device *child, +static ssize_t duty_cycle_show(struct device *pwm_dev, struct device_attribute *attr, char *buf) { - const struct pwm_device *pwm = child_to_pwm_device(child); + const struct pwm_device *pwm = pwm_from_dev(pwm_dev); struct pwm_state state; pwm_get_state(pwm, &state); @@ -80,11 +85,11 @@ static ssize_t duty_cycle_show(struct device *child, return sysfs_emit(buf, "%llu\n", state.duty_cycle); } -static ssize_t duty_cycle_store(struct device *child, +static ssize_t duty_cycle_store(struct device *pwm_dev, struct device_attribute *attr, const char *buf, size_t size) { - struct pwm_export *export = child_to_pwm_export(child); + struct pwm_export *export = pwmexport_from_dev(pwm_dev); struct pwm_device *pwm = export->pwm; struct pwm_state state; u64 val; @@ -103,11 +108,11 @@ static ssize_t duty_cycle_store(struct device *child, return ret ? : size; } -static ssize_t enable_show(struct device *child, +static ssize_t enable_show(struct device *pwm_dev, struct device_attribute *attr, char *buf) { - const struct pwm_device *pwm = child_to_pwm_device(child); + const struct pwm_device *pwm = pwm_from_dev(pwm_dev); struct pwm_state state; pwm_get_state(pwm, &state); @@ -115,11 +120,11 @@ static ssize_t enable_show(struct device *child, return sysfs_emit(buf, "%d\n", state.enabled); } -static ssize_t enable_store(struct device *child, +static ssize_t enable_store(struct device *pwm_dev, struct device_attribute *attr, const char *buf, size_t size) { - struct pwm_export *export = child_to_pwm_export(child); + struct pwm_export *export = pwmexport_from_dev(pwm_dev); struct pwm_device *pwm = export->pwm; struct pwm_state state; int val, ret; @@ -151,11 +156,11 @@ static ssize_t enable_store(struct device *child, return ret ? : size; } -static ssize_t polarity_show(struct device *child, +static ssize_t polarity_show(struct device *pwm_dev, struct device_attribute *attr, char *buf) { - const struct pwm_device *pwm = child_to_pwm_device(child); + const struct pwm_device *pwm = pwm_from_dev(pwm_dev); const char *polarity = "unknown"; struct pwm_state state; @@ -174,11 +179,11 @@ static ssize_t polarity_show(struct device *child, return sysfs_emit(buf, "%s\n", polarity); } -static ssize_t polarity_store(struct device *child, +static ssize_t polarity_store(struct device *pwm_dev, struct device_attribute *attr, const char *buf, size_t size) { - struct pwm_export *export = child_to_pwm_export(child); + struct pwm_export *export = pwmexport_from_dev(pwm_dev); struct pwm_device *pwm = export->pwm; enum pwm_polarity polarity; struct pwm_state state; @@ -200,11 +205,11 @@ static ssize_t polarity_store(struct device *child, return ret ? : size; } -static ssize_t capture_show(struct device *child, +static ssize_t capture_show(struct device *pwm_dev, struct device_attribute *attr, char *buf) { - struct pwm_device *pwm = child_to_pwm_device(child); + struct pwm_device *pwm = pwm_from_dev(pwm_dev); struct pwm_capture result; int ret; @@ -231,14 +236,14 @@ static struct attribute *pwm_attrs[] = { }; ATTRIBUTE_GROUPS(pwm); -static void pwm_export_release(struct device *child) +static void pwm_export_release(struct device *pwm_dev) { - struct pwm_export *export = child_to_pwm_export(child); + struct pwm_export *export = pwmexport_from_dev(pwm_dev); kfree(export); } -static int pwm_export_child(struct device *parent, struct pwm_device *pwm) +static int pwm_export_child(struct device *pwmchip_dev, struct pwm_device *pwm) { struct pwm_export *export; char *pwm_prop[2]; @@ -256,62 +261,62 @@ static int pwm_export_child(struct device *parent, struct pwm_device *pwm) export->pwm = pwm; mutex_init(&export->lock); - export->child.release = pwm_export_release; - export->child.parent = parent; - export->child.devt = MKDEV(0, 0); - export->child.groups = pwm_groups; - dev_set_name(&export->child, "pwm%u", pwm->hwpwm); + export->pwm_dev.release = pwm_export_release; + export->pwm_dev.parent = pwmchip_dev; + export->pwm_dev.devt = MKDEV(0, 0); + export->pwm_dev.groups = pwm_groups; + dev_set_name(&export->pwm_dev, "pwm%u", pwm->hwpwm); - ret = device_register(&export->child); + ret = device_register(&export->pwm_dev); if (ret) { clear_bit(PWMF_EXPORTED, &pwm->flags); - put_device(&export->child); + put_device(&export->pwm_dev); export = NULL; return ret; } pwm_prop[0] = kasprintf(GFP_KERNEL, "EXPORT=pwm%u", pwm->hwpwm); pwm_prop[1] = NULL; - kobject_uevent_env(&parent->kobj, KOBJ_CHANGE, pwm_prop); + kobject_uevent_env(&pwmchip_dev->kobj, KOBJ_CHANGE, pwm_prop); kfree(pwm_prop[0]); return 0; } -static int pwm_unexport_match(struct device *child, void *data) +static int pwm_unexport_match(struct device *pwm_dev, void *data) { - return child_to_pwm_device(child) == data; + return pwm_from_dev(pwm_dev) == data; } -static int pwm_unexport_child(struct device *parent, struct pwm_device *pwm) +static int pwm_unexport_child(struct device *pwmchip_dev, struct pwm_device *pwm) { - struct device *child; + struct device *pwm_dev; char *pwm_prop[2]; if (!test_and_clear_bit(PWMF_EXPORTED, &pwm->flags)) return -ENODEV; - child = device_find_child(parent, pwm, pwm_unexport_match); - if (!child) + pwm_dev = device_find_child(pwmchip_dev, pwm, pwm_unexport_match); + if (!pwm_dev) return -ENODEV; pwm_prop[0] = kasprintf(GFP_KERNEL, "UNEXPORT=pwm%u", pwm->hwpwm); pwm_prop[1] = NULL; - kobject_uevent_env(&parent->kobj, KOBJ_CHANGE, pwm_prop); + kobject_uevent_env(&pwmchip_dev->kobj, KOBJ_CHANGE, pwm_prop); kfree(pwm_prop[0]); /* for device_find_child() */ - put_device(child); - device_unregister(child); + put_device(pwm_dev); + device_unregister(pwm_dev); pwm_put(pwm); return 0; } -static ssize_t export_store(struct device *parent, +static ssize_t export_store(struct device *pwmchip_dev, struct device_attribute *attr, const char *buf, size_t len) { - struct pwm_chip *chip = dev_get_drvdata(parent); + struct pwm_chip *chip = pwmchip_from_dev(pwmchip_dev); struct pwm_device *pwm; unsigned int hwpwm; int ret; @@ -327,7 +332,7 @@ static ssize_t export_store(struct device *parent, if (IS_ERR(pwm)) return PTR_ERR(pwm); - ret = pwm_export_child(parent, pwm); + ret = pwm_export_child(pwmchip_dev, pwm); if (ret < 0) pwm_put(pwm); @@ -335,11 +340,11 @@ static ssize_t export_store(struct device *parent, } static DEVICE_ATTR_WO(export); -static ssize_t unexport_store(struct device *parent, +static ssize_t unexport_store(struct device *pwmchip_dev, struct device_attribute *attr, const char *buf, size_t len) { - struct pwm_chip *chip = dev_get_drvdata(parent); + struct pwm_chip *chip = pwmchip_from_dev(pwmchip_dev); unsigned int hwpwm; int ret; @@ -350,16 +355,16 @@ static ssize_t unexport_store(struct device *parent, if (hwpwm >= chip->npwm) return -ENODEV; - ret = pwm_unexport_child(parent, &chip->pwms[hwpwm]); + ret = pwm_unexport_child(pwmchip_dev, &chip->pwms[hwpwm]); return ret ? : len; } static DEVICE_ATTR_WO(unexport); -static ssize_t npwm_show(struct device *parent, struct device_attribute *attr, +static ssize_t npwm_show(struct device *pwmchip_dev, struct device_attribute *attr, char *buf) { - const struct pwm_chip *chip = dev_get_drvdata(parent); + const struct pwm_chip *chip = pwmchip_from_dev(pwmchip_dev); return sysfs_emit(buf, "%u\n", chip->npwm); } @@ -374,22 +379,22 @@ static struct attribute *pwm_chip_attrs[] = { ATTRIBUTE_GROUPS(pwm_chip); /* takes export->lock on success */ -static struct pwm_export *pwm_class_get_state(struct device *parent, +static struct pwm_export *pwm_class_get_state(struct device *pwmchip_dev, struct pwm_device *pwm, struct pwm_state *state) { - struct device *child; + struct device *pwm_dev; struct pwm_export *export; if (!test_bit(PWMF_EXPORTED, &pwm->flags)) return NULL; - child = device_find_child(parent, pwm, pwm_unexport_match); - if (!child) + pwm_dev = device_find_child(pwmchip_dev, pwm, pwm_unexport_match); + if (!pwm_dev) return NULL; - export = child_to_pwm_export(child); - put_device(child); /* for device_find_child() */ + export = pwmexport_from_dev(pwm_dev); + put_device(pwm_dev); /* for device_find_child() */ mutex_lock(&export->lock); pwm_get_state(pwm, state); @@ -409,9 +414,9 @@ static int pwm_class_apply_state(struct pwm_export *export, return ret; } -static int pwm_class_resume_npwm(struct device *parent, unsigned int npwm) +static int pwm_class_resume_npwm(struct device *pwmchip_dev, unsigned int npwm) { - struct pwm_chip *chip = dev_get_drvdata(parent); + struct pwm_chip *chip = pwmchip_from_dev(pwmchip_dev); unsigned int i; int ret = 0; @@ -420,7 +425,7 @@ static int pwm_class_resume_npwm(struct device *parent, unsigned int npwm) struct pwm_state state; struct pwm_export *export; - export = pwm_class_get_state(parent, pwm, &state); + export = pwm_class_get_state(pwmchip_dev, pwm, &state); if (!export) continue; @@ -440,9 +445,9 @@ static int pwm_class_resume_npwm(struct device *parent, unsigned int npwm) return ret; } -static int pwm_class_suspend(struct device *parent) +static int pwm_class_suspend(struct device *pwmchip_dev) { - struct pwm_chip *chip = dev_get_drvdata(parent); + struct pwm_chip *chip = pwmchip_from_dev(pwmchip_dev); unsigned int i; int ret = 0; @@ -451,7 +456,7 @@ static int pwm_class_suspend(struct device *parent) struct pwm_state state; struct pwm_export *export; - export = pwm_class_get_state(parent, pwm, &state); + export = pwm_class_get_state(pwmchip_dev, pwm, &state); if (!export) continue; @@ -473,7 +478,7 @@ static int pwm_class_suspend(struct device *parent) * roll back the PWM devices that were disabled by * this suspend function. */ - pwm_class_resume_npwm(parent, i); + pwm_class_resume_npwm(pwmchip_dev, i); break; } } @@ -481,11 +486,11 @@ static int pwm_class_suspend(struct device *parent) return ret; } -static int pwm_class_resume(struct device *parent) +static int pwm_class_resume(struct device *pwmchip_dev) { - struct pwm_chip *chip = dev_get_drvdata(parent); + struct pwm_chip *chip = pwmchip_from_dev(pwmchip_dev); - return pwm_class_resume_npwm(parent, chip->npwm); + return pwm_class_resume_npwm(pwmchip_dev, chip->npwm); } static DEFINE_SIMPLE_DEV_PM_OPS(pwm_class_pm_ops, pwm_class_suspend, pwm_class_resume); @@ -496,22 +501,22 @@ static struct class pwm_class = { .pm = pm_sleep_ptr(&pwm_class_pm_ops), }; -static int pwmchip_sysfs_match(struct device *parent, const void *data) +static int pwmchip_sysfs_match(struct device *pwmchip_dev, const void *data) { - return dev_get_drvdata(parent) == data; + return pwmchip_from_dev(pwmchip_dev) == data; } void pwmchip_sysfs_export(struct pwm_chip *chip) { - struct device *parent; + struct device *pwmchip_dev; /* * If device_create() fails the pwm_chip is still usable by * the kernel it's just not exported. */ - parent = device_create(&pwm_class, pwmchip_parent(chip), MKDEV(0, 0), chip, + pwmchip_dev = device_create(&pwm_class, pwmchip_parent(chip), MKDEV(0, 0), chip, "pwmchip%d", chip->id); - if (IS_ERR(parent)) { + if (IS_ERR(pwmchip_dev)) { dev_warn(pwmchip_parent(chip), "device_create failed for pwm_chip sysfs export\n"); } @@ -519,23 +524,23 @@ void pwmchip_sysfs_export(struct pwm_chip *chip) void pwmchip_sysfs_unexport(struct pwm_chip *chip) { - struct device *parent; + struct device *pwmchip_dev; unsigned int i; - parent = class_find_device(&pwm_class, NULL, chip, + pwmchip_dev = class_find_device(&pwm_class, NULL, chip, pwmchip_sysfs_match); - if (!parent) + if (!pwmchip_dev) return; for (i = 0; i < chip->npwm; i++) { struct pwm_device *pwm = &chip->pwms[i]; if (test_bit(PWMF_EXPORTED, &pwm->flags)) - pwm_unexport_child(parent, pwm); + pwm_unexport_child(pwmchip_dev, pwm); } - put_device(parent); - device_unregister(parent); + put_device(pwmchip_dev); + device_unregister(pwmchip_dev); } static int __init pwm_sysfs_init(void) From e9cc807f87ffd1ccc919731e8f624982935af3e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Sun, 17 Mar 2024 11:40:34 +0100 Subject: [PATCH 12/34] pwm: Move contents of sysfs.c into core.c MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With the upcoming restructuring having all in a single file simplifies things a bit. The relevant and somewhat visible changes are: - Some dropped prototypes from include/linux/pwm.h that were only necessary that core.c has a declaration of the symbols defined in sysfs.c. The respective functions are static now. - The pwm class now also exists if CONFIG_SYSFS isn't enabled. Having CONFIG_SYSFS is not very relevant today, but even without it the class and device stuff still provides lifetime tracking. - Both files had an initcall, these are merged into a single one now. Instead of a big #ifdef block for CONFIG_DEBUG_FS, a single if (IS_ENABLED(CONFIG_DEBUG_FS)) is used now. This increases compile coverage a bit and is a tad nicer on the eyes. Link: https://lore.kernel.org/r/9e2d39a5280d7dda5bfc6682a8aef510148635b2.1710670958.git.u.kleine-koenig@pengutronix.de Signed-off-by: Uwe Kleine-König --- drivers/pwm/Kconfig | 4 - drivers/pwm/Makefile | 1 - drivers/pwm/core.c | 540 +++++++++++++++++++++++++++++++++++++++++- drivers/pwm/sysfs.c | 550 ------------------------------------------- include/linux/pwm.h | 13 - 5 files changed, 534 insertions(+), 574 deletions(-) delete mode 100644 drivers/pwm/sysfs.c diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig index 4b956d661755..1dd7921194f5 100644 --- a/drivers/pwm/Kconfig +++ b/drivers/pwm/Kconfig @@ -29,10 +29,6 @@ menuconfig PWM if PWM -config PWM_SYSFS - bool - default y if SYSFS - config PWM_DEBUG bool "PWM lowlevel drivers additional checks and debug messages" depends on DEBUG_KERNEL diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile index c5ec9e168ee7..90913519f11a 100644 --- a/drivers/pwm/Makefile +++ b/drivers/pwm/Makefile @@ -1,6 +1,5 @@ # SPDX-License-Identifier: GPL-2.0 obj-$(CONFIG_PWM) += core.o -obj-$(CONFIG_PWM_SYSFS) += sysfs.o obj-$(CONFIG_PWM_AB8500) += pwm-ab8500.o obj-$(CONFIG_PWM_APPLE) += pwm-apple.o obj-$(CONFIG_PWM_ATMEL) += pwm-atmel.o diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c index d3f475eca97f..faa0f885e057 100644 --- a/drivers/pwm/core.c +++ b/drivers/pwm/core.c @@ -454,6 +454,535 @@ of_pwm_single_xlate(struct pwm_chip *chip, const struct of_phandle_args *args) } EXPORT_SYMBOL_GPL(of_pwm_single_xlate); +struct pwm_export { + struct device pwm_dev; + struct pwm_device *pwm; + struct mutex lock; + struct pwm_state suspend; +}; + +static inline struct pwm_chip *pwmchip_from_dev(struct device *pwmchip_dev) +{ + return dev_get_drvdata(pwmchip_dev); +} + +static inline struct pwm_export *pwmexport_from_dev(struct device *pwm_dev) +{ + return container_of(pwm_dev, struct pwm_export, pwm_dev); +} + +static inline struct pwm_device *pwm_from_dev(struct device *pwm_dev) +{ + struct pwm_export *export = pwmexport_from_dev(pwm_dev); + + return export->pwm; +} + +static ssize_t period_show(struct device *pwm_dev, + struct device_attribute *attr, + char *buf) +{ + const struct pwm_device *pwm = pwm_from_dev(pwm_dev); + struct pwm_state state; + + pwm_get_state(pwm, &state); + + return sysfs_emit(buf, "%llu\n", state.period); +} + +static ssize_t period_store(struct device *pwm_dev, + struct device_attribute *attr, + const char *buf, size_t size) +{ + struct pwm_export *export = pwmexport_from_dev(pwm_dev); + struct pwm_device *pwm = export->pwm; + struct pwm_state state; + u64 val; + int ret; + + ret = kstrtou64(buf, 0, &val); + if (ret) + return ret; + + mutex_lock(&export->lock); + pwm_get_state(pwm, &state); + state.period = val; + ret = pwm_apply_might_sleep(pwm, &state); + mutex_unlock(&export->lock); + + return ret ? : size; +} + +static ssize_t duty_cycle_show(struct device *pwm_dev, + struct device_attribute *attr, + char *buf) +{ + const struct pwm_device *pwm = pwm_from_dev(pwm_dev); + struct pwm_state state; + + pwm_get_state(pwm, &state); + + return sysfs_emit(buf, "%llu\n", state.duty_cycle); +} + +static ssize_t duty_cycle_store(struct device *pwm_dev, + struct device_attribute *attr, + const char *buf, size_t size) +{ + struct pwm_export *export = pwmexport_from_dev(pwm_dev); + struct pwm_device *pwm = export->pwm; + struct pwm_state state; + u64 val; + int ret; + + ret = kstrtou64(buf, 0, &val); + if (ret) + return ret; + + mutex_lock(&export->lock); + pwm_get_state(pwm, &state); + state.duty_cycle = val; + ret = pwm_apply_might_sleep(pwm, &state); + mutex_unlock(&export->lock); + + return ret ? : size; +} + +static ssize_t enable_show(struct device *pwm_dev, + struct device_attribute *attr, + char *buf) +{ + const struct pwm_device *pwm = pwm_from_dev(pwm_dev); + struct pwm_state state; + + pwm_get_state(pwm, &state); + + return sysfs_emit(buf, "%d\n", state.enabled); +} + +static ssize_t enable_store(struct device *pwm_dev, + struct device_attribute *attr, + const char *buf, size_t size) +{ + struct pwm_export *export = pwmexport_from_dev(pwm_dev); + struct pwm_device *pwm = export->pwm; + struct pwm_state state; + int val, ret; + + ret = kstrtoint(buf, 0, &val); + if (ret) + return ret; + + mutex_lock(&export->lock); + + pwm_get_state(pwm, &state); + + switch (val) { + case 0: + state.enabled = false; + break; + case 1: + state.enabled = true; + break; + default: + ret = -EINVAL; + goto unlock; + } + + ret = pwm_apply_might_sleep(pwm, &state); + +unlock: + mutex_unlock(&export->lock); + return ret ? : size; +} + +static ssize_t polarity_show(struct device *pwm_dev, + struct device_attribute *attr, + char *buf) +{ + const struct pwm_device *pwm = pwm_from_dev(pwm_dev); + const char *polarity = "unknown"; + struct pwm_state state; + + pwm_get_state(pwm, &state); + + switch (state.polarity) { + case PWM_POLARITY_NORMAL: + polarity = "normal"; + break; + + case PWM_POLARITY_INVERSED: + polarity = "inversed"; + break; + } + + return sysfs_emit(buf, "%s\n", polarity); +} + +static ssize_t polarity_store(struct device *pwm_dev, + struct device_attribute *attr, + const char *buf, size_t size) +{ + struct pwm_export *export = pwmexport_from_dev(pwm_dev); + struct pwm_device *pwm = export->pwm; + enum pwm_polarity polarity; + struct pwm_state state; + int ret; + + if (sysfs_streq(buf, "normal")) + polarity = PWM_POLARITY_NORMAL; + else if (sysfs_streq(buf, "inversed")) + polarity = PWM_POLARITY_INVERSED; + else + return -EINVAL; + + mutex_lock(&export->lock); + pwm_get_state(pwm, &state); + state.polarity = polarity; + ret = pwm_apply_might_sleep(pwm, &state); + mutex_unlock(&export->lock); + + return ret ? : size; +} + +static ssize_t capture_show(struct device *pwm_dev, + struct device_attribute *attr, + char *buf) +{ + struct pwm_device *pwm = pwm_from_dev(pwm_dev); + struct pwm_capture result; + int ret; + + ret = pwm_capture(pwm, &result, jiffies_to_msecs(HZ)); + if (ret) + return ret; + + return sysfs_emit(buf, "%u %u\n", result.period, result.duty_cycle); +} + +static DEVICE_ATTR_RW(period); +static DEVICE_ATTR_RW(duty_cycle); +static DEVICE_ATTR_RW(enable); +static DEVICE_ATTR_RW(polarity); +static DEVICE_ATTR_RO(capture); + +static struct attribute *pwm_attrs[] = { + &dev_attr_period.attr, + &dev_attr_duty_cycle.attr, + &dev_attr_enable.attr, + &dev_attr_polarity.attr, + &dev_attr_capture.attr, + NULL +}; +ATTRIBUTE_GROUPS(pwm); + +static void pwm_export_release(struct device *pwm_dev) +{ + struct pwm_export *export = pwmexport_from_dev(pwm_dev); + + kfree(export); +} + +static int pwm_export_child(struct device *pwmchip_dev, struct pwm_device *pwm) +{ + struct pwm_export *export; + char *pwm_prop[2]; + int ret; + + if (test_and_set_bit(PWMF_EXPORTED, &pwm->flags)) + return -EBUSY; + + export = kzalloc(sizeof(*export), GFP_KERNEL); + if (!export) { + clear_bit(PWMF_EXPORTED, &pwm->flags); + return -ENOMEM; + } + + export->pwm = pwm; + mutex_init(&export->lock); + + export->pwm_dev.release = pwm_export_release; + export->pwm_dev.parent = pwmchip_dev; + export->pwm_dev.devt = MKDEV(0, 0); + export->pwm_dev.groups = pwm_groups; + dev_set_name(&export->pwm_dev, "pwm%u", pwm->hwpwm); + + ret = device_register(&export->pwm_dev); + if (ret) { + clear_bit(PWMF_EXPORTED, &pwm->flags); + put_device(&export->pwm_dev); + export = NULL; + return ret; + } + pwm_prop[0] = kasprintf(GFP_KERNEL, "EXPORT=pwm%u", pwm->hwpwm); + pwm_prop[1] = NULL; + kobject_uevent_env(&pwmchip_dev->kobj, KOBJ_CHANGE, pwm_prop); + kfree(pwm_prop[0]); + + return 0; +} + +static int pwm_unexport_match(struct device *pwm_dev, void *data) +{ + return pwm_from_dev(pwm_dev) == data; +} + +static int pwm_unexport_child(struct device *pwmchip_dev, struct pwm_device *pwm) +{ + struct device *pwm_dev; + char *pwm_prop[2]; + + if (!test_and_clear_bit(PWMF_EXPORTED, &pwm->flags)) + return -ENODEV; + + pwm_dev = device_find_child(pwmchip_dev, pwm, pwm_unexport_match); + if (!pwm_dev) + return -ENODEV; + + pwm_prop[0] = kasprintf(GFP_KERNEL, "UNEXPORT=pwm%u", pwm->hwpwm); + pwm_prop[1] = NULL; + kobject_uevent_env(&pwmchip_dev->kobj, KOBJ_CHANGE, pwm_prop); + kfree(pwm_prop[0]); + + /* for device_find_child() */ + put_device(pwm_dev); + device_unregister(pwm_dev); + pwm_put(pwm); + + return 0; +} + +static ssize_t export_store(struct device *pwmchip_dev, + struct device_attribute *attr, + const char *buf, size_t len) +{ + struct pwm_chip *chip = pwmchip_from_dev(pwmchip_dev); + struct pwm_device *pwm; + unsigned int hwpwm; + int ret; + + ret = kstrtouint(buf, 0, &hwpwm); + if (ret < 0) + return ret; + + if (hwpwm >= chip->npwm) + return -ENODEV; + + pwm = pwm_request_from_chip(chip, hwpwm, "sysfs"); + if (IS_ERR(pwm)) + return PTR_ERR(pwm); + + ret = pwm_export_child(pwmchip_dev, pwm); + if (ret < 0) + pwm_put(pwm); + + return ret ? : len; +} +static DEVICE_ATTR_WO(export); + +static ssize_t unexport_store(struct device *pwmchip_dev, + struct device_attribute *attr, + const char *buf, size_t len) +{ + struct pwm_chip *chip = pwmchip_from_dev(pwmchip_dev); + unsigned int hwpwm; + int ret; + + ret = kstrtouint(buf, 0, &hwpwm); + if (ret < 0) + return ret; + + if (hwpwm >= chip->npwm) + return -ENODEV; + + ret = pwm_unexport_child(pwmchip_dev, &chip->pwms[hwpwm]); + + return ret ? : len; +} +static DEVICE_ATTR_WO(unexport); + +static ssize_t npwm_show(struct device *pwmchip_dev, struct device_attribute *attr, + char *buf) +{ + const struct pwm_chip *chip = pwmchip_from_dev(pwmchip_dev); + + return sysfs_emit(buf, "%u\n", chip->npwm); +} +static DEVICE_ATTR_RO(npwm); + +static struct attribute *pwm_chip_attrs[] = { + &dev_attr_export.attr, + &dev_attr_unexport.attr, + &dev_attr_npwm.attr, + NULL, +}; +ATTRIBUTE_GROUPS(pwm_chip); + +/* takes export->lock on success */ +static struct pwm_export *pwm_class_get_state(struct device *pwmchip_dev, + struct pwm_device *pwm, + struct pwm_state *state) +{ + struct device *pwm_dev; + struct pwm_export *export; + + if (!test_bit(PWMF_EXPORTED, &pwm->flags)) + return NULL; + + pwm_dev = device_find_child(pwmchip_dev, pwm, pwm_unexport_match); + if (!pwm_dev) + return NULL; + + export = pwmexport_from_dev(pwm_dev); + put_device(pwm_dev); /* for device_find_child() */ + + mutex_lock(&export->lock); + pwm_get_state(pwm, state); + + return export; +} + +static int pwm_class_apply_state(struct pwm_export *export, + struct pwm_device *pwm, + struct pwm_state *state) +{ + int ret = pwm_apply_might_sleep(pwm, state); + + /* release lock taken in pwm_class_get_state */ + mutex_unlock(&export->lock); + + return ret; +} + +static int pwm_class_resume_npwm(struct device *pwmchip_dev, unsigned int npwm) +{ + struct pwm_chip *chip = pwmchip_from_dev(pwmchip_dev); + unsigned int i; + int ret = 0; + + for (i = 0; i < npwm; i++) { + struct pwm_device *pwm = &chip->pwms[i]; + struct pwm_state state; + struct pwm_export *export; + + export = pwm_class_get_state(pwmchip_dev, pwm, &state); + if (!export) + continue; + + /* If pwmchip was not enabled before suspend, do nothing. */ + if (!export->suspend.enabled) { + /* release lock taken in pwm_class_get_state */ + mutex_unlock(&export->lock); + continue; + } + + state.enabled = export->suspend.enabled; + ret = pwm_class_apply_state(export, pwm, &state); + if (ret < 0) + break; + } + + return ret; +} + +static int pwm_class_suspend(struct device *pwmchip_dev) +{ + struct pwm_chip *chip = pwmchip_from_dev(pwmchip_dev); + unsigned int i; + int ret = 0; + + for (i = 0; i < chip->npwm; i++) { + struct pwm_device *pwm = &chip->pwms[i]; + struct pwm_state state; + struct pwm_export *export; + + export = pwm_class_get_state(pwmchip_dev, pwm, &state); + if (!export) + continue; + + /* + * If pwmchip was not enabled before suspend, save + * state for resume time and do nothing else. + */ + export->suspend = state; + if (!state.enabled) { + /* release lock taken in pwm_class_get_state */ + mutex_unlock(&export->lock); + continue; + } + + state.enabled = false; + ret = pwm_class_apply_state(export, pwm, &state); + if (ret < 0) { + /* + * roll back the PWM devices that were disabled by + * this suspend function. + */ + pwm_class_resume_npwm(pwmchip_dev, i); + break; + } + } + + return ret; +} + +static int pwm_class_resume(struct device *pwmchip_dev) +{ + struct pwm_chip *chip = pwmchip_from_dev(pwmchip_dev); + + return pwm_class_resume_npwm(pwmchip_dev, chip->npwm); +} + +static DEFINE_SIMPLE_DEV_PM_OPS(pwm_class_pm_ops, pwm_class_suspend, pwm_class_resume); + +static struct class pwm_class = { + .name = "pwm", + .dev_groups = pwm_chip_groups, + .pm = pm_sleep_ptr(&pwm_class_pm_ops), +}; + +static int pwmchip_sysfs_match(struct device *pwmchip_dev, const void *data) +{ + return pwmchip_from_dev(pwmchip_dev) == data; +} + +static void pwmchip_sysfs_export(struct pwm_chip *chip) +{ + struct device *pwmchip_dev; + + /* + * If device_create() fails the pwm_chip is still usable by + * the kernel it's just not exported. + */ + pwmchip_dev = device_create(&pwm_class, pwmchip_parent(chip), MKDEV(0, 0), chip, + "pwmchip%d", chip->id); + if (IS_ERR(pwmchip_dev)) { + dev_warn(pwmchip_parent(chip), + "device_create failed for pwm_chip sysfs export\n"); + } +} + +static void pwmchip_sysfs_unexport(struct pwm_chip *chip) +{ + struct device *pwmchip_dev; + unsigned int i; + + pwmchip_dev = class_find_device(&pwm_class, NULL, chip, + pwmchip_sysfs_match); + if (!pwmchip_dev) + return; + + for (i = 0; i < chip->npwm; i++) { + struct pwm_device *pwm = &chip->pwms[i]; + + if (test_bit(PWMF_EXPORTED, &pwm->flags)) + pwm_unexport_child(pwmchip_dev, pwm); + } + + put_device(pwmchip_dev); + device_unregister(pwmchip_dev); +} + #define PWMCHIP_ALIGN ARCH_DMA_MINALIGN static void *pwmchip_priv(struct pwm_chip *chip) @@ -1086,7 +1615,6 @@ 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) { unsigned int i; @@ -1171,11 +1699,11 @@ static const struct seq_operations pwm_debugfs_sops = { DEFINE_SEQ_ATTRIBUTE(pwm_debugfs); -static int __init pwm_debugfs_init(void) +static int __init pwm_init(void) { - debugfs_create_file("pwm", 0444, NULL, NULL, &pwm_debugfs_fops); + if (IS_ENABLED(CONFIG_DEBUG_FS)) + debugfs_create_file("pwm", 0444, NULL, NULL, &pwm_debugfs_fops); - return 0; + return class_register(&pwm_class); } -subsys_initcall(pwm_debugfs_init); -#endif /* CONFIG_DEBUG_FS */ +subsys_initcall(pwm_init); diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c deleted file mode 100644 index 1a1b1f6dd98f..000000000000 --- a/drivers/pwm/sysfs.c +++ /dev/null @@ -1,550 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0-or-later -/* - * A simple sysfs interface for the generic PWM framework - * - * Copyright (C) 2013 H Hartley Sweeten - * - * Based on previous work by Lars Poeschel - */ - -#include -#include -#include -#include -#include -#include - -struct pwm_export { - struct device pwm_dev; - struct pwm_device *pwm; - struct mutex lock; - struct pwm_state suspend; -}; - -static inline struct pwm_chip *pwmchip_from_dev(struct device *pwmchip_dev) -{ - return dev_get_drvdata(pwmchip_dev); -} - -static inline struct pwm_export *pwmexport_from_dev(struct device *pwm_dev) -{ - return container_of(pwm_dev, struct pwm_export, pwm_dev); -} - -static inline struct pwm_device *pwm_from_dev(struct device *pwm_dev) -{ - struct pwm_export *export = pwmexport_from_dev(pwm_dev); - - return export->pwm; -} - -static ssize_t period_show(struct device *pwm_dev, - struct device_attribute *attr, - char *buf) -{ - const struct pwm_device *pwm = pwm_from_dev(pwm_dev); - struct pwm_state state; - - pwm_get_state(pwm, &state); - - return sysfs_emit(buf, "%llu\n", state.period); -} - -static ssize_t period_store(struct device *pwm_dev, - struct device_attribute *attr, - const char *buf, size_t size) -{ - struct pwm_export *export = pwmexport_from_dev(pwm_dev); - struct pwm_device *pwm = export->pwm; - struct pwm_state state; - u64 val; - int ret; - - ret = kstrtou64(buf, 0, &val); - if (ret) - return ret; - - mutex_lock(&export->lock); - pwm_get_state(pwm, &state); - state.period = val; - ret = pwm_apply_might_sleep(pwm, &state); - mutex_unlock(&export->lock); - - return ret ? : size; -} - -static ssize_t duty_cycle_show(struct device *pwm_dev, - struct device_attribute *attr, - char *buf) -{ - const struct pwm_device *pwm = pwm_from_dev(pwm_dev); - struct pwm_state state; - - pwm_get_state(pwm, &state); - - return sysfs_emit(buf, "%llu\n", state.duty_cycle); -} - -static ssize_t duty_cycle_store(struct device *pwm_dev, - struct device_attribute *attr, - const char *buf, size_t size) -{ - struct pwm_export *export = pwmexport_from_dev(pwm_dev); - struct pwm_device *pwm = export->pwm; - struct pwm_state state; - u64 val; - int ret; - - ret = kstrtou64(buf, 0, &val); - if (ret) - return ret; - - mutex_lock(&export->lock); - pwm_get_state(pwm, &state); - state.duty_cycle = val; - ret = pwm_apply_might_sleep(pwm, &state); - mutex_unlock(&export->lock); - - return ret ? : size; -} - -static ssize_t enable_show(struct device *pwm_dev, - struct device_attribute *attr, - char *buf) -{ - const struct pwm_device *pwm = pwm_from_dev(pwm_dev); - struct pwm_state state; - - pwm_get_state(pwm, &state); - - return sysfs_emit(buf, "%d\n", state.enabled); -} - -static ssize_t enable_store(struct device *pwm_dev, - struct device_attribute *attr, - const char *buf, size_t size) -{ - struct pwm_export *export = pwmexport_from_dev(pwm_dev); - struct pwm_device *pwm = export->pwm; - struct pwm_state state; - int val, ret; - - ret = kstrtoint(buf, 0, &val); - if (ret) - return ret; - - mutex_lock(&export->lock); - - pwm_get_state(pwm, &state); - - switch (val) { - case 0: - state.enabled = false; - break; - case 1: - state.enabled = true; - break; - default: - ret = -EINVAL; - goto unlock; - } - - ret = pwm_apply_might_sleep(pwm, &state); - -unlock: - mutex_unlock(&export->lock); - return ret ? : size; -} - -static ssize_t polarity_show(struct device *pwm_dev, - struct device_attribute *attr, - char *buf) -{ - const struct pwm_device *pwm = pwm_from_dev(pwm_dev); - const char *polarity = "unknown"; - struct pwm_state state; - - pwm_get_state(pwm, &state); - - switch (state.polarity) { - case PWM_POLARITY_NORMAL: - polarity = "normal"; - break; - - case PWM_POLARITY_INVERSED: - polarity = "inversed"; - break; - } - - return sysfs_emit(buf, "%s\n", polarity); -} - -static ssize_t polarity_store(struct device *pwm_dev, - struct device_attribute *attr, - const char *buf, size_t size) -{ - struct pwm_export *export = pwmexport_from_dev(pwm_dev); - struct pwm_device *pwm = export->pwm; - enum pwm_polarity polarity; - struct pwm_state state; - int ret; - - if (sysfs_streq(buf, "normal")) - polarity = PWM_POLARITY_NORMAL; - else if (sysfs_streq(buf, "inversed")) - polarity = PWM_POLARITY_INVERSED; - else - return -EINVAL; - - mutex_lock(&export->lock); - pwm_get_state(pwm, &state); - state.polarity = polarity; - ret = pwm_apply_might_sleep(pwm, &state); - mutex_unlock(&export->lock); - - return ret ? : size; -} - -static ssize_t capture_show(struct device *pwm_dev, - struct device_attribute *attr, - char *buf) -{ - struct pwm_device *pwm = pwm_from_dev(pwm_dev); - struct pwm_capture result; - int ret; - - ret = pwm_capture(pwm, &result, jiffies_to_msecs(HZ)); - if (ret) - return ret; - - return sysfs_emit(buf, "%u %u\n", result.period, result.duty_cycle); -} - -static DEVICE_ATTR_RW(period); -static DEVICE_ATTR_RW(duty_cycle); -static DEVICE_ATTR_RW(enable); -static DEVICE_ATTR_RW(polarity); -static DEVICE_ATTR_RO(capture); - -static struct attribute *pwm_attrs[] = { - &dev_attr_period.attr, - &dev_attr_duty_cycle.attr, - &dev_attr_enable.attr, - &dev_attr_polarity.attr, - &dev_attr_capture.attr, - NULL -}; -ATTRIBUTE_GROUPS(pwm); - -static void pwm_export_release(struct device *pwm_dev) -{ - struct pwm_export *export = pwmexport_from_dev(pwm_dev); - - kfree(export); -} - -static int pwm_export_child(struct device *pwmchip_dev, struct pwm_device *pwm) -{ - struct pwm_export *export; - char *pwm_prop[2]; - int ret; - - if (test_and_set_bit(PWMF_EXPORTED, &pwm->flags)) - return -EBUSY; - - export = kzalloc(sizeof(*export), GFP_KERNEL); - if (!export) { - clear_bit(PWMF_EXPORTED, &pwm->flags); - return -ENOMEM; - } - - export->pwm = pwm; - mutex_init(&export->lock); - - export->pwm_dev.release = pwm_export_release; - export->pwm_dev.parent = pwmchip_dev; - export->pwm_dev.devt = MKDEV(0, 0); - export->pwm_dev.groups = pwm_groups; - dev_set_name(&export->pwm_dev, "pwm%u", pwm->hwpwm); - - ret = device_register(&export->pwm_dev); - if (ret) { - clear_bit(PWMF_EXPORTED, &pwm->flags); - put_device(&export->pwm_dev); - export = NULL; - return ret; - } - pwm_prop[0] = kasprintf(GFP_KERNEL, "EXPORT=pwm%u", pwm->hwpwm); - pwm_prop[1] = NULL; - kobject_uevent_env(&pwmchip_dev->kobj, KOBJ_CHANGE, pwm_prop); - kfree(pwm_prop[0]); - - return 0; -} - -static int pwm_unexport_match(struct device *pwm_dev, void *data) -{ - return pwm_from_dev(pwm_dev) == data; -} - -static int pwm_unexport_child(struct device *pwmchip_dev, struct pwm_device *pwm) -{ - struct device *pwm_dev; - char *pwm_prop[2]; - - if (!test_and_clear_bit(PWMF_EXPORTED, &pwm->flags)) - return -ENODEV; - - pwm_dev = device_find_child(pwmchip_dev, pwm, pwm_unexport_match); - if (!pwm_dev) - return -ENODEV; - - pwm_prop[0] = kasprintf(GFP_KERNEL, "UNEXPORT=pwm%u", pwm->hwpwm); - pwm_prop[1] = NULL; - kobject_uevent_env(&pwmchip_dev->kobj, KOBJ_CHANGE, pwm_prop); - kfree(pwm_prop[0]); - - /* for device_find_child() */ - put_device(pwm_dev); - device_unregister(pwm_dev); - pwm_put(pwm); - - return 0; -} - -static ssize_t export_store(struct device *pwmchip_dev, - struct device_attribute *attr, - const char *buf, size_t len) -{ - struct pwm_chip *chip = pwmchip_from_dev(pwmchip_dev); - struct pwm_device *pwm; - unsigned int hwpwm; - int ret; - - ret = kstrtouint(buf, 0, &hwpwm); - if (ret < 0) - return ret; - - if (hwpwm >= chip->npwm) - return -ENODEV; - - pwm = pwm_request_from_chip(chip, hwpwm, "sysfs"); - if (IS_ERR(pwm)) - return PTR_ERR(pwm); - - ret = pwm_export_child(pwmchip_dev, pwm); - if (ret < 0) - pwm_put(pwm); - - return ret ? : len; -} -static DEVICE_ATTR_WO(export); - -static ssize_t unexport_store(struct device *pwmchip_dev, - struct device_attribute *attr, - const char *buf, size_t len) -{ - struct pwm_chip *chip = pwmchip_from_dev(pwmchip_dev); - unsigned int hwpwm; - int ret; - - ret = kstrtouint(buf, 0, &hwpwm); - if (ret < 0) - return ret; - - if (hwpwm >= chip->npwm) - return -ENODEV; - - ret = pwm_unexport_child(pwmchip_dev, &chip->pwms[hwpwm]); - - return ret ? : len; -} -static DEVICE_ATTR_WO(unexport); - -static ssize_t npwm_show(struct device *pwmchip_dev, struct device_attribute *attr, - char *buf) -{ - const struct pwm_chip *chip = pwmchip_from_dev(pwmchip_dev); - - return sysfs_emit(buf, "%u\n", chip->npwm); -} -static DEVICE_ATTR_RO(npwm); - -static struct attribute *pwm_chip_attrs[] = { - &dev_attr_export.attr, - &dev_attr_unexport.attr, - &dev_attr_npwm.attr, - NULL, -}; -ATTRIBUTE_GROUPS(pwm_chip); - -/* takes export->lock on success */ -static struct pwm_export *pwm_class_get_state(struct device *pwmchip_dev, - struct pwm_device *pwm, - struct pwm_state *state) -{ - struct device *pwm_dev; - struct pwm_export *export; - - if (!test_bit(PWMF_EXPORTED, &pwm->flags)) - return NULL; - - pwm_dev = device_find_child(pwmchip_dev, pwm, pwm_unexport_match); - if (!pwm_dev) - return NULL; - - export = pwmexport_from_dev(pwm_dev); - put_device(pwm_dev); /* for device_find_child() */ - - mutex_lock(&export->lock); - pwm_get_state(pwm, state); - - return export; -} - -static int pwm_class_apply_state(struct pwm_export *export, - struct pwm_device *pwm, - struct pwm_state *state) -{ - int ret = pwm_apply_might_sleep(pwm, state); - - /* release lock taken in pwm_class_get_state */ - mutex_unlock(&export->lock); - - return ret; -} - -static int pwm_class_resume_npwm(struct device *pwmchip_dev, unsigned int npwm) -{ - struct pwm_chip *chip = pwmchip_from_dev(pwmchip_dev); - unsigned int i; - int ret = 0; - - for (i = 0; i < npwm; i++) { - struct pwm_device *pwm = &chip->pwms[i]; - struct pwm_state state; - struct pwm_export *export; - - export = pwm_class_get_state(pwmchip_dev, pwm, &state); - if (!export) - continue; - - /* If pwmchip was not enabled before suspend, do nothing. */ - if (!export->suspend.enabled) { - /* release lock taken in pwm_class_get_state */ - mutex_unlock(&export->lock); - continue; - } - - state.enabled = export->suspend.enabled; - ret = pwm_class_apply_state(export, pwm, &state); - if (ret < 0) - break; - } - - return ret; -} - -static int pwm_class_suspend(struct device *pwmchip_dev) -{ - struct pwm_chip *chip = pwmchip_from_dev(pwmchip_dev); - unsigned int i; - int ret = 0; - - for (i = 0; i < chip->npwm; i++) { - struct pwm_device *pwm = &chip->pwms[i]; - struct pwm_state state; - struct pwm_export *export; - - export = pwm_class_get_state(pwmchip_dev, pwm, &state); - if (!export) - continue; - - /* - * If pwmchip was not enabled before suspend, save - * state for resume time and do nothing else. - */ - export->suspend = state; - if (!state.enabled) { - /* release lock taken in pwm_class_get_state */ - mutex_unlock(&export->lock); - continue; - } - - state.enabled = false; - ret = pwm_class_apply_state(export, pwm, &state); - if (ret < 0) { - /* - * roll back the PWM devices that were disabled by - * this suspend function. - */ - pwm_class_resume_npwm(pwmchip_dev, i); - break; - } - } - - return ret; -} - -static int pwm_class_resume(struct device *pwmchip_dev) -{ - struct pwm_chip *chip = pwmchip_from_dev(pwmchip_dev); - - return pwm_class_resume_npwm(pwmchip_dev, chip->npwm); -} - -static DEFINE_SIMPLE_DEV_PM_OPS(pwm_class_pm_ops, pwm_class_suspend, pwm_class_resume); - -static struct class pwm_class = { - .name = "pwm", - .dev_groups = pwm_chip_groups, - .pm = pm_sleep_ptr(&pwm_class_pm_ops), -}; - -static int pwmchip_sysfs_match(struct device *pwmchip_dev, const void *data) -{ - return pwmchip_from_dev(pwmchip_dev) == data; -} - -void pwmchip_sysfs_export(struct pwm_chip *chip) -{ - struct device *pwmchip_dev; - - /* - * If device_create() fails the pwm_chip is still usable by - * the kernel it's just not exported. - */ - pwmchip_dev = device_create(&pwm_class, pwmchip_parent(chip), MKDEV(0, 0), chip, - "pwmchip%d", chip->id); - if (IS_ERR(pwmchip_dev)) { - dev_warn(pwmchip_parent(chip), - "device_create failed for pwm_chip sysfs export\n"); - } -} - -void pwmchip_sysfs_unexport(struct pwm_chip *chip) -{ - struct device *pwmchip_dev; - unsigned int i; - - pwmchip_dev = class_find_device(&pwm_class, NULL, chip, - pwmchip_sysfs_match); - if (!pwmchip_dev) - return; - - for (i = 0; i < chip->npwm; i++) { - struct pwm_device *pwm = &chip->pwms[i]; - - if (test_bit(PWMF_EXPORTED, &pwm->flags)) - pwm_unexport_child(pwmchip_dev, pwm); - } - - put_device(pwmchip_dev); - device_unregister(pwmchip_dev); -} - -static int __init pwm_sysfs_init(void) -{ - return class_register(&pwm_class); -} -subsys_initcall(pwm_sysfs_init); diff --git a/include/linux/pwm.h b/include/linux/pwm.h index 94a642a88817..17e45d8413ed 100644 --- a/include/linux/pwm.h +++ b/include/linux/pwm.h @@ -630,17 +630,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 */ From ee37bf50749f06b29394d7ba8a85b47f023b61e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Sun, 17 Mar 2024 11:40:35 +0100 Subject: [PATCH 13/34] pwm: Ensure a struct pwm has the same lifetime as its pwm_chip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It's required to not free the memory underlying a requested PWM while a consumer still has a reference to it. While currently a pwm_chip doesn't live long enough in all cases, linking the struct pwm to the pwm_chip results in the right lifetime as soon as the pwmchip is living long enough. This happens with the following commits. Note this is a breaking change for all pwm drivers that don't use pwmchip_alloc(). Reviewed-by: Gustavo A. R. Silva # for struct_size() and __counted_by() Link: https://lore.kernel.org/r/7e9e958841f049026c0023b309cc9deecf0ab61d.1710670958.git.u.kleine-koenig@pengutronix.de Signed-off-by: Uwe Kleine-König --- drivers/pwm/core.c | 27 ++++++++++----------------- include/linux/pwm.h | 2 +- 2 files changed, 11 insertions(+), 18 deletions(-) diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c index faa0f885e057..19386452e6c8 100644 --- a/drivers/pwm/core.c +++ b/drivers/pwm/core.c @@ -987,7 +987,7 @@ static void pwmchip_sysfs_unexport(struct pwm_chip *chip) static void *pwmchip_priv(struct pwm_chip *chip) { - return (void *)chip + ALIGN(sizeof(*chip), PWMCHIP_ALIGN); + return (void *)chip + ALIGN(struct_size(chip, pwms, chip->npwm), PWMCHIP_ALIGN); } /* This is the counterpart to pwmchip_alloc() */ @@ -1001,8 +1001,10 @@ struct pwm_chip *pwmchip_alloc(struct device *parent, unsigned int npwm, size_t { struct pwm_chip *chip; size_t alloc_size; + unsigned int i; - alloc_size = size_add(ALIGN(sizeof(*chip), PWMCHIP_ALIGN), sizeof_priv); + alloc_size = size_add(ALIGN(struct_size(chip, pwms, npwm), PWMCHIP_ALIGN), + sizeof_priv); chip = kzalloc(alloc_size, GFP_KERNEL); if (!chip) @@ -1014,6 +1016,12 @@ struct pwm_chip *pwmchip_alloc(struct device *parent, unsigned int npwm, size_t pwmchip_set_drvdata(chip, pwmchip_priv(chip)); + for (i = 0; i < chip->npwm; i++) { + struct pwm_device *pwm = &chip->pwms[i]; + pwm->chip = chip; + pwm->hwpwm = i; + } + return chip; } EXPORT_SYMBOL_GPL(pwmchip_alloc); @@ -1085,7 +1093,6 @@ 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; if (!chip || !pwmchip_parent(chip) || !chip->ops || !chip->npwm) @@ -1105,28 +1112,16 @@ int __pwmchip_add(struct pwm_chip *chip, struct module *owner) chip->owner = owner; - chip->pwms = kcalloc(chip->npwm, sizeof(*chip->pwms), GFP_KERNEL); - if (!chip->pwms) - return -ENOMEM; - mutex_lock(&pwm_lock); ret = idr_alloc(&pwm_chips, 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); if (IS_ENABLED(CONFIG_OF)) @@ -1156,8 +1151,6 @@ void pwmchip_remove(struct pwm_chip *chip) idr_remove(&pwm_chips, chip->id); mutex_unlock(&pwm_lock); - - kfree(chip->pwms); } EXPORT_SYMBOL_GPL(pwmchip_remove); diff --git a/include/linux/pwm.h b/include/linux/pwm.h index 17e45d8413ed..78b9061572ff 100644 --- a/include/linux/pwm.h +++ b/include/linux/pwm.h @@ -290,7 +290,7 @@ struct pwm_chip { /* only used internally by the PWM framework */ bool uses_pwmchip_alloc; void *driver_data; - struct pwm_device *pwms; + struct pwm_device pwms[] __counted_by(npwm); }; static inline struct device *pwmchip_parent(const struct pwm_chip *chip) From 4c56b1434b814899c42a9d9f43d8265371282cd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Sun, 17 Mar 2024 11:40:36 +0100 Subject: [PATCH 14/34] pwm: Add a struct device to struct pwm_chip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This replaces the formerly dynamically allocated struct device. This allows to additionally use it to track the lifetime of the struct pwm_chip. Otherwise the new struct device provides the same sysfs API as was provided by the dynamic device before. Link: https://lore.kernel.org/r/35c65ea7f6de789a568ff39d7b6b4ce80de4b7dc.1710670958.git.u.kleine-koenig@pengutronix.de Signed-off-by: Uwe Kleine-König --- drivers/pwm/core.c | 93 +++++++++++++++++++++++++-------------------- include/linux/pwm.h | 5 ++- 2 files changed, 54 insertions(+), 44 deletions(-) diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c index 19386452e6c8..32871687c0a4 100644 --- a/drivers/pwm/core.c +++ b/drivers/pwm/core.c @@ -343,9 +343,16 @@ static int pwm_device_request(struct pwm_device *pwm, const char *label) if (!try_module_get(chip->owner)) return -ENODEV; + if (!get_device(&chip->dev)) { + err = -ENODEV; + goto err_get_device; + } + if (ops->request) { err = ops->request(chip, pwm); if (err) { + put_device(&chip->dev); +err_get_device: module_put(chip->owner); return err; } @@ -463,7 +470,7 @@ struct pwm_export { static inline struct pwm_chip *pwmchip_from_dev(struct device *pwmchip_dev) { - return dev_get_drvdata(pwmchip_dev); + return container_of(pwmchip_dev, struct pwm_chip, dev); } static inline struct pwm_export *pwmexport_from_dev(struct device *pwm_dev) @@ -941,46 +948,16 @@ static struct class pwm_class = { .pm = pm_sleep_ptr(&pwm_class_pm_ops), }; -static int pwmchip_sysfs_match(struct device *pwmchip_dev, const void *data) -{ - return pwmchip_from_dev(pwmchip_dev) == data; -} - -static void pwmchip_sysfs_export(struct pwm_chip *chip) -{ - struct device *pwmchip_dev; - - /* - * If device_create() fails the pwm_chip is still usable by - * the kernel it's just not exported. - */ - pwmchip_dev = device_create(&pwm_class, pwmchip_parent(chip), MKDEV(0, 0), chip, - "pwmchip%d", chip->id); - if (IS_ERR(pwmchip_dev)) { - dev_warn(pwmchip_parent(chip), - "device_create failed for pwm_chip sysfs export\n"); - } -} - static void pwmchip_sysfs_unexport(struct pwm_chip *chip) { - struct device *pwmchip_dev; unsigned int i; - pwmchip_dev = class_find_device(&pwm_class, NULL, chip, - pwmchip_sysfs_match); - if (!pwmchip_dev) - return; - for (i = 0; i < chip->npwm; i++) { struct pwm_device *pwm = &chip->pwms[i]; if (test_bit(PWMF_EXPORTED, &pwm->flags)) - pwm_unexport_child(pwmchip_dev, pwm); + pwm_unexport_child(&chip->dev, pwm); } - - put_device(pwmchip_dev); - device_unregister(pwmchip_dev); } #define PWMCHIP_ALIGN ARCH_DMA_MINALIGN @@ -993,13 +970,21 @@ static void *pwmchip_priv(struct pwm_chip *chip) /* This is the counterpart to pwmchip_alloc() */ void pwmchip_put(struct pwm_chip *chip) { - kfree(chip); + put_device(&chip->dev); } EXPORT_SYMBOL_GPL(pwmchip_put); +static void pwmchip_release(struct device *pwmchip_dev) +{ + struct pwm_chip *chip = pwmchip_from_dev(pwmchip_dev); + + kfree(chip); +} + struct pwm_chip *pwmchip_alloc(struct device *parent, unsigned int npwm, size_t sizeof_priv) { struct pwm_chip *chip; + struct device *pwmchip_dev; size_t alloc_size; unsigned int i; @@ -1010,10 +995,15 @@ struct pwm_chip *pwmchip_alloc(struct device *parent, unsigned int npwm, size_t if (!chip) return ERR_PTR(-ENOMEM); - chip->dev = parent; chip->npwm = npwm; chip->uses_pwmchip_alloc = true; + pwmchip_dev = &chip->dev; + device_initialize(pwmchip_dev); + pwmchip_dev->class = &pwm_class; + pwmchip_dev->parent = parent; + pwmchip_dev->release = pwmchip_release; + pwmchip_set_drvdata(chip, pwmchip_priv(chip)); for (i = 0; i < chip->npwm; i++) { @@ -1115,21 +1105,34 @@ int __pwmchip_add(struct pwm_chip *chip, struct module *owner) mutex_lock(&pwm_lock); ret = idr_alloc(&pwm_chips, chip, 0, 0, GFP_KERNEL); - if (ret < 0) { - mutex_unlock(&pwm_lock); - return ret; - } + if (ret < 0) + goto err_idr_alloc; chip->id = ret; - mutex_unlock(&pwm_lock); + dev_set_name(&chip->dev, "pwmchip%u", chip->id); if (IS_ENABLED(CONFIG_OF)) of_pwmchip_add(chip); - pwmchip_sysfs_export(chip); + ret = device_add(&chip->dev); + if (ret) + goto err_device_add; + + mutex_unlock(&pwm_lock); return 0; + +err_device_add: + if (IS_ENABLED(CONFIG_OF)) + of_pwmchip_remove(chip); + + idr_remove(&pwm_chips, chip->id); +err_idr_alloc: + + mutex_unlock(&pwm_lock); + + return ret; } EXPORT_SYMBOL_GPL(__pwmchip_add); @@ -1151,6 +1154,8 @@ void pwmchip_remove(struct pwm_chip *chip) idr_remove(&pwm_chips, chip->id); mutex_unlock(&pwm_lock); + + device_del(&chip->dev); } EXPORT_SYMBOL_GPL(pwmchip_remove); @@ -1520,6 +1525,8 @@ EXPORT_SYMBOL_GPL(pwm_get); */ void pwm_put(struct pwm_device *pwm) { + struct pwm_chip *chip = pwm->chip; + if (!pwm) return; @@ -1530,12 +1537,14 @@ void pwm_put(struct pwm_device *pwm) goto out; } - if (pwm->chip->ops->free) + if (chip->ops->free) pwm->chip->ops->free(pwm->chip, pwm); pwm->label = NULL; - module_put(pwm->chip->owner); + put_device(&chip->dev); + + module_put(chip->owner); out: mutex_unlock(&pwm_lock); } diff --git a/include/linux/pwm.h b/include/linux/pwm.h index 78b9061572ff..495e23761f34 100644 --- a/include/linux/pwm.h +++ b/include/linux/pwm.h @@ -2,6 +2,7 @@ #ifndef __LINUX_PWM_H #define __LINUX_PWM_H +#include #include #include #include @@ -277,7 +278,7 @@ struct pwm_ops { * @pwms: array of PWM devices allocated by the framework */ struct pwm_chip { - struct device *dev; + struct device dev; const struct pwm_ops *ops; struct module *owner; unsigned int id; @@ -295,7 +296,7 @@ struct pwm_chip { static inline struct device *pwmchip_parent(const struct pwm_chip *chip) { - return chip->dev; + return chip->dev.parent; } static inline void *pwmchip_get_drvdata(struct pwm_chip *chip) From c6837aa18016da3e524eb7bb51b8941c1ce8cd73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Fri, 29 Mar 2024 11:07:56 +0100 Subject: [PATCH 15/34] pwm: Don't check pointer for being non-NULL after use MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After assigning chip = pwm->chip; the compiler is free to assume that pwm is non-NULL and so can optimize out the check for pwm against NULL. While it's probably a programming error to pass a NULL pointer to pwm_put() this shouldn't be dropped without careful consideration and wasn't intended. So assign chip only after the NULL check. Reported-by: David Lechner Link: https://lore.kernel.org/r/66a6f562-1fdd-4e45-995a-e7995432aa0c@baylibre.com Fixes: 4c56b1434b81 ("pwm: Add a struct device to struct pwm_chip") Link: https://lore.kernel.org/r/20240329101648.544155-2-u.kleine-koenig@pengutronix.de Reviewed-by: David Lechner Signed-off-by: Uwe Kleine-König --- drivers/pwm/core.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c index 32871687c0a4..18574857641e 100644 --- a/drivers/pwm/core.c +++ b/drivers/pwm/core.c @@ -1525,11 +1525,13 @@ EXPORT_SYMBOL_GPL(pwm_get); */ void pwm_put(struct pwm_device *pwm) { - struct pwm_chip *chip = pwm->chip; + struct pwm_chip *chip; if (!pwm) return; + chip = pwm->chip; + mutex_lock(&pwm_lock); if (!test_and_clear_bit(PWMF_REQUESTED, &pwm->flags)) { From 38ae7142e35165571123997addd71393a9dabde5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Sun, 17 Mar 2024 11:40:37 +0100 Subject: [PATCH 16/34] pwm: Make pwmchip_[sg]et_drvdata() a wrapper around dev_set_drvdata() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that a pwm_chip has a dedicated struct device, pwmchip_set_drvdata() and pwmchip_get_drvdata() can be made thin wrappers around dev_set_drvdata() and dev_get_drvdata() respectively and the previously needed pointer can be dropped from struct pwm_chip. Link: https://lore.kernel.org/r/a5e05bd2d83421a26fdef6a87d69253c0f98becf.1710670958.git.u.kleine-koenig@pengutronix.de Signed-off-by: Uwe Kleine-König --- include/linux/pwm.h | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/include/linux/pwm.h b/include/linux/pwm.h index 495e23761f34..60b92c2c75ef 100644 --- a/include/linux/pwm.h +++ b/include/linux/pwm.h @@ -274,7 +274,6 @@ struct pwm_ops { * @of_xlate: request a PWM device given a device tree PWM specifier * @atomic: can the driver's ->apply() be called in atomic context * @uses_pwmchip_alloc: signals if pwmchip_allow was used to allocate this chip - * @driver_data: Private pointer for driver specific info * @pwms: array of PWM devices allocated by the framework */ struct pwm_chip { @@ -290,7 +289,6 @@ struct pwm_chip { /* only used internally by the PWM framework */ bool uses_pwmchip_alloc; - void *driver_data; struct pwm_device pwms[] __counted_by(npwm); }; @@ -301,20 +299,12 @@ static inline struct device *pwmchip_parent(const struct pwm_chip *chip) static inline void *pwmchip_get_drvdata(struct pwm_chip *chip) { - /* - * After pwm_chip got a dedicated struct device, this can be replaced by - * dev_get_drvdata(&chip->dev); - */ - return chip->driver_data; + return dev_get_drvdata(&chip->dev); } static inline void pwmchip_set_drvdata(struct pwm_chip *chip, void *data) { - /* - * After pwm_chip got a dedicated struct device, this can be replaced by - * dev_set_drvdata(&chip->dev, data); - */ - chip->driver_data = data; + dev_set_drvdata(&chip->dev, data); } #if IS_ENABLED(CONFIG_PWM) From 80bd81cb7a87e98c17f8faa31b0700c466c94e92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Fri, 15 Mar 2024 15:54:43 +0100 Subject: [PATCH 17/34] pwm: stm32: Add error messages in .probe()'s error paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Giving an indication about the problem if probing a device fails is a nice move. Do that for the stm32 pwm driver. Reviewed-by: Fabrice Gasnier Link: https://lore.kernel.org/r/20240315145443.982807-2-u.kleine-koenig@pengutronix.de Signed-off-by: Uwe Kleine-König --- drivers/pwm/pwm-stm32.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c index 0c028d17c075..ffe572b76174 100644 --- a/drivers/pwm/pwm-stm32.c +++ b/drivers/pwm/pwm-stm32.c @@ -648,11 +648,13 @@ static int stm32_pwm_probe(struct platform_device *pdev) priv->max_arr = ddata->max_arr; if (!priv->regmap || !priv->clk) - return -EINVAL; + return dev_err_probe(dev, -EINVAL, "Failed to get %s\n", + priv->regmap ? "clk" : "regmap"); ret = stm32_pwm_probe_breakinputs(priv, np); if (ret) - return ret; + return dev_err_probe(dev, ret, + "Failed to configure breakinputs\n"); stm32_pwm_detect_complementary(priv); @@ -664,7 +666,8 @@ static int stm32_pwm_probe(struct platform_device *pdev) ret = devm_pwmchip_add(dev, chip); if (ret < 0) - return ret; + return dev_err_probe(dev, ret, + "Failed to register pwmchip\n"); platform_set_drvdata(pdev, chip); From e419617847b688799a91126eb6c94b936bfb35ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Sun, 17 Mar 2024 22:52:14 +0100 Subject: [PATCH 18/34] pwm: stm32: Improve precision of calculation in .apply() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While mathematically it's ok to calculate the number of cyles for the duty cycle as: duty_cycles = period_cycles * duty_ns / period_ns this doesn't always give the right result when doing integer math. This is best demonstrated using an example: With the input clock running at 208877930 Hz a request for duty_cycle = 383 ns and period = 49996 ns results in period_cycles = clkrate * period_ns / NSEC_PER_SEC = 10443.06098828 Now calculating duty_cycles with the above formula gives: duty_cycles = 10443.06098828 * 383 / 49996 = 80.00024719 However with period_cycle truncated to an integer results in: duty_cycles = 10443 * 383 / 49996 = 79.99977998239859 So while a value of (a little more than) 80 would be the right result, only 79 is used here. The problem here is that 14443 is a rounded result that should better not be used to do further math. So to fix that use the exact formular similar to how period_cycles is calculated. Link: https://lore.kernel.org/r/7628ecd8a7538aa5a7397f0fc4199a077168e8a6.1710711976.git.u.kleine-koenig@pengutronix.de Signed-off-by: Uwe Kleine-König --- drivers/pwm/pwm-stm32.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c index ffe572b76174..fb714936ff8f 100644 --- a/drivers/pwm/pwm-stm32.c +++ b/drivers/pwm/pwm-stm32.c @@ -351,8 +351,9 @@ static int stm32_pwm_config(struct stm32_pwm *priv, unsigned int ch, regmap_set_bits(priv->regmap, TIM_CR1, TIM_CR1_ARPE); /* Calculate the duty cycles */ - dty = prd * duty_ns; - do_div(dty, period_ns); + dty = (unsigned long long)clk_get_rate(priv->clk) * duty_ns; + do_div(dty, prescaler + 1); + do_div(dty, NSEC_PER_SEC); regmap_write(priv->regmap, TIM_CCR1 + 4 * ch, dty); From d44d635635a7192c773a75e674a8590a163e879e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Sun, 17 Mar 2024 22:52:15 +0100 Subject: [PATCH 19/34] pwm: stm32: Fix for settings using period > UINT32_MAX MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit stm32_pwm_config() took the duty_cycle and period values with the type int, however stm32_pwm_apply() passed u64 values there. Expand the function parameters to u64 to not discard relevant bits and adapt the calculations to the wider type. To ensure the calculations won't overflow, check in .probe() the input clk doesn't run faster than 1 GHz. Link: https://lore.kernel.org/r/06b4a650a608d0887d934c1b2b8919e0f78e4db2.1710711976.git.u.kleine-koenig@pengutronix.de Signed-off-by: Uwe Kleine-König --- drivers/pwm/pwm-stm32.c | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c index fb714936ff8f..ea3d7d9117f2 100644 --- a/drivers/pwm/pwm-stm32.c +++ b/drivers/pwm/pwm-stm32.c @@ -309,16 +309,18 @@ static int stm32_pwm_capture(struct pwm_chip *chip, struct pwm_device *pwm, } static int stm32_pwm_config(struct stm32_pwm *priv, unsigned int ch, - int duty_ns, int period_ns) + u64 duty_ns, u64 period_ns) { unsigned long long prd, div, dty; unsigned int prescaler = 0; u32 ccmr, mask, shift; - /* Period and prescaler values depends on clock rate */ - div = (unsigned long long)clk_get_rate(priv->clk) * period_ns; - - do_div(div, NSEC_PER_SEC); + /* + * .probe() asserted that clk_get_rate() is not bigger than 1 GHz, so + * this won't overflow. + */ + div = mul_u64_u64_div_u64(period_ns, clk_get_rate(priv->clk), + NSEC_PER_SEC); prd = div; while (div > priv->max_arr) { @@ -351,9 +353,8 @@ static int stm32_pwm_config(struct stm32_pwm *priv, unsigned int ch, regmap_set_bits(priv->regmap, TIM_CR1, TIM_CR1_ARPE); /* Calculate the duty cycles */ - dty = (unsigned long long)clk_get_rate(priv->clk) * duty_ns; - do_div(dty, prescaler + 1); - do_div(dty, NSEC_PER_SEC); + dty = mul_u64_u64_div_u64(duty_ns, clk_get_rate(priv->clk), + (u64)NSEC_PER_SEC * (prescaler + 1)); regmap_write(priv->regmap, TIM_CCR1 + 4 * ch, dty); @@ -659,6 +660,17 @@ static int stm32_pwm_probe(struct platform_device *pdev) stm32_pwm_detect_complementary(priv); + ret = devm_clk_rate_exclusive_get(dev, priv->clk); + if (ret) + return dev_err_probe(dev, ret, "Failed to lock clock\n"); + + /* + * With the clk running with not more than 1 GHz the calculations in + * .apply() won't overflow. + */ + if (clk_get_rate(priv->clk) > 1000000000) + return dev_err_probe(dev, -EINVAL, "Failed to lock clock\n"); + chip->ops = &stm32pwm_ops; /* Initialize clock refcount to number of enabled PWM channels. */ From 8002fbeef1e469b2c397d5cd2940e37b32a17849 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Sun, 17 Mar 2024 22:52:16 +0100 Subject: [PATCH 20/34] pwm: stm32: Calculate prescaler with a division instead of a loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of looping over increasing values for the prescaler and testing if it's big enough, calculate the value using a single division. Link: https://lore.kernel.org/r/498a44b313a6c0a84ccddd03cd67aadaaaf7daf2.1710711976.git.u.kleine-koenig@pengutronix.de Signed-off-by: Uwe Kleine-König --- drivers/pwm/pwm-stm32.c | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c index ea3d7d9117f2..a2f231d13a9f 100644 --- a/drivers/pwm/pwm-stm32.c +++ b/drivers/pwm/pwm-stm32.c @@ -311,29 +311,33 @@ static int stm32_pwm_capture(struct pwm_chip *chip, struct pwm_device *pwm, static int stm32_pwm_config(struct stm32_pwm *priv, unsigned int ch, u64 duty_ns, u64 period_ns) { - unsigned long long prd, div, dty; - unsigned int prescaler = 0; + unsigned long long prd, dty; + unsigned long long prescaler; u32 ccmr, mask, shift; /* * .probe() asserted that clk_get_rate() is not bigger than 1 GHz, so - * this won't overflow. + * the calculations here won't overflow. + * First we need to find the minimal value for prescaler such that + * + * period_ns * clkrate + * ------------------------------ + * NSEC_PER_SEC * (prescaler + 1) + * + * isn't bigger than max_arr. */ - div = mul_u64_u64_div_u64(period_ns, clk_get_rate(priv->clk), - NSEC_PER_SEC); - prd = div; - while (div > priv->max_arr) { - prescaler++; - div = prd; - do_div(div, prescaler + 1); - } - - prd = div; + prescaler = mul_u64_u64_div_u64(period_ns, clk_get_rate(priv->clk), + (u64)NSEC_PER_SEC * priv->max_arr); + if (prescaler > 0) + prescaler -= 1; if (prescaler > MAX_TIM_PSC) return -EINVAL; + prd = mul_u64_u64_div_u64(period_ns, clk_get_rate(priv->clk), + (u64)NSEC_PER_SEC * (prescaler + 1)); + /* * All channels share the same prescaler and counter so when two * channels are active at the same time we can't change them From 664d8dbb54671564196e08c76d4c7c063eacb14b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Sun, 10 Mar 2024 14:47:19 +0100 Subject: [PATCH 21/34] pwm: bcm2835: Introduce a local variable for &pdev->dev MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit &pdev->dev is used several times in bcm2835_pwm_probe(). Introduce a local variable to simplify all usages. Reviewed-by: Florian Fainelli Link: https://lore.kernel.org/r/3f302472e30e21c7ef5624a1d0a2890d9fdf3c7f.1710078146.git.u.kleine-koenig@pengutronix.de Signed-off-by: Uwe Kleine-König --- drivers/pwm/pwm-bcm2835.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/pwm/pwm-bcm2835.c b/drivers/pwm/pwm-bcm2835.c index aa35acbb0cbc..3d0c089c9ef0 100644 --- a/drivers/pwm/pwm-bcm2835.c +++ b/drivers/pwm/pwm-bcm2835.c @@ -133,11 +133,12 @@ static void devm_clk_rate_exclusive_put(void *data) static int bcm2835_pwm_probe(struct platform_device *pdev) { + struct device *dev = &pdev->dev; struct pwm_chip *chip; struct bcm2835_pwm *pc; int ret; - chip = devm_pwmchip_alloc(&pdev->dev, 2, sizeof(*pc)); + chip = devm_pwmchip_alloc(dev, 2, sizeof(*pc)); if (IS_ERR(chip)) return PTR_ERR(chip); pc = to_bcm2835_pwm(chip); @@ -146,24 +147,24 @@ static int bcm2835_pwm_probe(struct platform_device *pdev) if (IS_ERR(pc->base)) return PTR_ERR(pc->base); - pc->clk = devm_clk_get_enabled(&pdev->dev, NULL); + pc->clk = devm_clk_get_enabled(dev, NULL); if (IS_ERR(pc->clk)) - return dev_err_probe(&pdev->dev, PTR_ERR(pc->clk), + return dev_err_probe(dev, PTR_ERR(pc->clk), "clock not found\n"); ret = clk_rate_exclusive_get(pc->clk); if (ret) - return dev_err_probe(&pdev->dev, ret, + return dev_err_probe(dev, ret, "fail to get exclusive rate\n"); - ret = devm_add_action_or_reset(&pdev->dev, devm_clk_rate_exclusive_put, + ret = devm_add_action_or_reset(dev, devm_clk_rate_exclusive_put, pc->clk); if (ret) return ret; pc->rate = clk_get_rate(pc->clk); if (!pc->rate) - return dev_err_probe(&pdev->dev, -EINVAL, + return dev_err_probe(dev, -EINVAL, "failed to get clock rate\n"); chip->ops = &bcm2835_pwm_ops; @@ -171,10 +172,9 @@ static int bcm2835_pwm_probe(struct platform_device *pdev) platform_set_drvdata(pdev, pc); - ret = devm_pwmchip_add(&pdev->dev, chip); + ret = devm_pwmchip_add(dev, chip); if (ret < 0) - return dev_err_probe(&pdev->dev, ret, - "failed to add pwmchip\n"); + return dev_err_probe(dev, ret, "failed to add pwmchip\n"); return 0; } From 141a8502214df0b114a81d60d2cd613c52c02b0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Sun, 10 Mar 2024 14:47:20 +0100 Subject: [PATCH 22/34] pwm: bcm2835: Drop open coded variant of devm_clk_rate_exclusive_get() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since commit b0cde62e4c54 ("clk: Add a devm variant of clk_rate_exclusive_get()") the clk subsystem provides devm_clk_rate_exclusive_get(). Replace the open coded implementation by the new function. Reviewed-by: Florian Fainelli Link: https://lore.kernel.org/r/8e1a5151a7bcd455996c873bb3d13ab86def3490.1710078146.git.u.kleine-koenig@pengutronix.de Signed-off-by: Uwe Kleine-König --- drivers/pwm/pwm-bcm2835.c | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/drivers/pwm/pwm-bcm2835.c b/drivers/pwm/pwm-bcm2835.c index 3d0c089c9ef0..578e95e0296c 100644 --- a/drivers/pwm/pwm-bcm2835.c +++ b/drivers/pwm/pwm-bcm2835.c @@ -124,13 +124,6 @@ static const struct pwm_ops bcm2835_pwm_ops = { .apply = bcm2835_pwm_apply, }; -static void devm_clk_rate_exclusive_put(void *data) -{ - struct clk *clk = data; - - clk_rate_exclusive_put(clk); -} - static int bcm2835_pwm_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; @@ -152,16 +145,11 @@ static int bcm2835_pwm_probe(struct platform_device *pdev) return dev_err_probe(dev, PTR_ERR(pc->clk), "clock not found\n"); - ret = clk_rate_exclusive_get(pc->clk); + ret = devm_clk_rate_exclusive_get(dev, pc->clk); if (ret) return dev_err_probe(dev, ret, "fail to get exclusive rate\n"); - ret = devm_add_action_or_reset(dev, devm_clk_rate_exclusive_put, - pc->clk); - if (ret) - return ret; - pc->rate = clk_get_rate(pc->clk); if (!pc->rate) return dev_err_probe(dev, -EINVAL, From 1031c2b4befd725cb88e373d207bd147572d8fbc Mon Sep 17 00:00:00 2001 From: Jerome Brunet Date: Wed, 21 Feb 2024 16:11:51 +0100 Subject: [PATCH 23/34] pwm: meson: Add generic compatible for meson8 to sm1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce a new compatible support in the Amlogic PWM driver. The PWM HW is actually the same for all SoCs supported so far. A specific compatible is needed only because the clock sources of the PWMs are hard-coded in the driver. It is better to have the clock source described in DT but this changes the bindings so a new compatible must be introduced. When all supported platform have migrated to the new compatible, support for the legacy ones may be removed from the driver. The addition of this new compatible makes the old ones obsolete, as described in the DT documentation. Adding a callback to setup the clock will also make it easier to add support for the new PWM HW found in a1, s4, c3 and t7 SoC families. Signed-off-by: Jerome Brunet Link: https://lore.kernel.org/r/20240221151154.26452-6-jbrunet@baylibre.com Signed-off-by: Uwe Kleine-König --- drivers/pwm/pwm-meson.c | 195 +++++++++++++++++++++++++--------------- 1 file changed, 121 insertions(+), 74 deletions(-) diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c index a02fdbc61256..ea96c5973488 100644 --- a/drivers/pwm/pwm-meson.c +++ b/drivers/pwm/pwm-meson.c @@ -98,6 +98,7 @@ struct meson_pwm_channel { struct meson_pwm_data { const char *const parent_names[MESON_NUM_MUX_PARENTS]; + int (*channels_init)(struct pwm_chip *chip); }; struct meson_pwm { @@ -338,86 +339,16 @@ static const struct pwm_ops meson_pwm_ops = { .get_state = meson_pwm_get_state, }; -static const struct meson_pwm_data pwm_meson8b_data = { - .parent_names = { "xtal", NULL, "fclk_div4", "fclk_div3" }, -}; - -/* - * Only the 2 first inputs of the GXBB AO PWMs are valid - * The last 2 are grounded - */ -static const struct meson_pwm_data pwm_gxbb_ao_data = { - .parent_names = { "xtal", "clk81", NULL, NULL }, -}; - -static const struct meson_pwm_data pwm_axg_ee_data = { - .parent_names = { "xtal", "fclk_div5", "fclk_div4", "fclk_div3" }, -}; - -static const struct meson_pwm_data pwm_axg_ao_data = { - .parent_names = { "xtal", "axg_ao_clk81", "fclk_div4", "fclk_div5" }, -}; - -static const struct meson_pwm_data pwm_g12a_ao_ab_data = { - .parent_names = { "xtal", "g12a_ao_clk81", "fclk_div4", "fclk_div5" }, -}; - -static const struct meson_pwm_data pwm_g12a_ao_cd_data = { - .parent_names = { "xtal", "g12a_ao_clk81", NULL, NULL }, -}; - -static const struct of_device_id meson_pwm_matches[] = { - { - .compatible = "amlogic,meson8b-pwm", - .data = &pwm_meson8b_data - }, - { - .compatible = "amlogic,meson-gxbb-pwm", - .data = &pwm_meson8b_data - }, - { - .compatible = "amlogic,meson-gxbb-ao-pwm", - .data = &pwm_gxbb_ao_data - }, - { - .compatible = "amlogic,meson-axg-ee-pwm", - .data = &pwm_axg_ee_data - }, - { - .compatible = "amlogic,meson-axg-ao-pwm", - .data = &pwm_axg_ao_data - }, - { - .compatible = "amlogic,meson-g12a-ee-pwm", - .data = &pwm_meson8b_data - }, - { - .compatible = "amlogic,meson-g12a-ao-pwm-ab", - .data = &pwm_g12a_ao_ab_data - }, - { - .compatible = "amlogic,meson-g12a-ao-pwm-cd", - .data = &pwm_g12a_ao_cd_data - }, - {}, -}; -MODULE_DEVICE_TABLE(of, meson_pwm_matches); - -static int meson_pwm_init_channels(struct pwm_chip *chip) +static int meson_pwm_init_clocks_meson8b(struct pwm_chip *chip, + struct clk_parent_data *mux_parent_data) { struct meson_pwm *meson = to_meson_pwm(chip); - struct clk_parent_data mux_parent_data[MESON_NUM_MUX_PARENTS] = {}; struct device *dev = pwmchip_parent(chip); unsigned int i; char name[255]; int err; - for (i = 0; i < MESON_NUM_MUX_PARENTS; i++) { - mux_parent_data[i].index = -1; - mux_parent_data[i].name = meson->data->parent_names[i]; - } - - for (i = 0; i < chip->npwm; i++) { + for (i = 0; i < MESON_NUM_PWMS; i++) { struct meson_pwm_channel *channel = &meson->channels[i]; struct clk_parent_data div_parent = {}, gate_parent = {}; struct clk_init_data init = {}; @@ -495,6 +426,122 @@ static int meson_pwm_init_channels(struct pwm_chip *chip) return 0; } +static int meson_pwm_init_channels_meson8b_legacy(struct pwm_chip *chip) +{ + struct clk_parent_data mux_parent_data[MESON_NUM_MUX_PARENTS] = {}; + struct meson_pwm *meson = to_meson_pwm(chip); + int i; + + dev_warn_once(pwmchip_parent(chip), + "using obsolete compatible, please consider updating dt\n"); + + for (i = 0; i < MESON_NUM_MUX_PARENTS; i++) { + mux_parent_data[i].index = -1; + mux_parent_data[i].name = meson->data->parent_names[i]; + } + + return meson_pwm_init_clocks_meson8b(chip, mux_parent_data); +} + +static int meson_pwm_init_channels_meson8b_v2(struct pwm_chip *chip) +{ + struct clk_parent_data mux_parent_data[MESON_NUM_MUX_PARENTS] = {}; + int i; + + /* + * NOTE: Instead of relying on the hard coded names in the driver + * as the legacy version, this relies on DT to provide the list of + * clocks. + * For once, using input numbers actually makes more sense than names. + * Also DT requires clock-names to be explicitly ordered, so there is + * no point bothering with clock names in this case. + */ + for (i = 0; i < MESON_NUM_MUX_PARENTS; i++) + mux_parent_data[i].index = i; + + return meson_pwm_init_clocks_meson8b(chip, mux_parent_data); +} + +static const struct meson_pwm_data pwm_meson8b_data = { + .parent_names = { "xtal", NULL, "fclk_div4", "fclk_div3" }, + .channels_init = meson_pwm_init_channels_meson8b_legacy, +}; + +/* + * Only the 2 first inputs of the GXBB AO PWMs are valid + * The last 2 are grounded + */ +static const struct meson_pwm_data pwm_gxbb_ao_data = { + .parent_names = { "xtal", "clk81", NULL, NULL }, + .channels_init = meson_pwm_init_channels_meson8b_legacy, +}; + +static const struct meson_pwm_data pwm_axg_ee_data = { + .parent_names = { "xtal", "fclk_div5", "fclk_div4", "fclk_div3" }, + .channels_init = meson_pwm_init_channels_meson8b_legacy, +}; + +static const struct meson_pwm_data pwm_axg_ao_data = { + .parent_names = { "xtal", "axg_ao_clk81", "fclk_div4", "fclk_div5" }, + .channels_init = meson_pwm_init_channels_meson8b_legacy, +}; + +static const struct meson_pwm_data pwm_g12a_ao_ab_data = { + .parent_names = { "xtal", "g12a_ao_clk81", "fclk_div4", "fclk_div5" }, + .channels_init = meson_pwm_init_channels_meson8b_legacy, +}; + +static const struct meson_pwm_data pwm_g12a_ao_cd_data = { + .parent_names = { "xtal", "g12a_ao_clk81", NULL, NULL }, + .channels_init = meson_pwm_init_channels_meson8b_legacy, +}; + +static const struct meson_pwm_data pwm_meson8_v2_data = { + .channels_init = meson_pwm_init_channels_meson8b_v2, +}; + +static const struct of_device_id meson_pwm_matches[] = { + { + .compatible = "amlogic,meson8-pwm-v2", + .data = &pwm_meson8_v2_data + }, + /* The following compatibles are obsolete */ + { + .compatible = "amlogic,meson8b-pwm", + .data = &pwm_meson8b_data + }, + { + .compatible = "amlogic,meson-gxbb-pwm", + .data = &pwm_meson8b_data + }, + { + .compatible = "amlogic,meson-gxbb-ao-pwm", + .data = &pwm_gxbb_ao_data + }, + { + .compatible = "amlogic,meson-axg-ee-pwm", + .data = &pwm_axg_ee_data + }, + { + .compatible = "amlogic,meson-axg-ao-pwm", + .data = &pwm_axg_ao_data + }, + { + .compatible = "amlogic,meson-g12a-ee-pwm", + .data = &pwm_meson8b_data + }, + { + .compatible = "amlogic,meson-g12a-ao-pwm-ab", + .data = &pwm_g12a_ao_ab_data + }, + { + .compatible = "amlogic,meson-g12a-ao-pwm-cd", + .data = &pwm_g12a_ao_cd_data + }, + {}, +}; +MODULE_DEVICE_TABLE(of, meson_pwm_matches); + static int meson_pwm_probe(struct platform_device *pdev) { struct pwm_chip *chip; @@ -515,7 +562,7 @@ static int meson_pwm_probe(struct platform_device *pdev) meson->data = of_device_get_match_data(&pdev->dev); - err = meson_pwm_init_channels(chip); + err = meson->data->channels_init(chip); if (err < 0) return err; From b3c23dcc3c4617b4cefcfbeba47494a640706fcc Mon Sep 17 00:00:00 2001 From: Alexandre Mergnat Date: Thu, 18 Apr 2024 16:16:59 +0200 Subject: [PATCH 24/34] dt-bindings: pwm: mediatek,pwm-disp: add compatible for mt8365 SoC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a compatible string for MediaTek Genio 350 MT8365's display PWM block: this is the same as MT8183. Reviewed-by: AngeloGioacchino Del Regno Acked-by: Rob Herring (Arm) Signed-off-by: Alexandre Mergnat Acked-by: Krzysztof Kozlowski Link: https://lore.kernel.org/r/20231023-display-support-v3-11-53388f3ed34b@baylibre.com Signed-off-by: Uwe Kleine-König --- Documentation/devicetree/bindings/pwm/mediatek,pwm-disp.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/pwm/mediatek,pwm-disp.yaml b/Documentation/devicetree/bindings/pwm/mediatek,pwm-disp.yaml index bc813fe74fab..6b83513c9426 100644 --- a/Documentation/devicetree/bindings/pwm/mediatek,pwm-disp.yaml +++ b/Documentation/devicetree/bindings/pwm/mediatek,pwm-disp.yaml @@ -31,6 +31,7 @@ properties: - mediatek,mt8188-disp-pwm - mediatek,mt8192-disp-pwm - mediatek,mt8195-disp-pwm + - mediatek,mt8365-disp-pwm - const: mediatek,mt8183-disp-pwm reg: From 6b2d60a543c4ffbb6bd9703a2714b2d5fbfc5781 Mon Sep 17 00:00:00 2001 From: George Stark Date: Thu, 25 Apr 2024 20:12:51 +0300 Subject: [PATCH 25/34] pwm: meson: Drop unneeded check in .get_state() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drop checking state argument for NULL pointer in meson_pwm_get_state() due to it is called only from pwm core with always valid arguments. Signed-off-by: Dmitry Rokosov Signed-off-by: George Stark Link: https://lore.kernel.org/r/20240425171253.2752877-2-gnstark@salutedevices.com Signed-off-by: Uwe Kleine-König --- drivers/pwm/pwm-meson.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c index ea96c5973488..f4d70da621ec 100644 --- a/drivers/pwm/pwm-meson.c +++ b/drivers/pwm/pwm-meson.c @@ -311,9 +311,6 @@ static int meson_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm, struct meson_pwm_channel *channel; u32 value; - if (!state) - return 0; - channel = &meson->channels[pwm->hwpwm]; channel_data = &meson_pwm_per_channel_data[pwm->hwpwm]; From 3e551115aee079931b82e1ec78c05f3d5033473f Mon Sep 17 00:00:00 2001 From: George Stark Date: Thu, 25 Apr 2024 20:12:52 +0300 Subject: [PATCH 26/34] pwm: meson: Add check for error from clk_round_rate() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit clk_round_rate() can return not only zero if requested frequency can not be provided but also negative error code so add check for it too. Also change type of variable holding clk_round_rate() result from unsigned long to long. It's safe due to clk_round_rate() returns long. Fixes: 329db102a26d ("pwm: meson: make full use of common clock framework") Signed-off-by: Dmitry Rokosov Signed-off-by: George Stark Link: https://lore.kernel.org/r/20240425171253.2752877-3-gnstark@salutedevices.com Signed-off-by: Uwe Kleine-König --- drivers/pwm/pwm-meson.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c index f4d70da621ec..4a652d500dfc 100644 --- a/drivers/pwm/pwm-meson.c +++ b/drivers/pwm/pwm-meson.c @@ -148,7 +148,7 @@ static int meson_pwm_calc(struct pwm_chip *chip, struct pwm_device *pwm, struct meson_pwm *meson = to_meson_pwm(chip); struct meson_pwm_channel *channel = &meson->channels[pwm->hwpwm]; unsigned int cnt, duty_cnt; - unsigned long fin_freq; + long fin_freq; u64 duty, period, freq; duty = state->duty_cycle; @@ -168,12 +168,13 @@ static int meson_pwm_calc(struct pwm_chip *chip, struct pwm_device *pwm, freq = ULONG_MAX; fin_freq = clk_round_rate(channel->clk, freq); - if (fin_freq == 0) { - dev_err(pwmchip_parent(chip), "invalid source clock frequency\n"); - return -EINVAL; + if (fin_freq <= 0) { + dev_err(pwmchip_parent(chip), + "invalid source clock frequency %llu\n", freq); + return fin_freq ? fin_freq : -EINVAL; } - dev_dbg(pwmchip_parent(chip), "fin_freq: %lu Hz\n", fin_freq); + dev_dbg(pwmchip_parent(chip), "fin_freq: %ld Hz\n", fin_freq); cnt = div_u64(fin_freq * period, NSEC_PER_SEC); if (cnt > 0xffff) { From 32c44e1fa921aebf8a5ef9f778534a30aab39313 Mon Sep 17 00:00:00 2001 From: George Stark Date: Thu, 25 Apr 2024 20:12:53 +0300 Subject: [PATCH 27/34] pwm: meson: Use mul_u64_u64_div_u64() for frequency calculating MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While calculating frequency for the given period u64 numbers are multiplied before division what can lead to overflow in theory so use secure mul_u64_u64_div_u64() which handles overflow correctly. Fixes: 329db102a26d ("pwm: meson: make full use of common clock framework") Suggested-by: Uwe Kleine-König Signed-off-by: George Stark Link: https://lore.kernel.org/r/20240425171253.2752877-4-gnstark@salutedevices.com Signed-off-by: Uwe Kleine-König --- drivers/pwm/pwm-meson.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c index 4a652d500dfc..b2f97dfb01bb 100644 --- a/drivers/pwm/pwm-meson.c +++ b/drivers/pwm/pwm-meson.c @@ -176,7 +176,7 @@ static int meson_pwm_calc(struct pwm_chip *chip, struct pwm_device *pwm, dev_dbg(pwmchip_parent(chip), "fin_freq: %ld Hz\n", fin_freq); - cnt = div_u64(fin_freq * period, NSEC_PER_SEC); + cnt = mul_u64_u64_div_u64(fin_freq, period, NSEC_PER_SEC); if (cnt > 0xffff) { dev_err(pwmchip_parent(chip), "unable to get period cnt\n"); return -EINVAL; @@ -191,7 +191,7 @@ static int meson_pwm_calc(struct pwm_chip *chip, struct pwm_device *pwm, channel->hi = 0; channel->lo = cnt; } else { - duty_cnt = div_u64(fin_freq * duty, NSEC_PER_SEC); + duty_cnt = mul_u64_u64_div_u64(fin_freq, duty, NSEC_PER_SEC); dev_dbg(pwmchip_parent(chip), "duty=%llu duty_cnt=%u\n", duty, duty_cnt); From aa5be4c6a3a9a2eb8d1cadd1636c7bdddfcae063 Mon Sep 17 00:00:00 2001 From: Binbin Zhou Date: Tue, 30 Apr 2024 15:32:02 +0800 Subject: [PATCH 28/34] dt-bindings: pwm: bcm2835: Do not require pwm-cells twice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pwm-cells property is already required by pwm.yaml schema. Signed-off-by: Binbin Zhou Acked-by: Conor Dooley Link: https://lore.kernel.org/r/85d7dc606d60a7d0d1557b98ac3a600c660b7421.1714450308.git.zhoubinbin@loongson.cn Signed-off-by: Uwe Kleine-König --- Documentation/devicetree/bindings/pwm/pwm-bcm2835.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/Documentation/devicetree/bindings/pwm/pwm-bcm2835.yaml b/Documentation/devicetree/bindings/pwm/pwm-bcm2835.yaml index 15e7fd98defc..9dc25f38fb94 100644 --- a/Documentation/devicetree/bindings/pwm/pwm-bcm2835.yaml +++ b/Documentation/devicetree/bindings/pwm/pwm-bcm2835.yaml @@ -29,7 +29,6 @@ required: - compatible - reg - clocks - - "#pwm-cells" additionalProperties: false From fb91b5f41b4db2f5382f2b2a7196bf4fae8701f0 Mon Sep 17 00:00:00 2001 From: Binbin Zhou Date: Tue, 30 Apr 2024 15:32:03 +0800 Subject: [PATCH 29/34] dt-bindings: pwm: google,cros-ec: Do not require pwm-cells twice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pwm-cells property is already required by pwm.yaml schema. Signed-off-by: Binbin Zhou Acked-by: Conor Dooley Link: https://lore.kernel.org/r/231df058d9cb35cfcf4bcdf4385f4ad8cb21a046.1714450308.git.zhoubinbin@loongson.cn Signed-off-by: Uwe Kleine-König --- Documentation/devicetree/bindings/pwm/google,cros-ec-pwm.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/Documentation/devicetree/bindings/pwm/google,cros-ec-pwm.yaml b/Documentation/devicetree/bindings/pwm/google,cros-ec-pwm.yaml index 3afe1480df52..f7bc84b05a87 100644 --- a/Documentation/devicetree/bindings/pwm/google,cros-ec-pwm.yaml +++ b/Documentation/devicetree/bindings/pwm/google,cros-ec-pwm.yaml @@ -35,7 +35,6 @@ properties: required: - compatible - - '#pwm-cells' additionalProperties: false From b3d8d1205104167203210af56af966f4f4d26404 Mon Sep 17 00:00:00 2001 From: Binbin Zhou Date: Tue, 30 Apr 2024 15:32:04 +0800 Subject: [PATCH 30/34] dt-bindings: pwm: marvell,pxa: Do not require pwm-cells twice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pwm-cells property is already required by pwm.yaml schema. Signed-off-by: Binbin Zhou Acked-by: Conor Dooley Link: https://lore.kernel.org/r/06765e0dd9a842dc51ff9c9cea93f26b8792e44b.1714450308.git.zhoubinbin@loongson.cn Signed-off-by: Uwe Kleine-König --- Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml b/Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml index ba6325575ea0..9ee1946dc2e1 100644 --- a/Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml +++ b/Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml @@ -34,7 +34,6 @@ properties: required: - compatible - reg - - "#pwm-cells" - clocks additionalProperties: false From 488ab429e3af5bf5d9d3d651f0cb4b988531980a Mon Sep 17 00:00:00 2001 From: Binbin Zhou Date: Tue, 30 Apr 2024 15:32:34 +0800 Subject: [PATCH 31/34] dt-bindings: pwm: mediatek,mt2712: Do not require pwm-cells twice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pwm-cells property is already required by pwm.yaml schema. Signed-off-by: Binbin Zhou Acked-by: Conor Dooley Link: https://lore.kernel.org/r/0a0c13d6d716a52540e165e4835c0c406cc5d8ec.1714450308.git.zhoubinbin@loongson.cn Signed-off-by: Uwe Kleine-König --- Documentation/devicetree/bindings/pwm/mediatek,mt2712-pwm.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/Documentation/devicetree/bindings/pwm/mediatek,mt2712-pwm.yaml b/Documentation/devicetree/bindings/pwm/mediatek,mt2712-pwm.yaml index a5c308801619..d515c09e1021 100644 --- a/Documentation/devicetree/bindings/pwm/mediatek,mt2712-pwm.yaml +++ b/Documentation/devicetree/bindings/pwm/mediatek,mt2712-pwm.yaml @@ -66,7 +66,6 @@ properties: required: - compatible - reg - - "#pwm-cells" - clocks - clock-names From 70504411710e70967a740bf6a7dfa820cb6aee54 Mon Sep 17 00:00:00 2001 From: Binbin Zhou Date: Tue, 30 Apr 2024 15:32:35 +0800 Subject: [PATCH 32/34] dt-bindings: pwm: mediatek,pwm-disp: Do not require pwm-cells twice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pwm-cells property is already required by pwm.yaml schema. Signed-off-by: Binbin Zhou Acked-by: Conor Dooley Link: https://lore.kernel.org/r/480bcdc37958a9d99f4b61c26a13b844a1e416df.1714450308.git.zhoubinbin@loongson.cn Signed-off-by: Uwe Kleine-König --- Documentation/devicetree/bindings/pwm/mediatek,pwm-disp.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/Documentation/devicetree/bindings/pwm/mediatek,pwm-disp.yaml b/Documentation/devicetree/bindings/pwm/mediatek,pwm-disp.yaml index 6b83513c9426..195e4371196b 100644 --- a/Documentation/devicetree/bindings/pwm/mediatek,pwm-disp.yaml +++ b/Documentation/devicetree/bindings/pwm/mediatek,pwm-disp.yaml @@ -59,7 +59,6 @@ properties: required: - compatible - reg - - "#pwm-cells" - clocks - clock-names From b664fc60d7f8190627ac35797d552acb5cbde3e7 Mon Sep 17 00:00:00 2001 From: Binbin Zhou Date: Tue, 30 Apr 2024 15:32:36 +0800 Subject: [PATCH 33/34] dt-bindings: pwm: snps,dw-apb-timers: Do not require pwm-cells twice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pwm-cells property is already required by pwm.yaml schema. Signed-off-by: Binbin Zhou Acked-by: Conor Dooley Link: https://lore.kernel.org/r/68eaecf11db9bec7bdb90dbf38507332628a0434.1714450308.git.zhoubinbin@loongson.cn Signed-off-by: Uwe Kleine-König --- .../devicetree/bindings/pwm/snps,dw-apb-timers-pwm2.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/Documentation/devicetree/bindings/pwm/snps,dw-apb-timers-pwm2.yaml b/Documentation/devicetree/bindings/pwm/snps,dw-apb-timers-pwm2.yaml index 4d0b5964443d..7523a89a1773 100644 --- a/Documentation/devicetree/bindings/pwm/snps,dw-apb-timers-pwm2.yaml +++ b/Documentation/devicetree/bindings/pwm/snps,dw-apb-timers-pwm2.yaml @@ -51,7 +51,6 @@ properties: required: - compatible - reg - - "#pwm-cells" - clocks - clock-names From 4817118f257e49b043f3d80f021a327b7e1d796f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Wed, 8 May 2024 15:06:18 +0200 Subject: [PATCH 34/34] pwm: pca9685: Drop explicit initialization of struct i2c_device_id::driver_data to 0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The driver doesn't use the driver_data member of struct i2c_device_id, so don't explicitly initialize this member. This prepares putting driver_data in an anonymous union which requires either no initialization or named designators. But it's also a nice cleanup on its own. While add it, also remove the trailing commas after the sentinel entry. Link: https://lore.kernel.org/r/20240508130618.2148631-2-u.kleine-koenig@pengutronix.de Signed-off-by: Uwe Kleine-König --- 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 c5da2a6ed846..1298b29183e5 100644 --- a/drivers/pwm/pwm-pca9685.c +++ b/drivers/pwm/pwm-pca9685.c @@ -634,8 +634,8 @@ static int __maybe_unused pca9685_pwm_runtime_resume(struct device *dev) } static const struct i2c_device_id pca9685_id[] = { - { "pca9685", 0 }, - { /* sentinel */ }, + { "pca9685" }, + { /* sentinel */ } }; MODULE_DEVICE_TABLE(i2c, pca9685_id);