[PATCH v4 009/115] pwm: atmel: Make use of pwmchip_parent() macro

[PATCH v4 009/115] pwm: atmel: Make use of pwmchip_parent() macro

[Uwe Kleine-König]

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 macro
provided for exactly this purpose.

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

diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c
index 47bcc8a3bf9d..4ef91fe6f147 100644
--- a/drivers/pwm/pwm-atmel.c
+++ b/drivers/pwm/pwm-atmel.c
@@ -210,7 +210,7 @@ static int atmel_pwm_calculate_cprd_and_pres(struct pwm_chip *chip,
 	shift = fls(cycles) - atmel_pwm->data->cfg.period_bits;
 
 	if (shift > PWM_MAX_PRES) {
-		dev_err(chip->dev, "pres exceeds the maximum value\n");
+		dev_err(pwmchip_parent(chip), "pres exceeds the maximum value\n");
 		return -EINVAL;
 	} else if (shift > 0) {
 		*pres = shift;
@@ -321,7 +321,7 @@ static int atmel_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 		ret = atmel_pwm_calculate_cprd_and_pres(chip, clkrate, state, &cprd,
 							&pres);
 		if (ret) {
-			dev_err(chip->dev,
+			dev_err(pwmchip_parent(chip),
 				"failed to calculate cprd and prescaler\n");
 			return ret;
 		}
@@ -333,7 +333,7 @@ static int atmel_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 		} else {
 			ret = clk_enable(atmel_pwm->clk);
 			if (ret) {
-				dev_err(chip->dev, "failed to enable clock\n");
+				dev_err(pwmchip_parent(chip), "failed to enable clock\n");
 				return ret;
 			}
 		}
@@ -462,8 +462,9 @@ static const struct of_device_id atmel_pwm_dt_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, atmel_pwm_dt_ids);
 
-static int atmel_pwm_enable_clk_if_on(struct atmel_pwm_chip *atmel_pwm, bool on)
+static int atmel_pwm_enable_clk_if_on(struct pwm_chip *chip, bool on)
 {
+	struct atmel_pwm_chip *atmel_pwm = to_atmel_pwm_chip(chip);
 	unsigned int i, cnt = 0;
 	unsigned long sr;
 	int ret = 0;
@@ -480,7 +481,7 @@ static int atmel_pwm_enable_clk_if_on(struct atmel_pwm_chip *atmel_pwm, bool on)
 	for (i = 0; i < cnt; i++) {
 		ret = clk_enable(atmel_pwm->clk);
 		if (ret) {
-			dev_err(atmel_pwm->chip.dev,
+			dev_err(pwmchip_parent(chip),
 				"failed to enable clock for pwm %pe\n",
 				ERR_PTR(ret));
 
@@ -525,7 +526,7 @@ static int atmel_pwm_probe(struct platform_device *pdev)
 	atmel_pwm->chip.ops = &atmel_pwm_ops;
 	atmel_pwm->chip.npwm = 4;
 
-	ret = atmel_pwm_enable_clk_if_on(atmel_pwm, true);
+	ret = atmel_pwm_enable_clk_if_on(&atmel_pwm->chip, true);
 	if (ret < 0)
 		return ret;
 
@@ -538,7 +539,7 @@ static int atmel_pwm_probe(struct platform_device *pdev)
 	return 0;
 
 disable_clk:
-	atmel_pwm_enable_clk_if_on(atmel_pwm, false);
+	atmel_pwm_enable_clk_if_on(&atmel_pwm->chip, false);
 
 	return ret;
 }
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 010/115] pwm: atmel-tcb: Make use of pwmchip_parent() macro

[Uwe Kleine-König]

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 macro
provided for exactly this purpose.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/pwm/pwm-atmel-tcb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pwm/pwm-atmel-tcb.c b/drivers/pwm/pwm-atmel-tcb.c
index d42c897cb85e..80121ab81a01 100644
--- a/drivers/pwm/pwm-atmel-tcb.c
+++ b/drivers/pwm/pwm-atmel-tcb.c
@@ -327,7 +327,7 @@ static int atmel_tcb_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	if ((atcbpwm && atcbpwm->duty > 0 &&
 			atcbpwm->duty != atcbpwm->period) &&
 		(atcbpwm->div != i || atcbpwm->period != period)) {
-		dev_err(chip->dev,
+		dev_err(pwmchip_parent(chip),
 			"failed to configure period_ns: PWM group already configured with a different value\n");
 		return -EINVAL;
 	}
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 018/115] pwm: imx27: Make use of pwmchip_parent() macro

[Uwe Kleine-König]

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 macro
provided for exactly this purpose.

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

diff --git a/drivers/pwm/pwm-imx27.c b/drivers/pwm/pwm-imx27.c
index 7d9bc43f12b0..5d796453519a 100644
--- a/drivers/pwm/pwm-imx27.c
+++ b/drivers/pwm/pwm-imx27.c
@@ -145,7 +145,7 @@ static int pwm_imx27_get_state(struct pwm_chip *chip,
 		state->polarity = PWM_POLARITY_INVERSED;
 		break;
 	default:
-		dev_warn(chip->dev, "can't set polarity, output disconnected");
+		dev_warn(pwmchip_parent(chip), "can't set polarity, output disconnected");
 	}
 
 	prescaler = MX3_PWMCR_PRESCALER_GET(val);
@@ -177,7 +177,7 @@ static int pwm_imx27_get_state(struct pwm_chip *chip,
 static void pwm_imx27_sw_reset(struct pwm_chip *chip)
 {
 	struct pwm_imx27_chip *imx = to_pwm_imx27_chip(chip);
-	struct device *dev = chip->dev;
+	struct device *dev = pwmchip_parent(chip);
 	int wait_count = 0;
 	u32 cr;
 
@@ -196,7 +196,7 @@ static void pwm_imx27_wait_fifo_slot(struct pwm_chip *chip,
 				     struct pwm_device *pwm)
 {
 	struct pwm_imx27_chip *imx = to_pwm_imx27_chip(chip);
-	struct device *dev = chip->dev;
+	struct device *dev = pwmchip_parent(chip);
 	unsigned int period_ms;
 	int fifoav;
 	u32 sr;
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 020/115] pwm: lpc18xx-sct: Make use of parent device pointer in driver data

[Uwe Kleine-König]

struct pwm_chip::dev is about to change. To not have to touch this
driver in the same commit as struct pwm_chip::dev, make use of the
already existing pointer to the parent device in driver data. However
rename the pointer to "parent" for consistency with other drivers.

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

diff --git a/drivers/pwm/pwm-lpc18xx-sct.c b/drivers/pwm/pwm-lpc18xx-sct.c
index b3d4a955aa31..9e1db43a5171 100644
--- a/drivers/pwm/pwm-lpc18xx-sct.c
+++ b/drivers/pwm/pwm-lpc18xx-sct.c
@@ -92,7 +92,7 @@ struct lpc18xx_pwm_data {
 };
 
 struct lpc18xx_pwm_chip {
-	struct device *dev;
+	struct device *parent;
 	struct pwm_chip chip;
 	void __iomem *base;
 	struct clk *pwm_clk;
@@ -198,7 +198,7 @@ static int lpc18xx_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 
 	if (period_ns < lpc18xx_pwm->min_period_ns ||
 	    period_ns > lpc18xx_pwm->max_period_ns) {
-		dev_err(chip->dev, "period %d not in range\n", period_ns);
+		dev_err(lpc18xx_pwm->parent, "period %d not in range\n", period_ns);
 		return -ERANGE;
 	}
 
@@ -214,7 +214,7 @@ static int lpc18xx_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	 */
 	if (requested_events > 2 && lpc18xx_pwm->period_ns != period_ns &&
 	    lpc18xx_pwm->period_ns) {
-		dev_err(chip->dev, "conflicting period requested for PWM %u\n",
+		dev_err(lpc18xx_pwm->parent, "conflicting period requested for PWM %u\n",
 			pwm->hwpwm);
 		mutex_unlock(&lpc18xx_pwm->period_lock);
 		return -EBUSY;
@@ -291,7 +291,7 @@ static int lpc18xx_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
 				    LPC18XX_PWM_EVENT_MAX);
 
 	if (event >= LPC18XX_PWM_EVENT_MAX) {
-		dev_err(lpc18xx_pwm->dev,
+		dev_err(lpc18xx_pwm->parent,
 			"maximum number of simultaneous channels reached\n");
 		return -EBUSY;
 	}
@@ -360,7 +360,7 @@ static int lpc18xx_pwm_probe(struct platform_device *pdev)
 	if (!lpc18xx_pwm)
 		return -ENOMEM;
 
-	lpc18xx_pwm->dev = &pdev->dev;
+	lpc18xx_pwm->parent = &pdev->dev;
 
 	lpc18xx_pwm->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(lpc18xx_pwm->base))
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 022/115] pwm: mediatek: Make use of pwmchip_parent() macro

[Uwe Kleine-König]

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 macro
provided for exactly this purpose.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/pwm/pwm-mediatek.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pwm/pwm-mediatek.c b/drivers/pwm/pwm-mediatek.c
index 17d290f847af..d7d1c326e8cf 100644
--- a/drivers/pwm/pwm-mediatek.c
+++ b/drivers/pwm/pwm-mediatek.c
@@ -150,7 +150,7 @@ static int pwm_mediatek_config(struct pwm_chip *chip, struct pwm_device *pwm,
 
 	if (clkdiv > PWM_CLK_DIV_MAX) {
 		pwm_mediatek_clk_disable(chip, pwm);
-		dev_err(chip->dev, "period of %d ns not supported\n", period_ns);
+		dev_err(pwmchip_parent(chip), "period of %d ns not supported\n", period_ns);
 		return -EINVAL;
 	}
 
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 023/115] pwm: meson: Make use of pwmchip_parent() macro

[Uwe Kleine-König]

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 macro
provided for exactly this purpose.

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

diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
index 5bea53243ed2..640d6fe63799 100644
--- a/drivers/pwm/pwm-meson.c
+++ b/drivers/pwm/pwm-meson.c
@@ -122,7 +122,7 @@ static int meson_pwm_request(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];
-	struct device *dev = chip->dev;
+	struct device *dev = pwmchip_parent(chip);
 	int err;
 
 	err = clk_prepare_enable(channel->clk);
@@ -143,9 +143,10 @@ static void meson_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
 	clk_disable_unprepare(channel->clk);
 }
 
-static int meson_pwm_calc(struct meson_pwm *meson, struct pwm_device *pwm,
+static int meson_pwm_calc(struct pwm_chip *chip, struct pwm_device *pwm,
 			  const struct pwm_state *state)
 {
+	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;
@@ -169,19 +170,19 @@ static int meson_pwm_calc(struct meson_pwm *meson, struct pwm_device *pwm,
 
 	fin_freq = clk_round_rate(channel->clk, freq);
 	if (fin_freq == 0) {
-		dev_err(meson->chip.dev, "invalid source clock frequency\n");
+		dev_err(pwmchip_parent(chip), "invalid source clock frequency\n");
 		return -EINVAL;
 	}
 
-	dev_dbg(meson->chip.dev, "fin_freq: %lu Hz\n", fin_freq);
+	dev_dbg(pwmchip_parent(chip), "fin_freq: %lu Hz\n", fin_freq);
 
 	cnt = div_u64(fin_freq * period, NSEC_PER_SEC);
 	if (cnt > 0xffff) {
-		dev_err(meson->chip.dev, "unable to get period cnt\n");
+		dev_err(pwmchip_parent(chip), "unable to get period cnt\n");
 		return -EINVAL;
 	}
 
-	dev_dbg(meson->chip.dev, "period=%llu cnt=%u\n", period, cnt);
+	dev_dbg(pwmchip_parent(chip), "period=%llu cnt=%u\n", period, cnt);
 
 	if (duty == period) {
 		channel->hi = cnt;
@@ -192,7 +193,7 @@ static int meson_pwm_calc(struct meson_pwm *meson, struct pwm_device *pwm,
 	} else {
 		duty_cnt = div_u64(fin_freq * duty, NSEC_PER_SEC);
 
-		dev_dbg(meson->chip.dev, "duty=%llu duty_cnt=%u\n", duty, duty_cnt);
+		dev_dbg(pwmchip_parent(chip), "duty=%llu duty_cnt=%u\n", duty, duty_cnt);
 
 		channel->hi = duty_cnt;
 		channel->lo = cnt - duty_cnt;
@@ -203,8 +204,9 @@ static int meson_pwm_calc(struct meson_pwm *meson, struct pwm_device *pwm,
 	return 0;
 }
 
-static void meson_pwm_enable(struct meson_pwm *meson, struct pwm_device *pwm)
+static void meson_pwm_enable(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];
 	struct meson_pwm_channel_data *channel_data;
 	unsigned long flags;
@@ -215,7 +217,7 @@ static void meson_pwm_enable(struct meson_pwm *meson, struct pwm_device *pwm)
 
 	err = clk_set_rate(channel->clk, channel->rate);
 	if (err)
-		dev_err(meson->chip.dev, "setting clock rate failed\n");
+		dev_err(pwmchip_parent(chip), "setting clock rate failed\n");
 
 	spin_lock_irqsave(&meson->lock, flags);
 
@@ -230,8 +232,9 @@ static void meson_pwm_enable(struct meson_pwm *meson, struct pwm_device *pwm)
 	spin_unlock_irqrestore(&meson->lock, flags);
 }
 
-static void meson_pwm_disable(struct meson_pwm *meson, struct pwm_device *pwm)
+static void meson_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
 {
+	struct meson_pwm *meson = to_meson_pwm(chip);
 	unsigned long flags;
 	u32 value;
 
@@ -269,16 +272,16 @@ static int meson_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 			channel->hi = ~0;
 			channel->lo = 0;
 
-			meson_pwm_enable(meson, pwm);
+			meson_pwm_enable(chip, pwm);
 		} else {
-			meson_pwm_disable(meson, pwm);
+			meson_pwm_disable(chip, pwm);
 		}
 	} else {
-		err = meson_pwm_calc(meson, pwm, state);
+		err = meson_pwm_calc(chip, pwm, state);
 		if (err < 0)
 			return err;
 
-		meson_pwm_enable(meson, pwm);
+		meson_pwm_enable(chip, pwm);
 	}
 
 	return 0;
@@ -432,10 +435,11 @@ static const struct of_device_id meson_pwm_matches[] = {
 };
 MODULE_DEVICE_TABLE(of, meson_pwm_matches);
 
-static int meson_pwm_init_channels(struct meson_pwm *meson)
+static int meson_pwm_init_channels(struct pwm_chip *chip)
 {
+	struct meson_pwm *meson = to_meson_pwm(chip);
 	struct clk_parent_data mux_parent_data[MESON_MAX_MUX_PARENTS] = {};
-	struct device *dev = meson->chip.dev;
+	struct device *dev = pwmchip_parent(chip);
 	unsigned int i;
 	char name[255];
 	int err;
@@ -549,7 +553,7 @@ static int meson_pwm_probe(struct platform_device *pdev)
 
 	meson->data = of_device_get_match_data(&pdev->dev);
 
-	err = meson_pwm_init_channels(meson);
+	err = meson_pwm_init_channels(&meson->chip);
 	if (err < 0)
 		return err;
 
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 024/115] pwm: mtk-disp: Make use of pwmchip_parent() macro

[Uwe Kleine-König]

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 macro
provided for exactly this purpose.

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

diff --git a/drivers/pwm/pwm-mtk-disp.c b/drivers/pwm/pwm-mtk-disp.c
index a72f7be36996..186aa5c04435 100644
--- a/drivers/pwm/pwm-mtk-disp.c
+++ b/drivers/pwm/pwm-mtk-disp.c
@@ -91,14 +91,14 @@ static int mtk_disp_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	if (!mdp->enabled) {
 		err = clk_prepare_enable(mdp->clk_main);
 		if (err < 0) {
-			dev_err(chip->dev, "Can't enable mdp->clk_main: %pe\n",
+			dev_err(pwmchip_parent(chip), "Can't enable mdp->clk_main: %pe\n",
 				ERR_PTR(err));
 			return err;
 		}
 
 		err = clk_prepare_enable(mdp->clk_mm);
 		if (err < 0) {
-			dev_err(chip->dev, "Can't enable mdp->clk_mm: %pe\n",
+			dev_err(pwmchip_parent(chip), "Can't enable mdp->clk_mm: %pe\n",
 				ERR_PTR(err));
 			clk_disable_unprepare(mdp->clk_main);
 			return err;
@@ -181,13 +181,13 @@ static int mtk_disp_pwm_get_state(struct pwm_chip *chip,
 
 	err = clk_prepare_enable(mdp->clk_main);
 	if (err < 0) {
-		dev_err(chip->dev, "Can't enable mdp->clk_main: %pe\n", ERR_PTR(err));
+		dev_err(pwmchip_parent(chip), "Can't enable mdp->clk_main: %pe\n", ERR_PTR(err));
 		return err;
 	}
 
 	err = clk_prepare_enable(mdp->clk_mm);
 	if (err < 0) {
-		dev_err(chip->dev, "Can't enable mdp->clk_mm: %pe\n", ERR_PTR(err));
+		dev_err(pwmchip_parent(chip), "Can't enable mdp->clk_mm: %pe\n", ERR_PTR(err));
 		clk_disable_unprepare(mdp->clk_main);
 		return err;
 	}
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 027/115] pwm: raspberrypi-poe: Make use of pwmchip_parent() macro

[Uwe Kleine-König]

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 macro
provided for exactly this purpose.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/pwm/pwm-raspberrypi-poe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pwm/pwm-raspberrypi-poe.c b/drivers/pwm/pwm-raspberrypi-poe.c
index 1ad814fdec6b..f10e8a624c51 100644
--- a/drivers/pwm/pwm-raspberrypi-poe.c
+++ b/drivers/pwm/pwm-raspberrypi-poe.c
@@ -122,7 +122,7 @@ static int raspberrypi_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	ret = raspberrypi_pwm_set_property(rpipwm->firmware, RPI_PWM_CUR_DUTY_REG,
 					   duty_cycle);
 	if (ret) {
-		dev_err(chip->dev, "Failed to set duty cycle: %pe\n",
+		dev_err(pwmchip_parent(chip), "Failed to set duty cycle: %pe\n",
 			ERR_PTR(ret));
 		return ret;
 	}
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 030/115] pwm: samsung: Make use of pwmchip_parent() macro

[Uwe Kleine-König]

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 macro
provided for exactly this purpose.

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

diff --git a/drivers/pwm/pwm-samsung.c b/drivers/pwm/pwm-samsung.c
index 6e77302f7368..a97cae49406e 100644
--- a/drivers/pwm/pwm-samsung.c
+++ b/drivers/pwm/pwm-samsung.c
@@ -197,12 +197,12 @@ static unsigned long pwm_samsung_calc_tin(struct samsung_pwm_chip *our_chip,
 				return rate;
 		}
 
-		dev_warn(our_chip->chip.dev,
+		dev_warn(pwmchip_parent(&our_chip->chip),
 			"tclk of PWM %d is inoperational, using tdiv\n", chan);
 	}
 
 	rate = pwm_samsung_get_tin_rate(our_chip, chan);
-	dev_dbg(our_chip->chip.dev, "tin parent at %lu\n", rate);
+	dev_dbg(pwmchip_parent(&our_chip->chip), "tin parent at %lu\n", rate);
 
 	/*
 	 * Compare minimum PWM frequency that can be achieved with possible
@@ -232,7 +232,7 @@ static int pwm_samsung_request(struct pwm_chip *chip, struct pwm_device *pwm)
 	struct samsung_pwm_chip *our_chip = to_samsung_pwm_chip(chip);
 
 	if (!(our_chip->variant.output_mask & BIT(pwm->hwpwm))) {
-		dev_warn(chip->dev,
+		dev_warn(pwmchip_parent(chip),
 			"tried to request PWM channel %d without output\n",
 			pwm->hwpwm);
 		return -EINVAL;
@@ -326,12 +326,12 @@ static int __pwm_samsung_config(struct pwm_chip *chip, struct pwm_device *pwm,
 
 		period = NSEC_PER_SEC / period_ns;
 
-		dev_dbg(our_chip->chip.dev, "duty_ns=%d, period_ns=%d (%u)\n",
+		dev_dbg(pwmchip_parent(chip), "duty_ns=%d, period_ns=%d (%u)\n",
 						duty_ns, period_ns, period);
 
 		tin_rate = pwm_samsung_calc_tin(our_chip, pwm->hwpwm, period);
 
-		dev_dbg(our_chip->chip.dev, "tin_rate=%lu\n", tin_rate);
+		dev_dbg(pwmchip_parent(chip), "tin_rate=%lu\n", tin_rate);
 
 		tin_ns = NSEC_PER_SEC / tin_rate;
 		tcnt = period_ns / tin_ns;
@@ -355,8 +355,7 @@ static int __pwm_samsung_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	/* -1UL will give 100% duty. */
 	--tcmp;
 
-	dev_dbg(our_chip->chip.dev,
-				"tin_ns=%u, tcmp=%u/%u\n", tin_ns, tcmp, tcnt);
+	dev_dbg(pwmchip_parent(chip), "tin_ns=%u, tcmp=%u/%u\n", tin_ns, tcmp, tcnt);
 
 	/* Update PWM registers. */
 	writel(tcnt, our_chip->base + REG_TCNTB(pwm->hwpwm));
@@ -368,7 +367,7 @@ static int __pwm_samsung_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	 * shortly afer this update (before it autoreloaded the new values).
 	 */
 	if (oldtcmp == (u32) -1) {
-		dev_dbg(our_chip->chip.dev, "Forcing manual update");
+		dev_dbg(pwmchip_parent(chip), "Forcing manual update");
 		pwm_samsung_manual_update(our_chip, pwm);
 	}
 
@@ -509,7 +508,7 @@ MODULE_DEVICE_TABLE(of, samsung_pwm_matches);
 
 static int pwm_samsung_parse_dt(struct samsung_pwm_chip *our_chip)
 {
-	struct device_node *np = our_chip->chip.dev->of_node;
+	struct device_node *np = pwmchip_parent(&our_chip->chip)->of_node;
 	const struct of_device_id *match;
 	struct property *prop;
 	const __be32 *cur;
@@ -523,7 +522,7 @@ static int pwm_samsung_parse_dt(struct samsung_pwm_chip *our_chip)
 
 	of_property_for_each_u32(np, "samsung,pwm-outputs", prop, cur, val) {
 		if (val >= SAMSUNG_PWM_NUM) {
-			dev_err(our_chip->chip.dev,
+			dev_err(pwmchip_parent(&our_chip->chip),
 				"%s: invalid channel index in samsung,pwm-outputs property\n",
 								__func__);
 			continue;
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 032/115] pwm: stm32-lp: Make use of pwmchip_parent() macro

[Uwe Kleine-König]

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 macro
provided for exactly this purpose.

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

diff --git a/drivers/pwm/pwm-stm32-lp.c b/drivers/pwm/pwm-stm32-lp.c
index 439068f3eca1..3754faeca838 100644
--- a/drivers/pwm/pwm-stm32-lp.c
+++ b/drivers/pwm/pwm-stm32-lp.c
@@ -61,7 +61,7 @@ static int stm32_pwm_lp_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	do_div(div, NSEC_PER_SEC);
 	if (!div) {
 		/* Clock is too slow to achieve requested period. */
-		dev_dbg(priv->chip.dev, "Can't reach %llu ns\n", state->period);
+		dev_dbg(pwmchip_parent(chip), "Can't reach %llu ns\n", state->period);
 		return -EINVAL;
 	}
 
@@ -69,7 +69,7 @@ static int stm32_pwm_lp_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	while (div > STM32_LPTIM_MAX_ARR) {
 		presc++;
 		if ((1 << presc) > STM32_LPTIM_MAX_PRESCALER) {
-			dev_err(priv->chip.dev, "max prescaler exceeded\n");
+			dev_err(pwmchip_parent(chip), "max prescaler exceeded\n");
 			return -EINVAL;
 		}
 		div = prd >> presc;
@@ -130,7 +130,7 @@ static int stm32_pwm_lp_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 				       (val & STM32_LPTIM_CMPOK_ARROK) == STM32_LPTIM_CMPOK_ARROK,
 				       100, 1000);
 	if (ret) {
-		dev_err(priv->chip.dev, "ARR/CMP registers write issue\n");
+		dev_err(pwmchip_parent(chip), "ARR/CMP registers write issue\n");
 		goto err;
 	}
 	ret = regmap_write(priv->regmap, STM32_LPTIM_ICR,
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 033/115] pwm: stm32: Make use of pwmchip_parent() macro

[Uwe Kleine-König]

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 macro
provided for exactly this purpose.

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

diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c
index 5f10cba492ec..d1dc9e3ca2a1 100644
--- a/drivers/pwm/pwm-stm32.c
+++ b/drivers/pwm/pwm-stm32.c
@@ -94,7 +94,7 @@ static int stm32_pwm_raw_capture(struct stm32_pwm *priv, struct pwm_device *pwm,
 				 unsigned long tmo_ms, u32 *raw_prd,
 				 u32 *raw_dty)
 {
-	struct device *parent = priv->chip.dev->parent;
+	struct device *parent = pwmchip_parent(&priv->chip)->parent;
 	enum stm32_timers_dmas dma_id;
 	u32 ccen, ccr;
 	int ret;
@@ -170,7 +170,7 @@ static int stm32_pwm_capture(struct pwm_chip *chip, struct pwm_device *pwm,
 
 	ret = clk_enable(priv->clk);
 	if (ret) {
-		dev_err(priv->chip.dev, "failed to enable counter clock\n");
+		dev_err(pwmchip_parent(chip), "failed to enable counter clock\n");
 		goto unlock;
 	}
 
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 034/115] pwm: stmpe: Make use of pwmchip_parent() macro

[Uwe Kleine-König]

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 macro
provided for exactly this purpose.

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

diff --git a/drivers/pwm/pwm-stmpe.c b/drivers/pwm/pwm-stmpe.c
index d730631c6583..941b1e218e35 100644
--- a/drivers/pwm/pwm-stmpe.c
+++ b/drivers/pwm/pwm-stmpe.c
@@ -44,7 +44,7 @@ static int stmpe_24xx_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 
 	ret = stmpe_reg_read(stmpe_pwm->stmpe, STMPE24XX_PWMCS);
 	if (ret < 0) {
-		dev_err(chip->dev, "error reading PWM#%u control\n",
+		dev_err(pwmchip_parent(chip), "error reading PWM#%u control\n",
 			pwm->hwpwm);
 		return ret;
 	}
@@ -53,7 +53,7 @@ static int stmpe_24xx_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 
 	ret = stmpe_reg_write(stmpe_pwm->stmpe, STMPE24XX_PWMCS, value);
 	if (ret) {
-		dev_err(chip->dev, "error writing PWM#%u control\n",
+		dev_err(pwmchip_parent(chip), "error writing PWM#%u control\n",
 			pwm->hwpwm);
 		return ret;
 	}
@@ -70,7 +70,7 @@ static int stmpe_24xx_pwm_disable(struct pwm_chip *chip,
 
 	ret = stmpe_reg_read(stmpe_pwm->stmpe, STMPE24XX_PWMCS);
 	if (ret < 0) {
-		dev_err(chip->dev, "error reading PWM#%u control\n",
+		dev_err(pwmchip_parent(chip), "error reading PWM#%u control\n",
 			pwm->hwpwm);
 		return ret;
 	}
@@ -79,7 +79,7 @@ static int stmpe_24xx_pwm_disable(struct pwm_chip *chip,
 
 	ret = stmpe_reg_write(stmpe_pwm->stmpe, STMPE24XX_PWMCS, value);
 	if (ret)
-		dev_err(chip->dev, "error writing PWM#%u control\n",
+		dev_err(pwmchip_parent(chip), "error writing PWM#%u control\n",
 			pwm->hwpwm);
 	return ret;
 }
@@ -125,7 +125,7 @@ static int stmpe_24xx_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 		ret = stmpe_set_altfunc(stmpe_pwm->stmpe, BIT(pin),
 					STMPE_BLOCK_PWM);
 		if (ret) {
-			dev_err(chip->dev, "unable to connect PWM#%u to pin\n",
+			dev_err(pwmchip_parent(chip), "unable to connect PWM#%u to pin\n",
 				pwm->hwpwm);
 			return ret;
 		}
@@ -150,7 +150,7 @@ static int stmpe_24xx_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 		return -ENODEV;
 	}
 
-	dev_dbg(chip->dev, "PWM#%u: config duty %d ns, period %d ns\n",
+	dev_dbg(pwmchip_parent(chip), "PWM#%u: config duty %d ns, period %d ns\n",
 		pwm->hwpwm, duty_ns, period_ns);
 
 	if (duty_ns == 0) {
@@ -216,7 +216,7 @@ static int stmpe_24xx_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 			program[1] = BRANCH;
 		}
 
-		dev_dbg(chip->dev,
+		dev_dbg(pwmchip_parent(chip),
 			"PWM#%u: value = %02x, last_duty = %02x, program=%04x,%04x,%04x\n",
 			pwm->hwpwm, value, last, program[0], program[1],
 			program[2]);
@@ -233,7 +233,7 @@ static int stmpe_24xx_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 
 		ret = stmpe_reg_write(stmpe_pwm->stmpe, offset, value);
 		if (ret) {
-			dev_err(chip->dev, "error writing register %02x: %d\n",
+			dev_err(pwmchip_parent(chip), "error writing register %02x: %d\n",
 				offset, ret);
 			return ret;
 		}
@@ -242,7 +242,7 @@ static int stmpe_24xx_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 
 		ret = stmpe_reg_write(stmpe_pwm->stmpe, offset, value);
 		if (ret) {
-			dev_err(chip->dev, "error writing register %02x: %d\n",
+			dev_err(pwmchip_parent(chip), "error writing register %02x: %d\n",
 				offset, ret);
 			return ret;
 		}
@@ -255,7 +255,7 @@ static int stmpe_24xx_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	/* Sleep for 200ms so we're sure it will take effect */
 	msleep(200);
 
-	dev_dbg(chip->dev, "programmed PWM#%u, %u bytes\n", pwm->hwpwm, i);
+	dev_dbg(pwmchip_parent(chip), "programmed PWM#%u, %u bytes\n", pwm->hwpwm, i);
 
 	return 0;
 }
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 035/115] pwm: sun4i: Make use of pwmchip_parent() macro

[Uwe Kleine-König]

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 macro
provided for exactly this purpose.

Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/pwm/pwm-sun4i.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
index 1a439025540d..44edf1ce5739 100644
--- a/drivers/pwm/pwm-sun4i.c
+++ b/drivers/pwm/pwm-sun4i.c
@@ -245,7 +245,7 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	if (!cstate.enabled) {
 		ret = clk_prepare_enable(sun4i_pwm->clk);
 		if (ret) {
-			dev_err(chip->dev, "failed to enable PWM clock\n");
+			dev_err(pwmchip_parent(chip), "failed to enable PWM clock\n");
 			return ret;
 		}
 	}
@@ -253,7 +253,7 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	ret = sun4i_pwm_calculate(sun4i_pwm, state, &duty, &period, &prescaler,
 				  &bypass);
 	if (ret) {
-		dev_err(chip->dev, "period exceeds the maximum value\n");
+		dev_err(pwmchip_parent(chip), "period exceeds the maximum value\n");
 		if (!cstate.enabled)
 			clk_disable_unprepare(sun4i_pwm->clk);
 		return ret;
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 040/115] pwm: vt8500: Make use of pwmchip_parent() macro

[Uwe Kleine-König]

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 macro
provided for exactly this purpose.

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

diff --git a/drivers/pwm/pwm-vt8500.c b/drivers/pwm/pwm-vt8500.c
index bdea60389487..ff5e76afbddf 100644
--- a/drivers/pwm/pwm-vt8500.c
+++ b/drivers/pwm/pwm-vt8500.c
@@ -53,8 +53,9 @@ struct vt8500_chip {
 #define to_vt8500_chip(chip)	container_of(chip, struct vt8500_chip, chip)
 
 #define msecs_to_loops(t) (loops_per_jiffy / 1000 * HZ * t)
-static inline void vt8500_pwm_busy_wait(struct vt8500_chip *vt8500, int nr, u8 bitmask)
+static inline void vt8500_pwm_busy_wait(struct pwm_chip *chip, int nr, u8 bitmask)
 {
+	struct vt8500_chip *vt8500 = to_vt8500_chip(chip);
 	int loops = msecs_to_loops(10);
 	u32 mask = bitmask << (nr << 8);
 
@@ -62,7 +63,7 @@ static inline void vt8500_pwm_busy_wait(struct vt8500_chip *vt8500, int nr, u8 b
 		cpu_relax();
 
 	if (unlikely(!loops))
-		dev_warn(vt8500->chip.dev, "Waiting for status bits 0x%x to clear timed out\n",
+		dev_warn(pwmchip_parent(chip), "Waiting for status bits 0x%x to clear timed out\n",
 			 mask);
 }
 
@@ -77,7 +78,7 @@ static int vt8500_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 
 	err = clk_enable(vt8500->clk);
 	if (err < 0) {
-		dev_err(chip->dev, "failed to enable clock\n");
+		dev_err(pwmchip_parent(chip), "failed to enable clock\n");
 		return err;
 	}
 
@@ -103,18 +104,18 @@ static int vt8500_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	dc = div64_u64(c, period_ns);
 
 	writel(prescale, vt8500->base + REG_SCALAR(pwm->hwpwm));
-	vt8500_pwm_busy_wait(vt8500, pwm->hwpwm, STATUS_SCALAR_UPDATE);
+	vt8500_pwm_busy_wait(chip, pwm->hwpwm, STATUS_SCALAR_UPDATE);
 
 	writel(pv, vt8500->base + REG_PERIOD(pwm->hwpwm));
-	vt8500_pwm_busy_wait(vt8500, pwm->hwpwm, STATUS_PERIOD_UPDATE);
+	vt8500_pwm_busy_wait(chip, pwm->hwpwm, STATUS_PERIOD_UPDATE);
 
 	writel(dc, vt8500->base + REG_DUTY(pwm->hwpwm));
-	vt8500_pwm_busy_wait(vt8500, pwm->hwpwm, STATUS_DUTY_UPDATE);
+	vt8500_pwm_busy_wait(chip, pwm->hwpwm, STATUS_DUTY_UPDATE);
 
 	val = readl(vt8500->base + REG_CTRL(pwm->hwpwm));
 	val |= CTRL_AUTOLOAD;
 	writel(val, vt8500->base + REG_CTRL(pwm->hwpwm));
-	vt8500_pwm_busy_wait(vt8500, pwm->hwpwm, STATUS_CTRL_UPDATE);
+	vt8500_pwm_busy_wait(chip, pwm->hwpwm, STATUS_CTRL_UPDATE);
 
 	clk_disable(vt8500->clk);
 	return 0;
@@ -128,14 +129,14 @@ static int vt8500_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 
 	err = clk_enable(vt8500->clk);
 	if (err < 0) {
-		dev_err(chip->dev, "failed to enable clock\n");
+		dev_err(pwmchip_parent(chip), "failed to enable clock\n");
 		return err;
 	}
 
 	val = readl(vt8500->base + REG_CTRL(pwm->hwpwm));
 	val |= CTRL_ENABLE;
 	writel(val, vt8500->base + REG_CTRL(pwm->hwpwm));
-	vt8500_pwm_busy_wait(vt8500, pwm->hwpwm, STATUS_CTRL_UPDATE);
+	vt8500_pwm_busy_wait(chip, pwm->hwpwm, STATUS_CTRL_UPDATE);
 
 	return 0;
 }
@@ -148,7 +149,7 @@ static void vt8500_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
 	val = readl(vt8500->base + REG_CTRL(pwm->hwpwm));
 	val &= ~CTRL_ENABLE;
 	writel(val, vt8500->base + REG_CTRL(pwm->hwpwm));
-	vt8500_pwm_busy_wait(vt8500, pwm->hwpwm, STATUS_CTRL_UPDATE);
+	vt8500_pwm_busy_wait(chip, pwm->hwpwm, STATUS_CTRL_UPDATE);
 
 	clk_disable(vt8500->clk);
 }
@@ -168,7 +169,7 @@ static int vt8500_pwm_set_polarity(struct pwm_chip *chip,
 		val &= ~CTRL_INVERT;
 
 	writel(val, vt8500->base + REG_CTRL(pwm->hwpwm));
-	vt8500_pwm_busy_wait(vt8500, pwm->hwpwm, STATUS_CTRL_UPDATE);
+	vt8500_pwm_busy_wait(chip, pwm->hwpwm, STATUS_CTRL_UPDATE);
 
 	return 0;
 }
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 045/115] pwm: apple: Make use of devm_pwmchip_alloc() function

[Uwe Kleine-König]

This prepares the pwm-apple 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.

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

diff --git a/drivers/pwm/pwm-apple.c b/drivers/pwm/pwm-apple.c
index 4d755b628d9e..6e58aca2f13c 100644
--- a/drivers/pwm/pwm-apple.c
+++ b/drivers/pwm/pwm-apple.c
@@ -32,14 +32,13 @@
 #define APPLE_PWM_CTRL_OUTPUT_ENABLE BIT(14)
 
 struct apple_pwm {
-	struct pwm_chip chip;
 	void __iomem *base;
 	u64 clkrate;
 };
 
 static inline struct apple_pwm *to_apple_pwm(struct pwm_chip *chip)
 {
-	return container_of(chip, struct apple_pwm, chip);
+	return pwmchip_get_drvdata(chip);
 }
 
 static int apple_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
@@ -103,13 +102,16 @@ static const struct pwm_ops apple_pwm_ops = {
 
 static int apple_pwm_probe(struct platform_device *pdev)
 {
+	struct pwm_chip *chip;
 	struct apple_pwm *fpwm;
 	struct clk *clk;
 	int ret;
 
-	fpwm = devm_kzalloc(&pdev->dev, sizeof(*fpwm), GFP_KERNEL);
-	if (!fpwm)
-		return -ENOMEM;
+	chip = devm_pwmchip_alloc(&pdev->dev, 1, sizeof(*fpwm));
+	if (IS_ERR(chip))
+		return PTR_ERR(chip);
+
+	fpwm = to_apple_pwm(chip);
 
 	fpwm->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(fpwm->base))
@@ -129,11 +131,9 @@ static int apple_pwm_probe(struct platform_device *pdev)
 	if (fpwm->clkrate > NSEC_PER_SEC)
 		return dev_err_probe(&pdev->dev, -EINVAL, "pwm clock out of range");
 
-	fpwm->chip.dev = &pdev->dev;
-	fpwm->chip.npwm = 1;
-	fpwm->chip.ops = &apple_pwm_ops;
+	chip->ops = &apple_pwm_ops;
 
-	ret = devm_pwmchip_add(&pdev->dev, &fpwm->chip);
+	ret = devm_pwmchip_add(&pdev->dev, chip);
 	if (ret < 0)
 		return dev_err_probe(&pdev->dev, ret, "unable to add pwm chip");
 
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 046/115] pwm: atmel-hlcdc: Make use of devm_pwmchip_alloc() function

[Uwe Kleine-König]

This prepares the pwm-atme-hlcdc 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.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/pwm/pwm-atmel-hlcdc.c | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/pwm/pwm-atmel-hlcdc.c b/drivers/pwm/pwm-atmel-hlcdc.c
index 3f2c5031a3ba..aa1b67b6f2ea 100644
--- a/drivers/pwm/pwm-atmel-hlcdc.c
+++ b/drivers/pwm/pwm-atmel-hlcdc.c
@@ -28,7 +28,6 @@ struct atmel_hlcdc_pwm_errata {
 };
 
 struct atmel_hlcdc_pwm {
-	struct pwm_chip chip;
 	struct atmel_hlcdc *hlcdc;
 	struct clk *cur_clk;
 	const struct atmel_hlcdc_pwm_errata *errata;
@@ -36,7 +35,7 @@ struct atmel_hlcdc_pwm {
 
 static inline struct atmel_hlcdc_pwm *to_atmel_hlcdc_pwm(struct pwm_chip *chip)
 {
-	return container_of(chip, struct atmel_hlcdc_pwm, chip);
+	return pwmchip_get_drvdata(chip);
 }
 
 static int atmel_hlcdc_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
@@ -182,10 +181,11 @@ static const struct atmel_hlcdc_pwm_errata atmel_hlcdc_pwm_sama5d3_errata = {
 
 static int atmel_hlcdc_pwm_suspend(struct device *dev)
 {
-	struct atmel_hlcdc_pwm *atmel = dev_get_drvdata(dev);
+	struct pwm_chip *chip = dev_get_drvdata(dev);
+	struct atmel_hlcdc_pwm *atmel = to_atmel_hlcdc_pwm(chip);
 
 	/* Keep the periph clock enabled if the PWM is still running. */
-	if (pwm_is_enabled(&atmel->chip.pwms[0]))
+	if (pwm_is_enabled(&chip->pwms[0]))
 		clk_disable_unprepare(atmel->hlcdc->periph_clk);
 
 	return 0;
@@ -193,11 +193,12 @@ static int atmel_hlcdc_pwm_suspend(struct device *dev)
 
 static int atmel_hlcdc_pwm_resume(struct device *dev)
 {
-	struct atmel_hlcdc_pwm *atmel = dev_get_drvdata(dev);
+	struct pwm_chip *chip = dev_get_drvdata(dev);
+	struct atmel_hlcdc_pwm *atmel = to_atmel_hlcdc_pwm(chip);
 	struct pwm_state state;
 	int ret;
 
-	pwm_get_state(&atmel->chip.pwms[0], &state);
+	pwm_get_state(&chip->pwms[0], &state);
 
 	/* Re-enable the periph clock it was stopped during suspend. */
 	if (!state.enabled) {
@@ -206,8 +207,7 @@ static int atmel_hlcdc_pwm_resume(struct device *dev)
 			return ret;
 	}
 
-	return atmel_hlcdc_pwm_apply(&atmel->chip, &atmel->chip.pwms[0],
-				     &state);
+	return atmel_hlcdc_pwm_apply(chip, &chip->pwms[0], &state);
 }
 
 static DEFINE_SIMPLE_DEV_PM_OPS(atmel_hlcdc_pwm_pm_ops,
@@ -243,15 +243,17 @@ static int atmel_hlcdc_pwm_probe(struct platform_device *pdev)
 {
 	const struct of_device_id *match;
 	struct device *dev = &pdev->dev;
+	struct pwm_chip *chip;
 	struct atmel_hlcdc_pwm *atmel;
 	struct atmel_hlcdc *hlcdc;
 	int ret;
 
 	hlcdc = dev_get_drvdata(dev->parent);
 
-	atmel = devm_kzalloc(dev, sizeof(*atmel), GFP_KERNEL);
-	if (!atmel)
-		return -ENOMEM;
+	chip = devm_pwmchip_alloc(dev, 1, sizeof(*atmel));
+	if (IS_ERR(chip))
+		return PTR_ERR(chip);
+	atmel = to_atmel_hlcdc_pwm(chip);
 
 	ret = clk_prepare_enable(hlcdc->periph_clk);
 	if (ret)
@@ -262,11 +264,9 @@ static int atmel_hlcdc_pwm_probe(struct platform_device *pdev)
 		atmel->errata = match->data;
 
 	atmel->hlcdc = hlcdc;
-	atmel->chip.ops = &atmel_hlcdc_pwm_ops;
-	atmel->chip.dev = dev;
-	atmel->chip.npwm = 1;
+	chip->ops = &atmel_hlcdc_pwm_ops;
 
-	ret = pwmchip_add(&atmel->chip);
+	ret = pwmchip_add(chip);
 	if (ret) {
 		clk_disable_unprepare(hlcdc->periph_clk);
 		return ret;
@@ -279,9 +279,10 @@ static int atmel_hlcdc_pwm_probe(struct platform_device *pdev)
 
 static void atmel_hlcdc_pwm_remove(struct platform_device *pdev)
 {
-	struct atmel_hlcdc_pwm *atmel = platform_get_drvdata(pdev);
+	struct pwm_chip *chip = platform_get_drvdata(pdev);
+	struct atmel_hlcdc_pwm *atmel = to_atmel_hlcdc_pwm(chip);
 
-	pwmchip_remove(&atmel->chip);
+	pwmchip_remove(chip);
 
 	clk_disable_unprepare(atmel->hlcdc->periph_clk);
 }
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 047/115] pwm: atmel: Make use of devm_pwmchip_alloc() function

[Uwe Kleine-König]

This prepares the pwm-atmel 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.

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

diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c
index 4ef91fe6f147..ab84229161ef 100644
--- a/drivers/pwm/pwm-atmel.c
+++ b/drivers/pwm/pwm-atmel.c
@@ -77,7 +77,6 @@ struct atmel_pwm_data {
 };
 
 struct atmel_pwm_chip {
-	struct pwm_chip chip;
 	struct clk *clk;
 	void __iomem *base;
 	const struct atmel_pwm_data *data;
@@ -99,7 +98,7 @@ struct atmel_pwm_chip {
 
 static inline struct atmel_pwm_chip *to_atmel_pwm_chip(struct pwm_chip *chip)
 {
-	return container_of(chip, struct atmel_pwm_chip, chip);
+	return pwmchip_get_drvdata(chip);
 }
 
 static inline u32 atmel_pwm_readl(struct atmel_pwm_chip *chip,
@@ -473,7 +472,7 @@ static int atmel_pwm_enable_clk_if_on(struct pwm_chip *chip, bool on)
 	if (!sr)
 		return 0;
 
-	cnt = bitmap_weight(&sr, atmel_pwm->chip.npwm);
+	cnt = bitmap_weight(&sr, chip->npwm);
 
 	if (!on)
 		goto disable_clk;
@@ -481,9 +480,8 @@ static int atmel_pwm_enable_clk_if_on(struct pwm_chip *chip, bool on)
 	for (i = 0; i < cnt; i++) {
 		ret = clk_enable(atmel_pwm->clk);
 		if (ret) {
-			dev_err(pwmchip_parent(chip),
-				"failed to enable clock for pwm %pe\n",
-				ERR_PTR(ret));
+			dev_err_probe(pwmchip_parent(chip), ret,
+				"failed to enable clock for pwm\n");
 
 			cnt = i;
 			goto disable_clk;
@@ -501,12 +499,14 @@ static int atmel_pwm_enable_clk_if_on(struct pwm_chip *chip, bool on)
 
 static int atmel_pwm_probe(struct platform_device *pdev)
 {
+	struct pwm_chip *chip;
 	struct atmel_pwm_chip *atmel_pwm;
 	int ret;
 
-	atmel_pwm = devm_kzalloc(&pdev->dev, sizeof(*atmel_pwm), GFP_KERNEL);
-	if (!atmel_pwm)
-		return -ENOMEM;
+	chip = devm_pwmchip_alloc(&pdev->dev, 4, sizeof(*atmel_pwm));
+	if (IS_ERR(chip))
+		return PTR_ERR(chip);
+	atmel_pwm = to_atmel_pwm_chip(chip);
 
 	atmel_pwm->data = of_device_get_match_data(&pdev->dev);
 
@@ -522,15 +522,13 @@ static int atmel_pwm_probe(struct platform_device *pdev)
 		return dev_err_probe(&pdev->dev, PTR_ERR(atmel_pwm->clk),
 				     "failed to get prepared PWM clock\n");
 
-	atmel_pwm->chip.dev = &pdev->dev;
-	atmel_pwm->chip.ops = &atmel_pwm_ops;
-	atmel_pwm->chip.npwm = 4;
+	chip->ops = &atmel_pwm_ops;
 
-	ret = atmel_pwm_enable_clk_if_on(&atmel_pwm->chip, true);
+	ret = atmel_pwm_enable_clk_if_on(chip, true);
 	if (ret < 0)
 		return ret;
 
-	ret = devm_pwmchip_add(&pdev->dev, &atmel_pwm->chip);
+	ret = devm_pwmchip_add(&pdev->dev, chip);
 	if (ret < 0) {
 		dev_err_probe(&pdev->dev, ret, "failed to add PWM chip\n");
 		goto disable_clk;
@@ -539,7 +537,7 @@ static int atmel_pwm_probe(struct platform_device *pdev)
 	return 0;
 
 disable_clk:
-	atmel_pwm_enable_clk_if_on(&atmel_pwm->chip, false);
+	atmel_pwm_enable_clk_if_on(chip, false);
 
 	return ret;
 }
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 048/115] pwm: atmel-tcb: Make use of devm_pwmchip_alloc() function

[Uwe Kleine-König]

This prepares the pwm-atmel-tcb 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.

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

diff --git a/drivers/pwm/pwm-atmel-tcb.c b/drivers/pwm/pwm-atmel-tcb.c
index 80121ab81a01..0663685532ec 100644
--- a/drivers/pwm/pwm-atmel-tcb.c
+++ b/drivers/pwm/pwm-atmel-tcb.c
@@ -47,7 +47,6 @@ struct atmel_tcb_channel {
 };
 
 struct atmel_tcb_pwm_chip {
-	struct pwm_chip chip;
 	spinlock_t lock;
 	u8 channel;
 	u8 width;
@@ -63,7 +62,7 @@ static const u8 atmel_tcb_divisors[] = { 2, 8, 32, 128, 0, };
 
 static inline struct atmel_tcb_pwm_chip *to_tcb_chip(struct pwm_chip *chip)
 {
-	return container_of(chip, struct atmel_tcb_pwm_chip, chip);
+	return pwmchip_get_drvdata(chip);
 }
 
 static int atmel_tcb_pwm_request(struct pwm_chip *chip,
@@ -389,6 +388,7 @@ static const struct of_device_id atmel_tcb_of_match[] = {
 static int atmel_tcb_pwm_probe(struct platform_device *pdev)
 {
 	const struct of_device_id *match;
+	struct pwm_chip *chip;
 	struct atmel_tcb_pwm_chip *tcbpwm;
 	const struct atmel_tcb_config *config;
 	struct device_node *np = pdev->dev.of_node;
@@ -396,9 +396,10 @@ static int atmel_tcb_pwm_probe(struct platform_device *pdev)
 	int err;
 	int channel;
 
-	tcbpwm = devm_kzalloc(&pdev->dev, sizeof(*tcbpwm), GFP_KERNEL);
-	if (tcbpwm == NULL)
-		return -ENOMEM;
+	chip = devm_pwmchip_alloc(&pdev->dev, NPWM, sizeof(*tcbpwm));
+	if (IS_ERR(chip))
+		return PTR_ERR(chip);
+	tcbpwm = to_tcb_chip(chip);
 
 	err = of_property_read_u32(np, "reg", &channel);
 	if (err < 0) {
@@ -436,9 +437,7 @@ static int atmel_tcb_pwm_probe(struct platform_device *pdev)
 		}
 	}
 
-	tcbpwm->chip.dev = &pdev->dev;
-	tcbpwm->chip.ops = &atmel_tcb_pwm_ops;
-	tcbpwm->chip.npwm = NPWM;
+	chip->ops = &atmel_tcb_pwm_ops;
 	tcbpwm->channel = channel;
 	tcbpwm->width = config->counter_width;
 
@@ -448,11 +447,11 @@ static int atmel_tcb_pwm_probe(struct platform_device *pdev)
 
 	spin_lock_init(&tcbpwm->lock);
 
-	err = pwmchip_add(&tcbpwm->chip);
+	err = pwmchip_add(chip);
 	if (err < 0)
 		goto err_disable_clk;
 
-	platform_set_drvdata(pdev, tcbpwm);
+	platform_set_drvdata(pdev, chip);
 
 	return 0;
 
@@ -473,9 +472,10 @@ static int atmel_tcb_pwm_probe(struct platform_device *pdev)
 
 static void atmel_tcb_pwm_remove(struct platform_device *pdev)
 {
-	struct atmel_tcb_pwm_chip *tcbpwm = platform_get_drvdata(pdev);
+	struct pwm_chip *chip = platform_get_drvdata(pdev);
+	struct atmel_tcb_pwm_chip *tcbpwm = to_tcb_chip(chip);
 
-	pwmchip_remove(&tcbpwm->chip);
+	pwmchip_remove(chip);
 
 	clk_disable_unprepare(tcbpwm->slow_clk);
 	clk_put(tcbpwm->gclk);
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 049/115] pwm: bcm2835: Make use of devm_pwmchip_alloc() function

[Uwe Kleine-König]

This prepares the pwm-bcm2835 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.

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

diff --git a/drivers/pwm/pwm-bcm2835.c b/drivers/pwm/pwm-bcm2835.c
index ab30667f4f95..826f0494dc5f 100644
--- a/drivers/pwm/pwm-bcm2835.c
+++ b/drivers/pwm/pwm-bcm2835.c
@@ -24,7 +24,6 @@
 #define PERIOD_MIN		0x2
 
 struct bcm2835_pwm {
-	struct pwm_chip chip;
 	struct device *dev;
 	void __iomem *base;
 	struct clk *clk;
@@ -32,7 +31,7 @@ struct bcm2835_pwm {
 
 static inline struct bcm2835_pwm *to_bcm2835_pwm(struct pwm_chip *chip)
 {
-	return container_of(chip, struct bcm2835_pwm, chip);
+	return pwmchip_get_drvdata(chip);
 }
 
 static int bcm2835_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
@@ -133,12 +132,14 @@ static const struct pwm_ops bcm2835_pwm_ops = {
 
 static int bcm2835_pwm_probe(struct platform_device *pdev)
 {
+	struct pwm_chip *chip;
 	struct bcm2835_pwm *pc;
 	int ret;
 
-	pc = devm_kzalloc(&pdev->dev, sizeof(*pc), GFP_KERNEL);
-	if (!pc)
-		return -ENOMEM;
+	chip = devm_pwmchip_alloc(&pdev->dev, 2, sizeof(*pc));
+	if (IS_ERR(chip))
+		return PTR_ERR(chip);
+	pc = to_bcm2835_pwm(chip);
 
 	pc->dev = &pdev->dev;
 
@@ -151,13 +152,11 @@ static int bcm2835_pwm_probe(struct platform_device *pdev)
 		return dev_err_probe(&pdev->dev, PTR_ERR(pc->clk),
 				     "clock not found\n");
 
-	pc->chip.dev = &pdev->dev;
-	pc->chip.ops = &bcm2835_pwm_ops;
-	pc->chip.npwm = 2;
+	chip->ops = &bcm2835_pwm_ops;
 
 	platform_set_drvdata(pdev, pc);
 
-	ret = devm_pwmchip_add(&pdev->dev, &pc->chip);
+	ret = devm_pwmchip_add(&pdev->dev, chip);
 	if (ret < 0)
 		return dev_err_probe(&pdev->dev, ret,
 				     "failed to add pwmchip\n");
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 050/115] pwm: bcm-iproc: Make use of devm_pwmchip_alloc() function

[Uwe Kleine-König]

This prepares the pwm-bcm-iproc 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.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/pwm/pwm-bcm-iproc.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/pwm/pwm-bcm-iproc.c b/drivers/pwm/pwm-bcm-iproc.c
index 758254025683..f4c9f10e490e 100644
--- a/drivers/pwm/pwm-bcm-iproc.c
+++ b/drivers/pwm/pwm-bcm-iproc.c
@@ -34,14 +34,13 @@
 #define IPROC_PWM_PRESCALE_MAX			0x3f
 
 struct iproc_pwmc {
-	struct pwm_chip chip;
 	void __iomem *base;
 	struct clk *clk;
 };
 
 static inline struct iproc_pwmc *to_iproc_pwmc(struct pwm_chip *chip)
 {
-	return container_of(chip, struct iproc_pwmc, chip);
+	return pwmchip_get_drvdata(chip);
 }
 
 static void iproc_pwmc_enable(struct iproc_pwmc *ip, unsigned int channel)
@@ -187,20 +186,20 @@ static const struct pwm_ops iproc_pwm_ops = {
 
 static int iproc_pwmc_probe(struct platform_device *pdev)
 {
+	struct pwm_chip *chip;
 	struct iproc_pwmc *ip;
 	unsigned int i;
 	u32 value;
 	int ret;
 
-	ip = devm_kzalloc(&pdev->dev, sizeof(*ip), GFP_KERNEL);
-	if (!ip)
-		return -ENOMEM;
+	chip = devm_pwmchip_alloc(&pdev->dev, 4, sizeof(*ip));
+	if (IS_ERR(chip))
+		return PTR_ERR(chip);
+	ip = to_iproc_pwmc(chip);
 
 	platform_set_drvdata(pdev, ip);
 
-	ip->chip.dev = &pdev->dev;
-	ip->chip.ops = &iproc_pwm_ops;
-	ip->chip.npwm = 4;
+	chip->ops = &iproc_pwm_ops;
 
 	ip->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(ip->base))
@@ -214,14 +213,14 @@ static int iproc_pwmc_probe(struct platform_device *pdev)
 	/* Set full drive and normal polarity for all channels */
 	value = readl(ip->base + IPROC_PWM_CTRL_OFFSET);
 
-	for (i = 0; i < ip->chip.npwm; i++) {
+	for (i = 0; i < chip->npwm; i++) {
 		value &= ~(1 << IPROC_PWM_CTRL_TYPE_SHIFT(i));
 		value |= 1 << IPROC_PWM_CTRL_POLARITY_SHIFT(i);
 	}
 
 	writel(value, ip->base + IPROC_PWM_CTRL_OFFSET);
 
-	ret = devm_pwmchip_add(&pdev->dev, &ip->chip);
+	ret = devm_pwmchip_add(&pdev->dev, chip);
 	if (ret < 0)
 		return dev_err_probe(&pdev->dev, ret,
 				     "failed to add PWM chip\n");
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 053/115] pwm: brcmstb: Make use of devm_pwmchip_alloc() function

[Uwe Kleine-König]

This prepares the pwm-brcmstb 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.

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

diff --git a/drivers/pwm/pwm-brcmstb.c b/drivers/pwm/pwm-brcmstb.c
index 0fdeb0b2dbf3..82d27d07ba91 100644
--- a/drivers/pwm/pwm-brcmstb.c
+++ b/drivers/pwm/pwm-brcmstb.c
@@ -54,7 +54,6 @@
 struct brcmstb_pwm {
 	void __iomem *base;
 	struct clk *clk;
-	struct pwm_chip chip;
 };
 
 static inline u32 brcmstb_pwm_readl(struct brcmstb_pwm *p,
@@ -77,7 +76,7 @@ static inline void brcmstb_pwm_writel(struct brcmstb_pwm *p, u32 value,
 
 static inline struct brcmstb_pwm *to_brcmstb_pwm(struct pwm_chip *chip)
 {
-	return container_of(chip, struct brcmstb_pwm, chip);
+	return pwmchip_get_drvdata(chip);
 }
 
 /*
@@ -230,12 +229,14 @@ MODULE_DEVICE_TABLE(of, brcmstb_pwm_of_match);
 
 static int brcmstb_pwm_probe(struct platform_device *pdev)
 {
+	struct pwm_chip *chip;
 	struct brcmstb_pwm *p;
 	int ret;
 
-	p = devm_kzalloc(&pdev->dev, sizeof(*p), GFP_KERNEL);
-	if (!p)
-		return -ENOMEM;
+	chip = devm_pwmchip_alloc(&pdev->dev, 2, sizeof(*p));
+	if (IS_ERR(chip))
+		return PTR_ERR(chip);
+	p = to_brcmstb_pwm(chip);
 
 	p->clk = devm_clk_get_enabled(&pdev->dev, NULL);
 	if (IS_ERR(p->clk))
@@ -244,15 +245,13 @@ static int brcmstb_pwm_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, p);
 
-	p->chip.dev = &pdev->dev;
-	p->chip.ops = &brcmstb_pwm_ops;
-	p->chip.npwm = 2;
+	chip->ops = &brcmstb_pwm_ops;
 
 	p->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(p->base))
 		return PTR_ERR(p->base);
 
-	ret = devm_pwmchip_add(&pdev->dev, &p->chip);
+	ret = devm_pwmchip_add(&pdev->dev, chip);
 	if (ret)
 		return dev_err_probe(&pdev->dev, ret, "failed to add PWM chip\n");
 
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 055/115] pwm: clps711x: Make use of devm_pwmchip_alloc() function

[Uwe Kleine-König]

This prepares the pwm-clps711x 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.

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

diff --git a/drivers/pwm/pwm-clps711x.c b/drivers/pwm/pwm-clps711x.c
index 42179b3f7ec3..9b80077fef20 100644
--- a/drivers/pwm/pwm-clps711x.c
+++ b/drivers/pwm/pwm-clps711x.c
@@ -12,7 +12,6 @@
 #include <linux/pwm.h>
 
 struct clps711x_chip {
-	struct pwm_chip chip;
 	void __iomem *pmpcon;
 	struct clk *clk;
 	spinlock_t lock;
@@ -20,7 +19,7 @@ struct clps711x_chip {
 
 static inline struct clps711x_chip *to_clps711x_chip(struct pwm_chip *chip)
 {
-	return container_of(chip, struct clps711x_chip, chip);
+	return pwmchip_get_drvdata(chip);
 }
 
 static int clps711x_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
@@ -85,11 +84,13 @@ static struct pwm_device *clps711x_pwm_xlate(struct pwm_chip *chip,
 
 static int clps711x_pwm_probe(struct platform_device *pdev)
 {
+	struct pwm_chip *chip;
 	struct clps711x_chip *priv;
 
-	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
-	if (!priv)
-		return -ENOMEM;
+	chip = devm_pwmchip_alloc(&pdev->dev, 2, sizeof(*priv));
+	if (IS_ERR(chip))
+		return PTR_ERR(chip);
+	priv = to_clps711x_chip(chip);
 
 	priv->pmpcon = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(priv->pmpcon))
@@ -99,15 +100,13 @@ static int clps711x_pwm_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->clk))
 		return PTR_ERR(priv->clk);
 
-	priv->chip.ops = &clps711x_pwm_ops;
-	priv->chip.dev = &pdev->dev;
-	priv->chip.npwm = 2;
-	priv->chip.of_xlate = clps711x_pwm_xlate;
-	priv->chip.of_pwm_n_cells = 1;
+	chip->ops = &clps711x_pwm_ops;
+	chip->of_xlate = clps711x_pwm_xlate;
+	chip->of_pwm_n_cells = 1;
 
 	spin_lock_init(&priv->lock);
 
-	return devm_pwmchip_add(&pdev->dev, &priv->chip);
+	return devm_pwmchip_add(&pdev->dev, chip);
 }
 
 static const struct of_device_id __maybe_unused clps711x_pwm_dt_ids[] = {
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 063/115] pwm: imx1: Make use of devm_pwmchip_alloc() function

[Uwe Kleine-König]

This prepares the pwm-imx1 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.

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

diff --git a/drivers/pwm/pwm-imx1.c b/drivers/pwm/pwm-imx1.c
index d175d895f22a..1d2aae2d278f 100644
--- a/drivers/pwm/pwm-imx1.c
+++ b/drivers/pwm/pwm-imx1.c
@@ -28,10 +28,12 @@ struct pwm_imx1_chip {
 	struct clk *clk_ipg;
 	struct clk *clk_per;
 	void __iomem *mmio_base;
-	struct pwm_chip chip;
 };
 
-#define to_pwm_imx1_chip(chip)	container_of(chip, struct pwm_imx1_chip, chip)
+static inline struct pwm_imx1_chip *to_pwm_imx1_chip(struct pwm_chip *chip)
+{
+	return pwmchip_get_drvdata(chip);
+}
 
 static int pwm_imx1_clk_prepare_enable(struct pwm_chip *chip)
 {
@@ -156,11 +158,13 @@ MODULE_DEVICE_TABLE(of, pwm_imx1_dt_ids);
 
 static int pwm_imx1_probe(struct platform_device *pdev)
 {
+	struct pwm_chip *chip;
 	struct pwm_imx1_chip *imx;
 
-	imx = devm_kzalloc(&pdev->dev, sizeof(*imx), GFP_KERNEL);
-	if (!imx)
-		return -ENOMEM;
+	chip = devm_pwmchip_alloc(&pdev->dev, 1, sizeof(*imx));
+	if (IS_ERR(chip))
+		return PTR_ERR(chip);
+	imx = to_pwm_imx1_chip(chip);
 
 	imx->clk_ipg = devm_clk_get(&pdev->dev, "ipg");
 	if (IS_ERR(imx->clk_ipg))
@@ -172,15 +176,13 @@ static int pwm_imx1_probe(struct platform_device *pdev)
 		return dev_err_probe(&pdev->dev, PTR_ERR(imx->clk_per),
 				     "failed to get peripheral clock\n");
 
-	imx->chip.ops = &pwm_imx1_ops;
-	imx->chip.dev = &pdev->dev;
-	imx->chip.npwm = 1;
+	chip->ops = &pwm_imx1_ops;
 
 	imx->mmio_base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(imx->mmio_base))
 		return PTR_ERR(imx->mmio_base);
 
-	return devm_pwmchip_add(&pdev->dev, &imx->chip);
+	return devm_pwmchip_add(&pdev->dev, chip);
 }
 
 static struct platform_driver pwm_imx1_driver = {
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 064/115] pwm: imx27: Make use of devm_pwmchip_alloc() function

[Uwe Kleine-König]

This prepares the pwm-imx27 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.

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/pwm/pwm-imx27.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/pwm/pwm-imx27.c b/drivers/pwm/pwm-imx27.c
index 5d796453519a..54589f0b3b78 100644
--- a/drivers/pwm/pwm-imx27.c
+++ b/drivers/pwm/pwm-imx27.c
@@ -83,7 +83,6 @@ struct pwm_imx27_chip {
 	struct clk	*clk_ipg;
 	struct clk	*clk_per;
 	void __iomem	*mmio_base;
-	struct pwm_chip	chip;
 
 	/*
 	 * The driver cannot read the current duty cycle from the hardware if
@@ -93,7 +92,10 @@ struct pwm_imx27_chip {
 	unsigned int duty_cycle;
 };
 
-#define to_pwm_imx27_chip(chip)	container_of(chip, struct pwm_imx27_chip, chip)
+static inline struct pwm_imx27_chip *to_pwm_imx27_chip(struct pwm_chip *chip)
+{
+	return pwmchip_get_drvdata(chip);
+}
 
 static int pwm_imx27_clk_prepare_enable(struct pwm_imx27_chip *imx)
 {
@@ -306,13 +308,15 @@ MODULE_DEVICE_TABLE(of, pwm_imx27_dt_ids);
 
 static int pwm_imx27_probe(struct platform_device *pdev)
 {
+	struct pwm_chip *chip;
 	struct pwm_imx27_chip *imx;
 	int ret;
 	u32 pwmcr;
 
-	imx = devm_kzalloc(&pdev->dev, sizeof(*imx), GFP_KERNEL);
-	if (imx == NULL)
-		return -ENOMEM;
+	chip = devm_pwmchip_alloc(&pdev->dev, 1, sizeof(*imx));
+	if (IS_ERR(chip))
+		return PTR_ERR(chip);
+	imx = to_pwm_imx27_chip(chip);
 
 	imx->clk_ipg = devm_clk_get(&pdev->dev, "ipg");
 	if (IS_ERR(imx->clk_ipg))
@@ -324,9 +328,7 @@ static int pwm_imx27_probe(struct platform_device *pdev)
 		return dev_err_probe(&pdev->dev, PTR_ERR(imx->clk_per),
 				     "failed to get peripheral clock\n");
 
-	imx->chip.ops = &pwm_imx27_ops;
-	imx->chip.dev = &pdev->dev;
-	imx->chip.npwm = 1;
+	chip->ops = &pwm_imx27_ops;
 
 	imx->mmio_base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(imx->mmio_base))
@@ -341,7 +343,7 @@ static int pwm_imx27_probe(struct platform_device *pdev)
 	if (!(pwmcr & MX3_PWMCR_EN))
 		pwm_imx27_clk_disable_unprepare(imx);
 
-	return devm_pwmchip_add(&pdev->dev, &imx->chip);
+	return devm_pwmchip_add(&pdev->dev, chip);
 }
 
 static struct platform_driver imx_pwm_driver = {
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 065/115] pwm: imx-tpm: Make use of devm_pwmchip_alloc() function

[Uwe Kleine-König]

This prepares the pwm-tmp 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.

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

diff --git a/drivers/pwm/pwm-imx-tpm.c b/drivers/pwm/pwm-imx-tpm.c
index 9fc290e647e1..04f8ab315748 100644
--- a/drivers/pwm/pwm-imx-tpm.c
+++ b/drivers/pwm/pwm-imx-tpm.c
@@ -57,7 +57,6 @@
 #define PWM_IMX_TPM_MOD_MOD	GENMASK(PWM_IMX_TPM_MOD_WIDTH - 1, 0)
 
 struct imx_tpm_pwm_chip {
-	struct pwm_chip chip;
 	struct clk *clk;
 	void __iomem *base;
 	struct mutex lock;
@@ -75,7 +74,7 @@ struct imx_tpm_pwm_param {
 static inline struct imx_tpm_pwm_chip *
 to_imx_tpm_pwm_chip(struct pwm_chip *chip)
 {
-	return container_of(chip, struct imx_tpm_pwm_chip, chip);
+	return pwmchip_get_drvdata(chip);
 }
 
 /*
@@ -336,35 +335,40 @@ static const struct pwm_ops imx_tpm_pwm_ops = {
 
 static int pwm_imx_tpm_probe(struct platform_device *pdev)
 {
+	struct pwm_chip *chip;
 	struct imx_tpm_pwm_chip *tpm;
+	void __iomem *base;
 	int ret;
+	unsigned int npwm;
 	u32 val;
 
-	tpm = devm_kzalloc(&pdev->dev, sizeof(*tpm), GFP_KERNEL);
-	if (!tpm)
-		return -ENOMEM;
+	base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	/* get number of channels */
+	val = readl(tpm->base + PWM_IMX_TPM_PARAM);
+	npwm = FIELD_GET(PWM_IMX_TPM_PARAM_CHAN, val);
+
+	chip = devm_pwmchip_alloc(&pdev->dev, npwm, sizeof(*tpm));
+	if (IS_ERR(chip))
+		return PTR_ERR(chip);
+	tpm = to_imx_tpm_pwm_chip(chip);
 
 	platform_set_drvdata(pdev, tpm);
 
-	tpm->base = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(tpm->base))
-		return PTR_ERR(tpm->base);
+	tpm->base = base;
 
 	tpm->clk = devm_clk_get_enabled(&pdev->dev, NULL);
 	if (IS_ERR(tpm->clk))
 		return dev_err_probe(&pdev->dev, PTR_ERR(tpm->clk),
 				     "failed to get PWM clock\n");
 
-	tpm->chip.dev = &pdev->dev;
-	tpm->chip.ops = &imx_tpm_pwm_ops;
-
-	/* get number of channels */
-	val = readl(tpm->base + PWM_IMX_TPM_PARAM);
-	tpm->chip.npwm = FIELD_GET(PWM_IMX_TPM_PARAM_CHAN, val);
+	chip->ops = &imx_tpm_pwm_ops;
 
 	mutex_init(&tpm->lock);
 
-	ret = devm_pwmchip_add(&pdev->dev, &tpm->chip);
+	ret = devm_pwmchip_add(&pdev->dev, chip);
 	if (ret)
 		return dev_err_probe(&pdev->dev, ret, "failed to add PWM chip\n");
 
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 071/115] pwm: lpc18xx-sct: Make use of devm_pwmchip_alloc() function

[Uwe Kleine-König]

This prepares the pwm-lpc18xx-sct 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.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/pwm/pwm-lpc18xx-sct.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/pwm/pwm-lpc18xx-sct.c b/drivers/pwm/pwm-lpc18xx-sct.c
index 9e1db43a5171..39bfcd5b6c29 100644
--- a/drivers/pwm/pwm-lpc18xx-sct.c
+++ b/drivers/pwm/pwm-lpc18xx-sct.c
@@ -93,7 +93,6 @@ struct lpc18xx_pwm_data {
 
 struct lpc18xx_pwm_chip {
 	struct device *parent;
-	struct pwm_chip chip;
 	void __iomem *base;
 	struct clk *pwm_clk;
 	unsigned long clk_rate;
@@ -110,7 +109,7 @@ struct lpc18xx_pwm_chip {
 static inline struct lpc18xx_pwm_chip *
 to_lpc18xx_pwm_chip(struct pwm_chip *chip)
 {
-	return container_of(chip, struct lpc18xx_pwm_chip, chip);
+	return pwmchip_get_drvdata(chip);
 }
 
 static inline void lpc18xx_pwm_writel(struct lpc18xx_pwm_chip *lpc18xx_pwm,
@@ -351,14 +350,15 @@ MODULE_DEVICE_TABLE(of, lpc18xx_pwm_of_match);
 
 static int lpc18xx_pwm_probe(struct platform_device *pdev)
 {
+	struct pwm_chip *chip;
 	struct lpc18xx_pwm_chip *lpc18xx_pwm;
 	int ret;
 	u64 val;
 
-	lpc18xx_pwm = devm_kzalloc(&pdev->dev, sizeof(*lpc18xx_pwm),
-				   GFP_KERNEL);
-	if (!lpc18xx_pwm)
-		return -ENOMEM;
+	chip = devm_pwmchip_alloc(&pdev->dev, LPC18XX_NUM_PWMS, sizeof(*lpc18xx_pwm));
+	if (IS_ERR(chip))
+		return PTR_ERR(chip);
+	lpc18xx_pwm = to_lpc18xx_pwm_chip(chip);
 
 	lpc18xx_pwm->parent = &pdev->dev;
 
@@ -391,9 +391,7 @@ static int lpc18xx_pwm_probe(struct platform_device *pdev)
 	lpc18xx_pwm->min_period_ns = DIV_ROUND_UP(NSEC_PER_SEC,
 						  lpc18xx_pwm->clk_rate);
 
-	lpc18xx_pwm->chip.dev = &pdev->dev;
-	lpc18xx_pwm->chip.ops = &lpc18xx_pwm_ops;
-	lpc18xx_pwm->chip.npwm = LPC18XX_NUM_PWMS;
+	chip->ops = &lpc18xx_pwm_ops;
 
 	/* SCT counter must be in unify (32 bit) mode */
 	lpc18xx_pwm_writel(lpc18xx_pwm, LPC18XX_PWM_CONFIG,
@@ -425,21 +423,22 @@ static int lpc18xx_pwm_probe(struct platform_device *pdev)
 	val |= LPC18XX_PWM_PRE(0);
 	lpc18xx_pwm_writel(lpc18xx_pwm, LPC18XX_PWM_CTRL, val);
 
-	ret = pwmchip_add(&lpc18xx_pwm->chip);
+	ret = pwmchip_add(chip);
 	if (ret < 0)
 		return dev_err_probe(&pdev->dev, ret, "pwmchip_add failed\n");
 
-	platform_set_drvdata(pdev, lpc18xx_pwm);
+	platform_set_drvdata(pdev, chip);
 
 	return 0;
 }
 
 static void lpc18xx_pwm_remove(struct platform_device *pdev)
 {
-	struct lpc18xx_pwm_chip *lpc18xx_pwm = platform_get_drvdata(pdev);
+	struct pwm_chip *chip = platform_get_drvdata(pdev);
+	struct lpc18xx_pwm_chip *lpc18xx_pwm = to_lpc18xx_pwm_chip(chip);
 	u32 val;
 
-	pwmchip_remove(&lpc18xx_pwm->chip);
+	pwmchip_remove(chip);
 
 	val = lpc18xx_pwm_readl(lpc18xx_pwm, LPC18XX_PWM_CTRL);
 	lpc18xx_pwm_writel(lpc18xx_pwm, LPC18XX_PWM_CTRL,
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 072/115] pwm: lpc32xx: Make use of devm_pwmchip_alloc() function

[Uwe Kleine-König]

This prepares the pwm-lpc32xx 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.

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

diff --git a/drivers/pwm/pwm-lpc32xx.c b/drivers/pwm/pwm-lpc32xx.c
index 1d9f3e7a2434..c748537e57d1 100644
--- a/drivers/pwm/pwm-lpc32xx.c
+++ b/drivers/pwm/pwm-lpc32xx.c
@@ -15,7 +15,6 @@
 #include <linux/slab.h>
 
 struct lpc32xx_pwm_chip {
-	struct pwm_chip chip;
 	struct clk *clk;
 	void __iomem *base;
 };
@@ -23,8 +22,10 @@ struct lpc32xx_pwm_chip {
 #define PWM_ENABLE	BIT(31)
 #define PWM_PIN_LEVEL	BIT(30)
 
-#define to_lpc32xx_pwm_chip(_chip) \
-	container_of(_chip, struct lpc32xx_pwm_chip, chip)
+static inline struct lpc32xx_pwm_chip *to_lpc32xx_pwm_chip(struct pwm_chip *chip)
+{
+	return pwmchip_get_drvdata(chip);
+}
 
 static int lpc32xx_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 			      int duty_ns, int period_ns)
@@ -119,13 +120,15 @@ static const struct pwm_ops lpc32xx_pwm_ops = {
 
 static int lpc32xx_pwm_probe(struct platform_device *pdev)
 {
+	struct pwm_chip *chip;
 	struct lpc32xx_pwm_chip *lpc32xx;
 	int ret;
 	u32 val;
 
-	lpc32xx = devm_kzalloc(&pdev->dev, sizeof(*lpc32xx), GFP_KERNEL);
-	if (!lpc32xx)
-		return -ENOMEM;
+	chip = devm_pwmchip_alloc(&pdev->dev, 1, sizeof(*lpc32xx));
+	if (IS_ERR(chip))
+		return PTR_ERR(chip);
+	lpc32xx = to_lpc32xx_pwm_chip(chip);
 
 	lpc32xx->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(lpc32xx->base))
@@ -135,16 +138,14 @@ static int lpc32xx_pwm_probe(struct platform_device *pdev)
 	if (IS_ERR(lpc32xx->clk))
 		return PTR_ERR(lpc32xx->clk);
 
-	lpc32xx->chip.dev = &pdev->dev;
-	lpc32xx->chip.ops = &lpc32xx_pwm_ops;
-	lpc32xx->chip.npwm = 1;
+	chip->ops = &lpc32xx_pwm_ops;
 
 	/* If PWM is disabled, configure the output to the default value */
 	val = readl(lpc32xx->base);
 	val &= ~PWM_PIN_LEVEL;
 	writel(val, lpc32xx->base);
 
-	ret = devm_pwmchip_add(&pdev->dev, &lpc32xx->chip);
+	ret = devm_pwmchip_add(&pdev->dev, chip);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "failed to add PWM chip, error %d\n", ret);
 		return ret;
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 074/115] pwm: mediatek: Make use of devm_pwmchip_alloc() function

[Uwe Kleine-König]

This prepares the pwm-mediatek 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.

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

diff --git a/drivers/pwm/pwm-mediatek.c b/drivers/pwm/pwm-mediatek.c
index d7d1c326e8cf..d4329affccda 100644
--- a/drivers/pwm/pwm-mediatek.c
+++ b/drivers/pwm/pwm-mediatek.c
@@ -51,7 +51,6 @@ struct pwm_mediatek_of_data {
  * @soc: pointer to chip's platform data
  */
 struct pwm_mediatek_chip {
-	struct pwm_chip chip;
 	void __iomem *regs;
 	struct clk *clk_top;
 	struct clk *clk_main;
@@ -70,7 +69,7 @@ static const unsigned int mtk_pwm_reg_offset_v2[] = {
 static inline struct pwm_mediatek_chip *
 to_pwm_mediatek_chip(struct pwm_chip *chip)
 {
-	return container_of(chip, struct pwm_mediatek_chip, chip);
+	return pwmchip_get_drvdata(chip);
 }
 
 static int pwm_mediatek_clk_enable(struct pwm_chip *chip,
@@ -233,21 +232,26 @@ static const struct pwm_ops pwm_mediatek_ops = {
 
 static int pwm_mediatek_probe(struct platform_device *pdev)
 {
+	struct pwm_chip *chip;
 	struct pwm_mediatek_chip *pc;
+	const struct pwm_mediatek_of_data *soc;
 	unsigned int i;
 	int ret;
 
-	pc = devm_kzalloc(&pdev->dev, sizeof(*pc), GFP_KERNEL);
-	if (!pc)
-		return -ENOMEM;
+	soc = of_device_get_match_data(&pdev->dev);
 
-	pc->soc = of_device_get_match_data(&pdev->dev);
+	chip = devm_pwmchip_alloc(&pdev->dev, soc->num_pwms, sizeof(*pc));
+	if (IS_ERR(chip))
+		return PTR_ERR(chip);
+	pc = to_pwm_mediatek_chip(chip);
+
+	pc->soc = soc;
 
 	pc->regs = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(pc->regs))
 		return PTR_ERR(pc->regs);
 
-	pc->clk_pwms = devm_kmalloc_array(&pdev->dev, pc->soc->num_pwms,
+	pc->clk_pwms = devm_kmalloc_array(&pdev->dev, soc->num_pwms,
 				    sizeof(*pc->clk_pwms), GFP_KERNEL);
 	if (!pc->clk_pwms)
 		return -ENOMEM;
@@ -262,7 +266,7 @@ static int pwm_mediatek_probe(struct platform_device *pdev)
 		return dev_err_probe(&pdev->dev, PTR_ERR(pc->clk_main),
 				     "Failed to get main clock\n");
 
-	for (i = 0; i < pc->soc->num_pwms; i++) {
+	for (i = 0; i < soc->num_pwms; i++) {
 		char name[8];
 
 		snprintf(name, sizeof(name), "pwm%d", i + 1);
@@ -273,11 +277,9 @@ static int pwm_mediatek_probe(struct platform_device *pdev)
 					     "Failed to get %s clock\n", name);
 	}
 
-	pc->chip.dev = &pdev->dev;
-	pc->chip.ops = &pwm_mediatek_ops;
-	pc->chip.npwm = pc->soc->num_pwms;
+	chip->ops = &pwm_mediatek_ops;
 
-	ret = devm_pwmchip_add(&pdev->dev, &pc->chip);
+	ret = devm_pwmchip_add(&pdev->dev, chip);
 	if (ret < 0)
 		return dev_err_probe(&pdev->dev, ret, "pwmchip_add() failed\n");
 
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 075/115] pwm: meson: Make use of devm_pwmchip_alloc() function

[Uwe Kleine-König]

This prepares the pwm-meson 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.

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

diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
index 640d6fe63799..187cd6f35cce 100644
--- a/drivers/pwm/pwm-meson.c
+++ b/drivers/pwm/pwm-meson.c
@@ -102,7 +102,6 @@ struct meson_pwm_data {
 };
 
 struct meson_pwm {
-	struct pwm_chip chip;
 	const struct meson_pwm_data *data;
 	struct meson_pwm_channel channels[MESON_NUM_PWMS];
 	void __iomem *base;
@@ -115,7 +114,7 @@ struct meson_pwm {
 
 static inline struct meson_pwm *to_meson_pwm(struct pwm_chip *chip)
 {
-	return container_of(chip, struct meson_pwm, chip);
+	return pwmchip_get_drvdata(chip);
 }
 
 static int meson_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
@@ -449,7 +448,7 @@ static int meson_pwm_init_channels(struct pwm_chip *chip)
 		mux_parent_data[i].name = meson->data->parent_names[i];
 	}
 
-	for (i = 0; i < meson->chip.npwm; i++) {
+	for (i = 0; i < chip->npwm; i++) {
 		struct meson_pwm_channel *channel = &meson->channels[i];
 		struct clk_parent_data div_parent = {}, gate_parent = {};
 		struct clk_init_data init = {};
@@ -535,29 +534,29 @@ static int meson_pwm_init_channels(struct pwm_chip *chip)
 
 static int meson_pwm_probe(struct platform_device *pdev)
 {
+	struct pwm_chip *chip;
 	struct meson_pwm *meson;
 	int err;
 
-	meson = devm_kzalloc(&pdev->dev, sizeof(*meson), GFP_KERNEL);
-	if (!meson)
-		return -ENOMEM;
+	chip = devm_pwmchip_alloc(&pdev->dev, MESON_NUM_PWMS, sizeof(*meson));
+	if (IS_ERR(chip))
+		return PTR_ERR(chip);
+	meson = to_meson_pwm(chip);
 
 	meson->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(meson->base))
 		return PTR_ERR(meson->base);
 
 	spin_lock_init(&meson->lock);
-	meson->chip.dev = &pdev->dev;
-	meson->chip.ops = &meson_pwm_ops;
-	meson->chip.npwm = MESON_NUM_PWMS;
+	chip->ops = &meson_pwm_ops;
 
 	meson->data = of_device_get_match_data(&pdev->dev);
 
-	err = meson_pwm_init_channels(&meson->chip);
+	err = meson_pwm_init_channels(chip);
 	if (err < 0)
 		return err;
 
-	err = devm_pwmchip_add(&pdev->dev, &meson->chip);
+	err = devm_pwmchip_add(&pdev->dev, chip);
 	if (err < 0) {
 		dev_err(&pdev->dev, "failed to register PWM chip: %d\n", err);
 		return err;
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 077/115] pwm: mtk-disp: Make use of devm_pwmchip_alloc() function

[Uwe Kleine-König]

This prepares the pwm-mtk-disp 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.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/pwm/pwm-mtk-disp.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/pwm/pwm-mtk-disp.c b/drivers/pwm/pwm-mtk-disp.c
index 186aa5c04435..bafd6b6195f6 100644
--- a/drivers/pwm/pwm-mtk-disp.c
+++ b/drivers/pwm/pwm-mtk-disp.c
@@ -42,7 +42,6 @@ struct mtk_pwm_data {
 };
 
 struct mtk_disp_pwm {
-	struct pwm_chip chip;
 	const struct mtk_pwm_data *data;
 	struct clk *clk_main;
 	struct clk *clk_mm;
@@ -52,7 +51,7 @@ struct mtk_disp_pwm {
 
 static inline struct mtk_disp_pwm *to_mtk_disp_pwm(struct pwm_chip *chip)
 {
-	return container_of(chip, struct mtk_disp_pwm, chip);
+	return pwmchip_get_drvdata(chip);
 }
 
 static void mtk_disp_pwm_update_bits(struct mtk_disp_pwm *mdp, u32 offset,
@@ -231,12 +230,14 @@ static const struct pwm_ops mtk_disp_pwm_ops = {
 
 static int mtk_disp_pwm_probe(struct platform_device *pdev)
 {
+	struct pwm_chip *chip;
 	struct mtk_disp_pwm *mdp;
 	int ret;
 
-	mdp = devm_kzalloc(&pdev->dev, sizeof(*mdp), GFP_KERNEL);
-	if (!mdp)
-		return -ENOMEM;
+	chip = devm_pwmchip_alloc(&pdev->dev, 1, sizeof(*mdp));
+	if (IS_ERR(chip))
+		return PTR_ERR(chip);
+	mdp = to_mtk_disp_pwm(chip);
 
 	mdp->data = of_device_get_match_data(&pdev->dev);
 
@@ -254,11 +255,9 @@ static int mtk_disp_pwm_probe(struct platform_device *pdev)
 		return dev_err_probe(&pdev->dev, PTR_ERR(mdp->clk_mm),
 				     "Failed to get mm clock\n");
 
-	mdp->chip.dev = &pdev->dev;
-	mdp->chip.ops = &mtk_disp_pwm_ops;
-	mdp->chip.npwm = 1;
+	chip->ops = &mtk_disp_pwm_ops;
 
-	ret = devm_pwmchip_add(&pdev->dev, &mdp->chip);
+	ret = devm_pwmchip_add(&pdev->dev, chip);
 	if (ret < 0)
 		return dev_err_probe(&pdev->dev, ret, "pwmchip_add() failed\n");
 
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 078/115] pwm: mxs: Make use of devm_pwmchip_alloc() function

[Uwe Kleine-König]

This prepares the pwm-mxs 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.

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

diff --git a/drivers/pwm/pwm-mxs.c b/drivers/pwm/pwm-mxs.c
index 1b5e787d78f1..8cad214b1c29 100644
--- a/drivers/pwm/pwm-mxs.c
+++ b/drivers/pwm/pwm-mxs.c
@@ -37,12 +37,14 @@ static const u8 cdiv_shift[PERIOD_CDIV_MAX] = {
 };
 
 struct mxs_pwm_chip {
-	struct pwm_chip chip;
 	struct clk *clk;
 	void __iomem *base;
 };
 
-#define to_mxs_pwm_chip(_chip) container_of(_chip, struct mxs_pwm_chip, chip)
+static inline struct mxs_pwm_chip *to_mxs_pwm_chip(struct pwm_chip *chip)
+{
+	return pwmchip_get_drvdata(chip);
+}
 
 static int mxs_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 			 const struct pwm_state *state)
@@ -120,12 +122,21 @@ static const struct pwm_ops mxs_pwm_ops = {
 static int mxs_pwm_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
+	struct pwm_chip *chip;
 	struct mxs_pwm_chip *mxs;
+	u32 npwm;
 	int ret;
 
-	mxs = devm_kzalloc(&pdev->dev, sizeof(*mxs), GFP_KERNEL);
-	if (!mxs)
-		return -ENOMEM;
+	ret = of_property_read_u32(np, "fsl,pwm-number", &npwm);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to get pwm number: %d\n", ret);
+		return ret;
+	}
+
+	chip = devm_pwmchip_alloc(&pdev->dev, npwm, sizeof(*mxs));
+	if (IS_ERR(chip))
+		return PTR_ERR(chip);
+	mxs = to_mxs_pwm_chip(chip);
 
 	mxs->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(mxs->base))
@@ -135,21 +146,14 @@ static int mxs_pwm_probe(struct platform_device *pdev)
 	if (IS_ERR(mxs->clk))
 		return PTR_ERR(mxs->clk);
 
-	mxs->chip.dev = &pdev->dev;
-	mxs->chip.ops = &mxs_pwm_ops;
-
-	ret = of_property_read_u32(np, "fsl,pwm-number", &mxs->chip.npwm);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "failed to get pwm number: %d\n", ret);
-		return ret;
-	}
+	chip->ops = &mxs_pwm_ops;
 
 	/* FIXME: Only do this if the PWM isn't already running */
 	ret = stmp_reset_block(mxs->base);
 	if (ret)
 		return dev_err_probe(&pdev->dev, ret, "failed to reset PWM\n");
 
-	ret = devm_pwmchip_add(&pdev->dev, &mxs->chip);
+	ret = devm_pwmchip_add(&pdev->dev, chip);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "failed to add pwm chip %d\n", ret);
 		return ret;
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 083/115] pwm: raspberrypi-poe: Make use of devm_pwmchip_alloc() function

[Uwe Kleine-König]

This prepares the pwm-raspberrypi-poe 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.

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

diff --git a/drivers/pwm/pwm-raspberrypi-poe.c b/drivers/pwm/pwm-raspberrypi-poe.c
index f10e8a624c51..8921e7ea2cea 100644
--- a/drivers/pwm/pwm-raspberrypi-poe.c
+++ b/drivers/pwm/pwm-raspberrypi-poe.c
@@ -27,7 +27,6 @@
 
 struct raspberrypi_pwm {
 	struct rpi_firmware *firmware;
-	struct pwm_chip chip;
 	unsigned int duty_cycle;
 };
 
@@ -40,7 +39,7 @@ struct raspberrypi_pwm_prop {
 static inline
 struct raspberrypi_pwm *raspberrypi_pwm_from_chip(struct pwm_chip *chip)
 {
-	return container_of(chip, struct raspberrypi_pwm, chip);
+	return pwmchip_get_drvdata(chip);
 }
 
 static int raspberrypi_pwm_set_property(struct rpi_firmware *firmware,
@@ -142,6 +141,7 @@ static int raspberrypi_pwm_probe(struct platform_device *pdev)
 	struct device_node *firmware_node;
 	struct device *dev = &pdev->dev;
 	struct rpi_firmware *firmware;
+	struct pwm_chip *chip;
 	struct raspberrypi_pwm *rpipwm;
 	int ret;
 
@@ -157,14 +157,14 @@ static int raspberrypi_pwm_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, -EPROBE_DEFER,
 				     "Failed to get firmware handle\n");
 
-	rpipwm = devm_kzalloc(&pdev->dev, sizeof(*rpipwm), GFP_KERNEL);
-	if (!rpipwm)
-		return -ENOMEM;
+	chip = devm_pwmchip_alloc(&pdev->dev, RASPBERRYPI_FIRMWARE_PWM_NUM,
+				  sizeof(*rpipwm));
+	if (IS_ERR(chip))
+		return PTR_ERR(chip);
+	rpipwm = raspberrypi_pwm_from_chip(chip);
 
 	rpipwm->firmware = firmware;
-	rpipwm->chip.dev = dev;
-	rpipwm->chip.ops = &raspberrypi_pwm_ops;
-	rpipwm->chip.npwm = RASPBERRYPI_FIRMWARE_PWM_NUM;
+	chip->ops = &raspberrypi_pwm_ops;
 
 	ret = raspberrypi_pwm_get_property(rpipwm->firmware, RPI_PWM_CUR_DUTY_REG,
 					   &rpipwm->duty_cycle);
@@ -173,7 +173,7 @@ static int raspberrypi_pwm_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	return devm_pwmchip_add(dev, &rpipwm->chip);
+	return devm_pwmchip_add(dev, chip);
 }
 
 static const struct of_device_id raspberrypi_pwm_of_match[] = {
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 086/115] pwm: rockchip: Make use of devm_pwmchip_alloc() function

[Uwe Kleine-König]

This prepares the pwm-rockchip 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.

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

diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c
index a7c647e37837..0fa7575dbb54 100644
--- a/drivers/pwm/pwm-rockchip.c
+++ b/drivers/pwm/pwm-rockchip.c
@@ -30,7 +30,6 @@
 #define PWM_LP_DISABLE		(0 << 8)
 
 struct rockchip_pwm_chip {
-	struct pwm_chip chip;
 	struct clk *clk;
 	struct clk *pclk;
 	const struct rockchip_pwm_data *data;
@@ -54,7 +53,7 @@ struct rockchip_pwm_data {
 
 static inline struct rockchip_pwm_chip *to_rockchip_pwm_chip(struct pwm_chip *chip)
 {
-	return container_of(chip, struct rockchip_pwm_chip, chip);
+	return pwmchip_get_drvdata(chip);
 }
 
 static int rockchip_pwm_get_state(struct pwm_chip *chip,
@@ -296,14 +295,16 @@ MODULE_DEVICE_TABLE(of, rockchip_pwm_dt_ids);
 
 static int rockchip_pwm_probe(struct platform_device *pdev)
 {
+	struct pwm_chip *chip;
 	struct rockchip_pwm_chip *pc;
 	u32 enable_conf, ctrl;
 	bool enabled;
 	int ret, count;
 
-	pc = devm_kzalloc(&pdev->dev, sizeof(*pc), GFP_KERNEL);
-	if (!pc)
-		return -ENOMEM;
+	chip = devm_pwmchip_alloc(&pdev->dev, 1, sizeof(*pc));
+	if (IS_ERR(chip))
+		return PTR_ERR(chip);
+	pc = to_rockchip_pwm_chip(chip);
 
 	pc->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(pc->base))
@@ -337,18 +338,16 @@ static int rockchip_pwm_probe(struct platform_device *pdev)
 		goto err_clk;
 	}
 
-	platform_set_drvdata(pdev, pc);
+	platform_set_drvdata(pdev, chip);
 
 	pc->data = device_get_match_data(&pdev->dev);
-	pc->chip.dev = &pdev->dev;
-	pc->chip.ops = &rockchip_pwm_ops;
-	pc->chip.npwm = 1;
+	chip->ops = &rockchip_pwm_ops;
 
 	enable_conf = pc->data->enable_conf;
 	ctrl = readl_relaxed(pc->base + pc->data->regs.ctrl);
 	enabled = (ctrl & enable_conf) == enable_conf;
 
-	ret = pwmchip_add(&pc->chip);
+	ret = pwmchip_add(chip);
 	if (ret < 0) {
 		dev_err_probe(&pdev->dev, ret, "pwmchip_add() failed\n");
 		goto err_pclk;
@@ -372,9 +371,10 @@ static int rockchip_pwm_probe(struct platform_device *pdev)
 
 static void rockchip_pwm_remove(struct platform_device *pdev)
 {
-	struct rockchip_pwm_chip *pc = platform_get_drvdata(pdev);
+	struct pwm_chip *chip = platform_get_drvdata(pdev);
+	struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
 
-	pwmchip_remove(&pc->chip);
+	pwmchip_remove(chip);
 
 	clk_unprepare(pc->pclk);
 	clk_unprepare(pc->clk);
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 086/115] pwm: rockchip: Make use of devm_pwmchip_alloc() function

[Heiko Stübner]

Am Mittwoch, 6. Dezember 2023, 12:44:40 CET schrieb Uwe Kleine-König:
> This prepares the pwm-rockchip 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.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

I looked the important patches up on lore (adding devm_pwmchip_alloc
for example) and the change looks correct, so

Reviewed-by: Heiko Stuebner <heiko@sntech.de>



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 088/115] pwm: samsung: Make use of devm_pwmchip_alloc() function

[Uwe Kleine-König]

This prepares the pwm-samsung 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.

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

diff --git a/drivers/pwm/pwm-samsung.c b/drivers/pwm/pwm-samsung.c
index a97cae49406e..fcbe464b0c38 100644
--- a/drivers/pwm/pwm-samsung.c
+++ b/drivers/pwm/pwm-samsung.c
@@ -80,7 +80,6 @@ struct samsung_pwm_channel {
  * @channel:		per channel driver data
  */
 struct samsung_pwm_chip {
-	struct pwm_chip chip;
 	struct samsung_pwm_variant variant;
 	u8 inverter_mask;
 	u8 disabled_mask;
@@ -110,7 +109,7 @@ static DEFINE_SPINLOCK(samsung_pwm_lock);
 static inline
 struct samsung_pwm_chip *to_samsung_pwm_chip(struct pwm_chip *chip)
 {
-	return container_of(chip, struct samsung_pwm_chip, chip);
+	return pwmchip_get_drvdata(chip);
 }
 
 static inline unsigned int to_tcon_channel(unsigned int channel)
@@ -181,9 +180,10 @@ static unsigned long pwm_samsung_get_tin_rate(struct samsung_pwm_chip *our_chip,
 	return rate / (reg + 1);
 }
 
-static unsigned long pwm_samsung_calc_tin(struct samsung_pwm_chip *our_chip,
+static unsigned long pwm_samsung_calc_tin(struct pwm_chip *chip,
 					  unsigned int chan, unsigned long freq)
 {
+	struct samsung_pwm_chip *our_chip = to_samsung_pwm_chip(chip);
 	struct samsung_pwm_variant *variant = &our_chip->variant;
 	unsigned long rate;
 	struct clk *clk;
@@ -197,12 +197,12 @@ static unsigned long pwm_samsung_calc_tin(struct samsung_pwm_chip *our_chip,
 				return rate;
 		}
 
-		dev_warn(pwmchip_parent(&our_chip->chip),
+		dev_warn(pwmchip_parent(chip),
 			"tclk of PWM %d is inoperational, using tdiv\n", chan);
 	}
 
 	rate = pwm_samsung_get_tin_rate(our_chip, chan);
-	dev_dbg(pwmchip_parent(&our_chip->chip), "tin parent at %lu\n", rate);
+	dev_dbg(pwmchip_parent(chip), "tin parent at %lu\n", rate);
 
 	/*
 	 * Compare minimum PWM frequency that can be achieved with possible
@@ -329,7 +329,7 @@ static int __pwm_samsung_config(struct pwm_chip *chip, struct pwm_device *pwm,
 		dev_dbg(pwmchip_parent(chip), "duty_ns=%d, period_ns=%d (%u)\n",
 						duty_ns, period_ns, period);
 
-		tin_rate = pwm_samsung_calc_tin(our_chip, pwm->hwpwm, period);
+		tin_rate = pwm_samsung_calc_tin(chip, pwm->hwpwm, period);
 
 		dev_dbg(pwmchip_parent(chip), "tin_rate=%lu\n", tin_rate);
 
@@ -506,9 +506,10 @@ static const struct of_device_id samsung_pwm_matches[] = {
 };
 MODULE_DEVICE_TABLE(of, samsung_pwm_matches);
 
-static int pwm_samsung_parse_dt(struct samsung_pwm_chip *our_chip)
+static int pwm_samsung_parse_dt(struct pwm_chip *chip)
 {
-	struct device_node *np = pwmchip_parent(&our_chip->chip)->of_node;
+	struct samsung_pwm_chip *our_chip = to_samsung_pwm_chip(chip);
+	struct device_node *np = pwmchip_parent(chip)->of_node;
 	const struct of_device_id *match;
 	struct property *prop;
 	const __be32 *cur;
@@ -522,7 +523,7 @@ static int pwm_samsung_parse_dt(struct samsung_pwm_chip *our_chip)
 
 	of_property_for_each_u32(np, "samsung,pwm-outputs", prop, cur, val) {
 		if (val >= SAMSUNG_PWM_NUM) {
-			dev_err(pwmchip_parent(&our_chip->chip),
+			dev_err(pwmchip_parent(chip),
 				"%s: invalid channel index in samsung,pwm-outputs property\n",
 								__func__);
 			continue;
@@ -533,7 +534,7 @@ static int pwm_samsung_parse_dt(struct samsung_pwm_chip *our_chip)
 	return 0;
 }
 #else
-static int pwm_samsung_parse_dt(struct samsung_pwm_chip *our_chip)
+static int pwm_samsung_parse_dt(struct pwm_chip *chip)
 {
 	return -ENODEV;
 }
@@ -542,21 +543,22 @@ static int pwm_samsung_parse_dt(struct samsung_pwm_chip *our_chip)
 static int pwm_samsung_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
+	struct pwm_chip *chip;
 	struct samsung_pwm_chip *our_chip;
 	unsigned int chan;
 	int ret;
 
-	our_chip = devm_kzalloc(&pdev->dev, sizeof(*our_chip), GFP_KERNEL);
-	if (our_chip == NULL)
+	chip = devm_pwmchip_alloc(&pdev->dev, SAMSUNG_PWM_NUM, sizeof(*chip));
+	if (chip == NULL)
 		return -ENOMEM;
+	our_chip = to_samsung_pwm_chip(chip);
+
+	chip->ops = &pwm_samsung_ops;
 
-	our_chip->chip.dev = &pdev->dev;
-	our_chip->chip.ops = &pwm_samsung_ops;
-	our_chip->chip.npwm = SAMSUNG_PWM_NUM;
 	our_chip->inverter_mask = BIT(SAMSUNG_PWM_NUM) - 1;
 
 	if (IS_ENABLED(CONFIG_OF) && pdev->dev.of_node) {
-		ret = pwm_samsung_parse_dt(our_chip);
+		ret = pwm_samsung_parse_dt(chip);
 		if (ret)
 			return ret;
 	} else {
@@ -595,7 +597,7 @@ static int pwm_samsung_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, our_chip);
 
-	ret = pwmchip_add(&our_chip->chip);
+	ret = pwmchip_add(chip);
 	if (ret < 0) {
 		dev_err(dev, "failed to register PWM chip\n");
 		clk_disable_unprepare(our_chip->base_clk);
@@ -612,17 +614,18 @@ static int pwm_samsung_probe(struct platform_device *pdev)
 
 static void pwm_samsung_remove(struct platform_device *pdev)
 {
-	struct samsung_pwm_chip *our_chip = platform_get_drvdata(pdev);
+	struct pwm_chip *chip = platform_get_drvdata(pdev);
+	struct samsung_pwm_chip *our_chip = to_samsung_pwm_chip(chip);
 
-	pwmchip_remove(&our_chip->chip);
+	pwmchip_remove(chip);
 
 	clk_disable_unprepare(our_chip->base_clk);
 }
 
 static int pwm_samsung_resume(struct device *dev)
 {
-	struct samsung_pwm_chip *our_chip = dev_get_drvdata(dev);
-	struct pwm_chip *chip = &our_chip->chip;
+	struct pwm_chip *chip = dev_get_drvdata(dev);
+	struct samsung_pwm_chip *our_chip = to_samsung_pwm_chip(chip);
 	unsigned int i;
 
 	for (i = 0; i < SAMSUNG_PWM_NUM; i++) {
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 094/115] pwm: stm32-lp: Make use of devm_pwmchip_alloc() function

[Uwe Kleine-König]

This prepares the pwm-stm32-lp 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.

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

diff --git a/drivers/pwm/pwm-stm32-lp.c b/drivers/pwm/pwm-stm32-lp.c
index 3754faeca838..579fea9a0da4 100644
--- a/drivers/pwm/pwm-stm32-lp.c
+++ b/drivers/pwm/pwm-stm32-lp.c
@@ -18,14 +18,13 @@
 #include <linux/pwm.h>
 
 struct stm32_pwm_lp {
-	struct pwm_chip chip;
 	struct clk *clk;
 	struct regmap *regmap;
 };
 
 static inline struct stm32_pwm_lp *to_stm32_pwm_lp(struct pwm_chip *chip)
 {
-	return container_of(chip, struct stm32_pwm_lp, chip);
+	return pwmchip_get_drvdata(chip);
 }
 
 /* STM32 Low-Power Timer is preceded by a configurable power-of-2 prescaler */
@@ -196,20 +195,20 @@ static const struct pwm_ops stm32_pwm_lp_ops = {
 static int stm32_pwm_lp_probe(struct platform_device *pdev)
 {
 	struct stm32_lptimer *ddata = dev_get_drvdata(pdev->dev.parent);
+	struct pwm_chip *chip;
 	struct stm32_pwm_lp *priv;
 	int ret;
 
-	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
-	if (!priv)
-		return -ENOMEM;
+	chip = devm_pwmchip_alloc(&pdev->dev, 1, sizeof(*priv));
+	if (IS_ERR(chip))
+		return PTR_ERR(chip);
+	priv = to_stm32_pwm_lp(chip);
 
 	priv->regmap = ddata->regmap;
 	priv->clk = ddata->clk;
-	priv->chip.dev = &pdev->dev;
-	priv->chip.ops = &stm32_pwm_lp_ops;
-	priv->chip.npwm = 1;
+	chip->ops = &stm32_pwm_lp_ops;
 
-	ret = devm_pwmchip_add(&pdev->dev, &priv->chip);
+	ret = devm_pwmchip_add(&pdev->dev, chip);
 	if (ret < 0)
 		return ret;
 
@@ -220,13 +219,13 @@ static int stm32_pwm_lp_probe(struct platform_device *pdev)
 
 static int stm32_pwm_lp_suspend(struct device *dev)
 {
-	struct stm32_pwm_lp *priv = dev_get_drvdata(dev);
+	struct pwm_chip *chip = dev_get_drvdata(dev);
 	struct pwm_state state;
 
-	pwm_get_state(&priv->chip.pwms[0], &state);
+	pwm_get_state(&chip->pwms[0], &state);
 	if (state.enabled) {
 		dev_err(dev, "The consumer didn't stop us (%s)\n",
-			priv->chip.pwms[0].label);
+			chip->pwms[0].label);
 		return -EBUSY;
 	}
 
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 095/115] pwm: stm32: Make use of devm_pwmchip_alloc() function

[Uwe Kleine-König]

This prepares the pwm-stm32 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.

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

diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c
index d1dc9e3ca2a1..aae8bd4f40ab 100644
--- a/drivers/pwm/pwm-stm32.c
+++ b/drivers/pwm/pwm-stm32.c
@@ -27,7 +27,6 @@ struct stm32_breakinput {
 };
 
 struct stm32_pwm {
-	struct pwm_chip chip;
 	struct mutex lock; /* protect pwm config/enable */
 	struct clk *clk;
 	struct regmap *regmap;
@@ -40,7 +39,7 @@ struct stm32_pwm {
 
 static inline struct stm32_pwm *to_stm32_pwm_dev(struct pwm_chip *chip)
 {
-	return container_of(chip, struct stm32_pwm, chip);
+	return pwmchip_get_drvdata(chip);
 }
 
 static u32 active_channels(struct stm32_pwm *dev)
@@ -90,11 +89,12 @@ static u32 active_channels(struct stm32_pwm *dev)
  * - Period     = t2 - t0
  * - Duty cycle = t1 - t0
  */
-static int stm32_pwm_raw_capture(struct stm32_pwm *priv, struct pwm_device *pwm,
+static int stm32_pwm_raw_capture(struct pwm_chip *chip, struct pwm_device *pwm,
 				 unsigned long tmo_ms, u32 *raw_prd,
 				 u32 *raw_dty)
 {
-	struct device *parent = pwmchip_parent(&priv->chip)->parent;
+	struct stm32_pwm *priv = to_stm32_pwm_dev(chip);
+	struct device *parent = pwmchip_parent(chip)->parent;
 	enum stm32_timers_dmas dma_id;
 	u32 ccen, ccr;
 	int ret;
@@ -208,7 +208,7 @@ static int stm32_pwm_capture(struct pwm_chip *chip, struct pwm_device *pwm,
 			   TIM_CCER_CC12P : TIM_CCER_CC34P, pwm->hwpwm < 2 ?
 			   TIM_CCER_CC2P : TIM_CCER_CC4P);
 
-	ret = stm32_pwm_raw_capture(priv, pwm, tmo_ms, &raw_prd, &raw_dty);
+	ret = stm32_pwm_raw_capture(chip, pwm, tmo_ms, &raw_prd, &raw_dty);
 	if (ret)
 		goto stop;
 
@@ -229,7 +229,7 @@ static int stm32_pwm_capture(struct pwm_chip *chip, struct pwm_device *pwm,
 		/* 2nd measure with new scale */
 		psc /= scale;
 		regmap_write(priv->regmap, TIM_PSC, psc);
-		ret = stm32_pwm_raw_capture(priv, pwm, tmo_ms, &raw_prd,
+		ret = stm32_pwm_raw_capture(chip, pwm, tmo_ms, &raw_prd,
 					    &raw_dty);
 		if (ret)
 			goto stop;
@@ -257,7 +257,7 @@ static int stm32_pwm_capture(struct pwm_chip *chip, struct pwm_device *pwm,
 			   FIELD_PREP(TIM_CCMR_IC1PSC, icpsc) |
 			   FIELD_PREP(TIM_CCMR_IC2PSC, icpsc));
 
-	ret = stm32_pwm_raw_capture(priv, pwm, tmo_ms, &raw_prd, &raw_dty);
+	ret = stm32_pwm_raw_capture(chip, pwm, tmo_ms, &raw_prd, &raw_dty);
 	if (ret)
 		goto stop;
 
@@ -605,7 +605,7 @@ static void stm32_pwm_detect_complementary(struct stm32_pwm *priv)
 	priv->have_complementary_output = (ccer != 0);
 }
 
-static unsigned int stm32_pwm_detect_channels(struct stm32_pwm *priv,
+static unsigned int stm32_pwm_detect_channels(struct stm32_timers *ddata,
 					      unsigned int *num_enabled)
 {
 	u32 ccer, ccer_backup;
@@ -614,10 +614,10 @@ static unsigned int stm32_pwm_detect_channels(struct stm32_pwm *priv,
 	 * If channels enable bits don't exist writing 1 will have no
 	 * effect so we can detect and count them.
 	 */
-	regmap_read(priv->regmap, TIM_CCER, &ccer_backup);
-	regmap_set_bits(priv->regmap, TIM_CCER, TIM_CCER_CCXE);
-	regmap_read(priv->regmap, TIM_CCER, &ccer);
-	regmap_write(priv->regmap, TIM_CCER, ccer_backup);
+	regmap_read(ddata->regmap, TIM_CCER, &ccer_backup);
+	regmap_set_bits(ddata->regmap, TIM_CCER, TIM_CCER_CCXE);
+	regmap_read(ddata->regmap, TIM_CCER, &ccer);
+	regmap_write(ddata->regmap, TIM_CCER, ccer_backup);
 
 	*num_enabled = hweight32(ccer_backup & TIM_CCER_CCXE);
 
@@ -632,11 +632,14 @@ static int stm32_pwm_probe(struct platform_device *pdev)
 	struct stm32_pwm *priv;
 	unsigned int num_enabled;
 	unsigned int i;
+	struct pwm_chip *chip;
 	int ret;
 
-	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
-	if (!priv)
-		return -ENOMEM;
+	chip = devm_pwmchip_alloc(dev, stm32_pwm_detect_channels(ddata, &num_enabled),
+				  sizeof(*priv));
+	if (IS_ERR(chip))
+		return PTR_ERR(chip);
+	priv = to_stm32_pwm_dev(chip);
 
 	mutex_init(&priv->lock);
 	priv->regmap = ddata->regmap;
@@ -652,37 +655,36 @@ static int stm32_pwm_probe(struct platform_device *pdev)
 
 	stm32_pwm_detect_complementary(priv);
 
-	priv->chip.dev = dev;
-	priv->chip.ops = &stm32pwm_ops;
-	priv->chip.npwm = stm32_pwm_detect_channels(priv, &num_enabled);
+	chip->ops = &stm32pwm_ops;
 
 	/* Initialize clock refcount to number of enabled PWM channels. */
 	for (i = 0; i < num_enabled; i++)
 		clk_enable(priv->clk);
 
-	ret = devm_pwmchip_add(dev, &priv->chip);
+	ret = devm_pwmchip_add(dev, chip);
 	if (ret < 0)
 		return ret;
 
-	platform_set_drvdata(pdev, priv);
+	platform_set_drvdata(pdev, chip);
 
 	return 0;
 }
 
 static int stm32_pwm_suspend(struct device *dev)
 {
-	struct stm32_pwm *priv = dev_get_drvdata(dev);
+	struct pwm_chip *chip = dev_get_drvdata(dev);
+	struct stm32_pwm *priv = to_stm32_pwm_dev(chip);
 	unsigned int i;
 	u32 ccer, mask;
 
 	/* Look for active channels */
 	ccer = active_channels(priv);
 
-	for (i = 0; i < priv->chip.npwm; i++) {
+	for (i = 0; i < chip->npwm; i++) {
 		mask = TIM_CCER_CC1E << (i * 4);
 		if (ccer & mask) {
 			dev_err(dev, "PWM %u still in use by consumer %s\n",
-				i, priv->chip.pwms[i].label);
+				i, chip->pwms[i].label);
 			return -EBUSY;
 		}
 	}
@@ -692,7 +694,8 @@ static int stm32_pwm_suspend(struct device *dev)
 
 static int stm32_pwm_resume(struct device *dev)
 {
-	struct stm32_pwm *priv = dev_get_drvdata(dev);
+	struct pwm_chip *chip = dev_get_drvdata(dev);
+	struct stm32_pwm *priv = to_stm32_pwm_dev(chip);
 	int ret;
 
 	ret = pinctrl_pm_select_default_state(dev);
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 096/115] pwm: stmpe: Make use of devm_pwmchip_alloc() function

[Uwe Kleine-König]

This prepares the pwm-stmpe 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.

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

diff --git a/drivers/pwm/pwm-stmpe.c b/drivers/pwm/pwm-stmpe.c
index 941b1e218e35..b9d06bcc6e93 100644
--- a/drivers/pwm/pwm-stmpe.c
+++ b/drivers/pwm/pwm-stmpe.c
@@ -27,13 +27,12 @@
 
 struct stmpe_pwm {
 	struct stmpe *stmpe;
-	struct pwm_chip chip;
 	u8 last_duty;
 };
 
 static inline struct stmpe_pwm *to_stmpe_pwm(struct pwm_chip *chip)
 {
-	return container_of(chip, struct stmpe_pwm, chip);
+	return pwmchip_get_drvdata(chip);
 }
 
 static int stmpe_24xx_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
@@ -292,33 +291,36 @@ static const struct pwm_ops stmpe_24xx_pwm_ops = {
 static int __init stmpe_pwm_probe(struct platform_device *pdev)
 {
 	struct stmpe *stmpe = dev_get_drvdata(pdev->dev.parent);
+	struct pwm_chip *chip;
 	struct stmpe_pwm *stmpe_pwm;
 	int ret;
 
-	stmpe_pwm = devm_kzalloc(&pdev->dev, sizeof(*stmpe_pwm), GFP_KERNEL);
-	if (!stmpe_pwm)
-		return -ENOMEM;
+	switch (stmpe->partnum) {
+	case STMPE2401:
+	case STMPE2403:
+		break;
+	case STMPE1601:
+		return dev_err_probe(&pdev->dev, -EINVAL,
+				     "STMPE1601 not yet supported\n");
+	default:
+		return dev_err_probe(&pdev->dev, -EINVAL,
+				     "Unknown STMPE PWM\n");
+	}
+
+	chip = devm_pwmchip_alloc(&pdev->dev, 3, sizeof(*stmpe_pwm));
+	if (IS_ERR(chip))
+		return PTR_ERR(chip);
+	stmpe_pwm = to_stmpe_pwm(chip);
 
 	stmpe_pwm->stmpe = stmpe;
-	stmpe_pwm->chip.dev = &pdev->dev;
 
-	if (stmpe->partnum == STMPE2401 || stmpe->partnum == STMPE2403) {
-		stmpe_pwm->chip.ops = &stmpe_24xx_pwm_ops;
-		stmpe_pwm->chip.npwm = 3;
-	} else {
-		if (stmpe->partnum == STMPE1601)
-			dev_err(&pdev->dev, "STMPE1601 not yet supported\n");
-		else
-			dev_err(&pdev->dev, "Unknown STMPE PWM\n");
-
-		return -ENODEV;
-	}
+	chip->ops = &stmpe_24xx_pwm_ops;
 
 	ret = stmpe_enable(stmpe, STMPE_BLOCK_PWM);
 	if (ret)
 		return ret;
 
-	ret = pwmchip_add(&stmpe_pwm->chip);
+	ret = pwmchip_add(chip);
 	if (ret) {
 		stmpe_disable(stmpe, STMPE_BLOCK_PWM);
 		return ret;
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 097/115] pwm: sun4i: Make use of devm_pwmchip_alloc() function

[Uwe Kleine-König]

This prepares the pwm-sun4i 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.

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

diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
index 44edf1ce5739..36a8dc65b19c 100644
--- a/drivers/pwm/pwm-sun4i.c
+++ b/drivers/pwm/pwm-sun4i.c
@@ -81,7 +81,6 @@ struct sun4i_pwm_data {
 };
 
 struct sun4i_pwm_chip {
-	struct pwm_chip chip;
 	struct clk *bus_clk;
 	struct clk *clk;
 	struct reset_control *rst;
@@ -92,7 +91,7 @@ struct sun4i_pwm_chip {
 
 static inline struct sun4i_pwm_chip *to_sun4i_pwm_chip(struct pwm_chip *chip)
 {
-	return container_of(chip, struct sun4i_pwm_chip, chip);
+	return pwmchip_get_drvdata(chip);
 }
 
 static inline u32 sun4i_pwm_readl(struct sun4i_pwm_chip *chip,
@@ -384,17 +383,22 @@ MODULE_DEVICE_TABLE(of, sun4i_pwm_dt_ids);
 
 static int sun4i_pwm_probe(struct platform_device *pdev)
 {
+	struct pwm_chip *chip;
 	struct sun4i_pwm_chip *sun4ichip;
+	const struct sun4i_pwm_data *data;
 	int ret;
 
-	sun4ichip = devm_kzalloc(&pdev->dev, sizeof(*sun4ichip), GFP_KERNEL);
-	if (!sun4ichip)
-		return -ENOMEM;
-
-	sun4ichip->data = of_device_get_match_data(&pdev->dev);
-	if (!sun4ichip->data)
+	data = of_device_get_match_data(&pdev->dev);
+	if (!data)
 		return -ENODEV;
 
+	chip = devm_pwmchip_alloc(&pdev->dev, data->npwm, sizeof(*sun4ichip));
+	if (IS_ERR(chip))
+		return PTR_ERR(chip);
+	sun4ichip = to_sun4i_pwm_chip(chip);
+
+	sun4ichip->data = data;
+
 	sun4ichip->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(sun4ichip->base))
 		return PTR_ERR(sun4ichip->base);
@@ -451,19 +455,18 @@ static int sun4i_pwm_probe(struct platform_device *pdev)
 		goto err_bus;
 	}
 
-	sun4ichip->chip.dev = &pdev->dev;
-	sun4ichip->chip.ops = &sun4i_pwm_ops;
-	sun4ichip->chip.npwm = sun4ichip->data->npwm;
+	chip->ops = &sun4i_pwm_ops;
+	chip->npwm = sun4ichip->data->npwm;
 
 	spin_lock_init(&sun4ichip->ctrl_lock);
 
-	ret = pwmchip_add(&sun4ichip->chip);
+	ret = pwmchip_add(chip);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "failed to add PWM chip: %d\n", ret);
 		goto err_pwm_add;
 	}
 
-	platform_set_drvdata(pdev, sun4ichip);
+	platform_set_drvdata(pdev, chip);
 
 	return 0;
 
@@ -477,9 +480,10 @@ static int sun4i_pwm_probe(struct platform_device *pdev)
 
 static void sun4i_pwm_remove(struct platform_device *pdev)
 {
-	struct sun4i_pwm_chip *sun4ichip = platform_get_drvdata(pdev);
+	struct pwm_chip *chip = platform_get_drvdata(pdev);
+	struct sun4i_pwm_chip *sun4ichip = to_sun4i_pwm_chip(chip);
 
-	pwmchip_remove(&sun4ichip->chip);
+	pwmchip_remove(chip);
 
 	clk_disable_unprepare(sun4ichip->bus_clk);
 	reset_control_assert(sun4ichip->rst);
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 097/115] pwm: sun4i: Make use of devm_pwmchip_alloc() function

[Andre Przywara]

On Wed,  6 Dec 2023 12:44:51 +0100
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:

Hi,

> This prepares the pwm-sun4i 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.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

That looks alright to me, it's really only wrapping the access to
struct sun4i_pwm_chip using the newly provided functions.

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre

> ---
>  drivers/pwm/pwm-sun4i.c | 34 +++++++++++++++++++---------------
>  1 file changed, 19 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
> index 44edf1ce5739..36a8dc65b19c 100644
> --- a/drivers/pwm/pwm-sun4i.c
> +++ b/drivers/pwm/pwm-sun4i.c
> @@ -81,7 +81,6 @@ struct sun4i_pwm_data {
>  };
>  
>  struct sun4i_pwm_chip {
> -	struct pwm_chip chip;
>  	struct clk *bus_clk;
>  	struct clk *clk;
>  	struct reset_control *rst;
> @@ -92,7 +91,7 @@ struct sun4i_pwm_chip {
>  
>  static inline struct sun4i_pwm_chip *to_sun4i_pwm_chip(struct pwm_chip *chip)
>  {
> -	return container_of(chip, struct sun4i_pwm_chip, chip);
> +	return pwmchip_get_drvdata(chip);
>  }
>  
>  static inline u32 sun4i_pwm_readl(struct sun4i_pwm_chip *chip,
> @@ -384,17 +383,22 @@ MODULE_DEVICE_TABLE(of, sun4i_pwm_dt_ids);
>  
>  static int sun4i_pwm_probe(struct platform_device *pdev)
>  {
> +	struct pwm_chip *chip;
>  	struct sun4i_pwm_chip *sun4ichip;
> +	const struct sun4i_pwm_data *data;
>  	int ret;
>  
> -	sun4ichip = devm_kzalloc(&pdev->dev, sizeof(*sun4ichip), GFP_KERNEL);
> -	if (!sun4ichip)
> -		return -ENOMEM;
> -
> -	sun4ichip->data = of_device_get_match_data(&pdev->dev);
> -	if (!sun4ichip->data)
> +	data = of_device_get_match_data(&pdev->dev);
> +	if (!data)
>  		return -ENODEV;
>  
> +	chip = devm_pwmchip_alloc(&pdev->dev, data->npwm, sizeof(*sun4ichip));
> +	if (IS_ERR(chip))
> +		return PTR_ERR(chip);
> +	sun4ichip = to_sun4i_pwm_chip(chip);
> +
> +	sun4ichip->data = data;
> +
>  	sun4ichip->base = devm_platform_ioremap_resource(pdev, 0);
>  	if (IS_ERR(sun4ichip->base))
>  		return PTR_ERR(sun4ichip->base);
> @@ -451,19 +455,18 @@ static int sun4i_pwm_probe(struct platform_device *pdev)
>  		goto err_bus;
>  	}
>  
> -	sun4ichip->chip.dev = &pdev->dev;
> -	sun4ichip->chip.ops = &sun4i_pwm_ops;
> -	sun4ichip->chip.npwm = sun4ichip->data->npwm;
> +	chip->ops = &sun4i_pwm_ops;
> +	chip->npwm = sun4ichip->data->npwm;
>  
>  	spin_lock_init(&sun4ichip->ctrl_lock);
>  
> -	ret = pwmchip_add(&sun4ichip->chip);
> +	ret = pwmchip_add(chip);
>  	if (ret < 0) {
>  		dev_err(&pdev->dev, "failed to add PWM chip: %d\n", ret);
>  		goto err_pwm_add;
>  	}
>  
> -	platform_set_drvdata(pdev, sun4ichip);
> +	platform_set_drvdata(pdev, chip);
>  
>  	return 0;
>  
> @@ -477,9 +480,10 @@ static int sun4i_pwm_probe(struct platform_device *pdev)
>  
>  static void sun4i_pwm_remove(struct platform_device *pdev)
>  {
> -	struct sun4i_pwm_chip *sun4ichip = platform_get_drvdata(pdev);
> +	struct pwm_chip *chip = platform_get_drvdata(pdev);
> +	struct sun4i_pwm_chip *sun4ichip = to_sun4i_pwm_chip(chip);
>  
> -	pwmchip_remove(&sun4ichip->chip);
> +	pwmchip_remove(chip);
>  
>  	clk_disable_unprepare(sun4ichip->bus_clk);
>  	reset_control_assert(sun4ichip->rst);


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 097/115] pwm: sun4i: Make use of devm_pwmchip_alloc() function

[Jernej Škrabec]

On Wednesday, December 6, 2023 12:44:51 PM CET Uwe Kleine-König wrote:
> This prepares the pwm-sun4i 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.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 103/115] pwm: visconti: Make use of devm_pwmchip_alloc() function

[Uwe Kleine-König]

This prepares the pwm-visconti 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.

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

diff --git a/drivers/pwm/pwm-visconti.c b/drivers/pwm/pwm-visconti.c
index 8d736d558122..9e55380957be 100644
--- a/drivers/pwm/pwm-visconti.c
+++ b/drivers/pwm/pwm-visconti.c
@@ -34,13 +34,12 @@
 #define PIPGM_PWMC_POLARITY_MASK	GENMASK(5, 5)
 
 struct visconti_pwm_chip {
-	struct pwm_chip chip;
 	void __iomem *base;
 };
 
 static inline struct visconti_pwm_chip *visconti_pwm_from_chip(struct pwm_chip *chip)
 {
-	return container_of(chip, struct visconti_pwm_chip, chip);
+	return pwmchip_get_drvdata(chip);
 }
 
 static int visconti_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
@@ -134,22 +133,22 @@ static const struct pwm_ops visconti_pwm_ops = {
 static int visconti_pwm_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
+	struct pwm_chip *chip;
 	struct visconti_pwm_chip *priv;
 	int ret;
 
-	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
-	if (!priv)
-		return -ENOMEM;
+	chip = devm_pwmchip_alloc(dev, 4, sizeof(*priv));
+	if (IS_ERR(chip))
+		return PTR_ERR(chip);
+	priv = visconti_pwm_from_chip(chip);
 
 	priv->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(priv->base))
 		return PTR_ERR(priv->base);
 
-	priv->chip.dev = dev;
-	priv->chip.ops = &visconti_pwm_ops;
-	priv->chip.npwm = 4;
+	chip->ops = &visconti_pwm_ops;
 
-	ret = devm_pwmchip_add(&pdev->dev, &priv->chip);
+	ret = devm_pwmchip_add(&pdev->dev, chip);
 	if (ret < 0)
 		return dev_err_probe(&pdev->dev, ret, "Cannot register visconti PWM\n");
 
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 104/115] pwm: vt8500: Make use of devm_pwmchip_alloc() function

[Uwe Kleine-König]

This prepares the pwm-vt8500 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.

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

diff --git a/drivers/pwm/pwm-vt8500.c b/drivers/pwm/pwm-vt8500.c
index ff5e76afbddf..b24afee37488 100644
--- a/drivers/pwm/pwm-vt8500.c
+++ b/drivers/pwm/pwm-vt8500.c
@@ -45,12 +45,14 @@
 #define STATUS_ALL_UPDATE	0x0F
 
 struct vt8500_chip {
-	struct pwm_chip chip;
 	void __iomem *base;
 	struct clk *clk;
 };
 
-#define to_vt8500_chip(chip)	container_of(chip, struct vt8500_chip, chip)
+static inline struct vt8500_chip *to_vt8500_chip(struct pwm_chip *chip)
+{
+	return pwmchip_get_drvdata(chip);
+}
 
 #define msecs_to_loops(t) (loops_per_jiffy / 1000 * HZ * t)
 static inline void vt8500_pwm_busy_wait(struct pwm_chip *chip, int nr, u8 bitmask)
@@ -232,6 +234,7 @@ MODULE_DEVICE_TABLE(of, vt8500_pwm_dt_ids);
 
 static int vt8500_pwm_probe(struct platform_device *pdev)
 {
+	struct pwm_chip *chip;
 	struct vt8500_chip *vt8500;
 	struct device_node *np = pdev->dev.of_node;
 	int ret;
@@ -239,13 +242,12 @@ static int vt8500_pwm_probe(struct platform_device *pdev)
 	if (!np)
 		return dev_err_probe(&pdev->dev, -EINVAL, "invalid devicetree node\n");
 
-	vt8500 = devm_kzalloc(&pdev->dev, sizeof(*vt8500), GFP_KERNEL);
-	if (vt8500 == NULL)
+	chip = devm_pwmchip_alloc(&pdev->dev, VT8500_NR_PWMS, sizeof(*vt8500));
+	if (chip == NULL)
 		return -ENOMEM;
+	vt8500 = to_vt8500_chip(chip);
 
-	vt8500->chip.dev = &pdev->dev;
-	vt8500->chip.ops = &vt8500_pwm_ops;
-	vt8500->chip.npwm = VT8500_NR_PWMS;
+	chip->ops = &vt8500_pwm_ops;
 
 	vt8500->clk = devm_clk_get_prepared(&pdev->dev, NULL);
 	if (IS_ERR(vt8500->clk))
@@ -255,7 +257,7 @@ static int vt8500_pwm_probe(struct platform_device *pdev)
 	if (IS_ERR(vt8500->base))
 		return PTR_ERR(vt8500->base);
 
-	ret = devm_pwmchip_add(&pdev->dev, &vt8500->chip);
+	ret = devm_pwmchip_add(&pdev->dev, chip);
 	if (ret < 0)
 		return dev_err_probe(&pdev->dev, ret, "failed to add PWM chip\n");
 
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 105/115] pwm: xilinx: Make use of devm_pwmchip_alloc() function

[Uwe Kleine-König]

This prepares the pwm-xilinx 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.

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

diff --git a/drivers/pwm/pwm-xilinx.c b/drivers/pwm/pwm-xilinx.c
index 5f3c2a6fed11..465ac9617256 100644
--- a/drivers/pwm/pwm-xilinx.c
+++ b/drivers/pwm/pwm-xilinx.c
@@ -80,15 +80,10 @@ unsigned int xilinx_timer_get_period(struct xilinx_timer_priv *priv,
 #define TCSR_PWM_CLEAR (TCSR_MDT | TCSR_LOAD)
 #define TCSR_PWM_MASK (TCSR_PWM_SET | TCSR_PWM_CLEAR)
 
-struct xilinx_pwm_device {
-	struct pwm_chip chip;
-	struct xilinx_timer_priv priv;
-};
-
 static inline struct xilinx_timer_priv
 *xilinx_pwm_chip_to_priv(struct pwm_chip *chip)
 {
-	return &container_of(chip, struct xilinx_pwm_device, chip)->priv;
+	return pwmchip_get_drvdata(chip);
 }
 
 static bool xilinx_timer_pwm_enabled(u32 tcsr0, u32 tcsr1)
@@ -213,8 +208,8 @@ static int xilinx_pwm_probe(struct platform_device *pdev)
 	int ret;
 	struct device *dev = &pdev->dev;
 	struct device_node *np = dev->of_node;
+	struct pwm_chip *chip;
 	struct xilinx_timer_priv *priv;
-	struct xilinx_pwm_device *xilinx_pwm;
 	u32 pwm_cells, one_timer, width;
 	void __iomem *regs;
 
@@ -225,11 +220,11 @@ static int xilinx_pwm_probe(struct platform_device *pdev)
 	if (ret)
 		return dev_err_probe(dev, ret, "could not read #pwm-cells\n");
 
-	xilinx_pwm = devm_kzalloc(dev, sizeof(*xilinx_pwm), GFP_KERNEL);
-	if (!xilinx_pwm)
-		return -ENOMEM;
-	platform_set_drvdata(pdev, xilinx_pwm);
-	priv = &xilinx_pwm->priv;
+	chip = devm_pwmchip_alloc(dev, 1, sizeof(*priv));
+	if (IS_ERR(chip))
+		return PTR_ERR(chip);
+	priv = xilinx_pwm_chip_to_priv(chip);
+	platform_set_drvdata(pdev, chip);
 
 	regs = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(regs))
@@ -278,10 +273,8 @@ static int xilinx_pwm_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, ret, "Clock enable failed\n");
 	clk_rate_exclusive_get(priv->clk);
 
-	xilinx_pwm->chip.dev = dev;
-	xilinx_pwm->chip.ops = &xilinx_pwm_ops;
-	xilinx_pwm->chip.npwm = 1;
-	ret = pwmchip_add(&xilinx_pwm->chip);
+	chip->ops = &xilinx_pwm_ops;
+	ret = pwmchip_add(chip);
 	if (ret) {
 		clk_rate_exclusive_put(priv->clk);
 		clk_disable_unprepare(priv->clk);
@@ -293,11 +286,12 @@ static int xilinx_pwm_probe(struct platform_device *pdev)
 
 static void xilinx_pwm_remove(struct platform_device *pdev)
 {
-	struct xilinx_pwm_device *xilinx_pwm = platform_get_drvdata(pdev);
+	struct pwm_chip *chip = platform_get_drvdata(pdev);
+	struct xilinx_timer_priv *priv = xilinx_pwm_chip_to_priv(chip);
 
-	pwmchip_remove(&xilinx_pwm->chip);
-	clk_rate_exclusive_put(xilinx_pwm->priv.clk);
-	clk_disable_unprepare(xilinx_pwm->priv.clk);
+	pwmchip_remove(chip);
+	clk_rate_exclusive_put(priv->clk);
+	clk_disable_unprepare(priv->clk);
 }
 
 static const struct of_device_id xilinx_pwm_of_match[] = {
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 000/115] pwm: Fix lifetime issues for pwm_chips

[Thierry Reding]

[-- Attachment #1.1.1: Type: text/plain, Size: 3140 bytes --]

On Wed, Dec 06, 2023 at 12:43:14PM +0100, Uwe Kleine-König wrote:
> Hello,
> 
> This series is based on Thierry's for-next.
> 
> It starts with some cleanups that were all sent out separately already:
> 
>  - "pwm: Reduce number of pointer dereferences in pwm_device_request()"
>    https://lore.kernel.org/linux-pwm/20231130072105.966848-1-u.kleine-koenig@pengutronix.de
>  - "pwm: crc: Use consistent variable naming for driver data"
>    https://lore.kernel.org/linux-pwm/20231130074133.969806-1-u.kleine-koenig@pengutronix.de
>  - Two leds/qcom-lpg patches
>    https://lore.kernel.org/linux-leds/20231126095230.683204-1-u.kleine-koenig@pengutronix.de
>    Lee already claimed to have taken the series already, but it's not yet in
>    next.
>  - "leds: qcom-lpg: Introduce a wrapper for getting driver data from a pwm chip"
>    https://lore.kernel.org/linux-leds/20231124215208.616551-3-u.kleine-koenig@pengutronix.de
> 
> In the following patches I changed:
> 
>  - "pwm: cros-ec: Change prototype of helper to prepare further changes" +
>    This was simplified in response to feedback by Tzung-Bi Shih
>  - Make pwmchip_priv() static (and don't export it), let drivers use a new
>    pwmchip_get_drvdata() instead.
>  - For drm/ti-sn65dsi86.c and leds/qcom-lpg make use of
>    pwmchip_set_drvdata() which makes the conversion to
>    devm_pwmchip_alloc() much prettier.
>  - Some cleanups here and there
>  - Add review tags received in v3
>    I kept all tags even though the pwmchip_alloc() patches changed
>    slightly. Most of the time that's only
>    s/pwmchip_priv/pwmchip_get_drvdata/ though. Still, if you object,
>    just tell me. (This affects Paul Cercueil on patch #68, Conor Dooley
>    on patch #76 and Greg for patch #109.)
> 
> I kept the pwmchip_alloc() + pwmchip_register() approach despite Bart
> not liking it. To balance that out I don't like Bart's alternative
> approach. There are no technically relevant differences between the two
> approaches and no benchmarks that show either of the two to be better
> than the other. Conceptually the design ideas around pwmchip_alloc() +
> pwmchip_register() are used in several other subsystems, so it's a
> proven way to do things.

[Trimming the recipients, keeping only Bart and the mailing lists.]

I do think there are technically relevant differences. For one, the
better we isolate the internal data structure, the easier this becomes
to manage. I'm attaching a patch that I've prototyped which should
basically get us to somewhere around patch 110 of this series but with
something like 1/8th of the changes. It doesn't need every driver to
change and (mostly) decouples driver code from the core code.

Now, I know that you think this is all bad because it's not a single
allocation, but I much prefer the end result because it's got the driver
and internals much more cleanly separated. Going forward I think it
would be easier to apply all the ref-counting on top of that because we
only need to keep the PWM framework-internal data structure alive after
a PWM chip has gone away.

Thierry

[-- Attachment #1.1.2: 0001-pwm-Isolate-internal-data-into-a-separate-structure.patch --]
[-- Type: text/plain, Size: 32685 bytes --]

From 72ea79887d96850f9ccc832ce52b907ca276c940 Mon Sep 17 00:00:00 2001
From: Thierry Reding <thierry.reding@gmail.com>
Date: Tue, 28 Nov 2023 15:42:39 +0100
Subject: [PATCH] pwm: Isolate internal data into a separate structure

In order to prepare for proper reference counting of PWM chips and PWM
devices, move the internal data from the public PWM chip to a private
PWM chip structure. This ensures that the data that the subsystem core
may need to reference later on can stick around beyond the lifetime of
the driver-private data.

Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
---
 drivers/pwm/core.c            | 185 +++++++++++++++++++++-------------
 drivers/pwm/internal.h        |  38 +++++++
 drivers/pwm/pwm-atmel-hlcdc.c |   8 +-
 drivers/pwm/pwm-fsl-ftm.c     |   6 +-
 drivers/pwm/pwm-lpc18xx-sct.c |   4 +-
 drivers/pwm/pwm-lpss.c        |  14 +--
 drivers/pwm/pwm-pca9685.c     |   6 +-
 drivers/pwm/pwm-samsung.c     |   6 +-
 drivers/pwm/pwm-sifive.c      |   4 +-
 drivers/pwm/pwm-stm32-lp.c    |   6 +-
 drivers/pwm/pwm-stm32.c       |   6 +-
 drivers/pwm/pwm-tiecap.c      |   6 +-
 drivers/pwm/pwm-tiehrpwm.c    |   6 +-
 drivers/pwm/sysfs.c           |  48 ++++-----
 include/linux/pwm.h           |  26 +----
 15 files changed, 228 insertions(+), 141 deletions(-)
 create mode 100644 drivers/pwm/internal.h

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index f60b715abe62..54d57dec6dce 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -24,17 +24,19 @@
 #define CREATE_TRACE_POINTS
 #include <trace/events/pwm.h>
 
+#include "internal.h"
+
 static DEFINE_MUTEX(pwm_lookup_lock);
 static LIST_HEAD(pwm_lookup_list);
 
-/* protects access to pwmchip_idr */
+/* protects access to pwm_chips */
 static DEFINE_MUTEX(pwm_lock);
 
-static DEFINE_IDR(pwmchip_idr);
+static DEFINE_IDR(pwm_chips);
 
 static struct pwm_chip *pwmchip_find_by_name(const char *name)
 {
-	struct pwm_chip *chip;
+	struct pwm_chip_private *priv;
 	unsigned long id, tmp;
 
 	if (!name)
@@ -42,12 +44,12 @@ static struct pwm_chip *pwmchip_find_by_name(const char *name)
 
 	mutex_lock(&pwm_lock);
 
-	idr_for_each_entry_ul(&pwmchip_idr, chip, tmp, id) {
-		const char *chip_name = dev_name(chip->dev);
+	idr_for_each_entry_ul(&pwm_chips, priv, tmp, id) {
+		const char *chip_name = dev_name(priv->chip->dev);
 
 		if (chip_name && strcmp(chip_name, name) == 0) {
 			mutex_unlock(&pwm_lock);
-			return chip;
+			return priv->chip;
 		}
 	}
 
@@ -58,23 +60,24 @@ static struct pwm_chip *pwmchip_find_by_name(const char *name)
 
 static int pwm_device_request(struct pwm_device *pwm, const char *label)
 {
+	struct pwm_chip *chip = pwm->priv->chip;
 	int err;
 
 	if (test_bit(PWMF_REQUESTED, &pwm->flags))
 		return -EBUSY;
 
-	if (!try_module_get(pwm->chip->owner))
+	if (!try_module_get(pwm->priv->owner))
 		return -ENODEV;
 
-	if (pwm->chip->ops->request) {
-		err = pwm->chip->ops->request(pwm->chip, pwm);
+	if (chip->ops->request) {
+		err = chip->ops->request(chip, pwm);
 		if (err) {
-			module_put(pwm->chip->owner);
+			module_put(pwm->priv->owner);
 			return err;
 		}
 	}
 
-	if (pwm->chip->ops->get_state) {
+	if (chip->ops->get_state) {
 		/*
 		 * Zero-initialize state because most drivers are unaware of
 		 * .usage_power. The other members of state are supposed to be
@@ -84,7 +87,7 @@ static int pwm_device_request(struct pwm_device *pwm, const char *label)
 		 */
 		struct pwm_state state = { 0, };
 
-		err = pwm->chip->ops->get_state(pwm->chip, pwm, &state);
+		err = chip->ops->get_state(chip, pwm, &state);
 		trace_pwm_get(pwm, &state, err);
 
 		if (!err)
@@ -196,6 +199,64 @@ static bool pwm_ops_check(const struct pwm_chip *chip)
 	return true;
 }
 
+static struct pwm_chip_private *pwmchip_alloc(struct pwm_chip *chip,
+					      struct module *owner)
+{
+	struct pwm_chip_private *priv;
+	struct pwm_device *pwm;
+	unsigned int i;
+	int err;
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return ERR_PTR(-ENOMEM);
+
+	priv->pwms = kcalloc(chip->npwm, sizeof(*pwm), GFP_KERNEL);
+	if (!priv->pwms) {
+		err = -ENOMEM;
+		goto free;
+	}
+
+	priv->owner = owner;
+	priv->chip = chip;
+
+	for (i = 0; i < chip->npwm; i++) {
+		struct pwm_device *pwm = &priv->pwms[i];
+
+		pwm->priv = priv;
+		pwm->hwpwm = i;
+	}
+
+	mutex_lock(&pwm_lock);
+
+	err = idr_alloc(&pwm_chips, priv, 0, 0, GFP_KERNEL);
+	if (err < 0) {
+		mutex_unlock(&pwm_lock);
+		goto free;
+	}
+
+	mutex_unlock(&pwm_lock);
+
+	priv->id = err;
+
+	return priv;
+
+free:
+	kfree(priv->pwms);
+	kfree(priv);
+	return ERR_PTR(err);
+}
+
+static void pwmchip_free(struct pwm_chip_private *priv)
+{
+	mutex_lock(&pwm_lock);
+	idr_remove(&pwm_chips, priv->id);
+	mutex_unlock(&pwm_lock);
+
+	kfree(priv->pwms);
+	kfree(priv);
+}
+
 /**
  * __pwmchip_add() - register a new PWM chip
  * @chip: the PWM chip to add
@@ -208,8 +269,7 @@ static bool pwm_ops_check(const struct pwm_chip *chip)
  */
 int __pwmchip_add(struct pwm_chip *chip, struct module *owner)
 {
-	unsigned int i;
-	int ret;
+	struct pwm_chip_private *priv;
 
 	if (!chip || !chip->dev || !chip->ops || !chip->npwm)
 		return -EINVAL;
@@ -217,36 +277,16 @@ int __pwmchip_add(struct pwm_chip *chip, struct module *owner)
 	if (!pwm_ops_check(chip))
 		return -EINVAL;
 
-	chip->owner = owner;
-
-	chip->pwms = kcalloc(chip->npwm, sizeof(*chip->pwms), GFP_KERNEL);
-	if (!chip->pwms)
+	priv = pwmchip_alloc(chip, owner);
+	if (!priv)
 		return -ENOMEM;
 
-	mutex_lock(&pwm_lock);
-
-	ret = idr_alloc(&pwmchip_idr, chip, 0, 0, GFP_KERNEL);
-	if (ret < 0) {
-		mutex_unlock(&pwm_lock);
-		kfree(chip->pwms);
-		return ret;
-	}
-
-	chip->id = ret;
-
-	for (i = 0; i < chip->npwm; i++) {
-		struct pwm_device *pwm = &chip->pwms[i];
-
-		pwm->chip = chip;
-		pwm->hwpwm = i;
-	}
-
-	mutex_unlock(&pwm_lock);
+	chip->priv = priv;
 
 	if (IS_ENABLED(CONFIG_OF))
 		of_pwmchip_add(chip);
 
-	pwmchip_sysfs_export(chip);
+	pwmchip_sysfs_export(priv);
 
 	return 0;
 }
@@ -260,18 +300,14 @@ EXPORT_SYMBOL_GPL(__pwmchip_add);
  */
 void pwmchip_remove(struct pwm_chip *chip)
 {
-	pwmchip_sysfs_unexport(chip);
-
-	if (IS_ENABLED(CONFIG_OF))
-		of_pwmchip_remove(chip);
-
 	mutex_lock(&pwm_lock);
 
-	idr_remove(&pwmchip_idr, chip->id);
+	pwmchip_sysfs_unexport(chip->priv);
 
-	mutex_unlock(&pwm_lock);
+	if (IS_ENABLED(CONFIG_OF))
+		of_pwmchip_remove(chip);
 
-	kfree(chip->pwms);
+	pwmchip_free(chip->priv);
 }
 EXPORT_SYMBOL_GPL(pwmchip_remove);
 
@@ -315,7 +351,7 @@ struct pwm_device *pwm_request_from_chip(struct pwm_chip *chip,
 		return ERR_PTR(-EINVAL);
 
 	mutex_lock(&pwm_lock);
-	pwm = &chip->pwms[index];
+	pwm = &chip->priv->pwms[index];
 
 	err = pwm_device_request(pwm, label);
 	if (err < 0)
@@ -329,9 +365,9 @@ EXPORT_SYMBOL_GPL(pwm_request_from_chip);
 static void pwm_apply_state_debug(struct pwm_device *pwm,
 				  const struct pwm_state *state)
 {
-	struct pwm_state *last = &pwm->last;
-	struct pwm_chip *chip = pwm->chip;
+	struct pwm_chip *chip = pwm->priv->chip;
 	struct pwm_state s1 = { 0 }, s2 = { 0 };
+	struct pwm_state *last = &pwm->last;
 	int err;
 
 	if (!IS_ENABLED(CONFIG_PWM_DEBUG))
@@ -439,7 +475,6 @@ static void pwm_apply_state_debug(struct pwm_device *pwm,
  */
 int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state)
 {
-	struct pwm_chip *chip;
 	int err;
 
 	/*
@@ -455,8 +490,6 @@ int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state)
 	    state->duty_cycle > state->period)
 		return -EINVAL;
 
-	chip = pwm->chip;
-
 	if (state->period == pwm->state.period &&
 	    state->duty_cycle == pwm->state.duty_cycle &&
 	    state->polarity == pwm->state.polarity &&
@@ -464,7 +497,7 @@ int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state)
 	    state->usage_power == pwm->state.usage_power)
 		return 0;
 
-	err = chip->ops->apply(chip, pwm, state);
+	err = pwm->priv->chip->ops->apply(pwm->priv->chip, pwm, state);
 	trace_pwm_apply(pwm, state, err);
 	if (err)
 		return err;
@@ -492,16 +525,19 @@ EXPORT_SYMBOL_GPL(pwm_apply_state);
 int pwm_capture(struct pwm_device *pwm, struct pwm_capture *result,
 		unsigned long timeout)
 {
+	struct pwm_chip *chip;
 	int err;
 
-	if (!pwm || !pwm->chip->ops)
+	if (!pwm)
 		return -EINVAL;
 
-	if (!pwm->chip->ops->capture)
+	chip = pwm->priv->chip;
+
+	if (!chip || !chip->ops || !chip->ops->capture)
 		return -ENOSYS;
 
 	mutex_lock(&pwm_lock);
-	err = pwm->chip->ops->capture(pwm->chip, pwm, result, timeout);
+	err = chip->ops->capture(chip, pwm, result, timeout);
 	mutex_unlock(&pwm_lock);
 
 	return err;
@@ -566,16 +602,19 @@ EXPORT_SYMBOL_GPL(pwm_adjust_config);
 
 static struct pwm_chip *fwnode_to_pwmchip(struct fwnode_handle *fwnode)
 {
-	struct pwm_chip *chip;
+	struct pwm_chip_private *priv;
 	unsigned long id, tmp;
 
 	mutex_lock(&pwm_lock);
 
-	idr_for_each_entry_ul(&pwmchip_idr, chip, tmp, id)
+	idr_for_each_entry_ul(&pwm_chips, priv, tmp, id) {
+		struct pwm_chip *chip = priv->chip;
+
 		if (chip->dev && device_match_fwnode(chip->dev, fwnode)) {
 			mutex_unlock(&pwm_lock);
 			return chip;
 		}
+	}
 
 	mutex_unlock(&pwm_lock);
 
@@ -585,6 +624,7 @@ static struct pwm_chip *fwnode_to_pwmchip(struct fwnode_handle *fwnode)
 static struct device_link *pwm_device_link_add(struct device *dev,
 					       struct pwm_device *pwm)
 {
+	struct pwm_chip *chip = pwm->priv->chip;
 	struct device_link *dl;
 
 	if (!dev) {
@@ -593,15 +633,15 @@ static struct device_link *pwm_device_link_add(struct device *dev,
 		 * impact the PM sequence ordering: the PWM supplier may get
 		 * suspended before the consumer.
 		 */
-		dev_warn(pwm->chip->dev,
+		dev_warn(chip->dev,
 			 "No consumer device specified to create a link to\n");
 		return NULL;
 	}
 
-	dl = device_link_add(dev, pwm->chip->dev, DL_FLAG_AUTOREMOVE_CONSUMER);
+	dl = device_link_add(dev, chip->dev, DL_FLAG_AUTOREMOVE_CONSUMER);
 	if (!dl) {
 		dev_err(dev, "failed to create device link to %s\n",
-			dev_name(pwm->chip->dev));
+			dev_name(chip->dev));
 		return ERR_PTR(-EINVAL);
 	}
 
@@ -918,12 +958,12 @@ void pwm_put(struct pwm_device *pwm)
 		goto out;
 	}
 
-	if (pwm->chip->ops->free)
-		pwm->chip->ops->free(pwm->chip, pwm);
+	if (pwm->priv->chip->ops->free)
+		pwm->priv->chip->ops->free(pwm->priv->chip, pwm);
 
 	pwm->label = NULL;
 
-	module_put(pwm->chip->owner);
+	module_put(pwm->priv->owner);
 out:
 	mutex_unlock(&pwm_lock);
 }
@@ -997,12 +1037,12 @@ struct pwm_device *devm_fwnode_pwm_get(struct device *dev,
 EXPORT_SYMBOL_GPL(devm_fwnode_pwm_get);
 
 #ifdef CONFIG_DEBUG_FS
-static void pwm_dbg_show(struct pwm_chip *chip, struct seq_file *s)
+static void pwm_dbg_show(struct pwm_chip_private *priv, struct seq_file *s)
 {
 	unsigned int i;
 
-	for (i = 0; i < chip->npwm; i++) {
-		struct pwm_device *pwm = &chip->pwms[i];
+	for (i = 0; i < priv->chip->npwm; i++) {
+		struct pwm_device *pwm = &priv->pwms[i];
 		struct pwm_state state;
 
 		pwm_get_state(pwm, &state);
@@ -1035,7 +1075,7 @@ static void *pwm_seq_start(struct seq_file *s, loff_t *pos)
 	mutex_lock(&pwm_lock);
 	s->private = "";
 
-	ret = idr_get_next_ul(&pwmchip_idr, &id);
+	ret = idr_get_next_ul(&pwm_chips, &id);
 	*pos = id;
 	return ret;
 }
@@ -1047,7 +1087,7 @@ static void *pwm_seq_next(struct seq_file *s, void *v, loff_t *pos)
 
 	s->private = "\n";
 
-	ret = idr_get_next_ul(&pwmchip_idr, &id);
+	ret = idr_get_next_ul(&pwm_chips, &id);
 	*pos = id;
 	return ret;
 }
@@ -1059,15 +1099,16 @@ static void pwm_seq_stop(struct seq_file *s, void *v)
 
 static int pwm_seq_show(struct seq_file *s, void *v)
 {
-	struct pwm_chip *chip = v;
+	struct pwm_chip_private *priv = v;
+	struct pwm_chip *chip = priv->chip;
 
 	seq_printf(s, "%s%d: %s/%s, %d PWM device%s\n",
-		   (char *)s->private, chip->id,
+		   (char *)s->private, priv->id,
 		   chip->dev->bus ? chip->dev->bus->name : "no-bus",
 		   dev_name(chip->dev), chip->npwm,
 		   (chip->npwm != 1) ? "s" : "");
 
-	pwm_dbg_show(chip, s);
+	pwm_dbg_show(priv, s);
 
 	return 0;
 }
diff --git a/drivers/pwm/internal.h b/drivers/pwm/internal.h
new file mode 100644
index 000000000000..44fffd3b6744
--- /dev/null
+++ b/drivers/pwm/internal.h
@@ -0,0 +1,38 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef PWM_INTERNAL_H
+#define PWM_INTERNAL_H
+
+#include <linux/list.h>
+
+struct pwm_chip;
+struct pwm_device;
+
+/**
+ * struct pwm_chip_private - subsystem-private PWM chip data
+ * @chip: driver-private PWM chip data
+ * @owner: module providing the chip
+ * @pwms: array of PWM devices allocated by the framework
+ * @base: number of first PWM controlled by this chip
+ */
+struct pwm_chip_private {
+	struct pwm_chip *chip;
+	struct module *owner;
+	struct pwm_device *pwms;
+	unsigned int id;
+};
+
+#ifdef CONFIG_PWM_SYSFS
+void pwmchip_sysfs_export(struct pwm_chip_private *priv);
+void pwmchip_sysfs_unexport(struct pwm_chip_private *priv);
+#else
+static inline void pwmchip_sysfs_export(struct pwm_chip_private *priv)
+{
+}
+
+static inline void pwmchip_sysfs_unexport(struct pwm_chip_private *priv)
+{
+}
+#endif /* CONFIG_PWM_SYSFS */
+
+#endif /* PWM_INTERNAL_H */
diff --git a/drivers/pwm/pwm-atmel-hlcdc.c b/drivers/pwm/pwm-atmel-hlcdc.c
index 3f2c5031a3ba..6bbbfaa582c1 100644
--- a/drivers/pwm/pwm-atmel-hlcdc.c
+++ b/drivers/pwm/pwm-atmel-hlcdc.c
@@ -15,6 +15,8 @@
 #include <linux/pwm.h>
 #include <linux/regmap.h>
 
+#include "internal.h"
+
 #define ATMEL_HLCDC_PWMCVAL_MASK	GENMASK(15, 8)
 #define ATMEL_HLCDC_PWMCVAL(x)		(((x) << 8) & ATMEL_HLCDC_PWMCVAL_MASK)
 #define ATMEL_HLCDC_PWMPOL		BIT(4)
@@ -185,7 +187,7 @@ static int atmel_hlcdc_pwm_suspend(struct device *dev)
 	struct atmel_hlcdc_pwm *atmel = dev_get_drvdata(dev);
 
 	/* Keep the periph clock enabled if the PWM is still running. */
-	if (pwm_is_enabled(&atmel->chip.pwms[0]))
+	if (pwm_is_enabled(&atmel->chip.priv->pwms[0]))
 		clk_disable_unprepare(atmel->hlcdc->periph_clk);
 
 	return 0;
@@ -197,7 +199,7 @@ static int atmel_hlcdc_pwm_resume(struct device *dev)
 	struct pwm_state state;
 	int ret;
 
-	pwm_get_state(&atmel->chip.pwms[0], &state);
+	pwm_get_state(&atmel->chip.priv->pwms[0], &state);
 
 	/* Re-enable the periph clock it was stopped during suspend. */
 	if (!state.enabled) {
@@ -206,7 +208,7 @@ static int atmel_hlcdc_pwm_resume(struct device *dev)
 			return ret;
 	}
 
-	return atmel_hlcdc_pwm_apply(&atmel->chip, &atmel->chip.pwms[0],
+	return atmel_hlcdc_pwm_apply(&atmel->chip, &atmel->chip.priv->pwms[0],
 				     &state);
 }
 
diff --git a/drivers/pwm/pwm-fsl-ftm.c b/drivers/pwm/pwm-fsl-ftm.c
index d1b6d1aa4773..72467d620a90 100644
--- a/drivers/pwm/pwm-fsl-ftm.c
+++ b/drivers/pwm/pwm-fsl-ftm.c
@@ -19,6 +19,8 @@
 #include <linux/slab.h>
 #include <linux/fsl/ftm.h>
 
+#include "internal.h"
+
 #define FTM_SC_CLK(c)	(((c) + 1) << FTM_SC_CLK_MASK_SHIFT)
 
 enum fsl_pwm_clk {
@@ -468,7 +470,7 @@ static int fsl_pwm_suspend(struct device *dev)
 	regcache_mark_dirty(fpc->regmap);
 
 	for (i = 0; i < fpc->chip.npwm; i++) {
-		struct pwm_device *pwm = &fpc->chip.pwms[i];
+		struct pwm_device *pwm = &fpc->chip.priv->pwms[i];
 
 		if (!test_bit(PWMF_REQUESTED, &pwm->flags))
 			continue;
@@ -491,7 +493,7 @@ static int fsl_pwm_resume(struct device *dev)
 	int i;
 
 	for (i = 0; i < fpc->chip.npwm; i++) {
-		struct pwm_device *pwm = &fpc->chip.pwms[i];
+		struct pwm_device *pwm = &fpc->chip.priv->pwms[i];
 
 		if (!test_bit(PWMF_REQUESTED, &pwm->flags))
 			continue;
diff --git a/drivers/pwm/pwm-lpc18xx-sct.c b/drivers/pwm/pwm-lpc18xx-sct.c
index b3d4a955aa31..80f89ad2bc8f 100644
--- a/drivers/pwm/pwm-lpc18xx-sct.c
+++ b/drivers/pwm/pwm-lpc18xx-sct.c
@@ -27,6 +27,8 @@
 #include <linux/platform_device.h>
 #include <linux/pwm.h>
 
+#include "internal.h"
+
 /* LPC18xx SCT registers */
 #define LPC18XX_PWM_CONFIG		0x000
 #define LPC18XX_PWM_CONFIG_UNIFY	BIT(0)
@@ -224,7 +226,7 @@ static int lpc18xx_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	    !lpc18xx_pwm->period_ns) {
 		lpc18xx_pwm->period_ns = period_ns;
 		for (i = 0; i < chip->npwm; i++)
-			pwm_set_period(&chip->pwms[i], period_ns);
+			pwm_set_period(&chip->priv->pwms[i], period_ns);
 		lpc18xx_pwm_config_period(chip, period_ns);
 	}
 
diff --git a/drivers/pwm/pwm-lpss.c b/drivers/pwm/pwm-lpss.c
index a6ea3ce7e019..085d3c17f96b 100644
--- a/drivers/pwm/pwm-lpss.c
+++ b/drivers/pwm/pwm-lpss.c
@@ -19,6 +19,8 @@
 #include <linux/pm_runtime.h>
 #include <linux/time.h>
 
+#include "internal.h"
+
 #define DEFAULT_SYMBOL_NAMESPACE PWM_LPSS
 
 #include "pwm-lpss.h"
@@ -73,21 +75,21 @@ static inline struct pwm_lpss_chip *to_lpwm(struct pwm_chip *chip)
 
 static inline u32 pwm_lpss_read(const struct pwm_device *pwm)
 {
-	struct pwm_lpss_chip *lpwm = to_lpwm(pwm->chip);
+	struct pwm_lpss_chip *lpwm = to_lpwm(pwm->priv->chip);
 
 	return readl(lpwm->regs + pwm->hwpwm * PWM_SIZE + PWM);
 }
 
 static inline void pwm_lpss_write(const struct pwm_device *pwm, u32 value)
 {
-	struct pwm_lpss_chip *lpwm = to_lpwm(pwm->chip);
+	struct pwm_lpss_chip *lpwm = to_lpwm(pwm->priv->chip);
 
 	writel(value, lpwm->regs + pwm->hwpwm * PWM_SIZE + PWM);
 }
 
 static int pwm_lpss_wait_for_update(struct pwm_device *pwm)
 {
-	struct pwm_lpss_chip *lpwm = to_lpwm(pwm->chip);
+	struct pwm_lpss_chip *lpwm = to_lpwm(pwm->priv->chip);
 	const void __iomem *addr = lpwm->regs + pwm->hwpwm * PWM_SIZE + PWM;
 	const unsigned int ms = 500 * USEC_PER_MSEC;
 	u32 val;
@@ -106,7 +108,7 @@ static int pwm_lpss_wait_for_update(struct pwm_device *pwm)
 	 */
 	err = readl_poll_timeout(addr, val, !(val & PWM_SW_UPDATE), 40, ms);
 	if (err)
-		dev_err(pwm->chip->dev, "PWM_SW_UPDATE was not cleared\n");
+		dev_err(pwm->priv->chip->dev, "PWM_SW_UPDATE was not cleared\n");
 
 	return err;
 }
@@ -114,7 +116,7 @@ static int pwm_lpss_wait_for_update(struct pwm_device *pwm)
 static inline int pwm_lpss_is_updating(struct pwm_device *pwm)
 {
 	if (pwm_lpss_read(pwm) & PWM_SW_UPDATE) {
-		dev_err(pwm->chip->dev, "PWM_SW_UPDATE is still set, skipping update\n");
+		dev_err(pwm->priv->chip->dev, "PWM_SW_UPDATE is still set, skipping update\n");
 		return -EBUSY;
 	}
 
@@ -278,7 +280,7 @@ struct pwm_lpss_chip *devm_pwm_lpss_probe(struct device *dev, void __iomem *base
 	}
 
 	for (i = 0; i < lpwm->info->npwm; i++) {
-		ctrl = pwm_lpss_read(&lpwm->chip.pwms[i]);
+		ctrl = pwm_lpss_read(&lpwm->chip.priv->pwms[i]);
 		if (ctrl & PWM_ENABLE)
 			pm_runtime_get(dev);
 	}
diff --git a/drivers/pwm/pwm-pca9685.c b/drivers/pwm/pwm-pca9685.c
index e79b1de8c4d8..d82cca90dba9 100644
--- a/drivers/pwm/pwm-pca9685.c
+++ b/drivers/pwm/pwm-pca9685.c
@@ -22,6 +22,8 @@
 #include <linux/pm_runtime.h>
 #include <linux/bitmap.h>
 
+#include "internal.h"
+
 /*
  * Because the PCA9685 has only one prescaler per chip, only the first channel
  * that is enabled is allowed to change the prescale register.
@@ -134,7 +136,7 @@ static int pca9685_write_reg(struct pca9685 *pca, unsigned int reg, unsigned int
 /* Helper function to set the duty cycle ratio to duty/4096 (e.g. duty=2048 -> 50%) */
 static void pca9685_pwm_set_duty(struct pca9685 *pca, int channel, unsigned int duty)
 {
-	struct pwm_device *pwm = &pca->chip.pwms[channel];
+	struct pwm_device *pwm = &pca->chip.priv->pwms[channel];
 	unsigned int on, off;
 
 	if (duty == 0) {
@@ -173,7 +175,7 @@ static void pca9685_pwm_set_duty(struct pca9685 *pca, int channel, unsigned int
 
 static unsigned int pca9685_pwm_get_duty(struct pca9685 *pca, int channel)
 {
-	struct pwm_device *pwm = &pca->chip.pwms[channel];
+	struct pwm_device *pwm = &pca->chip.priv->pwms[channel];
 	unsigned int off = 0, on = 0, val = 0;
 
 	if (WARN_ON(channel >= PCA9685_MAXCHAN)) {
diff --git a/drivers/pwm/pwm-samsung.c b/drivers/pwm/pwm-samsung.c
index 6e77302f7368..e1baede405bc 100644
--- a/drivers/pwm/pwm-samsung.c
+++ b/drivers/pwm/pwm-samsung.c
@@ -26,6 +26,8 @@
 /* For struct samsung_timer_variant and samsung_pwm_lock. */
 #include <clocksource/samsung_pwm.h>
 
+#include "internal.h"
+
 #define REG_TCFG0			0x00
 #define REG_TCFG1			0x04
 #define REG_TCON			0x08
@@ -627,10 +629,10 @@ static int pwm_samsung_resume(struct device *dev)
 	unsigned int i;
 
 	for (i = 0; i < SAMSUNG_PWM_NUM; i++) {
-		struct pwm_device *pwm = &chip->pwms[i];
+		struct pwm_device *pwm = &chip->priv->pwms[i];
 		struct samsung_pwm_channel *chan = &our_chip->channel[i];
 
-		if (!test_bit(PWMF_REQUESTED, &pwm->flags))
+		if (WARN_ON(!pwm) || !test_bit(PWMF_REQUESTED, &pwm->flags))
 			continue;
 
 		if (our_chip->variant.output_mask & BIT(i))
diff --git a/drivers/pwm/pwm-sifive.c b/drivers/pwm/pwm-sifive.c
index 089e50bdbbf0..e1dccaf33398 100644
--- a/drivers/pwm/pwm-sifive.c
+++ b/drivers/pwm/pwm-sifive.c
@@ -20,6 +20,8 @@
 #include <linux/slab.h>
 #include <linux/bitfield.h>
 
+#include "internal.h"
+
 /* Register offsets */
 #define PWM_SIFIVE_PWMCFG		0x0
 #define PWM_SIFIVE_PWMCOUNT		0x8
@@ -322,7 +324,7 @@ static void pwm_sifive_remove(struct platform_device *dev)
 	clk_notifier_unregister(ddata->clk, &ddata->notifier);
 
 	for (ch = 0; ch < ddata->chip.npwm; ch++) {
-		pwm = &ddata->chip.pwms[ch];
+		pwm = &ddata->chip.priv->pwms[ch];
 		if (pwm->state.enabled)
 			clk_disable(ddata->clk);
 	}
diff --git a/drivers/pwm/pwm-stm32-lp.c b/drivers/pwm/pwm-stm32-lp.c
index 439068f3eca1..cb32caf5368a 100644
--- a/drivers/pwm/pwm-stm32-lp.c
+++ b/drivers/pwm/pwm-stm32-lp.c
@@ -17,6 +17,8 @@
 #include <linux/platform_device.h>
 #include <linux/pwm.h>
 
+#include "internal.h"
+
 struct stm32_pwm_lp {
 	struct pwm_chip chip;
 	struct clk *clk;
@@ -223,10 +225,10 @@ static int stm32_pwm_lp_suspend(struct device *dev)
 	struct stm32_pwm_lp *priv = dev_get_drvdata(dev);
 	struct pwm_state state;
 
-	pwm_get_state(&priv->chip.pwms[0], &state);
+	pwm_get_state(&priv->chip.priv->pwms[0], &state);
 	if (state.enabled) {
 		dev_err(dev, "The consumer didn't stop us (%s)\n",
-			priv->chip.pwms[0].label);
+			priv->chip.priv->pwms[0].label);
 		return -EBUSY;
 	}
 
diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c
index 5f10cba492ec..81933df80768 100644
--- a/drivers/pwm/pwm-stm32.c
+++ b/drivers/pwm/pwm-stm32.c
@@ -16,6 +16,8 @@
 #include <linux/platform_device.h>
 #include <linux/pwm.h>
 
+#include "internal.h"
+
 #define CCMR_CHANNEL_SHIFT 8
 #define CCMR_CHANNEL_MASK  0xFF
 #define MAX_BREAKINPUT 2
@@ -127,7 +129,7 @@ static int stm32_pwm_raw_capture(struct stm32_pwm *priv, struct pwm_device *pwm,
 		*raw_prd = priv->max_arr - priv->capture[0] + priv->capture[2];
 
 	/* Duty cycle capture requires at least two capture units */
-	if (pwm->chip->npwm < 2)
+	if (priv->chip.npwm < 2)
 		*raw_dty = 0;
 	else if (priv->capture[0] <= priv->capture[3])
 		*raw_dty = priv->capture[3] - priv->capture[0];
@@ -682,7 +684,7 @@ static int stm32_pwm_suspend(struct device *dev)
 		mask = TIM_CCER_CC1E << (i * 4);
 		if (ccer & mask) {
 			dev_err(dev, "PWM %u still in use by consumer %s\n",
-				i, priv->chip.pwms[i].label);
+				i, priv->chip.priv->pwms[i].label);
 			return -EBUSY;
 		}
 	}
diff --git a/drivers/pwm/pwm-tiecap.c b/drivers/pwm/pwm-tiecap.c
index d974f4414ac9..f089257051d1 100644
--- a/drivers/pwm/pwm-tiecap.c
+++ b/drivers/pwm/pwm-tiecap.c
@@ -14,6 +14,8 @@
 #include <linux/pwm.h>
 #include <linux/of.h>
 
+#include "internal.h"
+
 /* ECAP registers and bits definitions */
 #define CAP1			0x08
 #define CAP2			0x0C
@@ -288,7 +290,7 @@ static void ecap_pwm_restore_context(struct ecap_pwm_chip *pc)
 static int ecap_pwm_suspend(struct device *dev)
 {
 	struct ecap_pwm_chip *pc = dev_get_drvdata(dev);
-	struct pwm_device *pwm = pc->chip.pwms;
+	struct pwm_device *pwm = pc->chip.priv->pwms;
 
 	ecap_pwm_save_context(pc);
 
@@ -302,7 +304,7 @@ static int ecap_pwm_suspend(struct device *dev)
 static int ecap_pwm_resume(struct device *dev)
 {
 	struct ecap_pwm_chip *pc = dev_get_drvdata(dev);
-	struct pwm_device *pwm = pc->chip.pwms;
+	struct pwm_device *pwm = pc->chip.priv->pwms;
 
 	/* Enable explicitly if PWM was running */
 	if (pwm_is_enabled(pwm))
diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
index af231fa74fa9..0d137100b040 100644
--- a/drivers/pwm/pwm-tiehrpwm.c
+++ b/drivers/pwm/pwm-tiehrpwm.c
@@ -14,6 +14,8 @@
 #include <linux/pm_runtime.h>
 #include <linux/of.h>
 
+#include "internal.h"
+
 /* EHRPWM registers and bits definitions */
 
 /* Time base module registers */
@@ -557,7 +559,7 @@ static int ehrpwm_pwm_suspend(struct device *dev)
 	ehrpwm_pwm_save_context(pc);
 
 	for (i = 0; i < pc->chip.npwm; i++) {
-		struct pwm_device *pwm = &pc->chip.pwms[i];
+		struct pwm_device *pwm = &pc->chip.priv->pwms[i];
 
 		if (!pwm_is_enabled(pwm))
 			continue;
@@ -575,7 +577,7 @@ static int ehrpwm_pwm_resume(struct device *dev)
 	unsigned int i;
 
 	for (i = 0; i < pc->chip.npwm; i++) {
-		struct pwm_device *pwm = &pc->chip.pwms[i];
+		struct pwm_device *pwm = &pc->chip.priv->pwms[i];
 
 		if (!pwm_is_enabled(pwm))
 			continue;
diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c
index 4edb994fa2e1..653df20afe1c 100644
--- a/drivers/pwm/sysfs.c
+++ b/drivers/pwm/sysfs.c
@@ -14,6 +14,8 @@
 #include <linux/kdev_t.h>
 #include <linux/pwm.h>
 
+#include "internal.h"
+
 struct pwm_export {
 	struct device child;
 	struct pwm_device *pwm;
@@ -311,7 +313,7 @@ static ssize_t export_store(struct device *parent,
 			    struct device_attribute *attr,
 			    const char *buf, size_t len)
 {
-	struct pwm_chip *chip = dev_get_drvdata(parent);
+	struct pwm_chip_private *priv = dev_get_drvdata(parent);
 	struct pwm_device *pwm;
 	unsigned int hwpwm;
 	int ret;
@@ -320,10 +322,10 @@ static ssize_t export_store(struct device *parent,
 	if (ret < 0)
 		return ret;
 
-	if (hwpwm >= chip->npwm)
+	if (hwpwm >= priv->chip->npwm)
 		return -ENODEV;
 
-	pwm = pwm_request_from_chip(chip, hwpwm, "sysfs");
+	pwm = pwm_request_from_chip(priv->chip, hwpwm, "sysfs");
 	if (IS_ERR(pwm))
 		return PTR_ERR(pwm);
 
@@ -339,7 +341,7 @@ static ssize_t unexport_store(struct device *parent,
 			      struct device_attribute *attr,
 			      const char *buf, size_t len)
 {
-	struct pwm_chip *chip = dev_get_drvdata(parent);
+	struct pwm_chip_private *priv = dev_get_drvdata(parent);
 	unsigned int hwpwm;
 	int ret;
 
@@ -347,10 +349,10 @@ static ssize_t unexport_store(struct device *parent,
 	if (ret < 0)
 		return ret;
 
-	if (hwpwm >= chip->npwm)
+	if (hwpwm >= priv->chip->npwm)
 		return -ENODEV;
 
-	ret = pwm_unexport_child(parent, &chip->pwms[hwpwm]);
+	ret = pwm_unexport_child(parent, &priv->pwms[hwpwm]);
 
 	return ret ? : len;
 }
@@ -359,9 +361,9 @@ static DEVICE_ATTR_WO(unexport);
 static ssize_t npwm_show(struct device *parent, struct device_attribute *attr,
 			 char *buf)
 {
-	const struct pwm_chip *chip = dev_get_drvdata(parent);
+	const struct pwm_chip_private *priv = dev_get_drvdata(parent);
 
-	return sysfs_emit(buf, "%u\n", chip->npwm);
+	return sysfs_emit(buf, "%u\n", priv->chip->npwm);
 }
 static DEVICE_ATTR_RO(npwm);
 
@@ -411,12 +413,12 @@ static int pwm_class_apply_state(struct pwm_export *export,
 
 static int pwm_class_resume_npwm(struct device *parent, unsigned int npwm)
 {
-	struct pwm_chip *chip = dev_get_drvdata(parent);
+	struct pwm_chip_private *priv = dev_get_drvdata(parent);
 	unsigned int i;
 	int ret = 0;
 
 	for (i = 0; i < npwm; i++) {
-		struct pwm_device *pwm = &chip->pwms[i];
+		struct pwm_device *pwm = &priv->pwms[i];
 		struct pwm_state state;
 		struct pwm_export *export;
 
@@ -442,12 +444,12 @@ static int pwm_class_resume_npwm(struct device *parent, unsigned int npwm)
 
 static int pwm_class_suspend(struct device *parent)
 {
-	struct pwm_chip *chip = dev_get_drvdata(parent);
+	struct pwm_chip_private *priv = dev_get_drvdata(parent);
 	unsigned int i;
 	int ret = 0;
 
-	for (i = 0; i < chip->npwm; i++) {
-		struct pwm_device *pwm = &chip->pwms[i];
+	for (i = 0; i < priv->chip->npwm; i++) {
+		struct pwm_device *pwm = &priv->pwms[i];
 		struct pwm_state state;
 		struct pwm_export *export;
 
@@ -483,9 +485,9 @@ static int pwm_class_suspend(struct device *parent)
 
 static int pwm_class_resume(struct device *parent)
 {
-	struct pwm_chip *chip = dev_get_drvdata(parent);
+	struct pwm_chip_private *priv = dev_get_drvdata(parent);
 
-	return pwm_class_resume_npwm(parent, chip->npwm);
+	return pwm_class_resume_npwm(parent, priv->chip->npwm);
 }
 
 static DEFINE_SIMPLE_DEV_PM_OPS(pwm_class_pm_ops, pwm_class_suspend, pwm_class_resume);
@@ -501,7 +503,7 @@ static int pwmchip_sysfs_match(struct device *parent, const void *data)
 	return dev_get_drvdata(parent) == data;
 }
 
-void pwmchip_sysfs_export(struct pwm_chip *chip)
+void pwmchip_sysfs_export(struct pwm_chip_private *priv)
 {
 	struct device *parent;
 
@@ -509,26 +511,26 @@ void pwmchip_sysfs_export(struct pwm_chip *chip)
 	 * If device_create() fails the pwm_chip is still usable by
 	 * the kernel it's just not exported.
 	 */
-	parent = device_create(&pwm_class, chip->dev, MKDEV(0, 0), chip,
-			       "pwmchip%d", chip->id);
+	parent = device_create(&pwm_class, priv->chip->dev, MKDEV(0, 0), priv,
+			       "pwmchip%d", priv->id);
 	if (IS_ERR(parent)) {
-		dev_warn(chip->dev,
+		dev_warn(priv->chip->dev,
 			 "device_create failed for pwm_chip sysfs export\n");
 	}
 }
 
-void pwmchip_sysfs_unexport(struct pwm_chip *chip)
+void pwmchip_sysfs_unexport(struct pwm_chip_private *priv)
 {
 	struct device *parent;
 	unsigned int i;
 
-	parent = class_find_device(&pwm_class, NULL, chip,
+	parent = class_find_device(&pwm_class, NULL, priv,
 				   pwmchip_sysfs_match);
 	if (!parent)
 		return;
 
-	for (i = 0; i < chip->npwm; i++) {
-		struct pwm_device *pwm = &chip->pwms[i];
+	for (i = 0; i < priv->chip->npwm; i++) {
+		struct pwm_device *pwm = &priv->pwms[i];
 
 		if (test_bit(PWMF_EXPORTED, &pwm->flags))
 			pwm_unexport_child(parent, pwm);
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index f87655c06c82..5c478cd592d7 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -6,6 +6,7 @@
 #include <linux/mutex.h>
 #include <linux/of.h>
 
+struct pwm_chip_private;
 struct pwm_chip;
 
 /**
@@ -79,11 +80,12 @@ struct pwm_device {
 	const char *label;
 	unsigned long flags;
 	unsigned int hwpwm;
-	struct pwm_chip *chip;
 
 	struct pwm_args args;
 	struct pwm_state state;
 	struct pwm_state last;
+
+	struct pwm_chip_private *priv;
 };
 
 /**
@@ -280,26 +282,21 @@ struct pwm_ops {
  * struct pwm_chip - abstract a PWM controller
  * @dev: device providing the PWMs
  * @ops: callbacks for this PWM controller
- * @owner: module providing this chip
- * @id: unique number of this PWM chip
  * @npwm: number of PWMs controlled by this chip
  * @of_xlate: request a PWM device given a device tree PWM specifier
  * @of_pwm_n_cells: number of cells expected in the device tree PWM specifier
- * @pwms: array of PWM devices allocated by the framework
+ * @priv: subsystem internal chip data
  */
 struct pwm_chip {
 	struct device *dev;
 	const struct pwm_ops *ops;
-	struct module *owner;
-	unsigned int id;
 	unsigned int npwm;
 
 	struct pwm_device * (*of_xlate)(struct pwm_chip *chip,
 					const struct of_phandle_args *args);
 	unsigned int of_pwm_n_cells;
 
-	/* only used internally by the PWM framework */
-	struct pwm_device *pwms;
+	struct pwm_chip_private *priv;
 };
 
 #if IS_ENABLED(CONFIG_PWM)
@@ -564,17 +561,4 @@ static inline void pwm_remove_table(struct pwm_lookup *table, size_t num)
 }
 #endif
 
-#ifdef CONFIG_PWM_SYSFS
-void pwmchip_sysfs_export(struct pwm_chip *chip);
-void pwmchip_sysfs_unexport(struct pwm_chip *chip);
-#else
-static inline void pwmchip_sysfs_export(struct pwm_chip *chip)
-{
-}
-
-static inline void pwmchip_sysfs_unexport(struct pwm_chip *chip)
-{
-}
-#endif /* CONFIG_PWM_SYSFS */
-
 #endif /* __LINUX_PWM_H */
-- 
2.43.0


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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 000/115] pwm: Fix lifetime issues for pwm_chips

[Uwe Kleine-König]

[-- Attachment #1.1: Type: text/plain, Size: 26605 bytes --]

Hello Thierry,

On Fri, Dec 08, 2023 at 04:41:39PM +0100, Thierry Reding wrote:
> On Wed, Dec 06, 2023 at 12:43:14PM +0100, Uwe Kleine-König wrote:
> > This series is based on Thierry's for-next.
> > 
> > It starts with some cleanups that were all sent out separately already:
> > 
> >  - "pwm: Reduce number of pointer dereferences in pwm_device_request()"
> >    https://lore.kernel.org/linux-pwm/20231130072105.966848-1-u.kleine-koenig@pengutronix.de
> >  - "pwm: crc: Use consistent variable naming for driver data"
> >    https://lore.kernel.org/linux-pwm/20231130074133.969806-1-u.kleine-koenig@pengutronix.de
> >  - Two leds/qcom-lpg patches
> >    https://lore.kernel.org/linux-leds/20231126095230.683204-1-u.kleine-koenig@pengutronix.de
> >    Lee already claimed to have taken the series already, but it's not yet in
> >    next.
> >  - "leds: qcom-lpg: Introduce a wrapper for getting driver data from a pwm chip"
> >    https://lore.kernel.org/linux-leds/20231124215208.616551-3-u.kleine-koenig@pengutronix.de
> > 
> > In the following patches I changed:
> > 
> >  - "pwm: cros-ec: Change prototype of helper to prepare further changes" +
> >    This was simplified in response to feedback by Tzung-Bi Shih
> >  - Make pwmchip_priv() static (and don't export it), let drivers use a new
> >    pwmchip_get_drvdata() instead.
> >  - For drm/ti-sn65dsi86.c and leds/qcom-lpg make use of
> >    pwmchip_set_drvdata() which makes the conversion to
> >    devm_pwmchip_alloc() much prettier.
> >  - Some cleanups here and there
> >  - Add review tags received in v3
> >    I kept all tags even though the pwmchip_alloc() patches changed
> >    slightly. Most of the time that's only
> >    s/pwmchip_priv/pwmchip_get_drvdata/ though. Still, if you object,
> >    just tell me. (This affects Paul Cercueil on patch #68, Conor Dooley
> >    on patch #76 and Greg for patch #109.)
> > 
> > I kept the pwmchip_alloc() + pwmchip_register() approach despite Bart
> > not liking it. To balance that out I don't like Bart's alternative
> > approach. There are no technically relevant differences between the two
> > approaches and no benchmarks that show either of the two to be better
> > than the other. Conceptually the design ideas around pwmchip_alloc() +
> > pwmchip_register() are used in several other subsystems, so it's a
> > proven way to do things.
> 
> [Trimming the recipients, keeping only Bart and the mailing lists.]
> 
> I do think there are technically relevant differences. For one, the
> better we isolate the internal data structure, the easier this becomes
> to manage. I'm attaching a patch that I've prototyped which should
> basically get us to somewhere around patch 110 of this series but with
> something like 1/8th of the changes. It doesn't need every driver to
> change and (mostly) decouples driver code from the core code.

You don't need to touch all drivers because you didn't change struct
pwm_chip::dev yet. (If you really want, you don't need to change that,
but then you have some duplication as chip->dev holds the same value as
priv->dev.parent in the end.)

> Now, I know that you think this is all bad because it's not a single
> allocation, but I much prefer the end result because it's got the driver
> and internals much more cleanly separated. Going forward I think it
> would be easier to apply all the ref-counting on top of that because we
> only need to keep the PWM framework-internal data structure alive after
> a PWM chip has gone away.

Nearly all drivers need driver private data. Isn't it a nice service by
the pwm core to make handling this private data easier for the lowlevel
drivers?

> From 72ea79887d96850f9ccc832ce52b907ca276c940 Mon Sep 17 00:00:00 2001
> From: Thierry Reding <thierry.reding@gmail.com>
> Date: Tue, 28 Nov 2023 15:42:39 +0100
> Subject: [PATCH] pwm: Isolate internal data into a separate structure
> 
> In order to prepare for proper reference counting of PWM chips and PWM
> devices, move the internal data from the public PWM chip to a private
> PWM chip structure. This ensures that the data that the subsystem core
> may need to reference later on can stick around beyond the lifetime of
> the driver-private data.
> 
> Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
> ---
>  drivers/pwm/core.c            | 185 +++++++++++++++++++++-------------
>  drivers/pwm/internal.h        |  38 +++++++
>  drivers/pwm/pwm-atmel-hlcdc.c |   8 +-
>  drivers/pwm/pwm-fsl-ftm.c     |   6 +-
>  drivers/pwm/pwm-lpc18xx-sct.c |   4 +-
>  drivers/pwm/pwm-lpss.c        |  14 +--
>  drivers/pwm/pwm-pca9685.c     |   6 +-
>  drivers/pwm/pwm-samsung.c     |   6 +-
>  drivers/pwm/pwm-sifive.c      |   4 +-
>  drivers/pwm/pwm-stm32-lp.c    |   6 +-
>  drivers/pwm/pwm-stm32.c       |   6 +-
>  drivers/pwm/pwm-tiecap.c      |   6 +-
>  drivers/pwm/pwm-tiehrpwm.c    |   6 +-
>  drivers/pwm/sysfs.c           |  48 ++++-----
>  include/linux/pwm.h           |  26 +----
>  15 files changed, 228 insertions(+), 141 deletions(-)
>  create mode 100644 drivers/pwm/internal.h
> 
> diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
> index f60b715abe62..54d57dec6dce 100644
> --- a/drivers/pwm/core.c
> +++ b/drivers/pwm/core.c
> @@ -24,17 +24,19 @@
>  #define CREATE_TRACE_POINTS
>  #include <trace/events/pwm.h>
>  
> +#include "internal.h"
> +
>  static DEFINE_MUTEX(pwm_lookup_lock);
>  static LIST_HEAD(pwm_lookup_list);
>  
> -/* protects access to pwmchip_idr */
> +/* protects access to pwm_chips */
>  static DEFINE_MUTEX(pwm_lock);
>  
> -static DEFINE_IDR(pwmchip_idr);
> +static DEFINE_IDR(pwm_chips);

That's an unrelated change.

>  static struct pwm_chip *pwmchip_find_by_name(const char *name)
>  {
> -	struct pwm_chip *chip;
> +	struct pwm_chip_private *priv;
>  	unsigned long id, tmp;
>  
>  	if (!name)
> @@ -42,12 +44,12 @@ static struct pwm_chip *pwmchip_find_by_name(const char *name)
>  
>  	mutex_lock(&pwm_lock);
>  
> -	idr_for_each_entry_ul(&pwmchip_idr, chip, tmp, id) {
> -		const char *chip_name = dev_name(chip->dev);
> +	idr_for_each_entry_ul(&pwm_chips, priv, tmp, id) {
> +		const char *chip_name = dev_name(priv->chip->dev);

two pointer dereferences instead of one before.

>  		if (chip_name && strcmp(chip_name, name) == 0) {
>  			mutex_unlock(&pwm_lock);
> -			return chip;
> +			return priv->chip;

one pointer reference instead of none before.

>  		}
>  	}
>  
> @@ -58,23 +60,24 @@ static struct pwm_chip *pwmchip_find_by_name(const char *name)
>  
>  static int pwm_device_request(struct pwm_device *pwm, const char *label)
>  {
> +	struct pwm_chip *chip = pwm->priv->chip;

With my approach getting the chip of a struct pwm_device is only one
pointer dereference away. You need two.

>  	int err;
>  
>  	if (test_bit(PWMF_REQUESTED, &pwm->flags))
>  		return -EBUSY;
>  
> -	if (!try_module_get(pwm->chip->owner))
> +	if (!try_module_get(pwm->priv->owner))
>  		return -ENODEV;
>  
> -	if (pwm->chip->ops->request) {
> -		err = pwm->chip->ops->request(pwm->chip, pwm);
> +	if (chip->ops->request) {
> +		err = chip->ops->request(chip, pwm);

You seem to have reimplemented half of
https://lore.kernel.org/linux-pwm/20231130072105.966848-1-u.kleine-koenig@pengutronix.de
here.

>  		if (err) {
> -			module_put(pwm->chip->owner);
> +			module_put(pwm->priv->owner);
>  			return err;
>  		}
>  	}
>  
> -	if (pwm->chip->ops->get_state) {
> +	if (chip->ops->get_state) {
>  		/*
>  		 * Zero-initialize state because most drivers are unaware of
>  		 * .usage_power. The other members of state are supposed to be
> @@ -84,7 +87,7 @@ static int pwm_device_request(struct pwm_device *pwm, const char *label)
>  		 */
>  		struct pwm_state state = { 0, };
>  
> -		err = pwm->chip->ops->get_state(pwm->chip, pwm, &state);
> +		err = chip->ops->get_state(chip, pwm, &state);
>  		trace_pwm_get(pwm, &state, err);
>  
>  		if (!err)
> @@ -196,6 +199,64 @@ static bool pwm_ops_check(const struct pwm_chip *chip)
>  	return true;
>  }
>  
> +static struct pwm_chip_private *pwmchip_alloc(struct pwm_chip *chip,
> +					      struct module *owner)
> +{
> +	struct pwm_chip_private *priv;
> +	struct pwm_device *pwm;
> +	unsigned int i;
> +	int err;
> +
> +	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return ERR_PTR(-ENOMEM);
> +
> +	priv->pwms = kcalloc(chip->npwm, sizeof(*pwm), GFP_KERNEL);
> +	if (!priv->pwms) {
> +		err = -ENOMEM;
> +		goto free;
> +	}

Oh, so you don't use one additional allocation but two.

> +	priv->owner = owner;
> +	priv->chip = chip;
> +
> +	for (i = 0; i < chip->npwm; i++) {
> +		struct pwm_device *pwm = &priv->pwms[i];
> +
> +		pwm->priv = priv;
> +		pwm->hwpwm = i;
> +	}
> +
> +	mutex_lock(&pwm_lock);
> +
> +	err = idr_alloc(&pwm_chips, priv, 0, 0, GFP_KERNEL);
> +	if (err < 0) {
> +		mutex_unlock(&pwm_lock);
> +		goto free;
> +	}
> +
> +	mutex_unlock(&pwm_lock);
> +
> +	priv->id = err;
> +
> +	return priv;
> +
> +free:
> +	kfree(priv->pwms);
> +	kfree(priv);
> +	return ERR_PTR(err);
> +}
> +
> +static void pwmchip_free(struct pwm_chip_private *priv)
> +{
> +	mutex_lock(&pwm_lock);
> +	idr_remove(&pwm_chips, priv->id);
> +	mutex_unlock(&pwm_lock);
> +
> +	kfree(priv->pwms);
> +	kfree(priv);
> +}
> +
>  /**
>   * __pwmchip_add() - register a new PWM chip
>   * @chip: the PWM chip to add
> @@ -208,8 +269,7 @@ static bool pwm_ops_check(const struct pwm_chip *chip)
>   */
>  int __pwmchip_add(struct pwm_chip *chip, struct module *owner)
>  {
> -	unsigned int i;
> -	int ret;
> +	struct pwm_chip_private *priv;
>  
>  	if (!chip || !chip->dev || !chip->ops || !chip->npwm)
>  		return -EINVAL;
> @@ -217,36 +277,16 @@ int __pwmchip_add(struct pwm_chip *chip, struct module *owner)
>  	if (!pwm_ops_check(chip))
>  		return -EINVAL;
>  
> -	chip->owner = owner;
> -
> -	chip->pwms = kcalloc(chip->npwm, sizeof(*chip->pwms), GFP_KERNEL);
> -	if (!chip->pwms)
> +	priv = pwmchip_alloc(chip, owner);
> +	if (!priv)
>  		return -ENOMEM;
>  
> -	mutex_lock(&pwm_lock);
> -
> -	ret = idr_alloc(&pwmchip_idr, chip, 0, 0, GFP_KERNEL);
> -	if (ret < 0) {
> -		mutex_unlock(&pwm_lock);
> -		kfree(chip->pwms);
> -		return ret;
> -	}
> -
> -	chip->id = ret;
> -
> -	for (i = 0; i < chip->npwm; i++) {
> -		struct pwm_device *pwm = &chip->pwms[i];
> -
> -		pwm->chip = chip;
> -		pwm->hwpwm = i;
> -	}
> -
> -	mutex_unlock(&pwm_lock);
> +	chip->priv = priv;
>  
>  	if (IS_ENABLED(CONFIG_OF))
>  		of_pwmchip_add(chip);
>  
> -	pwmchip_sysfs_export(chip);
> +	pwmchip_sysfs_export(priv);
>  
>  	return 0;
>  }
> @@ -260,18 +300,14 @@ EXPORT_SYMBOL_GPL(__pwmchip_add);
>   */
>  void pwmchip_remove(struct pwm_chip *chip)
>  {
> -	pwmchip_sysfs_unexport(chip);
> -
> -	if (IS_ENABLED(CONFIG_OF))
> -		of_pwmchip_remove(chip);
> -
>  	mutex_lock(&pwm_lock);
>  
> -	idr_remove(&pwmchip_idr, chip->id);
> +	pwmchip_sysfs_unexport(chip->priv);
>  
> -	mutex_unlock(&pwm_lock);
> +	if (IS_ENABLED(CONFIG_OF))
> +		of_pwmchip_remove(chip);
>  
> -	kfree(chip->pwms);
> +	pwmchip_free(chip->priv);
>  }
>  EXPORT_SYMBOL_GPL(pwmchip_remove);
>  
> @@ -315,7 +351,7 @@ struct pwm_device *pwm_request_from_chip(struct pwm_chip *chip,
>  		return ERR_PTR(-EINVAL);
>  
>  	mutex_lock(&pwm_lock);
> -	pwm = &chip->pwms[index];
> +	pwm = &chip->priv->pwms[index];

two pointer dereferences instead of one before.

>  	err = pwm_device_request(pwm, label);
>  	if (err < 0)
> @@ -329,9 +365,9 @@ EXPORT_SYMBOL_GPL(pwm_request_from_chip);
>  static void pwm_apply_state_debug(struct pwm_device *pwm,
>  				  const struct pwm_state *state)
>  {
> -	struct pwm_state *last = &pwm->last;
> -	struct pwm_chip *chip = pwm->chip;
> +	struct pwm_chip *chip = pwm->priv->chip;
>  	struct pwm_state s1 = { 0 }, s2 = { 0 };
> +	struct pwm_state *last = &pwm->last;
>  	int err;
>  
>  	if (!IS_ENABLED(CONFIG_PWM_DEBUG))
> @@ -439,7 +475,6 @@ static void pwm_apply_state_debug(struct pwm_device *pwm,
>   */
>  int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state)
>  {
> -	struct pwm_chip *chip;
>  	int err;
>  
>  	/*
> @@ -455,8 +490,6 @@ int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state)
>  	    state->duty_cycle > state->period)
>  		return -EINVAL;
>  
> -	chip = pwm->chip;
> -
>  	if (state->period == pwm->state.period &&
>  	    state->duty_cycle == pwm->state.duty_cycle &&
>  	    state->polarity == pwm->state.polarity &&
> @@ -464,7 +497,7 @@ int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state)
>  	    state->usage_power == pwm->state.usage_power)
>  		return 0;
>  
> -	err = chip->ops->apply(chip, pwm, state);
> +	err = pwm->priv->chip->ops->apply(pwm->priv->chip, pwm, state);

four pointer dereferences instead of two before.

>  	trace_pwm_apply(pwm, state, err);
>  	if (err)
>  		return err;
> @@ -492,16 +525,19 @@ EXPORT_SYMBOL_GPL(pwm_apply_state);
>  int pwm_capture(struct pwm_device *pwm, struct pwm_capture *result,
>  		unsigned long timeout)
>  {
> +	struct pwm_chip *chip;
>  	int err;
>  
> -	if (!pwm || !pwm->chip->ops)
> +	if (!pwm)
>  		return -EINVAL;
>  
> -	if (!pwm->chip->ops->capture)
> +	chip = pwm->priv->chip;

One pointer deference more than before (assuming sensible compilation).

> +	if (!chip || !chip->ops || !chip->ops->capture)
>  		return -ENOSYS;

Changing the return code for !chip->ops isn't intended, is it?

>  	mutex_lock(&pwm_lock);
> -	err = pwm->chip->ops->capture(pwm->chip, pwm, result, timeout);
> +	err = chip->ops->capture(chip, pwm, result, timeout);
>  	mutex_unlock(&pwm_lock);
>  
>  	return err;
> @@ -566,16 +602,19 @@ EXPORT_SYMBOL_GPL(pwm_adjust_config);
>  
>  static struct pwm_chip *fwnode_to_pwmchip(struct fwnode_handle *fwnode)
>  {
> -	struct pwm_chip *chip;
> +	struct pwm_chip_private *priv;
>  	unsigned long id, tmp;
>  
>  	mutex_lock(&pwm_lock);
>  
> -	idr_for_each_entry_ul(&pwmchip_idr, chip, tmp, id)
> +	idr_for_each_entry_ul(&pwm_chips, priv, tmp, id) {
> +		struct pwm_chip *chip = priv->chip;
> +

One pointer dereference more per iteration than before.

>  		if (chip->dev && device_match_fwnode(chip->dev, fwnode)) {
>  			mutex_unlock(&pwm_lock);
>  			return chip;
>  		}
> +	}
>  
>  	mutex_unlock(&pwm_lock);
>  
> @@ -585,6 +624,7 @@ static struct pwm_chip *fwnode_to_pwmchip(struct fwnode_handle *fwnode)
>  static struct device_link *pwm_device_link_add(struct device *dev,
>  					       struct pwm_device *pwm)
>  {
> +	struct pwm_chip *chip = pwm->priv->chip;
>  	struct device_link *dl;
>  
>  	if (!dev) {
> @@ -593,15 +633,15 @@ static struct device_link *pwm_device_link_add(struct device *dev,
>  		 * impact the PM sequence ordering: the PWM supplier may get
>  		 * suspended before the consumer.
>  		 */
> -		dev_warn(pwm->chip->dev,
> +		dev_warn(chip->dev,

One pointer deference more than before (assuming sensible compilation).

>  			 "No consumer device specified to create a link to\n");
>  		return NULL;
>  	}
>  
> -	dl = device_link_add(dev, pwm->chip->dev, DL_FLAG_AUTOREMOVE_CONSUMER);
> +	dl = device_link_add(dev, chip->dev, DL_FLAG_AUTOREMOVE_CONSUMER);
>  	if (!dl) {
>  		dev_err(dev, "failed to create device link to %s\n",
> -			dev_name(pwm->chip->dev));
> +			dev_name(chip->dev));
>  		return ERR_PTR(-EINVAL);
>  	}
>  
> @@ -918,12 +958,12 @@ void pwm_put(struct pwm_device *pwm)
>  		goto out;
>  	}
>  
> -	if (pwm->chip->ops->free)
> -		pwm->chip->ops->free(pwm->chip, pwm);
> +	if (pwm->priv->chip->ops->free)
> +		pwm->priv->chip->ops->free(pwm->priv->chip, pwm);

One pointer deference more than before (assuming sensible compilation).

>  	pwm->label = NULL;
>  
> -	module_put(pwm->chip->owner);
> +	module_put(pwm->priv->owner);
>  out:
>  	mutex_unlock(&pwm_lock);
>  }
> @@ -997,12 +1037,12 @@ struct pwm_device *devm_fwnode_pwm_get(struct device *dev,
>  EXPORT_SYMBOL_GPL(devm_fwnode_pwm_get);
>  
>  #ifdef CONFIG_DEBUG_FS
> -static void pwm_dbg_show(struct pwm_chip *chip, struct seq_file *s)
> +static void pwm_dbg_show(struct pwm_chip_private *priv, struct seq_file *s)
>  {
>  	unsigned int i;
>  
> -	for (i = 0; i < chip->npwm; i++) {
> -		struct pwm_device *pwm = &chip->pwms[i];
> +	for (i = 0; i < priv->chip->npwm; i++) {
> +		struct pwm_device *pwm = &priv->pwms[i];
>  		struct pwm_state state;
>  
>  		pwm_get_state(pwm, &state);
> @@ -1035,7 +1075,7 @@ static void *pwm_seq_start(struct seq_file *s, loff_t *pos)
>  	mutex_lock(&pwm_lock);
>  	s->private = "";
>  
> -	ret = idr_get_next_ul(&pwmchip_idr, &id);
> +	ret = idr_get_next_ul(&pwm_chips, &id);
>  	*pos = id;
>  	return ret;
>  }
> @@ -1047,7 +1087,7 @@ static void *pwm_seq_next(struct seq_file *s, void *v, loff_t *pos)
>  
>  	s->private = "\n";
>  
> -	ret = idr_get_next_ul(&pwmchip_idr, &id);
> +	ret = idr_get_next_ul(&pwm_chips, &id);
>  	*pos = id;
>  	return ret;
>  }
> @@ -1059,15 +1099,16 @@ static void pwm_seq_stop(struct seq_file *s, void *v)
>  
>  static int pwm_seq_show(struct seq_file *s, void *v)
>  {
> -	struct pwm_chip *chip = v;
> +	struct pwm_chip_private *priv = v;
> +	struct pwm_chip *chip = priv->chip;

And another additional pointer dereference.

>  	seq_printf(s, "%s%d: %s/%s, %d PWM device%s\n",
> -		   (char *)s->private, chip->id,
> +		   (char *)s->private, priv->id,
>  		   chip->dev->bus ? chip->dev->bus->name : "no-bus",
>  		   dev_name(chip->dev), chip->npwm,
>  		   (chip->npwm != 1) ? "s" : "");
>  
> -	pwm_dbg_show(chip, s);
> +	pwm_dbg_show(priv, s);
>  
>  	return 0;
>  }
> diff --git a/drivers/pwm/internal.h b/drivers/pwm/internal.h
> new file mode 100644
> index 000000000000..44fffd3b6744
> --- /dev/null
> +++ b/drivers/pwm/internal.h
> @@ -0,0 +1,38 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef PWM_INTERNAL_H
> +#define PWM_INTERNAL_H
> +
> +#include <linux/list.h>

This is unused?

> +struct pwm_chip;
> +struct pwm_device;
> +
> +/**
> + * struct pwm_chip_private - subsystem-private PWM chip data
> + * @chip: driver-private PWM chip data
> + * @owner: module providing the chip
> + * @pwms: array of PWM devices allocated by the framework
> + * @base: number of first PWM controlled by this chip
> + */
> +struct pwm_chip_private {
> +	struct pwm_chip *chip;
> +	struct module *owner;
> +	struct pwm_device *pwms;
> +	unsigned int id;
> +};

With my approach the struct pwm_chip * pointer isn't needed because it's
all in one structure. So you have two structures where I only have one
and your two are bigger in sum than my single one (not counting memory
management overhead and alignment).

> +#ifdef CONFIG_PWM_SYSFS
> +void pwmchip_sysfs_export(struct pwm_chip_private *priv);
> +void pwmchip_sysfs_unexport(struct pwm_chip_private *priv);
> +#else
> +static inline void pwmchip_sysfs_export(struct pwm_chip_private *priv)
> +{
> +}
> +
> +static inline void pwmchip_sysfs_unexport(struct pwm_chip_private *priv)
> +{
> +}
> +#endif /* CONFIG_PWM_SYSFS */
> +
> +#endif /* PWM_INTERNAL_H */
> diff --git a/drivers/pwm/pwm-atmel-hlcdc.c b/drivers/pwm/pwm-atmel-hlcdc.c
> index 3f2c5031a3ba..6bbbfaa582c1 100644
> --- a/drivers/pwm/pwm-atmel-hlcdc.c
> +++ b/drivers/pwm/pwm-atmel-hlcdc.c
> @@ -15,6 +15,8 @@
>  #include <linux/pwm.h>
>  #include <linux/regmap.h>
>  
> +#include "internal.h"
> +
>  #define ATMEL_HLCDC_PWMCVAL_MASK	GENMASK(15, 8)
>  #define ATMEL_HLCDC_PWMCVAL(x)		(((x) << 8) & ATMEL_HLCDC_PWMCVAL_MASK)
>  #define ATMEL_HLCDC_PWMPOL		BIT(4)
> @@ -185,7 +187,7 @@ static int atmel_hlcdc_pwm_suspend(struct device *dev)
>  	struct atmel_hlcdc_pwm *atmel = dev_get_drvdata(dev);
>  
>  	/* Keep the periph clock enabled if the PWM is still running. */
> -	if (pwm_is_enabled(&atmel->chip.pwms[0]))
> +	if (pwm_is_enabled(&atmel->chip.priv->pwms[0]))

Didn't work so well to hide the internals of pwm_chip_private from the
drivers?

>  		clk_disable_unprepare(atmel->hlcdc->periph_clk);
>  
>  	return 0;
>  

[... a few more drivers where hiding the internals failed ...]

> diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c
> index 4edb994fa2e1..653df20afe1c 100644
> --- a/drivers/pwm/sysfs.c
> +++ b/drivers/pwm/sysfs.c
> @@ -14,6 +14,8 @@
>  #include <linux/kdev_t.h>
>  #include <linux/pwm.h>
>  
> +#include "internal.h"
> +
>  struct pwm_export {
>  	struct device child;
>  	struct pwm_device *pwm;
> @@ -311,7 +313,7 @@ static ssize_t export_store(struct device *parent,
>  			    struct device_attribute *attr,
>  			    const char *buf, size_t len)
>  {
> -	struct pwm_chip *chip = dev_get_drvdata(parent);
> +	struct pwm_chip_private *priv = dev_get_drvdata(parent);
>  	struct pwm_device *pwm;
>  	unsigned int hwpwm;
>  	int ret;
> @@ -320,10 +322,10 @@ static ssize_t export_store(struct device *parent,
>  	if (ret < 0)
>  		return ret;
>  
> -	if (hwpwm >= chip->npwm)
> +	if (hwpwm >= priv->chip->npwm)

One more pointer dereference than before. You could trade that for some
data duplication by copying npwm to struct pwmchip_priv for the case
when you need .npwm but the chip is already gone.

>  		return -ENODEV;
>  
> -	pwm = pwm_request_from_chip(chip, hwpwm, "sysfs");
> +	pwm = pwm_request_from_chip(priv->chip, hwpwm, "sysfs");
>  	if (IS_ERR(pwm))
>  		return PTR_ERR(pwm);
>  
> @@ -339,7 +341,7 @@ static ssize_t unexport_store(struct device *parent,
>  			      struct device_attribute *attr,
>  			      const char *buf, size_t len)
>  {
> -	struct pwm_chip *chip = dev_get_drvdata(parent);
> +	struct pwm_chip_private *priv = dev_get_drvdata(parent);
>  	unsigned int hwpwm;
>  	int ret;
>  
> @@ -347,10 +349,10 @@ static ssize_t unexport_store(struct device *parent,
>  	if (ret < 0)
>  		return ret;
>  
> -	if (hwpwm >= chip->npwm)
> +	if (hwpwm >= priv->chip->npwm)

One more pointer dereference than before.

>  		return -ENODEV;
>  
> -	ret = pwm_unexport_child(parent, &chip->pwms[hwpwm]);
> +	ret = pwm_unexport_child(parent, &priv->pwms[hwpwm]);
>  
>  	return ret ? : len;
>  }
> @@ -359,9 +361,9 @@ static DEVICE_ATTR_WO(unexport);
>  static ssize_t npwm_show(struct device *parent, struct device_attribute *attr,
>  			 char *buf)
>  {
> -	const struct pwm_chip *chip = dev_get_drvdata(parent);
> +	const struct pwm_chip_private *priv = dev_get_drvdata(parent);
>  
> -	return sysfs_emit(buf, "%u\n", chip->npwm);
> +	return sysfs_emit(buf, "%u\n", priv->chip->npwm);
>  }
>  static DEVICE_ATTR_RO(npwm);
>  
> @@ -411,12 +413,12 @@ static int pwm_class_apply_state(struct pwm_export *export,
>  
>  static int pwm_class_resume_npwm(struct device *parent, unsigned int npwm)
>  {
> -	struct pwm_chip *chip = dev_get_drvdata(parent);
> +	struct pwm_chip_private *priv = dev_get_drvdata(parent);
>  	unsigned int i;
>  	int ret = 0;
>  
>  	for (i = 0; i < npwm; i++) {
> -		struct pwm_device *pwm = &chip->pwms[i];
> +		struct pwm_device *pwm = &priv->pwms[i];
>  		struct pwm_state state;
>  		struct pwm_export *export;
>  
> @@ -442,12 +444,12 @@ static int pwm_class_resume_npwm(struct device *parent, unsigned int npwm)
>  
>  static int pwm_class_suspend(struct device *parent)
>  {
> -	struct pwm_chip *chip = dev_get_drvdata(parent);
> +	struct pwm_chip_private *priv = dev_get_drvdata(parent);
>  	unsigned int i;
>  	int ret = 0;
>  
> -	for (i = 0; i < chip->npwm; i++) {
> -		struct pwm_device *pwm = &chip->pwms[i];
> +	for (i = 0; i < priv->chip->npwm; i++) {
> +		struct pwm_device *pwm = &priv->pwms[i];
>  		struct pwm_state state;
>  		struct pwm_export *export;
>  
> @@ -483,9 +485,9 @@ static int pwm_class_suspend(struct device *parent)
>  
>  static int pwm_class_resume(struct device *parent)
>  {
> -	struct pwm_chip *chip = dev_get_drvdata(parent);
> +	struct pwm_chip_private *priv = dev_get_drvdata(parent);
>  
> -	return pwm_class_resume_npwm(parent, chip->npwm);
> +	return pwm_class_resume_npwm(parent, priv->chip->npwm);

One more pointer dereference than before.

>  }
>  
>  static DEFINE_SIMPLE_DEV_PM_OPS(pwm_class_pm_ops, pwm_class_suspend, pwm_class_resume);
> @@ -501,7 +503,7 @@ static int pwmchip_sysfs_match(struct device *parent, const void *data)
>  	return dev_get_drvdata(parent) == data;
>  }
>  
> -void pwmchip_sysfs_export(struct pwm_chip *chip)
> +void pwmchip_sysfs_export(struct pwm_chip_private *priv)
>  {
>  	struct device *parent;
>  
> @@ -509,26 +511,26 @@ void pwmchip_sysfs_export(struct pwm_chip *chip)
>  	 * If device_create() fails the pwm_chip is still usable by
>  	 * the kernel it's just not exported.
>  	 */
> -	parent = device_create(&pwm_class, chip->dev, MKDEV(0, 0), chip,
> -			       "pwmchip%d", chip->id);
> +	parent = device_create(&pwm_class, priv->chip->dev, MKDEV(0, 0), priv,
> +			       "pwmchip%d", priv->id);
>  	if (IS_ERR(parent)) {
> -		dev_warn(chip->dev,
> +		dev_warn(priv->chip->dev,

One more pointer dereference than before.

>  			 "device_create failed for pwm_chip sysfs export\n");
>  	}
>  }

The TL;DR; is essentially what I already wrote in my last reply to Bart
in the v3 thread[1]:

 - My approach needs more changes to the individual drivers (which I
   don't consider a relevant disadvantage given that the resulting code
   is better);
 - My approach works with less pointer dereferences which IMHO also
   simplifies understanding the code as all relevant data is in a single
   place.
 - My approach has a weaker separation between the core and the lowlevel
   drivers. That's ok in my book given that this doesn't complicate the
   lowlevel drivers and that hiding details considerably better doesn't
   work anyhow (see the drivers that need internal.h in your patch).

For me the single allocation issue is only an added bonus. The relevant
advantage of my approach is that the code is easier and (probably) more
efficient.

Best regards
Uwe

[1] https://lore.kernel.org/linux-pwm/20231205093145.l7p52xtw2ak756ra@pengutronix.de


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

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 000/115] pwm: Fix lifetime issues for pwm_chips

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 25448 bytes --]

On Fri, Dec 08, 2023 at 07:50:33PM +0100, Uwe Kleine-König wrote:
> Hello Thierry,
> 
> On Fri, Dec 08, 2023 at 04:41:39PM +0100, Thierry Reding wrote:
> > On Wed, Dec 06, 2023 at 12:43:14PM +0100, Uwe Kleine-König wrote:
> > > This series is based on Thierry's for-next.
> > > 
> > > It starts with some cleanups that were all sent out separately already:
> > > 
> > >  - "pwm: Reduce number of pointer dereferences in pwm_device_request()"
> > >    https://lore.kernel.org/linux-pwm/20231130072105.966848-1-u.kleine-koenig@pengutronix.de
> > >  - "pwm: crc: Use consistent variable naming for driver data"
> > >    https://lore.kernel.org/linux-pwm/20231130074133.969806-1-u.kleine-koenig@pengutronix.de
> > >  - Two leds/qcom-lpg patches
> > >    https://lore.kernel.org/linux-leds/20231126095230.683204-1-u.kleine-koenig@pengutronix.de
> > >    Lee already claimed to have taken the series already, but it's not yet in
> > >    next.
> > >  - "leds: qcom-lpg: Introduce a wrapper for getting driver data from a pwm chip"
> > >    https://lore.kernel.org/linux-leds/20231124215208.616551-3-u.kleine-koenig@pengutronix.de
> > > 
> > > In the following patches I changed:
> > > 
> > >  - "pwm: cros-ec: Change prototype of helper to prepare further changes" +
> > >    This was simplified in response to feedback by Tzung-Bi Shih
> > >  - Make pwmchip_priv() static (and don't export it), let drivers use a new
> > >    pwmchip_get_drvdata() instead.
> > >  - For drm/ti-sn65dsi86.c and leds/qcom-lpg make use of
> > >    pwmchip_set_drvdata() which makes the conversion to
> > >    devm_pwmchip_alloc() much prettier.
> > >  - Some cleanups here and there
> > >  - Add review tags received in v3
> > >    I kept all tags even though the pwmchip_alloc() patches changed
> > >    slightly. Most of the time that's only
> > >    s/pwmchip_priv/pwmchip_get_drvdata/ though. Still, if you object,
> > >    just tell me. (This affects Paul Cercueil on patch #68, Conor Dooley
> > >    on patch #76 and Greg for patch #109.)
> > > 
> > > I kept the pwmchip_alloc() + pwmchip_register() approach despite Bart
> > > not liking it. To balance that out I don't like Bart's alternative
> > > approach. There are no technically relevant differences between the two
> > > approaches and no benchmarks that show either of the two to be better
> > > than the other. Conceptually the design ideas around pwmchip_alloc() +
> > > pwmchip_register() are used in several other subsystems, so it's a
> > > proven way to do things.
> > 
> > [Trimming the recipients, keeping only Bart and the mailing lists.]
> > 
> > I do think there are technically relevant differences. For one, the
> > better we isolate the internal data structure, the easier this becomes
> > to manage. I'm attaching a patch that I've prototyped which should
> > basically get us to somewhere around patch 110 of this series but with
> > something like 1/8th of the changes. It doesn't need every driver to
> > change and (mostly) decouples driver code from the core code.
> 
> You don't need to touch all drivers because you didn't change struct
> pwm_chip::dev yet. (If you really want, you don't need to change that,
> but then you have some duplication as chip->dev holds the same value as
> priv->dev.parent in the end.)

I don't think that's a problem. These are for two logically separate
things, after all. Duplication can also sometimes be useful to simplify
things. There are plently of cases where we use local variables for the
same reason.

> > Now, I know that you think this is all bad because it's not a single
> > allocation, but I much prefer the end result because it's got the driver
> > and internals much more cleanly separated. Going forward I think it
> > would be easier to apply all the ref-counting on top of that because we
> > only need to keep the PWM framework-internal data structure alive after
> > a PWM chip has gone away.
> 
> Nearly all drivers need driver private data. Isn't it a nice service by
> the pwm core to make handling this private data easier for the lowlevel
> drivers?

That's only true if you think pwmchip_alloc() makes it easier. I happen
to think that it doesn't. On the contrary, I think having drivers pass
in a PWM chip descriptor that can be embedded in driver-private data is
much easier.

> > From 72ea79887d96850f9ccc832ce52b907ca276c940 Mon Sep 17 00:00:00 2001
> > From: Thierry Reding <thierry.reding@gmail.com>
> > Date: Tue, 28 Nov 2023 15:42:39 +0100
> > Subject: [PATCH] pwm: Isolate internal data into a separate structure
> > 
> > In order to prepare for proper reference counting of PWM chips and PWM
> > devices, move the internal data from the public PWM chip to a private
> > PWM chip structure. This ensures that the data that the subsystem core
> > may need to reference later on can stick around beyond the lifetime of
> > the driver-private data.
> > 
> > Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
> > ---
> >  drivers/pwm/core.c            | 185 +++++++++++++++++++++-------------
> >  drivers/pwm/internal.h        |  38 +++++++
> >  drivers/pwm/pwm-atmel-hlcdc.c |   8 +-
> >  drivers/pwm/pwm-fsl-ftm.c     |   6 +-
> >  drivers/pwm/pwm-lpc18xx-sct.c |   4 +-
> >  drivers/pwm/pwm-lpss.c        |  14 +--
> >  drivers/pwm/pwm-pca9685.c     |   6 +-
> >  drivers/pwm/pwm-samsung.c     |   6 +-
> >  drivers/pwm/pwm-sifive.c      |   4 +-
> >  drivers/pwm/pwm-stm32-lp.c    |   6 +-
> >  drivers/pwm/pwm-stm32.c       |   6 +-
> >  drivers/pwm/pwm-tiecap.c      |   6 +-
> >  drivers/pwm/pwm-tiehrpwm.c    |   6 +-
> >  drivers/pwm/sysfs.c           |  48 ++++-----
> >  include/linux/pwm.h           |  26 +----
> >  15 files changed, 228 insertions(+), 141 deletions(-)
> >  create mode 100644 drivers/pwm/internal.h
> > 
> > diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
> > index f60b715abe62..54d57dec6dce 100644
> > --- a/drivers/pwm/core.c
> > +++ b/drivers/pwm/core.c
> > @@ -24,17 +24,19 @@
> >  #define CREATE_TRACE_POINTS
> >  #include <trace/events/pwm.h>
> >  
> > +#include "internal.h"
> > +
> >  static DEFINE_MUTEX(pwm_lookup_lock);
> >  static LIST_HEAD(pwm_lookup_list);
> >  
> > -/* protects access to pwmchip_idr */
> > +/* protects access to pwm_chips */
> >  static DEFINE_MUTEX(pwm_lock);
> >  
> > -static DEFINE_IDR(pwmchip_idr);
> > +static DEFINE_IDR(pwm_chips);
> 
> That's an unrelated change.

Yeah, it should probably go into the patch that changes the list to an
IDR. There's really no reason why this needed to change in the first
place.

> 
> >  static struct pwm_chip *pwmchip_find_by_name(const char *name)
> >  {
> > -	struct pwm_chip *chip;
> > +	struct pwm_chip_private *priv;
> >  	unsigned long id, tmp;
> >  
> >  	if (!name)
> > @@ -42,12 +44,12 @@ static struct pwm_chip *pwmchip_find_by_name(const char *name)
> >  
> >  	mutex_lock(&pwm_lock);
> >  
> > -	idr_for_each_entry_ul(&pwmchip_idr, chip, tmp, id) {
> > -		const char *chip_name = dev_name(chip->dev);
> > +	idr_for_each_entry_ul(&pwm_chips, priv, tmp, id) {
> > +		const char *chip_name = dev_name(priv->chip->dev);
> 
> two pointer dereferences instead of one before.
> 
> >  		if (chip_name && strcmp(chip_name, name) == 0) {
> >  			mutex_unlock(&pwm_lock);
> > -			return chip;
> > +			return priv->chip;
> 
> one pointer reference instead of none before.
> 
> >  		}
> >  	}
> >  
> > @@ -58,23 +60,24 @@ static struct pwm_chip *pwmchip_find_by_name(const char *name)
> >  
> >  static int pwm_device_request(struct pwm_device *pwm, const char *label)
> >  {
> > +	struct pwm_chip *chip = pwm->priv->chip;
> 
> With my approach getting the chip of a struct pwm_device is only one
> pointer dereference away. You need two.

None of the functions here are called very often, so even if this isn't
optimized away it would hardly matter.

> >  	int err;
> >  
> >  	if (test_bit(PWMF_REQUESTED, &pwm->flags))
> >  		return -EBUSY;
> >  
> > -	if (!try_module_get(pwm->chip->owner))
> > +	if (!try_module_get(pwm->priv->owner))
> >  		return -ENODEV;
> >  
> > -	if (pwm->chip->ops->request) {
> > -		err = pwm->chip->ops->request(pwm->chip, pwm);
> > +	if (chip->ops->request) {
> > +		err = chip->ops->request(chip, pwm);
> 
> You seem to have reimplemented half of
> https://lore.kernel.org/linux-pwm/20231130072105.966848-1-u.kleine-koenig@pengutronix.de
> here.

I started prototyping this before you sent out that patch.

> >  		if (err) {
> > -			module_put(pwm->chip->owner);
> > +			module_put(pwm->priv->owner);
> >  			return err;
> >  		}
> >  	}
> >  
> > -	if (pwm->chip->ops->get_state) {
> > +	if (chip->ops->get_state) {
> >  		/*
> >  		 * Zero-initialize state because most drivers are unaware of
> >  		 * .usage_power. The other members of state are supposed to be
> > @@ -84,7 +87,7 @@ static int pwm_device_request(struct pwm_device *pwm, const char *label)
> >  		 */
> >  		struct pwm_state state = { 0, };
> >  
> > -		err = pwm->chip->ops->get_state(pwm->chip, pwm, &state);
> > +		err = chip->ops->get_state(chip, pwm, &state);
> >  		trace_pwm_get(pwm, &state, err);
> >  
> >  		if (!err)
> > @@ -196,6 +199,64 @@ static bool pwm_ops_check(const struct pwm_chip *chip)
> >  	return true;
> >  }
> >  
> > +static struct pwm_chip_private *pwmchip_alloc(struct pwm_chip *chip,
> > +					      struct module *owner)
> > +{
> > +	struct pwm_chip_private *priv;
> > +	struct pwm_device *pwm;
> > +	unsigned int i;
> > +	int err;
> > +
> > +	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> > +	if (!priv)
> > +		return ERR_PTR(-ENOMEM);
> > +
> > +	priv->pwms = kcalloc(chip->npwm, sizeof(*pwm), GFP_KERNEL);
> > +	if (!priv->pwms) {
> > +		err = -ENOMEM;
> > +		goto free;
> > +	}
> 
> Oh, so you don't use one additional allocation but two.

This could easily be folded into one, but we already determined that we
both have different opinions about whether this matters a lot or not.

> 
> > +	priv->owner = owner;
> > +	priv->chip = chip;
> > +
> > +	for (i = 0; i < chip->npwm; i++) {
> > +		struct pwm_device *pwm = &priv->pwms[i];
> > +
> > +		pwm->priv = priv;
> > +		pwm->hwpwm = i;
> > +	}
> > +
> > +	mutex_lock(&pwm_lock);
> > +
> > +	err = idr_alloc(&pwm_chips, priv, 0, 0, GFP_KERNEL);
> > +	if (err < 0) {
> > +		mutex_unlock(&pwm_lock);
> > +		goto free;
> > +	}
> > +
> > +	mutex_unlock(&pwm_lock);
> > +
> > +	priv->id = err;
> > +
> > +	return priv;
> > +
> > +free:
> > +	kfree(priv->pwms);
> > +	kfree(priv);
> > +	return ERR_PTR(err);
> > +}
> > +
> > +static void pwmchip_free(struct pwm_chip_private *priv)
> > +{
> > +	mutex_lock(&pwm_lock);
> > +	idr_remove(&pwm_chips, priv->id);
> > +	mutex_unlock(&pwm_lock);
> > +
> > +	kfree(priv->pwms);
> > +	kfree(priv);
> > +}
> > +
> >  /**
> >   * __pwmchip_add() - register a new PWM chip
> >   * @chip: the PWM chip to add
> > @@ -208,8 +269,7 @@ static bool pwm_ops_check(const struct pwm_chip *chip)
> >   */
> >  int __pwmchip_add(struct pwm_chip *chip, struct module *owner)
> >  {
> > -	unsigned int i;
> > -	int ret;
> > +	struct pwm_chip_private *priv;
> >  
> >  	if (!chip || !chip->dev || !chip->ops || !chip->npwm)
> >  		return -EINVAL;
> > @@ -217,36 +277,16 @@ int __pwmchip_add(struct pwm_chip *chip, struct module *owner)
> >  	if (!pwm_ops_check(chip))
> >  		return -EINVAL;
> >  
> > -	chip->owner = owner;
> > -
> > -	chip->pwms = kcalloc(chip->npwm, sizeof(*chip->pwms), GFP_KERNEL);
> > -	if (!chip->pwms)
> > +	priv = pwmchip_alloc(chip, owner);
> > +	if (!priv)
> >  		return -ENOMEM;
> >  
> > -	mutex_lock(&pwm_lock);
> > -
> > -	ret = idr_alloc(&pwmchip_idr, chip, 0, 0, GFP_KERNEL);
> > -	if (ret < 0) {
> > -		mutex_unlock(&pwm_lock);
> > -		kfree(chip->pwms);
> > -		return ret;
> > -	}
> > -
> > -	chip->id = ret;
> > -
> > -	for (i = 0; i < chip->npwm; i++) {
> > -		struct pwm_device *pwm = &chip->pwms[i];
> > -
> > -		pwm->chip = chip;
> > -		pwm->hwpwm = i;
> > -	}
> > -
> > -	mutex_unlock(&pwm_lock);
> > +	chip->priv = priv;
> >  
> >  	if (IS_ENABLED(CONFIG_OF))
> >  		of_pwmchip_add(chip);
> >  
> > -	pwmchip_sysfs_export(chip);
> > +	pwmchip_sysfs_export(priv);
> >  
> >  	return 0;
> >  }
> > @@ -260,18 +300,14 @@ EXPORT_SYMBOL_GPL(__pwmchip_add);
> >   */
> >  void pwmchip_remove(struct pwm_chip *chip)
> >  {
> > -	pwmchip_sysfs_unexport(chip);
> > -
> > -	if (IS_ENABLED(CONFIG_OF))
> > -		of_pwmchip_remove(chip);
> > -
> >  	mutex_lock(&pwm_lock);
> >  
> > -	idr_remove(&pwmchip_idr, chip->id);
> > +	pwmchip_sysfs_unexport(chip->priv);
> >  
> > -	mutex_unlock(&pwm_lock);
> > +	if (IS_ENABLED(CONFIG_OF))
> > +		of_pwmchip_remove(chip);
> >  
> > -	kfree(chip->pwms);
> > +	pwmchip_free(chip->priv);
> >  }
> >  EXPORT_SYMBOL_GPL(pwmchip_remove);
> >  
> > @@ -315,7 +351,7 @@ struct pwm_device *pwm_request_from_chip(struct pwm_chip *chip,
> >  		return ERR_PTR(-EINVAL);
> >  
> >  	mutex_lock(&pwm_lock);
> > -	pwm = &chip->pwms[index];
> > +	pwm = &chip->priv->pwms[index];
> 
> two pointer dereferences instead of one before.
> 
> >  	err = pwm_device_request(pwm, label);
> >  	if (err < 0)
> > @@ -329,9 +365,9 @@ EXPORT_SYMBOL_GPL(pwm_request_from_chip);
> >  static void pwm_apply_state_debug(struct pwm_device *pwm,
> >  				  const struct pwm_state *state)
> >  {
> > -	struct pwm_state *last = &pwm->last;
> > -	struct pwm_chip *chip = pwm->chip;
> > +	struct pwm_chip *chip = pwm->priv->chip;
> >  	struct pwm_state s1 = { 0 }, s2 = { 0 };
> > +	struct pwm_state *last = &pwm->last;
> >  	int err;
> >  
> >  	if (!IS_ENABLED(CONFIG_PWM_DEBUG))
> > @@ -439,7 +475,6 @@ static void pwm_apply_state_debug(struct pwm_device *pwm,
> >   */
> >  int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state)
> >  {
> > -	struct pwm_chip *chip;
> >  	int err;
> >  
> >  	/*
> > @@ -455,8 +490,6 @@ int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state)
> >  	    state->duty_cycle > state->period)
> >  		return -EINVAL;
> >  
> > -	chip = pwm->chip;
> > -
> >  	if (state->period == pwm->state.period &&
> >  	    state->duty_cycle == pwm->state.duty_cycle &&
> >  	    state->polarity == pwm->state.polarity &&
> > @@ -464,7 +497,7 @@ int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state)
> >  	    state->usage_power == pwm->state.usage_power)
> >  		return 0;
> >  
> > -	err = chip->ops->apply(chip, pwm, state);
> > +	err = pwm->priv->chip->ops->apply(pwm->priv->chip, pwm, state);
> 
> four pointer dereferences instead of two before.
> 
> >  	trace_pwm_apply(pwm, state, err);
> >  	if (err)
> >  		return err;
> > @@ -492,16 +525,19 @@ EXPORT_SYMBOL_GPL(pwm_apply_state);
> >  int pwm_capture(struct pwm_device *pwm, struct pwm_capture *result,
> >  		unsigned long timeout)
> >  {
> > +	struct pwm_chip *chip;
> >  	int err;
> >  
> > -	if (!pwm || !pwm->chip->ops)
> > +	if (!pwm)
> >  		return -EINVAL;
> >  
> > -	if (!pwm->chip->ops->capture)
> > +	chip = pwm->priv->chip;
> 
> One pointer deference more than before (assuming sensible compilation).
> 
> > +	if (!chip || !chip->ops || !chip->ops->capture)
> >  		return -ENOSYS;
> 
> Changing the return code for !chip->ops isn't intended, is it?

It's a quick prototype, I didn't pay particular attention to keeping all
error codes exactly as they were before.

> > diff --git a/drivers/pwm/internal.h b/drivers/pwm/internal.h
> > new file mode 100644
> > index 000000000000..44fffd3b6744
> > --- /dev/null
> > +++ b/drivers/pwm/internal.h
> > @@ -0,0 +1,38 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +
> > +#ifndef PWM_INTERNAL_H
> > +#define PWM_INTERNAL_H
> > +
> > +#include <linux/list.h>
> 
> This is unused?

Yes, can be dropped now. This was prototyped prior to the IDR rework and
I forgot to drop this after rebasing it on top.

> > +struct pwm_chip;
> > +struct pwm_device;
> > +
> > +/**
> > + * struct pwm_chip_private - subsystem-private PWM chip data
> > + * @chip: driver-private PWM chip data
> > + * @owner: module providing the chip
> > + * @pwms: array of PWM devices allocated by the framework
> > + * @base: number of first PWM controlled by this chip
> > + */
> > +struct pwm_chip_private {
> > +	struct pwm_chip *chip;
> > +	struct module *owner;
> > +	struct pwm_device *pwms;
> > +	unsigned int id;
> > +};
> 
> With my approach the struct pwm_chip * pointer isn't needed because it's
> all in one structure. So you have two structures where I only have one
> and your two are bigger in sum than my single one (not counting memory
> management overhead and alignment).

The split is on purpose and has the benefit of clearly separating the
PWM internal data structures from the driver accessible ones. So in my
book this is an advantage.

> > +#ifdef CONFIG_PWM_SYSFS
> > +void pwmchip_sysfs_export(struct pwm_chip_private *priv);
> > +void pwmchip_sysfs_unexport(struct pwm_chip_private *priv);
> > +#else
> > +static inline void pwmchip_sysfs_export(struct pwm_chip_private *priv)
> > +{
> > +}
> > +
> > +static inline void pwmchip_sysfs_unexport(struct pwm_chip_private *priv)
> > +{
> > +}
> > +#endif /* CONFIG_PWM_SYSFS */
> > +
> > +#endif /* PWM_INTERNAL_H */
> > diff --git a/drivers/pwm/pwm-atmel-hlcdc.c b/drivers/pwm/pwm-atmel-hlcdc.c
> > index 3f2c5031a3ba..6bbbfaa582c1 100644
> > --- a/drivers/pwm/pwm-atmel-hlcdc.c
> > +++ b/drivers/pwm/pwm-atmel-hlcdc.c
> > @@ -15,6 +15,8 @@
> >  #include <linux/pwm.h>
> >  #include <linux/regmap.h>
> >  
> > +#include "internal.h"
> > +
> >  #define ATMEL_HLCDC_PWMCVAL_MASK	GENMASK(15, 8)
> >  #define ATMEL_HLCDC_PWMCVAL(x)		(((x) << 8) & ATMEL_HLCDC_PWMCVAL_MASK)
> >  #define ATMEL_HLCDC_PWMPOL		BIT(4)
> > @@ -185,7 +187,7 @@ static int atmel_hlcdc_pwm_suspend(struct device *dev)
> >  	struct atmel_hlcdc_pwm *atmel = dev_get_drvdata(dev);
> >  
> >  	/* Keep the periph clock enabled if the PWM is still running. */
> > -	if (pwm_is_enabled(&atmel->chip.pwms[0]))
> > +	if (pwm_is_enabled(&atmel->chip.priv->pwms[0]))
> 
> Didn't work so well to hide the internals of pwm_chip_private from the
> drivers?
> 
> >  		clk_disable_unprepare(atmel->hlcdc->periph_clk);
> >  
> >  	return 0;
> >  
> 
> [... a few more drivers where hiding the internals failed ...]

Yes, and you'll recall that these are for exactly the cases where in the
past you have argued that the core should take a more active role in
making sure that PWMs get properly disabled. These should be temporary
and once this logic is all moved into the core these should disappear.

> 
> > diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c
> > index 4edb994fa2e1..653df20afe1c 100644
> > --- a/drivers/pwm/sysfs.c
> > +++ b/drivers/pwm/sysfs.c
> > @@ -14,6 +14,8 @@
> >  #include <linux/kdev_t.h>
> >  #include <linux/pwm.h>
> >  
> > +#include "internal.h"
> > +
> >  struct pwm_export {
> >  	struct device child;
> >  	struct pwm_device *pwm;
> > @@ -311,7 +313,7 @@ static ssize_t export_store(struct device *parent,
> >  			    struct device_attribute *attr,
> >  			    const char *buf, size_t len)
> >  {
> > -	struct pwm_chip *chip = dev_get_drvdata(parent);
> > +	struct pwm_chip_private *priv = dev_get_drvdata(parent);
> >  	struct pwm_device *pwm;
> >  	unsigned int hwpwm;
> >  	int ret;
> > @@ -320,10 +322,10 @@ static ssize_t export_store(struct device *parent,
> >  	if (ret < 0)
> >  		return ret;
> >  
> > -	if (hwpwm >= chip->npwm)
> > +	if (hwpwm >= priv->chip->npwm)
> 
> One more pointer dereference than before. You could trade that for some
> data duplication by copying npwm to struct pwmchip_priv for the case
> when you need .npwm but the chip is already gone.

When the chip is gone, none of these functions should be doing anything
other than returning an error. None of that is implemented in this yet.
This is merely a preparatory patch.

> 
> >  		return -ENODEV;
> >  
> > -	pwm = pwm_request_from_chip(chip, hwpwm, "sysfs");
> > +	pwm = pwm_request_from_chip(priv->chip, hwpwm, "sysfs");
> >  	if (IS_ERR(pwm))
> >  		return PTR_ERR(pwm);
> >  
> > @@ -339,7 +341,7 @@ static ssize_t unexport_store(struct device *parent,
> >  			      struct device_attribute *attr,
> >  			      const char *buf, size_t len)
> >  {
> > -	struct pwm_chip *chip = dev_get_drvdata(parent);
> > +	struct pwm_chip_private *priv = dev_get_drvdata(parent);
> >  	unsigned int hwpwm;
> >  	int ret;
> >  
> > @@ -347,10 +349,10 @@ static ssize_t unexport_store(struct device *parent,
> >  	if (ret < 0)
> >  		return ret;
> >  
> > -	if (hwpwm >= chip->npwm)
> > +	if (hwpwm >= priv->chip->npwm)
> 
> One more pointer dereference than before.
> 
> >  		return -ENODEV;
> >  
> > -	ret = pwm_unexport_child(parent, &chip->pwms[hwpwm]);
> > +	ret = pwm_unexport_child(parent, &priv->pwms[hwpwm]);
> >  
> >  	return ret ? : len;
> >  }
> > @@ -359,9 +361,9 @@ static DEVICE_ATTR_WO(unexport);
> >  static ssize_t npwm_show(struct device *parent, struct device_attribute *attr,
> >  			 char *buf)
> >  {
> > -	const struct pwm_chip *chip = dev_get_drvdata(parent);
> > +	const struct pwm_chip_private *priv = dev_get_drvdata(parent);
> >  
> > -	return sysfs_emit(buf, "%u\n", chip->npwm);
> > +	return sysfs_emit(buf, "%u\n", priv->chip->npwm);
> >  }
> >  static DEVICE_ATTR_RO(npwm);
> >  
> > @@ -411,12 +413,12 @@ static int pwm_class_apply_state(struct pwm_export *export,
> >  
> >  static int pwm_class_resume_npwm(struct device *parent, unsigned int npwm)
> >  {
> > -	struct pwm_chip *chip = dev_get_drvdata(parent);
> > +	struct pwm_chip_private *priv = dev_get_drvdata(parent);
> >  	unsigned int i;
> >  	int ret = 0;
> >  
> >  	for (i = 0; i < npwm; i++) {
> > -		struct pwm_device *pwm = &chip->pwms[i];
> > +		struct pwm_device *pwm = &priv->pwms[i];
> >  		struct pwm_state state;
> >  		struct pwm_export *export;
> >  
> > @@ -442,12 +444,12 @@ static int pwm_class_resume_npwm(struct device *parent, unsigned int npwm)
> >  
> >  static int pwm_class_suspend(struct device *parent)
> >  {
> > -	struct pwm_chip *chip = dev_get_drvdata(parent);
> > +	struct pwm_chip_private *priv = dev_get_drvdata(parent);
> >  	unsigned int i;
> >  	int ret = 0;
> >  
> > -	for (i = 0; i < chip->npwm; i++) {
> > -		struct pwm_device *pwm = &chip->pwms[i];
> > +	for (i = 0; i < priv->chip->npwm; i++) {
> > +		struct pwm_device *pwm = &priv->pwms[i];
> >  		struct pwm_state state;
> >  		struct pwm_export *export;
> >  
> > @@ -483,9 +485,9 @@ static int pwm_class_suspend(struct device *parent)
> >  
> >  static int pwm_class_resume(struct device *parent)
> >  {
> > -	struct pwm_chip *chip = dev_get_drvdata(parent);
> > +	struct pwm_chip_private *priv = dev_get_drvdata(parent);
> >  
> > -	return pwm_class_resume_npwm(parent, chip->npwm);
> > +	return pwm_class_resume_npwm(parent, priv->chip->npwm);
> 
> One more pointer dereference than before.
> 
> >  }
> >  
> >  static DEFINE_SIMPLE_DEV_PM_OPS(pwm_class_pm_ops, pwm_class_suspend, pwm_class_resume);
> > @@ -501,7 +503,7 @@ static int pwmchip_sysfs_match(struct device *parent, const void *data)
> >  	return dev_get_drvdata(parent) == data;
> >  }
> >  
> > -void pwmchip_sysfs_export(struct pwm_chip *chip)
> > +void pwmchip_sysfs_export(struct pwm_chip_private *priv)
> >  {
> >  	struct device *parent;
> >  
> > @@ -509,26 +511,26 @@ void pwmchip_sysfs_export(struct pwm_chip *chip)
> >  	 * If device_create() fails the pwm_chip is still usable by
> >  	 * the kernel it's just not exported.
> >  	 */
> > -	parent = device_create(&pwm_class, chip->dev, MKDEV(0, 0), chip,
> > -			       "pwmchip%d", chip->id);
> > +	parent = device_create(&pwm_class, priv->chip->dev, MKDEV(0, 0), priv,
> > +			       "pwmchip%d", priv->id);
> >  	if (IS_ERR(parent)) {
> > -		dev_warn(chip->dev,
> > +		dev_warn(priv->chip->dev,
> 
> One more pointer dereference than before.
> 
> >  			 "device_create failed for pwm_chip sysfs export\n");
> >  	}
> >  }
> 
> The TL;DR; is essentially what I already wrote in my last reply to Bart
> in the v3 thread[1]:
> 
>  - My approach needs more changes to the individual drivers (which I
>    don't consider a relevant disadvantage given that the resulting code
>    is better);
>  - My approach works with less pointer dereferences which IMHO also
>    simplifies understanding the code as all relevant data is in a single
>    place.
>  - My approach has a weaker separation between the core and the lowlevel
>    drivers. That's ok in my book given that this doesn't complicate the
>    lowlevel drivers and that hiding details considerably better doesn't
>    work anyhow (see the drivers that need internal.h in your patch).
> 
> For me the single allocation issue is only an added bonus. The relevant
> advantage of my approach is that the code is easier and (probably) more
> efficient.

I happen to disagree. I think adding pwmchip_alloc() makes things much
more complicated for low level drivers.

Thierry

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 000/115] pwm: Fix lifetime issues for pwm_chips

[Uwe Kleine-König]

[-- Attachment #1.1: Type: text/plain, Size: 3593 bytes --]

Hello Thierry,

On Mon, Dec 11, 2023 at 12:33:04PM +0100, Thierry Reding wrote:
> On Fri, Dec 08, 2023 at 07:50:33PM +0100, Uwe Kleine-König wrote:
> > The TL;DR; is essentially what I already wrote in my last reply to Bart
> > in the v3 thread[1]:
> > 
> >  - My approach needs more changes to the individual drivers (which I
> >    don't consider a relevant disadvantage given that the resulting code
> >    is better);
> >  - My approach works with less pointer dereferences which IMHO also
> >    simplifies understanding the code as all relevant data is in a single
> >    place.
> >  - My approach has a weaker separation between the core and the lowlevel
> >    drivers. That's ok in my book given that this doesn't complicate the
> >    lowlevel drivers and that hiding details considerably better doesn't
> >    work anyhow (see the drivers that need internal.h in your patch).
> > 
> > For me the single allocation issue is only an added bonus. The relevant
> > advantage of my approach is that the code is easier and (probably) more
> > efficient.
> 
> I happen to disagree. I think adding pwmchip_alloc() makes things much
> more complicated for low level drivers.

Looking at e.g.
https://lore.kernel.org/linux-pwm/2dda818b8bbbe8ba4b9df5ab54f960ff4a4f1ab5.1701860672.git.u.kleine-koenig@pengutronix.de/
I wonder where you see "much more complication". OK, there are two
pointers now for chip and private data, but I'd call that at most a
"mild" complication[1] which is more than balanced out by the
simplifications in the remaining parts of that patch.

Best regards
Uwe

[1] I'm not sure I'd refuse someone suggesting the following patch on
    top of today's next:

	diff --git a/drivers/pwm/pwm-microchip-core.c b/drivers/pwm/pwm-microchip-core.c
	index c0c53968f3e9..d32e65914599 100644
	--- a/drivers/pwm/pwm-microchip-core.c
	+++ b/drivers/pwm/pwm-microchip-core.c
	@@ -448,12 +448,14 @@ MODULE_DEVICE_TABLE(of, mchp_core_of_match);
	 static int mchp_core_pwm_probe(struct platform_device *pdev)
	 {
		struct mchp_core_pwm_chip *mchp_core_pwm;
	+	struct pwm_chip *chip;
		struct resource *regs;
		int ret;
	 
		mchp_core_pwm = devm_kzalloc(&pdev->dev, sizeof(*mchp_core_pwm), GFP_KERNEL);
		if (!mchp_core_pwm)
			return -ENOMEM;
	+	chip = &mchp_core_pwm->chip;
	 
		mchp_core_pwm->base = devm_platform_get_and_ioremap_resource(pdev, 0, &regs);
		if (IS_ERR(mchp_core_pwm->base))
	@@ -470,9 +472,9 @@ static int mchp_core_pwm_probe(struct platform_device *pdev)
	 
		mutex_init(&mchp_core_pwm->lock);
	 
	-	mchp_core_pwm->chip.dev = &pdev->dev;
	-	mchp_core_pwm->chip.ops = &mchp_core_pwm_ops;
	-	mchp_core_pwm->chip.npwm = 16;
	+	chip->dev = &pdev->dev;
	+	chip->ops = &mchp_core_pwm_ops;
	+	chip->npwm = 16;
	 
		mchp_core_pwm->channel_enabled = readb_relaxed(mchp_core_pwm->base + MCHPCOREPWM_EN(0));
		mchp_core_pwm->channel_enabled |=
	@@ -485,7 +487,7 @@ static int mchp_core_pwm_probe(struct platform_device *pdev)
		writel_relaxed(1U, mchp_core_pwm->base + MCHPCOREPWM_SYNC_UPD);
		mchp_core_pwm->update_timestamp = ktime_get();
	 
	-	ret = devm_pwmchip_add(&pdev->dev, &mchp_core_pwm->chip);
	+	ret = devm_pwmchip_add(&pdev->dev, chip);
		if (ret)
			return dev_err_probe(&pdev->dev, ret, "Failed to add pwmchip\n");
 
    With that applied before the above mentioned patch there is no
    complication at all in my eyes.


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

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 000/115] pwm: Fix lifetime issues for pwm_chips

[Uwe Kleine-König]

[-- Attachment #1.1: Type: text/plain, Size: 1945 bytes --]

Hello Thierry,

On Mon, Dec 11, 2023 at 12:33:04PM +0100, Thierry Reding wrote:
> On Fri, Dec 08, 2023 at 07:50:33PM +0100, Uwe Kleine-König wrote:
> > You don't need to touch all drivers because you didn't change struct
> > pwm_chip::dev yet. (If you really want, you don't need to change that,
> > but then you have some duplication as chip->dev holds the same value as
> > priv->dev.parent in the end.)
> 
> I don't think that's a problem. These are for two logically separate
> things, after all.

How are they different? I'd say one is the initializer for the other and
(ideally) unused after that. With that interpretation they are indeed
different, but then it's ugly that the initializer keeps staying around.

> Duplication can also sometimes be useful to simplify
> things. There are plently of cases where we use local variables for the
> same reason.

local variables go away though after the respective function is left.
chip->dev and its copy priv->dev.parent stay around for the full
lifetime of the chip.

> > > @@ -58,23 +60,24 @@ static struct pwm_chip *pwmchip_find_by_name(const char *name)
> > >  
> > >  static int pwm_device_request(struct pwm_device *pwm, const char *label)
> > >  {
> > > +	struct pwm_chip *chip = pwm->priv->chip;
> > 
> > With my approach getting the chip of a struct pwm_device is only one
> > pointer dereference away. You need two.
> 
> None of the functions here are called very often, so even if this isn't
> optimized away it would hardly matter.

I'd say pwm_apply_state() at least matters. Also I think that making a
slow path quicker is a good thing. 

I wonder how we'll converge to an approach that can go into the
mainline given that we both have our strong opinions.

Best regards
Uwe

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

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel