| Message ID | 20231121134901.208535-15-u.kleine-koenig@pengutronix.de (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | pwm: Fix lifetime issues for pwm_chips | expand |
Hi Uwe, Le mardi 21 novembre 2023 à 14:49 +0100, Uwe Kleine-König a écrit : > 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-jz4740.c | 17 ++++++++--------- > 1 file changed, 8 insertions(+), 9 deletions(-) > > diff --git a/drivers/pwm/pwm-jz4740.c b/drivers/pwm/pwm-jz4740.c > index e9375de60ad6..555c2db3968d 100644 > --- a/drivers/pwm/pwm-jz4740.c > +++ b/drivers/pwm/pwm-jz4740.c > @@ -35,13 +35,12 @@ static inline struct jz4740_pwm_chip > *to_jz4740(struct pwm_chip *chip) > return container_of(chip, struct jz4740_pwm_chip, chip); > } > > -static bool jz4740_pwm_can_use_chn(struct jz4740_pwm_chip *jz, > - unsigned int channel) > +static bool jz4740_pwm_can_use_chn(struct pwm_chip *chip, unsigned > int channel) > { > /* Enable all TCU channels for PWM use by default except > channels 0/1 */ > - u32 pwm_channels_mask = GENMASK(jz->chip.npwm - 1, 2); > + u32 pwm_channels_mask = GENMASK(chip->npwm - 1, 2); > > - device_property_read_u32(jz->chip.dev->parent, > + device_property_read_u32(pwmchip_parent(chip)->parent, > "ingenic,pwm-channels-mask", > &pwm_channels_mask); You could have used pwmchip_parent(&jz->chip) and not change the prototype. But the patch is tiny enough that I don't really care, so: Acked-by: Paul Cercueil <paul@crapouillou.net> > > @@ -55,14 +54,14 @@ static int jz4740_pwm_request(struct pwm_chip > *chip, struct pwm_device *pwm) > char name[16]; > int err; > > - if (!jz4740_pwm_can_use_chn(jz, pwm->hwpwm)) > + if (!jz4740_pwm_can_use_chn(chip, pwm->hwpwm)) > return -EBUSY; > > snprintf(name, sizeof(name), "timer%u", pwm->hwpwm); > > - clk = clk_get(chip->dev, name); > + clk = clk_get(pwmchip_parent(chip), name); > if (IS_ERR(clk)) > - return dev_err_probe(chip->dev, PTR_ERR(clk), > + return dev_err_probe(pwmchip_parent(chip), > PTR_ERR(clk), > "Failed to get clock\n"); > > err = clk_prepare_enable(clk); > @@ -149,7 +148,7 @@ static int jz4740_pwm_apply(struct pwm_chip > *chip, struct pwm_device *pwm, > */ > rate = clk_round_rate(clk, tmp); > if (rate < 0) { > - dev_err(chip->dev, "Unable to round rate: %ld", > rate); > + dev_err(pwmchip_parent(chip), "Unable to round rate: > %ld", rate); While you're at it - and if you need a v4 - maybe sneak in a \n there? > return rate; > } > > @@ -170,7 +169,7 @@ static int jz4740_pwm_apply(struct pwm_chip > *chip, struct pwm_device *pwm, > > err = clk_set_rate(clk, rate); > if (err) { > - dev_err(chip->dev, "Unable to set rate: %d", err); > + dev_err(pwmchip_parent(chip), "Unable to set rate: > %d", err); And there. > return err; > } > Cheers, -Paul
Hello Paul, On Tue, Nov 21, 2023 at 03:13:58PM +0100, Paul Cercueil wrote: > Le mardi 21 novembre 2023 à 14:49 +0100, Uwe Kleine-König a écrit : > > 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-jz4740.c | 17 ++++++++--------- > > 1 file changed, 8 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/pwm/pwm-jz4740.c b/drivers/pwm/pwm-jz4740.c > > index e9375de60ad6..555c2db3968d 100644 > > --- a/drivers/pwm/pwm-jz4740.c > > +++ b/drivers/pwm/pwm-jz4740.c > > @@ -35,13 +35,12 @@ static inline struct jz4740_pwm_chip > > *to_jz4740(struct pwm_chip *chip) > > return container_of(chip, struct jz4740_pwm_chip, chip); > > } > > > > -static bool jz4740_pwm_can_use_chn(struct jz4740_pwm_chip *jz, > > - unsigned int channel) > > +static bool jz4740_pwm_can_use_chn(struct pwm_chip *chip, unsigned > > int channel) > > { > > /* Enable all TCU channels for PWM use by default except > > channels 0/1 */ > > - u32 pwm_channels_mask = GENMASK(jz->chip.npwm - 1, 2); > > + u32 pwm_channels_mask = GENMASK(chip->npwm - 1, 2); > > > > - device_property_read_u32(jz->chip.dev->parent, > > + device_property_read_u32(pwmchip_parent(chip)->parent, > > "ingenic,pwm-channels-mask", > > &pwm_channels_mask); > > You could have used pwmchip_parent(&jz->chip) and not change the > prototype. Later in the series jz->chip goes away. So following your advice only makes me touch this code once more later. > > @@ -149,7 +148,7 @@ static int jz4740_pwm_apply(struct pwm_chip > > *chip, struct pwm_device *pwm, > > */ > > rate = clk_round_rate(clk, tmp); > > if (rate < 0) { > > - dev_err(chip->dev, "Unable to round rate: %ld", > > rate); > > + dev_err(pwmchip_parent(chip), "Unable to round rate: > > %ld", rate); > > While you're at it - and if you need a v4 - maybe sneak in a \n there? I'll try to remember :-) Best regards Uwe
diff --git a/drivers/pwm/pwm-jz4740.c b/drivers/pwm/pwm-jz4740.c index e9375de60ad6..555c2db3968d 100644 --- a/drivers/pwm/pwm-jz4740.c +++ b/drivers/pwm/pwm-jz4740.c @@ -35,13 +35,12 @@ static inline struct jz4740_pwm_chip *to_jz4740(struct pwm_chip *chip) return container_of(chip, struct jz4740_pwm_chip, chip); } -static bool jz4740_pwm_can_use_chn(struct jz4740_pwm_chip *jz, - unsigned int channel) +static bool jz4740_pwm_can_use_chn(struct pwm_chip *chip, unsigned int channel) { /* Enable all TCU channels for PWM use by default except channels 0/1 */ - u32 pwm_channels_mask = GENMASK(jz->chip.npwm - 1, 2); + u32 pwm_channels_mask = GENMASK(chip->npwm - 1, 2); - device_property_read_u32(jz->chip.dev->parent, + device_property_read_u32(pwmchip_parent(chip)->parent, "ingenic,pwm-channels-mask", &pwm_channels_mask); @@ -55,14 +54,14 @@ static int jz4740_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm) char name[16]; int err; - if (!jz4740_pwm_can_use_chn(jz, pwm->hwpwm)) + if (!jz4740_pwm_can_use_chn(chip, pwm->hwpwm)) return -EBUSY; snprintf(name, sizeof(name), "timer%u", pwm->hwpwm); - clk = clk_get(chip->dev, name); + clk = clk_get(pwmchip_parent(chip), name); if (IS_ERR(clk)) - return dev_err_probe(chip->dev, PTR_ERR(clk), + return dev_err_probe(pwmchip_parent(chip), PTR_ERR(clk), "Failed to get clock\n"); err = clk_prepare_enable(clk); @@ -149,7 +148,7 @@ static int jz4740_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, */ rate = clk_round_rate(clk, tmp); if (rate < 0) { - dev_err(chip->dev, "Unable to round rate: %ld", rate); + dev_err(pwmchip_parent(chip), "Unable to round rate: %ld", rate); return rate; } @@ -170,7 +169,7 @@ static int jz4740_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, err = clk_set_rate(clk, rate); if (err) { - dev_err(chip->dev, "Unable to set rate: %d", err); + dev_err(pwmchip_parent(chip), "Unable to set rate: %d", err); return err; }
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-jz4740.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-)