Cousson, Benoit had written, on 03/22/2010 12:46 PM, the following:
From: Menon, Nishanth
Sent: Monday, March 22, 2010 2:30 PM
Cousson, Benoit had written, on 03/21/2010 04:50 PM, the following:
[...]
now in the approach I took,
you could have:
struct sr_ntarget_type{
unsigned long nTarget;
something else if needed
}
I'm still not convinced...
It appears that there is still some confusion with what OPP is or should
be.
Errr.. Overall, the idea is to provide an infrastructure to store
information per opp, OPP layer does not make policy decision - that is
left to the caller (a.k.a user modules).
The OPP layer, for my point of view was done to handle the freq ->
voltage association per IP. This layer should be flexible because we can
add or remove dynamically any OPP.
yes, this is the basic SOC generic definition of OPP - i agree.
All the data related to SR and ABB belong to the characterized voltages
that belong to a certain OPP for a voltage rail. These informations are not
configurable at all and not related to any IP. They are well defined for a
particular SoC techno (65nm, 45nm) for a voltage domain.
exactly the reason why this patch is relevant nominal voltage and
frequencies per OPP is not considered dynamic. OPP layer is a data store
layer - it stores information in a centralized manner allowing other
dependent module to query, store and operate on that information.
the dependency as you rightly pointed out is:
voltage -> SR,ABB information.
but as you already know,
OPP = (freq,voltage)
hence there is a indirect dependency of SR, ABB information per OPP.
Not at all, you missed the point of storing information per IP vs. information per voltage domain.
You first translate the freq to voltage for all IPs that belong to that voltage domain. If you have 10 IPs in the same voltage domain, you will not duplicate 10 times the SR informations.
I dont think you will. if you register with opp layer 10 times different
data, well.. you'd do it.. but anyways.. that is a minor point.
So they should belong to a SoC specific file, and should be hard coded
for a voltage domain.
ACK - the question is how would you do it? try to answer these questions:
a) where do you store SR ntarget values which is per OPP in a SOC
generic way?
b) how do you allow SR ntarget value to be queried in a SOC generic way?
By adding a voltage management API similar to the OPP management API, but dedicated to voltage domains.
I like that approach - do we have anything that does it today? nope. the
intent of doing SR changes was to introduce these changes, but seems to
have been missed somewhere down the pipe. shrug.
The point is that you should not tie SR Ntarget, ABB... to the OPP itself
but to the voltage. Hence the need to have other voltage -> SR, ABB table
in the voltage management code only.
err.. that is exactly how it is done.
Not really, it uses the OPP ID for the moment.
Seriously - that is not an explanation - it is a bad implementation.
We should not clutter the OPP layer with something that is voltage
related.
I dont see your point here. OPP layer is just providing an
infrastructure to store OPP related data.
This is the point... OPP layer is a freq/voltage tuple management layer not a voltage control layer. There is not necessarily a one to one mapping between OPP and voltage.
I agree from a theoretical perspective. the user of voltage management
layer needs the opp layer to know the voltages. The question is where do
you want to store the voltage related information?
The other point is that up to now, all voltage related data are a super
set of the previous SoC data, so you can define only one voltage_data
structure that work for all SoC. If the data is useless for previous
platform, just put 0 or a flag to disable that parameter.
welcome to redundant data. How do you plan to support 35xx devices which
do not have nTarget data? by putting 0s for those fields? you need to
store this data at the end of the day in some data structure, you dont
have a choice. the solution as I pointed in the previous mailchain is to
provide an SOC generic manner which ties the data corresponding to a
OPP(freq/voltage) to the OPP itself.
Welcome to complex data management for a couple of 32 bits data :-)
:)
If you look at the size of the infrastructure you have to put in place, I'm not even sure that you will save one bit.
I dont claim so.. but it stores only relevant information.. lists are
not exactly the most optimal logic, it is just the most flexible :)
if you look at the implementation from Thara's patches today, see:
See:
https://patchwork.kernel.org/patch/86642/
you will endup with a code as follows:
arch/arm/mach-omap2/srdevice.c:
+/* Read EFUSE values from control registers for OMAP3430 */
+static void __init omap34xx_sr_read_efuse(struct omap_smartreflex_data
*sr_data,
+ int sr_id)
+{
+ if (sr_id == SR1) {
[...]
+ sr_data->sr_nvalue[0] = omap_ctrl_readl(
+ OMAP343X_CONTROL_FUSE_OPP1_VDD2);
+ }
+}
well.. that is what you get if you decide that ONLY the module should
store the data-> it will have to have the concept of OPP ID as a result!
Sorry, I disagree with this.
That, I agree (with you). We need to get rid of the OPP ID stuff.
[...] [I am not about to repeat everything I stated in previous
threads.. so snipping the discussion down.]
Otherwise the primary idea to remove OPP ID is good, and the way to go.
good. So lets NAK that SR series and see how else we can implement it
without OPP ID. I am open to any clean mechanisms you may propose
without using OPP ID referencing :).
--
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
