Documentation/timers/hpet_example.c compile bug

PATCH Superseded Heiko Carstens 4 participants 9 messages May 28, 2010 – Jun 1, 2010

Subsystems (2)

Documentation High-resolution timers, timer wheel, clockevents

lore

Share & tools

lore

Heiko Carstens

author

2010-05-28 11:34 UTC #0 lore raw

Hi Randy,

since your commit e61863099f58e458f08c101da87da752a1e009f5 "Documentation/timers/: split txt and source files" I get this compile error on s390 with an allyesconfig:

In file included from Documentation/timers/hpet_example.c:15:
/usr/include/linux/hpet.h:11: error: expected specifier-qualifier-list before 'u64'

Please fix.

Thanks!

Randy Dunlap

2010-05-29 18:42 UTC #1 lore raw

Quoted 8 lines · Heiko Carstens

Hi Randy,
since your commit e61863099f58e458f08c101da87da752a1e009f5 "Documentation/timers/: split txt and source files" I get this compile error on s390 with an allyesconfig:
In file included from Documentation/timers/hpet_example.c:15:
/usr/include/linux/hpet.h:11: error: expected specifier-qualifier-list before 'u64'

Where is the u64? My (x86_64) copy of /usr/include/linux/hpet.h looks like:

     1	#ifndef	__HPET__
     2	#define	__HPET__ 1
     3	
     4	
     5	
     6	
     7	struct hpet_info {
     8		unsigned long hi_ireqfreq;	/* Hz */
     9		unsigned long hi_flags;	/* information */
    10		unsigned short hi_hpet;
    11		unsigned short hi_timer;
    12	};
    13	
    14	#define HPET_INFO_PERIODIC	0x0010	/* periodic-capable comparator */
    15	
    16	#define	HPET_IE_ON	_IO('h', 0x01)	/* interrupt on */
    17	#define	HPET_IE_OFF	_IO('h', 0x02)	/* interrupt off */
    18	#define	HPET_INFO	_IOR('h', 0x03, struct hpet_info)
    19	#define	HPET_EPI	_IO('h', 0x04)	/* enable periodic */
    20	#define	HPET_DPI	_IO('h', 0x05)	/* disable periodic */
    21	#define	HPET_IRQFREQ	_IOW('h', 0x6, unsigned long)	/* IRQFREQ usec */
    22	
    23	#endif				/* !__HPET__ */

I guess s390 hpet.h looks different? Does it just need to #include <linux/types.h> ? Or is the problem the kernel-space u64 vs. userspace __u64?

Quoted 1 line

Please fix.

Any suggestions?

Signature · 2 lines

~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Randy Dunlap

2010-05-31 02:36 UTC #2 lore raw

Quoted 36 lines · Randy Dunlap

Quoted 8 lines · Heiko Carstens
Hi Randy,
since your commit e61863099f58e458f08c101da87da752a1e009f5 "Documentation/timers/: split txt and source files" I get this compile error on s390 with an allyesconfig:
In file included from Documentation/timers/hpet_example.c:15:
/usr/include/linux/hpet.h:11: error: expected specifier-qualifier-list before 'u64'
Where is the u64? My (x86_64) copy of /usr/include/linux/hpet.h looks like:
     1	#ifndef	__HPET__
     2	#define	__HPET__ 1
     3	
     4	
     5	
     6	
     7	struct hpet_info {
     8		unsigned long hi_ireqfreq;	/* Hz */
     9		unsigned long hi_flags;	/* information */
    10		unsigned short hi_hpet;
    11		unsigned short hi_timer;
    12	};
    13	
    14	#define HPET_INFO_PERIODIC	0x0010	/* periodic-capable comparator */
    15	
    16	#define	HPET_IE_ON	_IO('h', 0x01)	/* interrupt on */
    17	#define	HPET_IE_OFF	_IO('h', 0x02)	/* interrupt off */
    18	#define	HPET_INFO	_IOR('h', 0x03, struct hpet_info)
    19	#define	HPET_EPI	_IO('h', 0x04)	/* enable periodic */
    20	#define	HPET_DPI	_IO('h', 0x05)	/* disable periodic */
    21	#define	HPET_IRQFREQ	_IOW('h', 0x6, unsigned long)	/* IRQFREQ usec */
    22	
    23	#endif				/* !__HPET__ */

Quoted 6 lines

I guess s390 hpet.h looks different? Does it just need to #include <linux/types.h> ? Or is the problem the kernel-space u64 vs. userspace __u64?
Quoted 1 line
Please fix.
Any suggestions?

Does it even make sense to build hpet_example on s390?
I.e., does s390 have hpet?
If not, then a simple Kconfig "depends on X86" or "depends on !S390" will fix this.

Signature · 2 lines

~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Arjan van de Ven

2010-05-31 02:46 UTC #3 lore raw

Quoted 5 lines · On Sun, 30 May 2010 19:34:35 -0700 Randy Dunlap <[email protected]> wrote:

Does it even make sense to build hpet_example on s390? I.e., does s390 have hpet?
If not, then a simple Kconfig "depends on X86" or "depends on !S390" will fix this.

HPET is very PC specific.

but.. even on x86, it does not really make sense, and exposing this as an interface to userspace is a historical mistake, the use of which we really ought to discourage.

Randy Dunlap

2010-05-31 02:59 UTC #4 lore raw

Quoted 8 lines · Arjan van de Ven

Quoted 5 lines · On Sun, 30 May 2010 19:34:35 -0700 Randy Dunlap <[email protected]> wrote:
Does it even make sense to build hpet_example on s390? I.e., does s390 have hpet?
If not, then a simple Kconfig "depends on X86" or "depends on !S390" will fix this.

Quoted 5 lines

HPET is very PC specific.
but.. even on x86, it does not really make sense, and exposing this as an interface to userspace is a historical mistake, the use of which we really ought to discourage.

Do you mean exposing anything that is in /usr/include/linux/hpet.h via ioctls? especially struct hpet_info ?

Signature · 2 lines

~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Arjan van de Ven

2010-05-31 03:07 UTC #5 lore raw

Quoted 9 lines · On Sun, 30 May 2010 19:58:38 -0700 Randy Dunlap <[email protected]> wrote:

Quoted 8 lines · Arjan van de Ven
Quoted 5 lines · On Sun, 30 May 2010 19:34:35 -0700 Randy Dunlap <[email protected]> wrote:
Does it even make sense to build hpet_example on s390? I.e., does s390 have hpet?
If not, then a simple Kconfig "depends on X86" or "depends on !S390" will fix this.

Quoted 8 lines

Quoted 5 lines
HPET is very PC specific.
but.. even on x86, it does not really make sense, and exposing this as an interface to userspace is a historical mistake, the use of which we really ought to discourage.
Do you mean exposing anything that is in /usr/include/linux/hpet.h via ioctls? especially struct hpet_info ?

exposing even the concept of HPET, so yeah, including that header. not all systems have hpet.. and frankly, we have better interfaces for the same functionality...

Signature · 3 lines

Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Randy Dunlap

2010-06-01 02:31 UTC #6 lore raw

PATCH 1/1 Documentation/timers/hpet_example.c: only build on X64

In reply to Heiko Carstens

From: Randy Dunlap <[email protected]>

We should only build hpet_example on X64, where it is implemented. It can cause build errors on other arch-es.

Signed-off-by Randy Dunlap <[email protected]>

Reported-by Heiko Carstens <[email protected]>

Cc Heiko Carstens <[email protected]>

Cc Arjan van de Ven <[email protected]>

---
 Documentation/timers/Makefile |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Documentation/timers/Makefile +1 -1

@@ -2,7 +2,7 @@
 obj- := dummy.o
 
 # List of programs to build
-hostprogs-y := hpet_example
+hostprogs-$(CONFIG_X86) := hpet_example
 
 # Tell kbuild to always build the programs
 always := $(hostprogs-y)

Linus Torvalds

2010-06-01 15:59 UTC #7 lore raw

Re: [PATCH] Documentation/timers/hpet_example.c: only build on X64

Quoted 3 lines · Randy Dunlap

We should only build hpet_example on X64, where it is implemented. It can cause build errors on other arch-es.

Both the subject and the body say "X64" (don't use that, btw, it's x86-64, please), but the patch itself says CONFIG_X86.

So what is it? Is it any x86, is it just 64-bit, or what?

So if it's any x86, then please say so in the explanation. And if it's really just x86-64, then use CONFIG_X86_64 as the config variable (and x86-64 rather than X64 in the commentary).

Linus

Randy Dunlap

2010-06-01 16:57 UTC #8 lore raw

PATCH v2 1/1 Documentation/timers/hpet_example.c: only build on X86

From: Randy Dunlap <[email protected]>

We should only build hpet_example on (any) X86, where it is implemented. It can cause build errors on other arch-es.

Signed-off-by Randy Dunlap <[email protected]>

Reported-by Heiko Carstens <[email protected]>

Cc Heiko Carstens <[email protected]>

Cc Arjan van de Ven <[email protected]>

---
 Documentation/timers/Makefile |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Documentation/timers/Makefile +1 -1

@@ -2,7 +2,7 @@
 obj- := dummy.o
 
 # List of programs to build
-hostprogs-y := hpet_example
+hostprogs-$(CONFIG_X86) := hpet_example
 
 # Tell kbuild to always build the programs
 always := $(hostprogs-y)