lkml.org 
[lkml]   [2009]   [Jan]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: hp_accel: do not call ACPI from invalid context
    On Tue 2009-01-13 14:17:21, Andrew Morton wrote:
    > On Mon, 12 Jan 2009 11:31:55 +0100
    > Pavel Machek <pavel@suse.cz> wrote:
    >
    > >
    > > LED on HP notebooks is connected through ACPI. That unfortunately
    > > means that it needs to be delayed by using schedule_work() to avoid
    > > calling ACPI interpretter in invalid context. This patch fixes that.
    > >
    > > ...
    > >
    > > +/* Delayed LEDs infrastructure ------------------------------------ */
    > > +
    > > +/* Special LED class that can defer work */
    > > +struct delayed_led_classdev {
    > > + struct led_classdev led_classdev;
    > > + struct work_struct work;
    > > + enum led_brightness new_brightness;
    > > +
    > > + unsigned int led; /* For driver */
    > > + void (*set_brightness)(struct delayed_led_classdev *data, enum led_brightness value);
    > > +};
    > > +
    > > +static inline void delayed_set_status_worker(struct work_struct *work)
    > > +{
    > > + struct delayed_led_classdev *data =
    > > + container_of(work, struct delayed_led_classdev, work);
    > > +
    > > + data->set_brightness(data, data->new_brightness);
    > > +}
    > > +
    > > +static inline void delayed_sysfs_set(struct led_classdev *led_cdev,
    > > + enum led_brightness brightness)
    > > +{
    > > + struct delayed_led_classdev *data = container_of(led_cdev,
    > > + struct delayed_led_classdev, led_classdev);
    > > + data->new_brightness = brightness;
    > > + schedule_work(&data->work);
    > > +}
    >
    > Is this the only LED driver int he current and future history of the
    > world which uses ACPI?
    >
    > coz otherwise, this code might better live in LEDs core somewhere, so
    > those other drivers can use it?

    There are 2 other drivers (thinkpad_acpi and asus-something IIRC) that
    want the same infrastructure. So I put that code in
    include/linux/delayed-leds.h in my tree, but then I realized I'd need
    to coordinate too many maintainers and just inlined it for the
    merge. (We do not want to schedule from atomic, right?)

    I already have a patch for thinkpad_acpi, but it needs a bit more
    testing and perhaps some tweaks by maintainer. So yes, eventually
    putting that into shared place is a plan.

    > > if (ret) {
    > > - led_classdev_unregister(&hpled_led);
    > > + while (work_pending(&hpled_led.work))
    > > + schedule();
    >
    > We have flush_work() for this?

    I did not realize flush_work() exists/does what I need... sorry.
    Pavel
    --
    (english) http://www.livejournal.com/~pavelmachek
    (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


    \
     
     \ /
      Last update: 2009-01-13 23:43    [W:5.615 / U:0.224 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site