how to submit patches to i8k kernel driver

[Toby Murray]

Thanks for your response Paul.

On Mon, 2006-01-02 at 21:13 +0000, Paul Sladen wrote:
> the format of '/proc/i8k' already has a version field at the start:
>    vvvvv
>     1.0 A17 2J59L02 52 2 1 8040 6420 1 2
>     |   +-------------------------------------- 2.  [...]
>     +------------------------------------------ 1.  /proc/i8k format version
> 
> ...the best approach would probably be to bump the version number to '1.1'
> and add the extra fields you require on the end. 
This is what my patch does. It's just that it only bumps the version
number at the request of the user, not by default. The ioctl() is
paramterised by the version that the user wants the output in, which
means that apps can be written for a particular version of the output
and not need to be changed right away, every time the driver is extended
to include more info. 

There are a number of reasons for this, not the least of which is that
the majority of apps that I've seen that use /proc/i8k don't bother to
check the version number currently (gkrelm and GNOME Sensors Applet I'm
certain would be broken instantly if the format for /proc/i8k was
altered). 
 
That's why I wanted to "hide" the change behind an ioctl().

Either way, all existing applications will require to be patched. With
your approach, we immediately break them all and require them to be
patched. This is probably acceptable if the change is occuring within a
particular distribution (eg. ubuntu) since we can patch all relevant
userspace apps at hte same time. However, my initial goal had been to
try to get the patch accepted upstream, in which case I felt it would be
better to break as little existing stuff as possible.



>  Patching the
> userspace code is much safer and the various utilities can simply check for
> which version-number of data the driver is outputting in '/proc/i8k'. 
I agree in principle but none of the relevant apps appear to do this.
Hence the problem.

>  Since
> you'd have to patch all the applications to send the 'ioctl' you might
> aswell patch them to eat the extra data.
I definitely see your point here. My question then is, is it OK to break
all existing apps and expect them to be patched?

> 
> In addition, ioctl() commands are quite hard
> to call from shell scripts, as are probably not really a suitable interface.
This is true. 
> 
> An even better solution would be move this code to a 'dell_acpi' module
> exposing the appropriate ACPI events and data under '/proc/acpi/dell/*'.  
> This would also remove the need for the 'i8kbuttons' daemon.  My hunch is
> that you might be able to get this directly into Ubuntu as it progresses the
> laptop-mission goals and then have the Ubuntu kernel folks worry about
> pushing it upstream!
I'm not at all familiar with the acpi system, however I tend to agree
with you here. It would be much nicer to expose this stuff through
already existing acpi interfaces than through the custom /proc/i8k
interface.

Anyone out there with knowledge of acpi hooks that could provide some
info here would be great.

Thanks,
Toby