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

Baruch Siach
2021-07-26T04:31:59Z

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@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -