RE: [RFC] Common mechanism to identify Si revision
Hello Sanjeev, another question. I have heard that on OMAP4, some chip versions will not be identified with an ES revision number. How to handle those? - Paul -- 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: [RFC] Common mechanism to identify Si revision
-Original Message- From: Paul Walmsley [mailto:p...@pwsan.com] Sent: Thursday, September 10, 2009 2:59 PM To: Premi, Sanjeev Cc: linux-omap@vger.kernel.org Subject: Re: [RFC] Common mechanism to identify Si revision Hello Sanjeev, On Thu, 3 Sep 2009, Premi, Sanjeev wrote: Here, I am proposing a common mechanism to identify the si revision; that focuses on the revision bits alone. (See code below) The usage would then be (example): if (omap_rev() OMAP3430_REV_ES1_0) To if (cpu_is_omap34xx() OMAP_REV_GT(OMAP_ES_1_0) For use in structures like clock34xx.h and powerdomains34xx.h, would you propose that we use two different fields then - one for OMAP type, the other for revision restrictions? Or a different scheme? - Paul Paul, First answer: We could use two fields e.g. omap_type and omap_rev. For example: .omap_type= CHIP_IS_OMAP3430, .omap_rev = OMAP_ES_2_1, Instead of : .omap_chip= OMAP_CHIP_INIT(CHIP_IS_OMAP3430), .omap_chip= OMAP_CHIP_INIT(CHIP_IS_OMAP3430ES1 | CHIP_IS_OMAP3430ES2 | CHIP_IS_OMAP3430ES3_0), .omap_chip= OMAP_CHIP_INIT(CHIP_GE_OMAP3430ES3_1), Second answer: From the snippets above, it appears that we need to: 1) identify which chip is in use 2) which si rev (if any) does the clock/powerdomain definition applies to. Actually, we are checking for: a) Does definition depend upon a si rev b) if so, which rev? - usually LE and GE should be sufficient. Which chip in use? - can either be a run-time check OR mapped to an boot-time setting of valid field for the si being used. e.g. (translating the snippet above): .valid= (cpu_is_omap34xx()) .valid= (cpu_is_omap34xx() (OMAP_REV_EQ(OMAP_ES_1_0) || OMAP_REV_EQ(OMAP_ES_2_1) || OMAP_REV_EQ(OMAP_ES_3_0))) .valid= (cpu_is_omap34xx() (OMAP_REV_GE(OMAP_ES_3_1)) Now decision at runtime can be made based on the just this flag. I haven't really done any prototyping on this, so there may be holes in explanation above; but I hope general idea is coming clear. Best regards, Sanjeev -- 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: [RFC] Common mechanism to identify Si revision
Hi Kevin, -Original Message- From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap- ow...@vger.kernel.org] On Behalf Of Kevin Hilman Sent: Thursday, September 10, 2009 7:55 PM To: Olof Johansson Cc: Premi, Sanjeev; linux-omap@vger.kernel.org Subject: Re: [RFC] Common mechanism to identify Si revision Olof Johansson o...@lixom.net writes: On Thu, Sep 03, 2009 at 04:14:28PM +0530, Premi, Sanjeev wrote: Hi, Currently there are multiple mechanisms for identifying the si revisions. Most places the comparison is against omap_rev() as a whole number. Example: arch/arm/mach-omap2/board-3430sdp.c:695:if (omap_rev() OMAP3430_REV_ES1_0) arch/arm/mach-omap2/board-3430sdp.c:728:if (omap_rev() OMAP3430_REV_ES1_0) Then, there are custom macros. Example (cpu.h): #define CHIP_GE_OMAP3430ES3_1 (CHIP_IS_OMAP3430ES3_1) #define CHIP_GE_OMAP3430ES3(CHIP_IS_OMAP3430ES3_0 | \ CHIP_GE_OMAP3430ES3_1) #define CHIP_GE_OMAP3430ES2(CHIP_IS_OMAP3430ES2 | \ CHIP_GE_OMAP3430ES3) The problem with comparing against a whole number is that comparison is invalid for another processor series. E.g. OMAP3430 and OMAP3517. Here, I am proposing a common mechanism to identify the si revision; that focuses on the revision bits alone. (See code below) The usage would then be (example): if (omap_rev() OMAP3430_REV_ES1_0) To if (cpu_is_omap34xx() OMAP_REV_GT(OMAP_ES_1_0) What's the purpose of most of these checks in the first place? I can see two immediate needs: 1) To check for various errata and do appropriate workarounds 2) To check if the current chip has a certain feature Both of these could just as well be abstracted away such that you use tests on the form: if (OMAP_HAS_ERRATA_FOO) ... or: if (OMAP_FEATURE_FOO) ... And then move the actual checking of a feature into the header file where the errata/feature setups are defined. There's two major benefits to this: 1) Readability. No need to sit and look up in a manual why there's a check for version X here. (and/or no need to add a specific comment about it). 2) Keeping changes centralized. If there's a new revision or chip, there's just one header file to update, not 20 different source files. For example, a bunch of the checks in pm34xx.c would be nicer to have as: if (OMAP_HAS_USBHOST()) I tend to agree with Olaf here and am in favor of the new 'features' patch that Sanjeev has already proposed. I agree as well, most of the differences between several ES and OMAP variant are related to changes in IPs and not really to the chip version. That doesn't mean that I'm opposed to this change in principle, but would rather see most of the omap_rev() and cpu_is_* checks disappear in favor of more generic omap_has_feature() checks. In that case taking into account the feature version might be useful in order to get rid of most of the omap_rev(). Most of the time new ES are due to new feature or bug fix in an IP that will increase the IP version. Regards, Benoit -- 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: [RFC] Common mechanism to identify Si revision
-Original Message- From: Kevin Hilman [mailto:khil...@deeprootsystems.com] Sent: Thursday, September 10, 2009 11:25 PM To: Olof Johansson Cc: Premi, Sanjeev; linux-omap@vger.kernel.org Subject: Re: [RFC] Common mechanism to identify Si revision snip--snip There's two major benefits to this: 1) Readability. No need to sit and look up in a manual why there's a check for version X here. (and/or no need to add a specific comment about it). 2) Keeping changes centralized. If there's a new revision or chip, there's just one header file to update, not 20 different source files. For example, a bunch of the checks in pm34xx.c would be nicer to have as: if (OMAP_HAS_USBHOST()) I tend to agree with Olaf here and am in favor of the new 'features' patch that Sanjeev has already proposed. That doesn't mean that I'm opposed to this change in principle, but would rather see most of the omap_rev() and cpu_is_* checks disappear in favor of more generic omap_has_feature() checks. Kevin Tony, I am planning to submit another patch for speed - enhanced part for OMAP3530. I see enhanced speed as a feature; and would like to build upon the last submission. BTW, there is no change in ES revision for new part. Just additional set of control bits in TAP area. Do you see any issues with the patch: [1] http://marc.info/?l=linux-omapm=125050987112798w=2 Best regards, Sanjeev -- 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: [RFC] Common mechanism to identify Si revision
Hello Sanjeev, On Thu, 3 Sep 2009, Premi, Sanjeev wrote: Here, I am proposing a common mechanism to identify the si revision; that focuses on the revision bits alone. (See code below) The usage would then be (example): if (omap_rev() OMAP3430_REV_ES1_0) To if (cpu_is_omap34xx() OMAP_REV_GT(OMAP_ES_1_0) For use in structures like clock34xx.h and powerdomains34xx.h, would you propose that we use two different fields then - one for OMAP type, the other for revision restrictions? Or a different scheme? - Paul -- 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: [RFC] Common mechanism to identify Si revision
Olof Johansson o...@lixom.net writes: On Thu, Sep 03, 2009 at 04:14:28PM +0530, Premi, Sanjeev wrote: Hi, Currently there are multiple mechanisms for identifying the si revisions. Most places the comparison is against omap_rev() as a whole number. Example: arch/arm/mach-omap2/board-3430sdp.c:695:if (omap_rev() OMAP3430_REV_ES1_0) arch/arm/mach-omap2/board-3430sdp.c:728:if (omap_rev() OMAP3430_REV_ES1_0) Then, there are custom macros. Example (cpu.h): #define CHIP_GE_OMAP3430ES3_1(CHIP_IS_OMAP3430ES3_1) #define CHIP_GE_OMAP3430ES3 (CHIP_IS_OMAP3430ES3_0 | \ CHIP_GE_OMAP3430ES3_1) #define CHIP_GE_OMAP3430ES2 (CHIP_IS_OMAP3430ES2 | \ CHIP_GE_OMAP3430ES3) The problem with comparing against a whole number is that comparison is invalid for another processor series. E.g. OMAP3430 and OMAP3517. Here, I am proposing a common mechanism to identify the si revision; that focuses on the revision bits alone. (See code below) The usage would then be (example): if (omap_rev() OMAP3430_REV_ES1_0) To if (cpu_is_omap34xx() OMAP_REV_GT(OMAP_ES_1_0) What's the purpose of most of these checks in the first place? I can see two immediate needs: 1) To check for various errata and do appropriate workarounds 2) To check if the current chip has a certain feature Both of these could just as well be abstracted away such that you use tests on the form: if (OMAP_HAS_ERRATA_FOO) ... or: if (OMAP_FEATURE_FOO) ... And then move the actual checking of a feature into the header file where the errata/feature setups are defined. There's two major benefits to this: 1) Readability. No need to sit and look up in a manual why there's a check for version X here. (and/or no need to add a specific comment about it). 2) Keeping changes centralized. If there's a new revision or chip, there's just one header file to update, not 20 different source files. For example, a bunch of the checks in pm34xx.c would be nicer to have as: if (OMAP_HAS_USBHOST()) I tend to agree with Olaf here and am in favor of the new 'features' patch that Sanjeev has already proposed. That doesn't mean that I'm opposed to this change in principle, but would rather see most of the omap_rev() and cpu_is_* checks disappear in favor of more generic omap_has_feature() checks. Kevin -- 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: [RFC] Common mechanism to identify Si revision
-Original Message- From: Olof Johansson [mailto:o...@lixom.net] Sent: Sunday, September 06, 2009 6:17 PM To: Premi, Sanjeev Cc: linux-omap@vger.kernel.org Subject: Re: [RFC] Common mechanism to identify Si revision On Thu, Sep 03, 2009 at 04:14:28PM +0530, Premi, Sanjeev wrote: Hi, Currently there are multiple mechanisms for identifying the si revisions. Most places the comparison is against omap_rev() as a whole number. Example: arch/arm/mach-omap2/board-3430sdp.c:695:if (omap_rev() OMAP3430_REV_ES1_0) arch/arm/mach-omap2/board-3430sdp.c:728:if (omap_rev() OMAP3430_REV_ES1_0) Then, there are custom macros. Example (cpu.h): #define CHIP_GE_OMAP3430ES3_1 (CHIP_IS_OMAP3430ES3_1) #define CHIP_GE_OMAP3430ES3 (CHIP_IS_OMAP3430ES3_0 | \ CHIP_GE_OMAP3430ES3_1) #define CHIP_GE_OMAP3430ES2 (CHIP_IS_OMAP3430ES2 | \ CHIP_GE_OMAP3430ES3) The problem with comparing against a whole number is that comparison is invalid for another processor series. E.g. OMAP3430 and OMAP3517. Here, I am proposing a common mechanism to identify the si revision; that focuses on the revision bits alone. (See code below) The usage would then be (example): if (omap_rev() OMAP3430_REV_ES1_0) To if (cpu_is_omap34xx() OMAP_REV_GT(OMAP_ES_1_0) What's the purpose of most of these checks in the first place? I can see two immediate needs: 1) To check for various errata and do appropriate workarounds [sp] I believe the only need would be to make easy check if the version has multiple errata fixes and/or enhancements. And, of course, a verbose information to user who may not have (and may not need) information of all the errata/enhancements. Today, there are multiple ways of doing the same thing... Each way builds upon minor issues in the existing one; but adds its own. 2) To check if the current chip has a certain feature Both of these could just as well be abstracted away such that you use tests on the form: if (OMAP_HAS_ERRATA_FOO) ... or: if (OMAP_FEATURE_FOO) ... And then move the actual checking of a feature into the header file where the errata/feature setups are defined. [sp I have submitted a patch that takes the first step toward this: http://marc.info/?l=linux-omapm=125050987112798w=2 ...still waiting to hear from Tony on this. There's two major benefits to this: 1) Readability. No need to sit and look up in a manual why there's a check for version X here. (and/or no need to add a specific comment about it). 2) Keeping changes centralized. If there's a new revision or chip, there's just one header file to update, not 20 different source files. For example, a bunch of the checks in pm34xx.c would be nicer to have as: if (OMAP_HAS_USBHOST()) [sp] Can you look and comment on this discussion as well: http://marc.info/?l=linux-omapm=125017671720718w=2 Best regards, Sanjeev Then the current settings. -Olof -- 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: [RFC] Common mechanism to identify Si revision
Hi, On Mon, Sep 07, 2009 at 11:33:32AM +0530, Premi, Sanjeev wrote: What's the purpose of most of these checks in the first place? I can see two immediate needs: 1) To check for various errata and do appropriate workarounds [sp] I believe the only need would be to make easy check if the version has multiple errata fixes and/or enhancements. And, of course, a verbose information to user who may not have (and may not need) information of all the errata/enhancements. Today, there are multiple ways of doing the same thing... Each way builds upon minor issues in the existing one; but adds its own. Yeah, ok. I was just fearing that this added ES-level checking would be used to clutter up the code with extra testing instead of hiding it away in header files. But it seems like most of it is covered already. 2) To check if the current chip has a certain feature Both of these could just as well be abstracted away such that you use tests on the form: if (OMAP_HAS_ERRATA_FOO) ... or: if (OMAP_FEATURE_FOO) ... And then move the actual checking of a feature into the header file where the errata/feature setups are defined. [sp I have submitted a patch that takes the first step toward this: http://marc.info/?l=linux-omapm=125050987112798w=2 ...still waiting to hear from Tony on this. Looks good. I didn't subscribe to linux-omap until recently so I don't have the original emails (and thus can't comment on them), but my only feedback is that it really doesn't belong in /proc/cpuinfo whether a SoC has SGX on-chip or not. It'd be better to move that information somewhere else. Keep in mind that adding information to a /proc file means that you are changing an API that we would need to live with forever. There's two major benefits to this: 1) Readability. No need to sit and look up in a manual why there's a check for version X here. (and/or no need to add a specific comment about it). 2) Keeping changes centralized. If there's a new revision or chip, there's just one header file to update, not 20 different source files. For example, a bunch of the checks in pm34xx.c would be nicer to have as: if (OMAP_HAS_USBHOST()) [sp] Can you look and comment on this discussion as well: http://marc.info/?l=linux-omapm=125017671720718w=2 I think it is a great step in the right direction. My only concern is that the usage conventions are confusing: OMAP3_HAS_FEATURE(neon, NEON) Just seeing that, what is the difference between neon and NEON? Also, it always requires two consecutive calls to check for a feature (get the flags, then check the mask). I would personally prefer to keep a global that is initialized early in boot (called __omap_features or so -- the __-prefix would indicate that it is not for direct use in code), then have OMAP3_HAS_FEATURE() just look at that value instead of having it passed in. It's not like it changes during runtime anyway. It can be debated if the format should then be OMAP3_HAS_FEATURE_SGX or OMAP3_HAS_FEATURE(SGX), but either is fine with me. -Olof -- 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: [RFC] Common mechanism to identify Si revision
On Sep 7, 2009, at 8:02 AM, Premi, Sanjeev wrote:
I think it is a great step in the right direction. My only concern is
that the usage conventions are confusing:
OMAP3_HAS_FEATURE(neon, NEON)
[sp] This is only used to declare a function that would translate to:
unsigned int omap3_has_neon(void)
{
return (omap_features OMAP_HAS_NEON);
}
EXPORT_SYMBOL(omap3_has_neon);
A user would always do something like:
if (omap_has_neon())
do_neon_specific_stuff();
Ok, all is well then. :)
-Olof
--
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: [RFC] Common mechanism to identify Si revision
-Original Message-
From: linux-omap-ow...@vger.kernel.org
[mailto:linux-omap-ow...@vger.kernel.org] On Behalf Of Premi, Sanjeev
Sent: Thursday, September 03, 2009 4:14 PM
To: linux-omap@vger.kernel.org
Subject: [RFC] Common mechanism to identify Si revision
Hi,
Currently there are multiple mechanisms for identifying the
si revisions.
Most places the comparison is against omap_rev() as a whole
number. Example:
arch/arm/mach-omap2/board-3430sdp.c:695:if (omap_rev()
OMAP3430_REV_ES1_0)
arch/arm/mach-omap2/board-3430sdp.c:728:if (omap_rev()
OMAP3430_REV_ES1_0)
Then, there are custom macros. Example (cpu.h):
#define CHIP_GE_OMAP3430ES3_1 (CHIP_IS_OMAP3430ES3_1)
#define CHIP_GE_OMAP3430ES3 (CHIP_IS_OMAP3430ES3_0 | \
CHIP_GE_OMAP3430ES3_1)
#define CHIP_GE_OMAP3430ES2 (CHIP_IS_OMAP3430ES2 | \
CHIP_GE_OMAP3430ES3)
The problem with comparing against a whole number is that
comparison is invalid
for another processor series. E.g. OMAP3430 and OMAP3517.
Here, I am proposing a common mechanism to identify the si
revision; that focuses
on the revision bits alone. (See code below)
The usage would then be (example):
if (omap_rev() OMAP3430_REV_ES1_0)
To
if (cpu_is_omap34xx() OMAP_REV_GT(OMAP_ES_1_0)
Though I have not looked at the OMAP1 directory, I am
assuming similar case there.
Since there aren't any new devices in the OMAP1 category,
this may not appear as
a need. But consistency could help.
Few items that I feel need to be looked at:
- Possible differences in the MASK defined for Si revision.
Irrespective of (possible) difference in the HW, the bits
in 'omap_revision' can
be same.
- How to phase in the change - one shot OR in steps.
I am not sending this as patch, for now; just to get the
discussion started.
Best regards,
Sanjeev
[sp] sorry, missed the return statement in the inline functions below:
... perils of writing code in mail :)
~sanjeev
(cpu.h)
/*
* Identify silicon revision.
*/
#define OMAP_REV_MASK 0xff00
#define OMAP_ES_1_0 0x00
#define OMAP_ES_2_0 0x10
#define OMAP_ES_2_1 0x20
#define OMAP_ES_3_0 0x30
#define OMAP_ES_3_1 0x40
#define OMAP_REV_IS(revid)
\
static inline u8 omap_rev_is_ ##revid (void)
\
{
\
(((omap_rev() OMAP_REV_MASK) 8) == revid) ? 1 : 0
; \
}
#define OMAP_REV_LT(revid)
\
static inline u8 omap_rev_lt_ ##revid (void)
\
{
\
(((omap_rev() OMAP_REV_MASK) 8) revid) ? 1 : 0 ; \
}
#define OMAP_REV_LE(revid)
\
static inline u8 omap_rev_le_ ##revid (void)
\
{
\
(((omap_rev() OMAP_REV_MASK) 8) = revid) ? 1 : 0
; \
}
#define OMAP_REV_GE(revid)
\
static inline u8 omap_rev_ge_ ##revid (void)
\
{
\
(((omap_rev() OMAP_REV_MASK) 8) = revid) ? 1 : 0
; \
}
#define OMAP_REV_GT(revid)
\
static inline u8 omap_rev_gt_ ##revid (void)
\
{
\
(((omap_rev() OMAP_REV_MASK) 8) revid) ? 1 : 0 ; \
}
OMAP_REV_IS(OMAP_ES_1_0)
OMAP_REV_LT(OMAP_ES_1_0)
OMAP_REV_LE(OMAP_ES_1_0)
OMAP_REV_GT(OMAP_ES_1_0)
OMAP_REV_GE(OMAP_ES_1_0)
OMAP_REV_IS(OMAP_ES_2_0)
OMAP_REV_LT(OMAP_ES_2_0)
OMAP_REV_LE(OMAP_ES_2_0)
OMAP_REV_GT(OMAP_ES_2_0)
OMAP_REV_GE(OMAP_ES_2_0)
OMAP_REV_IS(OMAP_ES_2_1)
OMAP_REV_LT(OMAP_ES_2_1)
OMAP_REV_LE(OMAP_ES_2_1)
OMAP_REV_GT(OMAP_ES_2_1)
OMAP_REV_GE(OMAP_ES_2_1)
OMAP_REV_IS(OMAP_ES_3_0)
OMAP_REV_LT(OMAP_ES_3_0)
OMAP_REV_LE(OMAP_ES_3_0)
OMAP_REV_GT(OMAP_ES_3_0)
OMAP_REV_GE(OMAP_ES_3_0)
OMAP_REV_IS(OMAP_ES_3_1)
OMAP_REV_LT(OMAP_ES_3_1)
OMAP_REV_LE(OMAP_ES_3_1)
OMAP_REV_GT(OMAP_ES_3_1)
OMAP_REV_GE(OMAP_ES_3_1)
--
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
--
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
