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

[Chao Xie]

On Mon, Jun 24, 2013 at 6:14 PM, Mark Brown <broonie@xxxxxxxxxx> wrote:
> 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@xxxxxxxxxx> 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.
>
OK. I will try to define my own mapping function and use the sel APIs.

>> >> +     } 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.
>
Some regulators will not be exported to kernel depending on boards which are
for special usage. So i do not use a array inside the platform data.

>> > 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.
I see. I will use module_platform_driver
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/