Re: [PATCH v2 0/3] leds: blink resolution improvements

[Stas Sergeev]

04.05.2015 10:55, Jacek Anaszewski ÐÐÑÐÑ:
> > So it seems the problem is already solved on the per-driver
> > basis. I don't have leds-aat1290 driver, it is probably not
> > in the kernel.
> It is currently on linux-next/master branch.
So that driver is not in line with others.

> > It is likely forgetting to use the work-queue
> > the way all other drivers do. So I think my patch is good for
> > the in-kernel drivers.
> >
> > There is also a led_cdev->set_brightness_work, and it looks
> > unused. I could use it for my patch, but for what, if the
> > drivers already use the work queue when needed?
> It is used in led_set_brightness function.
Only under that condition:
---
if (led_cdev->blink_delay_on || led_cdev->blink_delay_off) {
        led_cdev->delayed_set_value = brightness;
        schedule_work(&led_cdev->set_brightness_work);
---

But the main condition is:
---
if (led_cdev->flags & SET_BRIGHTNESS_ASYNC) {
        led_set_brightness_async(led_cdev, brightness);
---

So I think it is actually unused.
I don't see why schedule_work() above can't be just replaced
with led_set_brightness_async(). Is there the reason not to do so?

> I think that using work queues would compromise the whole idea of
> introducing intervals less than 1ms. After the task is delegated to
> work queue we are losing the control over the moment when it will get
> executed.
No one is going to allow sub-ms intervals when work-queue
is used. The proper solution would be to use work-queue for
drivers that can sleep, and allow sub-ms resolution for others.
Fortunately the drivers seem to already have that information
internally, and use work-queue on their own when needed.
leds-aat1290 may be an exception from that.

> I am becoming reluctant towards the whole idea, as we will be
> unable to guarantee the stability of a delay interval.
So why are you against the idea of improving the precision,
rather than against the code that prevents us from doing so?
The per-driver work queue use can be moved to led-core,
and the precise intervals can be guaranteed for the drivers
that do not need work queue.
Now your leds-aat1290 already asks for such a change,
because it can sleep but does not use a work-queue the
way other drivers do.
If I do such a massive change, I will certainly not be able
to properly test it, while you have a good test-case and
even a driver that needs such a change anyway. So I don't
see the point of being against that.

So what should we do?
I can try the aforementioned massive clean-up with removing
the work-queue from every driver and using the one in
led-core, but my attempts have few chances to succeed
because of no test-cases. Or can you do this instead, so
that leds-aat1290 driver is in line with the others? Or any
other options I can try?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/