The following patch changes the usages of PVR_440* into strcmp's with
the cpu_name field, and removes the defines altogether. The Ebony
portion was briefly tested.
Signed-off-by: Tom Rini <redacted>
arch/ppc/platforms/4xx/ebony.c | 13 ++++---------
arch/ppc/syslib/ibm440gx_common.c | 7 ++++---
include/asm-ppc/reg.h | 6 ------
3 files changed, 8 insertions(+), 18 deletions(-)
--- 1.11/arch/ppc/platforms/4xx/ebony.c 2005-03-04 23:41:48 -07:00
+++ edited/arch/ppc/platforms/4xx/ebony.c 2005-03-08 15:22:19 -07:00
@@ -97,15 +97,10 @@
* on Rev. C silicon then errata forces us to
* use the internal clock.
*/
- switch (PVR_REV(mfspr(PVR))) {
- case PVR_REV(PVR_440GP_RB):
- freq = EBONY_440GP_RB_SYSCLK;
- break;
- case PVR_REV(PVR_440GP_RC1):
- default:
- freq = EBONY_440GP_RC_SYSCLK;
- break;
- }
+ if (strcmp(cur_cpu_spec[0]->cpu_name, "440GP Rev. B") == 0)
+ freq = EBONY_440GP_RB_SYSCLK;
+ else
+ freq = EBONY_440GP_RC_SYSCLK;
ibm44x_calibrate_decr(freq);
}
--- 1.3/arch/ppc/syslib/ibm440gx_common.c 2004-11-07 19:08:31 -07:00
+++ edited/arch/ppc/syslib/ibm440gx_common.c 2005-03-21 08:20:17 -07:00@@ -221,9 +221,10 @@
/* Disable L2C on rev.A, rev.B and 800MHz version of rev.C,
enable it on all other revisions
*/
- u32 pvr = mfspr(PVR);
- if (pvr == PVR_440GX_RA || pvr == PVR_440GX_RB ||
- (pvr == PVR_440GX_RC && p->cpu > 667000000))
+ if (strcmp(cur_cpu_spec[0]->cpu_name, "440GX Rev. A") == 0 ||
+ strcmp(cur_cpu_spec[0]->cpu_name, "440GX Rev. B") == 0
+ || (strcmp(cur_cpu_spec[0]->cpu_name, "440GX Rev. C")
+ == 0 && p->cpu > 667000000))
ibm440gx_l2c_disable();
else
ibm440gx_l2c_enable();
--- 1.16/include/asm-ppc/reg.h 2005-03-04 23:41:17 -07:00
+++ edited/include/asm-ppc/reg.h 2005-03-21 08:21:22 -07:00
@@ -449,12 +449,6 @@
#define PVR_STB03XXX 0x40310000
#define PVR_NP405H 0x41410000
#define PVR_NP405L 0x41610000
-#define PVR_440GP_RB 0x40120440
-#define PVR_440GP_RC1 0x40120481
-#define PVR_440GP_RC2 0x40200481
-#define PVR_440GX_RA 0x51b21850
-#define PVR_440GX_RB 0x51b21851
-#define PVR_440GX_RC 0x51b21892
#define PVR_601 0x00010000
#define PVR_602 0x00050000
#define PVR_603 0x00030000
--
Tom Rini
http://gate.crashing.org/~trini/
On Mon, Mar 21, 2005 at 08:27:29AM -0700, Tom Rini wrote:
quoted hunk
The following patch changes the usages of PVR_440* into strcmp's with
the cpu_name field, and removes the defines altogether. The Ebony
portion was briefly tested.
Signed-off-by: Tom Rini <redacted>
arch/ppc/platforms/4xx/ebony.c | 13 ++++---------
arch/ppc/syslib/ibm440gx_common.c | 7 ++++---
include/asm-ppc/reg.h | 6 ------
3 files changed, 8 insertions(+), 18 deletions(-)
--- 1.11/arch/ppc/platforms/4xx/ebony.c 2005-03-04 23:41:48 -07:00
+++ edited/arch/ppc/platforms/4xx/ebony.c 2005-03-08 15:22:19 -07:00
@@ -97,15 +97,10 @@
* on Rev. C silicon then errata forces us to
* use the internal clock.
*/
- switch (PVR_REV(mfspr(PVR))) {
- case PVR_REV(PVR_440GP_RB):
- freq = EBONY_440GP_RB_SYSCLK;
- break;
- case PVR_REV(PVR_440GP_RC1):
- default:
- freq = EBONY_440GP_RC_SYSCLK;
- break;
- }
+ if (strcmp(cur_cpu_spec[0]->cpu_name, "440GP Rev. B") == 0)
+ freq = EBONY_440GP_RB_SYSCLK;
+ else
+ freq = EBONY_440GP_RC_SYSCLK;
ibm44x_calibrate_decr(freq);
}
--- 1.3/arch/ppc/syslib/ibm440gx_common.c 2004-11-07 19:08:31 -07:00
+++ edited/arch/ppc/syslib/ibm440gx_common.c 2005-03-21 08:20:17 -07:00@@ -221,9 +221,10 @@
/* Disable L2C on rev.A, rev.B and 800MHz version of rev.C,
enable it on all other revisions
*/
- u32 pvr = mfspr(PVR);
- if (pvr == PVR_440GX_RA || pvr == PVR_440GX_RB ||
- (pvr == PVR_440GX_RC && p->cpu > 667000000))
+ if (strcmp(cur_cpu_spec[0]->cpu_name, "440GX Rev. A") == 0 ||
+ strcmp(cur_cpu_spec[0]->cpu_name, "440GX Rev. B") == 0
+ || (strcmp(cur_cpu_spec[0]->cpu_name, "440GX Rev. C")
+ == 0 && p->cpu > 667000000))
While I applaud Tom's quest to eliminate _useless_ PVR defines I think
this change looks strange.
It substitutes simple and clear (for those who are reading this code)
check to something more involved without good reason, IMHO.
Also, it adds additional "point of failure" making this code more
fragile. We never used those CPU ID strings anywhere in the kernel
code before, people are used to the fact that they don't matter much
(maybe only for user-mode) and it's possible that during some future
"cleanup" code will break. One could say that we aren't allowed to
change such strings because something in user-mode could break, and
while I agree with that, I don't think it's good argument to _add_
additional point where code could break.
--
Eugene
On Mon, Mar 21, 2005 at 09:40:49AM -0800, Eugene Surovegin wrote:
[snip]
quoted
- u32 pvr = mfspr(PVR);
- if (pvr == PVR_440GX_RA || pvr == PVR_440GX_RB ||
- (pvr == PVR_440GX_RC && p->cpu > 667000000))
+ if (strcmp(cur_cpu_spec[0]->cpu_name, "440GX Rev. A") == 0 ||
+ strcmp(cur_cpu_spec[0]->cpu_name, "440GX Rev. B") == 0
+ || (strcmp(cur_cpu_spec[0]->cpu_name, "440GX Rev. C")
+ == 0 && p->cpu > 667000000))
While I applaud Tom's quest to eliminate _useless_ PVR defines I think
this change looks strange.
It substitutes simple and clear (for those who are reading this code)
check to something more involved without good reason, IMHO.
Also, it adds additional "point of failure" making this code more
fragile. We never used those CPU ID strings anywhere in the kernel
code before, people are used to the fact that they don't matter much
(maybe only for user-mode) and it's possible that during some future
"cleanup" code will break. One could say that we aren't allowed to
change such strings because something in user-mode could break, and
while I agree with that, I don't think it's good argument to _add_
additional point where code could break.
While talking with galak today on IRC, he mentioned one of the reason,
why we may want to use string comparison instead of PVR.
For some chips, e.g. for 8272 family, there is NO way (even if we use
IMMR[16-31] and REV_NUM in addition to PVR) to detect correctly CPU
(which I think is pretty lame, but I digress :). For such cases, board
port can "fixup" CPU name, if this information can be implied by the
board type/revision/etc.
So, I think having a way to specify/name/detect CPU which is more
general and even can solve some real problems, is beneficial.
Therefore, I don't object anymore to such changes :).
--
Eugene