[PATCH v6 2/4] pwm: driver for qualcomm ipq6018 pwm block

[Baruch Siach]

Hi Bjorn,

On Sun, Jul 25 2021, Bjorn Andersson wrote:
> On Thu 22 Jul 05:01 CDT 2021, Baruch Siach wrote:
>> +	if (IS_ERR(pwm->clk))
>> +		return dev_err_probe(dev, PTR_ERR(pwm->clk),
>> +				"failed to get core clock");
>> +
>> +	ret = clk_prepare_enable(pwm->clk);
>
> Not sure if Uwe asked you this already, but do you need to clock the
> supply even when the PWM isn't enabled?

I guess not. However, tracking clock enable/disable per PWM signal
complicates the code. We'd need to balance enables with matching
disables in the .remove callback. I'd prefer to leave that as room for
future optimization.

>> +	if (ret)
>> +		return dev_err_probe(dev, ret, "clock enable failed");
>> +
>> +	pwm->chip.dev = dev;
>> +	pwm->chip.ops = &ipq_pwm_ops;
>> +	pwm->chip.npwm = 4;
>> +
>> +	ret = pwmchip_add(&pwm->chip);
>
> Depending on above answer you may or may not have the need to ensure the
> ordering of clk_disable_unprepare() in the remove function.

According to Uwe pwmchip_remove() must precede clock disable:

  https://lore.kernel.org/linux-arm-msm/20210714201839.kfyqcyvowekc4ejs@pengutronix.de/

How is that related to per PWM signal clock handling?

> Otherwise devm_pwmchip_add() would be nice here.

Thanks,
baruch

-- 
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -