diff mbox

[RFC,1/4] ARM: OMAP2: DRA7: Modify optimize string comparisons in soc_is calls

Message ID 1441189213-8199-2-git-send-email-j-keerthy@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

J, KEERTHY Sept. 2, 2015, 10:20 a.m. UTC
Currently everytime soc_is calls are made, firstly device tree nodes
are parsed and then string comparisons are made to determine the
soc version. Optimizing it to be done one time and store the result.
Use the stored value in all the subsequent checks for soc_is calls.

Signed-off-by: Keerthy <j-keerthy@ti.com>
---
 arch/arm/mach-omap2/id.c  | 18 ++++++++++++++++++
 arch/arm/mach-omap2/soc.h | 15 ++++++++++++---
 2 files changed, 30 insertions(+), 3 deletions(-)

Comments

Felipe Balbi Sept. 2, 2015, 12:31 p.m. UTC | #1
On Wed, Sep 02, 2015 at 03:50:10PM +0530, Keerthy wrote:
> Currently everytime soc_is calls are made, firstly device tree nodes
> are parsed and then string comparisons are made to determine the
> soc version. Optimizing it to be done one time and store the result.

but is that really a problem ? Are soc_is* calls made in any critical
path ? Please provide some benchmarking data.
Tony Lindgren Sept. 3, 2015, 3:28 p.m. UTC | #2
* Felipe Balbi <balbi@ti.com> [150902 05:34]:
> On Wed, Sep 02, 2015 at 03:50:10PM +0530, Keerthy wrote:
> > Currently everytime soc_is calls are made, firstly device tree nodes
> > are parsed and then string comparisons are made to determine the
> > soc version. Optimizing it to be done one time and store the result.
> 
> but is that really a problem ? Are soc_is* calls made in any critical
> path ? Please provide some benchmarking data.

Regarding the SoC detection, there are multiple issues to consider here:

1. We want to unify the SoC detection so it behaves the same way
   for all SoCs

2. We don't want the dts files to explode with boards having
   multiple SoC revisions. So SoC detection and features
   detection for SGX and DSP etc should be done after setting
   the major SoC class based on compatible value

3. We don't want to do pointless string comparison over and
   over

Regards,

Tony
--
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
Tony Lindgren Sept. 3, 2015, 4:15 p.m. UTC | #3
* Keerthy <j-keerthy@ti.com> [150902 03:23]:
> Currently everytime soc_is calls are made, firstly device tree nodes
> are parsed and then string comparisons are made to determine the
> soc version. Optimizing it to be done one time and store the result.
> Use the stored value in all the subsequent checks for soc_is calls.
...

> --- a/arch/arm/mach-omap2/id.c
> +++ b/arch/arm/mach-omap2/id.c
> @@ -44,12 +44,29 @@ static char soc_name[OMAP_SOC_MAX_NAME_LENGTH];
>  static char soc_rev[OMAP_SOC_MAX_NAME_LENGTH];
>  u32 omap_features;
>  
> +static int soc_ids[MAX_ID];

Hmm why would we need to keep an array of the non-booted
socs? We know they're all false after booting..

A bitmask is usually used here?

Regards,

Tony
--
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
Keerthy Sept. 4, 2015, 4:09 a.m. UTC | #4
On Thursday 03 September 2015 09:45 PM, Tony Lindgren wrote:
> * Keerthy <j-keerthy@ti.com> [150902 03:23]:
>> Currently everytime soc_is calls are made, firstly device tree nodes
>> are parsed and then string comparisons are made to determine the
>> soc version. Optimizing it to be done one time and store the result.
>> Use the stored value in all the subsequent checks for soc_is calls.
> ...
>
>> --- a/arch/arm/mach-omap2/id.c
>> +++ b/arch/arm/mach-omap2/id.c
>> @@ -44,12 +44,29 @@ static char soc_name[OMAP_SOC_MAX_NAME_LENGTH];
>>   static char soc_rev[OMAP_SOC_MAX_NAME_LENGTH];
>>   u32 omap_features;
>>
>> +static int soc_ids[MAX_ID];
>
> Hmm why would we need to keep an array of the non-booted
> socs? We know they're all false after booting..
>
> A bitmask is usually used here?

Yes. This will be more optimal. I will implement using a bit mask.

>
> Regards,
>
> Tony
> --
> 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
>
--
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
diff mbox

Patch

diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c
index e3f713f..19289df 100644
--- a/arch/arm/mach-omap2/id.c
+++ b/arch/arm/mach-omap2/id.c
@@ -44,12 +44,29 @@  static char soc_name[OMAP_SOC_MAX_NAME_LENGTH];
 static char soc_rev[OMAP_SOC_MAX_NAME_LENGTH];
 u32 omap_features;
 
+static int soc_ids[MAX_ID];
+
 unsigned int omap_rev(void)
 {
 	return omap_revision;
 }
 EXPORT_SYMBOL(omap_rev);
 
+void init_dra_soc_id(void)
+{
+	if (of_machine_is_compatible("ti,dra7"))
+		soc_ids[DRA7XX] = 1;
+	if (of_machine_is_compatible("ti,dra74"))
+		soc_ids[DRA74X] = 1;
+	if (of_machine_is_compatible("ti,dra72"))
+		soc_ids[DRA72X] = 1;
+}
+
+int check_dra_soc(int id)
+{
+	return soc_ids[id];
+}
+
 int omap_type(void)
 {
 	static u32 val = OMAP2_DEVICETYPE_MASK;
@@ -643,6 +660,7 @@  void __init dra7xxx_check_revision(void)
 	u16 hawkeye;
 	u8 rev;
 
+	init_dra_soc_id();
 	idcode = read_tap_reg(OMAP_TAP_IDCODE);
 	hawkeye = (idcode >> 12) & 0xffff;
 	rev = (idcode >> 28) & 0xff;
diff --git a/arch/arm/mach-omap2/soc.h b/arch/arm/mach-omap2/soc.h
index f97654d..cd10260 100644
--- a/arch/arm/mach-omap2/soc.h
+++ b/arch/arm/mach-omap2/soc.h
@@ -38,6 +38,15 @@ 
 #include <linux/bitops.h>
 #include <linux/of.h>
 
+enum soc_id_flags {
+	DRA7XX = 0,
+	DRA74X,
+	DRA72X,
+	MAX_ID,
+};
+
+int check_dra_soc(int id);
+
 /*
  * Test if multicore OMAP support is needed
  */
@@ -397,9 +406,9 @@  IS_OMAP_TYPE(3430, 0x3430)
 #undef soc_is_dra7xx
 #undef soc_is_dra74x
 #undef soc_is_dra72x
-#define soc_is_dra7xx()	(of_machine_is_compatible("ti,dra7"))
-#define soc_is_dra74x()	(of_machine_is_compatible("ti,dra74"))
-#define soc_is_dra72x()	(of_machine_is_compatible("ti,dra72"))
+#define soc_is_dra7xx()			check_dra_soc(DRA7XX)
+#define soc_is_dra74x()			check_dra_soc(DRA74X)
+#define soc_is_dra72x()			check_dra_soc(DRA72X)
 #endif
 
 /* Various silicon revisions for omap2 */