Messages in this thread | | | Date | Tue, 30 Aug 2011 17:56:44 -0400 | From | Vivien Didelot <> | Subject | Re: [v2 3/4] platform: (TS-5500) add LED support |
| |
On Tue, 30 Aug 2011 14:17:15 -0700, "H. Peter Anvin" <hpa@zytor.com> wrote:
> On 08/30/2011 02:15 PM, Mark Brown wrote: > > On Tue, Aug 30, 2011 at 05:14:24PM -0400, Vivien Didelot wrote: > >> "H. Peter Anvin" <hpa@zytor.com> wrote: > >>> On 08/29/2011 03:16 PM, Mark Brown wrote: > > > >>>> Can you not do outb() from atomic context? The reason lots of > >>>> LED drivers update the hardware in a workqueue is that they > >>>> communicate with the hardware over buses that can't be used in > >>>> atomic context like I2C or SPI but if that's not an issue then > >>>> the workqueue is not required and the code can be simplified. > > > >>> outb() can definitely be executed from atomic context. > > > >> Good to know, thanks. I removed the work_struct and instead lock a > >> mutex before setting led->new_brightness and calling outb(). > > > > You can't take a mutex in atomic context... > > OK, so what is the potential race that this mutex is called for? If > it just means that the brightness can be redundantly set to the same > value more than once, no atomicity is needed. > > -hpa > I wrote the led_set function like:
static void ts5500_led_set(struct led_classdev *led_cdev, enum led_brightness value) { struct ts5500_led *led = container_of(led_cdev, struct ts5500_led, cdev); mutex_lock(&led->lock); led->new_brightness = (value == LED_OFF) ? LED_OFF : LED_FULL; outb(value, led->ioaddr); mutex_unlock(&led->lock); }
I guess the wrong value could be read if we get preempted just before the outb() call, am I wrong?
Vivien
| |