linux-kernel - Re: [PATCH V2] regulator: 88pm800: add regulator driver for 88pm800

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: <20130624101454.GG27646@sirena.org.uk>
Date:	Mon, 24 Jun 2013 11:14:54 +0100
From:	Mark Brown <broonie@...nel.org>
To:	Chao Xie <xiechao.mail@...il.com>
Cc:	Chao Xie <chao.xie@...vell.com>, lgirdwood@...il.com,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	yizhang@...vell.com
Subject: Re: [PATCH V2] regulator: 88pm800: add regulator driver for 88pm800

On Mon, Jun 24, 2013 at 10:01:39AM +0800, Chao Xie wrote:
> On Fri, Jun 21, 2013 at 11:24 PM, Mark Brown <broonie@...nel.org> wrote:

> > Just provide get_voltage_sel(), the core will do the mapping to voltages
> > using list_voltage().

> I am a little confused.
> The BUCK voltage is not linear, and it contains a lot of voltages.
> If we use map_voltage_ascend, it will looking a a suitable voltage
> from begin to end one by one.

What does this have to do with get_voltage()?  And note that you can
write your own mapping function for a reason...

> If we directly make use of set_voltage and get_voltage, we can
> directly calculates the voltage which is
> suitable, and do not go through all the voltags.
> for example, now BUCK voltage table is
> range 1 from 600000 to 1587500, each step is 12500
> range 2 from 1600000 to 1800000, each step is 50000

No, this is nothing at all to do with using the selector versions of the
API.  Think about what the API is doing and take a look at the code.

> >> +     } else if (pdata->num_regulators) {
> >> +             /* Check whether num_regulator is valid. */
> >> +             unsigned int count = 0;
> >> +             for (i = 0; pdata->regulators[i]; i++)
> >> +                     count++;
> >> +             if (count != pdata->num_regulators)
> >> +                     return -EINVAL;

> > This looks...  odd.

> It is just make sure that pdata has correct number of regulators.
> It you think that it is redundant, i can remove it.

If you really need to have platform data for all the regulators then
just embed the array inside the platform data so there's no possibility
of any confusion.

> > With deferred probing you should just be able to use
> > module_platform_driver().

> The regulator controlles some BUCK regulators.
> These regulators may be used by application CPU or CP(communication
> CPU) for telephony.
> The CP may need different voltages if it goes deep initialization. if
> we defer the setting later, it is too late for
> CP initialization, and will impact the performance.

If your kernel startup is taking long enough for this to be an issue it
seems like there's much bigger problems here and things are going to be
very fragile anyway, it's going to be better to figure out what the root
issue is.

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)