Hi Billy,

On Wed, 2021-12-01 at 03:30 +0000, Billy Tsai wrote:

Hi Philipp,

On 2021/11/30, 5:52 PM, "Philipp Zabel" [off-list ref] wrote:

    On Mon, 2021-11-29 at 14:43 +0800, Billy Tsai wrote:
    [...]
    >   > +	ret = clk_prepare_enable(priv->clk);
    >   > +	if (ret)
    >   > +		return dev_err_probe(dev, ret, "Couldn't enable clock\n");
    >   > +
    >   > +	ret = reset_control_deassert(priv->reset);
    >   > +	if (ret) {
    >   > +		dev_err_probe(dev, ret, "Couldn't deassert reset control\n");
    >   > +		goto err_disable_clk;
    >   > +	}

    >   Is there any reason to keep the clocks running and the controller out of
    >   reset while the PWM outputs are disabled?

Can you tell me about your concerns with this process?

No particular concerns, just curiosity.

In my opinion, they are used to provide the clock and de-assert the reset of the PWM engine. If we didn't release
them in probe stage the CPU can't and shouldn't read the register of the PWM engine when call the get_state.
Assume that we want to adjust them dynamically, the driver needs to add more conditions to check and keep the status
of each PWM channel, which is not a good deal for the server platform.

Thanks. I don't know the hardware, so I have no idea whether disabling
the clocks would even save a measurable (let alone appreciable) amount
of power.

I've just seen other PWM drivers use runtime PM or enable/disable clocks
dynamically, and wondered why this one doesn't.

regards
Philipp

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