Skip to contentSkip to conversation

fix gcc4 warning in asm-ppc/time.h

PATCH In review Christoph Hellwig 5 participants 7 messages Aug 2, 2005 – Aug 4, 2005

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 fix gcc4 warning in asm-ppc/time.h

A function must not return a const value, instead use __attribute_const__.

include/asm/time.h +2 -1
Index: linux-2.6/include/asm/time.h
===================================================================
@@ -10,6 +10,7 @@
 #define __ASM_TIME_H__
 
 #include <linux/config.h>
+#include <linux/compiler.h>
 #include <linux/types.h>
 #include <linux/rtc.h>
 #include <linux/threads.h>
@@ -58,7 +59,7 @@
 /* Accessor functions for the timebase (RTC on 601) registers. */
 /* If one day CONFIG_POWER is added just define __USE_RTC as 1 */
 #ifdef CONFIG_6xx
-extern __inline__ int const __USE_RTC(void) {
+extern __inline__ int __attribute_const__ __USE_RTC(void) {
 	return (mfspr(SPRN_PVR)>>16) == 1;
 }
 #else
#1 lore raw
Quoted 18 lines · Christoph Hellwig

A function must not return a const value, instead use __attribute_const__.

Index: linux-2.6/include/asm/time.h
===================================================================

include/asm/time.h +1 -0
@@ -10,6 +10,7 @@
 #define __ASM_TIME_H__
 
 #include <linux/config.h>
+#include <linux/compiler.h>
 #include <linux/types.h>
 #include <linux/rtc.h>
 #include <linux/threads.h>

include/asm/time.h line 65

Quoted 8 lines
include/asm/time.h +1 -1
@@ -58,7 +59,7 @@
 /* Accessor functions for the timebase (RTC on 601) registers. */
 /* If one day CONFIG_POWER is added just define __USE_RTC as 1 */
 #ifdef CONFIG_6xx
-extern __inline__ int const __USE_RTC(void) {
+extern __inline__ int __attribute_const__ __USE_RTC(void) {
 	return (mfspr(SPRN_PVR)>>16) == 1;
 }
 #else

Do we really want to do this? The above implies that mfspr(SPRN_PVR) will return the same value across the entire uptime of the kernel. Is that true?

Why is this function paying with const anyway?

#2 lore raw
Quoted 23 lines · Andrew Morton
Quoted 22 lines · Christoph Hellwig

A function must not return a const value, instead use __attribute_const__.

Index: linux-2.6/include/asm/time.h
===================================================================

include/asm/time.h +1 -1
@@ -10,6 +10,7 @@
 #define __ASM_TIME_H__
 
 #include <linux/config.h>
+#include <linux/compiler.h>
 #include <linux/types.h>
 #include <linux/rtc.h>
 #include <linux/threads.h>
@@ -58,7 +59,7 @@
 /* Accessor functions for the timebase (RTC on 601) registers. */
 /* If one day CONFIG_POWER is added just define __USE_RTC as 1 */
 #ifdef CONFIG_6xx
-extern __inline__ int const __USE_RTC(void) {
Quoted 8 lines

include/asm/time.h line 65

Quoted 4 lines
include/asm/time.h +1 -0
+extern __inline__ int __attribute_const__ __USE_RTC(void) {
 	return (mfspr(SPRN_PVR)>>16) == 1;
 }
 #else

Do we really want to do this? The above implies that mfspr(SPRN_PVR) will return the same value across the entire uptime of the kernel. Is that true?

Yes, it is a read-only register. PVR means processor version register and is linked to the revision of the silicon. Besides that it is only used in this context to check for processors of the 601 family (the very first PPC processors). Nobody expects them to be hotpluggable, nor even used in SMP systems for the matter.

Quoted 2 lines

Why is this function paying with const anyway?

I believe that it was originally to tell the compiler to be able to cache it across several tests (a good compiler on PPC would even cache the result of the test in a condition register to allow branch prediction to work optimally).

Otherwise the compiler would never cache it since mfspr is a volatile asm.

However this was written in pre gcc2.95 days so my memory may be failing.

Gabriel

#3 lore raw
Quoted 33 lines · Gabriel Paubert
Quoted 23 lines · Andrew Morton
Quoted 22 lines · Christoph Hellwig

A function must not return a const value, instead use __attribute_const__.

Index: linux-2.6/include/asm/time.h
===================================================================

include/asm/time.h +1 -1
@@ -10,6 +10,7 @@
 #define __ASM_TIME_H__
 
 #include <linux/config.h>
+#include <linux/compiler.h>
 #include <linux/types.h>
 #include <linux/rtc.h>
 #include <linux/threads.h>
@@ -58,7 +59,7 @@
 /* Accessor functions for the timebase (RTC on 601) registers. */
 /* If one day CONFIG_POWER is added just define __USE_RTC as 1 */
 #ifdef CONFIG_6xx
-extern __inline__ int const __USE_RTC(void) {
Quoted 8 lines

include/asm/time.h line 65

Quoted 4 lines
include/asm/time.h +1 -0
+extern __inline__ int __attribute_const__ __USE_RTC(void) {
 	return (mfspr(SPRN_PVR)>>16) == 1;
 }
 #else

Do we really want to do this? The above implies that mfspr(SPRN_PVR) will return the same value across the entire uptime of the kernel. Is that true?

Quoted 5 lines

Yes, it is a read-only register. PVR means processor version register and is linked to the revision of the silicon. Besides that it is only used in this context to check for processors of the 601 family (the very first PPC processors). Nobody expects them to be hotpluggable, nor even used in SMP systems for the matter.

Yes ... However, we want to get rid of PVR accesses. They are a problem for Xen-like virtualization without HW support (that is running the kernel actually in user space) among other things.

The above should be replaced by a test of CPU features.

Ben.

#4 lore raw
Quoted 40 lines · Benjamin Herrenschmidt
Quoted 33 lines · Gabriel Paubert
Quoted 23 lines · Andrew Morton
Quoted 22 lines · Christoph Hellwig

A function must not return a const value, instead use __attribute_const__.

Index: linux-2.6/include/asm/time.h
===================================================================

include/asm/time.h +1 -1
@@ -10,6 +10,7 @@
 #define __ASM_TIME_H__
 
 #include <linux/config.h>
+#include <linux/compiler.h>
 #include <linux/types.h>
 #include <linux/rtc.h>
 #include <linux/threads.h>
@@ -58,7 +59,7 @@
 /* Accessor functions for the timebase (RTC on 601) registers. */
 /* If one day CONFIG_POWER is added just define __USE_RTC as 1 */
 #ifdef CONFIG_6xx
-extern __inline__ int const __USE_RTC(void) {
Quoted 8 lines

include/asm/time.h line 65

Quoted 4 lines
include/asm/time.h +1 -0
+extern __inline__ int __attribute_const__ __USE_RTC(void) {
 	return (mfspr(SPRN_PVR)>>16) == 1;
 }
 #else

Do we really want to do this? The above implies that mfspr(SPRN_PVR) will return the same value across the entire uptime of the kernel. Is that true?

Quoted 5 lines

Yes, it is a read-only register. PVR means processor version register and is linked to the revision of the silicon. Besides that it is only used in this context to check for processors of the 601 family (the very first PPC processors). Nobody expects them to be hotpluggable, nor even used in SMP systems for the matter.

Quoted 5 lines

Yes ... However, we want to get rid of PVR accesses. They are a problem for Xen-like virtualization without HW support (that is running the kernel actually in user space) among other things.

The above should be replaced by a test of CPU features.

I agree, this is old code that predates CPU features (the first version was written in 1998), when the things were simple: no PM, no Altivec, no really available 64 bit machines.

Now I wonder whether current kernels really boot reliably on 601 (after all even something as stupid as the RI bit in the MSR is marked as non implemented on my 601 doc).

Gabriel

#5 lore raw
In reply to Andrew Morton
Quoted 3 lines · Andrew Morton

Do we really want to do this? The above implies that mfspr(SPRN_PVR) will return the same value across the entire uptime of the kernel. Is that true?

Yes, the PVR is the processor version register, and it would be quite disturbing if the silicon changed underneath us. :)

Quoted 1 line

Why is this function paying with const anyway?

It seemed like a good idea at the time? :)

Paul.

#6 lore raw
In reply to Christoph Hellwig
Quoted 9 lines · Christoph Hellwig

A function must not return a const value, instead use __attribute_const__.

Index: linux-2.6/include/asm/time.h
===================================================================
--- linux-2.6.orig/include/asm/time.h 2005-04-30 10:17:19.000000000 +0200
+++ linux-2.6/include/asm/time.h 2005-08-01 11:57:12.000000000 +0200

asm-ppc, please. That didn't apply very well on an x86 tree...