linux-kernel - Re: [PATCH] regulator: core: Skip balancing of the enabled regulators in regulator

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]

Message-ID: <86eb668d-7bcf-798b-dabb-95071d16cbb6@samsung.com>
Date:   Tue, 8 Oct 2019 18:02:08 +0200
From:   Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>
To:     Mark Brown <broonie@...nel.org>
Cc:     Marek Szyprowski <m.szyprowski@...sung.com>,
        linux-kernel@...r.kernel.org, Dmitry Osipenko <digetx@...il.com>,
        Liam Girdwood <lgirdwood@...il.com>,
        Lucas Stach <l.stach@...gutronix.de>,
        Krzysztof Kozlowski <krzk@...nel.org>,
        linux-samsung-soc@...r.kernel.org,
        Viresh Kumar <viresh.kumar@...aro.org>,
        Kamil Konieczny <k.konieczny@...sung.com>
Subject: Re: [PATCH] regulator: core: Skip balancing of the enabled
 regulators in regulator_enable()


On 10/8/19 5:48 PM, Mark Brown wrote:
> On Tue, Oct 08, 2019 at 03:24:17PM +0200, Bartlomiej Zolnierkiewicz wrote:
> 
>> Commit 7f93ff73f7c8 ("opp: core: add regulators enable and disable")
>> currently can be safely reverted as all affected users use always-on
>> regulators. However IMHO it should be possible to enable always-on
>> regulator without side-effects.
> 
> With coupled regulators you might have something kicking in because a
> change was made on a completely different regulator...  If we don't take
> account of coupling requirements we'd doubtless have issues with that at
> some point.

OK, I have not considered this.

>> When it comes to setting regulator constraints before doing enable
>> operation, it also seems to be possible solution but would require
>> splitting regulator_set_voltage() operation on two functions:
> 
>> - one for setting constraints (before regulator_enable() operation)
> 
>> - the other one actually setting voltage (after enable operation)
> 
> I don't follow?  What would a "constraint" be in this context and how
> would it be different to the voltage range you'd set in normal operation?

The constraint here would be just the voltage range. I just wanted to
point out that the actual voltage set operation (on the hardware itself
not the internal subsystem bookkeeping) shouldn't be done before enable
operation (especially in context of non-coupled regulators).

Taking into account your remark about enable operation on coupled
regulators and Dmitry's mail about cpufreq issue I think now that just
dropping opp change is the most straightforward fix.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics