On 05/13/2015 04:14 PM, Dmitry Eremin-Solenikov wrote:
2015-05-13 13:31 GMT+03:00 Jacek Anaszewski <j.anaszewski@xxxxxxxxxxx>:
On 05/12/2015 05:35 PM, Dmitry Eremin-Solenikov wrote:
2015-05-06 18:05 GMT+03:00 Jacek Anaszewski <j.anaszewski@xxxxxxxxxxx>:
On 04/28/2015 01:55 AM, Dmitry Eremin-Solenikov wrote:
Adapt locomo leds driver to new locomo core setup.
Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@xxxxxxxxx>
---
drivers/leds/Kconfig | 1 -
drivers/leds/leds-locomo.c | 119
+++++++++++++++++++++++----------------------
2 files changed, 61 insertions(+), 59 deletions(-)
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 966b960..4b4650b 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -79,7 +79,6 @@ config LEDS_LM3642
config LEDS_LOCOMO
tristate "LED Support for Locomo device"
depends on LEDS_CLASS
- depends on SHARP_LOCOMO
Why do you remove this dependency?
Because SHARP_LOCOMO is a Kconfig symbol for the old driver. New driver
uses MFD_LOCOMO Kconfig entry. Also the driver now uses generic platform
device and regmap interfaces, so there is no direct dependency on main
LoCoMo driver. And the policy (IIRC) was not to have such dependencies.
Ack. Shouldn't you also need "select REGMAP_MMIO" ?
No. Maybe I should add "select REGMAP" instead.
REGMAP is enabled by default if REGMAP_MMIO is enabled. Having
REGMAP_MMIO selected would also allow to notice at first glance which
regmap backend is used. This would in turn provide a straightforward
explanation on why brightness_set op is not delegated to work queue.
static void locomoled_brightness_set(struct led_classdev *led_cdev,
- enum led_brightness value, int offset)
+ enum led_brightness value)
{
- struct locomo_dev *locomo_dev =
LOCOMO_DEV(led_cdev->dev->parent);
- unsigned long flags;
+ struct locomo_led *led = container_of(led_cdev, struct
locomo_led,
led);
- local_irq_save(flags);
- if (value)
- locomo_writel(LOCOMO_LPT_TOFH, locomo_dev->mapbase +
offset);
- else
- locomo_writel(LOCOMO_LPT_TOFL, locomo_dev->mapbase +
offset);
- local_irq_restore(flags);
+ regmap_write(led->regmap, led->reg,
+ value ? LOCOMO_LPT_TOFH : LOCOMO_LPT_TOFL);
}
Please use work queue for setting brightness. This is required for the
driver to be compatible with led triggers. You can refer to the
existing LED drivers on how to implement this.
Hmm. Why? The regmap here uses MMIO access, so it is atomic operation
and doesn't need to be wrapped into work queue, does it?
Triggers can call brightness_set op in the interrupt context, so it
cannot sleep. I see "map->lock(map->lock_arg)" in regmap_write, thus
I am inferring that it can sleep.
I am not sure if regmap implements its 'lock' op when using MMIO.
The best way to figure this out is turning "LED Timer Trigger" on
in the config and execute:
echo "timer" > trigger
It works without any "might sleep/sleeping in atomic context" warnings.
Technically I'd agree with you. If I'm using regmaps, ideally I should not
depend on the exact backend and put working with it to the work queue.
However as this is a driver for quite old IP block, it is not used with
regmap backends other than MMIO, I'd deduce, it's ok to do things
in a more relaxed way and to call regmap_write even from atomic
contexts.
Ack.
--
Best Regards,
Jacek Anaszewski
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
