kern/111810: [ath] [patch] Changes to dev.ath.0.ledpin not recognized immediately

[Henrik Brix Andersen]

>Number:         111810
>Category:       kern
>Synopsis:       [ath] [patch] Changes to dev.ath.0.ledpin not recognized immediately
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Wed Apr 18 08:30:01 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 #10: Wed Apr 18 01:03:47 CEST 2007 root at lothlorien.brixandersen.dk:/usr/obj/usr/src/sys/LOTHLORIEN i386


	
>Description:
Currently, changes to the dev.ath.0.ledpin sysctl are not recognized
immediately since changing the value of this sysctl doesn't trigger a
call to ath_hal_gpioCfgOutput() and ath_hal_gpioset().

This means that when changing the above mentioned sysctl, one has to
do something like the following to get ath(4) to start using the new
GPIO pin for driving the LED:

# sysctl dev.ath.0.ledpin
0
# sysctl dev.ath.0.softled
1

# sysctl dev.ath.0.ledpin=1
# sysctl dev.ath.0.softled=0
# sysctl dev.ath.0.softled=1

The above toggles dev.ath.0.softled after setting dev.ath.0.ledpin=1
and thus triggers a call to ath_hal_gpioCfgOutput() and
ath_hal_gpioset(), configuring the newly selected GPIO pin.
	
>How-To-Repeat:

You need an Atheros card with a LED connected to a GPIO pin other than
0, which is the "default".

Load the ath(4) driver and set the dev.ath.0.ledpin according to the
hardware:

# sysctl dev.ath.0.ledpin=1

Notice that the LED doesn't light up.

Toggle the dev.ath.0.softled sysctl:

# sysctl dev.ath.0.softled=0
# sysctl dev.ath.0.softled=1

Notice that the LED now lights up as expected.

	
>Fix:
The patch below implements a custom sysctl function for handling
changes to dev.ath.0.ledpin, eliminating the need to toggle
dev.ath.0.softled after making changes to dev.ath.0.ledpin.

	

--- if_ath.c.diff begins here ---
--- sys/dev/ath/if_ath.c.orig	Tue Apr 17 21:26:52 2007
+++ sys/dev/ath/if_ath.c	Tue Apr 17 22:26:05 2007
@@ -5349,6 +5349,27 @@ ath_sysctl_softled(SYSCTL_HANDLER_ARGS)
 }
 
 static int
+ath_sysctl_ledpin(SYSCTL_HANDLER_ARGS)
+{
+	struct ath_softc *sc = arg1;
+	int ledpin = sc->sc_ledpin;
+	int error;
+
+	error = sysctl_handle_int(oidp, &ledpin, 0, req);
+	if (error || !req->newptr)
+		return error;
+	if (ledpin != sc->sc_ledpin) {
+		sc->sc_ledpin = ledpin;
+		if (sc->sc_softled) {
+			ath_hal_gpioCfgOutput(sc->sc_ah, sc->sc_ledpin);
+			ath_hal_gpioset(sc->sc_ah, sc->sc_ledpin,
+				!sc->sc_ledon);
+		}
+	}
+	return 0;
+}
+
+static int
 ath_sysctl_txantenna(SYSCTL_HANDLER_ARGS)
 {
 	struct ath_softc *sc = arg1;
@@ -5582,9 +5603,9 @@ ath_sysctlattach(struct ath_softc *sc)
 	SYSCTL_ADD_PROC(ctx, SYSCTL_CHILDREN(tree), OID_AUTO,
 		"softled", CTLTYPE_INT | CTLFLAG_RW, sc, 0,
 		ath_sysctl_softled, "I", "enable/disable software LED support");
-	SYSCTL_ADD_INT(ctx, SYSCTL_CHILDREN(tree), OID_AUTO,
-		"ledpin", CTLFLAG_RW, &sc->sc_ledpin, 0,
-		"GPIO pin connected to LED");
+	SYSCTL_ADD_PROC(ctx, SYSCTL_CHILDREN(tree), OID_AUTO,
+		"ledpin", CTLTYPE_INT | CTLFLAG_RW, sc, 0,
+		ath_sysctl_ledpin, "I", "GPIO pin connected to LED");
 	SYSCTL_ADD_INT(ctx, SYSCTL_CHILDREN(tree), OID_AUTO,
 		"ledon", CTLFLAG_RW, &sc->sc_ledon, 0,
 		"setting to turn LED on");
--- if_ath.c.diff ends here ---


>Release-Note:
>Audit-Trail:
>Unformatted: