From owner-freebsd-bugs@FreeBSD.ORG Sun Apr 22 14:10:06 2007 Return-Path: X-Original-To: freebsd-bugs@hub.freebsd.org Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 48A5416A401 for ; Sun, 22 Apr 2007 14:10:06 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [69.147.83.40]) by mx1.freebsd.org (Postfix) with ESMTP id 27A2A13C44B for ; Sun, 22 Apr 2007 14:10:06 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.13.4/8.13.4) with ESMTP id l3MEA5Jf092421 for ; Sun, 22 Apr 2007 14:10:06 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.13.4/8.13.4/Submit) id l3MEA59L092419; Sun, 22 Apr 2007 14:10:05 GMT (envelope-from gnats) Resent-Date: Sun, 22 Apr 2007 14:10:05 GMT Resent-Message-Id: <200704221410.l3MEA59L092419@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-bugs@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, Henrik Brix Andersen Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 5434916A402 for ; Sun, 22 Apr 2007 14:06:16 +0000 (UTC) (envelope-from brix@lothlorien.brixandersen.dk) Received: from solow.pil.dk (relay.pil.dk [195.41.47.164]) by mx1.freebsd.org (Postfix) with ESMTP id 9E18713C465 for ; Sun, 22 Apr 2007 14:06:15 +0000 (UTC) (envelope-from brix@lothlorien.brixandersen.dk) Received: from lothlorien.brixandersen.dk (osgiliath.brixandersen.dk [87.53.223.189]) by solow.pil.dk (Postfix) with ESMTP id 8DEE91CC09C; Sun, 22 Apr 2007 16:06:14 +0200 (CEST) Received: by lothlorien.brixandersen.dk (Postfix, from userid 1001) id 496C71141F; Sun, 22 Apr 2007 16:06:14 +0200 (CEST) Message-Id: <20070422140614.496C71141F@lothlorien.brixandersen.dk> Date: Sun, 22 Apr 2007 16:06:14 +0200 (CEST) From: Henrik Brix Andersen To: FreeBSD-gnats-submit@FreeBSD.org X-Send-Pr-Version: 3.113 Cc: Poul-Henning Kamp Subject: kern/112008: [led(4)] [patch] led_create() unconditionally turns off the annunciator X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Henrik Brix Andersen List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 22 Apr 2007 14:10:06 -0000 >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: