Hi,
> Sorry to bring the bearer of bad news, but your commit 2a5a8fa8b23
> ("leds: trigger: use RCU to protect the led_cdevs list"),
> is causing the following RCU warning when used with blinking
> triggers on I2C LED controllers which support hw blinking.
Err, well, surely that is a pre-existing driver bug then?
> The specific problem is drivers/leds/led-triggers.c:
> led_trigger_blink_setup() which does:
>
> rcu_read_lock();
> list_for_each_entry_rcu(led_cdev, &trig->led_cdevs, trig_list) {
> if (oneshot)
> led_blink_set_oneshot(led_cdev, delay_on, delay_off,
> invert);
> else
> led_blink_set(led_cdev, delay_on, delay_off);
> }
> rcu_read_unlock();
>
> And that led_blink_set() call then hits this path:
>
> if (!test_bit(LED_BLINK_ONESHOT, &led_cdev->work_flags) &&
> led_cdev->blink_set &&
> !led_cdev->blink_set(led_cdev, delay_on, delay_off))
> return;
>
> Which calls directly into the LED controller driver
Sure, so far so good.
> which talks to the LED controller over I2C which may sleep.
Which seems to me was already wrong before my patch, since the code was:
read_lock_irqsave(&trig->leddev_list_lock, flags);
list_for_each_entry(led_cdev, &trig->led_cdevs, trig_list) {
if (oneshot)
led_blink_set_oneshot(led_cdev, delay_on, delay_off,
invert);
else
led_blink_set(led_cdev, delay_on, delay_off);
}
read_unlock_irqrestore(&trig->leddev_list_lock, flags);
Surely, the code wasn't allowed to sleep in an _irqsave() section? You'd
just see a different check complain, rather than about RCU, I guess.
So maybe you're bringing bad news, but I don't think it's for me ;-) I
don't see cht_wc_leds_brightness_get() or a driver/module called
leds_cht_wcove even in linux-next, so I guess you should look wherever
you got _that_ from.
johannes
