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

From: Baruch Siach <baruch@tkos.co.il>
To: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Thierry Reding <thierry.reding@gmail.com>,
	Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>,
	Lee Jones <lee.jones@linaro.org>, Andy Gross <agross@kernel.org>,
	Balaji Prakash J <bjagadee@codeaurora.org>,
	Rob Herring <robh+dt@kernel.org>,
	Robert Marko <robert.marko@sartura.hr>,
	Kathiravan T <kathirav@codeaurora.org>,
	linux-pwm@vger.kernel.org, devicetree@vger.kernel.org,
	linux-arm-msm@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v6 2/4] pwm: driver for qualcomm ipq6018 pwm block
Date: Mon, 26 Jul 2021 07:31:55 +0300	[thread overview]
Message-ID: <87czr5wv9g.fsf@tarshish> (raw)
In-Reply-To: <YP2u9Hz2/+av6JLG@yoga>

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 -

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel