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

[Xianwei Zhao <xianwei.zhao@amlogic.com>]

Hi Uwe,
    Thanks for your review.

On 2026/9/8 02:44, Uwe Kleine-König wrote:
> 
> 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@amlogic.com>
>>>
>>> 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!
> 

Is there anything else I need to do before this patch can be merged?

> Best regards
> Uwe