Re: [alsa-devel] [PATCH v2 02/17] leds: port locomo leds driver to new locomo core

2015-05-06 18:05 GMT+03:00 Jacek Anaszewski j.anaszewski@samsung.com:

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@gmail.com

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.

...

    help
      This option enables support for the LEDs on Sharp Locomo.
      Zaurus models SL-5500 and SL-5600.

diff --git a/drivers/leds/leds-locomo.c b/drivers/leds/leds-locomo.c index 80ba048..7fde812 100644 --- a/drivers/leds/leds-locomo.c +++ b/drivers/leds/leds-locomo.c @@ -9,89 +9,92 @@ */

#include <linux/kernel.h> -#include <linux/init.h> -#include <linux/module.h> -#include <linux/device.h> #include <linux/leds.h>

-#include <mach/hardware.h> -#include <asm/hardware/locomo.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/regmap.h> +#include <linux/mfd/locomo.h>

Please keep alphabetical order.

Ack

...

+struct locomo_led {

•   struct led_classdev led;
  

•   struct regmap *regmap;
  

•   unsigned int reg;
  

+};

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?

[skipped]