[patch 0/7] New misc/asus-laptop.c driver

[patch 0/7] New misc/asus-laptop.c driver

[Corentin CHARY]

Hi,
I've done a lot of work on asus_acpi, see :

diff asus_acpi.c asus-laptop.c -u | diffstat
 asus-laptop.c | 1663 
++++++++++++++++++++++++----------------------------------
 1 file changed, 711 insertions(+), 952 deletions(-)

So because the new driver work in a very different way, and have a new 
interface (no more /proc/acpi/asus/), I think it's time for 
misc/asus-laptop.c, but we should also keep asus_acpi.c for sometime.

In asus_acpi, we had to add all models by hand, and the list was growing very 
fast... The new detection system is inspired by ibm-acpi (we search well 
known acpi method, and if we found one then the feature is available).

Now leds are handled by /sys/class/led/asus:*/ , backlight 
with /sys/class/backlight/asus-laptop/ .
asus-laptop is now a platform driver, so files like wlan, bluetooth, 
ls_switch, ls_level, info, etc .. are now 
in /sys/devices/platform/asus-laptop/ .

These patch are designed to apply to  acpi-test-20060707-2.6.20-rc4, but as 
it's a new driver, it's easy to apply against other trees.

-- 
CHARY 'Iksaif' Corentin
http://xf.iksaif.net

Re: [patch 0/7] New misc/asus-laptop.c driver

[Len Brown]

On Thursday 25 January 2007 06:54, Corentin CHARY wrote:
> Hi,
> I've done a lot of work on asus_acpi
Excellent.

A couple of nits on e-mail patch format:

Please fit the comments within 80 lines.
ie. pipe them through fmt -w 68 or something.

The --- in your e-mail should come before the diffstat, not after.

thanks,
-Len

>From Documentation/SubmittingPatches

12) The canonical patch format

The canonical patch subject line is:

    Subject: [PATCH 001/123] subsystem: summary phrase

The canonical patch message body contains the following:

  - A "from" line specifying the patch author.

  - An empty line.

  - The body of the explanation, which will be copied to the
    permanent changelog to describe this patch.

  - The "Signed-off-by:" lines, described above, which will
    also go in the changelog.

  - A marker line containing simply "---".

  - Any additional comments not suitable for the changelog.

...
The "---" marker line serves the essential purpose of marking for patch
handling tools where the changelog message ends.

One good use for the additional comments after the "---" marker is for
a diffstat, to show what files have changed, and the number of inserted
and deleted lines per file.  A diffstat is especially useful on bigger
patches.  Other comments relevant only to the moment or the maintainer,
not suitable for the permanent changelog, should also go here.
Use diffstat options "-p 1 -w 70" so that filenames are listed from the
top of the kernel source tree and don't use too much horizontal space
(easily fit in 80 columns, maybe with some indentation).

  - The actual patch (diff output).
...

Re: [patch 0/7] New misc/asus-laptop.c driver

[Corentin CHARY]

Le vendredi 26 janvier 2007 08:51, Len Brown a écrit :
> On Thursday 25 January 2007 06:54, Corentin CHARY wrote:
> > Hi,
> > I've done a lot of work on asus_acpi
>
> Excellent.
>
> A couple of nits on e-mail patch format:
>
> Please fit the comments within 80 lines.
> ie. pipe them through fmt -w 68 or something.
>
> The --- in your e-mail should come before the diffstat, not after.
>
done :) thanks.
I just send the new series, with some cleanups (write_led -> write_status, 
etc ..), and some bugfix ... This series is made to apply on top of 
2.6.20-rc6. This time, my mails are well formatted, I hope :p.

-- 
CHARY 'Iksaif' Corentin
http://xf.iksaif.net
-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch 0/7] New misc/asus-laptop.c driver

[Len Brown]

On Friday 26 January 2007 08:04, Corentin CHARY wrote:
> Le vendredi 26 janvier 2007 08:51, Len Brown a écrit :
> > On Thursday 25 January 2007 06:54, Corentin CHARY wrote:
> > > Hi,
> > > I've done a lot of work on asus_acpi
> >
> > Excellent.
> >
> > A couple of nits on e-mail patch format:
> >
> > Please fit the comments within 80 lines.
> > ie. pipe them through fmt -w 68 or something.
> >
> > The --- in your e-mail should come before the diffstat, not after.
> >
> done :) thanks.
> I just send the new series, with some cleanups (write_led -> write_status, 
> etc ..), and some bugfix ... This series is made to apply on top of 
> 2.6.20-rc6. This time, my mails are well formatted, I hope :p.

1-7 applied to acpi-test
I also ran scripts/Lindent over it to fix the whitespace.
Then, after merging into acpi-test I also applied the patch below.

Your choice if you want to make future updates with incremental patches
on top of acpi-test or re-send the series.

Also, while I see that distros do set CONFIG_LEDS_CLASS,
depending on it will make the driver vanish from menuconfig
if somebody hasn't set it.  I don't know if this is a big deal or not.
One possibility is to not depend on it in Kconfig, but via #ifdef's in the source.
The other is to retreat to using select -- which I'm hopeful we can avoid.
Or, maybe folks that don't get the driver via distro (who tend to enable everything)
will not get stumped by this.

thanks
-Len

commit 57262852805a54ab312c00fe3129e8b99a24c94b
Author: Len Brown <len.brown@intel.com>
Date:   Tue Jan 30 02:44:03 2007 -0500

    asus-laptop: handle new ACPI table manager
    
    acpi_get_table() used to give us a copy of a table,
    now it gives us a mapping to the BIOS' copy of the table.
    
    Signed-off-by: Len Brown <len.brown@intel.com>

diff --git a/drivers/misc/asus-laptop.c b/drivers/misc/asus-laptop.c
index b624350..861c399 100644
--- a/drivers/misc/asus-laptop.c
+++ b/drivers/misc/asus-laptop.c
@@ -777,7 +777,6 @@ static int asus_handle_init(char *name, acpi_handle * handle,
 static int asus_hotk_get_info(void)
 {
 	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
-	struct acpi_buffer dsdt = { ACPI_ALLOCATE_BUFFER, NULL };
 	union acpi_object *model = NULL;
 	int bsts_result, hwrs_result;
 	char *string = NULL;
@@ -791,11 +790,9 @@ static int asus_hotk_get_info(void)
 	 * HID), this bit will be moved. A global variable asus_info contains
 	 * the DSDT header.
 	 */
-	status = acpi_get_table(ACPI_TABLE_ID_DSDT, 1, &dsdt);
+	status = acpi_get_table(ACPI_SIG_DSDT, 1, &asus_info);
 	if (ACPI_FAILURE(status))
 		printk(ASUS_WARNING "Couldn't get the DSDT table header\n");
-	else
-		asus_info = dsdt.pointer;
 
 	/* We have to write 0 on init this far for all ASUS models */
 	if (!write_acpi_int(hotk->handle, "INIT", 0, &buffer)) {
@@ -1014,8 +1011,6 @@ static void __exit asus_laptop_exit(void)
 	sysfs_remove_group(&asuspf_device->dev.kobj, &asuspf_attribute_group);
 	platform_device_unregister(asuspf_device);
 	platform_driver_unregister(&asuspf_driver);
-
-	kfree(asus_info);
 }
 
 static int asus_backlight_init(struct device *dev)
-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch 0/7] New misc/asus-laptop.c driver

[Corentin CHARY]

>
> 1-7 applied to acpi-test
> I also ran scripts/Lindent over it to fix the whitespace.
> Then, after merging into acpi-test I also applied the patch below.
>
> Your choice if you want to make future updates with incremental patches
> on top of acpi-test or re-send the series.
incremental patches on top of acpi-test are fine :)

> Also, while I see that distros do set CONFIG_LEDS_CLASS,
> depending on it will make the driver vanish from menuconfig
> if somebody hasn't set it.  I don't know if this is a big deal or not.
> One possibility is to not depend on it in Kconfig, but via #ifdef's in the
> source. The other is to retreat to using select -- which I'm hopeful we can
> avoid. Or, maybe folks that don't get the driver via distro (who tend to
> enable everything) will not get stumped by this.
As the old asus_acpi doesn't work on new models, and still buggy, I think 
distros will enable asus-laptop (and LED/BACKLIGHT) by default ... Those that 
don't get the driver via distro will find it if they search (as the name 
asus-laptop is accepted, I will post a warning on acpi4asus.sf.net, to say 
that the driver have a new name, etc ...).
Thanks =) 

> thanks
> -Len

-- 
CHARY 'Iksaif' Corentin
http://xf.iksaif.net