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@xxxxxxxxxxxxxx>
> > ---
> > 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:
signature.asc
Description: PGP signature
