| Message ID | 1351765401-12487-1-git-send-email-ivan.khoronzhuk@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thursday 01 November 2012 03:53 PM, Ivan Khoronzhuk wrote: > Replaces several flags bearing the same meaning. There is no need > to set flags due to different omap types here, it can be checked > in appropriate places as well. > > Cc: Tony Lindgren <tony@atomide.com> > Cc: Russell King <linux@arm.linux.org.uk> > Cc: linux-omap@vger.kernel.org > Cc: linux-arm-kernel@lists.infradead.org > Cc: linux-kernel@vger.kernel.org > Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com> > --- > arch/arm/mach-omap2/id.c | 25 +++++++------------------ > arch/arm/mach-omap2/soc.h | 8 ++------ > 2 files changed, 9 insertions(+), 24 deletions(-) > > diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c > index cf2362c..3c47a19 100644 > --- a/arch/arm/mach-omap2/id.c > +++ b/arch/arm/mach-omap2/id.c > @@ -28,6 +28,9 @@ > #include "soc.h" > #include "control.h" > > +#define OMAP4_SILICON_TYPE_STANDARD 0x01 > +#define OMAP4_SILICON_TYPE_PERFORMANCE 0x02 > + > static unsigned int omap_revision; > static const char *cpu_rev; > u32 omap_features; > @@ -273,25 +276,11 @@ void __init omap4xxx_check_features(void) > { > u32 si_type; > > - if (cpu_is_omap443x()) > - omap_features |= OMAP4_HAS_MPU_1GHZ; > - > + si_type = > + (read_tap_reg(OMAP4_CTRL_MODULE_CORE_STD_FUSE_PROD_ID_1) >> 16) & 0x03; > > - if (cpu_is_omap446x()) { > - si_type = > - read_tap_reg(OMAP4_CTRL_MODULE_CORE_STD_FUSE_PROD_ID_1); > - switch ((si_type & (3 << 16)) >> 16) { > - case 2: > - /* High performance device */ > - omap_features |= OMAP4_HAS_MPU_1_5GHZ; > - break; > - case 1: > - default: > - /* Standard device */ > - omap_features |= OMAP4_HAS_MPU_1_2GHZ; > - break; > - } > - } > + if (si_type == OMAP4_SILICON_TYPE_PERFORMANCE) > + omap_features = OMAP4_HAS_PERF_SILICON; Well the detection isn't for performance/standard but there are some features depend on it. For example 1 GHz doesn't DPLL DCC enable feature where as 1.2 GHz, 1.5 GHz doesn't need. This is the main reason this information is also effused. Have you considered this aspect while creating this patch ? Regards Santosh
On 11/01/2012 01:35 PM, Santosh Shilimkar wrote: > On Thursday 01 November 2012 03:53 PM, Ivan Khoronzhuk wrote: >> Replaces several flags bearing the same meaning. There is no need >> to set flags due to different omap types here, it can be checked >> in appropriate places as well. >> >> Cc: Tony Lindgren <tony@atomide.com> >> Cc: Russell King <linux@arm.linux.org.uk> >> Cc: linux-omap@vger.kernel.org >> Cc: linux-arm-kernel@lists.infradead.org >> Cc: linux-kernel@vger.kernel.org >> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com> >> --- >> arch/arm/mach-omap2/id.c | 25 +++++++------------------ >> arch/arm/mach-omap2/soc.h | 8 ++------ >> 2 files changed, 9 insertions(+), 24 deletions(-) >> >> diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c >> index cf2362c..3c47a19 100644 >> --- a/arch/arm/mach-omap2/id.c >> +++ b/arch/arm/mach-omap2/id.c >> @@ -28,6 +28,9 @@ >> #include "soc.h" >> #include "control.h" >> >> +#define OMAP4_SILICON_TYPE_STANDARD 0x01 >> +#define OMAP4_SILICON_TYPE_PERFORMANCE 0x02 >> + >> static unsigned int omap_revision; >> static const char *cpu_rev; >> u32 omap_features; >> @@ -273,25 +276,11 @@ void __init omap4xxx_check_features(void) >> { >> u32 si_type; >> >> - if (cpu_is_omap443x()) >> - omap_features |= OMAP4_HAS_MPU_1GHZ; >> - >> + si_type = >> + (read_tap_reg(OMAP4_CTRL_MODULE_CORE_STD_FUSE_PROD_ID_1) >> 16) >> & 0x03; >> >> - if (cpu_is_omap446x()) { >> - si_type = >> - read_tap_reg(OMAP4_CTRL_MODULE_CORE_STD_FUSE_PROD_ID_1); >> - switch ((si_type & (3 << 16)) >> 16) { >> - case 2: >> - /* High performance device */ >> - omap_features |= OMAP4_HAS_MPU_1_5GHZ; >> - break; >> - case 1: >> - default: >> - /* Standard device */ >> - omap_features |= OMAP4_HAS_MPU_1_2GHZ; >> - break; >> - } >> - } >> + if (si_type == OMAP4_SILICON_TYPE_PERFORMANCE) >> + omap_features = OMAP4_HAS_PERF_SILICON; > > Well the detection isn't for performance/standard but there are some > features depend on it. For example 1 GHz doesn't DPLL DCC enable feature > where as 1.2 GHz, 1.5 GHz doesn't need. This is the main reason this > information is also effused. Have you considered this aspect while > creating this patch ? > > Regards > Santosh > I had considered it. There is no dependency on the features. DCC usage depends on asked frequency on the fly, not from the features. Depending on "performance/standard" feature the available frequencies should be chosen in places where they are needed, for example while initializing OPPs. Currently we have several features while it is only one indeed.
On Thursday 01 November 2012 09:50 PM, ivan.khoronzhuk wrote: > On 11/01/2012 01:35 PM, Santosh Shilimkar wrote: >> On Thursday 01 November 2012 03:53 PM, Ivan Khoronzhuk wrote: >>> Replaces several flags bearing the same meaning. There is no need >>> to set flags due to different omap types here, it can be checked >>> in appropriate places as well. >>> >>> Cc: Tony Lindgren <tony@atomide.com> >>> Cc: Russell King <linux@arm.linux.org.uk> >>> Cc: linux-omap@vger.kernel.org >>> Cc: linux-arm-kernel@lists.infradead.org >>> Cc: linux-kernel@vger.kernel.org >>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com> >>> --- >>> arch/arm/mach-omap2/id.c | 25 +++++++------------------ >>> arch/arm/mach-omap2/soc.h | 8 ++------ >>> 2 files changed, 9 insertions(+), 24 deletions(-) >>> >>> diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c >>> index cf2362c..3c47a19 100644 >>> --- a/arch/arm/mach-omap2/id.c >>> +++ b/arch/arm/mach-omap2/id.c >>> @@ -28,6 +28,9 @@ >>> #include "soc.h" >>> #include "control.h" >>> >>> +#define OMAP4_SILICON_TYPE_STANDARD 0x01 >>> +#define OMAP4_SILICON_TYPE_PERFORMANCE 0x02 >>> + >>> static unsigned int omap_revision; >>> static const char *cpu_rev; >>> u32 omap_features; >>> @@ -273,25 +276,11 @@ void __init omap4xxx_check_features(void) >>> { >>> u32 si_type; >>> >>> - if (cpu_is_omap443x()) >>> - omap_features |= OMAP4_HAS_MPU_1GHZ; >>> - >>> + si_type = >>> + (read_tap_reg(OMAP4_CTRL_MODULE_CORE_STD_FUSE_PROD_ID_1) >> 16) >>> & 0x03; >>> >>> - if (cpu_is_omap446x()) { >>> - si_type = >>> - read_tap_reg(OMAP4_CTRL_MODULE_CORE_STD_FUSE_PROD_ID_1); >>> - switch ((si_type & (3 << 16)) >> 16) { >>> - case 2: >>> - /* High performance device */ >>> - omap_features |= OMAP4_HAS_MPU_1_5GHZ; >>> - break; >>> - case 1: >>> - default: >>> - /* Standard device */ >>> - omap_features |= OMAP4_HAS_MPU_1_2GHZ; >>> - break; >>> - } >>> - } >>> + if (si_type == OMAP4_SILICON_TYPE_PERFORMANCE) >>> + omap_features = OMAP4_HAS_PERF_SILICON; >> >> Well the detection isn't for performance/standard but there are some >> features depend on it. For example 1 GHz doesn't DPLL DCC enable feature >> where as 1.2 GHz, 1.5 GHz doesn't need. This is the main reason this >> information is also effused. Have you considered this aspect while >> creating this patch ? >> >> Regards >> Santosh >> > > I had considered it. There is no dependency on the features. > DCC usage depends on asked frequency on the fly, not from the features. > Depending on "performance/standard" feature the available frequencies > should be chosen in places where they are needed, for example while > initializing OPPs. > You are correct about the way DCC is handled in the clock code. Infact all these features like L2CACHE, SGX, IVA etc is more for to display boot messages. > Currently we have several features while it is only one indeed. > 1GHz, 1.2GHz, 1.5 GHz are not same since the silicon capability itself is different. git blame tells me that Nishant has sent this update so looping him if above differentiation in boot log helps. Nishant, What's your take on removing above freq prints and marking those silicon as performance silicons as the $subject patch does ? Regards Santosh
On 22:05-20121101, Santosh Shilimkar wrote: > On Thursday 01 November 2012 09:50 PM, ivan.khoronzhuk wrote: > >On 11/01/2012 01:35 PM, Santosh Shilimkar wrote: > >>On Thursday 01 November 2012 03:53 PM, Ivan Khoronzhuk wrote: > >>>Replaces several flags bearing the same meaning. There is no need > >>>to set flags due to different omap types here, it can be checked > >>>in appropriate places as well. > >>> > >>>Cc: Tony Lindgren <tony@atomide.com> > >>>Cc: Russell King <linux@arm.linux.org.uk> > >>>Cc: linux-omap@vger.kernel.org > >>>Cc: linux-arm-kernel@lists.infradead.org > >>>Cc: linux-kernel@vger.kernel.org > >>>Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com> > >>>--- > >>> arch/arm/mach-omap2/id.c | 25 +++++++------------------ > >>> arch/arm/mach-omap2/soc.h | 8 ++------ > >>> 2 files changed, 9 insertions(+), 24 deletions(-) > >>> > >>>diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c > >>>index cf2362c..3c47a19 100644 > >>>--- a/arch/arm/mach-omap2/id.c > >>>+++ b/arch/arm/mach-omap2/id.c > >>>@@ -28,6 +28,9 @@ > >>> #include "soc.h" > >>> #include "control.h" > >>> > >>>+#define OMAP4_SILICON_TYPE_STANDARD 0x01 > >>>+#define OMAP4_SILICON_TYPE_PERFORMANCE 0x02 > >>>+ > >>> static unsigned int omap_revision; > >>> static const char *cpu_rev; > >>> u32 omap_features; > >>>@@ -273,25 +276,11 @@ void __init omap4xxx_check_features(void) > >>> { > >>> u32 si_type; > >>> > >>>- if (cpu_is_omap443x()) > >>>- omap_features |= OMAP4_HAS_MPU_1GHZ; > >>>- > >>>+ si_type = > >>>+ (read_tap_reg(OMAP4_CTRL_MODULE_CORE_STD_FUSE_PROD_ID_1) >> 16) > >>>& 0x03; > >>> > >>>- if (cpu_is_omap446x()) { > >>>- si_type = > >>>- read_tap_reg(OMAP4_CTRL_MODULE_CORE_STD_FUSE_PROD_ID_1); > >>>- switch ((si_type & (3 << 16)) >> 16) { > >>>- case 2: > >>>- /* High performance device */ > >>>- omap_features |= OMAP4_HAS_MPU_1_5GHZ; > >>>- break; > >>>- case 1: > >>>- default: > >>>- /* Standard device */ > >>>- omap_features |= OMAP4_HAS_MPU_1_2GHZ; > >>>- break; > >>>- } > >>>- } > >>>+ if (si_type == OMAP4_SILICON_TYPE_PERFORMANCE) > >>>+ omap_features = OMAP4_HAS_PERF_SILICON; > >> > >>Well the detection isn't for performance/standard but there are some > >>features depend on it. For example 1 GHz doesn't DPLL DCC enable feature > >>where as 1.2 GHz, 1.5 GHz doesn't need. This is the main reason this > >>information is also effused. Have you considered this aspect while > >>creating this patch ? > >> > >>Regards > >>Santosh > >> > > > >I had considered it. There is no dependency on the features. > >DCC usage depends on asked frequency on the fly, not from the features. > >Depending on "performance/standard" feature the available frequencies > >should be chosen in places where they are needed, for example while > >initializing OPPs. > > > You are correct about the way DCC is handled in the clock code. Infact > all these features like L2CACHE, SGX, IVA etc is more for to display > boot messages. > > >Currently we have several features while it is only one indeed. > > > 1GHz, 1.2GHz, 1.5 GHz are not same since the silicon capability itself > is different. > > git blame tells me that Nishant has sent this update so looping him > if above differentiation in boot log helps. > > Nishant, > What's your take on removing above freq prints and marking > those silicon as performance silicons as the $subject patch does ? > > Regards > Santosh Yes $subject patch is a better approach compared to having freq based handling which just increases the number of macros we need to enable depending on SoC variants that we spin off the main SoC. This also allows us to conserve the features bitfield ahead as well. I hate to admit, but after a couple of generations of SoC spinoffs, my original logic is proving to be was pretty short sighted, unfortunately :( So, approach Acked-by: Nishanth Menon <nm@ti.com>
On Thursday 01 November 2012 10:36 PM, Nishanth Menon wrote: > On 22:05-20121101, Santosh Shilimkar wrote: >> On Thursday 01 November 2012 09:50 PM, ivan.khoronzhuk wrote: >>> On 11/01/2012 01:35 PM, Santosh Shilimkar wrote: >>>> On Thursday 01 November 2012 03:53 PM, Ivan Khoronzhuk wrote: >>>>> Replaces several flags bearing the same meaning. There is no need >>>>> to set flags due to different omap types here, it can be checked >>>>> in appropriate places as well. >>>>> >>>>> Cc: Tony Lindgren <tony@atomide.com> >>>>> Cc: Russell King <linux@arm.linux.org.uk> >>>>> Cc: linux-omap@vger.kernel.org >>>>> Cc: linux-arm-kernel@lists.infradead.org >>>>> Cc: linux-kernel@vger.kernel.org >>>>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com> >>>>> --- [..] >>>>> + if (si_type == OMAP4_SILICON_TYPE_PERFORMANCE) >>>>> + omap_features = OMAP4_HAS_PERF_SILICON; >>>> >>>> Well the detection isn't for performance/standard but there are some >>>> features depend on it. For example 1 GHz doesn't DPLL DCC enable feature >>>> where as 1.2 GHz, 1.5 GHz doesn't need. This is the main reason this >>>> information is also effused. Have you considered this aspect while >>>> creating this patch ? >>>> >>>> Regards >>>> Santosh >>>> >>> >>> I had considered it. There is no dependency on the features. >>> DCC usage depends on asked frequency on the fly, not from the features. >>> Depending on "performance/standard" feature the available frequencies >>> should be chosen in places where they are needed, for example while >>> initializing OPPs. >>> >> You are correct about the way DCC is handled in the clock code. Infact >> all these features like L2CACHE, SGX, IVA etc is more for to display >> boot messages. >> >>> Currently we have several features while it is only one indeed. >>> >> 1GHz, 1.2GHz, 1.5 GHz are not same since the silicon capability itself >> is different. >> >> git blame tells me that Nishant has sent this update so looping him >> if above differentiation in boot log helps. >> >> Nishant, >> What's your take on removing above freq prints and marking >> those silicon as performance silicons as the $subject patch does ? >> >> Regards >> Santosh > Yes $subject patch is a better approach compared to having freq based > handling which just increases the number of macros we need to enable > depending on SoC variants that we spin off the main SoC. This also > allows us to conserve the features bitfield ahead as well. > > I hate to admit, but after a couple of generations of SoC spinoffs, > my original logic is proving to be was pretty short sighted, > unfortunately :( > > So, approach > Acked-by: Nishanth Menon <nm@ti.com> > Thanks Nishant for clarification and ack. With the clarification I also like the subject patch. Feel free add. Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c index cf2362c..3c47a19 100644 --- a/arch/arm/mach-omap2/id.c +++ b/arch/arm/mach-omap2/id.c @@ -28,6 +28,9 @@ #include "soc.h" #include "control.h" +#define OMAP4_SILICON_TYPE_STANDARD 0x01 +#define OMAP4_SILICON_TYPE_PERFORMANCE 0x02 + static unsigned int omap_revision; static const char *cpu_rev; u32 omap_features; @@ -273,25 +276,11 @@ void __init omap4xxx_check_features(void) { u32 si_type; - if (cpu_is_omap443x()) - omap_features |= OMAP4_HAS_MPU_1GHZ; - + si_type = + (read_tap_reg(OMAP4_CTRL_MODULE_CORE_STD_FUSE_PROD_ID_1) >> 16) & 0x03; - if (cpu_is_omap446x()) { - si_type = - read_tap_reg(OMAP4_CTRL_MODULE_CORE_STD_FUSE_PROD_ID_1); - switch ((si_type & (3 << 16)) >> 16) { - case 2: - /* High performance device */ - omap_features |= OMAP4_HAS_MPU_1_5GHZ; - break; - case 1: - default: - /* Standard device */ - omap_features |= OMAP4_HAS_MPU_1_2GHZ; - break; - } - } + if (si_type == OMAP4_SILICON_TYPE_PERFORMANCE) + omap_features = OMAP4_HAS_PERF_SILICON; } void __init ti81xx_check_features(void) diff --git a/arch/arm/mach-omap2/soc.h b/arch/arm/mach-omap2/soc.h index 0700964..f31d907 100644 --- a/arch/arm/mach-omap2/soc.h +++ b/arch/arm/mach-omap2/soc.h @@ -435,9 +435,7 @@ extern u32 omap_features; #define OMAP3_HAS_IO_WAKEUP BIT(6) #define OMAP3_HAS_SDRC BIT(7) #define OMAP3_HAS_IO_CHAIN_CTRL BIT(8) -#define OMAP4_HAS_MPU_1GHZ BIT(9) -#define OMAP4_HAS_MPU_1_2GHZ BIT(10) -#define OMAP4_HAS_MPU_1_5GHZ BIT(11) +#define OMAP4_HAS_PERF_SILICON BIT(9) #define OMAP3_HAS_FEATURE(feat,flag) \ @@ -465,9 +463,7 @@ static inline unsigned int omap4_has_ ##feat(void) \ return omap_features & OMAP4_HAS_ ##flag; \ } \ -OMAP4_HAS_FEATURE(mpu_1ghz, MPU_1GHZ) -OMAP4_HAS_FEATURE(mpu_1_2ghz, MPU_1_2GHZ) -OMAP4_HAS_FEATURE(mpu_1_5ghz, MPU_1_5GHZ) +OMAP4_HAS_FEATURE(perf_silicon, PERF_SILICON) #endif /* __ASSEMBLY__ */
Replaces several flags bearing the same meaning. There is no need to set flags due to different omap types here, it can be checked in appropriate places as well. Cc: Tony Lindgren <tony@atomide.com> Cc: Russell King <linux@arm.linux.org.uk> Cc: linux-omap@vger.kernel.org Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com> --- arch/arm/mach-omap2/id.c | 25 +++++++------------------ arch/arm/mach-omap2/soc.h | 8 ++------ 2 files changed, 9 insertions(+), 24 deletions(-)