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 for Android: free password hash cracker in your pocket

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

Message-ID: <A49ACA24-D607-443F-8C96-4E2BEF902F8E@aspeedtech.com>
Date:   Wed, 1 Dec 2021 10:53:48 +0000
From:   Billy Tsai <billy_tsai@...eedtech.com>
To:     Philipp Zabel <p.zabel@...gutronix.de>,
        "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 Philipp,

On 2021/12/1, 5:12 PM, "Philipp Zabel" <p.zabel@...gutronix.de> wrote:

    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.

Thanks for the heads up, I think that for the usage we don't need to implement that.

Best Regards,
Billy Tsai