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