linux-kernel - Re: [PATCH v2 3/3] regulator: axp20x: Fix axp22x ldo

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: <e8fb2bd1-b08e-177d-16f6-53bb47682049@redhat.com>
Date:	Wed, 27 Apr 2016 18:04:53 +0200
From:	Hans de Goede <hdegoede@...hat.com>
To:	Mark Brown <broonie@...nel.org>
Cc:	Liam Girdwood <lgirdwood@...il.com>,
	Maxime Ripard <maxime.ripard@...e-electrons.com>,
	Chen-Yu Tsai <wens@...e.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 3/3] regulator: axp20x: Fix axp22x ldo_io registration
 error on cold boot

Hi,

On 27-04-16 17:48, Mark Brown wrote:
> On Wed, Apr 27, 2016 at 05:35:31PM +0200, Hans de Goede wrote:
>> On 27-04-16 17:12, Mark Brown wrote:
>
>>> Why not just implement that?
>
>> Given the formula in the datasheet to calculate the ldo_io
>> regulator voltage 0x1f maps to 3.8V, but according to the
>> datasheet the maximum voltage supported is 3.3V, iow the
>> power-on-reset value of this register is out of spec
>> according to the datasheet.
>
> Well, I guess someone can just measure what happens?

What happens will likely depend on the pmic input voltage,
which can be either 5V from a charger / usb or can be approx
3.8V from a lion or lipo battery. All linear regulators in
the axp20x / axp22x pmic are listed as having a max output
voltage of 3.3V, this likely has to do with the minimum
voltage drop compared to the input value.

So in some conditions the output voltage at a 0x1f register
value may very well be different then at others. IMHO we
should just avoid any out of spec. values.

The 2 ldo-s we're talking about now are special in 2 ways:

1) They are the only ones to have an out of spec p-o-r value
2) They are the only ones which do not have a dedicated pin,
    they are muxed to the outside sharing pins with 2 gpio
    pins, with the muxing defaulting to gpio-input, which makes
    1) ok(-ish) I guess

I believe that we really need to write an in-spec value to the
register controlling the voltage, before enabling this regulator
(which is done by selecting the mux to connect it to the pin).

Since you do not like this patch, I believe that the best way
to do this instead is to make n_voltages span the whole
range, have get_voltage return 3.8 for 0x1f and limit things
using constraints so that if the register contains 0x1b - 0x1f
we will call set_voltage to a supported value when applying the
constraints.

>>> We know what it does and it preserves the
>>> expected behaviour where we don't touch the regualtor unless explicitly
>>> told it's OK.  We'll only ever try to set that value if the machine
>>> explicitly gives permission for it.
>
>> The problem is that if we do not fix the out of spec
>> register value then _regulator_get_voltage returns
>> -EINVAL because the register value exceeds n_voltages
>
> This will no longer be the case when the driver understands what the
> startup value means.

Ack, which is what I'm suggesting by suggesting to increase
n_voltages.

Regards,

Hans