Linux-PWM Archive mirror
 help / color / mirror / Atom feed
From: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
To: Lee Jones <lee@kernel.org>
Cc: Rob Herring <robh@kernel.org>,
	Bjorn Andersson <quic_bjorande@quicinc.com>,
	Kees Cook <keescook@chromium.org>,
	linux-pwm@vger.kernel.org, Luca Weiss <luca@z3ntu.xyz>,
	Conor Dooley <conor.dooley@microchip.com>,
	Thierry Reding <thierry.reding@gmail.com>,
	linux-leds@vger.kernel.org, Pavel Machek <pavel@ucw.cz>,
	kernel@pengutronix.de,
	Anjelique Melendez <quic_amelende@quicinc.com>
Subject: Re: [PATCH v3 102/108] leds: qcom-lpg: Make use of devm_pwmchip_alloc() function
Date: Wed, 22 Nov 2023 18:54:13 +0100	[thread overview]
Message-ID: <20231122175413.5yxsdveausehkbgm@pengutronix.de> (raw)
In-Reply-To: <20231122115621.GK173820@google.com>

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

Hello Lee,

On Wed, Nov 22, 2023 at 11:56:21AM +0000, Lee Jones wrote:
> On Tue, 21 Nov 2023, Uwe Kleine-König wrote:
> 
> > This prepares the pwm sub-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/leds/rgb/leds-qcom-lpg.c | 30 +++++++++++++++++++++---------
> >  1 file changed, 21 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/leds/rgb/leds-qcom-lpg.c b/drivers/leds/rgb/leds-qcom-lpg.c
> > index 68d82a682bf6..283227e02df6 100644
> > --- a/drivers/leds/rgb/leds-qcom-lpg.c
> > +++ b/drivers/leds/rgb/leds-qcom-lpg.c
> > @@ -77,7 +77,7 @@ struct lpg {
> >  
> >  	struct mutex lock;
> >  
> > -	struct pwm_chip pwm;
> > +	struct pwm_chip *pwm;
> >  
> >  	const struct lpg_data *data;
> >  
> > @@ -977,9 +977,15 @@ static int lpg_pattern_mc_clear(struct led_classdev *cdev)
> >  	return lpg_pattern_clear(led);
> >  }
> >  
> > +static inline struct lpg *lpg_pwm_from_chip(struct pwm_chip *chip)
> > +{
> > +	struct lpg **lpg = pwmchip_priv(chip);
> > +	return *lpg;
> > +}
> 
> I don't have easy-vis into the other patches, but if this is a common
> pattern, perhaps add a generic helper in <linux/pwm.h>?
> 
> >  static int lpg_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
> >  {
> > -	struct lpg *lpg = container_of(chip, struct lpg, pwm);
> > +	struct lpg *lpg = lpg_pwm_from_chip(chip);
> >  	struct lpg_channel *chan = &lpg->channels[pwm->hwpwm];
> >  
> >  	return chan->in_use ? -EBUSY : 0;
> > [...]
> > @@ -1089,13 +1095,19 @@ static const struct pwm_ops lpg_pwm_ops = {
> >  
> >  static int lpg_add_pwm(struct lpg *lpg)
> >  {
> > +	struct pwm_chip *chip;
> >  	int ret;
> >  
> > -	lpg->pwm.dev = lpg->dev;
> > -	lpg->pwm.npwm = lpg->num_channels;
> > -	lpg->pwm.ops = &lpg_pwm_ops;
> > +	lpg->pwm = chip = devm_pwmchip_alloc(lpg->dev, lpg->num_channels,
> > +					     sizeof(&lpg));
> > +	if (IS_ERR(chip))
> > +		return PTR_ERR(chip);
> >  
> > -	ret = pwmchip_add(&lpg->pwm);
> > +	*(struct lpg **)pwmchip_priv(chip) = lpg;
> 
> This is vile!

This is indeed one of the uglier conversions. It gets a bit prettier
with the following addon patch:

diff --git a/drivers/leds/rgb/leds-qcom-lpg.c b/drivers/leds/rgb/leds-qcom-lpg.c
index 283227e02df6..e09eba823057 100644
--- a/drivers/leds/rgb/leds-qcom-lpg.c
+++ b/drivers/leds/rgb/leds-qcom-lpg.c
@@ -206,6 +206,10 @@ struct lpg_data {
 	const struct lpg_channel_data *channels;
 };
 
+struct lpg_pwm_data {
+	struct lpg *lpg;
+};
+
 static int triled_set(struct lpg *lpg, unsigned int mask, unsigned int enable)
 {
 	/* Skip if we don't have a triled block */
@@ -979,8 +983,9 @@ static int lpg_pattern_mc_clear(struct led_classdev *cdev)
 
 static inline struct lpg *lpg_pwm_from_chip(struct pwm_chip *chip)
 {
-	struct lpg **lpg = pwmchip_priv(chip);
-	return *lpg;
+	struct lpg_pwm_data *lpg_pwm_data = pwmchip_priv(chip);
+
+	return lpg_pwm_data->lpg;
 }
 
 static int lpg_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
@@ -1096,14 +1101,16 @@ static const struct pwm_ops lpg_pwm_ops = {
 static int lpg_add_pwm(struct lpg *lpg)
 {
 	struct pwm_chip *chip;
+	struct lpg_pwm_data *lpg_pwm_data;
 	int ret;
 
 	lpg->pwm = chip = devm_pwmchip_alloc(lpg->dev, lpg->num_channels,
-					     sizeof(&lpg));
+					     sizeof(*lpg_pwm_data));
 	if (IS_ERR(chip))
 		return PTR_ERR(chip);
 
-	*(struct lpg **)pwmchip_priv(chip) = lpg;
+	lpg_pwm_data = pwmchip_priv(chip);
+	lpg_pwm_data->lpg = lpg;
 
 	chip->ops = &lpg_pwm_ops;
 
Would you like it better then?

Best regards
Uwe

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

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

  parent reply	other threads:[~2023-11-22 17:54 UTC|newest]

Thread overview: 163+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-21 13:49 [PATCH v3 000/108] pwm: Fix lifetime issues for pwm_chips Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 001/108] pwm: cros-ec: Change prototype of helper to prepare further changes Uwe Kleine-König
2023-11-22  8:52   ` Tzung-Bi Shih
2023-11-23  9:24     ` Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 002/108] pwm: Provide a macro to get the parent device of a given chip Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 003/108] pwm: ab8500: Make use of pwmchip_parent() macro Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 004/108] pwm: atmel: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 005/108] pwm: atmel-tcb: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 006/108] pwm: bcm-kona: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 007/108] pwm: crc: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 008/108] pwm: cros-ec: " Uwe Kleine-König
2023-11-22  8:52   ` Tzung-Bi Shih
2023-11-21 13:49 ` [PATCH v3 009/108] pwm: dwc: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 010/108] pwm: ep93xx: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 011/108] pwm: fsl-ftm: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 012/108] pwm: img: Make use of parent device pointer in driver data Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 013/108] pwm: imx27: Make use of pwmchip_parent() macro Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 014/108] pwm: jz4740: " Uwe Kleine-König
2023-11-21 14:13   ` Paul Cercueil
2023-11-21 15:51     ` Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 015/108] pwm: lpc18xx-sct: Make use of parent device pointer in driver data Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 016/108] pwm: lpss: Make use of pwmchip_parent() macro Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 017/108] pwm: mediatek: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 018/108] pwm: meson: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 019/108] pwm: mtk-disp: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 020/108] pwm: omap: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 021/108] pwm: pca9685: Store parent device in driver data Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 022/108] pwm: raspberrypi-poe: Make use of pwmchip_parent() macro Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 023/108] pwm: rcar: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 024/108] pwm: rz-mtu3: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 025/108] pwm: samsung: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 026/108] pwm: sifive: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 027/108] pwm: stm32-lp: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 028/108] pwm: stm32: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 029/108] pwm: stmpe: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 030/108] pwm: sun4i: " Uwe Kleine-König
2023-11-25 18:48   ` Jernej Škrabec
2023-11-21 13:49 ` [PATCH v3 031/108] pwm: tiecap: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 032/108] pwm: tiehrpwm: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 033/108] pwm: twl-led: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 034/108] pwm: twl: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 035/108] pwm: vt8500: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 036/108] staging: greybus: pwm: " Uwe Kleine-König
2023-11-23 12:55   ` Greg Kroah-Hartman
2023-11-21 13:49 ` [PATCH v3 037/108] pwm: Provide devm_pwmchip_alloc() function Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 038/108] pwm: ab8500: Make use of " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 039/108] pwm: apple: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 040/108] pwm: atmel-hlcdc: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 041/108] pwm: atmel: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 042/108] pwm: atmel-tcb: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 043/108] pwm: bcm2835: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 044/108] pwm: bcm-iproc: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 045/108] pwm: bcm-kona: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 046/108] pwm: berlin: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 047/108] pwm: brcmstb: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 048/108] pwm: clk: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 049/108] pwm: clps711x: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 050/108] pwm: crc: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 051/108] pwm: cros-ec: " Uwe Kleine-König
2023-11-22  8:52   ` Tzung-Bi Shih
2023-11-22 23:56     ` Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 052/108] pwm: dwc: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 053/108] pwm: ep93xx: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 054/108] pwm: fsl-ftm: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 055/108] pwm: hibvt: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 056/108] pwm: img: " Uwe Kleine-König
2023-11-21 13:49 ` [PATCH v3 057/108] pwm: imx1: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 058/108] pwm: imx27: " Uwe Kleine-König
2023-12-05 11:49   ` Philipp Zabel
2023-12-05 12:14     ` Uwe Kleine-König
2023-12-05 12:50       ` Philipp Zabel
2023-11-21 13:50 ` [PATCH v3 059/108] pwm: imx-tpm: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 060/108] pwm: intel-lgm: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 061/108] pwm: iqs620a: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 062/108] pwm: jz4740: " Uwe Kleine-König
2023-11-21 14:16   ` Paul Cercueil
2023-11-21 13:50 ` [PATCH v3 063/108] pwm: keembay: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 064/108] pwm: lp3943: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 065/108] pwm: lpc18xx-sct: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 066/108] pwm: lpc32xx: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 067/108] pwm: lpss-*: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 068/108] pwm: mediatek: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 069/108] pwm: meson: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 070/108] pwm: microchip-core: " Uwe Kleine-König
2023-11-22 11:14   ` Conor Dooley
2023-11-22 22:48     ` Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 071/108] pwm: mtk-disp: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 072/108] pwm: mxs: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 073/108] pwm: ntxec: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 074/108] pwm: omap-dmtimer: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 075/108] pwm: pca9685: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 076/108] pwm: pxa: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 077/108] pwm: raspberrypi-poe: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 078/108] pwm: rcar: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 079/108] pwm: renesas-tpu: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 080/108] pwm: rockchip: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 081/108] pwm: rz-mtu3: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 082/108] pwm: samsung: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 083/108] pwm: sifive: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 084/108] pwm: sl28cpld: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 085/108] pwm: spear: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 086/108] pwm: sprd: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 087/108] pwm: sti: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 088/108] pwm: stm32-lp: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 089/108] pwm: stm32: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 090/108] pwm: stmpe: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 091/108] pwm: sun4i: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 092/108] pwm: sunplus: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 093/108] pwm: tegra: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 094/108] pwm: tiecap: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 095/108] pwm: twl-led: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 096/108] pwm: twl: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 097/108] pwm: visconti: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 098/108] pwm: vt8500: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 099/108] pwm: xilinx: " Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 100/108] gpio: mvebu: " Uwe Kleine-König
2023-11-21 14:02   ` Bartosz Golaszewski
2023-11-21 16:11     ` Uwe Kleine-König
2023-11-22  9:05       ` Uwe Kleine-König
2023-11-22 10:36         ` Bartosz Golaszewski
2023-11-22 23:39           ` Uwe Kleine-König
2023-11-24 12:14           ` Thierry Reding
2023-11-24 21:16             ` Bartosz Golaszewski
2023-11-24 21:59               ` Uwe Kleine-König
2023-11-27 10:58               ` Uwe Kleine-König
2023-11-27 20:22                 ` Bartosz Golaszewski
2023-11-28  9:07                   ` Uwe Kleine-König
2023-12-01 10:14                     ` Bartosz Golaszewski
2023-12-02  0:43                       ` Uwe Kleine-König
2023-12-04 20:27                         ` Bartosz Golaszewski
2023-12-04 21:28                           ` Bartosz Golaszewski
2023-12-05  9:31                             ` Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 101/108] drm/bridge: ti-sn65dsi86: " Uwe Kleine-König
2023-11-21 15:15   ` Doug Anderson
2023-11-21 16:05     ` Uwe Kleine-König
2023-11-21 16:14       ` Doug Anderson
2023-11-23  9:17         ` Uwe Kleine-König
2023-11-27  9:32           ` Uwe Kleine-König
2023-11-23  9:46   ` Laurent Pinchart
2023-11-23 10:10     ` Uwe Kleine-König
2023-12-06 12:06       ` Laurent Pinchart
2023-12-06 14:23         ` Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 102/108] leds: qcom-lpg: " Uwe Kleine-König
2023-11-21 15:16   ` Lee Jones
2023-11-21 15:58     ` Uwe Kleine-König
2023-11-22 11:56   ` Lee Jones
2023-11-22 17:15     ` Thierry Reding
2023-11-23 10:44       ` Uwe Kleine-König
2023-11-24 12:27         ` Thierry Reding
2023-11-24 18:22           ` Uwe Kleine-König
2023-11-24 21:21             ` Bartosz Golaszewski
2023-11-22 17:54     ` Uwe Kleine-König [this message]
2023-11-23 10:21       ` Lee Jones
2023-11-23 10:54         ` Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 104/108] pwm: Ensure that pwm_chips are allocated using pwmchip_alloc() Uwe Kleine-König
2023-11-22 17:19   ` Thierry Reding
2023-11-22 23:52     ` Uwe Kleine-König
2023-11-24 12:28       ` Thierry Reding
2023-11-21 13:50 ` [PATCH v3 105/108] pwm: Ensure a struct pwm has the same lifetime as its pwm_chip Uwe Kleine-König
2023-11-21 15:53   ` Gustavo A. R. Silva
2023-11-21 13:50 ` [PATCH v3 106/108] pwm: Ensure the memory backing a PWM chip isn't freed while used Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 107/108] pwm: Add more locking Uwe Kleine-König
2023-11-21 13:50 ` [PATCH v3 108/108] WIP: pwm: Add support for pwmchip devices for faster and easier userspace access Uwe Kleine-König

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231122175413.5yxsdveausehkbgm@pengutronix.de \
    --to=u.kleine-koenig@pengutronix.de \
    --cc=conor.dooley@microchip.com \
    --cc=keescook@chromium.org \
    --cc=kernel@pengutronix.de \
    --cc=lee@kernel.org \
    --cc=linux-leds@vger.kernel.org \
    --cc=linux-pwm@vger.kernel.org \
    --cc=luca@z3ntu.xyz \
    --cc=pavel@ucw.cz \
    --cc=quic_amelende@quicinc.com \
    --cc=quic_bjorande@quicinc.com \
    --cc=robh@kernel.org \
    --cc=thierry.reding@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).