linux-kernel - Re: [v13 2/2] pwm: Add Aspeed ast2600 PWM support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 01 Dec 2021 10:11:42 +0100
From:   Philipp Zabel <p.zabel@...gutronix.de>
To:     Billy Tsai <billy_tsai@...eedtech.com>,
        "jdelvare@...e.com" <jdelvare@...e.com>,
        "linux@...ck-us.net" <linux@...ck-us.net>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "joel@....id.au" <joel@....id.au>,
        "andrew@...id.au" <andrew@...id.au>,
        "lee.jones@...aro.org" <lee.jones@...aro.org>,
        "thierry.reding@...il.com" <thierry.reding@...il.com>,
        "u.kleine-koenig@...gutronix.de" <u.kleine-koenig@...gutronix.de>,
        "linux-hwmon@...r.kernel.org" <linux-hwmon@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-aspeed@...ts.ozlabs.org" <linux-aspeed@...ts.ozlabs.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-pwm@...r.kernel.org" <linux-pwm@...r.kernel.org>
Cc:     BMC-SW <BMC-SW@...eedtech.com>
Subject: Re: [v13 2/2] pwm: Add Aspeed ast2600 PWM support

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" <p.zabel@...gutronix.de> 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