linux-kernel - Re: [PATCH 1/1] regulator: max77802: set opmode to normal if off is read from hw

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: <CABxcv=k2PH-BksYyWirAoL=DiVuy+O=Y1c+pMO=EK58XNKMsFw@mail.gmail.com>
Date:	Thu, 28 Aug 2014 00:44:18 +0200
From:	Javier Martinez Canillas <javier@...hile0.org>
To:	Tomasz Figa <tomasz.figa@...il.com>
Cc:	Mark Brown <broonie@...nel.org>,
	Javier Martinez Canillas <javier.martinez@...labora.co.uk>,
	Doug Anderson <dianders@...omium.org>,
	Olof Johansson <olof@...om.net>,
	Yuvaraj Kumar C D <yuvaraj.cd@...il.com>,
	"linux-samsung-soc@...r.kernel.org" 
	<linux-samsung-soc@...r.kernel.org>,
	Linux Kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/1] regulator: max77802: set opmode to normal if off is
 read from hw

Hello Tomasz and Mark,

Sorry for not answering before but I'm on vacations until Sep, 5 and I
have limited access to my email.

On Wed, Aug 27, 2014 at 11:03 PM, Tomasz Figa <tomasz.figa@...il.com> wrote:
>>>> From what I know based on my experience with Samsung boards we used, the
>>>> opmodes of regulators are preconfigured by board bootloader to certain
>>>> values based on power design of the board (i.e. there is no need to keep
>>>> a regulator in full power mode, if on given board only a little fraction
>>>> of it is needed).
>>>

This is the case for Chromebooks as well but the solution implemented
in the downstream Chrome OS 3.8 kernel is what Tomasz suggested
before, the max77802/686 PMIC regulator driver has a DT binding that
allows to define the initial opmode for each regulator:

Required properties for each regulator:
- regulator-op-mode : Regulator operating mode, the meaning is regulator-
     dependent. Valid values are 0-3.

>>> Well, presumably the bootloader is going to run again even for a warm
>>> reboot?
>>
>> This is strange, but apparently it's not the case for the hardware which
>> this patch is supposed to fix or at least this is how I understood it.
>>
>
> OK, I just realized that Javier's problem might be caused by the fact
> that the bootloader he use doesn't change the regulators from defaults
> at all. In this case this patch is just fine.
>

Yes, AFAIK the bootloader (none of them because Chromebooks use two
chained U-boots) change the regulators default opmode so once is set
to OFF on .disable, that value is preserved on warm reboot. This made
sense with the Chrome OS kernel since the kernel always set the opmode
defined in the "regulator-op-mode" DT property and did not relied on
the bootloader to set the most efficient default opmode.

As Tomasz said this issue also affects other PMICs, for example a
similar DT binding was proposed [0] for the max77686 PMIC which is
very similar to the max77802. Doug even suggested in that thread a
more generic DT binding that could be part of the regulator core.

At the end that DT binding was not merged and the max77686 driver just
sets the default operating mode for all regulators to NORMAL and does
not even read the value reported by the hardware registers. So in that
regard the max77802 driver (even after this patch) does better since
at least respects the default opmode read from the hardware register
if is not OFF.

Also, other drivers have customs operating mode DT properties, please
take a look at Documentation/devicetree/bindings/regulator/s5m8767-regulator.txt:

The above regulator entries are defined in regulator bindings documentation
except these properties:
        - op_mode: describes the different operating modes of the LDO's with
                power mode change in SOC. The different possible values are,
                0 - always off mode
                1 - on in normal mode
                2 - low power mode
                3 - suspend mode

This is not a great binding IMHO since not only uses a quite generic
"op_mode" for property that is driver specific ("s5m8767,op-mode" or
something would had been better) but also this seems to be a general
issue for many platforms so if we don't try to solve this with a
generic approach, each driver author will try to solve it in its own
way.

> Best regards,
> Tomasz
> --

Best regards,
Javier

[0]: https://patchwork.kernel.org/patch/1855331/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/