[PATCH v2 1/2] omap: add bits for future 3430/3630 ES revisions

[PATCH v2 1/2] omap: add bits for future 3430/3630 ES revisions

[Vikram Pandita]

Add bits for future expansion of omap_chip_id type field.
This is needed to accomodate 3630ES1 chip id which is bit10

Signed-off-by: Vikram Pandita <vikram.pandita@ti.com>
Signed-off-by: Menon, Nishanth <nm@ti.com>
---
 arch/arm/plat-omap/include/plat/cpu.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/plat-omap/include/plat/cpu.h b/arch/arm/plat-omap/include/plat/cpu.h
index 7cb0556..922bf1c 100644
--- a/arch/arm/plat-omap/include/plat/cpu.h
+++ b/arch/arm/plat-omap/include/plat/cpu.h
@@ -45,7 +45,7 @@ int omap_type(void);
 
 struct omap_chip_id {
 	u8 oc;
-	u8 type;
+	u32 type;
 };
 
 #define OMAP_CHIP_INIT(x)	{ .oc = x }
-- 
1.6.5.rc1.19.g8426

[PATCH v2 2/2] omap: 3630: update is_chip variable

[Vikram Pandita]

3630 is getting treated like next rev of 3430
omap_chip.oc variable has to be updated for 3630 version
Provide space for 3430 future ES rev (bit7 to bit9)

Otherwise the Core power domain is not getting registered.

This gets used in the registration of power domains in:
"arch/arm/mach-omap2/powerdomains34xx.h"
core_34xx_es3_1_pwrdm
OMAP_CHIP_INIT(CHIP_GE_OMAP3430ES3_1)

Core power doman will get registered for 3630 only when .oc is pouplated
correctly.

Tested on Zoom3(3630) board

Signed-off-by: Vikram Pandita <vikram.pandita@ti.com>
Signed-off-by: Menon, Nishanth <nm@ti.com>
---
 arch/arm/mach-omap2/id.c              |    2 ++
 arch/arm/plat-omap/include/plat/cpu.h |    8 ++++++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c
index 1c15112..189cf7a 100644
--- a/arch/arm/mach-omap2/id.c
+++ b/arch/arm/mach-omap2/id.c
@@ -359,6 +359,8 @@ void __init omap2_check_revision(void)
 			omap_chip.oc |= CHIP_IS_OMAP3430ES3_0;
 		else if (omap_rev() == OMAP3430_REV_ES3_1)
 			omap_chip.oc |= CHIP_IS_OMAP3430ES3_1;
+		else if (omap_rev() == OMAP3630_REV_ES1_0)
+			omap_chip.oc |= CHIP_IS_OMAP3630ES1;
 	} else {
 		pr_err("Uninitialized omap_chip, please fix!\n");
 	}
diff --git a/arch/arm/plat-omap/include/plat/cpu.h b/arch/arm/plat-omap/include/plat/cpu.h
index 922bf1c..5c95e4b 100644
--- a/arch/arm/plat-omap/include/plat/cpu.h
+++ b/arch/arm/plat-omap/include/plat/cpu.h
@@ -454,6 +454,8 @@ IS_OMAP_TYPE(3430, 0x3430)
 #define CHIP_IS_OMAP3430ES2		(1 << 4)
 #define CHIP_IS_OMAP3430ES3_0		(1 << 5)
 #define CHIP_IS_OMAP3430ES3_1		(1 << 6)
+/* Space for future revisions of 3430: bit7 to bit9 */
+#define CHIP_IS_OMAP3630ES1		(1 << 10)
 
 #define CHIP_IS_OMAP24XX		(CHIP_IS_OMAP2420 | CHIP_IS_OMAP2430)
 
@@ -465,8 +467,10 @@ IS_OMAP_TYPE(3430, 0x3430)
  */
 #define CHIP_GE_OMAP3430ES2		(CHIP_IS_OMAP3430ES2 | \
 					 CHIP_IS_OMAP3430ES3_0 | \
-					 CHIP_IS_OMAP3430ES3_1)
-#define CHIP_GE_OMAP3430ES3_1		(CHIP_IS_OMAP3430ES3_1)
+					 CHIP_IS_OMAP3430ES3_1 | \
+					 CHIP_IS_OMAP3630ES1)
+#define CHIP_GE_OMAP3430ES3_1		(CHIP_IS_OMAP3430ES3_1 | \
+					 CHIP_IS_OMAP3630ES1)
 
 
 int omap_chip_is(struct omap_chip_id oci);
-- 
1.6.5.rc1.19.g8426

Re: [PATCH v2 1/2] omap: add bits for future 3430/3630 ES revisions

[Paul Walmsley]

Hi Vikram, Nishanth, Richard,

a few comments on this:

On Tue, 20 Oct 2009, Vikram Pandita wrote:

> Add bits for future expansion of omap_chip_id type field.
> This is needed to accomodate 3630ES1 chip id which is bit10

...

> diff --git a/arch/arm/plat-omap/include/plat/cpu.h b/arch/arm/plat-omap/include/plat/cpu.h
> index 7cb0556..922bf1c 100644
> --- a/arch/arm/plat-omap/include/plat/cpu.h
> +++ b/arch/arm/plat-omap/include/plat/cpu.h
> @@ -45,7 +45,7 @@ int omap_type(void);
>  
>  struct omap_chip_id {
>  	u8 oc;
> -	u8 type;
> +	u32 type;
>  };

Just wanted to understand the motivation for using the u32.  

Earlier in the life of these patches, two comments were mentioned: the 
desire to 'futureproof' and the desire to reserve space for other 
34xx-family parts.

Regarding 'futureproofing:' that's part of the reason that a separate 
struct was defined for this: to prevent code that uses it from depending 
on the size of the type.  (Originally it was a typedef, but Linus hates 
typedefs...)  So it shouldn't matter how big or small the type is here, as 
long as it can handle all of the bits allocated for it.

Also mentioned was the idea of reserving space for other 34xx-family 
chips.  I'd suggest simply renumbering the bits when and if those versions 
appear.  Code that uses the omap_chip_id system should always use the 
macros (e.g. CHIP_IS_OMAP3430) and not encode separate bit shift values, 
so renumbering should be completely safe and transparent for core code.  
Module code shouldn't be using the omap_chip code, it's for core usage 
only.

So, since only one bit is being added, why not continue the use of the u8?  
Then when the next bits need to be added, the type can be expanded at that 
point, and the bits renumbered if necessary.  This should be a completely 
transparent operation for code that uses it.  Vikram's original patch:

    http://patchwork.kernel.org/patch/54847/

should be fine.

Or am I missing something?


- Paul

Re: [PATCH v2 1/2] omap: add bits for future 3430/3630 ES revisions

[Nishanth Menon]

Paul Walmsley had written, on 10/20/2009 06:14 PM, the following:
> Hi Vikram, Nishanth, Richard,
> 
> a few comments on this:
> 
> On Tue, 20 Oct 2009, Vikram Pandita wrote:
> 
>> Add bits for future expansion of omap_chip_id type field.
>> This is needed to accomodate 3630ES1 chip id which is bit10
> 
> ...
> 
>> diff --git a/arch/arm/plat-omap/include/plat/cpu.h b/arch/arm/plat-omap/include/plat/cpu.h
>> index 7cb0556..922bf1c 100644
>> --- a/arch/arm/plat-omap/include/plat/cpu.h
>> +++ b/arch/arm/plat-omap/include/plat/cpu.h
>> @@ -45,7 +45,7 @@ int omap_type(void);
>>  
>>  struct omap_chip_id {
>>  	u8 oc;
>> -	u8 type;
>> +	u32 type;
>>  };
> 
> Just wanted to understand the motivation for using the u32.  
> 
> Earlier in the life of these patches, two comments were mentioned: the 
> desire to 'futureproof' and the desire to reserve space for other 
> 34xx-family parts.
> 
> Regarding 'futureproofing:' that's part of the reason that a separate 
> struct was defined for this: to prevent code that uses it from depending 
> on the size of the type.  (Originally it was a typedef, but Linus hates 
> typedefs...)  So it shouldn't matter how big or small the type is here, as 
> long as it can handle all of the bits allocated for it.
> 
> Also mentioned was the idea of reserving space for other 34xx-family 
> chips.  I'd suggest simply renumbering the bits when and if those versions 
> appear.  Code that uses the omap_chip_id system should always use the 
> macros (e.g. CHIP_IS_OMAP3430) and not encode separate bit shift values, 
> so renumbering should be completely safe and transparent for core code.  
> Module code shouldn't be using the omap_chip code, it's for core usage 
> only.
> 
> So, since only one bit is being added, why not continue the use of the u8?  
> Then when the next bits need to be added, the type can be expanded at that 
> point, and the bits renumbered if necessary.  This should be a completely 
> transparent operation for code that uses it.  Vikram's original patch:
> 
>     http://patchwork.kernel.org/patch/54847/
> 
> should be fine.

Assumptions:
a) omap_chip_id is supposedly constant for all devices within the same 
family. 3630, 3430 rev x will belong to the same family.
b) we prefer bitops to using cpu_is_blahblah() for reasons of 
performance (if it could be argued so)..

Issues with the strategy of restricting to the current 8 bits:
a) Why extrabits now:
we have 8 bits now and we would have used all 8 bits with 3630 with the 
mentioned patch. What do we do with the next revision of 3430? Do we 
want to increase the size once it comes along? OR Do we want to do it 
right now? Why wait till we get additional silicons to go figure how to 
add those bits as Richard pointed out, when there could be one more in 
the pipeline?

b) How much extra bits should we add?
It does not matter when you pull in extra bits - u'd have to go u16 for 
next rev of 3430, it is gonna be a single entry, what if we end up (god 
forbid) further revisions of 3630 or even a new gen of omap3 family - 
call it 3XYZ devices?
Further how expensive is it to add the u16 Vs u32 size from a memory 
usage perspective?

Either way, we could choose:
a) We can choose to stick with u8 and take the next rev of 3430 ES 
revision when the time is right
b) we can choose to stick with u16 for the known future (+allowing for 5 
more revisions of silicons) of OMAP3.
c) We can choose to say - no one really knows how many ES/variants of a 
silicon family could happen for OMAP3/OMAP4/5/6/7 etc.. and choose u32 
giving us a limit beyond which we might choose to rearchitect things.

The choice is upto this community to make.. options are very easy to 
implement ;).

-- 
Regards,
Nishanth Menon

Re: [PATCH v2 1/2] omap: add bits for future 3430/3630 ES revisions

[Nishanth Menon]

Nishanth Menon had written, on 10/20/2009 08:07 PM, the following:
> Paul Walmsley had written, on 10/20/2009 06:14 PM, the following:
>> Hi Vikram, Nishanth, Richard,
>>
>> a few comments on this:
>>
>> On Tue, 20 Oct 2009, Vikram Pandita wrote:
>>
>>> Add bits for future expansion of omap_chip_id type field.
>>> This is needed to accomodate 3630ES1 chip id which is bit10
>> ...
>>
>>> diff --git a/arch/arm/plat-omap/include/plat/cpu.h b/arch/arm/plat-omap/include/plat/cpu.h
>>> index 7cb0556..922bf1c 100644
>>> --- a/arch/arm/plat-omap/include/plat/cpu.h
>>> +++ b/arch/arm/plat-omap/include/plat/cpu.h
>>> @@ -45,7 +45,7 @@ int omap_type(void);
>>>  
>>>  struct omap_chip_id {
>>>  	u8 oc;
>>> -	u8 type;
>>> +	u32 type;
>>>  };
>> Just wanted to understand the motivation for using the u32.  
>>
>> Earlier in the life of these patches, two comments were mentioned: the 
>> desire to 'futureproof' and the desire to reserve space for other 
>> 34xx-family parts.
>>
>> Regarding 'futureproofing:' that's part of the reason that a separate 
>> struct was defined for this: to prevent code that uses it from depending 
>> on the size of the type.  (Originally it was a typedef, but Linus hates 
>> typedefs...)  So it shouldn't matter how big or small the type is here, as 
>> long as it can handle all of the bits allocated for it.
>>
>> Also mentioned was the idea of reserving space for other 34xx-family 
>> chips.  I'd suggest simply renumbering the bits when and if those versions 
>> appear.  Code that uses the omap_chip_id system should always use the 
>> macros (e.g. CHIP_IS_OMAP3430) and not encode separate bit shift values, 
>> so renumbering should be completely safe and transparent for core code.  
>> Module code shouldn't be using the omap_chip code, it's for core usage 
>> only.
>>
>> So, since only one bit is being added, why not continue the use of the u8?  
>> Then when the next bits need to be added, the type can be expanded at that 
>> point, and the bits renumbered if necessary.  This should be a completely 
>> transparent operation for code that uses it.  Vikram's original patch:
>>
>>     http://patchwork.kernel.org/patch/54847/
>>
>> should be fine.
> 
> Assumptions:
> a) omap_chip_id is supposedly constant for all devices within the same 
> family. 3630, 3430 rev x will belong to the same family.
> b) we prefer bitops to using cpu_is_blahblah() for reasons of 
> performance (if it could be argued so)..
> 
> Issues with the strategy of restricting to the current 8 bits:
> a) Why extrabits now:
> we have 8 bits now and we would have used all 8 bits with 3630 with the 
> mentioned patch. What do we do with the next revision of 3430? Do we 
> want to increase the size once it comes along? OR Do we want to do it 
> right now? Why wait till we get additional silicons to go figure how to 
> add those bits as Richard pointed out, when there could be one more in 
> the pipeline?
> 
> b) How much extra bits should we add?
> It does not matter when you pull in extra bits - u'd have to go u16 for 
> next rev of 3430, it is gonna be a single entry, what if we end up (god 
> forbid) further revisions of 3630 or even a new gen of omap3 family - 
> call it 3XYZ devices?
> Further how expensive is it to add the u16 Vs u32 size from a memory 
> usage perspective?
> 
> Either way, we could choose:
> a) We can choose to stick with u8 and take the next rev of 3430 ES 
> revision when the time is right
> b) we can choose to stick with u16 for the known future (+allowing for 5 
> more revisions of silicons) of OMAP3.
> c) We can choose to say - no one really knows how many ES/variants of a 
> silicon family could happen for OMAP3/OMAP4/5/6/7 etc.. and choose u32 
> giving us a limit beyond which we might choose to rearchitect things.
> 
> The choice is upto this community to make.. options are very easy to 
> implement ;).
> 
forgot to add an important option (if I understood Paul correctly):
d) Renumber and change the implementation of GE macros.. and check if 
any other places we have'nt mixed up stuff.

-- 
Regards,
Nishanth Menon
/me feels much better now ;)

Re: [PATCH v2 1/2] omap: add bits for future 3430/3630 ES revisions

[Alexander Shishkin]

2009/10/21 Nishanth Menon <nm@ti.com>:
> Paul Walmsley had written, on 10/20/2009 06:14 PM, the following:
>>
>> Hi Vikram, Nishanth, Richard,
>>
>> a few comments on this:
>>
>> On Tue, 20 Oct 2009, Vikram Pandita wrote:
>>
>>> Add bits for future expansion of omap_chip_id type field.
>>> This is needed to accomodate 3630ES1 chip id which is bit10
>>
>> ...
>>
>>> diff --git a/arch/arm/plat-omap/include/plat/cpu.h
>>> b/arch/arm/plat-omap/include/plat/cpu.h
>>> index 7cb0556..922bf1c 100644
>>> --- a/arch/arm/plat-omap/include/plat/cpu.h
>>> +++ b/arch/arm/plat-omap/include/plat/cpu.h
>>> @@ -45,7 +45,7 @@ int omap_type(void);
>>>   struct omap_chip_id {
>>>        u8 oc;
>>> -       u8 type;
>>> +       u32 type;
>>>  };
>>
>> Just wanted to understand the motivation for using the u32.
>> Earlier in the life of these patches, two comments were mentioned: the
>> desire to 'futureproof' and the desire to reserve space for other
>> 34xx-family parts.
>>
>> Regarding 'futureproofing:' that's part of the reason that a separate
>> struct was defined for this: to prevent code that uses it from depending on
>> the size of the type.  (Originally it was a typedef, but Linus hates
>> typedefs...)  So it shouldn't matter how big or small the type is here, as
>> long as it can handle all of the bits allocated for it.
>>
>> Also mentioned was the idea of reserving space for other 34xx-family
>> chips.  I'd suggest simply renumbering the bits when and if those versions
>> appear.  Code that uses the omap_chip_id system should always use the macros
>> (e.g. CHIP_IS_OMAP3430) and not encode separate bit shift values, so
>> renumbering should be completely safe and transparent for core code.  Module
>> code shouldn't be using the omap_chip code, it's for core usage only.
>>
>> So, since only one bit is being added, why not continue the use of the u8?
>>  Then when the next bits need to be added, the type can be expanded at that
>> point, and the bits renumbered if necessary.  This should be a completely
>> transparent operation for code that uses it.  Vikram's original patch:
>>
>>    http://patchwork.kernel.org/patch/54847/
>>
>> should be fine.
>
> Assumptions:
> a) omap_chip_id is supposedly constant for all devices within the same
> family. 3630, 3430 rev x will belong to the same family.
If my understanding of the matter is correct, that's only possible if
you can foretell the total number of upcoming 34xx revisions worth
mentioning in the code. Also, can you please elaborate on why is it
supposed to be constant?

> Issues with the strategy of restricting to the current 8 bits:
> a) Why extrabits now:
> we have 8 bits now and we would have used all 8 bits with 3630 with the
> mentioned patch. What do we do with the next revision of 3430? Do we want to
> increase the size once it comes along? OR Do we want to do it right now? Why
> wait till we get additional silicons to go figure how to add those bits as
> Richard pointed out, when there could be one more in the pipeline?
But this code will have to be revisited for each additional silicon
revision anyway, right? Why reserve now?

Regards,
--
Alex
--
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 v2 1/2] omap: add bits for future 3430/3630 ES revisions

[Nishanth Menon]

Alexander Shishkin had written, on 11/02/2009 07:12 AM, the following:
> 2009/10/21 Nishanth Menon <nm@ti.com>:
>> Paul Walmsley had written, on 10/20/2009 06:14 PM, the following:
>>> Hi Vikram, Nishanth, Richard,
>>>
>>> a few comments on this:
>>>
>>> On Tue, 20 Oct 2009, Vikram Pandita wrote:
>>>
>>>> Add bits for future expansion of omap_chip_id type field.
>>>> This is needed to accomodate 3630ES1 chip id which is bit10
>>> ...
>>>
>>>> diff --git a/arch/arm/plat-omap/include/plat/cpu.h
>>>> b/arch/arm/plat-omap/include/plat/cpu.h
>>>> index 7cb0556..922bf1c 100644
>>>> --- a/arch/arm/plat-omap/include/plat/cpu.h
>>>> +++ b/arch/arm/plat-omap/include/plat/cpu.h
>>>> @@ -45,7 +45,7 @@ int omap_type(void);
>>>>   struct omap_chip_id {
>>>>        u8 oc;
>>>> -       u8 type;
>>>> +       u32 type;
>>>>  };
>>> Just wanted to understand the motivation for using the u32.
>>> Earlier in the life of these patches, two comments were mentioned: the
>>> desire to 'futureproof' and the desire to reserve space for other
>>> 34xx-family parts.
>>>
>>> Regarding 'futureproofing:' that's part of the reason that a separate
>>> struct was defined for this: to prevent code that uses it from depending on
>>> the size of the type.  (Originally it was a typedef, but Linus hates
>>> typedefs...)  So it shouldn't matter how big or small the type is here, as
>>> long as it can handle all of the bits allocated for it.
>>>
>>> Also mentioned was the idea of reserving space for other 34xx-family
>>> chips.  I'd suggest simply renumbering the bits when and if those versions
>>> appear.  Code that uses the omap_chip_id system should always use the macros
>>> (e.g. CHIP_IS_OMAP3430) and not encode separate bit shift values, so
>>> renumbering should be completely safe and transparent for core code.  Module
>>> code shouldn't be using the omap_chip code, it's for core usage only.
>>>
>>> So, since only one bit is being added, why not continue the use of the u8?
>>>  Then when the next bits need to be added, the type can be expanded at that
>>> point, and the bits renumbered if necessary.  This should be a completely
>>> transparent operation for code that uses it.  Vikram's original patch:
>>>
>>>    http://patchwork.kernel.org/patch/54847/
>>>
>>> should be fine.
>> Assumptions:
>> a) omap_chip_id is supposedly constant for all devices within the same
>> family. 3630, 3430 rev x will belong to the same family.
> If my understanding of the matter is correct, that's only possible if
> you can foretell the total number of upcoming 34xx revisions worth
> mentioning in the code. Also, can you please elaborate on why is it
> supposed to be constant?
The assumption I made was that omap_chip_id structure was to store info 
regarding the chip and used by the pm framework. The term constant, 
which I used, is probably wrong in this context, esp considering that we 
cannot foretell all the upcoming revisions of the 34xx family.

> 
>> Issues with the strategy of restricting to the current 8 bits:
>> a) Why extrabits now:
>> we have 8 bits now and we would have used all 8 bits with 3630 with the
>> mentioned patch. What do we do with the next revision of 3430? Do we want to
>> increase the size once it comes along? OR Do we want to do it right now? Why
>> wait till we get additional silicons to go figure how to add those bits as
>> Richard pointed out, when there could be one more in the pipeline?
> But this code will have to be revisited for each additional silicon
> revision anyway, right? Why reserve now?
> 
Agreed, that is one of the possible approaches we could take (and seems 
to be the common consensus), we can review the structure at a later 
point of time.

http://patchwork.kernel.org/patch/54847/ seems to be the right direction 
for now at least.

-- 
Regards,
Nishanth Menon

Re: [PATCH v2 1/2] omap: add bits for future 3430/3630 ES revisions

[Alexander Shishkin]

On Mon, Nov 02, 2009 at 08:13:21AM -0600, Nishanth Menon wrote:
> Alexander Shishkin had written, on 11/02/2009 07:12 AM, the following:
> >2009/10/21 Nishanth Menon <nm@ti.com>:
> >>Paul Walmsley had written, on 10/20/2009 06:14 PM, the following:
> >>>Hi Vikram, Nishanth, Richard,
> >>>
> >>>a few comments on this:
> >>>
> >>>On Tue, 20 Oct 2009, Vikram Pandita wrote:
> >>>
> >>>>Add bits for future expansion of omap_chip_id type field.
> >>>>This is needed to accomodate 3630ES1 chip id which is bit10
> >>>...
> >>>
> >>>>diff --git a/arch/arm/plat-omap/include/plat/cpu.h
> >>>>b/arch/arm/plat-omap/include/plat/cpu.h
> >>>>index 7cb0556..922bf1c 100644
> >>>>--- a/arch/arm/plat-omap/include/plat/cpu.h
> >>>>+++ b/arch/arm/plat-omap/include/plat/cpu.h
> >>>>@@ -45,7 +45,7 @@ int omap_type(void);
> >>>>  struct omap_chip_id {
> >>>>       u8 oc;
> >>>>-       u8 type;
> >>>>+       u32 type;
> >>>> };
> >>>Just wanted to understand the motivation for using the u32.
> >>>Earlier in the life of these patches, two comments were mentioned: the
> >>>desire to 'futureproof' and the desire to reserve space for other
> >>>34xx-family parts.
> >>>
> >>>Regarding 'futureproofing:' that's part of the reason that a separate
> >>>struct was defined for this: to prevent code that uses it from depending on
> >>>the size of the type.  (Originally it was a typedef, but Linus hates
> >>>typedefs...)  So it shouldn't matter how big or small the type is here, as
> >>>long as it can handle all of the bits allocated for it.
> >>>
> >>>Also mentioned was the idea of reserving space for other 34xx-family
> >>>chips.  I'd suggest simply renumbering the bits when and if those versions
> >>>appear.  Code that uses the omap_chip_id system should always use the macros
> >>>(e.g. CHIP_IS_OMAP3430) and not encode separate bit shift values, so
> >>>renumbering should be completely safe and transparent for core code.  Module
> >>>code shouldn't be using the omap_chip code, it's for core usage only.
> >>>
> >>>So, since only one bit is being added, why not continue the use of the u8?
> >>> Then when the next bits need to be added, the type can be expanded at that
> >>>point, and the bits renumbered if necessary.  This should be a completely
> >>>transparent operation for code that uses it.  Vikram's original patch:
> >>>
> >>>   http://patchwork.kernel.org/patch/54847/
> >>>
> >>>should be fine.
> >>Assumptions:
> >>a) omap_chip_id is supposedly constant for all devices within the same
> >>family. 3630, 3430 rev x will belong to the same family.
> >If my understanding of the matter is correct, that's only possible if
> >you can foretell the total number of upcoming 34xx revisions worth
> >mentioning in the code. Also, can you please elaborate on why is it
> >supposed to be constant?
> The assumption I made was that omap_chip_id structure was to store
> info regarding the chip and used by the pm framework. The term
> constant, which I used, is probably wrong in this context, esp
> considering that we cannot foretell all the upcoming revisions of
> the 34xx family.

Ok, thanks for clarifying.

> >>Issues with the strategy of restricting to the current 8 bits:
> >>a) Why extrabits now:
> >>we have 8 bits now and we would have used all 8 bits with 3630 with the
> >>mentioned patch. What do we do with the next revision of 3430? Do we want to
> >>increase the size once it comes along? OR Do we want to do it right now? Why
> >>wait till we get additional silicons to go figure how to add those bits as
> >>Richard pointed out, when there could be one more in the pipeline?
> >But this code will have to be revisited for each additional silicon
> >revision anyway, right? Why reserve now?
> >
> Agreed, that is one of the possible approaches we could take (and
> seems to be the common consensus), we can review the structure at a
> later point of time.
> 
> http://patchwork.kernel.org/patch/54847/ seems to be the right
> direction for now at least.

Ok, so let's have it in, perhaps.

Regards,
--
Alex

Re: [PATCH v2 1/2] omap: add bits for future 3430/3630 ES revisions

[Nishanth Menon]

Alexander Shishkin had written, on 11/02/2009 11:01 AM, the following:
[..]
>>>> Issues with the strategy of restricting to the current 8 bits:
>>>> a) Why extrabits now:
>>>> we have 8 bits now and we would have used all 8 bits with 3630 with the
>>>> mentioned patch. What do we do with the next revision of 3430? Do we want to
>>>> increase the size once it comes along? OR Do we want to do it right now? Why
>>>> wait till we get additional silicons to go figure how to add those bits as
>>>> Richard pointed out, when there could be one more in the pipeline?
>>> But this code will have to be revisited for each additional silicon
>>> revision anyway, right? Why reserve now?
>>>
>> Agreed, that is one of the possible approaches we could take (and
>> seems to be the common consensus), we can review the structure at a
>> later point of time.
>>
>> http://patchwork.kernel.org/patch/54847/ seems to be the right
>> direction for now at least.
> 
> Ok, so let's have it in, perhaps.
Tony, Kevin, upto you guys now..

-- 
Regards,
Nishanth Menon

Re: [PATCH v2 1/2] omap: add bits for future 3430/3630 ES revisions

[Paul Walmsley]

On Mon, 2 Nov 2009, Nishanth Menon wrote:

> Alexander Shishkin had written, on 11/02/2009 11:01 AM, the following:
> [..]
> > > > > Issues with the strategy of restricting to the current 8 bits:
> > > > > a) Why extrabits now:
> > > > > we have 8 bits now and we would have used all 8 bits with 3630 with
> > > > > the
> > > > > mentioned patch. What do we do with the next revision of 3430? Do we
> > > > > want to
> > > > > increase the size once it comes along? OR Do we want to do it right
> > > > > now? Why
> > > > > wait till we get additional silicons to go figure how to add those
> > > > > bits as
> > > > > Richard pointed out, when there could be one more in the pipeline?
> > > > But this code will have to be revisited for each additional silicon
> > > > revision anyway, right? Why reserve now?
> > > > 
> > > Agreed, that is one of the possible approaches we could take (and
> > > seems to be the common consensus), we can review the structure at a
> > > later point of time.
> > > 
> > > http://patchwork.kernel.org/patch/54847/ seems to be the right
> > > direction for now at least.
> > 
> > Ok, so let's have it in, perhaps.
> Tony, Kevin, upto you guys now..

The product of this good discussion has been merged into Abhijit's first 
OMAP4 powerdomain patch and is queued for .34.


- Paul

RE: [PATCH v2 1/2] omap: add bits for future 3430/3630 ES revisions

[Pandita, Vikram]

>-----Original Message-----
>From: Paul Walmsley [mailto:paul@pwsan.com]
>Sent: Tuesday, October 20, 2009 6:15 PM
>To: Pandita, Vikram; Woodruff, Richard; Menon, Nishanth
>Cc: linux-omap@vger.kernel.org
>Subject: Re: [PATCH v2 1/2] omap: add bits for future 3430/3630 ES revisions
>
>Hi Vikram, Nishanth, Richard,
>
>a few comments on this:
>
>On Tue, 20 Oct 2009, Vikram Pandita wrote:
>
>> Add bits for future expansion of omap_chip_id type field.
>> This is needed to accomodate 3630ES1 chip id which is bit10
>
>...
>
>> diff --git a/arch/arm/plat-omap/include/plat/cpu.h b/arch/arm/plat-omap/include/plat/cpu.h
>> index 7cb0556..922bf1c 100644
>> --- a/arch/arm/plat-omap/include/plat/cpu.h
>> +++ b/arch/arm/plat-omap/include/plat/cpu.h
>> @@ -45,7 +45,7 @@ int omap_type(void);
>>
>>  struct omap_chip_id {
>>  	u8 oc;
>> -	u8 type;
>> +	u32 type;
>>  };
>
>Just wanted to understand the motivation for using the u32.
>
>Earlier in the life of these patches, two comments were mentioned: the
>desire to 'futureproof' and the desire to reserve space for other
>34xx-family parts.
>
>Regarding 'futureproofing:' that's part of the reason that a separate
>struct was defined for this: to prevent code that uses it from depending
>on the size of the type.  (Originally it was a typedef, but Linus hates
>typedefs...)  So it shouldn't matter how big or small the type is here, as
>long as it can handle all of the bits allocated for it.
>
>Also mentioned was the idea of reserving space for other 34xx-family
>chips.  I'd suggest simply renumbering the bits when and if those versions
>appear.  Code that uses the omap_chip_id system should always use the
>macros (e.g. CHIP_IS_OMAP3430) and not encode separate bit shift values,
>so renumbering should be completely safe and transparent for core code.
>Module code shouldn't be using the omap_chip code, it's for core usage
>only.
>
>So, since only one bit is being added, why not continue the use of the u8?
>Then when the next bits need to be added, the type can be expanded at that
>point, and the bits renumbered if necessary.  This should be a completely
>transparent operation for code that uses it.  Vikram's original patch:
>
>    http://patchwork.kernel.org/patch/54847/

My preference is to go with version one patch.
As and when required, should we add the bits.


>
>should be fine.
>
>Or am I missing something?
>
>
>- Paul