RE: [PATCH 3/9 v3] omap: generic: introduce a single check_revision

2010-07-13 Thread Premi, Sanjeev
 -Original Message-
 From: Menon, Nishanth 
 Sent: Thursday, July 08, 2010 7:57 PM
 To: Tony Lindgren
 Cc: felipe.ba...@nokia.com; linux-omap; Angelo Arrifano; 
 Zebediah C. McClure; Alistair Buxton; Grazvydas Ignotas; Paul 
 Walmsley; Premi, Sanjeev; Shilimkar, Santosh; Guruswamy, 
 Senthilvadivu; Kevin Hilman; DebBarma, Tarun Kanti; 
 ValkeinenTomi (Nokia-MS/Helsinki); Koskinen Aaro 
 (Nokia-MS/Helsinki); Pandita, Vikram; S, Vishwanath
 Subject: Re: [PATCH 3/9 v3] omap: generic: introduce a single 
 check_revision
 
 Tony Lindgren had written, on 07/08/2010 07:21 AM, the following:
  * Menon, Nishanth n...@ti.com [100708 14:49]:
  - Original message -
  Hi,
 
  On Wed, Jul 07, 2010 at 07:24:16PM +0200, ext Nishanth 
 Menon wrote:
  I am not sure.. if you would like drivers to be 
 modprobabe, there may
  be
  quirks that you'd want to enable based on cpu_is_omapxxx 
 checks. so it
  probably does not make sense to __initdata the revision/feature
  variables.
 
  can't you pass the quirks via pdata, then ?
  If pdata is passed based on board: Imagine 3630 and uart 
 quirk. Why share errata xyz over pdata for every board using 
 3630? Quirks are cpu specific and not really domain of board..  
  
  We should be able to handle the quirks by passing some
  flag or function pointer from platform data.
  
  The drivers should be arch independent, using cpu_is_omap
  tests anywhere under drivers/* is wrong and should be
  fixed.
 
 there are two forms of quirks:
 a) quirks which can be detected based on IP rev
 b) quirks which are silicon integration related - only 
 cpu_is_ can 
 be used to detect them.
 for a) - I disagree that pdata should be used (this was my original 
 contention)
 for b) the question IMHO is: How is pdata provided to the 
 driver - that 
 is important. IMHO, pdata taken into drivers could have 
 quirks, but if 
 the quirk addition is done from board files, I disagree, then 
 should be 
 done in arch/arm/mach-omap[12]/somefile.c where somefile.c is 
 common for 
 all boards (e.g. device.c) and that allows the driver to be cpu 
 independent and allows board files not to have redundant information.
 
 BUT, features *should* be kept distinct from quirks for readability 
 purposes.

Nishanth,

Sorry missed most of discussion due to vacation. But just wanted to
know the benefit of check_version() without the processor context.

I had earlier submitted related patches; but 36x being considered
as 34x broke the logic. Before going on vacation I had created a
patch (against 2.6.32). Putting here for review (there are two
typos in this specific patch, pl ignore that):

Usage would be:
if (cpu_rev_eq(omap34xx, OMAP34XX_ES_1_0))


commit 6e4a9439fbc67eb2a55f440923cfade74c4509d2
Author: Sanjeev Premi pr...@ti.com
Date:   Thu Jun 3 21:28:39 2010 +0530

omap: Add macros to evaluate cpu revision

This patch adds macros to evaluate the cpu revision.
These macros increase readability by reducing the
repetitive code when multiple silicon revisions have
to be tested.

diff --git a/arch/arm/plat-omap/include/plat/cpu.h b/arch/arm/plat-omap/include/
index 7514174..7cc5611 100644
--- a/arch/arm/plat-omap/include/plat/cpu.h
+++ b/arch/arm/plat-omap/include/plat/cpu.h
@@ -70,6 +70,12 @@ unsigned int omap_rev(void);
 #define OMAP_REVBITS_200x20
 #define OMAP_REVBITS_300x30
 #define OMAP_REVBITS_400x40
+#define OMAP_REVBITS_500x50
+
+/*
+ * Get the CPU Id for OMAP devices
+ */
+#define GET_OMAP_ID()  ((omap_rev()  16)  0x)

 /*
  * Get the CPU revision for OMAP devices
@@ -385,6 +391,12 @@ IS_OMAP_TYPE(3517, 0x3517)
 #define OMAP3505_REV(v)(OMAP35XX_CLASS | (0x3505  16) | (v 
 #define OMAP3517_REV(v)(OMAP35XX_CLASS | (0x3517  16) | (v 

+#define AM37XX_CLASS   0x3734
+#define AM3703_REV(v)  (AM37XX_CLASS | (0x3503  16) | (v  8))
+#define AM3715_REV(v)  (AM37XX_CLASS | (0x3515  16) | (v  8))
+#define AM3725_REV(v)  (AM37XX_CLASS | (0x3525  16) | (v  8))
+#define AM3730_REV(v)  (AM37XX_CLASS | (0x3530  16) | (v  8))
+
 #define OMAP443X_CLASS 0x44300044
 #define OMAP4430_REV_ES1_0 0x44300044

@@ -458,4 +470,36 @@ OMAP3_HAS_FEATURE(neon, NEON)
 OMAP3_HAS_FEATURE(isp, ISP)
 OMAP3_HAS_FEATURE(192mhz_clk, 192MHZ_CLK)

+/*
+ * Map revision bits to silicon specific revisions
+ */
+#define OMAP34XX_ES_1_0OMAP_REVBITS_00
+#define OMAP34XX_ES_2_0OMAP_REVBITS_10
+#define OMAP34XX_ES_2_1OMAP_REVBITS_20
+#define OMAP34XX_ES_3_0OMAP_REVBITS_30
+#define OMAP34XX_ES_3_1OMAP_REVBITS_40
+#define OMAP34XX_ES_3_1_2  OMAP_REVBITS_50
+
+#define AM3517_ES_1_0  OMAP_REVBITS_00
+
+#define OMAP36XX_ES_1_0OMAP_REVBITS_00
+
+/*
+ * Macros to evaluate CPU revision
+ */
+#define cpu_rev_lt(cpu,rev)\
+   ((cpu_is_omap ##cpu

Re: [PATCH 3/9 v3] omap: generic: introduce a single check_revision

2010-07-13 Thread Nishanth Menon

Premi, Sanjeev had written, on 07/13/2010 10:06 AM, the following:

-Original Message-
From: Menon, Nishanth 
Sent: Thursday, July 08, 2010 7:57 PM

To: Tony Lindgren
Cc: felipe.ba...@nokia.com; linux-omap; Angelo Arrifano; 
Zebediah C. McClure; Alistair Buxton; Grazvydas Ignotas; Paul 
Walmsley; Premi, Sanjeev; Shilimkar, Santosh; Guruswamy, 
Senthilvadivu; Kevin Hilman; DebBarma, Tarun Kanti; 
ValkeinenTomi (Nokia-MS/Helsinki); Koskinen Aaro 
(Nokia-MS/Helsinki); Pandita, Vikram; S, Vishwanath
Subject: Re: [PATCH 3/9 v3] omap: generic: introduce a single 
check_revision


Tony Lindgren had written, on 07/08/2010 07:21 AM, the following:

* Menon, Nishanth n...@ti.com [100708 14:49]:

- Original message -

Hi,

On Wed, Jul 07, 2010 at 07:24:16PM +0200, ext Nishanth 

Menon wrote:
I am not sure.. if you would like drivers to be 

modprobabe, there may

be
quirks that you'd want to enable based on cpu_is_omapxxx 

checks. so it

probably does not make sense to __initdata the revision/feature

variables.

can't you pass the quirks via pdata, then ?
If pdata is passed based on board: Imagine 3630 and uart 
quirk. Why share errata xyz over pdata for every board using 
3630? Quirks are cpu specific and not really domain of board..  

We should be able to handle the quirks by passing some
flag or function pointer from platform data.

The drivers should be arch independent, using cpu_is_omap
tests anywhere under drivers/* is wrong and should be
fixed.

there are two forms of quirks:
a) quirks which can be detected based on IP rev
b) quirks which are silicon integration related - only 
cpu_is_ can 
be used to detect them.
for a) - I disagree that pdata should be used (this was my original 
contention)
for b) the question IMHO is: How is pdata provided to the 
driver - that 
is important. IMHO, pdata taken into drivers could have 
quirks, but if 
the quirk addition is done from board files, I disagree, then 
should be 
done in arch/arm/mach-omap[12]/somefile.c where somefile.c is 
common for 
all boards (e.g. device.c) and that allows the driver to be cpu 
independent and allows board files not to have redundant information.


BUT, features *should* be kept distinct from quirks for readability 
purposes.


Nishanth,

Sorry missed most of discussion due to vacation. But just wanted to
know the benefit of check_version() without the processor context.


lets try not to confuse things here:
a) feature - this debate is part of this thread
b) errata - done per driver
c) cpu_is_xyz() - used for cpu type detection
d) cpu_revision() - the patch that you post below - should be in a 
seperate thread.


I am not averse to the new functions you are introducing and IMHO, I 
agree that it will improve readability, can you rebase and post seperately?




I had earlier submitted related patches; but 36x being considered
as 34x broke the logic. Before going on vacation I had created a
patch (against 2.6.32). Putting here for review (there are two
typos in this specific patch, pl ignore that):

Usage would be:
if (cpu_rev_eq(omap34xx, OMAP34XX_ES_1_0))

maybe we could simplify this a bit(OMAP34XX prefix seems redundant):
if (cpu_rev_eq(omap34xx, ES_1_0))?
we should probably take the discussion seperately.




commit 6e4a9439fbc67eb2a55f440923cfade74c4509d2
Author: Sanjeev Premi pr...@ti.com
Date:   Thu Jun 3 21:28:39 2010 +0530

omap: Add macros to evaluate cpu revision

This patch adds macros to evaluate the cpu revision.
These macros increase readability by reducing the
repetitive code when multiple silicon revisions have
to be tested.

diff --git a/arch/arm/plat-omap/include/plat/cpu.h b/arch/arm/plat-omap/include/
index 7514174..7cc5611 100644
--- a/arch/arm/plat-omap/include/plat/cpu.h
+++ b/arch/arm/plat-omap/include/plat/cpu.h
@@ -70,6 +70,12 @@ unsigned int omap_rev(void);
 #define OMAP_REVBITS_200x20
 #define OMAP_REVBITS_300x30
 #define OMAP_REVBITS_400x40
+#define OMAP_REVBITS_500x50
+
+/*
+ * Get the CPU Id for OMAP devices
+ */
+#define GET_OMAP_ID()  ((omap_rev()  16)  0x)

 /*
  * Get the CPU revision for OMAP devices
@@ -385,6 +391,12 @@ IS_OMAP_TYPE(3517, 0x3517)
 #define OMAP3505_REV(v)(OMAP35XX_CLASS | (0x3505  16) | (v 
 #define OMAP3517_REV(v)(OMAP35XX_CLASS | (0x3517  16) | (v 

+#define AM37XX_CLASS   0x3734
+#define AM3703_REV(v)  (AM37XX_CLASS | (0x3503  16) | (v  8))
+#define AM3715_REV(v)  (AM37XX_CLASS | (0x3515  16) | (v  8))
+#define AM3725_REV(v)  (AM37XX_CLASS | (0x3525  16) | (v  8))
+#define AM3730_REV(v)  (AM37XX_CLASS | (0x3530  16) | (v  8))
+
 #define OMAP443X_CLASS 0x44300044
 #define OMAP4430_REV_ES1_0 0x44300044

@@ -458,4 +470,36 @@ OMAP3_HAS_FEATURE(neon, NEON)
 OMAP3_HAS_FEATURE(isp, ISP)
 OMAP3_HAS_FEATURE(192mhz_clk, 192MHZ_CLK)

+/*
+ * Map revision bits to silicon specific revisions

RE: [PATCH 3/9 v3] omap: generic: introduce a single check_revision

2010-07-13 Thread Premi, Sanjeev
 -Original Message-
 From: Menon, Nishanth 
 Sent: Tuesday, July 13, 2010 9:08 PM
 To: Premi, Sanjeev
 Cc: Tony Lindgren; felipe.ba...@nokia.com; linux-omap; Angelo 
 Arrifano; Zebediah C. McClure; Alistair Buxton; Grazvydas 
 Ignotas; Paul Walmsley; Shilimkar, Santosh; Guruswamy, 
 Senthilvadivu; Kevin Hilman; DebBarma, Tarun Kanti; 
 ValkeinenTomi (Nokia-MS/Helsinki); Koskinen Aaro 
 (Nokia-MS/Helsinki); Pandita, Vikram; S, Vishwanath
 Subject: Re: [PATCH 3/9 v3] omap: generic: introduce a single 
 check_revision
 
 Premi, Sanjeev had written, on 07/13/2010 10:06 AM, the following:
  -Original Message-
  From: Menon, Nishanth 
  Sent: Thursday, July 08, 2010 7:57 PM
  To: Tony Lindgren
  Cc: felipe.ba...@nokia.com; linux-omap; Angelo Arrifano; 
  Zebediah C. McClure; Alistair Buxton; Grazvydas Ignotas; Paul 
  Walmsley; Premi, Sanjeev; Shilimkar, Santosh; Guruswamy, 
  Senthilvadivu; Kevin Hilman; DebBarma, Tarun Kanti; 
  ValkeinenTomi (Nokia-MS/Helsinki); Koskinen Aaro 
  (Nokia-MS/Helsinki); Pandita, Vikram; S, Vishwanath
  Subject: Re: [PATCH 3/9 v3] omap: generic: introduce a single 
  check_revision
 
  Tony Lindgren had written, on 07/08/2010 07:21 AM, the following:
  * Menon, Nishanth n...@ti.com [100708 14:49]:
  - Original message -
  Hi,
 
  On Wed, Jul 07, 2010 at 07:24:16PM +0200, ext Nishanth 
  Menon wrote:
  I am not sure.. if you would like drivers to be 
  modprobabe, there may
  be
  quirks that you'd want to enable based on cpu_is_omapxxx 
  checks. so it
  probably does not make sense to __initdata the revision/feature
  variables.
 
  can't you pass the quirks via pdata, then ?
  If pdata is passed based on board: Imagine 3630 and uart 
  quirk. Why share errata xyz over pdata for every board using 
  3630? Quirks are cpu specific and not really domain of board..  
  We should be able to handle the quirks by passing some
  flag or function pointer from platform data.
 
  The drivers should be arch independent, using cpu_is_omap
  tests anywhere under drivers/* is wrong and should be
  fixed.
  there are two forms of quirks:
  a) quirks which can be detected based on IP rev
  b) quirks which are silicon integration related - only 
  cpu_is_ can 
  be used to detect them.
  for a) - I disagree that pdata should be used (this was my 
 original 
  contention)
  for b) the question IMHO is: How is pdata provided to the 
  driver - that 
  is important. IMHO, pdata taken into drivers could have 
  quirks, but if 
  the quirk addition is done from board files, I disagree, then 
  should be 
  done in arch/arm/mach-omap[12]/somefile.c where somefile.c is 
  common for 
  all boards (e.g. device.c) and that allows the driver to be cpu 
  independent and allows board files not to have redundant 
 information.
 
  BUT, features *should* be kept distinct from quirks for 
 readability 
  purposes.
  
  Nishanth,
  
  Sorry missed most of discussion due to vacation. But just wanted to
  know the benefit of check_version() without the processor context.
 
 lets try not to confuse things here:
 a) feature - this debate is part of this thread

[sp] The OMAP_FEATURE appears in the patch below only because the corresponding
lines didn't change in the patch I created. Not because I wanted to get
another discussion on this subject.

Subject of this mail is omap: generic: introduce a single check_revision
and hence I responded to same.

 b) errata - done per driver

[sp] Didn't get the context here.

 c) cpu_is_xyz() - used for cpu type detection
 d) cpu_revision() - the patch that you post below - should be in a 
 seperate thread.
 
 I am not averse to the new functions you are introducing and IMHO, I 
 agree that it will improve readability, can you rebase and 
 post seperately?

[sp] Will be doing it. The patch here was only for illustration.

 
  
  I had earlier submitted related patches; but 36x being considered
  as 34x broke the logic. Before going on vacation I had created a
  patch (against 2.6.32). Putting here for review (there are two
  typos in this specific patch, pl ignore that):
  
  Usage would be:
  if (cpu_rev_eq(omap34xx, OMAP34XX_ES_1_0))
 maybe we could simplify this a bit(OMAP34XX prefix seems redundant):

Yes. I know, but then I am, not sure if the bit definitions will
remain same in future. An early version of this patch had no
OMAP34XX prefix.

 if (cpu_rev_eq(omap34xx, ES_1_0))?
 we should probably take the discussion seperately.
 
  
  
  commit 6e4a9439fbc67eb2a55f440923cfade74c4509d2
  Author: Sanjeev Premi pr...@ti.com
  Date:   Thu Jun 3 21:28:39 2010 +0530
  
  omap: Add macros to evaluate cpu revision
  
  This patch adds macros to evaluate the cpu revision.
  These macros increase readability by reducing the
  repetitive code when multiple silicon revisions have
  to be tested.
  
  diff --git a/arch/arm/plat-omap/include/plat/cpu.h 
 b/arch/arm/plat-omap/include/
  index 7514174..7cc5611 100644
  --- a/arch/arm

Re: [PATCH 3/9 v3] omap: generic: introduce a single check_revision

2010-07-13 Thread Nishanth Menon

Premi, Sanjeev had written, on 07/13/2010 10:48 AM, the following:

-Original Message-
From: Menon, Nishanth 
Sent: Tuesday, July 13, 2010 9:08 PM

To: Premi, Sanjeev
Cc: Tony Lindgren; felipe.ba...@nokia.com; linux-omap; Angelo 
Arrifano; Zebediah C. McClure; Alistair Buxton; Grazvydas 
Ignotas; Paul Walmsley; Shilimkar, Santosh; Guruswamy, 
Senthilvadivu; Kevin Hilman; DebBarma, Tarun Kanti; 
ValkeinenTomi (Nokia-MS/Helsinki); Koskinen Aaro 
(Nokia-MS/Helsinki); Pandita, Vikram; S, Vishwanath
Subject: Re: [PATCH 3/9 v3] omap: generic: introduce a single 
check_revision


Premi, Sanjeev had written, on 07/13/2010 10:06 AM, the following:

-Original Message-
From: Menon, Nishanth 
Sent: Thursday, July 08, 2010 7:57 PM

To: Tony Lindgren
Cc: felipe.ba...@nokia.com; linux-omap; Angelo Arrifano; 
Zebediah C. McClure; Alistair Buxton; Grazvydas Ignotas; Paul 
Walmsley; Premi, Sanjeev; Shilimkar, Santosh; Guruswamy, 
Senthilvadivu; Kevin Hilman; DebBarma, Tarun Kanti; 
ValkeinenTomi (Nokia-MS/Helsinki); Koskinen Aaro 
(Nokia-MS/Helsinki); Pandita, Vikram; S, Vishwanath
Subject: Re: [PATCH 3/9 v3] omap: generic: introduce a single 
check_revision


Tony Lindgren had written, on 07/08/2010 07:21 AM, the following:

* Menon, Nishanth n...@ti.com [100708 14:49]:

- Original message -

Hi,

On Wed, Jul 07, 2010 at 07:24:16PM +0200, ext Nishanth 

Menon wrote:
I am not sure.. if you would like drivers to be 

modprobabe, there may

be
quirks that you'd want to enable based on cpu_is_omapxxx 

checks. so it

probably does not make sense to __initdata the revision/feature

variables.

can't you pass the quirks via pdata, then ?
If pdata is passed based on board: Imagine 3630 and uart 
quirk. Why share errata xyz over pdata for every board using 
3630? Quirks are cpu specific and not really domain of board..  

We should be able to handle the quirks by passing some
flag or function pointer from platform data.

The drivers should be arch independent, using cpu_is_omap
tests anywhere under drivers/* is wrong and should be
fixed.

there are two forms of quirks:
a) quirks which can be detected based on IP rev
b) quirks which are silicon integration related - only 
cpu_is_ can 
be used to detect them.
for a) - I disagree that pdata should be used (this was my 
original 

contention)
for b) the question IMHO is: How is pdata provided to the 
driver - that 
is important. IMHO, pdata taken into drivers could have 
quirks, but if 
the quirk addition is done from board files, I disagree, then 
should be 
done in arch/arm/mach-omap[12]/somefile.c where somefile.c is 
common for 
all boards (e.g. device.c) and that allows the driver to be cpu 
independent and allows board files not to have redundant 

information.
BUT, features *should* be kept distinct from quirks for 
readability 

purposes.

Nishanth,

Sorry missed most of discussion due to vacation. But just wanted to
know the benefit of check_version() without the processor context.

lets try not to confuse things here:
a) feature - this debate is part of this thread


[sp] The OMAP_FEATURE appears in the patch below only because the corresponding
lines didn't change in the patch I created. Not because I wanted to get
another discussion on this subject.

Subject of this mail is omap: generic: introduce a single check_revision
and hence I responded to same.


:) different context if you look at the thread itself..



b) errata - done per driver


[sp] Didn't get the context here.


previously in the thread we had a discussion on quirk handling - quirks 
in hardware such as erratas dont really follow the normal configuration 
flow and need to be selectively handled.





c) cpu_is_xyz() - used for cpu type detection
d) cpu_revision() - the patch that you post below - should be in a 
seperate thread.


I am not averse to the new functions you are introducing and IMHO, I 
agree that it will improve readability, can you rebase and 
post seperately?


[sp] Will be doing it. The patch here was only for illustration.


thanks.




I had earlier submitted related patches; but 36x being considered
as 34x broke the logic. Before going on vacation I had created a
patch (against 2.6.32). Putting here for review (there are two
typos in this specific patch, pl ignore that):

Usage would be:
if (cpu_rev_eq(omap34xx, OMAP34XX_ES_1_0))

maybe we could simplify this a bit(OMAP34XX prefix seems redundant):


Yes. I know, but then I am, not sure if the bit definitions will
remain same in future. An early version of this patch had no
OMAP34XX prefix.
lets take this up as part of the new thread.. you may also be interested 
in considering Anand G's recent patch for 3630

https://patchwork.kernel.org/patch/47/





if (cpu_rev_eq(omap34xx, ES_1_0))?
we should probably take the discussion seperately.



commit 6e4a9439fbc67eb2a55f440923cfade74c4509d2
Author: Sanjeev Premi pr...@ti.com
Date:   Thu Jun 3 21:28:39 2010 +0530

omap: Add macros to evaluate cpu

Re: [PATCH 3/9 v3] omap: generic: introduce a single check_revision

2010-07-08 Thread Felipe Balbi

Hi,

On Wed, Jul 07, 2010 at 07:24:16PM +0200, ext Nishanth Menon wrote:

I am not sure.. if you would like drivers to be modprobabe, there may be
quirks that you'd want to enable based on cpu_is_omapxxx checks. so it
probably does not make sense to __initdata the revision/feature variables.


can't you pass the quirks via pdata, then ?

--
balbi

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


Re: [PATCH 3/9 v3] omap: generic: introduce a single check_revision

2010-07-08 Thread Menon, Nishanth

- Original message -
 Hi,
 
 On Wed, Jul 07, 2010 at 07:24:16PM +0200, ext Nishanth Menon wrote:
  I am not sure.. if you would like drivers to be modprobabe, there may
 be
  quirks that you'd want to enable based on cpu_is_omapxxx checks. so it
  probably does not make sense to __initdata the revision/feature
 variables.
 
 can't you pass the quirks via pdata, then ?

If pdata is passed based on board: Imagine 3630 and uart quirk. Why share 
errata xyz over pdata for every board using 3630? Quirks are cpu specific and 
not really domain of board..  

Regards,
Nishanth Menon
 
 -- 
 balbi
 
 DefectiveByDesign.org
 
N�r��yb�X��ǧv�^�)޺{.n�+{��f��{ay�ʇڙ�,j��f���h���z��w���
���j:+v���w�j�mzZ+�ݢj��!�i

Re: [PATCH 3/9 v3] omap: generic: introduce a single check_revision

2010-07-08 Thread Tony Lindgren
* Menon, Nishanth n...@ti.com [100708 14:49]:
 
 - Original message -
  Hi,
  
  On Wed, Jul 07, 2010 at 07:24:16PM +0200, ext Nishanth Menon wrote:
   I am not sure.. if you would like drivers to be modprobabe, there may
  be
   quirks that you'd want to enable based on cpu_is_omapxxx checks. so it
   probably does not make sense to __initdata the revision/feature
  variables.
  
  can't you pass the quirks via pdata, then ?
 
 If pdata is passed based on board: Imagine 3630 and uart quirk. Why share 
 errata xyz over pdata for every board using 3630? Quirks are cpu specific and 
 not really domain of board..  

We should be able to handle the quirks by passing some
flag or function pointer from platform data.

The drivers should be arch independent, using cpu_is_omap
tests anywhere under drivers/* is wrong and should be
fixed.

Regards,

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


Re: [PATCH 3/9 v3] omap: generic: introduce a single check_revision

2010-07-08 Thread Felipe Balbi

On Thu, Jul 08, 2010 at 01:57:36PM +0200, ext Menon, Nishanth wrote:
If pdata is passed based on board: Imagine 3630 and uart quirk. Why 
share errata xyz over pdata for every board using 3630? Quirks are cpu 
specific and not really domain of board..


look at usb-musb.c, it groups the generic part and only asks boards to 
pass the board-specific bits (usb mode and power).


--
balbi

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


Re: [PATCH 3/9 v3] omap: generic: introduce a single check_revision

2010-07-08 Thread Nishanth Menon

Tony Lindgren had written, on 07/08/2010 07:21 AM, the following:

* Menon, Nishanth n...@ti.com [100708 14:49]:

- Original message -

Hi,

On Wed, Jul 07, 2010 at 07:24:16PM +0200, ext Nishanth Menon wrote:

I am not sure.. if you would like drivers to be modprobabe, there may

be

quirks that you'd want to enable based on cpu_is_omapxxx checks. so it
probably does not make sense to __initdata the revision/feature

variables.

can't you pass the quirks via pdata, then ?
If pdata is passed based on board: Imagine 3630 and uart quirk. Why share errata xyz over pdata for every board using 3630? Quirks are cpu specific and not really domain of board..  


We should be able to handle the quirks by passing some
flag or function pointer from platform data.

The drivers should be arch independent, using cpu_is_omap
tests anywhere under drivers/* is wrong and should be
fixed.


there are two forms of quirks:
a) quirks which can be detected based on IP rev
b) quirks which are silicon integration related - only cpu_is_ can 
be used to detect them.
for a) - I disagree that pdata should be used (this was my original 
contention)
for b) the question IMHO is: How is pdata provided to the driver - that 
is important. IMHO, pdata taken into drivers could have quirks, but if 
the quirk addition is done from board files, I disagree, then should be 
done in arch/arm/mach-omap[12]/somefile.c where somefile.c is common for 
all boards (e.g. device.c) and that allows the driver to be cpu 
independent and allows board files not to have redundant information.


BUT, features *should* be kept distinct from quirks for readability 
purposes.


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


Re: [PATCH 3/9 v3] omap: generic: introduce a single check_revision

2010-07-07 Thread Tony Lindgren
* Nishanth Menon n...@ti.com [100625 19:19]:
 --- a/arch/arm/mach-omap2/io.c
 +++ b/arch/arm/mach-omap2/io.c
 @@ -238,7 +238,7 @@ static void __init _omap2_map_common_io(void)
   local_flush_tlb_all();
   flush_cache_all();
  
 - omap2_check_revision();
 + omap_check_revision();
   omap_sram_init();
  }
  
 diff --git a/arch/arm/plat-omap/common.c b/arch/arm/plat-omap/common.c
 index fca73cd..4a0e333 100644
 --- a/arch/arm/plat-omap/common.c
 +++ b/arch/arm/plat-omap/common.c
 @@ -89,6 +89,12 @@ void __init omap_reserve(void)
   omap_vram_reserve_sdram_lmb();
  }
  
 +void __init omap_check_revision(void)
 +{
 + omap1_check_revision();
 + omap2_check_revision();
 +}
 +
  /*
   * 32KHz clocksource ... always available, on pretty most chips except
   * OMAP 730 and 1510.  Other timers could be used as clocksources, with
 diff --git a/arch/arm/plat-omap/include/plat/cpu.h 
 b/arch/arm/plat-omap/include/plat/cpu.h
 index 7514174..5f12a0b 100644
 --- a/arch/arm/plat-omap/include/plat/cpu.h
 +++ b/arch/arm/plat-omap/include/plat/cpu.h
 @@ -431,7 +431,18 @@ IS_OMAP_TYPE(3517, 0x3517)
  
  
  int omap_chip_is(struct omap_chip_id oci);
 -void omap2_check_revision(void);
 +#ifdef CONFIG_ARCH_OMAP2PLUS
 +extern void omap2_check_revision(void);
 +#else
 +static inline void omap2_check_revision(void) {}
 +#endif
 +
 +#ifdef CONFIG_ARCH_OMAP1
 +extern void omap1_check_revision(void);
 +#else
 +static inline void omap1_check_revision(void) {}
 +#endif
 +void omap_check_revision(void);

Hmm, to me it seems like we should have static omap_check_revision
in both mach-omap1/id.c and mach-omap2/id.c. Or do we need to call
these anywhere else outside both id.c files?

Then these can set u32 omap_revision flags in plat-omap/common.c,
and then we can have a common omap_get_revision() or something
in plat-omap/common.c?

There should not be need for cpu_is_omap tests for getting
the revision after it's initialized.

Regards,

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


Re: [PATCH 3/9 v3] omap: generic: introduce a single check_revision

2010-07-07 Thread Nishanth Menon

Tony Lindgren had written, on 07/07/2010 07:36 AM, the following:

* Nishanth Menon n...@ti.com [100625 19:19]:

--- a/arch/arm/mach-omap2/io.c
+++ b/arch/arm/mach-omap2/io.c
@@ -238,7 +238,7 @@ static void __init _omap2_map_common_io(void)
local_flush_tlb_all();
flush_cache_all();
 
-	omap2_check_revision();

+   omap_check_revision();
omap_sram_init();
 }
 
diff --git a/arch/arm/plat-omap/common.c b/arch/arm/plat-omap/common.c

index fca73cd..4a0e333 100644
--- a/arch/arm/plat-omap/common.c
+++ b/arch/arm/plat-omap/common.c
@@ -89,6 +89,12 @@ void __init omap_reserve(void)
omap_vram_reserve_sdram_lmb();
 }
 
+void __init omap_check_revision(void)

+{
+   omap1_check_revision();
+   omap2_check_revision();
+}
+
 /*
  * 32KHz clocksource ... always available, on pretty most chips except
  * OMAP 730 and 1510.  Other timers could be used as clocksources, with
diff --git a/arch/arm/plat-omap/include/plat/cpu.h 
b/arch/arm/plat-omap/include/plat/cpu.h
index 7514174..5f12a0b 100644
--- a/arch/arm/plat-omap/include/plat/cpu.h
+++ b/arch/arm/plat-omap/include/plat/cpu.h
@@ -431,7 +431,18 @@ IS_OMAP_TYPE(3517, 0x3517)
 
 
 int omap_chip_is(struct omap_chip_id oci);

-void omap2_check_revision(void);
+#ifdef CONFIG_ARCH_OMAP2PLUS
+extern void omap2_check_revision(void);
+#else
+static inline void omap2_check_revision(void) {}
+#endif
+
+#ifdef CONFIG_ARCH_OMAP1
+extern void omap1_check_revision(void);
+#else
+static inline void omap1_check_revision(void) {}
+#endif
+void omap_check_revision(void);


Hmm, to me it seems like we should have static omap_check_revision
in both mach-omap1/id.c and mach-omap2/id.c. Or do we need to call
these anywhere else outside both id.c files?
check_revision is called from mach-omap[12]/io.c - so no chance of the 
check_revision to be static..




Then these can set u32 omap_revision flags in plat-omap/common.c,
and then we can have a common omap_get_revision() or something
in plat-omap/common.c?

i think I managed to get rid of it entirely.. ref: attached patch

If we are ok with this, I will repost the series (i squashed 
omap1-rename-check_revision into this patch).




There should not be need for cpu_is_omap tests for getting
the revision after it's initialized.
I am not sure.. if you would like drivers to be modprobabe, there may be 
quirks that you'd want to enable based on cpu_is_omapxxx checks. so it 
probably does not make sense to __initdata the revision/feature variables.



--
Regards,
Nishanth Menon
From f72070e575433ad07ed018aef5c43677424003d0 Mon Sep 17 00:00:00 2001
From: Nishanth Menon n...@ti.com
Date: Fri, 21 May 2010 12:09:33 -0500
Subject: [PATCH 2/7] omap: generic: introduce a single check_revision

Introduce a single omap generic check_revision that routes the
request to the right revision of check_revision.

Note: OMAP1 and OMAP2+ are not built into a single kernel.

Cc: Tony Lindgren t...@atomide.com
Cc: Angelo Arrifano mik...@gmail.com
Cc: Zebediah C. McClure z...@lurian.net
Cc: Alistair Buxton a.j.bux...@gmail.com
Cc: Grazvydas Ignotas nota...@gmail.com
Cc: Paul Walmsley p...@pwsan.com
Cc: Sanjeev Premi pr...@ti.com
Cc: Santosh Shilimkar santosh.shilim...@ti.com
Cc: Senthilvadivu Gurusamy svad...@ti.com
Cc: Kevin Hilman khil...@deeprootsystems.com
Cc: Tarun Kanti DebBarma tarun.ka...@ti.com
Cc: Tomi Valkeinen tomi.valkei...@nokia.com
Cc: Aaro Koskinen aaro.koski...@nokia.com
Cc: Vikram Pandita vikram.pand...@ti.com
Cc: Vishwanath S vishw...@ti.com
Cc: linux-omap@vger.kernel.org

Signed-off-by: Nishanth Menon n...@ti.com
---
 arch/arm/mach-omap1/io.c  |1 -
 arch/arm/mach-omap2/id.c  |2 +-
 arch/arm/mach-omap2/io.c  |2 +-
 arch/arm/plat-omap/include/plat/cpu.h |3 ++-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-omap1/io.c b/arch/arm/mach-omap1/io.c
index 0ce3fec..4f9ee73 100644
--- a/arch/arm/mach-omap1/io.c
+++ b/arch/arm/mach-omap1/io.c
@@ -20,7 +20,6 @@
 
 #include clock.h
 
-extern void omap_check_revision(void);
 extern void omap_sram_init(void);
 
 /*
diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c
index c7bf0e1..80f0950 100644
--- a/arch/arm/mach-omap2/id.c
+++ b/arch/arm/mach-omap2/id.c
@@ -371,7 +371,7 @@ static void __init omap3_cpuinfo(void)
 /*
  * Try to detect the exact revision of the omap we're running on
  */
-void __init omap2_check_revision(void)
+void __init omap_check_revision(void)
 {
 	/*
 	 * At this point we have an idea about the processor revision set
diff --git a/arch/arm/mach-omap2/io.c b/arch/arm/mach-omap2/io.c
index b9ea70b..75883fe 100644
--- a/arch/arm/mach-omap2/io.c
+++ b/arch/arm/mach-omap2/io.c
@@ -238,7 +238,7 @@ static void __init _omap2_map_common_io(void)
 	local_flush_tlb_all();
 	flush_cache_all();
 
-	omap2_check_revision();
+	omap_check_revision();
 	omap_sram_init();
 }
 
diff --git a/arch/arm/plat-omap/include/plat/cpu.h b/arch/arm/plat-omap/include/plat/cpu.h

RE: [PATCH 3/9 v3] omap: generic: introduce a single check_revision

2010-06-25 Thread Shilimkar, Santosh
 -Original Message-
 From: Menon, Nishanth
 Sent: Friday, June 25, 2010 9:55 PM
 To: linux-omap
 Cc: Tony Lindgren; Menon, Nishanth; Angelo Arrifano; Zebediah C. McClure; 
 Alistair Buxton; Grazvydas
 Ignotas; Paul Walmsley; Premi, Sanjeev; Shilimkar, Santosh; Guruswamy, 
 Senthilvadivu; Kevin Hilman;
 DebBarma, Tarun Kanti; Tomi Valkeinen; Aaro Koskinen; Pandita, Vikram; S, 
 Vishwanath; linux-
 o...@vger.kernel.org
 Subject: [PATCH 3/9 v3] omap: generic: introduce a single check_revision
 
 Introduce a single omap generic check_revision that routes the
 request to the right revision of check_revision.
 
 Note: OMAP1 and OMAP2+ are not built into a single kernel. This
 allows for the headers definitions of omap1_check_revision() and
 omap2_check_revision() to be used without #ifdefs and additional cpu
 checks in our single check_revision.
 
 Cc: Tony Lindgren t...@atomide.com
 Cc: Angelo Arrifano mik...@gmail.com
 Cc: Zebediah C. McClure z...@lurian.net
 Cc: Alistair Buxton a.j.bux...@gmail.com
 Cc: Grazvydas Ignotas nota...@gmail.com
 Cc: Paul Walmsley p...@pwsan.com
 Cc: Sanjeev Premi pr...@ti.com
 Cc: Santosh Shilimkar santosh.shilim...@ti.com
 Cc: Senthilvadivu Gurusamy svad...@ti.com
 Cc: Kevin Hilman khil...@deeprootsystems.com
 Cc: Tarun Kanti DebBarma tarun.ka...@ti.com
 Cc: Tomi Valkeinen tomi.valkei...@nokia.com
 Cc: Aaro Koskinen aaro.koski...@nokia.com
 Cc: Vikram Pandita vikram.pand...@ti.com
 Cc: Vishwanath S vishw...@ti.com
 Cc: linux-omap@vger.kernel.org
 
 Signed-off-by: Nishanth Menon n...@ti.com
 ---
 V3: comments from http://marc.info/?t=12774725203r=1w=2
   fixed
 V2: comments from http://marc.info/?t=12772595616r=1w=2
   fixed
 V1: original
  arch/arm/mach-omap1/io.c  |3 +--
  arch/arm/mach-omap2/io.c  |2 +-
  arch/arm/plat-omap/common.c   |6 ++
  arch/arm/plat-omap/include/plat/cpu.h |   13 -
  4 files changed, 20 insertions(+), 4 deletions(-)
 
 diff --git a/arch/arm/mach-omap1/io.c b/arch/arm/mach-omap1/io.c
 index e4d8680..4f9ee73 100644
 --- a/arch/arm/mach-omap1/io.c
 +++ b/arch/arm/mach-omap1/io.c
 @@ -20,7 +20,6 @@
 
  #include clock.h
 
 -extern void omap1_check_revision(void);
  extern void omap_sram_init(void);
 
  /*
 @@ -102,7 +101,7 @@ void __init omap1_map_common_io(void)
   /* We want to check CPU revision early for cpu_is_omap() macros.
* IO space mapping must be initialized before we can do that.
*/
 - omap1_check_revision();
 + omap_check_revision();
 
  #if defined (CONFIG_ARCH_OMAP730) || defined (CONFIG_ARCH_OMAP850)
   if (cpu_is_omap7xx()) {
 diff --git a/arch/arm/mach-omap2/io.c b/arch/arm/mach-omap2/io.c
 index 4e1f53d..eeb0e30 100644
 --- a/arch/arm/mach-omap2/io.c
 +++ b/arch/arm/mach-omap2/io.c
 @@ -238,7 +238,7 @@ static void __init _omap2_map_common_io(void)
   local_flush_tlb_all();
   flush_cache_all();
 
 - omap2_check_revision();
 + omap_check_revision();
   omap_sram_init();
  }
 
 diff --git a/arch/arm/plat-omap/common.c b/arch/arm/plat-omap/common.c
 index fca73cd..4a0e333 100644
 --- a/arch/arm/plat-omap/common.c
 +++ b/arch/arm/plat-omap/common.c
 @@ -89,6 +89,12 @@ void __init omap_reserve(void)
   omap_vram_reserve_sdram_lmb();
  }
 
 +void __init omap_check_revision(void)
 +{
 + omap1_check_revision();
 + omap2_check_revision();
 +}
 +
  /*
   * 32KHz clocksource ... always available, on pretty most chips except
   * OMAP 730 and 1510.  Other timers could be used as clocksources, with
 diff --git a/arch/arm/plat-omap/include/plat/cpu.h 
 b/arch/arm/plat-omap/include/plat/cpu.h
 index 7514174..5f12a0b 100644
 --- a/arch/arm/plat-omap/include/plat/cpu.h
 +++ b/arch/arm/plat-omap/include/plat/cpu.h
 @@ -431,7 +431,18 @@ IS_OMAP_TYPE(3517, 0x3517)
 
 
  int omap_chip_is(struct omap_chip_id oci);
 -void omap2_check_revision(void);
 +#ifdef CONFIG_ARCH_OMAP2PLUS
 +extern void omap2_check_revision(void);
 +#else
 +static inline void omap2_check_revision(void) {}
I think codingstyle suggest empty function braces to be on next line
like
static inline void omap2_check_revision(void)
{}
 +#endif
 +
 +#ifdef CONFIG_ARCH_OMAP1
 +extern void omap1_check_revision(void);
 +#else
 +static inline void omap1_check_revision(void) {}
 +#endif
 +void omap_check_revision(void);
 
  /*
   * Runtime detection of OMAP3 features

Otherwise patch looks good to me. 
 --
 1.6.3.3

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


Re: [PATCH 3/9 v3] omap: generic: introduce a single check_revision

2010-06-25 Thread Nishanth Menon

Shilimkar, Santosh had written, on 06/25/2010 11:41 AM, the following:

-Original Message-
From: Menon, Nishanth
Sent: Friday, June 25, 2010 9:55 PM
To: linux-omap
Cc: Tony Lindgren; Menon, Nishanth; Angelo Arrifano; Zebediah C. McClure; 
Alistair Buxton; Grazvydas
Ignotas; Paul Walmsley; Premi, Sanjeev; Shilimkar, Santosh; Guruswamy, 
Senthilvadivu; Kevin Hilman;
DebBarma, Tarun Kanti; Tomi Valkeinen; Aaro Koskinen; Pandita, Vikram; S, 
Vishwanath; linux-
o...@vger.kernel.org
Subject: [PATCH 3/9 v3] omap: generic: introduce a single check_revision

Introduce a single omap generic check_revision that routes the
request to the right revision of check_revision.

Note: OMAP1 and OMAP2+ are not built into a single kernel. This
allows for the headers definitions of omap1_check_revision() and
omap2_check_revision() to be used without #ifdefs and additional cpu
checks in our single check_revision.

Cc: Tony Lindgren t...@atomide.com
Cc: Angelo Arrifano mik...@gmail.com
Cc: Zebediah C. McClure z...@lurian.net
Cc: Alistair Buxton a.j.bux...@gmail.com
Cc: Grazvydas Ignotas nota...@gmail.com
Cc: Paul Walmsley p...@pwsan.com
Cc: Sanjeev Premi pr...@ti.com
Cc: Santosh Shilimkar santosh.shilim...@ti.com
Cc: Senthilvadivu Gurusamy svad...@ti.com
Cc: Kevin Hilman khil...@deeprootsystems.com
Cc: Tarun Kanti DebBarma tarun.ka...@ti.com
Cc: Tomi Valkeinen tomi.valkei...@nokia.com
Cc: Aaro Koskinen aaro.koski...@nokia.com
Cc: Vikram Pandita vikram.pand...@ti.com
Cc: Vishwanath S vishw...@ti.com
Cc: linux-omap@vger.kernel.org

Signed-off-by: Nishanth Menon n...@ti.com
---
V3: comments from http://marc.info/?t=12774725203r=1w=2
fixed
V2: comments from http://marc.info/?t=12772595616r=1w=2
fixed
V1: original
 arch/arm/mach-omap1/io.c  |3 +--
 arch/arm/mach-omap2/io.c  |2 +-
 arch/arm/plat-omap/common.c   |6 ++
 arch/arm/plat-omap/include/plat/cpu.h |   13 -
 4 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-omap1/io.c b/arch/arm/mach-omap1/io.c
index e4d8680..4f9ee73 100644
--- a/arch/arm/mach-omap1/io.c
+++ b/arch/arm/mach-omap1/io.c
@@ -20,7 +20,6 @@

 #include clock.h

-extern void omap1_check_revision(void);
 extern void omap_sram_init(void);

 /*
@@ -102,7 +101,7 @@ void __init omap1_map_common_io(void)
/* We want to check CPU revision early for cpu_is_omap() macros.
 * IO space mapping must be initialized before we can do that.
 */
-   omap1_check_revision();
+   omap_check_revision();

 #if defined (CONFIG_ARCH_OMAP730) || defined (CONFIG_ARCH_OMAP850)
if (cpu_is_omap7xx()) {
diff --git a/arch/arm/mach-omap2/io.c b/arch/arm/mach-omap2/io.c
index 4e1f53d..eeb0e30 100644
--- a/arch/arm/mach-omap2/io.c
+++ b/arch/arm/mach-omap2/io.c
@@ -238,7 +238,7 @@ static void __init _omap2_map_common_io(void)
local_flush_tlb_all();
flush_cache_all();

-   omap2_check_revision();
+   omap_check_revision();
omap_sram_init();
 }

diff --git a/arch/arm/plat-omap/common.c b/arch/arm/plat-omap/common.c
index fca73cd..4a0e333 100644
--- a/arch/arm/plat-omap/common.c
+++ b/arch/arm/plat-omap/common.c
@@ -89,6 +89,12 @@ void __init omap_reserve(void)
omap_vram_reserve_sdram_lmb();
 }

+void __init omap_check_revision(void)
+{
+   omap1_check_revision();
+   omap2_check_revision();
+}
+
 /*
  * 32KHz clocksource ... always available, on pretty most chips except
  * OMAP 730 and 1510.  Other timers could be used as clocksources, with
diff --git a/arch/arm/plat-omap/include/plat/cpu.h 
b/arch/arm/plat-omap/include/plat/cpu.h
index 7514174..5f12a0b 100644
--- a/arch/arm/plat-omap/include/plat/cpu.h
+++ b/arch/arm/plat-omap/include/plat/cpu.h
@@ -431,7 +431,18 @@ IS_OMAP_TYPE(3517, 0x3517)


 int omap_chip_is(struct omap_chip_id oci);
-void omap2_check_revision(void);
+#ifdef CONFIG_ARCH_OMAP2PLUS
+extern void omap2_check_revision(void);
+#else
+static inline void omap2_check_revision(void) {}

I think codingstyle suggest empty function braces to be on next line
like
static inline void omap2_check_revision(void)
{}


are you sure about that? can you point me to the documentation for that?
Style I followed is off:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/SubmittingPatches;h=72651f788f4e3536149ef5e7ddfbed96a8f14d2f;hb=HEAD#l661


+#endif
+
+#ifdef CONFIG_ARCH_OMAP1
+extern void omap1_check_revision(void);
+#else
+static inline void omap1_check_revision(void) {}
+#endif
+void omap_check_revision(void);

 /*
  * Runtime detection of OMAP3 features


Otherwise patch looks good to me. 

thanks for the ack.


--
1.6.3.3





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


RE: [PATCH 3/9 v3] omap: generic: introduce a single check_revision

2010-06-25 Thread Shilimkar, Santosh
 -Original Message-
 From: Menon, Nishanth
 Sent: Friday, June 25, 2010 11:02 PM
 To: Shilimkar, Santosh
 Cc: linux-omap; Tony Lindgren; Angelo Arrifano; Zebediah C. McClure; Alistair 
 Buxton; Grazvydas
 Ignotas; Paul Walmsley; Premi, Sanjeev; Guruswamy, Senthilvadivu; Kevin 
 Hilman; DebBarma, Tarun
 Kanti; Tomi Valkeinen; Aaro Koskinen; Pandita, Vikram; S, Vishwanath
 Subject: Re: [PATCH 3/9 v3] omap: generic: introduce a single check_revision
 
 Shilimkar, Santosh had written, on 06/25/2010 11:41 AM, the following:
  -Original Message-
  From: Menon, Nishanth
  Sent: Friday, June 25, 2010 9:55 PM
  To: linux-omap
  Cc: Tony Lindgren; Menon, Nishanth; Angelo Arrifano; Zebediah C. McClure; 
  Alistair Buxton;
 Grazvydas
  Ignotas; Paul Walmsley; Premi, Sanjeev; Shilimkar, Santosh; Guruswamy, 
  Senthilvadivu; Kevin
 Hilman;
  DebBarma, Tarun Kanti; Tomi Valkeinen; Aaro Koskinen; Pandita, Vikram; S, 
  Vishwanath; linux-
  o...@vger.kernel.org
  Subject: [PATCH 3/9 v3] omap: generic: introduce a single check_revision
 
  Introduce a single omap generic check_revision that routes the
  request to the right revision of check_revision.
 
  Note: OMAP1 and OMAP2+ are not built into a single kernel. This
  allows for the headers definitions of omap1_check_revision() and
  omap2_check_revision() to be used without #ifdefs and additional cpu
  checks in our single check_revision.
 
  Cc: Tony Lindgren t...@atomide.com
  Cc: Angelo Arrifano mik...@gmail.com
  Cc: Zebediah C. McClure z...@lurian.net
  Cc: Alistair Buxton a.j.bux...@gmail.com
  Cc: Grazvydas Ignotas nota...@gmail.com
  Cc: Paul Walmsley p...@pwsan.com
  Cc: Sanjeev Premi pr...@ti.com
  Cc: Santosh Shilimkar santosh.shilim...@ti.com
  Cc: Senthilvadivu Gurusamy svad...@ti.com
  Cc: Kevin Hilman khil...@deeprootsystems.com
  Cc: Tarun Kanti DebBarma tarun.ka...@ti.com
  Cc: Tomi Valkeinen tomi.valkei...@nokia.com
  Cc: Aaro Koskinen aaro.koski...@nokia.com
  Cc: Vikram Pandita vikram.pand...@ti.com
  Cc: Vishwanath S vishw...@ti.com
  Cc: linux-omap@vger.kernel.org
 
  Signed-off-by: Nishanth Menon n...@ti.com
  ---
  V3: comments from http://marc.info/?t=12774725203r=1w=2
 fixed
  V2: comments from http://marc.info/?t=12772595616r=1w=2
 fixed
  V1: original
   arch/arm/mach-omap1/io.c  |3 +--
   arch/arm/mach-omap2/io.c  |2 +-
   arch/arm/plat-omap/common.c   |6 ++
   arch/arm/plat-omap/include/plat/cpu.h |   13 -
   4 files changed, 20 insertions(+), 4 deletions(-)
 
  diff --git a/arch/arm/mach-omap1/io.c b/arch/arm/mach-omap1/io.c
  index e4d8680..4f9ee73 100644
  --- a/arch/arm/mach-omap1/io.c
  +++ b/arch/arm/mach-omap1/io.c
  @@ -20,7 +20,6 @@
 
   #include clock.h
 
  -extern void omap1_check_revision(void);
   extern void omap_sram_init(void);
 
   /*
  @@ -102,7 +101,7 @@ void __init omap1_map_common_io(void)
 /* We want to check CPU revision early for cpu_is_omap() macros.
  * IO space mapping must be initialized before we can do that.
  */
  -  omap1_check_revision();
  +  omap_check_revision();
 
   #if defined (CONFIG_ARCH_OMAP730) || defined (CONFIG_ARCH_OMAP850)
 if (cpu_is_omap7xx()) {
  diff --git a/arch/arm/mach-omap2/io.c b/arch/arm/mach-omap2/io.c
  index 4e1f53d..eeb0e30 100644
  --- a/arch/arm/mach-omap2/io.c
  +++ b/arch/arm/mach-omap2/io.c
  @@ -238,7 +238,7 @@ static void __init _omap2_map_common_io(void)
 local_flush_tlb_all();
 flush_cache_all();
 
  -  omap2_check_revision();
  +  omap_check_revision();
 omap_sram_init();
   }
 
  diff --git a/arch/arm/plat-omap/common.c b/arch/arm/plat-omap/common.c
  index fca73cd..4a0e333 100644
  --- a/arch/arm/plat-omap/common.c
  +++ b/arch/arm/plat-omap/common.c
  @@ -89,6 +89,12 @@ void __init omap_reserve(void)
 omap_vram_reserve_sdram_lmb();
   }
 
  +void __init omap_check_revision(void)
  +{
  +  omap1_check_revision();
  +  omap2_check_revision();
  +}
  +
   /*
* 32KHz clocksource ... always available, on pretty most chips except
* OMAP 730 and 1510.  Other timers could be used as clocksources, with
  diff --git a/arch/arm/plat-omap/include/plat/cpu.h 
  b/arch/arm/plat-omap/include/plat/cpu.h
  index 7514174..5f12a0b 100644
  --- a/arch/arm/plat-omap/include/plat/cpu.h
  +++ b/arch/arm/plat-omap/include/plat/cpu.h
  @@ -431,7 +431,18 @@ IS_OMAP_TYPE(3517, 0x3517)
 
 
   int omap_chip_is(struct omap_chip_id oci);
  -void omap2_check_revision(void);
  +#ifdef CONFIG_ARCH_OMAP2PLUS
  +extern void omap2_check_revision(void);
  +#else
  +static inline void omap2_check_revision(void) {}
  I think codingstyle suggest empty function braces to be on next line
  like
  static inline void omap2_check_revision(void)
  {}
 
 are you sure about that? can you point me to the documentation for that?
 Style I followed is off:
 http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-
 2.6.git;a=blob;f=Documentation/SubmittingPatches;h

static inline function style (was Re: [PATCH 3/9 v3] omap: generic: introduce a single check_revision)

2010-06-25 Thread Nishanth Menon

Shilimkar, Santosh had written, on 06/25/2010 01:07 PM, the following:

-Original Message-
From: Menon, Nishanth
Sent: Friday, June 25, 2010 11:02 PM


[..]


--- a/arch/arm/plat-omap/include/plat/cpu.h
+++ b/arch/arm/plat-omap/include/plat/cpu.h
@@ -431,7 +431,18 @@ IS_OMAP_TYPE(3517, 0x3517)


 int omap_chip_is(struct omap_chip_id oci);
-void omap2_check_revision(void);
+#ifdef CONFIG_ARCH_OMAP2PLUS
+extern void omap2_check_revision(void);
+#else
+static inline void omap2_check_revision(void) {}

I think codingstyle suggest empty function braces to be on next line
like
static inline void omap2_check_revision(void)
{}

are you sure about that? can you point me to the documentation for that?
Style I followed is off:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-
2.6.git;a=blob;f=Documentation/SubmittingPatches;h=72651f788f4e3536149ef5e7ddfbed96a8f14d2f;hb=HEAD#l
661


I got similar comment long back and hence remembered. Looks like it's not 
explicitly documented

changing subject to get folks interested..

i would think that checkpatch should crib about it, but it being an 
automated script, could be messed up sometimes..


but I am curious -
static inline void foo(void) { }
static inline int foo(void)
{
return -ENODEV;
}

is the style I have seen to date. usually without a functional code, it 
made more sense to have it out of line and more in the style of a normal 
function..


could someone give any suggestions on this?
--
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html