30.06.2015 15:41, Jacek Anaszewski пишет: > On 06/30/2015 01:41 PM, Stas Sergeev wrote: >> 30.06.2015 11:27, Jacek Anaszewski пишет: >>> On 06/29/2015 05:17 PM, Stas Sergeev wrote:> 29.06.2015 17:05, Jacek Anaszewski пишет: >>>>> + * If need to disable soft blinking delegate this to the >>>>> + * work queue task to avoid problems in case we are >>>>> + * called from hard irq context. >>>>> + */ >>>>> + led_cdev->flags |= LED_BLINK_DISABLE; >>>> Wouldn't it be better to just enforce the callers >>>> to explicitly disable software blink, so that it to >>>> never happen from irq context? Something like in this >>>> patch: >>>> https://lkml.org/lkml/2015/5/13/491 >>>> >>> >>> Blinking can be disabled not only by removing trigger explicitly, >>> but also by setting brightness to 0 and led_set_brightness >>> can be called from hard irq context. set_brightness_work >>> was originally introduced exactly for this use case. >> Could you please describe where does this happen? > This in fact doesn't take place in the mainline kernel, > however there are some out of tree users apparently [1]. Oh, IMHO the hooks that are needed only for out-of-tree code require the appropriate comment, at least. I was entirely confused by its existence. IIRC in the past there was even the policy to not include any hooks for out-of-tree code at all. > Modifications I am proposing also need set_brightness_work, > so there is almost no cost of keeping the support for calling led_set_brightness from hard irq context intact. I wonder if there are possible races, eg you schedule disabling of the softblink timer, and someone else at the same time also disables it (and probably also re-enables). Well if you think that code is safe, I would agree that the cost is now very small with your patch, so maybe it is not a big deal any more. -- To unsubscribe from this list: send the line "unsubscribe linux-leds" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
