Problems porting asus_acpi to LED subsystem

Problems porting asus_acpi to LED subsystem

[Thomas Tuttle]

[-- Attachment #1: Type: text/plain, Size: 3170 bytes --]

Summary:

I'm trying to link asus_acpi into the LED subsystem.  My set_brightness
callback evaluates an ACPI method to set the LED state.  It works fine
manually changing the brightness, but when the callback is called by a
timer (using the timer or ide-disk triggers), it eventually causes an
Oops.  I can post my code or the Oops if it would help.  I'm new at
kernel coding, and I'm not on the list, so please CC me on any reply.
Sorry if this isn't appropriate on the list.

Long version:

I have an Asus laptop with two LED's that are supported by the asus_acpi
driver, and I'm trying to link the asus_acpi driver's LED features
(currently available only with a /proc interface) into the new LED
subsystem.

I tried to do this, by registering two LED devices with the LED
subsystem when the asus_acpi device is created.  This works, and the LED
devices show up in /sys/class/leds/.  I can change the brightness by
hand, by writing /sys/class/leds/asus:mail/brightness.  But if I try to
set the trigger to something that uses a timer, like ide-disk or timer,
it eventually causes an Oops.  I will try to capture the actual text of
the message, but the last one involved a stack of acpi_* functions,
followed by a couple of timer functions, the last being schedule_timer.

I haven't written any kernel code before, and I can't figure out what
I'm doing wrong.  The two functions that I created as the brightness_set
methods for the LED devices are essentially identical to the two used by
asus_acpi itself for the /proc interface.  All I can guess is that
either the ACPI methods are taking too long to run and the timer is
firing a new event while the old one is still running (but I don't
believe it is this, because timer_function in ledtrig-timer only
restarts the timer after it changes the LED value), or that the timer is
going off while the kernel is accessing something else with ACPI, and
things are getting corrupted.  Is there a way to schedule the ACPI call
that changes the LED state so it doesn't happen until it's "safe", or is
there some lock I should be acquiring before I change the LED state?

Alternatively, should the led timer trigger be more careful about when
it calls the LED set_brightness methods?  I don't know if there is a
convention for the context of that call--is it my responsibility to
ensure I'm not fiddling with ACPI stuff, or is it the timer's
responsibility to call my set_brightness method at a "good time"?

I'm sorry for posting to LKML with an issue like this, since I should
probably be able to figure it out myself, but I'm new at this and eager
to get this working.  I can post my code somewhere if anyone wants to
look at it.  (It's only about 20 lines added to asus_acpi.c.)

I'm not subscribed to the list, so I'd appreciate a CC on any reply.
I'll also keep an eye on the archives, just in case.

Thanks,

Thomas Tuttle

-- 
Thomas Tuttle (thinkinginbinary@gmail.com)
A List Apart: For people who make websites. alistapart.com
aim/y!m:thinkinginbinary; icq:198113263; jabber:thinkinginbinary@jabber.org
msn: thinkinginbinary@hotmail.com; pgp: 0xAF5112C6

[-- Attachment #2: Type: application/pgp-signature, Size: 191 bytes --]

Re: Problems porting asus_acpi to LED subsystem

[Pavel Machek]

Hi!

> Summary:
> 
> I'm trying to link asus_acpi into the LED subsystem.  My set_brightness
> callback evaluates an ACPI method to set the LED state.  It works fine
> manually changing the brightness, but when the callback is called by a
> timer (using the timer or ide-disk triggers), it eventually causes an
> Oops.  I can post my code or the Oops if it would help.  I'm new at
> kernel coding, and I'm not on the list, so please CC me on any reply.
> Sorry if this isn't appropriate on the list.

I _guess_ the LED subsystem can be called from interrupt, and ACPI
does not like that. So you probably need to use workqueue to defer
setting a bit.
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: Problems porting asus_acpi to LED subsystem

[Pavel Machek]

On Tue 04-07-06 10:50:40, Thomas Tuttle wrote:
> On July 04 at 04:50 EDT, Pavel Machek hastily scribbled:
> > I _guess_ the LED subsystem can be called from interrupt, and ACPI
> > does not like that. So you probably need to use workqueue to defer
> > setting a bit.
> 
> Ew, I would personally think that the LED subsystem should do this
> itself...

That would make LEDs factor 10000 or so slower on reasonable
platforms, sorry. But having flag 'this led needs process context'
might be god thing. Patch welcome.

> Anyway, can you give me a pointer on how to do this?  I've never used
> workqueues before.  (I've coded a grand total of about 40 lines of
> kernel code, and only about 20 work.)

grep is your friend, or some kernel programming intro.I do not have
any example near. Or maybe kernelnewbies.org.

-- 
Thanks for all the (sleeping) penguins.

Re: Problems porting asus_acpi to LED subsystem

[Pavel Machek]

On Tue 2006-07-04 10:50:22, Pavel Machek wrote:
> Hi!
> 
> > Summary:
> > 
> > I'm trying to link asus_acpi into the LED subsystem.  My set_brightness
> > callback evaluates an ACPI method to set the LED state.  It works fine
> > manually changing the brightness, but when the callback is called by a
> > timer (using the timer or ide-disk triggers), it eventually causes an
> > Oops.  I can post my code or the Oops if it would help.  I'm new at
> > kernel coding, and I'm not on the list, so please CC me on any reply.
> > Sorry if this isn't appropriate on the list.
> 
> I _guess_ the LED subsystem can be called from interrupt, and ACPI
> does not like that. So you probably need to use workqueue to defer
> setting a bit.

I guess 

	       schedule_delayed_work(&sharpsl_bat, msecs_to_jiffies(500));

is one of the ways how to move to process context from potentially
interrupt context. see sharpsl_pm.c for example usage.

								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html