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: <e80f4af9-e333-1c6e-7b3a-849186435ab0@redhat.com>
Date:	Wed, 27 Apr 2016 17:35:31 +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:12, Mark Brown wrote:
> On Wed, Apr 27, 2016 at 03:59:28PM +0200, Hans de Goede wrote:
>> The maximum supported voltage for ldo_io# is 3.3V, but on cold
>> boot the selector comes up at 0x1f, which maps to 3.8V.
>
> Why not just implement that?

I guess I was not clear in my commit msg, when I wrote:
"which maps to 3.8V", what I mean is 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.

Note the datasheet does not explicitly mention this being
out of spec. But a reset value of 0x1f has been observed,
and putting that in the formula for getting the voltage
leads to an out of spec value of 3.8V

> 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 causes things to fail when we do actually want to use the
regulator and on registering it try to apply constraints when
registering:

     [    1.467788] vcc-touchscreen: failed to get the current voltage(-22)
     [    1.474209] axp20x-regulator axp20x-regulator: Failed to register ldo_io1
     [    1.483363] axp20x-regulator: probe of axp20x-regulator failed with error -22

Are you suggesting that we simply make n_voltages 0x1f / 31 and rely
on dts constraints to never use the out of spec 3.4 - 3.8 volt
settings ? That will fix things, but it feels wrong.

Thinking more about this, doing this will result in the exact same behavior
(program the ldo to 3.3V when it is still at its power-on-reset value when
  registering) but only when the regulator is used, which means not touching
it unless explicitly told it's ok.

So I guess that this is how you want us to fix this ?

Regards,

Hans