[PATCH] Runtime detection of Si features

[PATCH] Runtime detection of Si features

[Sanjeev Premi]

The OMAP35x family has multiple variants differing
in the HW features. This patch detects these features
at runtime and prints information during the boot.

Since most of the code seemed repetitive, macros
have been used for readability.

Signed-off-by: Sanjeev Premi <premi@ti.com>
---
 arch/arm/mach-omap2/id.c |  106 ++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 102 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c
index a98201c..4476491 100644
--- a/arch/arm/mach-omap2/id.c
+++ b/arch/arm/mach-omap2/id.c
@@ -25,9 +25,49 @@
 #include <mach/control.h>
 #include <mach/cpu.h>
 
+/*
+ * OMAP3 features
+ */
+#define OMAP3_CONTROL_OMAP_STATUS	0x044c
+
+#define OMAP3_SGX_SHIFT			13
+#define OMAP3_SGX_MASK			(3 << OMAP3_SGX_SHIFT)
+#define		FEAT_SGX_FULL		0
+#define		FEAT_SGX_HALF		1
+#define		FEAT_SGX_NONE		2
+
+#define OMAP3_IVA_SHIFT			12
+#define OMAP3_IVA_MASK			(1 << OMAP3_SGX_SHIFT)
+#define		FEAT_IVA		0
+#define		FEAT_IVA_NONE		1
+
+#define OMAP3_L2CACHE_SHIFT		10
+#define OMAP3_L2CACHE_MASK		(3 << OMAP3_L2CACHE_SHIFT)
+#define		FEAT_L2CACHE_NONE	0
+#define		FEAT_L2CACHE_64KB	1
+#define		FEAT_L2CACHE_128KB	2
+#define		FEAT_L2CACHE_256KB	3
+
+#define OMAP3_ISP_SHIFT			5
+#define OMAP3_ISP_MASK			(1<< OMAP3_ISP_SHIFT)
+#define		FEAT_ISP		0
+#define		FEAT_ISP_NONE		1
+
+#define OMAP3_NEON_SHIFT		4
+#define OMAP3_NEON_MASK			(1<< OMAP3_NEON_SHIFT)
+#define		FEAT_NEON		0
+#define		FEAT_NEON_NONE		1
+
+
+#define OMAP_HAS_L2CACHE		1
+#define OMAP_HAS_IVA			(1 << 1)
+#define OMAP_HAS_SGX			(1 << 2)
+#define OMAP_HAS_NEON			(1 << 3)
+#define OMAP_HAS_ISP			(1 << 4)
+
 static struct omap_chip_id omap_chip;
 static unsigned int omap_revision;
-
+static u32 omap_features ;
 
 unsigned int omap_rev(void)
 {
@@ -35,6 +75,19 @@ unsigned int omap_rev(void)
 }
 EXPORT_SYMBOL(omap_rev);
 
+#define OMAP3_HAS_FEATURE(feat,flag)				\
+	unsigned int omap3_has_ ##feat(void)			\
+	{							\
+		return (omap_features & OMAP_HAS_ ##flag);	\
+	}							\
+	EXPORT_SYMBOL(omap3_has_ ##feat);
+
+OMAP3_HAS_FEATURE(l2cache, L2CACHE);
+OMAP3_HAS_FEATURE(sgx, SGX);
+OMAP3_HAS_FEATURE(iva, IVA);
+OMAP3_HAS_FEATURE(neon, NEON);
+OMAP3_HAS_FEATURE(isp, ISP);
+
 /**
  * omap_chip_is - test whether currently running OMAP matches a chip type
  * @oc: omap_chip_t to test against
@@ -155,7 +208,33 @@ void __init omap24xx_check_revision(void)
 	pr_info("\n");
 }
 
-void __init omap34xx_check_revision(void)
+#define OMAP3_CHECK_FEATURE(status,feat)				\
+	if (((status & OMAP3_ ##feat## _MASK) 				\
+		>> OMAP3_ ##feat## _SHIFT) != FEAT_ ##feat## _NONE) { 	\
+		omap_features |= OMAP_HAS_ ##feat;			\
+	}
+
+void __init omap3_check_features(void)
+{
+	u32 status, temp;
+
+	omap_features = 0;
+
+	status = omap_ctrl_readl(OMAP3_CONTROL_OMAP_STATUS);
+
+	OMAP3_CHECK_FEATURE(status, L2CACHE);
+	OMAP3_CHECK_FEATURE(status, IVA);
+	OMAP3_CHECK_FEATURE(status, SGX);
+	OMAP3_CHECK_FEATURE(status, NEON);
+	OMAP3_CHECK_FEATURE(status, ISP);
+
+	/*
+	 * TODO: Get additional info (where applicable)
+	 *       e.g. Size of L2 cache.
+	 */
+}
+
+void __init omap3_check_revision(void)
 {
 	u32 cpuid, idcode;
 	u16 hawkeye;
@@ -212,6 +291,22 @@ out:
 	pr_info("OMAP%04x %s\n", omap_rev() >> 16, rev_name);
 }
 
+#define OMAP3_SHOW_FEATURE(feat)		\
+	if (omap3_has_ ##feat()) {		\
+		pr_info (" - "#feat" : Y");	\
+	} else {				\
+		pr_info (" - "#feat" : N");	\
+	}
+
+void __init omap3_cpuinfo(void)
+{
+	OMAP3_SHOW_FEATURE(l2cache);
+	OMAP3_SHOW_FEATURE(iva);
+	OMAP3_SHOW_FEATURE(sgx);
+	OMAP3_SHOW_FEATURE(neon);
+	OMAP3_SHOW_FEATURE(isp);
+}
+
 /*
  * Try to detect the exact revision of the omap we're running on
  */
@@ -223,8 +318,11 @@ void __init omap2_check_revision(void)
 	 */
 	if (cpu_is_omap24xx())
 		omap24xx_check_revision();
-	else if (cpu_is_omap34xx())
-		omap34xx_check_revision();
+	else if (cpu_is_omap34xx()) {
+		omap3_check_features();
+		omap3_check_revision();
+		omap3_cpuinfo();
+	}
 	else if (cpu_is_omap44xx()) {
 		printk(KERN_INFO "FIXME: CPU revision = OMAP4430\n");
 		return;
-- 
1.6.2.2

Re: [PATCH] Runtime detection of Si features

[Kevin Hilman]

Sanjeev Premi <premi@ti.com> writes:

> The OMAP35x family has multiple variants differing
> in the HW features. This patch detects these features
> at runtime and prints information during the boot.
>
> Since most of the code seemed repetitive, macros
> have been used for readability.
>
> Signed-off-by: Sanjeev Premi <premi@ti.com>

I like the feature-based approach.

A couple questions though.  Is there a bit/register that reports the
collapsed powerdomains of the devices with modified PRCM?

Also, how will other code query the features?  You're currently
exporting the omap_has_*() functions, but there are no prototypes.

I think I'd rather see a static inline functions in <mach/cpu.h>
for checking features.  Comments to that end inlined below...

> ---
>  arch/arm/mach-omap2/id.c |  106 ++++++++++++++++++++++++++++++++++++++++++++--
>  1 files changed, 102 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c
> index a98201c..4476491 100644
> --- a/arch/arm/mach-omap2/id.c
> +++ b/arch/arm/mach-omap2/id.c
> @@ -25,9 +25,49 @@
>  #include <mach/control.h>
>  #include <mach/cpu.h>
>  
> +/*
> + * OMAP3 features
> + */
> +#define OMAP3_CONTROL_OMAP_STATUS	0x044c

This should probably go in <mach/control.h>

> +#define OMAP3_SGX_SHIFT			13
> +#define OMAP3_SGX_MASK			(3 << OMAP3_SGX_SHIFT)
> +#define		FEAT_SGX_FULL		0
> +#define		FEAT_SGX_HALF		1
> +#define		FEAT_SGX_NONE		2
>
> +#define OMAP3_IVA_SHIFT			12
> +#define OMAP3_IVA_MASK			(1 << OMAP3_SGX_SHIFT)
> +#define		FEAT_IVA		0
> +#define		FEAT_IVA_NONE		1
> +
> +#define OMAP3_L2CACHE_SHIFT		10
> +#define OMAP3_L2CACHE_MASK		(3 << OMAP3_L2CACHE_SHIFT)
> +#define		FEAT_L2CACHE_NONE	0
> +#define		FEAT_L2CACHE_64KB	1
> +#define		FEAT_L2CACHE_128KB	2
> +#define		FEAT_L2CACHE_256KB	3
> +
> +#define OMAP3_ISP_SHIFT			5
> +#define OMAP3_ISP_MASK			(1<< OMAP3_ISP_SHIFT)
> +#define		FEAT_ISP		0
> +#define		FEAT_ISP_NONE		1
> +
> +#define OMAP3_NEON_SHIFT		4
> +#define OMAP3_NEON_MASK			(1<< OMAP3_NEON_SHIFT)
> +#define		FEAT_NEON		0
> +#define		FEAT_NEON_NONE		1
> +
> +
> +#define OMAP_HAS_L2CACHE		1
> +#define OMAP_HAS_IVA			(1 << 1)
> +#define OMAP_HAS_SGX			(1 << 2)
> +#define OMAP_HAS_NEON			(1 << 3)
> +#define OMAP_HAS_ISP			(1 << 4)
> +

Use BIT()
Rename to OMAP3_*
move to <mach/cpu.h>

>  static struct omap_chip_id omap_chip;
>  static unsigned int omap_revision;
> -
> +static u32 omap_features ;

Call this omap3_features and make it global (with extern in <mach/cpu.h>)

>  unsigned int omap_rev(void)
>  {
> @@ -35,6 +75,19 @@ unsigned int omap_rev(void)
>  }
>  EXPORT_SYMBOL(omap_rev);
>  
> +#define OMAP3_HAS_FEATURE(feat,flag)				\
> +	unsigned int omap3_has_ ##feat(void)			\

make this 
	static inline unsigned int omap3_has_ ##feat(void) ...


> +	{							\
> +		return (omap_features & OMAP_HAS_ ##flag);	\
> +	}							\
> +	EXPORT_SYMBOL(omap3_has_ ##feat);
>
> +OMAP3_HAS_FEATURE(l2cache, L2CACHE);
> +OMAP3_HAS_FEATURE(sgx, SGX);
> +OMAP3_HAS_FEATURE(iva, IVA);
> +OMAP3_HAS_FEATURE(neon, NEON);
> +OMAP3_HAS_FEATURE(isp, ISP);
> +

... and move this set to <mach/cpu.h>

and I'm ok with the rest of this patch.

Kevin

>  /**
>   * omap_chip_is - test whether currently running OMAP matches a chip type
>   * @oc: omap_chip_t to test against
> @@ -155,7 +208,33 @@ void __init omap24xx_check_revision(void)
>  	pr_info("\n");
>  }
>  
> -void __init omap34xx_check_revision(void)
> +#define OMAP3_CHECK_FEATURE(status,feat)				\
> +	if (((status & OMAP3_ ##feat## _MASK) 				\
> +		>> OMAP3_ ##feat## _SHIFT) != FEAT_ ##feat## _NONE) { 	\
> +		omap_features |= OMAP_HAS_ ##feat;			\
> +	}
> +
> +void __init omap3_check_features(void)
> +{
> +	u32 status, temp;
> +
> +	omap_features = 0;
> +
> +	status = omap_ctrl_readl(OMAP3_CONTROL_OMAP_STATUS);
> +
> +	OMAP3_CHECK_FEATURE(status, L2CACHE);
> +	OMAP3_CHECK_FEATURE(status, IVA);
> +	OMAP3_CHECK_FEATURE(status, SGX);
> +	OMAP3_CHECK_FEATURE(status, NEON);
> +	OMAP3_CHECK_FEATURE(status, ISP);
> +
> +	/*
> +	 * TODO: Get additional info (where applicable)
> +	 *       e.g. Size of L2 cache.
> +	 */
> +}
> +
> +void __init omap3_check_revision(void)
>  {
>  	u32 cpuid, idcode;
>  	u16 hawkeye;
> @@ -212,6 +291,22 @@ out:
>  	pr_info("OMAP%04x %s\n", omap_rev() >> 16, rev_name);
>  }
>  
> +#define OMAP3_SHOW_FEATURE(feat)		\
> +	if (omap3_has_ ##feat()) {		\
> +		pr_info (" - "#feat" : Y");	\
> +	} else {				\
> +		pr_info (" - "#feat" : N");	\
> +	}
> +
> +void __init omap3_cpuinfo(void)
> +{
> +	OMAP3_SHOW_FEATURE(l2cache);
> +	OMAP3_SHOW_FEATURE(iva);
> +	OMAP3_SHOW_FEATURE(sgx);
> +	OMAP3_SHOW_FEATURE(neon);
> +	OMAP3_SHOW_FEATURE(isp);
> +}
> +
>  /*
>   * Try to detect the exact revision of the omap we're running on
>   */
> @@ -223,8 +318,11 @@ void __init omap2_check_revision(void)
>  	 */
>  	if (cpu_is_omap24xx())
>  		omap24xx_check_revision();
> -	else if (cpu_is_omap34xx())
> -		omap34xx_check_revision();
> +	else if (cpu_is_omap34xx()) {
> +		omap3_check_features();
> +		omap3_check_revision();
> +		omap3_cpuinfo();
> +	}
>  	else if (cpu_is_omap44xx()) {
>  		printk(KERN_INFO "FIXME: CPU revision = OMAP4430\n");
>  		return;
> -- 
> 1.6.2.2
>
> --
> 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

Re: [PATCH] Runtime detection of Si features

[Nishanth Menon]

Kevin Hilman had written, on 08/13/2009 11:13 AM, the following:
> Sanjeev Premi <premi@ti.com> writes:
> 
>> The OMAP35x family has multiple variants differing
>> in the HW features. This patch detects these features
>> at runtime and prints information during the boot.
>>
>> Since most of the code seemed repetitive, macros
>> have been used for readability.
>>
>> Signed-off-by: Sanjeev Premi <premi@ti.com>
> 
> I like the feature-based approach.
> 
> A couple questions though.  Is there a bit/register that reports the
> collapsed powerdomains of the devices with modified PRCM?
> 
> Also, how will other code query the features?  You're currently
> exporting the omap_has_*() functions, but there are no prototypes.
> 
> I think I'd rather see a static inline functions in <mach/cpu.h>
> for checking features.  Comments to that end inlined below...
Wonder if we can setup some sort of infrastructure for:
a) features
b) erratas
linked to OMAP revs + even better w.r.t silicon module(SGX,I2c) 
revisions since at times they are used across multiple OMAPs?

-- 
Regards,
Nishanth Menon

RE: [PATCH] Runtime detection of Si features

[Pandita, Vikram]

>-----Original Message-----
>From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of
>Nishanth Menon
>Sent: Thursday, August 13, 2009 11:31 AM
>To: Kevin Hilman
>Cc: Premi, Sanjeev; linux-omap@vger.kernel.org
>Subject: Re: [PATCH] Runtime detection of Si features
>
>Kevin Hilman had written, on 08/13/2009 11:13 AM, the following:
>> Sanjeev Premi <premi@ti.com> writes:
>>
>>> The OMAP35x family has multiple variants differing
>>> in the HW features. This patch detects these features
>>> at runtime and prints information during the boot.
>>>
>>> Since most of the code seemed repetitive, macros
>>> have been used for readability.
>>>
>>> Signed-off-by: Sanjeev Premi <premi@ti.com>
>>
>> I like the feature-based approach.
>>
>> A couple questions though.  Is there a bit/register that reports the
>> collapsed powerdomains of the devices with modified PRCM?
>>
>> Also, how will other code query the features?  You're currently
>> exporting the omap_has_*() functions, but there are no prototypes.
>>
>> I think I'd rather see a static inline functions in <mach/cpu.h>
>> for checking features.  Comments to that end inlined below...
>Wonder if we can setup some sort of infrastructure for:
>a) features
>b) erratas
>linked to OMAP revs + even better w.r.t silicon module(SGX,I2c)
>revisions since at times they are used across multiple OMAPs?

We are hitting exactly this issue with I2C errata 1.153
Instead of basing the errata check on cpu_is...(), 
its more appropriate to base it on IP revision of I2C.


>
>--
>Regards,
>Nishanth Menon
>--
>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

Re: [PATCH] Runtime detection of Si features

[Kevin Hilman]

"Pandita, Vikram" <vikram.pandita@ti.com> writes:

>>-----Original Message-----
>>From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of
>>Nishanth Menon
>>Sent: Thursday, August 13, 2009 11:31 AM
>>To: Kevin Hilman
>>Cc: Premi, Sanjeev; linux-omap@vger.kernel.org
>>Subject: Re: [PATCH] Runtime detection of Si features
>>
>>Kevin Hilman had written, on 08/13/2009 11:13 AM, the following:
>>> Sanjeev Premi <premi@ti.com> writes:
>>>
>>>> The OMAP35x family has multiple variants differing
>>>> in the HW features. This patch detects these features
>>>> at runtime and prints information during the boot.
>>>>
>>>> Since most of the code seemed repetitive, macros
>>>> have been used for readability.
>>>>
>>>> Signed-off-by: Sanjeev Premi <premi@ti.com>
>>>
>>> I like the feature-based approach.
>>>
>>> A couple questions though.  Is there a bit/register that reports the
>>> collapsed powerdomains of the devices with modified PRCM?
>>>
>>> Also, how will other code query the features?  You're currently
>>> exporting the omap_has_*() functions, but there are no prototypes.
>>>
>>> I think I'd rather see a static inline functions in <mach/cpu.h>
>>> for checking features.  Comments to that end inlined below...
>>Wonder if we can setup some sort of infrastructure for:
>>a) features
>>b) erratas
>>linked to OMAP revs + even better w.r.t silicon module(SGX,I2c)
>>revisions since at times they are used across multiple OMAPs?
>
> We are hitting exactly this issue with I2C errata 1.153
> Instead of basing the errata check on cpu_is...(), 
> its more appropriate to base it on IP revision of I2C.

Shouldn't the IP revision of I2C be avaialble in an I2C revision
register an be used in the driver instead of cpu_is*?

Kevin

Re: [PATCH] Runtime detection of Si features

[Nishanth Menon]

Kevin Hilman had written, on 08/13/2009 11:40 AM, the following:
> "Pandita, Vikram" <vikram.pandita@ti.com> writes:
> 
>>> -----Original Message-----
>>> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of
>>> Sent: Thursday, August 13, 2009 11:31 AM
>>>>> Since most of the code seemed repetitive, macros
>>>>> have been used for readability.
>>>>>
>>>>> Signed-off-by: Sanjeev Premi <premi@ti.com>
>>>> I like the feature-based approach.
>>>>
>>>> A couple questions though.  Is there a bit/register that reports the
>>>> collapsed powerdomains of the devices with modified PRCM?
>>>>
>>>> Also, how will other code query the features?  You're currently
>>>> exporting the omap_has_*() functions, but there are no prototypes.
>>>>
>>>> I think I'd rather see a static inline functions in <mach/cpu.h>
>>>> for checking features.  Comments to that end inlined below...
>>> Wonder if we can setup some sort of infrastructure for:
>>> a) features
>>> b) erratas
>>> linked to OMAP revs + even better w.r.t silicon module(SGX,I2c)
>>> revisions since at times they are used across multiple OMAPs?
>> We are hitting exactly this issue with I2C errata 1.153
>> Instead of basing the errata check on cpu_is...(), 
>> its more appropriate to base it on IP revision of I2C.
> 
> Shouldn't the IP revision of I2C be avaialble in an I2C revision
> register an be used in the driver instead of cpu_is*?
what I was proposing is a much more generic infrastructure which i2c 
among other modules can use. Getting IP revision is already available in 
the specific IP modules REVISION registers - we might want to 
standardize how drivers handle revision based feature/errata set to 
ensure that they would have an optimal way to handle the same.. just my 
2 cents..

-- 
Regards,
Nishanth Menon

Re: [PATCH] Runtime detection of Si features

[Nishanth Menon]

Nishanth Menon had written, on 08/13/2009 11:43 AM, the following:
> Kevin Hilman had written, on 08/13/2009 11:40 AM, the following:
>> "Pandita, Vikram" <vikram.pandita@ti.com> writes:
>>
>>>> -----Original Message-----
>>>> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of
>>>> Sent: Thursday, August 13, 2009 11:31 AM
>>>>>> Since most of the code seemed repetitive, macros
>>>>>> have been used for readability.
>>>>>>
>>>>>> Signed-off-by: Sanjeev Premi <premi@ti.com>
>>>>> I like the feature-based approach.
>>>>>
>>>>> A couple questions though.  Is there a bit/register that reports the
>>>>> collapsed powerdomains of the devices with modified PRCM?
>>>>>
>>>>> Also, how will other code query the features?  You're currently
>>>>> exporting the omap_has_*() functions, but there are no prototypes.
>>>>>
>>>>> I think I'd rather see a static inline functions in <mach/cpu.h>
>>>>> for checking features.  Comments to that end inlined below...
>>>> Wonder if we can setup some sort of infrastructure for:
>>>> a) features
>>>> b) erratas
>>>> linked to OMAP revs + even better w.r.t silicon module(SGX,I2c)
>>>> revisions since at times they are used across multiple OMAPs?
>>> We are hitting exactly this issue with I2C errata 1.153
>>> Instead of basing the errata check on cpu_is...(), 
>>> its more appropriate to base it on IP revision of I2C.
>> Shouldn't the IP revision of I2C be avaialble in an I2C revision
>> register an be used in the driver instead of cpu_is*?
> what I was proposing is a much more generic infrastructure which i2c 
> among other modules can use. Getting IP revision is already available in 
> the specific IP modules REVISION registers - we might want to 
> standardize how drivers handle revision based feature/errata set to 
> ensure that they would have an optimal way to handle the same.. just my 
> 2 cents..
> 
Thinking of this a little more:
driver's smart handling aside, having a sysfs entry to dump the features 
and erratas for each of the modules used is so much nice to have.. 
sigh.. just wondering if anyone has ideas how feasible this might be..

-- 
Regards,
Nishanth Menon

Re: [PATCH] Runtime detection of Si features

[Kevin Hilman]

Nishanth Menon <nm@ti.com> writes:

> Nishanth Menon had written, on 08/13/2009 11:43 AM, the following:
>> Kevin Hilman had written, on 08/13/2009 11:40 AM, the following:
>>> "Pandita, Vikram" <vikram.pandita@ti.com> writes:
>>>
>>>>> -----Original Message-----
>>>>> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of
>>>>> Sent: Thursday, August 13, 2009 11:31 AM
>>>>>>> Since most of the code seemed repetitive, macros
>>>>>>> have been used for readability.
>>>>>>>
>>>>>>> Signed-off-by: Sanjeev Premi <premi@ti.com>
>>>>>> I like the feature-based approach.
>>>>>>
>>>>>> A couple questions though.  Is there a bit/register that reports the
>>>>>> collapsed powerdomains of the devices with modified PRCM?
>>>>>>
>>>>>> Also, how will other code query the features?  You're currently
>>>>>> exporting the omap_has_*() functions, but there are no prototypes.
>>>>>>
>>>>>> I think I'd rather see a static inline functions in <mach/cpu.h>
>>>>>> for checking features.  Comments to that end inlined below...
>>>>> Wonder if we can setup some sort of infrastructure for:
>>>>> a) features
>>>>> b) erratas
>>>>> linked to OMAP revs + even better w.r.t silicon module(SGX,I2c)
>>>>> revisions since at times they are used across multiple OMAPs?
>>>> We are hitting exactly this issue with I2C errata 1.153
>>>> Instead of basing the errata check on cpu_is...(), its more
>>>> appropriate to base it on IP revision of I2C.
>>> Shouldn't the IP revision of I2C be avaialble in an I2C revision
>>> register an be used in the driver instead of cpu_is*?
>> what I was proposing is a much more generic infrastructure which i2c
>> among other modules can use. Getting IP revision is already
>> available in the specific IP modules REVISION registers - we might
>> want to standardize how drivers handle revision based feature/errata
>> set to ensure that they would have an optimal way to handle the
>> same.. just my 2 cents..
>>
> Thinking of this a little more:
> driver's smart handling aside, having a sysfs entry to dump the
> features and erratas for each of the modules used is so much nice to
> have.. sigh.. just wondering if anyone has ideas how feasible this
> might be..

$SUBJECT patch will dump all the chip features during boot, and it
shouldn't be much work too add this to /proc/cpuinfo either.

The errata are another story and should be left to another patch IMHO
since Sanjeev is just tring to get base 35xx support enabled for now.

Kevin

RE: [PATCH] Runtime detection of Si features

[Premi, Sanjeev]

> -----Original Message-----
> From: Kevin Hilman [mailto:khilman@deeprootsystems.com] 
> Sent: Thursday, August 13, 2009 9:43 PM
> To: Premi, Sanjeev
> Cc: linux-omap@vger.kernel.org
> Subject: Re: [PATCH] Runtime detection of Si features
> 
> Sanjeev Premi <premi@ti.com> writes:
> 
> > The OMAP35x family has multiple variants differing
> > in the HW features. This patch detects these features
> > at runtime and prints information during the boot.
> >
> > Since most of the code seemed repetitive, macros
> > have been used for readability.
> >
> > Signed-off-by: Sanjeev Premi <premi@ti.com>
> 
> I like the feature-based approach.
> 
> A couple questions though.  Is there a bit/register that reports the
> collapsed powerdomains of the devices with modified PRCM?

[sp] Unfortunately no. This is the reason, I used cpu_is_omap3505()
and cpu_is_omap3517() in my previous set of patches. This is what I
plan to do here as well; once I can identify the si based on the
features + the hawkeye number.

> 
> Also, how will other code query the features?  You're currently
> exporting the omap_has_*() functions, but there are no prototypes.

I think you missed the definition of macro OMAP3_HAS_FEATURE.
Since most of the code was repetitive, I used this macro for better
Readability.

> 
> I think I'd rather see a static inline functions in <mach/cpu.h>
> for checking features.  Comments to that end inlined below...

Yes. That can be done. With this patch I just wanted to get a view
on the overall approach.

> 
> > ---
> >  arch/arm/mach-omap2/id.c |  106 
> ++++++++++++++++++++++++++++++++++++++++++++--
> >  1 files changed, 102 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c
> > index a98201c..4476491 100644
> > --- a/arch/arm/mach-omap2/id.c
> > +++ b/arch/arm/mach-omap2/id.c
> > @@ -25,9 +25,49 @@
> >  #include <mach/control.h>
> >  #include <mach/cpu.h>
> >  
> > +/*
> > + * OMAP3 features
> > + */
> > +#define OMAP3_CONTROL_OMAP_STATUS	0x044c
> 
> This should probably go in <mach/control.h>
> 

[sp] This was originally in control.h. Just shifted here for this
Patch. Will revert it back.

> > +#define OMAP3_SGX_SHIFT			13
> > +#define OMAP3_SGX_MASK			(3 << OMAP3_SGX_SHIFT)
> > +#define		FEAT_SGX_FULL		0
> > +#define		FEAT_SGX_HALF		1
> > +#define		FEAT_SGX_NONE		2
> >
> > +#define OMAP3_IVA_SHIFT			12
> > +#define OMAP3_IVA_MASK			(1 << OMAP3_SGX_SHIFT)
> > +#define		FEAT_IVA		0
> > +#define		FEAT_IVA_NONE		1
> > +
> > +#define OMAP3_L2CACHE_SHIFT		10
> > +#define OMAP3_L2CACHE_MASK		(3 << OMAP3_L2CACHE_SHIFT)
> > +#define		FEAT_L2CACHE_NONE	0
> > +#define		FEAT_L2CACHE_64KB	1
> > +#define		FEAT_L2CACHE_128KB	2
> > +#define		FEAT_L2CACHE_256KB	3
> > +
> > +#define OMAP3_ISP_SHIFT			5
> > +#define OMAP3_ISP_MASK			(1<< OMAP3_ISP_SHIFT)
> > +#define		FEAT_ISP		0
> > +#define		FEAT_ISP_NONE		1
> > +
> > +#define OMAP3_NEON_SHIFT		4
> > +#define OMAP3_NEON_MASK			(1<< OMAP3_NEON_SHIFT)
> > +#define		FEAT_NEON		0
> > +#define		FEAT_NEON_NONE		1
> > +
> > +
> > +#define OMAP_HAS_L2CACHE		1
> > +#define OMAP_HAS_IVA			(1 << 1)
> > +#define OMAP_HAS_SGX			(1 << 2)
> > +#define OMAP_HAS_NEON			(1 << 3)
> > +#define OMAP_HAS_ISP			(1 << 4)
> > +
> 
> Use BIT()
> Rename to OMAP3_*
> move to <mach/cpu.h>

[sp] Okay

> 
> >  static struct omap_chip_id omap_chip;
> >  static unsigned int omap_revision;
> > -
> > +static u32 omap_features ;
> 
> Call this omap3_features and make it global (with extern in 
> <mach/cpu.h>)
> 

[sp] Okay.

> >  unsigned int omap_rev(void)
> >  {
> > @@ -35,6 +75,19 @@ unsigned int omap_rev(void)
> >  }
> >  EXPORT_SYMBOL(omap_rev);
> >  
> > +#define OMAP3_HAS_FEATURE(feat,flag)			
> 	\
> > +	unsigned int omap3_has_ ##feat(void)			\
> 
> make this 
> 	static inline unsigned int omap3_has_ ##feat(void) ...
> 
> 
> > +	{							\
> > +		return (omap_features & OMAP_HAS_ ##flag);	\
> > +	}							\
> > +	EXPORT_SYMBOL(omap3_has_ ##feat);
> >
> > +OMAP3_HAS_FEATURE(l2cache, L2CACHE);
> > +OMAP3_HAS_FEATURE(sgx, SGX);
> > +OMAP3_HAS_FEATURE(iva, IVA);
> > +OMAP3_HAS_FEATURE(neon, NEON);
> > +OMAP3_HAS_FEATURE(isp, ISP);
> > +
> 
> ... and move this set to <mach/cpu.h>
> 
> and I'm ok with the rest of this patch.
> 
> Kevin
> 
> >  /**
> >   * omap_chip_is - test whether currently running OMAP 
> matches a chip type
> >   * @oc: omap_chip_t to test against
> > @@ -155,7 +208,33 @@ void __init omap24xx_check_revision(void)
> >  	pr_info("\n");
> >  }
> >  
> > -void __init omap34xx_check_revision(void)
> > +#define OMAP3_CHECK_FEATURE(status,feat)			
> 	\
> > +	if (((status & OMAP3_ ##feat## _MASK) 			
> 	\
> > +		>> OMAP3_ ##feat## _SHIFT) != FEAT_ ##feat## 
> _NONE) { 	\
> > +		omap_features |= OMAP_HAS_ ##feat;		
> 	\
> > +	}
> > +
> > +void __init omap3_check_features(void)
> > +{
> > +	u32 status, temp;
> > +
> > +	omap_features = 0;
> > +
> > +	status = omap_ctrl_readl(OMAP3_CONTROL_OMAP_STATUS);
> > +
> > +	OMAP3_CHECK_FEATURE(status, L2CACHE);
> > +	OMAP3_CHECK_FEATURE(status, IVA);
> > +	OMAP3_CHECK_FEATURE(status, SGX);
> > +	OMAP3_CHECK_FEATURE(status, NEON);
> > +	OMAP3_CHECK_FEATURE(status, ISP);
> > +
> > +	/*
> > +	 * TODO: Get additional info (where applicable)
> > +	 *       e.g. Size of L2 cache.
> > +	 */
> > +}
> > +
> > +void __init omap3_check_revision(void)
> >  {
> >  	u32 cpuid, idcode;
> >  	u16 hawkeye;
> > @@ -212,6 +291,22 @@ out:
> >  	pr_info("OMAP%04x %s\n", omap_rev() >> 16, rev_name);
> >  }
> >  
> > +#define OMAP3_SHOW_FEATURE(feat)		\
> > +	if (omap3_has_ ##feat()) {		\
> > +		pr_info (" - "#feat" : Y");	\
> > +	} else {				\
> > +		pr_info (" - "#feat" : N");	\
> > +	}
> > +
> > +void __init omap3_cpuinfo(void)
> > +{
> > +	OMAP3_SHOW_FEATURE(l2cache);
> > +	OMAP3_SHOW_FEATURE(iva);
> > +	OMAP3_SHOW_FEATURE(sgx);
> > +	OMAP3_SHOW_FEATURE(neon);
> > +	OMAP3_SHOW_FEATURE(isp);
> > +}
> > +
> >  /*
> >   * Try to detect the exact revision of the omap we're running on
> >   */
> > @@ -223,8 +318,11 @@ void __init omap2_check_revision(void)
> >  	 */
> >  	if (cpu_is_omap24xx())
> >  		omap24xx_check_revision();
> > -	else if (cpu_is_omap34xx())
> > -		omap34xx_check_revision();
> > +	else if (cpu_is_omap34xx()) {
> > +		omap3_check_features();
> > +		omap3_check_revision();
> > +		omap3_cpuinfo();
> > +	}
> >  	else if (cpu_is_omap44xx()) {
> >  		printk(KERN_INFO "FIXME: CPU revision = OMAP4430\n");
> >  		return;
> > -- 
> > 1.6.2.2
> >
> > --
> > 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
> 
> 

RE: [PATCH] Runtime detection of Si features

[Premi, Sanjeev]

 

> -----Original Message-----
> From: Menon, Nishanth 
> Sent: Thursday, August 13, 2009 11:29 PM
> To: Kevin Hilman
> Cc: Pandita, Vikram; Premi, Sanjeev; linux-omap@vger.kernel.org
> Subject: Re: [PATCH] Runtime detection of Si features
> 
> Nishanth Menon had written, on 08/13/2009 11:43 AM, the following:
> > Kevin Hilman had written, on 08/13/2009 11:40 AM, the following:
> >> "Pandita, Vikram" <vikram.pandita@ti.com> writes:
> >>
> >>>> -----Original Message-----
> >>>> From: linux-omap-owner@vger.kernel.org 
> [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of
> >>>> Sent: Thursday, August 13, 2009 11:31 AM
> >>>>>> Since most of the code seemed repetitive, macros
> >>>>>> have been used for readability.
> >>>>>>
> >>>>>> Signed-off-by: Sanjeev Premi <premi@ti.com>
> >>>>> I like the feature-based approach.
> >>>>>
> >>>>> A couple questions though.  Is there a bit/register 
> that reports the
> >>>>> collapsed powerdomains of the devices with modified PRCM?
> >>>>>
> >>>>> Also, how will other code query the features?  You're currently
> >>>>> exporting the omap_has_*() functions, but there are no 
> prototypes.
> >>>>>
> >>>>> I think I'd rather see a static inline functions in <mach/cpu.h>
> >>>>> for checking features.  Comments to that end inlined below...
> >>>> Wonder if we can setup some sort of infrastructure for:
> >>>> a) features
> >>>> b) erratas
> >>>> linked to OMAP revs + even better w.r.t silicon module(SGX,I2c)
> >>>> revisions since at times they are used across multiple OMAPs?
> >>> We are hitting exactly this issue with I2C errata 1.153
> >>> Instead of basing the errata check on cpu_is...(), 
> >>> its more appropriate to base it on IP revision of I2C.
> >> Shouldn't the IP revision of I2C be avaialble in an I2C revision
> >> register an be used in the driver instead of cpu_is*?
> > what I was proposing is a much more generic infrastructure 
> which i2c 
> > among other modules can use. Getting IP revision is already 
> available in 
> > the specific IP modules REVISION registers - we might want to 
> > standardize how drivers handle revision based feature/errata set to 
> > ensure that they would have an optimal way to handle the 
> same.. just my 
> > 2 cents..
> > 
> Thinking of this a little more:
> driver's smart handling aside, having a sysfs entry to dump 
> the features 
> and erratas for each of the modules used is so much nice to have.. 
> sigh.. just wondering if anyone has ideas how feasible this might be..
> 

[sp] If all IPs are able to populate a "revision" field in the
     device structure during init/probe then there could be generic
     APIs get_ip_revision() and is_ip_revision();

     Since erratas are linked to the ip versions, this could just be
     a 'print' problem - whether to a sysfs/ a proc entry.

> -- 
> Regards,
> Nishanth Menon
>