[PATCH 0/8] pwm: Get rid of pwm_[sg]et_chip_data()

[PATCH 0/8] pwm: Get rid of pwm_[sg]et_chip_data()

[Uwe Kleine-König]

Hello,

the semantic of pwm_[sg]et_chip_data() overlaps with that of the
standard dev_[sg]et_drvdata() functions. Also as noted by George Stark
there is a problem in pwm-sti that relies on chipdata being available
even after a pwm_put (which clears chipdata).

To improve the situation this series converts all drivers that make use
of pwm chipdata to use well-known driver data instead and drop chipdata
support from the pwm framework.

Best regards
Uwe

Uwe Kleine-König (8):
  pwm: berlin: Put channel config into driver data
  pwm: samsung: Put channel data into driver data
  pwm: jz4740: Put per-channel clk into driver data
  pwm: lp3943: Drop usage of pwm_[gs]et_chip_data()
  pwm: renesas: Drop usage of pwm_[gs]et_chip_data()
  pwm: sti: Reduce number of allocations and drop usage of chip_data
  pwm: cros-ec: Put per channel data into driver data
  pwm: Drop pwm_[sg]et_chip_data()

 drivers/pwm/core.c            | 31 -----------------------------
 drivers/pwm/pwm-berlin.c      | 37 ++++++-----------------------------
 drivers/pwm/pwm-cros-ec.c     | 31 +++++++----------------------
 drivers/pwm/pwm-jz4740.c      | 11 +++++++----
 drivers/pwm/pwm-lp3943.c      | 21 +++++++-------------
 drivers/pwm/pwm-renesas-tpu.c | 22 ++++++++++-----------
 drivers/pwm/pwm-samsung.c     | 20 +++++--------------
 drivers/pwm/pwm-sti.c         | 29 +++++++++++++--------------
 include/linux/pwm.h           | 14 -------------
 9 files changed, 57 insertions(+), 159 deletions(-)


base-commit: 92554cdd428fce212d2a71a06939e7cab90f7c77
-- 
2.39.2

[PATCH 1/8] pwm: berlin: Put channel config into driver data

[Uwe Kleine-König]

Instead of allocating extra data in .request() provide the needed memory
in struct berlin_pwm_chip. This reduces the number of allocations. A
side effect is that on suspend and resume the state for all four
channels is always saved and restored. This is easier (and probably
quicker) than looking up the matching pwm_device and checking its
PWMF_REQUESTED bit.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/pwm/pwm-berlin.c | 37 ++++++-------------------------------
 1 file changed, 6 insertions(+), 31 deletions(-)

diff --git a/drivers/pwm/pwm-berlin.c b/drivers/pwm/pwm-berlin.c
index 0c5992a046b2..38dcd0c99efd 100644
--- a/drivers/pwm/pwm-berlin.c
+++ b/drivers/pwm/pwm-berlin.c
@@ -38,6 +38,8 @@
 #define BERLIN_PWM_TCNT			0xc
 #define  BERLIN_PWM_MAX_TCNT		65535
 
+#define BERLIN_PWM_NUMPWMS		4
+
 struct berlin_pwm_channel {
 	u32 enable;
 	u32 ctrl;
@@ -49,6 +51,7 @@ struct berlin_pwm_chip {
 	struct pwm_chip chip;
 	struct clk *clk;
 	void __iomem *base;
+	struct berlin_pwm_channel channel[BERLIN_PWM_NUMPWMS];
 };
 
 static inline struct berlin_pwm_chip *to_berlin_pwm_chip(struct pwm_chip *chip)
@@ -69,24 +72,6 @@ static inline void berlin_pwm_writel(struct berlin_pwm_chip *bpc,
 	writel_relaxed(value, bpc->base + channel * 0x10 + offset);
 }
 
-static int berlin_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
-{
-	struct berlin_pwm_channel *channel;
-
-	channel = kzalloc(sizeof(*channel), GFP_KERNEL);
-	if (!channel)
-		return -ENOMEM;
-
-	return pwm_set_chip_data(pwm, channel);
-}
-
-static void berlin_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
-{
-	struct berlin_pwm_channel *channel = pwm_get_chip_data(pwm);
-
-	kfree(channel);
-}
-
 static int berlin_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 			     u64 duty_ns, u64 period_ns)
 {
@@ -201,8 +186,6 @@ static int berlin_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 }
 
 static const struct pwm_ops berlin_pwm_ops = {
-	.request = berlin_pwm_request,
-	.free = berlin_pwm_free,
 	.apply = berlin_pwm_apply,
 	.owner = THIS_MODULE,
 };
@@ -236,7 +219,7 @@ static int berlin_pwm_probe(struct platform_device *pdev)
 
 	bpc->chip.dev = &pdev->dev;
 	bpc->chip.ops = &berlin_pwm_ops;
-	bpc->chip.npwm = 4;
+	bpc->chip.npwm = BERLIN_PWM_NUMPWMS;
 
 	ret = pwmchip_add(&bpc->chip);
 	if (ret < 0) {
@@ -266,11 +249,7 @@ static int berlin_pwm_suspend(struct device *dev)
 	unsigned int i;
 
 	for (i = 0; i < bpc->chip.npwm; i++) {
-		struct berlin_pwm_channel *channel;
-
-		channel = pwm_get_chip_data(&bpc->chip.pwms[i]);
-		if (!channel)
-			continue;
+		struct berlin_pwm_channel *channel = &bpc->channel[i];
 
 		channel->enable = berlin_pwm_readl(bpc, i, BERLIN_PWM_ENABLE);
 		channel->ctrl = berlin_pwm_readl(bpc, i, BERLIN_PWM_CONTROL);
@@ -294,11 +273,7 @@ static int berlin_pwm_resume(struct device *dev)
 		return ret;
 
 	for (i = 0; i < bpc->chip.npwm; i++) {
-		struct berlin_pwm_channel *channel;
-
-		channel = pwm_get_chip_data(&bpc->chip.pwms[i]);
-		if (!channel)
-			continue;
+		struct berlin_pwm_channel *channel = &bpc->channel[i];
 
 		berlin_pwm_writel(bpc, i, channel->ctrl, BERLIN_PWM_CONTROL);
 		berlin_pwm_writel(bpc, i, channel->duty, BERLIN_PWM_DUTY);
-- 
2.39.2

[PATCH 2/8] pwm: samsung: Put channel data into driver data

[Uwe Kleine-König]

Instead of allocating extra data in .request() provide the needed memory
in struct samsung_pwm_chip. This reduces the number of allocations. Even
though now all 5 channel structs are allocated this is probably
outweighed by the reduced overhead to track up to 6 smaller allocations.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/pwm/pwm-samsung.c | 20 +++++---------------
 1 file changed, 5 insertions(+), 15 deletions(-)

diff --git a/drivers/pwm/pwm-samsung.c b/drivers/pwm/pwm-samsung.c
index e8828f57ab15..d1a2bc01071b 100644
--- a/drivers/pwm/pwm-samsung.c
+++ b/drivers/pwm/pwm-samsung.c
@@ -88,6 +88,7 @@ struct samsung_pwm_chip {
 	struct clk *base_clk;
 	struct clk *tclk0;
 	struct clk *tclk1;
+	struct samsung_pwm_channel channel[SAMSUNG_PWM_NUM];
 };
 
 #ifndef CONFIG_CLKSRC_SAMSUNG_PWM
@@ -228,7 +229,6 @@ static unsigned long pwm_samsung_calc_tin(struct samsung_pwm_chip *chip,
 static int pwm_samsung_request(struct pwm_chip *chip, struct pwm_device *pwm)
 {
 	struct samsung_pwm_chip *our_chip = to_samsung_pwm_chip(chip);
-	struct samsung_pwm_channel *our_chan;
 
 	if (!(our_chip->variant.output_mask & BIT(pwm->hwpwm))) {
 		dev_warn(chip->dev,
@@ -237,20 +237,11 @@ static int pwm_samsung_request(struct pwm_chip *chip, struct pwm_device *pwm)
 		return -EINVAL;
 	}
 
-	our_chan = kzalloc(sizeof(*our_chan), GFP_KERNEL);
-	if (!our_chan)
-		return -ENOMEM;
-
-	pwm_set_chip_data(pwm, our_chan);
+	memset(&our_chip->channel[pwm->hwpwm], 0, sizeof(our_chip->channel[pwm->hwpwm]));
 
 	return 0;
 }
 
-static void pwm_samsung_free(struct pwm_chip *chip, struct pwm_device *pwm)
-{
-	kfree(pwm_get_chip_data(pwm));
-}
-
 static int pwm_samsung_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 {
 	struct samsung_pwm_chip *our_chip = to_samsung_pwm_chip(chip);
@@ -318,7 +309,7 @@ static int __pwm_samsung_config(struct pwm_chip *chip, struct pwm_device *pwm,
 				int duty_ns, int period_ns, bool force_period)
 {
 	struct samsung_pwm_chip *our_chip = to_samsung_pwm_chip(chip);
-	struct samsung_pwm_channel *chan = pwm_get_chip_data(pwm);
+	struct samsung_pwm_channel *chan = &our_chip->channel[pwm->hwpwm];
 	u32 tin_ns = chan->tin_ns, tcnt, tcmp, oldtcmp;
 
 	tcnt = readl(our_chip->base + REG_TCNTB(pwm->hwpwm));
@@ -473,7 +464,6 @@ static int pwm_samsung_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 
 static const struct pwm_ops pwm_samsung_ops = {
 	.request	= pwm_samsung_request,
-	.free		= pwm_samsung_free,
 	.apply		= pwm_samsung_apply,
 	.owner		= THIS_MODULE,
 };
@@ -639,9 +629,9 @@ static int pwm_samsung_resume(struct device *dev)
 
 	for (i = 0; i < SAMSUNG_PWM_NUM; i++) {
 		struct pwm_device *pwm = &chip->pwms[i];
-		struct samsung_pwm_channel *chan = pwm_get_chip_data(pwm);
+		struct samsung_pwm_channel *chan = &our_chip->channel[i];
 
-		if (!chan)
+		if (!(pwm->flags & PWMF_REQUESTED))
 			continue;
 
 		if (our_chip->variant.output_mask & BIT(i))
-- 
2.39.2

[PATCH 3/8] pwm: jz4740: Put per-channel clk into driver data

[Uwe Kleine-König]

Stop using chip_data which is about to go away. Instead track the
per-channel clk in struct jz4740_pwm_chip.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/pwm/pwm-jz4740.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/pwm/pwm-jz4740.c b/drivers/pwm/pwm-jz4740.c
index 3b7067f6cd0d..e0a57d71a60c 100644
--- a/drivers/pwm/pwm-jz4740.c
+++ b/drivers/pwm/pwm-jz4740.c
@@ -27,6 +27,7 @@ struct soc_info {
 struct jz4740_pwm_chip {
 	struct pwm_chip chip;
 	struct regmap *map;
+	struct clk *clk[];
 };
 
 static inline struct jz4740_pwm_chip *to_jz4740(struct pwm_chip *chip)
@@ -70,14 +71,15 @@ static int jz4740_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
 		return err;
 	}
 
-	pwm_set_chip_data(pwm, clk);
+	jz->clk[pwm->hwpwm] = clk;
 
 	return 0;
 }
 
 static void jz4740_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
 {
-	struct clk *clk = pwm_get_chip_data(pwm);
+	struct jz4740_pwm_chip *jz = to_jz4740(chip);
+	struct clk *clk = jz->clk[pwm->hwpwm];
 
 	clk_disable_unprepare(clk);
 	clk_put(clk);
@@ -123,7 +125,7 @@ static int jz4740_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 {
 	struct jz4740_pwm_chip *jz4740 = to_jz4740(pwm->chip);
 	unsigned long long tmp = 0xffffull * NSEC_PER_SEC;
-	struct clk *clk = pwm_get_chip_data(pwm);
+	struct clk *clk = jz4740->clk[pwm->hwpwm];
 	unsigned long period, duty;
 	long rate;
 	int err;
@@ -229,7 +231,8 @@ static int jz4740_pwm_probe(struct platform_device *pdev)
 	if (!info)
 		return -EINVAL;
 
-	jz4740 = devm_kzalloc(dev, sizeof(*jz4740), GFP_KERNEL);
+	jz4740 = devm_kzalloc(dev, sizeof(*jz4740) + info->num_pwms * sizeof(jz4740->clk[0]),
+			      GFP_KERNEL);
 	if (!jz4740)
 		return -ENOMEM;
 
-- 
2.39.2

Re: [PATCH 3/8] pwm: jz4740: Put per-channel clk into driver data

[Philippe Mathieu-Daudé]

On 29/6/23 11:48, Uwe Kleine-König wrote:
> Stop using chip_data which is about to go away. Instead track the
> per-channel clk in struct jz4740_pwm_chip.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>   drivers/pwm/pwm-jz4740.c | 11 +++++++----
>   1 file changed, 7 insertions(+), 4 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH 4/8] pwm: lp3943: Drop usage of pwm_[gs]et_chip_data()

[Uwe Kleine-König]

Instead of distributing the driver's bookkeeping over 3 (i.e.
LP3943_NUM_PWMS + 1) separately allocated memory chunks, put all
together in struct lp3943_pwm. This reduces the number of memory
allocations and so fragmentation and maybe even the number of cache
misses. Also &lp3943_pwm->pwm_map[pwm->hwpwm] is cheaper to evaluate
than pwm_get_chip_data(pwm) as the former is just an addition in machine
code while the latter involves a function call.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/pwm/pwm-lp3943.c | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/drivers/pwm/pwm-lp3943.c b/drivers/pwm/pwm-lp3943.c
index 35675e4058c6..00e7f63a73fe 100644
--- a/drivers/pwm/pwm-lp3943.c
+++ b/drivers/pwm/pwm-lp3943.c
@@ -22,6 +22,7 @@ struct lp3943_pwm {
 	struct pwm_chip chip;
 	struct lp3943 *lp3943;
 	struct lp3943_platform_data *pdata;
+	struct lp3943_pwm_map pwm_map[LP3943_NUM_PWMS];
 };
 
 static inline struct lp3943_pwm *to_lp3943_pwm(struct pwm_chip *_chip)
@@ -34,13 +35,9 @@ lp3943_pwm_request_map(struct lp3943_pwm *lp3943_pwm, int hwpwm)
 {
 	struct lp3943_platform_data *pdata = lp3943_pwm->pdata;
 	struct lp3943 *lp3943 = lp3943_pwm->lp3943;
-	struct lp3943_pwm_map *pwm_map;
+	struct lp3943_pwm_map *pwm_map = &lp3943_pwm->pwm_map[hwpwm];
 	int i, offset;
 
-	pwm_map = kzalloc(sizeof(*pwm_map), GFP_KERNEL);
-	if (!pwm_map)
-		return ERR_PTR(-ENOMEM);
-
 	pwm_map->output = pdata->pwms[hwpwm]->output;
 	pwm_map->num_outputs = pdata->pwms[hwpwm]->num_outputs;
 
@@ -48,10 +45,8 @@ lp3943_pwm_request_map(struct lp3943_pwm *lp3943_pwm, int hwpwm)
 		offset = pwm_map->output[i];
 
 		/* Return an error if the pin is already assigned */
-		if (test_and_set_bit(offset, &lp3943->pin_used)) {
-			kfree(pwm_map);
+		if (test_and_set_bit(offset, &lp3943->pin_used))
 			return ERR_PTR(-EBUSY);
-		}
 	}
 
 	return pwm_map;
@@ -66,7 +61,7 @@ static int lp3943_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
 	if (IS_ERR(pwm_map))
 		return PTR_ERR(pwm_map);
 
-	return pwm_set_chip_data(pwm, pwm_map);
+	return 0;
 }
 
 static void lp3943_pwm_free_map(struct lp3943_pwm *lp3943_pwm,
@@ -79,14 +74,12 @@ static void lp3943_pwm_free_map(struct lp3943_pwm *lp3943_pwm,
 		offset = pwm_map->output[i];
 		clear_bit(offset, &lp3943->pin_used);
 	}
-
-	kfree(pwm_map);
 }
 
 static void lp3943_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
 {
 	struct lp3943_pwm *lp3943_pwm = to_lp3943_pwm(chip);
-	struct lp3943_pwm_map *pwm_map = pwm_get_chip_data(pwm);
+	struct lp3943_pwm_map *pwm_map = &lp3943_pwm->pwm_map[pwm->hwpwm];
 
 	lp3943_pwm_free_map(lp3943_pwm, pwm_map);
 }
@@ -158,7 +151,7 @@ static int lp3943_pwm_set_mode(struct lp3943_pwm *lp3943_pwm,
 static int lp3943_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 {
 	struct lp3943_pwm *lp3943_pwm = to_lp3943_pwm(chip);
-	struct lp3943_pwm_map *pwm_map = pwm_get_chip_data(pwm);
+	struct lp3943_pwm_map *pwm_map = &lp3943_pwm->pwm_map[pwm->hwpwm];
 	u8 val;
 
 	if (pwm->hwpwm == 0)
@@ -177,7 +170,7 @@ static int lp3943_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 static void lp3943_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
 {
 	struct lp3943_pwm *lp3943_pwm = to_lp3943_pwm(chip);
-	struct lp3943_pwm_map *pwm_map = pwm_get_chip_data(pwm);
+	struct lp3943_pwm_map *pwm_map = &lp3943_pwm->pwm_map[pwm->hwpwm];
 
 	/*
 	 * LP3943 outputs are open-drain, so the pin should be configured
-- 
2.39.2

[PATCH 5/8] pwm: renesas: Drop usage of pwm_[gs]et_chip_data()

[Uwe Kleine-König]

Instead of distributing the driver's bookkeeping over 5 (i.e.
TPU_CHANNEL_MAX + 1) separately allocated memory chunks, put all
together in struct tpu_device. This reduces the number of memory
allocations and so fragmentation and maybe even the number of cache
misses. Also &tpu->tpd[pwm->hwpwm] is cheaper to evaluate than
pwm_get_chip_data(pwm) as the former is just an addition in machine code
while the latter involves a function call.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/pwm/pwm-renesas-tpu.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/pwm/pwm-renesas-tpu.c b/drivers/pwm/pwm-renesas-tpu.c
index d7311614c846..613dd549edf8 100644
--- a/drivers/pwm/pwm-renesas-tpu.c
+++ b/drivers/pwm/pwm-renesas-tpu.c
@@ -85,6 +85,7 @@ struct tpu_device {
 
 	void __iomem *base;
 	struct clk *clk;
+	struct tpu_pwm_device tpd[TPU_CHANNEL_MAX];
 };
 
 #define to_tpu_device(c)	container_of(c, struct tpu_device, chip)
@@ -215,9 +216,7 @@ static int tpu_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
 	if (pwm->hwpwm >= TPU_CHANNEL_MAX)
 		return -EINVAL;
 
-	tpd = kzalloc(sizeof(*tpd), GFP_KERNEL);
-	if (tpd == NULL)
-		return -ENOMEM;
+	tpd = &tpu->tpd[pwm->hwpwm];
 
 	tpd->tpu = tpu;
 	tpd->channel = pwm->hwpwm;
@@ -228,24 +227,22 @@ static int tpu_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
 
 	tpd->timer_on = false;
 
-	pwm_set_chip_data(pwm, tpd);
-
 	return 0;
 }
 
 static void tpu_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
 {
-	struct tpu_pwm_device *tpd = pwm_get_chip_data(pwm);
+	struct tpu_device *tpu = to_tpu_device(chip);
+	struct tpu_pwm_device *tpd = &tpu->tpd[pwm->hwpwm];
 
 	tpu_pwm_timer_stop(tpd);
-	kfree(tpd);
 }
 
 static int tpu_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 			  u64 duty_ns, u64 period_ns, bool enabled)
 {
-	struct tpu_pwm_device *tpd = pwm_get_chip_data(pwm);
 	struct tpu_device *tpu = to_tpu_device(chip);
+	struct tpu_pwm_device *tpd = &tpu->tpd[pwm->hwpwm];
 	unsigned int prescaler;
 	bool duty_only = false;
 	u32 clk_rate;
@@ -353,7 +350,8 @@ static int tpu_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 static int tpu_pwm_set_polarity(struct pwm_chip *chip, struct pwm_device *pwm,
 				enum pwm_polarity polarity)
 {
-	struct tpu_pwm_device *tpd = pwm_get_chip_data(pwm);
+	struct tpu_device *tpu = to_tpu_device(chip);
+	struct tpu_pwm_device *tpd = &tpu->tpd[pwm->hwpwm];
 
 	tpd->polarity = polarity;
 
@@ -362,7 +360,8 @@ static int tpu_pwm_set_polarity(struct pwm_chip *chip, struct pwm_device *pwm,
 
 static int tpu_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 {
-	struct tpu_pwm_device *tpd = pwm_get_chip_data(pwm);
+	struct tpu_device *tpu = to_tpu_device(chip);
+	struct tpu_pwm_device *tpd = &tpu->tpd[pwm->hwpwm];
 	int ret;
 
 	ret = tpu_pwm_timer_start(tpd);
@@ -384,7 +383,8 @@ static int tpu_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 
 static void tpu_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
 {
-	struct tpu_pwm_device *tpd = pwm_get_chip_data(pwm);
+	struct tpu_device *tpu = to_tpu_device(chip);
+	struct tpu_pwm_device *tpd = &tpu->tpd[pwm->hwpwm];
 
 	/* The timer must be running to modify the pin output configuration. */
 	tpu_pwm_timer_start(tpd);
-- 
2.39.2

[PATCH 6/8] pwm: sti: Reduce number of allocations and drop usage of chip_data

[Uwe Kleine-König]

Instead of using one allocation per capture channel, use a single one
for the whole chip. Also store it in driver data instead of chip data.

This has several advantages:

 - driver data isn't cleared when pwm_put() is called
 - Reduces memory fragmentation

Also register the pwm chip only after the per capture channel data is
initialized as the capture callback relies on this initialization and it
might be called even before pwmchip_add() returns.

It would be still better to have struct sti_pwm_compat_data and the
per-channel data struct sti_cpt_ddata in a single memory chunk, but
that's not easily possible because the number of capture channels isn't
known yet when the driver data struct is allocated.

Fixes: e926b12c611c ("pwm: Clear chip_data in pwm_put()")
Reported-by: George Stark <gnstark@sberdevices.ru>
Fixes: c97267ae831d ("pwm: sti: Add PWM capture callback")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/pwm/pwm-sti.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/pwm/pwm-sti.c b/drivers/pwm/pwm-sti.c
index b1d1373648a3..c8800f84b917 100644
--- a/drivers/pwm/pwm-sti.c
+++ b/drivers/pwm/pwm-sti.c
@@ -79,6 +79,7 @@ struct sti_pwm_compat_data {
 	unsigned int cpt_num_devs;
 	unsigned int max_pwm_cnt;
 	unsigned int max_prescale;
+	struct sti_cpt_ddata *ddata;
 };
 
 struct sti_pwm_chip {
@@ -314,7 +315,7 @@ static int sti_pwm_capture(struct pwm_chip *chip, struct pwm_device *pwm,
 {
 	struct sti_pwm_chip *pc = to_sti_pwmchip(chip);
 	struct sti_pwm_compat_data *cdata = pc->cdata;
-	struct sti_cpt_ddata *ddata = pwm_get_chip_data(pwm);
+	struct sti_cpt_ddata *ddata = &cdata->ddata[pwm->hwpwm];
 	struct device *dev = pc->dev;
 	unsigned int effective_ticks;
 	unsigned long long high, low;
@@ -440,7 +441,7 @@ static irqreturn_t sti_pwm_interrupt(int irq, void *data)
 	while (cpt_int_stat) {
 		devicenum = ffs(cpt_int_stat) - 1;
 
-		ddata = pwm_get_chip_data(&pc->chip.pwms[devicenum]);
+		ddata = &pc->cdata->ddata[devicenum];
 
 		/*
 		 * Capture input:
@@ -638,12 +639,23 @@ static int sti_pwm_probe(struct platform_device *pdev)
 			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;
 	}
 
 	pc->chip.dev = dev;
 	pc->chip.ops = &sti_pwm_ops;
 	pc->chip.npwm = pc->cdata->pwm_num_devs;
 
+	for (i = 0; i < cdata->cpt_num_devs; i++) {
+		struct sti_cpt_ddata *ddata = &cdata->ddata[i];
+
+		init_waitqueue_head(&ddata->wait);
+		mutex_init(&ddata->lock);
+	}
+
 	ret = pwmchip_add(&pc->chip);
 	if (ret < 0) {
 		clk_unprepare(pc->pwm_clk);
@@ -651,19 +663,6 @@ static int sti_pwm_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	for (i = 0; i < cdata->cpt_num_devs; i++) {
-		struct sti_cpt_ddata *ddata;
-
-		ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
-		if (!ddata)
-			return -ENOMEM;
-
-		init_waitqueue_head(&ddata->wait);
-		mutex_init(&ddata->lock);
-
-		pwm_set_chip_data(&pc->chip.pwms[i], ddata);
-	}
-
 	platform_set_drvdata(pdev, pc);
 
 	return 0;
-- 
2.39.2

[PATCH 7/8] pwm: cros-ec: Put per channel data into driver data

[Uwe Kleine-König]

Instead of an allocation of a single u16 per channel, allocate them all
in a single chunk which greatly reduces memory fragmentation and also
the overhead to track the allocated memory. Also put the channel data in
driver data where it's cheaper to determine the address (no function
call involved, just a trivial pointer addition).

This also allows to get rid of the request and free callbacks.

The only cost is that the channel data is allocated early, and even for
unused channels.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
If we'd know the number of channels already when memory is allocated for
struct cros_ec_pwm_device *ec_pwm in cros_ec_pwm_probe, the per channel
data could live directly in struct cros_ec_pwm_device which would
further reduce the memory allocation overhead and fragmentation.

 drivers/pwm/pwm-cros-ec.c | 31 +++++++------------------------
 1 file changed, 7 insertions(+), 24 deletions(-)

diff --git a/drivers/pwm/pwm-cros-ec.c b/drivers/pwm/pwm-cros-ec.c
index 74e863aa1d8d..2998153a5e92 100644
--- a/drivers/pwm/pwm-cros-ec.c
+++ b/drivers/pwm/pwm-cros-ec.c
@@ -27,6 +27,7 @@ struct cros_ec_pwm_device {
 	struct cros_ec_device *ec;
 	struct pwm_chip chip;
 	bool use_pwm_type;
+	struct cros_ec_pwm *channel;
 };
 
 /**
@@ -42,26 +43,6 @@ static inline struct cros_ec_pwm_device *pwm_to_cros_ec_pwm(struct pwm_chip *c)
 	return container_of(c, struct cros_ec_pwm_device, chip);
 }
 
-static int cros_ec_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
-{
-	struct cros_ec_pwm *channel;
-
-	channel = kzalloc(sizeof(*channel), GFP_KERNEL);
-	if (!channel)
-		return -ENOMEM;
-
-	pwm_set_chip_data(pwm, channel);
-
-	return 0;
-}
-
-static void cros_ec_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
-{
-	struct cros_ec_pwm *channel = pwm_get_chip_data(pwm);
-
-	kfree(channel);
-}
-
 static int cros_ec_dt_type_to_pwm_type(u8 dt_index, u8 *pwm_type)
 {
 	switch (dt_index) {
@@ -157,7 +138,7 @@ static int cros_ec_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 			     const struct pwm_state *state)
 {
 	struct cros_ec_pwm_device *ec_pwm = pwm_to_cros_ec_pwm(chip);
-	struct cros_ec_pwm *channel = pwm_get_chip_data(pwm);
+	struct cros_ec_pwm *channel = &ec_pwm->channel[pwm->hwpwm];
 	u16 duty_cycle;
 	int ret;
 
@@ -187,7 +168,7 @@ static int cros_ec_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
 				 struct pwm_state *state)
 {
 	struct cros_ec_pwm_device *ec_pwm = pwm_to_cros_ec_pwm(chip);
-	struct cros_ec_pwm *channel = pwm_get_chip_data(pwm);
+	struct cros_ec_pwm *channel = &ec_pwm->channel[pwm->hwpwm];
 	int ret;
 
 	ret = cros_ec_pwm_get_duty(ec_pwm, pwm->hwpwm);
@@ -236,8 +217,6 @@ cros_ec_pwm_xlate(struct pwm_chip *pc, const struct of_phandle_args *args)
 }
 
 static const struct pwm_ops cros_ec_pwm_ops = {
-	.request = cros_ec_pwm_request,
-	.free = cros_ec_pwm_free,
 	.get_state	= cros_ec_pwm_get_state,
 	.apply		= cros_ec_pwm_apply,
 	.owner		= THIS_MODULE,
@@ -316,6 +295,10 @@ static int cros_ec_pwm_probe(struct platform_device *pdev)
 		chip->npwm = ret;
 	}
 
+	ec_pwm->channel = devm_kzalloc(dev, chip->npwm * sizeof(*ec_pwm->channel), GFP_KERNEL);
+	if (!ec_pwm->channel)
+		return -ENOMEM;
+
 	dev_dbg(dev, "Probed %u PWMs\n", chip->npwm);
 
 	ret = pwmchip_add(chip);
-- 
2.39.2

Re: [PATCH 7/8] pwm: cros-ec: Put per channel data into driver data

[Tzung-Bi Shih]

On Thu, Jun 29, 2023 at 11:48:38AM +0200, Uwe Kleine-König wrote:
> Instead of an allocation of a single u16 per channel, allocate them all
> in a single chunk which greatly reduces memory fragmentation and also
> the overhead to track the allocated memory. Also put the channel data in
> driver data where it's cheaper to determine the address (no function
> call involved, just a trivial pointer addition).
> 
> This also allows to get rid of the request and free callbacks.
> 
> The only cost is that the channel data is allocated early, and even for
> unused channels.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

With comments addressed below:
Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>

> @@ -27,6 +27,7 @@ struct cros_ec_pwm_device {
>  	struct cros_ec_device *ec;
>  	struct pwm_chip chip;
>  	bool use_pwm_type;
> +	struct cros_ec_pwm *channel;

Please update the kernel-doc too.  Otherwise:
$ ./scripts/kernel-doc -none drivers/pwm/pwm-cros-ec.c
drivers/pwm/pwm-cros-ec.c:31: warning: Function parameter or member 'channel'
not described in 'cros_ec_pwm_device'

I have no strong preference: please consider to use `channels` if it makes
sense.

Re: [PATCH 7/8] pwm: cros-ec: Put per channel data into driver data

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 3121 bytes --]

On Fri, Jun 30, 2023 at 12:54:37PM +0800, Tzung-Bi Shih wrote:
> On Thu, Jun 29, 2023 at 11:48:38AM +0200, Uwe Kleine-König wrote:
> > Instead of an allocation of a single u16 per channel, allocate them all
> > in a single chunk which greatly reduces memory fragmentation and also
> > the overhead to track the allocated memory. Also put the channel data in
> > driver data where it's cheaper to determine the address (no function
> > call involved, just a trivial pointer addition).
> > 
> > This also allows to get rid of the request and free callbacks.
> > 
> > The only cost is that the channel data is allocated early, and even for
> > unused channels.
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> 
> With comments addressed below:
> Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>
> 
> > @@ -27,6 +27,7 @@ struct cros_ec_pwm_device {
> >  	struct cros_ec_device *ec;
> >  	struct pwm_chip chip;
> >  	bool use_pwm_type;
> > +	struct cros_ec_pwm *channel;
> 
> Please update the kernel-doc too.  Otherwise:
> $ ./scripts/kernel-doc -none drivers/pwm/pwm-cros-ec.c
> drivers/pwm/pwm-cros-ec.c:31: warning: Function parameter or member 'channel'
> not described in 'cros_ec_pwm_device'
> 
> I have no strong preference: please consider to use `channels` if it makes
> sense.

Thanks for your feedback. Regarding your suggestion about channel vs
channels: While I don't have a strong preference either you have to
choose between

	struct cros_ec_pwm *channel;

which is singular but for all channels, or you have

	ec_pwm->channesl[pwm->hwpwm]

which is plural but for only a single channel. So no matter how you do
it, it's strange at one place or the other.

While looking at the patch again I changed to

	ec_pwm->channel = devm_kcalloc(dev, chip->npwm, sizeof(*ec_pwm->channel), GFP_KERNEL)

(from devm_kzalloc with size chip->npwm * sizeof(*ec_pwm->channel)).

So in my tree I have the following change on top of the submission which
I will send out as a v2 soon:

diff --git a/drivers/pwm/pwm-cros-ec.c b/drivers/pwm/pwm-cros-ec.c
index 2998153a5e92..63c64c4dbf90 100644
--- a/drivers/pwm/pwm-cros-ec.c
+++ b/drivers/pwm/pwm-cros-ec.c
@@ -21,6 +21,7 @@
  * @ec: Pointer to EC device
  * @chip: PWM controller chip
  * @use_pwm_type: Use PWM types instead of generic channels
+ * @channel: array with per-channel data
  */
 struct cros_ec_pwm_device {
 	struct device *dev;
@@ -295,7 +296,8 @@ static int cros_ec_pwm_probe(struct platform_device *pdev)
 		chip->npwm = ret;
 	}
 
-	ec_pwm->channel = devm_kzalloc(dev, chip->npwm * sizeof(*ec_pwm->channel), GFP_KERNEL);
+	ec_pwm->channel = devm_kcalloc(dev, chip->npwm, sizeof(*ec_pwm->channel),
+					GFP_KERNEL);
 	if (!ec_pwm->channel)
 		return -ENOMEM;
 

I assume that also with this change it's ok to add your Reviewed-by tag.
If you don't agree, please let me know.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH 8/8] pwm: Drop pwm_[sg]et_chip_data()

[Uwe Kleine-König]

The semantic of chip_data is a bit surprising as it's cleared when
pwm_put() is called. Also there is a big overlap with the standard driver
data.

All drivers were adapted to not make use of chip_data any more, so it can
go away.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/pwm/core.c  | 31 -------------------------------
 include/linux/pwm.h | 14 --------------
 2 files changed, 45 deletions(-)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 3dacceaef4a9..2a786f3b6780 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -208,36 +208,6 @@ static void of_pwmchip_remove(struct pwm_chip *chip)
 		of_node_put(chip->dev->of_node);
 }
 
-/**
- * pwm_set_chip_data() - set private chip data for a PWM
- * @pwm: PWM device
- * @data: pointer to chip-specific data
- *
- * Returns: 0 on success or a negative error code on failure.
- */
-int pwm_set_chip_data(struct pwm_device *pwm, void *data)
-{
-	if (!pwm)
-		return -EINVAL;
-
-	pwm->chip_data = data;
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(pwm_set_chip_data);
-
-/**
- * pwm_get_chip_data() - get private chip data for a PWM
- * @pwm: PWM device
- *
- * Returns: A pointer to the chip-private data for the PWM device.
- */
-void *pwm_get_chip_data(struct pwm_device *pwm)
-{
-	return pwm ? pwm->chip_data : NULL;
-}
-EXPORT_SYMBOL_GPL(pwm_get_chip_data);
-
 static bool pwm_ops_check(const struct pwm_chip *chip)
 {
 	const struct pwm_ops *ops = chip->ops;
@@ -979,7 +949,6 @@ void pwm_put(struct pwm_device *pwm)
 	if (pwm->chip->ops->free)
 		pwm->chip->ops->free(pwm->chip, pwm);
 
-	pwm_set_chip_data(pwm, NULL);
 	pwm->label = NULL;
 
 	module_put(pwm->chip->ops->owner);
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index 04ae1d9073a7..e0cc42aafb1c 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -71,7 +71,6 @@ struct pwm_state {
  * @hwpwm: per-chip relative index of the PWM device
  * @pwm: global index of the PWM device
  * @chip: PWM chip providing this PWM device
- * @chip_data: chip-private data associated with the PWM device
  * @args: PWM arguments
  * @state: last applied state
  * @last: last implemented state (for PWM_DEBUG)
@@ -82,7 +81,6 @@ struct pwm_device {
 	unsigned int hwpwm;
 	unsigned int pwm;
 	struct pwm_chip *chip;
-	void *chip_data;
 
 	struct pwm_args args;
 	struct pwm_state state;
@@ -383,8 +381,6 @@ static inline void pwm_disable(struct pwm_device *pwm)
 /* PWM provider APIs */
 int pwm_capture(struct pwm_device *pwm, struct pwm_capture *result,
 		unsigned long timeout);
-int pwm_set_chip_data(struct pwm_device *pwm, void *data);
-void *pwm_get_chip_data(struct pwm_device *pwm);
 
 int pwmchip_add(struct pwm_chip *chip);
 void pwmchip_remove(struct pwm_chip *chip);
@@ -445,16 +441,6 @@ static inline int pwm_capture(struct pwm_device *pwm,
 	return -EINVAL;
 }
 
-static inline int pwm_set_chip_data(struct pwm_device *pwm, void *data)
-{
-	return -EINVAL;
-}
-
-static inline void *pwm_get_chip_data(struct pwm_device *pwm)
-{
-	return NULL;
-}
-
 static inline int pwmchip_add(struct pwm_chip *chip)
 {
 	return -EINVAL;
-- 
2.39.2

Re: [PATCH 3/8] pwm: jz4740: Put per-channel clk into driver data

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 2938 bytes --]

Hello Paul,

first of all, your mail is strange. I think the problem is that it
doesn't contain a Message-Id. The result is that I got it twice and in
return vger.kernel.org seems to have refused to take it. At least it's
neither in lore.kernel.org nor in
https://patchwork.ozlabs.org/project/linux-pwm/patch/20230629094839.757092-4-u.kleine-koenig@pengutronix.de/
.

On Thu, Jun 29, 2023 at 01:12:25PM +0200, Paul Cercueil wrote:
> Le 29 juin 2023 11:48, Uwe Kleine-König <u.kleine-koenig@pengutronix.de> a écrit :
> >
> > Stop using chip_data which is about to go away. Instead track the 
> > per-channel clk in struct jz4740_pwm_chip. 
> >
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> 
> > --- 
> > drivers/pwm/pwm-jz4740.c | 11 +++++++---- 
> > 1 file changed, 7 insertions(+), 4 deletions(-) 
> >
> > diff --git a/drivers/pwm/pwm-jz4740.c b/drivers/pwm/pwm-jz4740.c 
> > index 3b7067f6cd0d..e0a57d71a60c 100644 
> > --- a/drivers/pwm/pwm-jz4740.c 
> > +++ b/drivers/pwm/pwm-jz4740.c 
> > @@ -27,6 +27,7 @@ struct soc_info { 
> > struct jz4740_pwm_chip { 
> > struct pwm_chip chip; 
> > struct regmap *map; 
> > + struct clk *clk[]; 
> > }; 
> >
> > static inline struct jz4740_pwm_chip *to_jz4740(struct pwm_chip *chip) 
> > @@ -70,14 +71,15 @@ static int jz4740_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm) 
> > return err; 
> > } 
> >
> > - pwm_set_chip_data(pwm, clk); 
> > + jz->clk[pwm->hwpwm] = clk; 
> >
> > return 0; 
> > } 
> >
> > static void jz4740_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm) 
> > { 
> > - struct clk *clk = pwm_get_chip_data(pwm); 
> > + struct jz4740_pwm_chip *jz = to_jz4740(chip); 
> > + struct clk *clk = jz->clk[pwm->hwpwm]; 
> >
> > clk_disable_unprepare(clk); 
> > clk_put(clk); 
> > @@ -123,7 +125,7 @@ static int jz4740_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, 
> > { 
> > struct jz4740_pwm_chip *jz4740 = to_jz4740(pwm->chip); 
> > unsigned long long tmp = 0xffffull * NSEC_PER_SEC; 
> > - struct clk *clk = pwm_get_chip_data(pwm); 
> > + struct clk *clk = jz4740->clk[pwm->hwpwm]; 
> > unsigned long period, duty; 
> > long rate; 
> > int err; 
> > @@ -229,7 +231,8 @@ static int jz4740_pwm_probe(struct platform_device *pdev) 
> > if (!info) 
> > return -EINVAL; 
> >
> > - jz4740 = devm_kzalloc(dev, sizeof(*jz4740), GFP_KERNEL); 
> > + jz4740 = devm_kzalloc(dev, sizeof(*jz4740) + info->num_pwms * sizeof(jz4740->clk[0]), 
> > +       GFP_KERNEL); 
> 
> LGTM, but please use struct_size() from <linux/overflow.h>.

Ah, I thought there is such a macro, but I didn't find it neither by
grepping nor by asking in #kernelnewbies. Thanks, will respin the series
in a few days.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 3/8] pwm: jz4740: Put per-channel clk into driver data

[Paul Cercueil]

Le jeu. 29 juin 2023 à 16:07:45 +0200, Uwe Kleine-König 
<u.kleine-koenig@pengutronix.de> a écrit :
> Hello Paul,
> 
> first of all, your mail is strange. I think the problem is that it
> doesn't contain a Message-Id. The result is that I got it twice and in
> return vger.kernel.org seems to have refused to take it. At least it's
> neither in lore.kernel.org nor in
> https://patchwork.ozlabs.org/project/linux-pwm/patch/20230629094839.757092-4-u.kleine-koenig@pengutronix.de/
> .

Sorry about that, I'm at the Embedded Linux conference in Prague 
without my computer, so I just answered from my Android phone. I didn't 
know it would cause problems, I guess I'll refrain from doing that from 
now on.

Cheers,
-Paul

> 
> On Thu, Jun 29, 2023 at 01:12:25PM +0200, Paul Cercueil wrote:
>>  Le 29 juin 2023 11:48, Uwe Kleine-König 
>> <u.kleine-koenig@pengutronix.de> a écrit :
>>  >
>>  > Stop using chip_data which is about to go away. Instead track the
>>  > per-channel clk in struct jz4740_pwm_chip.
>>  >
>>  > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>>  > ---
>>  > drivers/pwm/pwm-jz4740.c | 11 +++++++----
>>  > 1 file changed, 7 insertions(+), 4 deletions(-)
>>  >
>>  > diff --git a/drivers/pwm/pwm-jz4740.c b/drivers/pwm/pwm-jz4740.c
>>  > index 3b7067f6cd0d..e0a57d71a60c 100644
>>  > --- a/drivers/pwm/pwm-jz4740.c
>>  > +++ b/drivers/pwm/pwm-jz4740.c
>>  > @@ -27,6 +27,7 @@ struct soc_info {
>>  > struct jz4740_pwm_chip {
>>  > struct pwm_chip chip;
>>  > struct regmap *map;
>>  > + struct clk *clk[];
>>  > };
>>  >
>>  > static inline struct jz4740_pwm_chip *to_jz4740(struct pwm_chip 
>> *chip)
>>  > @@ -70,14 +71,15 @@ static int jz4740_pwm_request(struct pwm_chip 
>> *chip, struct pwm_device *pwm)
>>  > return err;
>>  > }
>>  >
>>  > - pwm_set_chip_data(pwm, clk);
>>  > + jz->clk[pwm->hwpwm] = clk;
>>  >
>>  > return 0;
>>  > }
>>  >
>>  > static void jz4740_pwm_free(struct pwm_chip *chip, struct 
>> pwm_device *pwm)
>>  > {
>>  > - struct clk *clk = pwm_get_chip_data(pwm);
>>  > + struct jz4740_pwm_chip *jz = to_jz4740(chip);
>>  > + struct clk *clk = jz->clk[pwm->hwpwm];
>>  >
>>  > clk_disable_unprepare(clk);
>>  > clk_put(clk);
>>  > @@ -123,7 +125,7 @@ static int jz4740_pwm_apply(struct pwm_chip 
>> *chip, struct pwm_device *pwm,
>>  > {
>>  > struct jz4740_pwm_chip *jz4740 = to_jz4740(pwm->chip);
>>  > unsigned long long tmp = 0xffffull * NSEC_PER_SEC;
>>  > - struct clk *clk = pwm_get_chip_data(pwm);
>>  > + struct clk *clk = jz4740->clk[pwm->hwpwm];
>>  > unsigned long period, duty;
>>  > long rate;
>>  > int err;
>>  > @@ -229,7 +231,8 @@ static int jz4740_pwm_probe(struct 
>> platform_device *pdev)
>>  > if (!info)
>>  > return -EINVAL;
>>  >
>>  > - jz4740 = devm_kzalloc(dev, sizeof(*jz4740), GFP_KERNEL);
>>  > + jz4740 = devm_kzalloc(dev, sizeof(*jz4740) + info->num_pwms * 
>> sizeof(jz4740->clk[0]),
>>  > +       GFP_KERNEL);
>> 
>>  LGTM, but please use struct_size() from <linux/overflow.h>.
> 
> Ah, I thought there is such a macro, but I didn't find it neither by
> grepping nor by asking in #kernelnewbies. Thanks, will respin the 
> series
> in a few days.
> 
> Best regards
> Uwe
> 
> --
> Pengutronix e.K.                           | Uwe Kleine-König        
>     |
> Industrial Linux Solutions                 | 
> https://www.pengutronix.de/ |

Re: [PATCH 3/8] pwm: jz4740: Put per-channel clk into driver data

[Philippe Mathieu-Daudé]

On 29/6/23 16:07, Uwe Kleine-König wrote:

> On Thu, Jun 29, 2023 at 01:12:25PM +0200, Paul Cercueil wrote:
>> Le 29 juin 2023 11:48, Uwe Kleine-König <u.kleine-koenig@pengutronix.de> a écrit :
>>>
>>> Stop using chip_data which is about to go away. Instead track the
>>> per-channel clk in struct jz4740_pwm_chip.
>>>
>>> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>>> ---
>>> drivers/pwm/pwm-jz4740.c | 11 +++++++----
>>> 1 file changed, 7 insertions(+), 4 deletions(-)


>>> @@ -229,7 +231,8 @@ static int jz4740_pwm_probe(struct platform_device *pdev)
>>> if (!info)
>>> return -EINVAL;
>>>
>>> - jz4740 = devm_kzalloc(dev, sizeof(*jz4740), GFP_KERNEL);
>>> + jz4740 = devm_kzalloc(dev, sizeof(*jz4740) + info->num_pwms * sizeof(jz4740->clk[0]),
>>> +       GFP_KERNEL);
>>
>> LGTM, but please use struct_size() from <linux/overflow.h>.
> 
> Ah, I thought there is such a macro, but I didn't find it neither by
> grepping nor by asking in #kernelnewbies. Thanks, will respin the series
> in a few days.

Oh, TIL too :) R-b stands in respin.