On 06/02/2016 06:43 AM, Krzysztof Kozlowski wrote:
On 06/01/2016 01:37 PM, Jacek Anaszewski wrote:
Hi Krzysztof,
One thing drew my attention while reviewing this again:
max8997_led_brightness_set() can sleep, but the brightness_set
op it is assigned to must not sleep. At the time when this driver was
merged we were delegating brightness setting to workqueues task
in LED class drivers that can sleep during this call.
This must have been overlooked, which is even more likely, taking into
account that the initial patch doesn't have LED maintainer's ack.
The non-sleeping requirement is motivated by the fact that brightness
can be set from softirq context, e.g. when timer trigger is enabled.
Currently LED class drivers don't have to use workqueue on their own,
but are required to use brightness_set_blocking op instead of
brightness_set if they can sleep while setting brightness.
Apart of that, I think that operations in max8997_led_brightness_set()
should be protected with mutex to assure leaving the device in
a consistent state in case of concurrent calls.
I am aware that this is out of this patch scope, but I'd be grateful
if you could apply those changes and test them on hardware if you have
an access to.
The problem you mention existed before the patch. It was using sleeping
primitives (mutex) before adding regmap so I understand you don't have
anything against this patch, right?
Right, I just wanted to indicate the problem. The patch itself is ok.
I can fix the issue but it will be a little bit trickier because I don't
have the hardware. Other guys in the team tested the patchset for me so
I rely on them in that matter. Anyway I'll work on it.
Ack.
--
Best regards,
Jacek Anaszewski
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
