Re: [PATCH v4 3/4] pwm: meson: use bulk API for per-channel clocks

[Uwe Kleine-König]

Hello,

On Mon, Sep 07, 2026 at 05:22:44PM +0200, Alexandre Mergnat wrote:
> > The S4 PWM driver currently gets each channel clock individually with
> > of_clk_get() and uses a custom cleanup action to release the clocks.
> > 
> > Use devm_clk_bulk_get_all() instead to retrieve all per-channel clocks
> > at once. Verify that the number of clocks matches the number of PWM
> > channels and assign each clock to its corresponding channel.
> > 
> > This also allows the per-channel clock initialization code to be shared
> > by Meson PWM variants using one clock per channel.
> > 
> > Signed-off-by: Xianwei Zhao <xianwei.zhao@xxxxxxxxxxx>
> >
> > diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
> > index 22cefc6d5dd0..6f151464d7cc 100644
> > --- a/drivers/pwm/pwm-meson.c
> > +++ b/drivers/pwm/pwm-meson.c
> > @@ -497,33 +497,25 @@ static int meson_pwm_init_channels_meson8b_v2(struct pwm_chip *chip)
> >  	return meson_pwm_init_clocks_meson8b(chip, mux_parent_data);
> >  }
> >  
> > -static void meson_pwm_s4_put_clk(void *data)
> > -{
> > -	struct clk *clk = data;
> > -
> > -	clk_put(clk);
> > -}
> > -
> > -static int meson_pwm_init_channels_s4(struct pwm_chip *chip)
> > +static int meson_pwm_init_channels_per_channel_clk(struct pwm_chip *chip)
> >  {
> >  	struct device *dev = pwmchip_parent(chip);
> > -	struct device_node *np = dev->of_node;
> >  	struct meson_pwm *meson = to_meson_pwm(chip);
> > -	int i, ret;
> > +	struct clk_bulk_data *clks;
> > +	unsigned int i;
> > +	int num;
> >  
> > -	for (i = 0; i < chip->npwm; i++) {
> > -		meson->channels[i].clk = of_clk_get(np, i);
> > -		if (IS_ERR(meson->channels[i].clk))
> > -			return dev_err_probe(dev,
> > -					     PTR_ERR(meson->channels[i].clk),
> > -					     "Failed to get clk\n");
> > -
> > -		ret = devm_add_action_or_reset(dev, meson_pwm_s4_put_clk,
> > -					       meson->channels[i].clk);
> > -		if (ret)
> > -			return dev_err_probe(dev, ret,
> > -					     "Failed to add clk_put action\n");
> > -	}
> > +	num = devm_clk_bulk_get_all(dev, &clks);
> > +	if (num < 0)
> > +		return dev_err_probe(dev, num, "Failed to get clocks\n");
> > +
> > +	if (num != chip->npwm)
> > +		return dev_err_probe(dev, -EINVAL,
> > +				"expected %u clocks, got %d\n",
> 
> Small style nit: the continuation arguments are not aligned with the open
> parenthesis, whereas the dev_err_probe() just above and the rest of the driver
> do align them. Can you fix it please ?

No need to resend for that, I can fix that up while applying (after
checking I don't have further concerns).

Thanks Alex for your review!

Best regards
Uwe

Attachment: signature.asc
Description: PGP signature