From: Randy Dunlap <[email protected]>
Add __KERNEL__ block.
Use __KERNEL__ to allow ioctl interface to be usable.
Signed-off-by: Randy Dunlap <[email protected]>
---
include/linux/hpet.h | 36 ++++++++++++++++++++----------------
1 files changed, 20 insertions(+), 16 deletions(-)
--- linux-2616-work.orig/include/linux/hpet.h
+++ linux-2616-work/include/linux/hpet.h
@@ -3,6 +3,8 @@
#include <linux/compiler.h>
+#ifdef __KERNEL__
+
/*
* Offsets into HPET Registers
*/
@@ -85,22 +87,6 @@ struct hpet {
#define Tn_FSB_INT_ADDR_SHIFT (32UL)
#define Tn_FSB_INT_VAL_MASK (0x00000000ffffffffULL)
-struct hpet_info {
- unsigned long hi_ireqfreq; /* Hz */
- unsigned long hi_flags; /* information */
- unsigned short hi_hpet;
- unsigned short hi_timer;
-};
-
-#define HPET_INFO_PERIODIC 0x0001 /* timer is periodic */
-
-#define HPET_IE_ON _IO('h', 0x01) /* interrupt on */
-#define HPET_IE_OFF _IO('h', 0x02) /* interrupt off */
-#define HPET_INFO _IOR('h', 0x03, struct hpet_info)
-#define HPET_EPI _IO('h', 0x04) /* enable periodic */
-#define HPET_DPI _IO('h', 0x05) /* disable periodic */
-#define HPET_IRQFREQ _IOW('h', 0x6, unsigned long) /* IRQFREQ usec */
-
/*
* exported interfaces
*/
@@ -133,4 +119,22 @@ int hpet_register(struct hpet_task *, in
int hpet_unregister(struct hpet_task *);
int hpet_control(struct hpet_task *, unsigned int, unsigned long);
+#endif /* __KERNEL__ */
+
+struct hpet_info {
+ unsigned long hi_ireqfreq; /* Hz */
+ unsigned long hi_flags; /* information */
+ unsigned short hi_hpet;
+ unsigned short hi_timer;
+};
+
+#define HPET_INFO_PERIODIC 0x0001 /* timer is periodic */
+
+#define HPET_IE_ON _IO('h', 0x01) /* interrupt on */
+#define HPET_IE_OFF _IO('h', 0x02) /* interrupt off */
+#define HPET_INFO _IOR('h', 0x03, struct hpet_info)
+#define HPET_EPI _IO('h', 0x04) /* enable periodic */
+#define HPET_DPI _IO('h', 0x05) /* disable periodic */
+#define HPET_IRQFREQ _IOW('h', 0x6, unsigned long) /* IRQFREQ usec */
+
#endif /* !__HPET__ */
---
"Randy.Dunlap" <[email protected]> wrote:
>
> From: Randy Dunlap <[email protected]>
>
> Add __KERNEL__ block.
> Use __KERNEL__ to allow ioctl interface to be usable.
hm, why?
My general approach to __KERNEL__ fixes is to not support new includers of
kernel headers but to accept patches which fix up existing applications of
__KERNEL__.
It's basically a compromise between the
dont-include-kernel-headers-from-userspace fundamentalists and the
but-i-want-my-stuff-to-work pragmatists ;)
But hpet.h never had __KERNEL__, so there's no regression here.
That being said, it looks like a sensible change - let's see if we can
sneak it in without the fundies noticing.
oops.
On Tue, 21 Mar 2006 16:13:03 -0800 Andrew Morton wrote:
> "Randy.Dunlap" <[email protected]> wrote:
> >
> > From: Randy Dunlap <[email protected]>
> >
> > Add __KERNEL__ block.
> > Use __KERNEL__ to allow ioctl interface to be usable.
>
> hm, why?
because there is a test/example source file in (inside)
Documentation/hpet.txt that won't build otherwise.
And because hpet.h contains _userspace_ ioctl interface struct
and macros...
> My general approach to __KERNEL__ fixes is to not support new includers of
> kernel headers but to accept patches which fix up existing applications of
> __KERNEL__.
>
> It's basically a compromise between the
> dont-include-kernel-headers-from-userspace fundamentalists and the
> but-i-want-my-stuff-to-work pragmatists ;)
>
> But hpet.h never had __KERNEL__, so there's no regression here.
>
> That being said, it looks like a sensible change - let's see if we can
> sneak it in without the fundies noticing.
---
~Randy
On Tue, 2006-03-21 at 16:26 -0800, Randy.Dunlap wrote:
> On Tue, 21 Mar 2006 16:13:03 -0800 Andrew Morton wrote:
>
> > "Randy.Dunlap" <[email protected]> wrote:
> > >
> > > From: Randy Dunlap <[email protected]>
> > >
> > > Add __KERNEL__ block.
> > > Use __KERNEL__ to allow ioctl interface to be usable.
> >
> > hm, why?
>
> because there is a test/example source file in (inside)
> Documentation/hpet.txt that won't build otherwise.
> And because hpet.h contains _userspace_ ioctl interface struct
> and macros...
then please split the header in 2 parts; one for the kernel
and one for userspace
either put both here, or move the kernel one to the directory where the
source code is
On Tuesday 21 March 2006 23:46, Randy.Dunlap wrote:
> +#define????????HPET_IE_ON??????_IO('h', 0x01)??/* interrupt on */
> +#define????????HPET_IE_OFF?????_IO('h', 0x02)??/* interrupt off */
> +#define????????HPET_INFO???????_IOR('h', 0x03, struct hpet_info)
> +#define????????HPET_EPI????????_IO('h', 0x04)??/* enable periodic */
> +#define????????HPET_DPI????????_IO('h', 0x05)??/* disable periodic */
> +#define????????HPET_IRQFREQ????_IOW('h', 0x6, unsigned long)???/* IRQFREQ usec */
By the way, HPET_INFO and HPET_IRQFREQ don't work for 32 bit user space,
the driver needs a compat_ioctl() method to handle those.
Arnd <><
Arnd Bergmann wrote:
> On Tuesday 21 March 2006 23:46, Randy.Dunlap wrote:
> > +#define HPET_IE_ON _IO('h', 0x01) /* interrupt on */
> > +#define HPET_IE_OFF _IO('h', 0x02) /* interrupt off */
> > +#define HPET_INFO _IOR('h', 0x03, struct hpet_info)
> > +#define HPET_EPI _IO('h', 0x04) /* enable periodic */
> > +#define HPET_DPI _IO('h', 0x05) /* disable periodic */
> > +#define HPET_IRQFREQ _IOW('h', 0x6, unsigned long) /* IRQFREQ usec */
>
> By the way, HPET_INFO and HPET_IRQFREQ don't work for 32 bit user space,
> the driver needs a compat_ioctl() method to handle those.
There isn't any program (except the example in the docs) that uses any
of these ioctls, and I'm writing patches to make this device available
through portable timer APIs like hrtimer/POSIX clocks/ALSA that are much
easier to use besides, so I think it would be a good idea to just
schedule these ioctls for removal.
Clemens
On Wed, 22 Mar 2006 12:14:46 +0100 Clemens Ladisch wrote:
> Arnd Bergmann wrote:
> > On Tuesday 21 March 2006 23:46, Randy.Dunlap wrote:
> > > +#define HPET_IE_ON _IO('h', 0x01) /* interrupt on */
> > > +#define HPET_IE_OFF _IO('h', 0x02) /* interrupt off */
> > > +#define HPET_INFO _IOR('h', 0x03, struct hpet_info)
> > > +#define HPET_EPI _IO('h', 0x04) /* enable periodic */
> > > +#define HPET_DPI _IO('h', 0x05) /* disable periodic */
> > > +#define HPET_IRQFREQ _IOW('h', 0x6, unsigned long) /* IRQFREQ usec */
> >
> > By the way, HPET_INFO and HPET_IRQFREQ don't work for 32 bit user space,
> > the driver needs a compat_ioctl() method to handle those.
>
> There isn't any program (except the example in the docs) that uses any
> of these ioctls, and I'm writing patches to make this device available
> through portable timer APIs like hrtimer/POSIX clocks/ALSA that are much
> easier to use besides, so I think it would be a good idea to just
> schedule these ioctls for removal.
How do you (or can you) know that there are no programs that use
that ioctl? Yes, scheduling for removal would be OK.
---
~Randy
On Wed, 22 Mar 2006 10:02:19 +0100 Arjan van de Ven wrote:
> On Tue, 2006-03-21 at 16:26 -0800, Randy.Dunlap wrote:
> > On Tue, 21 Mar 2006 16:13:03 -0800 Andrew Morton wrote:
> >
> > > "Randy.Dunlap" <[email protected]> wrote:
> > > >
> > > > From: Randy Dunlap <[email protected]>
> > > >
> > > > Add __KERNEL__ block.
> > > > Use __KERNEL__ to allow ioctl interface to be usable.
> > >
> > > hm, why?
> >
> > because there is a test/example source file in (inside)
> > Documentation/hpet.txt that won't build otherwise.
> > And because hpet.h contains _userspace_ ioctl interface struct
> > and macros...
>
>
> then please split the header in 2 parts; one for the kernel
> and one for userspace
so would you tell me what the purpose (use) of __KERNEL__
is meant to be, please?
Fortunately there are only about 165 header files in include/
that use both __KERNEL__ and _IO() macros (out of 5425 header
files).
> either put both here, or move the kernel one to the directory where the
> source code is
---
~Randy
On Wed, 2006-03-22 at 09:26 -0800, Randy.Dunlap wrote:
> On Wed, 22 Mar 2006 10:02:19 +0100 Arjan van de Ven wrote:
>
> > On Tue, 2006-03-21 at 16:26 -0800, Randy.Dunlap wrote:
> > > On Tue, 21 Mar 2006 16:13:03 -0800 Andrew Morton wrote:
> > >
> > > > "Randy.Dunlap" <[email protected]> wrote:
> > > > >
> > > > > From: Randy Dunlap <[email protected]>
> > > > >
> > > > > Add __KERNEL__ block.
> > > > > Use __KERNEL__ to allow ioctl interface to be usable.
> > > >
> > > > hm, why?
> > >
> > > because there is a test/example source file in (inside)
> > > Documentation/hpet.txt that won't build otherwise.
> > > And because hpet.h contains _userspace_ ioctl interface struct
> > > and macros...
> >
> >
> > then please split the header in 2 parts; one for the kernel
> > and one for userspace
>
> so would you tell me what the purpose (use) of __KERNEL__
> is meant to be, please?
for legacy headers.. the same ;)
Thats no reason to fix up new cases... things should get better not just
get a small rubber bandaid...
On Wed, 22 Mar 2006 21:08:02 +0100 Arjan van de Ven wrote:
> On Wed, 2006-03-22 at 09:26 -0800, Randy.Dunlap wrote:
> > On Wed, 22 Mar 2006 10:02:19 +0100 Arjan van de Ven wrote:
> >
> > > On Tue, 2006-03-21 at 16:26 -0800, Randy.Dunlap wrote:
> > > > On Tue, 21 Mar 2006 16:13:03 -0800 Andrew Morton wrote:
> > > >
> > > > > "Randy.Dunlap" <[email protected]> wrote:
> > > > > >
> > > > > > From: Randy Dunlap <[email protected]>
> > > > > >
> > > > > > Add __KERNEL__ block.
> > > > > > Use __KERNEL__ to allow ioctl interface to be usable.
> > > > >
> > > > > hm, why?
> > > >
> > > > because there is a test/example source file in (inside)
> > > > Documentation/hpet.txt that won't build otherwise.
> > > > And because hpet.h contains _userspace_ ioctl interface struct
> > > > and macros...
> > >
> > >
> > > then please split the header in 2 parts; one for the kernel
> > > and one for userspace
> >
> > so would you tell me what the purpose (use) of __KERNEL__
> > is meant to be, please?
>
> for legacy headers.. the same ;)
"the same" as what? I'm not understanding these partial sentences.
> Thats no reason to fix up new cases... things should get better not just
> get a small rubber bandaid...
Yes, I'm not disagreeing with the no-bandaids part. I just haven't
read that __KERNEL__ is legacy only.
---
~Randy
On Wednesday 22 March 2006 12:14, Clemens Ladisch wrote:
> There isn't any program (except the example in the docs) that uses any
> of these ioctls, and I'm writing patches to make this device available
> through portable timer APIs like hrtimer/POSIX clocks/ALSA that are much
> easier to use besides, so I think it would be a good idea to just
> schedule these ioctls for removal.
Ok, in that case I guess all of the header file should be wrapped inside
#ifdef __KERNEL__. Until now it was not possible to include that header
file in order to get the ioctl definition. It would be somewhat
counterproductive to schedule the user interface for removal while at
the same time making it easier to use it.
Also, I don't see any user of the ioctl function in the kernel, although
it's exported. Are there any out-of-tree users?
Arnd <><
On Wed, 2006-03-22 at 23:45 +0100, Arnd Bergmann wrote:
> On Wednesday 22 March 2006 12:14, Clemens Ladisch wrote:
> > There isn't any program (except the example in the docs) that uses any
> > of these ioctls, and I'm writing patches to make this device available
> > through portable timer APIs like hrtimer/POSIX clocks/ALSA that are much
> > easier to use besides, so I think it would be a good idea to just
> > schedule these ioctls for removal.
>
> Ok, in that case I guess all of the header file should be wrapped inside
> #ifdef __KERNEL__. Until now it was not possible to include that header
> file in order to get the ioctl definition. It would be somewhat
> counterproductive to schedule the user interface for removal while at
> the same time making it easier to use it.
>
> Also, I don't see any user of the ioctl function in the kernel, although
> it's exported. Are there any out-of-tree users?
hmm there used to be (in s390 iirc ;) but now it's a good opportunity
for Adrian to do a cleanup patch for it
Randy.Dunlap wrote:
> On Wed, 22 Mar 2006 12:14:46 +0100 Clemens Ladisch wrote:
> > There isn't any program (except the example in the docs) that
> > uses any of these ioctls, and I'm writing patches to make this
> > device available through portable timer APIs like hrtimer/POSIX
> > clocks/ALSA that are much easier to use besides, so I think it
> > would be a good idea to just schedule these ioctls for removal.
>
> How do you (or can you) know that there are no programs that use
> that ioctl?
Because most parts of hpet.c were orignially written on an emulator
and were so buggy as to be effectively unusable until recently.
Additionally, there aren't many BIOSes out there that manage to
initialize the third HPET timer (the one used by these ioctls)
correctly, or that bother to enable the HPET device at all. (This
seems to be changing due to the Windows Vista logo requirements.)
Regards,
Clemens