Re: [PATCH v8 3/3] leds/powernv: Add driver for PowerNV platform

[Jacek Anaszewski <jacek.anaszewski@gmail.com>]

Vasant,

On 28.07.2015 15:40, Vasant Hegde wrote:
> On 07/27/2015 03:20 PM, Jacek Anaszewski wrote:
>> Hi Vasant,
>>
>> On 27.07.2015 05:41, Vasant Hegde wrote:
>>> On 07/27/2015 03:11 AM, Jacek Anaszewski wrote:
>>>> Hi Vasant,
>>>>
>>>
>>> Hi Jacek,
>>>
>>>> Two trivial details left. Please find them below.
>>>
>>> Thanks for the review/Ack. I'll fix below issues and resend patchset.
>>>
>>> I will ask Benh/Michael to take this patchset. But this patchset is depending
>>> on your core changes. Can you confirm that you are pushing that patchset in next
>>> merge window?
>
> Jacek,
>
>>
>> Without my core changes your driver won't work with led triggers, but
>> AFAIR this use case is not relevant for your LEDs? Eventually, we could
>> produce a patch set adding support for LED triggers if it will be clear
>> that LED core changes will not be merged in the upcoming merge window.
>
> IIUC current LED code doesn't allow me to sleep (without driver specific
> workqueue). And powernv_led_set() call will sleep. Hence I think it won't work.
>
> I did a quick test without your patch. It doesn't seems to be working.

Strange, it should work when led_set_brightness is called from non
atomic context, e.g. from sysfs interface. Could you share your test
scenario and describe the symptoms?

> Alternatively we can revert the changes (add driver specific workqueue now) and
> later when your changes goes to upstream, I can fix my code.

You can add this as an incremental patch which would be easy
to revert when LED core changes are merged.

-- 
Best Regards,
Jacek Anaszewski