All of lore.kernel.org
 help / color / mirror / Atom feed
* omap3 pm: dependency between opp layer and cpufreq
@ 2010-05-27 11:25 Premi, Sanjeev
  2010-05-27 13:19 ` Premi, Sanjeev
  2010-05-27 16:27 ` Nishanth Menon
  0 siblings, 2 replies; 20+ messages in thread
From: Premi, Sanjeev @ 2010-05-27 11:25 UTC (permalink / raw)
  To: linux-omap

Hi,

While compiling for omap3_evm_defconfig, at the head of linux-omap, I encounter
these errors:

arch/arm/mach-omap2/built-in.o: In function `sr_configure_vp':
/home/premi/linux-pm/arch/arm/mach-omap2/smartreflex.c:315: undefined reference to `omap_twl_uv_to_vsel'
/home/premi/linux-pm/arch/arm/mach-omap2/smartreflex.c:364: undefined reference to `omap_twl_uv_to_vsel'
arch/arm/mach-omap2/built-in.o: In function `sr_enable':
/home/premi/linux-pm/arch/arm/mach-omap2/smartreflex.c:609: undefined reference to `omap_twl_uv_to_vsel'
arch/arm/mach-omap2/built-in.o: In function `sr_reset_voltage':
/home/premi/linux-pm/arch/arm/mach-omap2/smartreflex.c:478: undefined reference to `omap_twl_uv_to_vsel'
/home/premi/linux-pm/arch/arm/mach-omap2/smartreflex.c:496: undefined reference to `omap_twl_uv_to_vsel'
make: *** [.tmp_vmlinux1] Error 1

Turn our that comment and code in plat-omap/Makefile don't match:

# OPP support in (OMAP3+ only at the moment)
# XXX The OPP TWL/TPS code should only be included when a TWL/TPS
# PMIC is selected.
ifdef CONFIG_CPU_FREQ
obj-$(CONFIG_ARCH_OMAP3) += opp.o opp_twl_tps.o
endif

But changing CONFIG_CPU_FREQ to CONFIG_TWL4030_POWER leads to these errors:

  CC      arch/arm/plat-omap/opp.o
arch/arm/plat-omap/opp.c:54: error: redefinition of 'opp_get_voltage'
arch/arm/plat-omap/include/plat/opp.h:240: error: previous definition of 'opp_get_voltage' was here
arch/arm/plat-omap/opp.c:63: error: redefinition of 'opp_get_freq'
arch/arm/plat-omap/include/plat/opp.h:245: error: previous definition of 'opp_get_freq' was here
arch/arm/plat-omap/opp.c:79: error: conflicting types for 'opp_find_by_opp_id'
arch/arm/plat-omap/include/plat/opp.h:296: error: previous definition of 'opp_find_by_opp_id' was here
arch/arm/plat-omap/opp.c:102: error: redefinition of 'opp_get_opp_id'
arch/arm/plat-omap/include/plat/opp.h:301: error: previous definition of 'opp_get_opp_id' was here
arch/arm/plat-omap/opp.c:107: error: conflicting types for 'opp_get_opp_count'
arch/arm/plat-omap/include/plat/opp.h:250: error: previous definition of 'opp_get_opp_count' was here
arch/arm/plat-omap/opp.c:129: error: conflicting types for 'opp_find_freq_exact'
arch/arm/plat-omap/include/plat/opp.h:256: error: previous definition of 'opp_find_freq_exact' was here
arch/arm/plat-omap/opp.c:153: error: conflicting types for 'opp_find_freq_ceil'
arch/arm/plat-omap/include/plat/opp.h:268: error: previous definition of 'opp_find_freq_ceil' was here
arch/arm/plat-omap/opp.c:182: error: conflicting types for 'opp_find_freq_floor'
arch/arm/plat-omap/include/plat/opp.h:262: error: previous definition of 'opp_find_freq_floor' was here
arch/arm/plat-omap/opp.c:223: error: conflicting types for 'opp_add'
arch/arm/plat-omap/include/plat/opp.h:280: error: previous definition of 'opp_add' was here
arch/arm/plat-omap/opp.c:291: error: conflicting types for 'opp_init_list'
arch/arm/plat-omap/include/plat/opp.h:274: error: previous definition of 'opp_init_list' was here
arch/arm/plat-omap/opp.c:335: error: redefinition of 'opp_enable'
arch/arm/plat-omap/include/plat/opp.h:285: error: previous definition of 'opp_enable' was here
arch/arm/plat-omap/opp.c:345: error: redefinition of 'opp_disable'
arch/arm/plat-omap/include/plat/opp.h:290: error: previous definition of 'opp_disable' was here
arch/arm/plat-omap/opp.c:356: error: conflicting types for 'opp_init_cpufreq_table'
arch/arm/plat-omap/include/plat/opp.h:307: error: previous definition of 'opp_init_cpufreq_table' was here
make[1]: *** [arch/arm/plat-omap/opp.o] Error 1

The contents of "plat-omap/include/plat/opp.h" seem to be based on assumption that definition of OPP is
needed only for cpufreq. But even if cpufreq is disabled, this information is required for setting the
correct voltage against 'pre-defined' ARM frequency.

Questions/ comments:

1) The linkage between OPP and Voltage are not driven by the PMIC.
   They are defined by/for the silicon itself.

2) The implementation for setting the voltage should depend upon the PMIC.

3) Was there any specific need to tie the functions "opp_get_voltage" and others to cpufreq only?

I am working on a patch that should remove some of these dependencies.
But, trying to open up a discussion as well...

Best regards,
Sanjeev

^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: omap3 pm: dependency between opp layer and cpufreq
  2010-05-27 11:25 omap3 pm: dependency between opp layer and cpufreq Premi, Sanjeev
@ 2010-05-27 13:19 ` Premi, Sanjeev
  2010-05-27 13:26   ` Premi, Sanjeev
  2010-05-27 16:27 ` Nishanth Menon
  1 sibling, 1 reply; 20+ messages in thread
From: Premi, Sanjeev @ 2010-05-27 13:19 UTC (permalink / raw)
  To: linux-omap

 

> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org 
> [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of Premi, Sanjeev
> Sent: Thursday, May 27, 2010 4:56 PM
> To: linux-omap@vger.kernel.org
> Subject: omap3 pm: dependency between opp layer and cpufreq
> 
> Hi,
> 
> While compiling for omap3_evm_defconfig, at the head of 
> linux-omap, I encounter
> these errors:
> 
> arch/arm/mach-omap2/built-in.o: In function `sr_configure_vp':
> /home/premi/linux-pm/arch/arm/mach-omap2/smartreflex.c:315: 
> undefined reference to `omap_twl_uv_to_vsel'
> /home/premi/linux-pm/arch/arm/mach-omap2/smartreflex.c:364: 
> undefined reference to `omap_twl_uv_to_vsel'
> arch/arm/mach-omap2/built-in.o: In function `sr_enable':
> /home/premi/linux-pm/arch/arm/mach-omap2/smartreflex.c:609: 
> undefined reference to `omap_twl_uv_to_vsel'
> arch/arm/mach-omap2/built-in.o: In function `sr_reset_voltage':
> /home/premi/linux-pm/arch/arm/mach-omap2/smartreflex.c:478: 
> undefined reference to `omap_twl_uv_to_vsel'
> /home/premi/linux-pm/arch/arm/mach-omap2/smartreflex.c:496: 
> undefined reference to `omap_twl_uv_to_vsel'
> make: *** [.tmp_vmlinux1] Error 1
> 
> Turn our that comment and code in plat-omap/Makefile don't match:
> 
> # OPP support in (OMAP3+ only at the moment)
> # XXX The OPP TWL/TPS code should only be included when a TWL/TPS
> # PMIC is selected.
> ifdef CONFIG_CPU_FREQ
> obj-$(CONFIG_ARCH_OMAP3) += opp.o opp_twl_tps.o
> endif
> 
> But changing CONFIG_CPU_FREQ to CONFIG_TWL4030_POWER leads to 
> these errors:
> 
>   CC      arch/arm/plat-omap/opp.o
> arch/arm/plat-omap/opp.c:54: error: redefinition of 'opp_get_voltage'
> arch/arm/plat-omap/include/plat/opp.h:240: error: previous 
> definition of 'opp_get_voltage' was here
> arch/arm/plat-omap/opp.c:63: error: redefinition of 'opp_get_freq'
> arch/arm/plat-omap/include/plat/opp.h:245: error: previous 
> definition of 'opp_get_freq' was here
> arch/arm/plat-omap/opp.c:79: error: conflicting types for 
> 'opp_find_by_opp_id'
> arch/arm/plat-omap/include/plat/opp.h:296: error: previous 
> definition of 'opp_find_by_opp_id' was here
> arch/arm/plat-omap/opp.c:102: error: redefinition of 'opp_get_opp_id'
> arch/arm/plat-omap/include/plat/opp.h:301: error: previous 
> definition of 'opp_get_opp_id' was here
> arch/arm/plat-omap/opp.c:107: error: conflicting types for 
> 'opp_get_opp_count'
> arch/arm/plat-omap/include/plat/opp.h:250: error: previous 
> definition of 'opp_get_opp_count' was here
> arch/arm/plat-omap/opp.c:129: error: conflicting types for 
> 'opp_find_freq_exact'
> arch/arm/plat-omap/include/plat/opp.h:256: error: previous 
> definition of 'opp_find_freq_exact' was here
> arch/arm/plat-omap/opp.c:153: error: conflicting types for 
> 'opp_find_freq_ceil'
> arch/arm/plat-omap/include/plat/opp.h:268: error: previous 
> definition of 'opp_find_freq_ceil' was here
> arch/arm/plat-omap/opp.c:182: error: conflicting types for 
> 'opp_find_freq_floor'
> arch/arm/plat-omap/include/plat/opp.h:262: error: previous 
> definition of 'opp_find_freq_floor' was here
> arch/arm/plat-omap/opp.c:223: error: conflicting types for 'opp_add'
> arch/arm/plat-omap/include/plat/opp.h:280: error: previous 
> definition of 'opp_add' was here
> arch/arm/plat-omap/opp.c:291: error: conflicting types for 
> 'opp_init_list'
> arch/arm/plat-omap/include/plat/opp.h:274: error: previous 
> definition of 'opp_init_list' was here
> arch/arm/plat-omap/opp.c:335: error: redefinition of 'opp_enable'
> arch/arm/plat-omap/include/plat/opp.h:285: error: previous 
> definition of 'opp_enable' was here
> arch/arm/plat-omap/opp.c:345: error: redefinition of 'opp_disable'
> arch/arm/plat-omap/include/plat/opp.h:290: error: previous 
> definition of 'opp_disable' was here
> arch/arm/plat-omap/opp.c:356: error: conflicting types for 
> 'opp_init_cpufreq_table'
> arch/arm/plat-omap/include/plat/opp.h:307: error: previous 
> definition of 'opp_init_cpufreq_table' was here
> make[1]: *** [arch/arm/plat-omap/opp.o] Error 1
> 
> The contents of "plat-omap/include/plat/opp.h" seem to be 
> based on assumption that definition of OPP is
> needed only for cpufreq. But even if cpufreq is disabled, 
> this information is required for setting the
> correct voltage against 'pre-defined' ARM frequency.
> 
> Questions/ comments:
> 
> 1) The linkage between OPP and Voltage are not driven by the PMIC.
>    They are defined by/for the silicon itself.
> 
> 2) The implementation for setting the voltage should depend 
> upon the PMIC.
> 
> 3) Was there any specific need to tie the functions 
> "opp_get_voltage" and others to cpufreq only?
> 
> I am working on a patch that should remove some of these dependencies.
> But, trying to open up a discussion as well...
> 
> Best regards,
> Sanjeev

Here is a quick patch for discussion.
(Will wait for comments before submitting a formal patch)

diff --git a/arch/arm/plat-omap/Makefile b/arch/arm/plat-omap/Makefile
index 2b9ebf0..18d291f 100644
--- a/arch/arm/plat-omap/Makefile
+++ b/arch/arm/plat-omap/Makefile
@@ -15,7 +15,7 @@ obj-$(CONFIG_ARCH_OMAP16XX) += ocpi.o
 # OPP support in (OMAP3+ only at the moment)
 # XXX The OPP TWL/TPS code should only be included when a TWL/TPS
 # PMIC is selected.
-ifdef CONFIG_CPU_FREQ
+ifdef CONFIG_TWL4030_POWER
 obj-$(CONFIG_ARCH_OMAP3) += opp.o opp_twl_tps.o
 endif

diff --git a/arch/arm/plat-omap/include/plat/opp.h b/arch/arm/plat-omap/include/plat/opp.h
index 7d16a46..03463d5 100644
--- a/arch/arm/plat-omap/include/plat/opp.h
+++ b/arch/arm/plat-omap/include/plat/opp.h
@@ -67,6 +67,14 @@ struct omap_opp_def {
 struct omap_opp;

 #ifdef CONFIG_CPU_FREQ
+void opp_init_cpufreq_table(enum opp_t opp_type,
+                           struct cpufreq_frequency_table **table);
+#else
+static inline void opp_init_cpufreq_table(struct omap_opp *opps,
+                           struct cpufreq_frequency_table **table)
+{
+}
+#endif

 /**
  * opp_get_voltage() - Gets the voltage corresponding to an opp
@@ -233,79 +241,4 @@ struct omap_opp * __deprecated opp_find_by_opp_id(enum opp_t opp_type,
                                                  u8 opp_id);
 u8 __deprecated opp_get_opp_id(struct omap_opp *opp);

-void opp_init_cpufreq_table(enum opp_t opp_type,
-                           struct cpufreq_frequency_table **table);
-#else
-static inline unsigned long opp_get_voltage(const struct omap_opp *opp)
-{
-       return 0;
-}
-
-static inline unsigned long opp_get_freq(const struct omap_opp *opp)
-{
-       return 0;
-}
-
-static inline int opp_get_opp_count(struct omap_opp *oppl)
-{
-       return 0;
-}
-
-static inline struct omap_opp *opp_find_freq_exact(struct omap_opp *oppl,
-                                    unsigned long freq, bool enabled)
-{
-       return ERR_PTR(-EINVAL);
-}
-
-static inline struct omap_opp *opp_find_freq_floor(struct omap_opp *oppl,
-                                    unsigned long *freq)
-{
-       return ERR_PTR(-EINVAL);
-}
-
-static inline struct omap_opp *opp_find_freq_ceil(struct omap_opp *oppl,
-                                       unsigned long *freq)
-{
-       return ERR_PTR(-EINVAL);
-}
-
-static inline
-struct omap_opp __init *opp_init_list(const struct omap_opp_def *opp_defs)
-{
-       return ERR_PTR(-EINVAL);
-}
-
-static inline struct omap_opp *opp_add(struct omap_opp *oppl,
-                        const struct omap_opp_def *opp_def)
-{
-       return ERR_PTR(-EINVAL);
-}
-
-static inline int opp_enable(struct omap_opp *opp)
-{
-       return 0;
-}
-
-static inline int opp_disable(struct omap_opp *opp)
-{
-       return 0;
-}
-
-static inline struct omap_opp * __deprecated
-opp_find_by_opp_id(struct omap_opp *opps, u8 opp_id)
-{
-       return ERR_PTR(-EINVAL);
-}
-
-static inline u8 __deprecated opp_get_opp_id(struct omap_opp *opp)
-{
-       return 0;
-}
-
-static inline void opp_init_cpufreq_table(struct omap_opp *opps,
-                           struct cpufreq_frequency_table **table)
-{
-}
-
-#endif         /* CONFIG_CPU_FREQ */
 #endif         /* __ASM_ARM_OMAP_OPP_H */
diff --git a/arch/arm/plat-omap/opp.c b/arch/arm/plat-omap/opp.c
index 13da451..76466fb 100644
--- a/arch/arm/plat-omap/opp.c
+++ b/arch/arm/plat-omap/opp.c
@@ -352,7 +352,7 @@ int opp_disable(struct omap_opp *opp)
        return 0;
 }

-/* XXX document */
+#ifdef CONFIG_CPU_FREQ
 void opp_init_cpufreq_table(enum opp_t opp_type,
                            struct cpufreq_frequency_table **table)
 {
@@ -397,3 +397,4 @@ void opp_init_cpufreq_table(enum opp_t opp_type,

        *table = &freq_table[0];
 }
+#endif

> --
> To unsubscribe from this list: send the line "unsubscribe 
> linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* RE: omap3 pm: dependency between opp layer and cpufreq
  2010-05-27 13:19 ` Premi, Sanjeev
@ 2010-05-27 13:26   ` Premi, Sanjeev
  0 siblings, 0 replies; 20+ messages in thread
From: Premi, Sanjeev @ 2010-05-27 13:26 UTC (permalink / raw)
  To: Premi, Sanjeev, linux-omap

> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org 
> [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of Premi, Sanjeev
> Sent: Thursday, May 27, 2010 6:50 PM
> To: linux-omap@vger.kernel.org
> Subject: RE: omap3 pm: dependency between opp layer and cpufreq
> 

[snip]--[snip]

>  
> diff --git a/arch/arm/plat-omap/Makefile b/arch/arm/plat-omap/Makefile
> index 2b9ebf0..18d291f 100644
> --- a/arch/arm/plat-omap/Makefile
> +++ b/arch/arm/plat-omap/Makefile
> @@ -15,7 +15,7 @@ obj-$(CONFIG_ARCH_OMAP16XX) += ocpi.o
>  # OPP support in (OMAP3+ only at the moment)
>  # XXX The OPP TWL/TPS code should only be included when a TWL/TPS
>  # PMIC is selected.
> -ifdef CONFIG_CPU_FREQ
> +ifdef CONFIG_TWL4030_POWER
>  obj-$(CONFIG_ARCH_OMAP3) += opp.o opp_twl_tps.o
>  endif
> 
After sending the mail, I realized that inclusion of opp_twl_tps.o should
depend upon CONFIG_TWL4030_POWER and opp.o should be included as default.

~sanjeev

[snip]--[snip]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: omap3 pm: dependency between opp layer and cpufreq
  2010-05-27 11:25 omap3 pm: dependency between opp layer and cpufreq Premi, Sanjeev
  2010-05-27 13:19 ` Premi, Sanjeev
@ 2010-05-27 16:27 ` Nishanth Menon
  2010-05-27 20:29   ` Premi, Sanjeev
  1 sibling, 1 reply; 20+ messages in thread
From: Nishanth Menon @ 2010-05-27 16:27 UTC (permalink / raw)
  To: Premi, Sanjeev; +Cc: linux-omap

On 05/27/2010 01:25 PM, Premi, Sanjeev wrote:
> Hi,
>
> While compiling for omap3_evm_defconfig, at the head of linux-omap, I encounter
> these errors:
>
> arch/arm/mach-omap2/built-in.o: In function `sr_configure_vp':
> /home/premi/linux-pm/arch/arm/mach-omap2/smartreflex.c:315: undefined reference to `omap_twl_uv_to_vsel'
> /home/premi/linux-pm/arch/arm/mach-omap2/smartreflex.c:364: undefined reference to `omap_twl_uv_to_vsel'
> arch/arm/mach-omap2/built-in.o: In function `sr_enable':
> /home/premi/linux-pm/arch/arm/mach-omap2/smartreflex.c:609: undefined reference to `omap_twl_uv_to_vsel'
> arch/arm/mach-omap2/built-in.o: In function `sr_reset_voltage':
> /home/premi/linux-pm/arch/arm/mach-omap2/smartreflex.c:478: undefined reference to `omap_twl_uv_to_vsel'
> /home/premi/linux-pm/arch/arm/mach-omap2/smartreflex.c:496: undefined reference to `omap_twl_uv_to_vsel'
> make: *** [.tmp_vmlinux1] Error 1
>
> Turn our that comment and code in plat-omap/Makefile don't match:
>
> # OPP support in (OMAP3+ only at the moment)
> # XXX The OPP TWL/TPS code should only be included when a TWL/TPS
> # PMIC is selected.
> ifdef CONFIG_CPU_FREQ
> obj-$(CONFIG_ARCH_OMAP3) += opp.o opp_twl_tps.o
ok this needs to be split:
a) opp_twl_tps should depend on TWL_CORE and not CPUFREQ - there is no 
need actually
b) opp.o should remain dependent on CPU_FREQ.

> endif
>
> But changing CONFIG_CPU_FREQ to CONFIG_TWL4030_POWER leads to these errors:
see (b)
>
>    CC      arch/arm/plat-omap/opp.o
> arch/arm/plat-omap/opp.c:54: error: redefinition of 'opp_get_voltage'
> arch/arm/plat-omap/include/plat/opp.h:240: error: previous definition of 'opp_get_voltage' was here
> arch/arm/plat-omap/opp.c:63: error: redefinition of 'opp_get_freq'
> arch/arm/plat-omap/include/plat/opp.h:245: error: previous definition of 'opp_get_freq' was here
> arch/arm/plat-omap/opp.c:79: error: conflicting types for 'opp_find_by_opp_id'
> arch/arm/plat-omap/include/plat/opp.h:296: error: previous definition of 'opp_find_by_opp_id' was here
> arch/arm/plat-omap/opp.c:102: error: redefinition of 'opp_get_opp_id'
> arch/arm/plat-omap/include/plat/opp.h:301: error: previous definition of 'opp_get_opp_id' was here
> arch/arm/plat-omap/opp.c:107: error: conflicting types for 'opp_get_opp_count'
> arch/arm/plat-omap/include/plat/opp.h:250: error: previous definition of 'opp_get_opp_count' was here
> arch/arm/plat-omap/opp.c:129: error: conflicting types for 'opp_find_freq_exact'
> arch/arm/plat-omap/include/plat/opp.h:256: error: previous definition of 'opp_find_freq_exact' was here
> arch/arm/plat-omap/opp.c:153: error: conflicting types for 'opp_find_freq_ceil'
> arch/arm/plat-omap/include/plat/opp.h:268: error: previous definition of 'opp_find_freq_ceil' was here
> arch/arm/plat-omap/opp.c:182: error: conflicting types for 'opp_find_freq_floor'
> arch/arm/plat-omap/include/plat/opp.h:262: error: previous definition of 'opp_find_freq_floor' was here
> arch/arm/plat-omap/opp.c:223: error: conflicting types for 'opp_add'
> arch/arm/plat-omap/include/plat/opp.h:280: error: previous definition of 'opp_add' was here
> arch/arm/plat-omap/opp.c:291: error: conflicting types for 'opp_init_list'
> arch/arm/plat-omap/include/plat/opp.h:274: error: previous definition of 'opp_init_list' was here
> arch/arm/plat-omap/opp.c:335: error: redefinition of 'opp_enable'
> arch/arm/plat-omap/include/plat/opp.h:285: error: previous definition of 'opp_enable' was here
> arch/arm/plat-omap/opp.c:345: error: redefinition of 'opp_disable'
> arch/arm/plat-omap/include/plat/opp.h:290: error: previous definition of 'opp_disable' was here
> arch/arm/plat-omap/opp.c:356: error: conflicting types for 'opp_init_cpufreq_table'
> arch/arm/plat-omap/include/plat/opp.h:307: error: previous definition of 'opp_init_cpufreq_table' was here
> make[1]: *** [arch/arm/plat-omap/opp.o] Error 1
>
> The contents of "plat-omap/include/plat/opp.h" seem to be based on assumption that definition of OPP is
> needed only for cpufreq. But even if cpufreq is disabled, this information is required for setting the
> correct voltage against 'pre-defined' ARM frequency.
>
> Questions/ comments:
>
> 1) The linkage between OPP and Voltage are not driven by the PMIC.
>     They are defined by/for the silicon itself.

look carefully at twl -> it is the abstraction where needed for pmic 
translation of a voltage value - this you agree is PMIC dependent I suppose.

>
> 2) The implementation for setting the voltage should depend upon the PMIC.
ACK- the concept should be independent of PMIC - each PMIC, like 
5030/tps Vs custom PMICs have thier own unique mechanisms of setting 
voltages - the renewed SR series from Thara addresses this concern.(not 
in pm branch yet - but potentially soon)

>
> 3) Was there any specific need to tie the functions "opp_get_voltage" and others to cpufreq only?

yes, because without cpufreq there is no transitions in the system :)

>
> I am working on a patch that should remove some of these dependencies.
> But, trying to open up a discussion as well...

do post them. thanks

>
> Best regards,
> Sanjeev
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Regards,
Nishanth Menon

^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: omap3 pm: dependency between opp layer and cpufreq
  2010-05-27 16:27 ` Nishanth Menon
@ 2010-05-27 20:29   ` Premi, Sanjeev
  2010-05-28  7:39     ` Menon, Nishanth
  0 siblings, 1 reply; 20+ messages in thread
From: Premi, Sanjeev @ 2010-05-27 20:29 UTC (permalink / raw)
  To: Menon, Nishanth; +Cc: linux-omap

From: Menon, Nishanth
Sent: Thursday, May 27, 2010 9:57 PM
To: Premi, Sanjeev
Cc: linux-omap@vger.kernel.org
Subject: Re: omap3 pm: dependency between opp layer and cpufreq

[snip]--[snip]

[sp] Responding via webmail.. so the formatting and quotes are non-standard :(

> # OPP support in (OMAP3+ only at the moment)
> # XXX The OPP TWL/TPS code should only be included when a TWL/TPS
> # PMIC is selected.
> ifdef CONFIG_CPU_FREQ
> obj-$(CONFIG_ARCH_OMAP3) += opp.o opp_twl_tps.o
ok this needs to be split:
a) opp_twl_tps should depend on TWL_CORE and not CPUFREQ - there is no
need actually
b) opp.o should remain dependent on CPU_FREQ.

[sp] That was in my next mail.

> endif
>
> But changing CONFIG_CPU_FREQ to CONFIG_TWL4030_POWER leads to these errors:
see (b)
>
[snip--[snip]

>
> Questions/ comments:
>
> 1) The linkage between OPP and Voltage are not driven by the PMIC.
>     They are defined by/for the silicon itself.

look carefully at twl -> it is the abstraction where needed for pmic
translation of a voltage value - this you agree is PMIC dependent I suppose.

>
> 2) The implementation for setting the voltage should depend upon the PMIC.
ACK- the concept should be independent of PMIC - each PMIC, like
5030/tps Vs custom PMICs have thier own unique mechanisms of setting
voltages - the renewed SR series from Thara addresses this concern.(not
in pm branch yet - but potentially soon)

>
> 3) Was there any specific need to tie the functions "opp_get_voltage" and others to cpufreq only?

yes, because without cpufreq there is no transitions in the system :)

[sp] I does - via bootarg - mpurate!

When kernel boots, volatge must be set properly.
We cannot rely on u-boot to be settiing everything correctly. e.g. 720MHz on OMAP3530 would fail at nominal 1.2V set by u-boot.

>
> I am working on a patch that should remove some of these dependencies.
> But, trying to open up a discussion as well...

do post them. thanks

>
> Best regards,
> Sanjeev
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Regards,
Nishanth Menon

^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: omap3 pm: dependency between opp layer and cpufreq
  2010-05-27 20:29   ` Premi, Sanjeev
@ 2010-05-28  7:39     ` Menon, Nishanth
  2010-05-28  8:56       ` Koen Kooi
  0 siblings, 1 reply; 20+ messages in thread
From: Menon, Nishanth @ 2010-05-28  7:39 UTC (permalink / raw)
  To: Premi, Sanjeev; +Cc: linux-omap, eduardo.valentin, Kevin Hilman

> -----Original Message-----
> From: Premi, Sanjeev
> Sent: Thursday, May 27, 2010 10:30 PM

[...]

> >
> > 3) Was there any specific need to tie the functions "opp_get_voltage"
> and others to cpufreq only?
> 
> yes, because without cpufreq there is no transitions in the system :)
> 
> [sp] I does - via bootarg - mpurate!
> 
> When kernel boots, volatge must be set properly.
> We cannot rely on u-boot to be settiing everything correctly. e.g. 720MHz
> on OMAP3530 would fail at nominal 1.2V set by u-boot.

I agree, but mpurate does not seem to use the cpufreq interfaces - so is 
kinda a question how it interfaces back -> but note, mpurate tells us what
the start freq is for the system - it still does no *dynamic* transitions - 
just a static startup frequency. But I agree, it assumes that if you provide 
mpurate, the system supposedly is operating at that frequency, aka all
setups have been done for that operational frequency(including voltage)


Regards,
Nishanth Menon

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: omap3 pm: dependency between opp layer and cpufreq
  2010-05-28  7:39     ` Menon, Nishanth
@ 2010-05-28  8:56       ` Koen Kooi
  2010-05-28 13:33         ` Nishanth Menon
  0 siblings, 1 reply; 20+ messages in thread
From: Koen Kooi @ 2010-05-28  8:56 UTC (permalink / raw)
  To: Menon, Nishanth
  Cc: Premi, Sanjeev, linux-omap, eduardo.valentin, Kevin Hilman


Op 28 mei 2010, om 09:39 heeft Menon, Nishanth het volgende geschreven:

>> -----Original Message-----
>> From: Premi, Sanjeev
>> Sent: Thursday, May 27, 2010 10:30 PM
> 
> [...]
> 
>>> 
>>> 3) Was there any specific need to tie the functions "opp_get_voltage"
>> and others to cpufreq only?
>> 
>> yes, because without cpufreq there is no transitions in the system :)
>> 
>> [sp] I does - via bootarg - mpurate!
>> 
>> When kernel boots, volatge must be set properly.
>> We cannot rely on u-boot to be settiing everything correctly. e.g. 720MHz
>> on OMAP3530 would fail at nominal 1.2V set by u-boot.
> 
> I agree, but mpurate does not seem to use the cpufreq interfaces - so is 
> kinda a question how it interfaces back -> but note, mpurate tells us what
> the start freq is for the system - it still does no *dynamic* transitions - 
> just a static startup frequency. But I agree, it assumes that if you provide 
> mpurate, the system supposedly is operating at that frequency, aka all
> setups have been done for that operational frequency(including voltage)

There's also a funny bug in the current (PSP) mpurate/cpufreq code. The mpurate code has a check for 720MHz on 35xx silicon, but cpufreq doesnt. So I can do:

setenv bootargs '<foo> mpurate=720'

And the kernel will say "unsupported" and not switch to 720MHz during boot. But if I do this after boot:

cpufreq-set -f 720

it *will* switch to 720MHz, even if the mpurate code explicitly forbids it. I tested on all the OMAP3 silicon I have and it will run at 720MHz fine, even if it's out of spec, so I'm happy with this bug :) 

regards,

Koen

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: omap3 pm: dependency between opp layer and cpufreq
  2010-05-28  8:56       ` Koen Kooi
@ 2010-05-28 13:33         ` Nishanth Menon
  2010-05-28 13:42           ` Premi, Sanjeev
  0 siblings, 1 reply; 20+ messages in thread
From: Nishanth Menon @ 2010-05-28 13:33 UTC (permalink / raw)
  To: Koen Kooi; +Cc: Premi, Sanjeev, linux-omap, eduardo.valentin, Kevin Hilman

On 05/28/2010 10:56 AM, Koen Kooi wrote:
>
> Op 28 mei 2010, om 09:39 heeft Menon, Nishanth het volgende geschreven:
>
>>> -----Original Message-----
>>> From: Premi, Sanjeev
>>> Sent: Thursday, May 27, 2010 10:30 PM
>>
>> [...]
>>
>>>>
>>>> 3) Was there any specific need to tie the functions "opp_get_voltage"
>>> and others to cpufreq only?
>>>
>>> yes, because without cpufreq there is no transitions in the system :)
>>>
>>> [sp] I does - via bootarg - mpurate!
>>>
>>> When kernel boots, volatge must be set properly.
>>> We cannot rely on u-boot to be settiing everything correctly. e.g. 720MHz
>>> on OMAP3530 would fail at nominal 1.2V set by u-boot.
>>
>> I agree, but mpurate does not seem to use the cpufreq interfaces - so is
>> kinda a question how it interfaces back ->  but note, mpurate tells us what
>> the start freq is for the system - it still does no *dynamic* transitions -
>> just a static startup frequency. But I agree, it assumes that if you provide
>> mpurate, the system supposedly is operating at that frequency, aka all
>> setups have been done for that operational frequency(including voltage)
>
> There's also a funny bug in the current (PSP) mpurate/cpufreq code. The mpurate code has a
 > check for 720MHz on 35xx silicon, but cpufreq doesnt. So I can do:
>
> setenv bootargs '<foo>  mpurate=720'
>
> And the kernel will say "unsupported" and not switch to 720MHz during boot. But if I do this after boot:
>
> cpufreq-set -f 720
>
> it *will* switch to 720MHz, even if the mpurate code explicitly forbids it. I tested on all the
 >OMAP3 silicon I have and it will run at 720MHz fine, even if it's out 
of spec, so I'm happy with this bug :)

:) on mainline, if you dont have the frequency in opp definitions and 
your board has not done an explicit opp_add, cpufreq will only set u to 
the nearest available freq - easy for mainline fix if someone would like 
to send a patch adding the OPPs and the detection logic involved for 
enabling them.

Now, thinking aloud, the voltage setting by SR will probably occur in 
late_init, if mpurate is setting the clock earlier in the boot process, 
we might have a potential conflict in the mpurate expecting the system 
to be set in a certain voltage based on Sanjeev's argument, but not 
actually there.. we expect ondemand+cpufreq to do the job on runtime 
anyways.

Regards,
Nishanth Menon

^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: omap3 pm: dependency between opp layer and cpufreq
  2010-05-28 13:33         ` Nishanth Menon
@ 2010-05-28 13:42           ` Premi, Sanjeev
  2010-05-28 13:55             ` Nishanth Menon
  0 siblings, 1 reply; 20+ messages in thread
From: Premi, Sanjeev @ 2010-05-28 13:42 UTC (permalink / raw)
  To: Menon, Nishanth, Koen Kooi; +Cc: linux-omap, eduardo.valentin, Kevin Hilman

> -----Original Message-----
> From: Menon, Nishanth 
> Sent: Friday, May 28, 2010 7:03 PM
> To: Koen Kooi
> Cc: Premi, Sanjeev; linux-omap@vger.kernel.org; 
> eduardo.valentin@nokia.com; Kevin Hilman
> Subject: Re: omap3 pm: dependency between opp layer and cpufreq
> 
> On 05/28/2010 10:56 AM, Koen Kooi wrote:
> >
> > Op 28 mei 2010, om 09:39 heeft Menon, Nishanth het volgende 
> geschreven:
> >
> >>> -----Original Message-----
> >>> From: Premi, Sanjeev
> >>> Sent: Thursday, May 27, 2010 10:30 PM
> >>
> >> [...]
> >>
> >>>>
> >>>> 3) Was there any specific need to tie the functions 
> "opp_get_voltage"
> >>> and others to cpufreq only?
> >>>
> >>> yes, because without cpufreq there is no transitions in 
> the system :)
> >>>
> >>> [sp] I does - via bootarg - mpurate!
> >>>
> >>> When kernel boots, volatge must be set properly.
> >>> We cannot rely on u-boot to be settiing everything 
> correctly. e.g. 720MHz
> >>> on OMAP3530 would fail at nominal 1.2V set by u-boot.
> >>
> >> I agree, but mpurate does not seem to use the cpufreq 
> interfaces - so is
> >> kinda a question how it interfaces back ->  but note, 
> mpurate tells us what
> >> the start freq is for the system - it still does no 
> *dynamic* transitions -
> >> just a static startup frequency. But I agree, it assumes 
> that if you provide
> >> mpurate, the system supposedly is operating at that 
> frequency, aka all
> >> setups have been done for that operational 
> frequency(including voltage)
> >
> > There's also a funny bug in the current (PSP) 
> mpurate/cpufreq code. The mpurate code has a
>  > check for 720MHz on 35xx silicon, but cpufreq doesnt. So I can do:
> >
> > setenv bootargs '<foo>  mpurate=720'
> >
> > And the kernel will say "unsupported" and not switch to 
> 720MHz during boot. But if I do this after boot:
> >
> > cpufreq-set -f 720
> >
> > it *will* switch to 720MHz, even if the mpurate code 
> explicitly forbids it. I tested on all the
>  >OMAP3 silicon I have and it will run at 720MHz fine, even 
> if it's out 
> of spec, so I'm happy with this bug :)
> 
> :) on mainline, if you dont have the frequency in opp definitions and 
> your board has not done an explicit opp_add, cpufreq will 
> only set u to 
> the nearest available freq - easy for mainline fix if someone 
> would like 
> to send a patch adding the OPPs and the detection logic involved for 
> enabling them.
> 
> Now, thinking aloud, the voltage setting by SR will probably occur in 
> late_init, if mpurate is setting the clock earlier in the 
> boot process, 
> we might have a potential conflict in the mpurate expecting 
> the system 
> to be set in a certain voltage based on Sanjeev's argument, but not 
> actually there.. we expect ondemand+cpufreq to do the job on runtime 
> anyways.

Nishanth,

When setting via mpurate, we need to get the appropriate voltage
corresponding to the mpurate so that right combination can be done.

This is where the mapping between freq and voltage needs to be queried.
And OPP layer is best placed to provide the info... without duplication.
The mechanism of changing the voltage itself can vary on the PMIC.

BTW, I am getting ready to submit an updated patch for mpurate. Just
waiting for an early resolution to this discussion.

~sanjeev
> 
> Regards,
> Nishanth Menon
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: omap3 pm: dependency between opp layer and cpufreq
  2010-05-28 13:42           ` Premi, Sanjeev
@ 2010-05-28 13:55             ` Nishanth Menon
  2010-05-28 14:02               ` Premi, Sanjeev
  0 siblings, 1 reply; 20+ messages in thread
From: Nishanth Menon @ 2010-05-28 13:55 UTC (permalink / raw)
  To: Premi, Sanjeev; +Cc: Koen Kooi, linux-omap, eduardo.valentin, Kevin Hilman

On 05/28/2010 03:42 PM, Premi, Sanjeev wrote:
>> -----Original Message-----
>> From: Menon, Nishanth
>> Sent: Friday, May 28, 2010 7:03 PM
>> To: Koen Kooi
>> Cc: Premi, Sanjeev; linux-omap@vger.kernel.org;
>> eduardo.valentin@nokia.com; Kevin Hilman
>> Subject: Re: omap3 pm: dependency between opp layer and cpufreq
>>
>> On 05/28/2010 10:56 AM, Koen Kooi wrote:
>>>
>>> Op 28 mei 2010, om 09:39 heeft Menon, Nishanth het volgende
>> geschreven:
>>>
>>>>> -----Original Message-----
>>>>> From: Premi, Sanjeev
>>>>> Sent: Thursday, May 27, 2010 10:30 PM
>>>>
>>>> [...]
>>>>
>>>>>>
>>>>>> 3) Was there any specific need to tie the functions
>> "opp_get_voltage"
>>>>> and others to cpufreq only?
>>>>>
>>>>> yes, because without cpufreq there is no transitions in
>> the system :)
>>>>>
>>>>> [sp] I does - via bootarg - mpurate!
>>>>>
>>>>> When kernel boots, volatge must be set properly.
>>>>> We cannot rely on u-boot to be settiing everything
>> correctly. e.g. 720MHz
>>>>> on OMAP3530 would fail at nominal 1.2V set by u-boot.
>>>>
>>>> I agree, but mpurate does not seem to use the cpufreq
>> interfaces - so is
>>>> kinda a question how it interfaces back ->   but note,
>> mpurate tells us what
>>>> the start freq is for the system - it still does no
>> *dynamic* transitions -
>>>> just a static startup frequency. But I agree, it assumes
>> that if you provide
>>>> mpurate, the system supposedly is operating at that
>> frequency, aka all
>>>> setups have been done for that operational
>> frequency(including voltage)
>>>
>>> There's also a funny bug in the current (PSP)
>> mpurate/cpufreq code. The mpurate code has a
>>   >  check for 720MHz on 35xx silicon, but cpufreq doesnt. So I can do:
>>>
>>> setenv bootargs '<foo>   mpurate=720'
>>>
>>> And the kernel will say "unsupported" and not switch to
>> 720MHz during boot. But if I do this after boot:
>>>
>>> cpufreq-set -f 720
>>>
>>> it *will* switch to 720MHz, even if the mpurate code
>> explicitly forbids it. I tested on all the
>>   >OMAP3 silicon I have and it will run at 720MHz fine, even
>> if it's out
>> of spec, so I'm happy with this bug :)
>>
>> :) on mainline, if you dont have the frequency in opp definitions and
>> your board has not done an explicit opp_add, cpufreq will
>> only set u to
>> the nearest available freq - easy for mainline fix if someone
>> would like
>> to send a patch adding the OPPs and the detection logic involved for
>> enabling them.
>>
>> Now, thinking aloud, the voltage setting by SR will probably occur in
>> late_init, if mpurate is setting the clock earlier in the
>> boot process,
>> we might have a potential conflict in the mpurate expecting
>> the system
>> to be set in a certain voltage based on Sanjeev's argument, but not
>> actually there.. we expect ondemand+cpufreq to do the job on runtime
>> anyways.
>
> Nishanth,
>
> When setting via mpurate, we need to get the appropriate voltage
> corresponding to the mpurate so that right combination can be done.
>
> This is where the mapping between freq and voltage needs to be queried.
> And OPP layer is best placed to provide the info... without duplication.
> The mechanism of changing the voltage itself can vary on the PMIC.
>
> BTW, I am getting ready to submit an updated patch for mpurate. Just
> waiting for an early resolution to this discussion.

aye, I am aware of the concept here, just questioning what does it mean 
by setting mpurate to the kernel - it could mean two things:
a) mean this is mpurate that the system was working on (aka) - now setup 
the required stuff to continue to function at that rate.
b) go to this rate and forget where you were running at.

the job of ondemand and other governors is to adjust to an optimal OPP 
using cpufreq which would conflict IMHO with (b), which kinda questions 
if you dont use cpufreq, does mpurate mean actually (a)?

anyways for cpufreq to work at 720Mhz, you need to add that frequency 
and corresponding voltage to the opptable, neither exists, further 
mpurate should work with opp table as well, else clockframework has no 
direct mechanism to verify the valid OPPs on a runtime system. that was 
the intent of opp layer - to provide the rest of the users with a 
mechanism to verify, query and use opps without functional knowledge of 
the silicon it works on..

Regards,
Nishanth Menon

^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: omap3 pm: dependency between opp layer and cpufreq
  2010-05-28 13:55             ` Nishanth Menon
@ 2010-05-28 14:02               ` Premi, Sanjeev
       [not found]                 ` <1275065163.10319.5.camel@Nokia-N900-51-1>
  0 siblings, 1 reply; 20+ messages in thread
From: Premi, Sanjeev @ 2010-05-28 14:02 UTC (permalink / raw)
  To: Menon, Nishanth; +Cc: Koen Kooi, linux-omap, eduardo.valentin, Kevin Hilman

> -----Original Message-----
> From: Menon, Nishanth 
> Sent: Friday, May 28, 2010 7:26 PM
> To: Premi, Sanjeev
> Cc: Koen Kooi; linux-omap@vger.kernel.org; 
> eduardo.valentin@nokia.com; Kevin Hilman
> Subject: Re: omap3 pm: dependency between opp layer and cpufreq
> 
> On 05/28/2010 03:42 PM, Premi, Sanjeev wrote:
> >> -----Original Message-----
> >> From: Menon, Nishanth
> >> Sent: Friday, May 28, 2010 7:03 PM
> >> To: Koen Kooi
> >> Cc: Premi, Sanjeev; linux-omap@vger.kernel.org;
> >> eduardo.valentin@nokia.com; Kevin Hilman
> >> Subject: Re: omap3 pm: dependency between opp layer and cpufreq
> >>
> >> On 05/28/2010 10:56 AM, Koen Kooi wrote:
> >>>
> >>> Op 28 mei 2010, om 09:39 heeft Menon, Nishanth het volgende
> >> geschreven:
> >>>
> >>>>> -----Original Message-----
> >>>>> From: Premi, Sanjeev
> >>>>> Sent: Thursday, May 27, 2010 10:30 PM
> >>>>
> >>>> [...]
> >>>>
> >>>>>>
> >>>>>> 3) Was there any specific need to tie the functions
> >> "opp_get_voltage"
> >>>>> and others to cpufreq only?
> >>>>>
> >>>>> yes, because without cpufreq there is no transitions in
> >> the system :)
> >>>>>
> >>>>> [sp] I does - via bootarg - mpurate!
> >>>>>
> >>>>> When kernel boots, volatge must be set properly.
> >>>>> We cannot rely on u-boot to be settiing everything
> >> correctly. e.g. 720MHz
> >>>>> on OMAP3530 would fail at nominal 1.2V set by u-boot.
> >>>>
> >>>> I agree, but mpurate does not seem to use the cpufreq
> >> interfaces - so is
> >>>> kinda a question how it interfaces back ->   but note,
> >> mpurate tells us what
> >>>> the start freq is for the system - it still does no
> >> *dynamic* transitions -
> >>>> just a static startup frequency. But I agree, it assumes
> >> that if you provide
> >>>> mpurate, the system supposedly is operating at that
> >> frequency, aka all
> >>>> setups have been done for that operational
> >> frequency(including voltage)
> >>>
> >>> There's also a funny bug in the current (PSP)
> >> mpurate/cpufreq code. The mpurate code has a
> >>   >  check for 720MHz on 35xx silicon, but cpufreq doesnt. 
> So I can do:
> >>>
> >>> setenv bootargs '<foo>   mpurate=720'
> >>>
> >>> And the kernel will say "unsupported" and not switch to
> >> 720MHz during boot. But if I do this after boot:
> >>>
> >>> cpufreq-set -f 720
> >>>
> >>> it *will* switch to 720MHz, even if the mpurate code
> >> explicitly forbids it. I tested on all the
> >>   >OMAP3 silicon I have and it will run at 720MHz fine, even
> >> if it's out
> >> of spec, so I'm happy with this bug :)
> >>
> >> :) on mainline, if you dont have the frequency in opp 
> definitions and
> >> your board has not done an explicit opp_add, cpufreq will
> >> only set u to
> >> the nearest available freq - easy for mainline fix if someone
> >> would like
> >> to send a patch adding the OPPs and the detection logic 
> involved for
> >> enabling them.
> >>
> >> Now, thinking aloud, the voltage setting by SR will 
> probably occur in
> >> late_init, if mpurate is setting the clock earlier in the
> >> boot process,
> >> we might have a potential conflict in the mpurate expecting
> >> the system
> >> to be set in a certain voltage based on Sanjeev's argument, but not
> >> actually there.. we expect ondemand+cpufreq to do the job 
> on runtime
> >> anyways.
> >
> > Nishanth,
> >
> > When setting via mpurate, we need to get the appropriate voltage
> > corresponding to the mpurate so that right combination can be done.
> >
> > This is where the mapping between freq and voltage needs to 
> be queried.
> > And OPP layer is best placed to provide the info... without 
> duplication.
> > The mechanism of changing the voltage itself can vary on the PMIC.
> >
> > BTW, I am getting ready to submit an updated patch for mpurate. Just
> > waiting for an early resolution to this discussion.
> 
> aye, I am aware of the concept here, just questioning what 
> does it mean 
> by setting mpurate to the kernel - it could mean two things:
> a) mean this is mpurate that the system was working on (aka) 
> - now setup 
> the required stuff to continue to function at that rate.
> b) go to this rate and forget where you were running at.
> 
> the job of ondemand and other governors is to adjust to an 
> optimal OPP 
> using cpufreq which would conflict IMHO with (b), which kinda 
> questions 
> if you dont use cpufreq, does mpurate mean actually (a)?

'mpurate' is usually used when cpufreq is not required. It
means - set me up for the specified freq and forget it. There
is no further change needed/ possible.

You could always argue that it can be done in u-boot; but this
bootarg helps people choose target freq keeping the u-boot same.

> 
> anyways for cpufreq to work at 720Mhz, you need to add that frequency 
> and corresponding voltage to the opptable, neither exists, further 
> mpurate should work with opp table as well, else 
> clockframework has no 
> direct mechanism to verify the valid OPPs on a runtime 
> system. that was 
> the intent of opp layer - to provide the rest of the users with a 
> mechanism to verify, query and use opps without functional 
> knowledge of 
> the silicon it works on..

Same for mpurate, if the user only requests a freq and doesn't care
about dependencies and mechanism for setting the freq.

> 
> Regards,
> Nishanth Menon
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: omap3 pm: dependency between opp layer and cpufreq
       [not found]                 ` <1275065163.10319.5.camel@Nokia-N900-51-1>
@ 2010-05-28 17:57                   ` Kevin Hilman
  2010-05-30 10:58                     ` Premi, Sanjeev
  2010-05-30 10:50                   ` Premi, Sanjeev
  1 sibling, 1 reply; 20+ messages in thread
From: Kevin Hilman @ 2010-05-28 17:57 UTC (permalink / raw)
  To: Nishanth menon
  Cc: Premi, Sanjeev, Menon, Nishanth, Koen Kooi, linux-omap, eduardo.valentin

Nishanth menon <menon.nishanth@gmail.com> writes:

[...]

>> 'mpurate' is usually used when cpufreq is not required. It
>> means - set me up for the specified freq and forget it. There
>> is no further change needed/ possible.
>
> That opens up the question - why not use cpufreq with userspace
> governor instead?  Esp if u dont want a change in freq, ok i get the
> part where u'd like a single freq for the system to function at, but
> u also mention, mpurate is for systems that dont care abt any other
> dependencies. So, bit of a contradiction if it depends on scaling
> voltage to the right level aka u are selecting an freq from opp
> table.
>
> This in my mind means u shud modify mpurate to use opp layer aka
> another user beyond cpufreq.

>>
>> You could always argue that it can be done in u-boot; but this
>> bootarg helps people choose target freq keeping the u-boot same.
>
> Errr..... Makes me feel that u shud really be using cpufreq instead! Either way
> i am not completely convinced u shud be doing voltage scaling when using
> mpurate given ur description- if u are trying to write a new cpufreq layer, why
> not fix why cpufreq doesn't work for u and help the rest of us as well ;)

Personally, I'm not opposed to supporting mpurate= (with CPUfreq
disabled) as this would also have the benefit of allowing a smaller
kernel if you really don't want DVFS.

After getting he right voltage from the OPP layer, what interface are
you planning to use to actually scale the voltage?  SR?  new voltage
layer directly?

Kevin

^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: omap3 pm: dependency between opp layer and cpufreq
       [not found]                 ` <1275065163.10319.5.camel@Nokia-N900-51-1>
  2010-05-28 17:57                   ` Kevin Hilman
@ 2010-05-30 10:50                   ` Premi, Sanjeev
  2010-05-31  4:19                     ` Nishanth Menon
  1 sibling, 1 reply; 20+ messages in thread
From: Premi, Sanjeev @ 2010-05-30 10:50 UTC (permalink / raw)
  To: Nishanth menon, Menon, Nishanth
  Cc: Koen Kooi, linux-omap, eduardo.valentin, Kevin Hilman


________________________________________
From: Nishanth menon [menon.nishanth@gmail.com]
Sent: Friday, May 28, 2010 10:16 PM
To: Premi, Sanjeev; Menon, Nishanth
Cc: Koen Kooi; linux-omap@vger.kernel.org; eduardo.valentin@nokia.com; Kevin Hilman
Subject: Re: omap3 pm: dependency between opp layer and cpufreq

----- Original message -----
(...)
> > > > > > > [sp] I does - via bootarg - mpurate!
> > > > > > >
> > > > > > > When kernel boots, volatge must be set properly.
> > > > > > > We cannot rely on u-boot to be settiing everything
> > > > correctly. e.g. 720MHz
> > > > > > > on OMAP3530 would fail at nominal 1.2V set by u-boot.
> > > > > >
> > > > > > I agree, but mpurate does not seem to use the cpufreq
> > > > interfaces - so is
> > > > > > kinda a question how it interfaces back ->    but note,
> > > > mpurate tells us what
> > > > > > the start freq is for the system - it still does no
> > > > *dynamic* transitions -
> > > > > > just a static startup frequency. But I agree, it assumes
> > > > that if you provide
> > > > > > mpurate, the system supposedly is operating at that
> > > > frequency, aka all
> > > > > > setups have been done for that operational
> > > > frequency(including voltage)
> > > > >
> > > > > There's also a funny bug in the current (PSP)
> > > > mpurate/cpufreq code. The mpurate code has a
> > > >    >  check for 720MHz on 35xx silicon, but cpufreq doesnt.
> > So I can do:
> > > > >
> > > > > setenv bootargs '<foo>    mpurate=720'
> > > > >
> > > > > And the kernel will say "unsupported" and not switch to
> > > > 720MHz during boot. But if I do this after boot:
> > > > >
> > > > > cpufreq-set -f 720
> > > > >
> > > > > it *will* switch to 720MHz, even if the mpurate code
> > > > explicitly forbids it. I tested on all the
> > > >    >OMAP3 silicon I have and it will run at 720MHz fine, even
> > > > if it's out
> > > > of spec, so I'm happy with this bug :)
> > > >
> > > > :) on mainline, if you dont have the frequency in opp
> > definitions and
> > > > your board has not done an explicit opp_add, cpufreq will
> > > > only set u to
> > > > the nearest available freq - easy for mainline fix if someone
> > > > would like
> > > > to send a patch adding the OPPs and the detection logic
> > involved for
> > > > enabling them.
> > > >
> > > > Now, thinking aloud, the voltage setting by SR will
> > probably occur in
> > > > late_init, if mpurate is setting the clock earlier in the
> > > > boot process,
> > > > we might have a potential conflict in the mpurate expecting
> > > > the system
> > > > to be set in a certain voltage based on Sanjeev's argument, but not
> > > > actually there.. we expect ondemand+cpufreq to do the job
> > on runtime
> > > > anyways.
> > >
> > > Nishanth,
> > >
> > > When setting via mpurate, we need to get the appropriate voltage
> > > corresponding to the mpurate so that right combination can be done.
> > >
> > > This is where the mapping between freq and voltage needs to
> > be queried.
> > > And OPP layer is best placed to provide the info... without
> > duplication.
> > > The mechanism of changing the voltage itself can vary on the PMIC.
> > >
> > > BTW, I am getting ready to submit an updated patch for mpurate. Just
> > > waiting for an early resolution to this discussion.
> >
> > aye, I am aware of the concept here, just questioning what
> > does it mean
> > by setting mpurate to the kernel - it could mean two things:
> > a) mean this is mpurate that the system was working on (aka)
> > - now setup
> > the required stuff to continue to function at that rate.
> > b) go to this rate and forget where you were running at.
> >
> > the job of ondemand and other governors is to adjust to an
> > optimal OPP
> > using cpufreq which would conflict IMHO with (b), which kinda
> > questions
> > if you dont use cpufreq, does mpurate mean actually (a)?
>
> 'mpurate' is usually used when cpufreq is not required. It
> means - set me up for the specified freq and forget it. There
> is no further change needed/ possible.
That opens up the question - why not use cpufreq with userspace governor instead? Esp if u dont want a change in freq, ok i get the part where u'd like a single freq for the system to function at, but u also mention, mpurate is for systems that dont care abt any other dependencies. So, bit of a contradiction if it depends on scaling voltage to the right level aka u are selecting an freq from opp table.

This in my mind means u shud modify mpurate to use opp layer aka another user beyond cpufreq.

[sp] That what the discussion is all about. but currently the opp layer is hidden/ wrapped inside CONFIG_CPU_FREQ. And I want to move it out. See an earlier patch that I send to this effect.

>
> You could always argue that it can be done in u-boot; but this
> bootarg helps people choose target freq keeping the u-boot same.

Errr..... Makes me feel that u shud really be using cpufreq instead!
[sp] This bootarg is meant for setting frequency. without baggage of cpufreq; so why not use it?

 Either way i am not completely convinced u shud be doing voltage scaling when using mpurate given ur description- 
[sp] So how would I run the 3530 at 720MHZ when the VDD1 is only 1.2V OR 3630 at 1GHz via 'mpurate'? Do you expect it to run magically :)

if u are trying to write a new cpufreq layer, why not fix why cpufreq doesn't work for u and help the rest of us as well ;)
[sp] There is no mention of cpufreq not working; but specifically the support of bootarg "mpurate" which is independent of cpufreq.

The bootarg mpurate has been existing since quite sometime. I am neither creating a new layer / replacement for cpufreq not trying to duplicate the code. The intent is simply as stated below:

1) Expose OPP layer - don't hinde it under CONFIG_CPU_FREQ.

2) Use OPP layer to:
    - Validate that the requested mpurate is defined in the OPP table for the device
    - And get the voltage corresponding to the OPP.

3) Ensure that right freq and voltage is set - at init time - based on the mpurate.

4) And at some poit later break the linkage between op player amd PMIC.

>
> >
> > anyways for cpufreq to work at 720Mhz, you need to add that frequency
> > and corresponding voltage to the opptable, neither exists, further
> > mpurate should work with opp table as well, else
> > clockframework has no
> > direct mechanism to verify the valid OPPs on a runtime
> > system. that was
> > the intent of opp layer - to provide the rest of the users with a
> > mechanism to verify, query and use opps without functional
> > knowledge of
> > the silicon it works on..
>
> Same for mpurate, if the user only requests a freq and doesn't care
> about dependencies and mechanism for setting the freq.
No it does have a dependency- u are depending on voltage being set right for the freq=cpufreq functionality IMHO.

regrds,
NM


^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: omap3 pm: dependency between opp layer and cpufreq
  2010-05-28 17:57                   ` Kevin Hilman
@ 2010-05-30 10:58                     ` Premi, Sanjeev
  0 siblings, 0 replies; 20+ messages in thread
From: Premi, Sanjeev @ 2010-05-30 10:58 UTC (permalink / raw)
  To: Kevin Hilman, Nishanth menon
  Cc: Menon, Nishanth, Koen Kooi, linux-omap, eduardo.valentin

_______________________________________
From: Kevin Hilman [khilman@deeprootsystems.com]
Sent: Friday, May 28, 2010 11:27 PM
To: Nishanth menon
Cc: Premi, Sanjeev; Menon, Nishanth; Koen Kooi; linux-omap@vger.kernel.org; eduardo.valentin@nokia.com
Subject: Re: omap3 pm: dependency between opp layer and cpufreq

Nishanth menon <menon.nishanth@gmail.com> writes:

[...]

>> 'mpurate' is usually used when cpufreq is not required. It
>> means - set me up for the specified freq and forget it. There
>> is no further change needed/ possible.
>
> That opens up the question - why not use cpufreq with userspace
> governor instead?  Esp if u dont want a change in freq, ok i get the
> part where u'd like a single freq for the system to function at, but
> u also mention, mpurate is for systems that dont care abt any other
> dependencies. So, bit of a contradiction if it depends on scaling
> voltage to the right level aka u are selecting an freq from opp
> table.
>
> This in my mind means u shud modify mpurate to use opp layer aka
> another user beyond cpufreq.

>>
>> You could always argue that it can be done in u-boot; but this
>> bootarg helps people choose target freq keeping the u-boot same.
>
> Errr..... Makes me feel that u shud really be using cpufreq instead! Either way
> i am not completely convinced u shud be doing voltage scaling when using
> mpurate given ur description- if u are trying to write a new cpufreq layer, why
> not fix why cpufreq doesn't work for u and help the rest of us as well ;)

Personally, I'm not opposed to supporting mpurate= (with CPUfreq
disabled) as this would also have the benefit of allowing a smaller
kernel if you really don't want DVFS.

After getting he right voltage from the OPP layer, what interface are
you planning to use to actually scale the voltage?  SR?  new voltage
layer directly?

[sp] I plan to use the existing infra only. SR is again a config option. In the
current internal implementation SR is being used. But this leads to an "ugly"
hack where actual setting of mpurate is delayed until SR is initialized.

In other mail (in response to Nishanth) I suggested that dependency between
OPP layer and PMIC needs to be broken. If done sooner, i would try to avoid
using SR (as preference).

I haven't yet looked at new SR layer good enough to make clear yes/no
response. 

~sanjeev

Kevin

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: omap3 pm: dependency between opp layer and cpufreq
  2010-05-30 10:50                   ` Premi, Sanjeev
@ 2010-05-31  4:19                     ` Nishanth Menon
  2010-06-03  0:00                       ` Kevin Hilman
  0 siblings, 1 reply; 20+ messages in thread
From: Nishanth Menon @ 2010-05-31  4:19 UTC (permalink / raw)
  To: Premi, Sanjeev
  Cc: Menon, Nishanth, Koen Kooi, linux-omap, eduardo.valentin, Kevin Hilman

On 05/30/2010 01:50 PM, Premi, Sanjeev wrote:
>
> ________________________________________
> From: Nishanth menon [menon.nishanth@gmail.com]
> Sent: Friday, May 28, 2010 10:16 PM
> To: Premi, Sanjeev; Menon, Nishanth
> Cc: Koen Kooi; linux-omap@vger.kernel.org; eduardo.valentin@nokia.com; Kevin Hilman
> Subject: Re: omap3 pm: dependency between opp layer and cpufreq
>
> ----- Original message -----
> (...)
>>>>>>>> [sp] I does - via bootarg - mpurate!
>>>>>>>>
>>>>>>>> When kernel boots, volatge must be set properly.
>>>>>>>> We cannot rely on u-boot to be settiing everything
>>>>> correctly. e.g. 720MHz
>>>>>>>> on OMAP3530 would fail at nominal 1.2V set by u-boot.
>>>>>>>
>>>>>>> I agree, but mpurate does not seem to use the cpufreq
>>>>> interfaces - so is
>>>>>>> kinda a question how it interfaces back ->     but note,
>>>>> mpurate tells us what
>>>>>>> the start freq is for the system - it still does no
>>>>> *dynamic* transitions -
>>>>>>> just a static startup frequency. But I agree, it assumes
>>>>> that if you provide
>>>>>>> mpurate, the system supposedly is operating at that
>>>>> frequency, aka all
>>>>>>> setups have been done for that operational
>>>>> frequency(including voltage)
>>>>>>
>>>>>> There's also a funny bug in the current (PSP)
>>>>> mpurate/cpufreq code. The mpurate code has a
>>>>>     >   check for 720MHz on 35xx silicon, but cpufreq doesnt.
>>> So I can do:
>>>>>>
>>>>>> setenv bootargs '<foo>     mpurate=720'
>>>>>>
>>>>>> And the kernel will say "unsupported" and not switch to
>>>>> 720MHz during boot. But if I do this after boot:
>>>>>>
>>>>>> cpufreq-set -f 720
>>>>>>
>>>>>> it *will* switch to 720MHz, even if the mpurate code
>>>>> explicitly forbids it. I tested on all the
>>>>>     >OMAP3 silicon I have and it will run at 720MHz fine, even
>>>>> if it's out
>>>>> of spec, so I'm happy with this bug :)
>>>>>
>>>>> :) on mainline, if you dont have the frequency in opp
>>> definitions and
>>>>> your board has not done an explicit opp_add, cpufreq will
>>>>> only set u to
>>>>> the nearest available freq - easy for mainline fix if someone
>>>>> would like
>>>>> to send a patch adding the OPPs and the detection logic
>>> involved for
>>>>> enabling them.
>>>>>
>>>>> Now, thinking aloud, the voltage setting by SR will
>>> probably occur in
>>>>> late_init, if mpurate is setting the clock earlier in the
>>>>> boot process,
>>>>> we might have a potential conflict in the mpurate expecting
>>>>> the system
>>>>> to be set in a certain voltage based on Sanjeev's argument, but not
>>>>> actually there.. we expect ondemand+cpufreq to do the job
>>> on runtime
>>>>> anyways.
>>>>
>>>> Nishanth,
>>>>
>>>> When setting via mpurate, we need to get the appropriate voltage
>>>> corresponding to the mpurate so that right combination can be done.
>>>>
>>>> This is where the mapping between freq and voltage needs to
>>> be queried.
>>>> And OPP layer is best placed to provide the info... without
>>> duplication.
>>>> The mechanism of changing the voltage itself can vary on the PMIC.
>>>>
>>>> BTW, I am getting ready to submit an updated patch for mpurate. Just
>>>> waiting for an early resolution to this discussion.
>>>
>>> aye, I am aware of the concept here, just questioning what
>>> does it mean
>>> by setting mpurate to the kernel - it could mean two things:
>>> a) mean this is mpurate that the system was working on (aka)
>>> - now setup
>>> the required stuff to continue to function at that rate.
>>> b) go to this rate and forget where you were running at.
>>>
>>> the job of ondemand and other governors is to adjust to an
>>> optimal OPP
>>> using cpufreq which would conflict IMHO with (b), which kinda
>>> questions
>>> if you dont use cpufreq, does mpurate mean actually (a)?
>>
>> 'mpurate' is usually used when cpufreq is not required. It
>> means - set me up for the specified freq and forget it. There
>> is no further change needed/ possible.
> That opens up the question - why not use cpufreq with userspace governor instead? Esp if u dont want a change in freq, ok i get the part where u'd like a single freq for the system to function at, but u also mention, mpurate is for systems that dont care abt any other dependencies. So, bit of a contradiction if it depends on scaling voltage to the right level aka u are selecting an freq from opp table.
>
> This in my mind means u shud modify mpurate to use opp layer aka another user beyond cpufreq.
>
> [sp] That what the discussion is all about. but currently the opp layer is hidden/ wrapped inside CONFIG_CPU_FREQ.
 > And I want to move it out. See an earlier patch that I send to this 
effect.

http://marc.info/?l=linux-omap&m=127496850617818&w=2 ? or did i miss the 
link? might be good to point to the right patch.. the opp layer 
initially was not under CONFIG_CPU_FREQ, it was moved after debate in 
this list. if there is a reason to move it out, i dont see why we cant 
do it..

>
>>
>> You could always argue that it can be done in u-boot; but this
>> bootarg helps people choose target freq keeping the u-boot same.
>
> Errr..... Makes me feel that u shud really be using cpufreq instead!
> [sp] This bootarg is meant for setting frequency. without baggage of cpufreq; so why not use it?
>
>   Either way i am not completely convinced u shud be doing voltage scaling when using mpurate given ur description-
> [sp] So how would I run the 3530 at 720MHZ when the VDD1 is only 1.2V OR 3630 at 1GHz via 'mpurate'? Do you expect it to run magically :)

I dont really care either way in reality, for me, cpufreq can:
a) set a single freq,
b) change frequencies dynamically.
Vs:
mpurate sets a single freq.

the way to set voltage will be to use the new voltage layer, if we 
isolate the frequencies to the opp layer, no reason why mpurate cant 
query for valid ones and use them.


my point of contention was: is mpurate meant for bootloader telling what 
configuration it is on to kernel, hence kernel continuing that 
configuration Vs, bootloader using mpurate to tell kernel what frequency 
it should run on. thanks it is clarified now that it is the later of the 
two.

>
> if u are trying to write a new cpufreq layer, why not fix why cpufreq doesn't work for u and help the rest of us as well ;)
> [sp] There is no mention of cpufreq not working; but specifically the support of bootarg "mpurate" which is independent of cpufreq.
>
> The bootarg mpurate has been existing since quite sometime. I am neither creating a new layer / replacement
 >for cpufreq not trying to duplicate the code. The intent is simply as 
stated below:
>
> 1) Expose OPP layer - don't hinde it under CONFIG_CPU_FREQ.
ok with this
>
> 2) Use OPP layer to:
>      - Validate that the requested mpurate is defined in the OPP table for the device
>      - And get the voltage corresponding to the OPP.
sounds good too
>
> 3) Ensure that right freq and voltage is set - at init time - based on the mpurate.
ok

>
> 4) And at some poit later break the linkage between op player amd PMIC.
>
aah you mean a simple patch as follows?
diff --git a/arch/arm/plat-omap/Makefile b/arch/arm/plat-omap/Makefile
index 2b9ebf0..bfb3d0e 100644
--- a/arch/arm/plat-omap/Makefile
+++ b/arch/arm/plat-omap/Makefile
@@ -16,7 +16,8 @@ obj-$(CONFIG_ARCH_OMAP16XX) += ocpi.o
  # XXX The OPP TWL/TPS code should only be included when a TWL/TPS
  # PMIC is selected.
  ifdef CONFIG_CPU_FREQ
-obj-$(CONFIG_ARCH_OMAP3) += opp.o opp_twl_tps.o
+obj-$(CONFIG_ARCH_OMAP3) += opp.o
+obj-$(CONFIG_TWL4030_CORE) += opp_twl_tps.o
  endif

  # omap_device support (OMAP2+ only at the moment)
--
note - opp layer was never tied to pmic -> there is pmic voltage 
conversion apis in opp_twl_tps.c

or is there more in your view?

>>
>>>
>>> anyways for cpufreq to work at 720Mhz, you need to add that frequency
>>> and corresponding voltage to the opptable, neither exists, further
>>> mpurate should work with opp table as well, else
>>> clockframework has no
>>> direct mechanism to verify the valid OPPs on a runtime
>>> system. that was
>>> the intent of opp layer - to provide the rest of the users with a
>>> mechanism to verify, query and use opps without functional
>>> knowledge of
>>> the silicon it works on..

ofcourse, please feel free to post a patch for the missing frequencies.


Regards,
Nishanth Menon

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: omap3 pm: dependency between opp layer and cpufreq
  2010-05-31  4:19                     ` Nishanth Menon
@ 2010-06-03  0:00                       ` Kevin Hilman
  2010-06-04 12:34                         ` Gopinath, Thara
  0 siblings, 1 reply; 20+ messages in thread
From: Kevin Hilman @ 2010-06-03  0:00 UTC (permalink / raw)
  To: Nishanth Menon
  Cc: Premi, Sanjeev, Menon, Nishanth, Koen Kooi, linux-omap, eduardo.valentin

Nishanth Menon <menon.nishanth@gmail.com> writes:

> On 05/30/2010 01:50 PM, Premi, Sanjeev wrote:
>>
[...]

>> [sp] There is no mention of cpufreq not working; but specifically the support of bootarg "mpurate" which is independent of cpufreq.
>>
>> The bootarg mpurate has been existing since quite sometime. I am neither creating a new layer / replacement
>> for cpufreq not trying to duplicate the code. The intent is simply
>> as 
> stated below:
>>
>> 1) Expose OPP layer - don't hinde it under CONFIG_CPU_FREQ.
> ok with this
>>
>> 2) Use OPP layer to:
>>      - Validate that the requested mpurate is defined in the OPP table for the device
>>      - And get the voltage corresponding to the OPP.
> sounds good too
>>
>> 3) Ensure that right freq and voltage is set - at init time - based on the mpurate.
> ok
>
>>
>> 4) And at some poit later break the linkage between op player amd PMIC.
>>
> aah you mean a simple patch as follows?
> diff --git a/arch/arm/plat-omap/Makefile b/arch/arm/plat-omap/Makefile
> index 2b9ebf0..bfb3d0e 100644
> --- a/arch/arm/plat-omap/Makefile
> +++ b/arch/arm/plat-omap/Makefile
> @@ -16,7 +16,8 @@ obj-$(CONFIG_ARCH_OMAP16XX) += ocpi.o
>  # XXX The OPP TWL/TPS code should only be included when a TWL/TPS
>  # PMIC is selected.
>  ifdef CONFIG_CPU_FREQ
> -obj-$(CONFIG_ARCH_OMAP3) += opp.o opp_twl_tps.o
> +obj-$(CONFIG_ARCH_OMAP3) += opp.o
> +obj-$(CONFIG_TWL4030_CORE) += opp_twl_tps.o
>  endif
>
>  # omap_device support (OMAP2+ only at the moment)
> --
> note - opp layer was never tied to pmic -> there is pmic voltage
> conversion apis in opp_twl_tps.c
>
> or is there more in your view?
>
>>>
>>>>
>>>> anyways for cpufreq to work at 720Mhz, you need to add that frequency
>>>> and corresponding voltage to the opptable, neither exists, further
>>>> mpurate should work with opp table as well, else
>>>> clockframework has no
>>>> direct mechanism to verify the valid OPPs on a runtime
>>>> system. that was
>>>> the intent of opp layer - to provide the rest of the users with a
>>>> mechanism to verify, query and use opps without functional
>>>> knowledge of
>>>> the silicon it works on..
>
> ofcourse, please feel free to post a patch for the missing frequencies.
>


OK, I must admit to not reading this whole thread since I've just
restructured OPP and CPUfreq support in the PM branch.

OPP support is now in the pm-opp branch (based on mainline) and
CPUfreq support is now in the pm-cpufreq branch (based on mainline.)

Please update this patch/series against one (or both) of those
branches for more discussion.

FWIW, I like the name change from cpufreq34xx --> opp34xx_data, and
the Makefile change above makes sense.  Both of these changes should
be re-submitted against my pm-opp branch.

Kevin

^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: omap3 pm: dependency between opp layer and cpufreq
  2010-06-03  0:00                       ` Kevin Hilman
@ 2010-06-04 12:34                         ` Gopinath, Thara
  2010-06-08  4:15                           ` opp layer query apis (was RE: omap3 pm: dependency between opp layer and cpufreq) Menon, Nishanth
  0 siblings, 1 reply; 20+ messages in thread
From: Gopinath, Thara @ 2010-06-04 12:34 UTC (permalink / raw)
  To: Kevin Hilman, Nishanth Menon
  Cc: Premi, Sanjeev, Menon, Nishanth, Koen Kooi, linux-omap, eduardo.valentin



>>-----Original Message-----
>>From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of Kevin
>>Hilman
>>Sent: Thursday, June 03, 2010 5:31 AM
>>To: Nishanth Menon
>>Cc: Premi, Sanjeev; Menon, Nishanth; Koen Kooi; linux-omap@vger.kernel.org;
>>eduardo.valentin@nokia.com
>>Subject: Re: omap3 pm: dependency between opp layer and cpufreq
>>
>>Nishanth Menon <menon.nishanth@gmail.com> writes:
>>
>>> On 05/30/2010 01:50 PM, Premi, Sanjeev wrote:
>>>>
>>[...]
>>
>>>> [sp] There is no mention of cpufreq not working; but specifically the support of bootarg "mpurate"
>>which is independent of cpufreq.
>>>>
>>>> The bootarg mpurate has been existing since quite sometime. I am neither creating a new layer /
>>replacement
>>>> for cpufreq not trying to duplicate the code. The intent is simply
>>>> as
>>> stated below:
>>>>
>>>> 1) Expose OPP layer - don't hinde it under CONFIG_CPU_FREQ.
>>> ok with this
>>>>
>>>> 2) Use OPP layer to:
>>>>      - Validate that the requested mpurate is defined in the OPP table for the device
>>>>      - And get the voltage corresponding to the OPP.
>>> sounds good too
>>>>
>>>> 3) Ensure that right freq and voltage is set - at init time - based on the mpurate.
>>> ok
>>>
>>>>
>>>> 4) And at some poit later break the linkage between op player amd PMIC.
>>>>
>>> aah you mean a simple patch as follows?
>>> diff --git a/arch/arm/plat-omap/Makefile b/arch/arm/plat-omap/Makefile
>>> index 2b9ebf0..bfb3d0e 100644
>>> --- a/arch/arm/plat-omap/Makefile
>>> +++ b/arch/arm/plat-omap/Makefile
>>> @@ -16,7 +16,8 @@ obj-$(CONFIG_ARCH_OMAP16XX) += ocpi.o
>>>  # XXX The OPP TWL/TPS code should only be included when a TWL/TPS
>>>  # PMIC is selected.
>>>  ifdef CONFIG_CPU_FREQ
>>> -obj-$(CONFIG_ARCH_OMAP3) += opp.o opp_twl_tps.o
>>> +obj-$(CONFIG_ARCH_OMAP3) += opp.o
>>> +obj-$(CONFIG_TWL4030_CORE) += opp_twl_tps.o
>>>  endif
>>>
>>>  # omap_device support (OMAP2+ only at the moment)
>>> --
>>> note - opp layer was never tied to pmic -> there is pmic voltage
>>> conversion apis in opp_twl_tps.c
>>>
>>> or is there more in your view?
>>>
>>>>>
>>>>>>
>>>>>> anyways for cpufreq to work at 720Mhz, you need to add that frequency
>>>>>> and corresponding voltage to the opptable, neither exists, further
>>>>>> mpurate should work with opp table as well, else
>>>>>> clockframework has no
>>>>>> direct mechanism to verify the valid OPPs on a runtime
>>>>>> system. that was
>>>>>> the intent of opp layer - to provide the rest of the users with a
>>>>>> mechanism to verify, query and use opps without functional
>>>>>> knowledge of
>>>>>> the silicon it works on..
>>>
>>> ofcourse, please feel free to post a patch for the missing frequencies.
>>>
>>
>>
>>OK, I must admit to not reading this whole thread since I've just
>>restructured OPP and CPUfreq support in the PM branch.
>>
>>OPP support is now in the pm-opp branch (based on mainline) and
>>CPUfreq support is now in the pm-cpufreq branch (based on mainline.)
>>
>>Please update this patch/series against one (or both) of those
>>branches for more discussion.
>>
>>FWIW, I like the name change from cpufreq34xx --> opp34xx_data, and
>>the Makefile change above makes sense.  Both of these changes should
>>be re-submitted against my pm-opp branch.
>>

Hi All, 

I was just going through the entire series. IMO, it is a good idea to take the opp layer
out of CONFIG_CPU_FREQ considering cpu freq  need not be the only layer using the opp
structures. Tomorrow we could have a l3 bus governor or a constraint framework
that will need to use the same framework. So it is best if the opp layer is not dependent on
the cpu freq layer.
Slightly off the topic but considering we are discussing opp layer here, I do not find API's
that take the OPP type (OPP_MPU or OPP_L3) and voltage/frequency as parameters and returns back the associated frequency/voltage. I do not want to get the opp table entry every time I do this. Any chance of introducing this in the framework? I could send a patch for the same.

Regards

^ permalink raw reply	[flat|nested] 20+ messages in thread

* opp layer query apis (was RE: omap3 pm: dependency between opp layer and cpufreq)
  2010-06-04 12:34                         ` Gopinath, Thara
@ 2010-06-08  4:15                           ` Menon, Nishanth
  2010-06-08  4:29                             ` Gopinath, Thara
  0 siblings, 1 reply; 20+ messages in thread
From: Menon, Nishanth @ 2010-06-08  4:15 UTC (permalink / raw)
  To: Gopinath, Thara, Kevin Hilman, Nishanth Menon
  Cc: Premi, Sanjeev, Koen Kooi, linux-omap, eduardo.valentin

> -----Original Message-----
> From: Gopinath, Thara
> Sent: Friday, June 04, 2010 3:35 PM
> To: Kevin Hilman; Nishanth Menon
> Cc: Premi, Sanjeev; Menon, Nishanth; Koen Kooi; linux-


> Slightly off the topic but considering we are discussing opp layer here, I
> do not find API's
> that take the OPP type (OPP_MPU or OPP_L3) and voltage/frequency as
> parameters and returns back the associated frequency/voltage. I do not
> want to get the opp table entry every time I do this. Any chance of
> introducing this in the framework? I could send a patch for the same.
Am curios on the usage that combination of 
opp_find_freq_exact 
opp_find_freq_ceil  
opp_find_freq_floor

and opp_get_freq, opp_get_voltage is unable to cater to? Would be interested 
in seeing a patch for the same.

Regards,
Nishanth Menon

^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: opp layer query apis (was RE: omap3 pm: dependency between opp layer and cpufreq)
  2010-06-08  4:15                           ` opp layer query apis (was RE: omap3 pm: dependency between opp layer and cpufreq) Menon, Nishanth
@ 2010-06-08  4:29                             ` Gopinath, Thara
  2010-06-08  4:35                               ` Menon, Nishanth
  0 siblings, 1 reply; 20+ messages in thread
From: Gopinath, Thara @ 2010-06-08  4:29 UTC (permalink / raw)
  To: Menon, Nishanth, Kevin Hilman, Nishanth Menon
  Cc: Premi, Sanjeev, Koen Kooi, linux-omap, eduardo.valentin



>>-----Original Message-----
>>From: Menon, Nishanth
>>Sent: Tuesday, June 08, 2010 9:45 AM
>>To: Gopinath, Thara; Kevin Hilman; Nishanth Menon
>>Cc: Premi, Sanjeev; Koen Kooi; linux-omap@vger.kernel.org; eduardo.valentin@nokia.com
>>Subject: opp layer query apis (was RE: omap3 pm: dependency between opp layer and cpufreq)
>>
>>> -----Original Message-----
>>> From: Gopinath, Thara
>>> Sent: Friday, June 04, 2010 3:35 PM
>>> To: Kevin Hilman; Nishanth Menon
>>> Cc: Premi, Sanjeev; Menon, Nishanth; Koen Kooi; linux-
>>
>>
>>> Slightly off the topic but considering we are discussing opp layer here, I
>>> do not find API's
>>> that take the OPP type (OPP_MPU or OPP_L3) and voltage/frequency as
>>> parameters and returns back the associated frequency/voltage. I do not
>>> want to get the opp table entry every time I do this. Any chance of
>>> introducing this in the framework? I could send a patch for the same.
>>Am curios on the usage that combination of
>>opp_find_freq_exact
>>opp_find_freq_ceil
>>opp_find_freq_floor
>>
>>and opp_get_freq, opp_get_voltage is unable to cater to? Would be interested
>>in seeing a patch for the same.

Ok here is the need. Let us suppose I know the opp type (OPP_MPU, OPP_L3 ...) etc and the frequency and I need to get the voltage. Today I have to do the following 
	struct omap_opp *opp = opp_find_freq_exact/opp_find_freq_floor/opp_find_freq_ceil(type, freq);
	u32 volt = opp_get_voltage(opp).

As a user of opp layer it would be much simpler for me to do
	opp_get_volt(type, freq, exact/ceil/floor) as  I really do not care about the opp struct at all.

Now let us suppose I have the opp type and the voltage I need to put the vdd associated with it to. Today there is no way to retrieve the frequency from the opp layer.

Again as a user of the opp layer I would like to have an API
	u32 freq = opp_get_freq(type, volt) 

Regards
Thara
>>
>>Regards,
>>Nishanth Menon

^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: opp layer query apis (was RE: omap3 pm: dependency between opp layer and cpufreq)
  2010-06-08  4:29                             ` Gopinath, Thara
@ 2010-06-08  4:35                               ` Menon, Nishanth
  0 siblings, 0 replies; 20+ messages in thread
From: Menon, Nishanth @ 2010-06-08  4:35 UTC (permalink / raw)
  To: Gopinath, Thara, Kevin Hilman, Nishanth Menon
  Cc: Premi, Sanjeev, Koen Kooi, linux-omap, eduardo.valentin

> -----Original Message-----
> From: Gopinath, Thara
> Sent: Tuesday, June 08, 2010 7:29 AM
> >>-----Original Message-----
> >>From: Menon, Nishanth
> >>Sent: Tuesday, June 08, 2010 9:45 AM
> >>> -----Original Message-----
> >>> From: Gopinath, Thara
> >>> Sent: Friday, June 04, 2010 3:35 PM
> >>
> >>> Slightly off the topic but considering we are discussing opp layer
> here, I
> >>> do not find API's
> >>> that take the OPP type (OPP_MPU or OPP_L3) and voltage/frequency as
> >>> parameters and returns back the associated frequency/voltage. I do not
> >>> want to get the opp table entry every time I do this. Any chance of
> >>> introducing this in the framework? I could send a patch for the same.
> >>Am curios on the usage that combination of
> >>opp_find_freq_exact
> >>opp_find_freq_ceil
> >>opp_find_freq_floor
> >>
> >>and opp_get_freq, opp_get_voltage is unable to cater to? Would be
> interested
> >>in seeing a patch for the same.
> 
> Ok here is the need. Let us suppose I know the opp type (OPP_MPU,
> OPP_L3 ...) etc and the frequency and I need to get the voltage. Today I
> have to do the following
> 	struct omap_opp *opp =
> opp_find_freq_exact/opp_find_freq_floor/opp_find_freq_ceil(type, freq);
> 	u32 volt = opp_get_voltage(opp).
> 
> As a user of opp layer it would be much simpler for me to do
> 	opp_get_volt(type, freq, exact/ceil/floor) as  I really do not care
> about the opp struct at all.
The intent was we are not always sure what exact behavior the caller desires.

Option1: (currently implemented)
Opp_get_volt(opp_find_freq_exact/opp_find_freq_floor/opp_find_freq_ceil(type, 
freq)

Option2:
Opp_get_volt_freq_exact(freq);
Opp_get_volt_freq_ceil(freq);
Opp_get_volt_freq_floor(freq);

The intent is if we don't really care about the opp structure, choose the 
nature of the search and plug it straight to get_volt.

> 
> Now let us suppose I have the opp type and the voltage I need to put the
> vdd associated with it to. Today there is no way to retrieve the frequency
> from the opp layer.


Why would we want to do this?

> 
> Again as a user of the opp layer I would like to have an API
> 	u32 freq = opp_get_freq(type, volt)

A voltage could be linked to 1 or more frequencies. OPP layer functions 
based on some sort of unique identifier. We aligned earlier this year(jan if 
I recollect), that frequency is the common unique identifier, is there a 
Reason that this is not good enough?

Regards,
Nishanth Menon

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2010-06-08  4:35 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-05-27 11:25 omap3 pm: dependency between opp layer and cpufreq Premi, Sanjeev
2010-05-27 13:19 ` Premi, Sanjeev
2010-05-27 13:26   ` Premi, Sanjeev
2010-05-27 16:27 ` Nishanth Menon
2010-05-27 20:29   ` Premi, Sanjeev
2010-05-28  7:39     ` Menon, Nishanth
2010-05-28  8:56       ` Koen Kooi
2010-05-28 13:33         ` Nishanth Menon
2010-05-28 13:42           ` Premi, Sanjeev
2010-05-28 13:55             ` Nishanth Menon
2010-05-28 14:02               ` Premi, Sanjeev
     [not found]                 ` <1275065163.10319.5.camel@Nokia-N900-51-1>
2010-05-28 17:57                   ` Kevin Hilman
2010-05-30 10:58                     ` Premi, Sanjeev
2010-05-30 10:50                   ` Premi, Sanjeev
2010-05-31  4:19                     ` Nishanth Menon
2010-06-03  0:00                       ` Kevin Hilman
2010-06-04 12:34                         ` Gopinath, Thara
2010-06-08  4:15                           ` opp layer query apis (was RE: omap3 pm: dependency between opp layer and cpufreq) Menon, Nishanth
2010-06-08  4:29                             ` Gopinath, Thara
2010-06-08  4:35                               ` Menon, Nishanth

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.