On Wed, Nov 30, 2011 at 04:07, Tero Kristo <t-kristo@xxxxxx> wrote:
> On Tue, 2011-11-29 at 12:26 -0600, Menon, Nishanth wrote:
>> On Fri, Nov 25, 2011 at 09:49, Tero Kristo <t-kristo@xxxxxx> wrote:
<more snip>
>> > diff --git a/arch/arm/mach-omap2/opp3xxx_data.c b/arch/arm/mach-omap2/opp3xxx_data.c
>> > index d95f3f9..1d44df5 100644
>> > --- a/arch/arm/mach-omap2/opp3xxx_data.c
>> > +++ b/arch/arm/mach-omap2/opp3xxx_data.c
>> >
>> > /* 34xx */
>> > +#define OMAP3_ON_VOLTAGE_UV 1200000
>> > +#define OMAP3_ONLP_VOLTAGE_UV 1000000
>> > +#define OMAP3_RET_VOLTAGE_UV 975000
>> > +#define OMAP3_OFF_VOLTAGE_UV 600000
>>
>> this approach has a problem -> ON, ONLP and RET voltage should consider:
Make that ON, ONLP, RET, OFF voltages! Sigh..
>> a) OMAP capabiltiy as above.
>> b) PMIC capability which is being removed in this patch
>>
>> the framework should use the combination of both to make a decision.
>
> So, I think we should limit these voltages based on the PMIC vddmin /
> vddmax values, right? I don't think PMIC has any other limitations, and
Ideally speaking - this is what we want:
PMIC says - on this rail - I can give max x V and min y V. and when
you(kernel) gets control, expect Voltage = ON.
OMAP requirements - which ever wierd ones they might be - will be very
specific to OMAP variants.
some factors such as timing closures also come into play -> ON, ONLP,
RET and OFF are OMAP reqs.
Now, we can have combinations like TPS+TWL+OMAP4430 (yep - those do
exist) or something like custom PMIC+OMAP4460
or many other combinations..
You could have additional complications as well - e.g. PMIC OFF path
is definitely our favourite.
writing 0x0 to TWL causes 0V and a 1 is 709..mV or 607..mV, 0x0 on TPS is 500mV!
> we shouldn't define the voltage levels for all of these modes for PMIC.
> PMIC limits should also probably be changed from current values (based
> on OMAP defines) and changed to actual values, like vddmin = 600mV
> (vsel=0), or whatever is the minimum voltage for the corresponding PMIC.
my point being: framework cannot expect any assumption about the PMIC
and the person writing the support for a new PMIC should be completely
ignorant for which OMAP he/she is writing for.
>> > +};
>> > +
>> > #define OMAP4430_VDD_CORE_OPP50_UV 1025000
>> > #define OMAP4430_VDD_CORE_OPP100_UV 1200000
>> >
>> > @@ -64,6 +93,17 @@ struct omap_volt_data omap44xx_vdd_core_volt_data[] = {
>> > VOLT_DATA_DEFINE(0, 0, 0, 0),
>> > };
>> >
>> > +struct omap_vp_param omap44xx_core_vp_data = {
>> > + .vddmin = OMAP4_VP_CORE_VLIMITTO_VDDMIN,
>> > + .vddmax = OMAP4_VP_CORE_VLIMITTO_VDDMAX,
>> > +};
>> > +
>> > +struct omap_vc_param omap44xx_core_vc_data = {
>> > + .on = OMAP4_ON_VOLTAGE_UV,
>> > + .onlp = OMAP4_ONLP_VOLTAGE_UV,
>> > + .ret = OMAP4_RET_VOLTAGE_UV,
>> > + .off = OMAP4_OFF_VOLTAGE_UV,
>> > +};
>>
>> NOTE: we will be reaching all combinations ahead - in time to come
>> ahead we will have 4470 as well - linking this to opp data seems wrong
>> to me..
>
> They are not really that much linked to opp data, they are just defined
> in this file. The actual attach to voltdm is done in
> voltagedomainsxxxx_data.c file, where we can link data to whatever
> voltagedomain we want to. See for example voltagedomains3xxx_data.c what
> is done for omap34xx vs. omap36xx. Should we move this data over there
> then...?
I think it belongs to VP/VC data and not in OPP file (which was meant
for OPPs in the first place).
we might want to think about replacing these with device tree data
someday - OPP data makes it
kinda hard to distinguish IMHO when we do that.
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
