Premi, Sanjeev wrote, on 01/08/2011 06:29 AM:
[..]
[1] SPRUFF1D-June 2009 section 1.5.2.1.1
Where is this doc?
[..]
[sp] Thanks for pointing out. It is not accessible for some reasons.
It can be... and is being looked into as i write this message.
thanks.
[snip]...[snip]
+#define SWCALC_OPP6_DELTA_NNT 379
+#define SWCALC_OPP6_DELTA_PNT 227
+#define GAIN_MAXLIMIT 16
Magic numbers - do they scale from 3430 to 3630?
[sp] The patch is meant for 720MHz - which is applicable for 3430 only.
So, these values are meant for 3430.
Right - and since sr_device is common for omap3, omap4 etc.. does'nt it
make sense to isolate code properly out?
[snip]...[snip]
+
+static u32 swcalc_opp6_nvalue(u32 opp5_nvalue)
so what does this actually do?
[sp] Returns a nvalue which corresponds to opp6 based on input which
is nvalue of opp5. Elementary?
yes Sherlock :). I apologize that I was'nt clear. But I was wondering
about the following:
what does the algo actually do - does it add a specific voltage delta or
some %v to previous nTarget?
It takes an input and does some magic - what does that magic algo do to
produce output?
it is possible that it is self evident to you, but it is not to me -
probably because I dont have the mentioned doc to refer to.
what is the overall objective of this code?
we have start nTarget from OPP5 and we are trying to put in a new one
for OPP6 out of the previous one.
things that pop in my mind are:
a) if this was the case, I just need OPP1 value, I could generate values
for all other OPPs.
b) if the algo itself was generic enough, we could use it in OMAP3430,
3630, OMAP4 etc.. - as far as I have confirmed from h/w team in the last
couple of years multiple times, ntarget generation algo for OMAP34,36,4x
series is not algo based, but production floor operation based (making
it unique and not really an algo based mechanism of nTarget programming).
+{
[snip]...[snip]
+ opp6_nvalue = (opp6_senPgain<< 0x14) |
(opp6_senNgain< 0x10) |
+ (opp6_senPRN<< 0x8) | opp6_senNRN;
+
+ return opp6_nvalue;
[sp] Elementary, if you preferred to read the code. Rather that glossing
over in hurry to respond compulsively.
I had hoped you could help my time a little (I do other things other
than review as well ;)) by documenting the algo in the function header?
+}
[snip]...[snip]
Sorry, Dumb question:
a) You are using OPP5's nTarget to use in OPP6's nTarget?
is'nt it fused
in for OPP6 offset?
[sp] Again, in hurry to respond, you missed the description for the patch
[quote]
The eFuse registers do not contain the nValue to be used
with 720MHz (OPP6). This patch implements procedure to
calculate the nValue(OPP6) based on the nValue(OPP5).
[/quote]
Right - how does it calculate the value for OPP6 from OPP5?
b) you are using OMAP343X_CONTROL_FUSE_OPP5_VDD1 itself
[sp] Quoting the description:
[quote]
The eFuse registers do not contain the nValue to be used
with 720MHz (OPP6). This patch implements procedure to
calculate the nValue(OPP6) based on the nValue(OPP5).
[/quote]
Repeating again, just in case you missed: nvalues for OPP6 are
calculated based on the nvalues for OPP5. So, i need to use
OMAP343X_CONTROL_FUSE_OPP5_VDD1.
why not define CONTROL_FUSE_OPP6_VDD1 with the same offset?
c) these should be __init functions.
[sp] They will be when i post formal patch.
d) how would you allow this code to work with 3440?
[sp] Does 3440 have same behavior? If so, it is quite easy by
updating this check.
[quote]
+ if (cpu_is_omap34xx()&& omap3_has_720mhz()) {
+ nvalue_table[count-1].nvalue = swcalc_opp6_nvalue(
+ nvalue_table[count-2].nvalue);
+ }
[/quote]
However, i am not conversant with 3440. This will have to be
covered by a separate patch.
Right - could you help start a TI internal conversation and alignment to
ensure 3440 is not broken by this change?
e) next time could you please try STOP using CaMELCASInG in your
variables and functions?
[sp] Did you ever notice any camelcase in any of my formal patch
submissions earlier. 10 brownie points for catching it here!
why do you waste reviewer time by posting CamelCasing anyways? your
responses make me wonder why did I even care to spend time to review
your code? could have waited for your formal patch and NAKed it :). ok..
no more discussions from me anymore untill your formal patch arrives
--
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
