Date: Sun, 22 Apr 2007 16:06:14 +0200 (CEST) From: Henrik Brix Andersen <henrik@brixandersen.dk> To: FreeBSD-gnats-submit@FreeBSD.org Cc: Poul-Henning Kamp <phk@FreeBSD.org> Subject: kern/112008: [led(4)] [patch] led_create() unconditionally turns off the annunciator Message-ID: <20070422140614.496C71141F@lothlorien.brixandersen.dk> Resent-Message-ID: <200704221410.l3MEA59L092419@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 112008
>Category: kern
>Synopsis: [led(4)] [patch] led_create() unconditionally turns off the annunciator
>Confidential: no
>Severity: non-critical
>Priority: low
>Responsible: freebsd-bugs
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: change-request
>Submitter-Id: current-users
>Arrival-Date: Sun Apr 22 14:10:05 GMT 2007
>Closed-Date:
>Last-Modified:
>Originator: Henrik Brix Andersen
>Release: FreeBSD 7.0-CURRENT i386
>Organization:
pil.dk
>Environment:
System: FreeBSD lothlorien.brixandersen.dk 7.0-CURRENT FreeBSD 7.0-CURRENT #15: Sun Apr 22 02:17:56 CEST 2007 root@lothlorien.brixandersen.dk:/usr/obj/usr/src/sys/LOTHLORIEN i386
>Description:
The current implementation of led_create() in led(4) uncondionally
turns off the annunciator - there is currently no way for the caller
to specify the initial state of the annunciator.
This causes acpi_ibm(4) to always turn off the thinklight upon driver
attach, despite the actual state of the thinklight.
>How-To-Repeat:
You need an IBM/Lenovo ThinkPad with a ThinkLight. Unload the
acpi_ibm(4) kernel module if already loaded and turn on the ThinkLight
using the Fn-key combination. Load acpi_ibm(4) and notice the
ThinkLight is turned off.
There may be other consumers of led(4) that have similar problems, but
to not have the hardware needed to look into this.
>Fix:
My proposed fix, which breaks the API of led_create(), is adding a
third parameter for specifying the initial state of the annunciator.
Below are two patches. One which changes the API of led_create() and
updates all in-kernel consumers to this new API (no functional
changes). The second patch updates acpi_ibm(4) to use the new API for
passing the actual state of the ThinkLight to led_create().
If this goes in, __FreeBSD_version in sys/param.h should be bumped to
indicate the changed API.
--- led-api.diff begins here ---
--- sys/dev/led/led.h.orig Wed Jun 16 11:46:49 2004
+++ sys/dev/led/led.h Sun Apr 22 02:03:12 2007
@@ -14,7 +14,7 @@
typedef void led_t(void *, int);
-struct cdev *led_create(led_t *, void *, char const *);
+struct cdev *led_create(led_t *, void *, char const *, int);
void led_destroy(struct cdev *);
#endif
--- sys/dev/led/led.c.orig Mon Mar 7 12:05:46 2005
+++ sys/dev/led/led.c Sun Apr 22 02:04:22 2007
@@ -240,7 +240,7 @@
};
struct cdev *
-led_create(led_t *func, void *priv, char const *name)
+led_create(led_t *func, void *priv, char const *name, int state)
{
struct ledsc *sc;
@@ -259,7 +259,7 @@
if (LIST_EMPTY(&led_list))
callout_reset(&led_ch, hz / 10, led_timeout, NULL);
LIST_INSERT_HEAD(&led_list, sc, list);
- sc->func(sc->private, 0);
+ sc->func(sc->private, (state != 0));
mtx_unlock(&led_mtx);
return (sc->dev);
--- share/man/man4/led.4.orig Sun Apr 22 02:45:55 2007
+++ share/man/man4/led.4 Sun Apr 22 13:13:19 2007
@@ -24,7 +24,7 @@
.\"
.\" $FreeBSD: src/share/man/man4/led.4,v 1.12 2005/07/21 08:55:46 phk Exp $
.\"
-.Dd May 29, 2005
+.Dd Apr 22, 2007
.Dt LED 4
.Os
.Sh NAME
@@ -35,7 +35,7 @@
.Pp
.Fd "typedef void led_t(void *priv, int onoff);"
.Ft struct cdev *
-.Fn led_create "led_t *func" "void *priv" "char const *name"
+.Fn led_create "led_t *func" "void *priv" "char const *name" "int state"
.Ft void
.Fn led_destroy "struct cdev *"
.Sh DESCRIPTION
@@ -53,6 +53,9 @@
.Fa priv
argument is passed back to this on/off function and can be used however
the hardware driver sees fit.
+The
+.Fa state
+argument specifies the initial on/off state of the annunciator.
.Pp
The lamp can be controlled by opening and writing
.Tn ASCII
--- ./sys/arm/xscale/ixp425/avila_led.c.orig Wed Nov 22 13:57:17 2006
+++ ./sys/arm/xscale/ixp425/avila_led.c Sun Apr 22 01:57:09 2007
@@ -89,7 +89,7 @@ led_avila_attach(device_t dev)
reg &= ~GPIO_LED_STATUS_BIT;
GPIO_CONF_WRITE_4(sc, IXP425_GPIO_GPOER, reg);
- gpioled = led_create(led_func, led, "gpioled");
+ gpioled = led_create(led_func, led, "gpioled", 0);
/* Turn on LED */
led_func(led, 1);
--- ./sys/dev/acpi_support/acpi_asus.c.orig Thu Mar 22 19:16:39 2007
+++ ./sys/dev/acpi_support/acpi_asus.c Sun Apr 22 01:57:09 2007
@@ -599,7 +599,7 @@ acpi_asus_attach(device_t dev)
sc->s_bled.sc = sc;
sc->s_bled.type = ACPI_ASUS_LED_BLED;
sc->s_bled.cdev =
- led_create((led_t *)acpi_asus_led, &sc->s_bled, "bled");
+ led_create((led_t *)acpi_asus_led, &sc->s_bled, "bled", 0);
}
if (sc->model->mled_set) {
@@ -607,7 +607,7 @@ acpi_asus_attach(device_t dev)
sc->s_mled.sc = sc;
sc->s_mled.type = ACPI_ASUS_LED_MLED;
sc->s_mled.cdev =
- led_create((led_t *)acpi_asus_led, &sc->s_mled, "mled");
+ led_create((led_t *)acpi_asus_led, &sc->s_mled, "mled", 0);
}
if (sc->model->tled_set) {
@@ -615,7 +615,7 @@ acpi_asus_attach(device_t dev)
sc->s_tled.sc = sc;
sc->s_tled.type = ACPI_ASUS_LED_TLED;
sc->s_tled.cdev =
- led_create((led_t *)acpi_asus_led, &sc->s_tled, "tled");
+ led_create((led_t *)acpi_asus_led, &sc->s_tled, "tled", 0);
}
if (sc->model->wled_set) {
@@ -623,7 +623,7 @@ acpi_asus_attach(device_t dev)
sc->s_wled.sc = sc;
sc->s_wled.type = ACPI_ASUS_LED_WLED;
sc->s_wled.cdev =
- led_create((led_t *)acpi_asus_led, &sc->s_wled, "wled");
+ led_create((led_t *)acpi_asus_led, &sc->s_wled, "wled", 0);
}
/* Activate hotkeys */
--- ./sys/dev/acpi_support/acpi_ibm.c.orig Sun Apr 22 01:39:51 2007
+++ ./sys/dev/acpi_support/acpi_ibm.c Sun Apr 22 01:57:09 2007
@@ -420,7 +420,7 @@ acpi_ibm_attach(device_t dev)
/* Hook up light to led(4) */
if (sc->light_set_supported)
- sc->led_dev = led_create(ibm_led, sc, "thinklight");
+ sc->led_dev = led_create(ibm_led, sc, "thinklight", 0);
return (0);
}
--- ./sys/dev/auxio/auxio.c.orig Thu Jan 26 20:04:18 2006
+++ ./sys/dev/auxio/auxio.c Sun Apr 22 01:57:09 2007
@@ -263,7 +263,7 @@ auxio_attach_common(struct auxio_softc *
}
sc->sc_led_stat = auxio_led_read(sc) & AUXIO_LED_LED;
- sc->sc_led_dev = led_create(auxio_led_func, sc, "auxioled");
+ sc->sc_led_dev = led_create(auxio_led_func, sc, "auxioled", 0);
/* turn on the LED */
auxio_led_func(sc, 1);
--- ./sys/i386/i386/elan-mmcr.c.orig Tue Mar 27 23:03:37 2007
+++ ./sys/i386/i386/elan-mmcr.c Sun Apr 22 01:57:09 2007
@@ -205,7 +205,7 @@ sysctl_machdep_elan_gpio_config(SYSCTL_H
mmcrptr[(0xc2a + v) / 2] |= u;
gpio_config[i] = buf[i];
led_dev[i] =
- led_create(gpio_led, &led_cookie[i], tmp);
+ led_create(gpio_led, &led_cookie[i], tmp, 0);
break;
case '.':
gpio_config[i] = buf[i];
@@ -489,7 +489,7 @@ elan_drvinit(void)
/* Create the error LED on GPIO9 */
led_cookie[9] = 0x02000c34;
- led_dev[9] = led_create(gpio_led, &led_cookie[9], "error");
+ led_dev[9] = led_create(gpio_led, &led_cookie[9], "error", 0);
/* Disable the unavailable GPIO pins */
strcpy(gpio_config, "-----....--..--------..---------");
--- ./sys/i386/i386/geode.c.orig Tue Mar 27 23:03:37 2007
+++ ./sys/i386/i386/geode.c Sun Apr 22 01:57:09 2007
@@ -206,15 +206,15 @@ geode_probe(device_t self)
if ( bios_oem_strings(&bios_soekris,
bios_oem, BIOS_OEM_MAXLEN) > 0 ) {
led1b = 20;
- led1 = led_create(led_func, &led1b, "error");
+ led1 = led_create(led_func, &led1b, "error", 0);
} else if ( bios_oem_strings(&bios_pcengines,
bios_oem, BIOS_OEM_MAXLEN) > 0 ) {
led1b = -2;
led2b = -3;
led3b = -18;
- led1 = led_create(led_func, &led1b, "led1");
- led2 = led_create(led_func, &led2b, "led2");
- led3 = led_create(led_func, &led3b, "led3");
+ led1 = led_create(led_func, &led1b, "led1", 0);
+ led2 = led_create(led_func, &led2b, "led2", 0);
+ led3 = led_create(led_func, &led3b, "led3", 0);
/*
* Turn on first LED so we don't make
* people think their box just died.
--- ./sys/sparc64/fhc/clkbrd.c.orig Tue Mar 28 21:46:48 2006
+++ ./sys/sparc64/fhc/clkbrd.c Sun Apr 22 01:57:09 2007
@@ -153,7 +153,7 @@ clkbrd_attach(device_t dev)
sc->sc_clk_ctrl = bus_space_read_1(sc->sc_bt[CLKBRD_CLK],
sc->sc_bh[CLKBRD_CLK], CLK_CTRL);
- sc->sc_led_dev = led_create(clkbrd_led_func, sc, "clockboard");
+ sc->sc_led_dev = led_create(clkbrd_led_func, sc, "clockboard", 0);
return (0);
--- ./sys/sparc64/fhc/fhc.c.orig Wed Mar 7 22:13:50 2007
+++ ./sys/sparc64/fhc/fhc.c Sun Apr 22 01:57:09 2007
@@ -234,7 +234,7 @@ fhc_attach(device_t dev)
if ((sc->sc_flags & FHC_CENTRAL) == 0) {
snprintf(ledname, sizeof(ledname), "board%d", sc->sc_board);
- sc->sc_led_dev = led_create(fhc_led_func, sc, ledname);
+ sc->sc_led_dev = led_create(fhc_led_func, sc, ledname, 0);
}
for (child = OF_child(node); child != 0; child = OF_peer(child)) {
--- led-api.diff ends here ---
--- acpi_ibm_thinklight.diff begins here ---
--- sys/dev/acpi_support/acpi_ibm.c.orig Sun Apr 22 01:57:09 2007
+++ sys/dev/acpi_support/acpi_ibm.c Sun Apr 22 02:37:08 2007
@@ -420,7 +420,7 @@ acpi_ibm_attach(device_t dev)
/* Hook up light to led(4) */
if (sc->light_set_supported)
- sc->led_dev = led_create(ibm_led, sc, "thinklight", 0);
+ sc->led_dev = led_create(ibm_led, sc, "thinklight", sc->light_val);
return (0);
}
@@ -843,7 +843,11 @@ acpi_ibm_sysctl_init(struct acpi_ibm_sof
sc->light_set_supported = (sc->light_handle &&
ACPI_FAILURE(AcpiGetHandle(sc->ec_handle, "LEDB", &ledb_handle)));
- if (sc->light_get_supported || sc->light_set_supported) {
+ if (sc->light_get_supported) {
+ acpi_GetInteger(sc->ec_handle, IBM_NAME_KEYLIGHT, &sc->light_val);
+ return (TRUE);
+ }
+ else if (sc->light_set_supported) {
sc->light_val = 0;
return (TRUE);
}
--- acpi_ibm_thinklight.diff ends here ---
>Release-Note:
>Audit-Trail:
>Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20070422140614.496C71141F>