Hi Sakari, Thanks for the review. On 12/04/2014 10:39 AM, Sakari Ailus wrote:
Hi Jacek, On Wed, Dec 03, 2014 at 05:06:40PM +0100, Jacek Anaszewski wrote:This patch adds led-flash support to Maxim max77693 chipset. A device can be exposed to user space through LED subsystem sysfs interface. Device supports up to two leds which can work in flash and torch mode. The leds can be triggered externally or by software. Signed-off-by: Jacek Anaszewski <j.anaszewski@xxxxxxxxxxx> Signed-off-by: Andrzej Hajda <a.hajda@xxxxxxxxxxx> Acked-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx> Cc: Bryan Wu <cooloney@xxxxxxxxx> Cc: Richard Purdie <rpurdie@xxxxxxxxx> Cc: Lee Jones <lee.jones@xxxxxxxxxx> Cc: Chanwoo Choi <cw00.choi@xxxxxxxxxxx> --- drivers/leds/Kconfig | 10 + drivers/leds/Makefile | 1 + drivers/leds/leds-max77693.c | 1023 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 1034 insertions(+) create mode 100644 drivers/leds/leds-max77693.c diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index fa8021e..2e66d55 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -463,6 +463,16 @@ config LEDS_TCA6507 LED driver chips accessed via the I2C bus. Driver support brightness control and hardware-assisted blinking. +config LEDS_MAX77693 + tristate "LED support for MAX77693 Flash" + depends on LEDS_CLASS_FLASH + depends on MFD_MAX77693 + depends on OF + help + This option enables support for the flash part of the MAX77693 + multifunction device. It has build in control for two leds in flash + and torch mode. + config LEDS_MAX8997 tristate "LED support for MAX8997 PMIC" depends on LEDS_CLASS && MFD_MAX8997 diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile index cbba921..57ca62b 100644 --- a/drivers/leds/Makefile +++ b/drivers/leds/Makefile @@ -52,6 +52,7 @@ obj-$(CONFIG_LEDS_MC13783) += leds-mc13783.o obj-$(CONFIG_LEDS_NS2) += leds-ns2.o obj-$(CONFIG_LEDS_NETXBIG) += leds-netxbig.o obj-$(CONFIG_LEDS_ASIC3) += leds-asic3.o +obj-$(CONFIG_LEDS_MAX77693) += leds-max77693.o obj-$(CONFIG_LEDS_MAX8997) += leds-max8997.o obj-$(CONFIG_LEDS_LM355x) += leds-lm355x.o obj-$(CONFIG_LEDS_BLINKM) += leds-blinkm.o diff --git a/drivers/leds/leds-max77693.c b/drivers/leds/leds-max77693.c new file mode 100644 index 0000000..67a2f8f --- /dev/null +++ b/drivers/leds/leds-max77693.c @@ -0,0 +1,1023 @@ +/* + * LED Flash class driver for the flash cell of max77693 mfd. + * + * Copyright (C) 2014, Samsung Electronics Co., Ltd. + * + * Authors: Jacek Anaszewski <j.anaszewski@xxxxxxxxxxx> + * Andrzej Hajda <a.hajda@xxxxxxxxxxx> + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * version 2 as published by the Free Software Foundation. + */ + +#include <asm/div64.h> +#include <linux/led-class-flash.h> +#include <linux/mfd/max77693.h> +#include <linux/mfd/max77693-private.h> +#include <linux/module.h> +#include <linux/mutex.h> +#include <linux/platform_device.h> +#include <linux/regmap.h> +#include <linux/slab.h> +#include <linux/workqueue.h> + +#define MODE_OFF 0 +#define MODE_FLASH1 (1 << 0) +#define MODE_FLASH2 (1 << 1) +#define MODE_TORCH1 (1 << 2) +#define MODE_TORCH2 (1 << 3) +#define MODE_FLASH_EXTERNAL1 (1 << 4) +#define MODE_FLASH_EXTERNAL2 (1 << 5)You could do this based on an argument (led number). E.g. #define MODE_FLASH_EXTERNAL(a) (1 << (4 + a))
OK.
+#define MODE_FLASH (MODE_FLASH1 | MODE_FLASH2 | \ + MODE_FLASH_EXTERNAL1 | MODE_FLASH_EXTERNAL2) + +#define FLED1_IOUT (1 << 0) +#define FLED2_IOUT (1 << 1) + +enum { + FLED1, + FLED2 +}; + +enum { + FLASH, + TORCH +}; + +struct max77693_sub_led { + struct led_classdev_flash ldev; + struct work_struct work_brightness_set; + + unsigned int torch_brightness; + unsigned int flash_timeout; +}; + +struct max77693_led {As this does not refer to a device, how about struct max77693_device, for instance?
OK.
+ struct regmap *regmap; + struct platform_device *pdev; + struct max77693_led_platform_data *pdata; + struct mutex lock; + + struct max77693_sub_led sub_leds[2]; + + unsigned int current_flash_timeout; + unsigned int mode_flags; + u8 torch_iout_reg; + bool iout_joint; + int strobing_sub_led_id; +}; + +struct max77693_led_settings { + struct led_flash_setting torch_brightness; + struct led_flash_setting flash_brightness; + struct led_flash_setting flash_timeout; +}; + +static u8 max77693_led_iout_to_reg(u32 ua) +{ + if (ua < FLASH_IOUT_MIN) + ua = FLASH_IOUT_MIN; + return (ua - FLASH_IOUT_MIN) / FLASH_IOUT_STEP; +} + +static u8 max77693_flash_timeout_to_reg(u32 us) +{ + return (us - FLASH_TIMEOUT_MIN) / FLASH_TIMEOUT_STEP; +} + +static inline struct max77693_led *ldev1_to_led( + struct led_classdev_flash *ldev) +{ + struct max77693_sub_led *sub_led = container_of(ldev, + struct max77693_sub_led, + ldev); + return container_of(sub_led, struct max77693_led, sub_leds[0]);You could have a common macro to find the flash controller struct if you add the LED number to struct max77693_sub_led.+} + +static inline struct max77693_led *ldev2_to_led( + struct led_classdev_flash *ldev) +{ + struct max77693_sub_led *sub_led = container_of(ldev, + struct max77693_sub_led, + ldev); + return container_of(sub_led, struct max77693_led, sub_leds[1]); +} + +static u8 max77693_led_vsys_to_reg(u32 mv) +{ + return ((mv - MAX_FLASH1_VSYS_MIN) / MAX_FLASH1_VSYS_STEP) << 2; +} + +static u8 max77693_led_vout_to_reg(u32 mv) +{ + return (mv - FLASH_VOUT_MIN) / FLASH_VOUT_STEP + FLASH_VOUT_RMIN; +} + +/* split composite current @i into two @iout according to @imax weights */What do you intend to do in the oint iout mode? A single LED connected to iout pins which are soldered together?
Exactly that what is written in the comment - split the current into both outputs.
+static void max77693_calc_iout(u32 iout[2], u32 i, u32 imax[2]) +{ + u64 t = i; + + t *= imax[1]; + do_div(t, imax[0] + imax[1]); + + iout[1] = (u32)t / FLASH_IOUT_STEP * FLASH_IOUT_STEP; + iout[0] = i - iout[1]; +} + +static int max77693_set_mode(struct max77693_led *led, unsigned int mode) +{ + struct max77693_led_platform_data *p = led->pdata; + struct regmap *rmap = led->regmap; + int ret, v = 0; + + if (mode & MODE_TORCH1) { + if (p->trigger[FLED1] & MAX77693_LED_TRIG_SOFT) + v |= FLASH_EN_ON << TORCH_EN_SHIFT(1); + } + + if (mode & MODE_TORCH2) { + if (p->trigger[FLED2] & MAX77693_LED_TRIG_SOFT) + v |= FLASH_EN_ON << TORCH_EN_SHIFT(2); + }The above could be turned to a loop from 0 to 1, as you have two LEDs.Just the MODE_* macro misses the argument.
Good point, thanks.
+ + if (mode & MODE_FLASH1) { + if (p->trigger[FLED1] & MAX77693_LED_TRIG_SOFT) + v |= FLASH_EN_ON << FLASH_EN_SHIFT(1); + } else if (mode & MODE_FLASH_EXTERNAL1) { + if (p->trigger[FLED1] & MAX77693_LED_TRIG_EXT) + v |= FLASH_EN_FLASH << FLASH_EN_SHIFT(2); + /* + * Enable hw triggering also for torch mode, as some camera + * sensors use torch led to fathom ambient light conditions + * before strobing the flash. + */ + if (p->trigger[FLED1] & MAX77693_LED_TRIG_EXT) + v |= FLASH_EN_TORCH << TORCH_EN_SHIFT(1); + } + + if (mode & MODE_FLASH2) { + if (p->trigger[FLED2] & MAX77693_LED_TRIG_SOFT) + v |= FLASH_EN_ON << FLASH_EN_SHIFT(2); + } else if (mode & MODE_FLASH_EXTERNAL2) { + if (p->trigger[FLED2] & MAX77693_LED_TRIG_EXT) + v |= FLASH_EN_FLASH << FLASH_EN_SHIFT(2); + /* + * Enable hw triggering also for torch mode, as some camera + * sensors use torch led to fathom ambient light conditions + * before strobing the flash. + */ + if (p->trigger[FLED2] & MAX77693_LED_TRIG_EXT) + v |= FLASH_EN_TORCH << TORCH_EN_SHIFT(2); + }Same here.
Agreed.
+ /* Reset the register only prior setting flash modes */ + if (mode & ~(MODE_TORCH1 | MODE_TORCH2)) { + ret = regmap_write(rmap, MAX77693_LED_REG_FLASH_EN, 0); + if (ret < 0) + return ret; + } + + return regmap_write(rmap, MAX77693_LED_REG_FLASH_EN, v); +} + +static void max77693_set_sync_strobe(struct max77693_led *led, + unsigned int *mode) +{ + struct max77693_sub_led *sub_leds = led->sub_leds; + struct led_classdev_flash *flash; + unsigned int m = *mode; + + /* + * If there are two leds then check if the other one + * wants to be strobed simultaneously. + */ + if (!led->iout_joint) {if (led->iout_joint) return; And you can more the rest left by one tab.
OK.
+ if (m & (MODE_FLASH1 | MODE_FLASH_EXTERNAL1)) {I think this matters only software strobe. The hardware strobe works this way automatically --- or I'd guess so at least.
No, the device allows to enable external strobe separately for each output.
+ flash = &sub_leds[FLED2].ldev; + if (flash->sync_strobe) + m |= m << 1; + } else if (m & (MODE_FLASH2 | MODE_FLASH_EXTERNAL2)) { + flash = &sub_leds[FLED1].ldev; + if (flash->sync_strobe) + m |= m >> 1; + } + } + + *mode = m; +} + +static int max77693_add_mode(struct max77693_led *led, unsigned int mode) +{ + int ret; + + /* Span the mode on FLED2 for joint iouts case */ + if (led->iout_joint) + mode |= (mode << 1); + + /* + * Torch mode once enabled remains active until turned off, + * and thus no action is required in this case. + */ + if ((mode & MODE_TORCH1) && + (led->mode_flags & MODE_TORCH1)) + return 0; + if ((mode & MODE_TORCH2) && + (led->mode_flags & MODE_TORCH2)) + return 0; + + /* Span a flash mode on the other led if it is to be synchronized */ + max77693_set_sync_strobe(led, &mode); + + /* + * FLASH_EXTERNAL mode activates FLASHEN and TORCHEN pins + * in the device. The related register settings interfere + * with SW triggerred modes, thus clear them to ensure proper + * device configuration. + */ + if (mode & MODE_FLASH_EXTERNAL1) + led->mode_flags &= (~MODE_TORCH1 & ~MODE_FLASH1); + if (mode & MODE_FLASH_EXTERNAL2) + led->mode_flags &= (~MODE_TORCH2 & ~MODE_FLASH2); + + led->mode_flags |= mode; + + ret = max77693_set_mode(led, led->mode_flags); + if (ret < 0) + return ret; + + /* + * Clear flash mode flag after setting the mode to avoid + * spurious flash strobing on every subsequent torch mode + * setting. + */ + if (mode & MODE_FLASH) + led->mode_flags &= ~mode; + + return ret; +} + +static int max77693_clear_mode(struct max77693_led *led, unsigned int mode) +{ + int ret; + + if (led->iout_joint) + /* Clear mode also on FLED2 for joint iouts case */ + mode |= (mode << 1); + else + /* + * Clear a flash mode on the other led + * if it is to be synchronized. + */ + max77693_set_sync_strobe(led, &mode); + + led->mode_flags &= ~mode; + + ret = max77693_set_mode(led, led->mode_flags); + + return ret; +} + +static int max77693_set_torch_current(struct max77693_led *led, + int led_id, u32 micro_amp) +{ + struct max77693_led_platform_data *p = led->pdata; + struct regmap *rmap = led->regmap; + u32 iout[2], iout_max[2]; + u8 iout1_reg = 0, iout2_reg = 0; + + iout_max[FLED1] = p->iout_torch[FLED1]; + iout_max[FLED2] = p->iout_torch[FLED2]; + + if (led_id == FLED1) { + /* + * Preclude splitting current to FLED2 if we + * are driving two separate leds. + */ + if (!led->iout_joint) + iout_max[FLED2] = 0; + max77693_calc_iout(iout, micro_amp, iout_max); + } else if (led_id == FLED2) { + iout_max[FLED1] = 0; + max77693_calc_iout(iout, micro_amp, iout_max); + } + + if (led_id == FLED1 || led->iout_joint) { + iout1_reg = max77693_led_iout_to_reg(iout[FLED1]); + led->torch_iout_reg &= 0xf0; + } + if (led_id == FLED2 || led->iout_joint) { + iout2_reg = max77693_led_iout_to_reg(iout[FLED2]); + led->torch_iout_reg &= 0x0f; + } + + led->torch_iout_reg |= ((iout1_reg << TORCH_IOUT1_SHIFT) | + (iout2_reg << TORCH_IOUT2_SHIFT)); + + return regmap_write(rmap, MAX77693_LED_REG_ITORCH, + led->torch_iout_reg); +} + +static int max77693_set_flash_current(struct max77693_led *led, + int led_id, + u32 micro_amp) +{ + struct max77693_led_platform_data *p = led->pdata; + struct regmap *rmap = led->regmap; + u32 iout[2], iout_max[2]; + u8 iout1_reg, iout2_reg; + int ret = -EINVAL; + + iout_max[FLED1] = p->iout_flash[FLED1]; + iout_max[FLED2] = p->iout_flash[FLED2]; + + if (led_id == FLED1) { + /* + * Preclude splitting current to FLED2 if we + * are driving two separate leds. + */ + if (!led->iout_joint) + iout_max[FLED2] = 0; + max77693_calc_iout(iout, micro_amp, iout_max); + } else if (led_id == FLED2) { + iout_max[FLED1] = 0; + max77693_calc_iout(iout, micro_amp, iout_max); + } + + if (led_id == FLED1 || led->iout_joint) { + iout1_reg = max77693_led_iout_to_reg(iout[FLED1]); + ret = regmap_write(rmap, MAX77693_LED_REG_IFLASH1, + iout1_reg); + if (ret < 0) + return ret; + } + if (led_id == FLED2 || led->iout_joint) { + iout2_reg = max77693_led_iout_to_reg(iout[FLED2]); + ret = regmap_write(rmap, MAX77693_LED_REG_IFLASH2, + iout2_reg); + } + + return ret; +} + +static int max77693_set_timeout(struct max77693_led *led, u32 timeout) +{ + struct max77693_led_platform_data *p = led->pdata; + struct regmap *rmap = led->regmap; + u8 v; + int ret; + + v = max77693_flash_timeout_to_reg(timeout); + + if (p->trigger_type[FLASH] == MAX77693_LED_TRIG_TYPE_LEVEL) + v |= FLASH_TMR_LEVEL; + + ret = regmap_write(rmap, MAX77693_LED_REG_FLASH_TIMER, v); + if (ret < 0) + return ret; + + led->current_flash_timeout = timeout; + + return 0; +} + +static int max77693_strobe_status_get(struct max77693_led *led, bool *state) +{ + struct regmap *rmap = led->regmap; + unsigned int v; + int ret; + + ret = regmap_read(rmap, MAX77693_LED_REG_FLASH_STATUS, &v); + if (ret < 0) + return ret; + + *state = v & FLASH_STATUS_FLASH_ON; + + return ret; +} + +static int max77693_int_flag_get(struct max77693_led *led, unsigned int *v) +{ + struct regmap *rmap = led->regmap; + + return regmap_read(rmap, MAX77693_LED_REG_FLASH_INT, v); +} + +static int max77693_setup(struct max77693_led *led) +{ + struct max77693_led_platform_data *p = led->pdata; + struct regmap *rmap = led->regmap; + int i, first_led, last_led, ret; + u32 max_flash_curr[2]; + u8 v; + + /* + * Initialize only flash current. Torch current doesn't + * require initialization as ITORCH register is written with + * new value each time brightness_set op is called. + */ + if (led->iout_joint) { + first_led = FLED1; + last_led = FLED1; + max_flash_curr[FLED1] = p->iout_flash[FLED1] + + p->iout_flash[FLED2]; + } else { + first_led = p->fleds[FLED1] ? FLED1 : FLED2; + last_led = p->num_leds == 2 ? FLED2 : first_led; + max_flash_curr[FLED1] = p->iout_flash[FLED1]; + max_flash_curr[FLED2] = p->iout_flash[FLED2]; + } + + for (i = first_led; i <= last_led; ++i) { + ret = max77693_set_flash_current(led, i, + max_flash_curr[i]); + if (ret < 0) + return ret; + } + + v = TORCH_TMR_NO_TIMER | MAX77693_LED_TRIG_TYPE_LEVEL; + ret = regmap_write(rmap, MAX77693_LED_REG_ITORCHTIMER, v); + if (ret < 0) + return ret; + + /* initially set FLED1 timeout */ + ret = max77693_set_timeout(led, p->flash_timeout[FLED1]); + if (ret < 0) + return ret; + + if (p->low_vsys > 0) + v = max77693_led_vsys_to_reg(p->low_vsys) | + MAX_FLASH1_MAX_FL_EN; + else + v = 0; + + ret = regmap_write(rmap, MAX77693_LED_REG_MAX_FLASH1, v); + if (ret < 0) + return ret; + ret = regmap_write(rmap, MAX77693_LED_REG_MAX_FLASH2, 0); + if (ret < 0) + return ret; + + if (p->boost_mode == MAX77693_LED_BOOST_FIXED) + v = FLASH_BOOST_FIXED; + else + v = p->boost_mode | p->boost_mode << 1; + if (p->fleds[FLED1] && p->fleds[FLED2]) + v |= FLASH_BOOST_LEDNUM_2; + ret = regmap_write(rmap, MAX77693_LED_REG_VOUT_CNTL, v); + if (ret < 0) + return ret; + + v = max77693_led_vout_to_reg(p->boost_vout); + ret = regmap_write(rmap, MAX77693_LED_REG_VOUT_FLASH1, v); + if (ret < 0) + return ret; + + return max77693_set_mode(led, MODE_OFF); +} + +static int max77693_led_brightness_set(struct max77693_led *led, + int led_id, enum led_brightness value) +{ + int ret; + + mutex_lock(&led->lock); + + if (value == 0) { + ret = max77693_clear_mode(led, MODE_TORCH1 << led_id); + if (ret < 0) + dev_dbg(&led->pdev->dev, + "Failed to clear torch mode (%d)\n", + ret); + goto unlock; + } + + ret = max77693_set_torch_current(led, led_id, value * TORCH_IOUT_STEP); + if (ret < 0) { + dev_dbg(&led->pdev->dev, + "Failed to set torch current (%d)\n", + ret); + goto unlock; + } + + ret = max77693_add_mode(led, MODE_TORCH1 << led_id); + if (ret < 0) + dev_dbg(&led->pdev->dev, + "Failed to set torch mode (%d)\n", + ret); +unlock: + mutex_unlock(&led->lock); + return ret; +} + +#define MAX77693_LED_BRIGHTNESS_SET_WORK(ID) \ +static void max77693_led##ID##_brightness_set_work( \ + struct work_struct *work) \ +{ \ + struct max77693_sub_led *sub_led = \ + container_of(work, struct max77693_sub_led, \ + work_brightness_set); \ + struct max77693_led *led = container_of(sub_led, \ + struct max77693_led, \ + sub_leds[FLED##ID]); \ + struct max77693_sub_led *sub_leds = led->sub_leds; \ + \ + max77693_led_brightness_set(led, FLED##ID, \ + sub_leds[FLED##ID].torch_brightness); \ +} + +/* LED subsystem callbacks */ + +#define MAX77693_LED_TORCH_BRIGHTNESS_SET(ID) \ +static int max77693_led##ID##_brightness_set_sync( \ + struct led_classdev *led_cdev, \ + enum led_brightness value) \ +{ \ + struct led_classdev_flash *flash = lcdev_to_flash(led_cdev); \ + struct max77693_led *led = ldev##ID##_to_led(flash); \ + \ + return max77693_led_brightness_set(led, FLED##ID, value); \ +} + +#define MAX77693_LED_BRIGHTNESS_SET(ID) \ +static void max77693_led##ID##_brightness_set( \ + struct led_classdev *led_cdev, \ + enum led_brightness value) \ +{ \ + struct led_classdev_flash *flash = lcdev_to_flash(led_cdev); \ + struct max77693_led *led = ldev##ID##_to_led(flash); \ + struct max77693_sub_led *sub_leds = led->sub_leds; \ + \ + sub_leds[FLED##ID].torch_brightness = value; \ + schedule_work(&sub_leds[FLED##ID].work_brightness_set); \ +} + +#define MAX77693_LED_FLASH_BRIGHTNESS_SET(ID) \ +static int max77693_led##ID##_flash_brightness_set( \ + struct led_classdev_flash *flash, \ + u32 brightness) \ +{ \ + struct max77693_led *led = ldev##ID##_to_led(flash); \ + int ret; \ + \ + mutex_lock(&led->lock); \ + ret = max77693_set_flash_current(led, FLED##ID, brightness); \ + mutex_unlock(&led->lock); \ + \ + return ret; \ +} + +#define MAX77693_LED_FLASH_STROBE_SET(ID) \ +static int max77693_led##ID##_flash_strobe_set( \ + struct led_classdev_flash *flash, \ + bool state) \ +{ \ + struct max77693_led *led = ldev##ID##_to_led(flash); \ + struct max77693_sub_led *sub_leds = led->sub_leds; \ + int ret; \ + \ + mutex_lock(&led->lock); \ + \ + if (!state) { \ + ret = max77693_clear_mode(led, MODE_FLASH##ID); \ + goto unlock; \ + } \ + \ + if (sub_leds[FLED##ID].flash_timeout != \ + led->current_flash_timeout) { \ + ret = max77693_set_timeout(led, \ + sub_leds[FLED##ID].flash_timeout); \ + if (ret < 0) \ + goto unlock; \ + } \ + \ + led->strobing_sub_led_id = ID; \ + \ + ret = max77693_add_mode(led, MODE_FLASH##ID); \ + \ +unlock: \ + mutex_unlock(&led->lock); \ + return ret; \ +} + +#define MAX77693_LED_FLASH_FAULT_GET(ID) \ +static int max77693_led##ID##_flash_fault_get( \ + struct led_classdev_flash *flash, \ + u32 *fault) \ +{ \ + struct max77693_led *led = ldev##ID##_to_led(flash); \ + unsigned int v; \ + int ret; \ + \ + ret = max77693_int_flag_get(led, &v); \ + if (ret < 0) \ + return ret; \ + \ + *fault = 0; \ + \ + if (v & FLASH_INT_FLED##ID##_OPEN) \ + *fault |= LED_FAULT_OVER_VOLTAGE; \ + if (v & FLASH_INT_FLED##ID##_SHORT) \ + *fault |= LED_FAULT_SHORT_CIRCUIT; \ + if (v & FLASH_INT_OVER_CURRENT) \ + *fault |= LED_FAULT_OVER_CURRENT; \ + \ + return 0; \ +} + +#define MAX77693_LED_FLASH_STROBE_GET(ID) \ +static int max77693_led##ID##_flash_strobe_get( \ + struct led_classdev_flash *flash, \ + bool *state) \ +{ \ + struct max77693_led *led = ldev##ID##_to_led(flash); \ + int ret; \ + \ + if (!state) \ + return -EINVAL; \ + \ + mutex_lock(&led->lock); \ + \ + ret = max77693_strobe_status_get(led, state); \ + \ + *state = !!(*state && led->strobing_sub_led_id == ID); \ + \ + mutex_unlock(&led->lock); \ + \ + return ret; \ +} + +#define MAX77693_LED_FLASH_TIMEOUT_SET(ID) \ +static int max77693_led##ID##_flash_timeout_set( \ + struct led_classdev_flash *flash, \ + u32 timeout) \ +{ \ + struct max77693_led *led = ldev##ID##_to_led(flash); \ + struct max77693_sub_led *sub_leds = led->sub_leds; \ + \ + mutex_lock(&led->lock); \ + sub_leds[FLED##ID].flash_timeout = timeout; \ + mutex_unlock(&led->lock); \ + \ + return 0; \ +} + +MAX77693_LED_BRIGHTNESS_SET(1) +MAX77693_LED_BRIGHTNESS_SET_WORK(1) +MAX77693_LED_TORCH_BRIGHTNESS_SET(1) +MAX77693_LED_FLASH_BRIGHTNESS_SET(1) +MAX77693_LED_FLASH_STROBE_SET(1) +MAX77693_LED_FLASH_STROBE_GET(1) +MAX77693_LED_FLASH_TIMEOUT_SET(1) +MAX77693_LED_FLASH_FAULT_GET(1) + +MAX77693_LED_BRIGHTNESS_SET(2) +MAX77693_LED_BRIGHTNESS_SET_WORK(2) +MAX77693_LED_TORCH_BRIGHTNESS_SET(2) +MAX77693_LED_FLASH_BRIGHTNESS_SET(2) +MAX77693_LED_FLASH_STROBE_SET(2) +MAX77693_LED_FLASH_STROBE_GET(2) +MAX77693_LED_FLASH_TIMEOUT_SET(2) +MAX77693_LED_FLASH_FAULT_GET(2)You could implement these without the help of macros so that the function gets the LED number from its arguments, directly or indirectly.
I'll do this provided there are no other LED class related obstacles I don't remember now.
+static int max77693_led_parse_dt(struct max77693_led *led, + struct device_node *node) +{ + struct max77693_led_platform_data *p = led->pdata; + struct device *dev = &led->pdev->dev; + struct device_node *child_node; + u32 fled_id; + int ret; + + of_property_read_u32_array(node, "maxim,fleds", p->fleds, 2); + of_property_read_u32_array(node, "maxim,trigger", p->trigger, 2); + of_property_read_u32_array(node, "maxim,trigger-type", p->trigger_type, + 2); + of_property_read_u32(node, "maxim,boost-mode", &p->boost_mode); + of_property_read_u32(node, "maxim,boost-vout", &p->boost_vout); + of_property_read_u32(node, "maxim,vsys-min", &p->low_vsys); + + for_each_available_child_of_node(node, child_node) { + ret = of_property_read_u32(child_node, "maxim,fled_id", + &fled_id); + if (ret < 0) { + dev_err(dev, "Error reading \"fled_id\" DT property\n"); + return ret; + } + + fled_id = clamp_val(fled_id, 1, 2);I think you should check fled_id is really correct, and not clamp it.
Right.
+ --fled_id; + + p->sub_nodes[fled_id] = child_node;p->sub_nodes is not accessed anywhere else. Do you plan to use it for something?
It is passed to v4l2_flash_init. In this patch set I add the V4L2 support in the separate patch. Probably this should be move there.
+ + ret = of_property_read_string(child_node, "label", + (const char **) &p->label[fled_id]); + if (ret < 0) { + dev_err(dev, "Error reading \"label\" DT property\n"); + return ret; + } + + of_property_read_u32(child_node, "max-microamp", + &p->iout_torch[fled_id]); + of_property_read_u32(child_node, "flash-max-microamp", + &p->iout_flash[fled_id]); + of_property_read_u32(child_node, "flash-timeout-microsec", + &p->flash_timeout[fled_id]); + if (++p->num_leds == 2) + break; + } + + return 0; +} + +static void clamp_align(u32 *v, u32 min, u32 max, u32 step) +{ + *v = clamp_val(*v, min, max); + if (step > 1) + *v = (*v - min) / step * step + min; +} + +static void max77693_led_validate_platform_data( + struct max77693_led_platform_data *p) +{ + u32 max; + int i; + + p->boost_mode = clamp_val(p->boost_mode, MAX77693_LED_BOOST_NONE, + MAX77693_LED_BOOST_FIXED); + + for (i = 0; i < ARRAY_SIZE(p->fleds); ++i) + p->fleds[i] = clamp_val(p->fleds[i], 0, 1); + + /* Ensure fleds configuration is sane */ + if (!p->fleds[FLED1] && !p->fleds[FLED2]) { + p->fleds[FLED1] = p->fleds[FLED2] = 1; + p->num_leds = 1; + } + + /* Ensure num_leds is consistent with fleds configuration */ + if ((!p->fleds[FLED1] || !p->fleds[FLED2]) && p->num_leds == 2) + p->num_leds = 1; + + /* + * boost must be enabled if current outputs + * are connected to separate leds. + */ + if ((p->num_leds == 2 || (p->fleds[FLED1] && p->fleds[FLED2])) && + p->boost_mode == MAX77693_LED_BOOST_NONE) + p->boost_mode = MAX77693_LED_BOOST_FIXED; + + max = p->boost_mode ? FLASH_IOUT_MAX_2LEDS : FLASH_IOUT_MAX_1LED; + + if (p->fleds[FLED1]) { + clamp_align(&p->iout_torch[FLED1], TORCH_IOUT_MIN, + TORCH_IOUT_MAX, TORCH_IOUT_STEP); + clamp_align(&p->iout_flash[FLED1], FLASH_IOUT_MIN, max, + FLASH_IOUT_STEP); + } else { + p->iout_torch[FLED1] = p->iout_flash[FLED1] = 0; + } + if (p->fleds[FLED2]) { + clamp_align(&p->iout_torch[FLED2], TORCH_IOUT_MIN, + TORCH_IOUT_MAX, TORCH_IOUT_STEP); + clamp_align(&p->iout_flash[FLED2], FLASH_IOUT_MIN, max, + FLASH_IOUT_STEP); + } else { + p->iout_torch[FLED2] = p->iout_flash[FLED2] = 0; + } + + for (i = 0; i < ARRAY_SIZE(p->trigger); ++i) + p->trigger[i] = clamp_val(p->trigger[i], 0, 7); + for (i = 0; i < ARRAY_SIZE(p->trigger_type); ++i) + p->trigger_type[i] = clamp_val(p->trigger_type[i], + MAX77693_LED_TRIG_TYPE_EDGE, + MAX77693_LED_TRIG_TYPE_LEVEL); + + for (i = 0; i < ARRAY_SIZE(p->flash_timeout); ++i) + clamp_align(&p->flash_timeout[i], FLASH_TIMEOUT_MIN, + FLASH_TIMEOUT_MAX, FLASH_TIMEOUT_STEP); + + clamp_align(&p->boost_vout, FLASH_VOUT_MIN, FLASH_VOUT_MAX, + FLASH_VOUT_STEP); + + if (p->low_vsys) + clamp_align(&p->low_vsys, MAX_FLASH1_VSYS_MIN, + MAX_FLASH1_VSYS_MAX, MAX_FLASH1_VSYS_STEP); +} + +static int max77693_led_get_platform_data(struct max77693_led *led)This isn't really platform data anymore. Could this be just "configuration", for instance?
OK, I was also pondering over it.
+{ + struct device *dev = &led->pdev->dev; + int ret; + + if (!dev->of_node) + return -EINVAL; + + led->pdata = devm_kzalloc(dev, sizeof(*led->pdata), GFP_KERNEL); + if (!led->pdata) + return -ENOMEM; + + ret = max77693_led_parse_dt(led, dev->of_node); + if (ret < 0) + return ret; + + max77693_led_validate_platform_data(led->pdata); + + return 0; +} + +#define MAX77693_LED_INIT_FLASH_OPS(ID) \ +static const struct led_flash_ops flash_ops##ID = { \ + \ + .flash_brightness_set = max77693_led##ID##_flash_brightness_set, \ + .strobe_set = max77693_led##ID##_flash_strobe_set, \ + .strobe_get = max77693_led##ID##_flash_strobe_get, \ + .timeout_set = max77693_led##ID##_flash_timeout_set, \ + .fault_get = max77693_led##ID##_flash_fault_get, \ +} + +MAX77693_LED_INIT_FLASH_OPS(1); +MAX77693_LED_INIT_FLASH_OPS(2);Same here, I think you can have just a single implementation of these.
I will check this out.
+static void max77693_init_flash_settings(struct max77693_led *led, + struct max77693_led_settings *s, + int led_id) +{ + struct max77693_led_platform_data *p = led->pdata; + struct led_flash_setting *setting; + + /* Init torch intensity setting */ + setting = &s->torch_brightness; + setting->min = led->iout_joint ? TORCH_IOUT_MIN * 2 : + TORCH_IOUT_MIN; + setting->max = led->iout_joint ? + p->iout_torch[FLED1] + p->iout_torch[FLED2] : + p->iout_torch[led_id]; + setting->step = TORCH_IOUT_STEP; + setting->val = setting->max; + + /* Init flash intensity setting */ + setting = &s->flash_brightness; + setting->min = led->iout_joint ? FLASH_IOUT_MIN * 2 : + FLASH_IOUT_MIN; + setting->max = led->iout_joint ? + p->iout_flash[FLED1] + p->iout_flash[FLED2] : + p->iout_flash[led_id]; + setting->step = FLASH_IOUT_STEP; + setting->val = setting->max; + + /* Init flash timeout setting */ + setting = &s->flash_timeout; + setting->min = FLASH_TIMEOUT_MIN; + setting->max = p->flash_timeout[led_id]; + setting->step = FLASH_TIMEOUT_STEP; + setting->val = setting->max; +} + +static int max77693_register_led(struct max77693_led *led, int id) +{ + struct platform_device *pdev = led->pdev; + struct led_classdev_flash *flash; + struct led_classdev *led_cdev; + struct max77693_sub_led *sub_leds = led->sub_leds; + struct max77693_led_settings settings; + + flash = &sub_leds[id].ldev; + + /* Initialize flash settings */ + max77693_init_flash_settings(led, &settings, id); + + /* Initialize LED Flash class device */ + led_cdev = &flash->led_cdev; + + led_cdev->name = led->pdata->label[id]; + + if (id == FLED1) { + led_cdev->brightness_set = max77693_led1_brightness_set; + led_cdev->brightness_set_sync = + max77693_led1_brightness_set_sync; + INIT_WORK(&sub_leds[id].work_brightness_set, + max77693_led1_brightness_set_work); + flash->ops = &flash_ops1; + } else { + led_cdev->brightness_set = max77693_led2_brightness_set; + led_cdev->brightness_set_sync = + max77693_led2_brightness_set_sync; + INIT_WORK(&sub_leds[id].work_brightness_set, + max77693_led2_brightness_set_work); + flash->ops = &flash_ops2; + } + + led_cdev->max_brightness = settings.torch_brightness.val / + TORCH_IOUT_STEP; + led_cdev->flags |= LED_DEV_CAP_FLASH; + if (led->pdata->num_leds == 2) + led_cdev->flags |= LED_DEV_CAP_COMPOUND; + + flash->brightness = settings.flash_brightness; + flash->timeout = settings.flash_timeout; + sub_leds[id].flash_timeout = flash->timeout.val; + + /* Register in the LED subsystem. */ + return led_classdev_flash_register(&pdev->dev, flash); +} + +static int max77693_led_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct max77693_dev *iodev = dev_get_drvdata(dev->parent); + struct max77693_led *led; + struct max77693_led_platform_data *p; + struct max77693_sub_led *sub_leds; + int ret; + + led = devm_kzalloc(dev, sizeof(*led), GFP_KERNEL); + if (!led) + return -ENOMEM; + + led->pdev = pdev; + led->regmap = iodev->regmap; + sub_leds = led->sub_leds; + platform_set_drvdata(pdev, led); + ret = max77693_led_get_platform_data(led); + if (ret < 0) + return -EINVAL; + + p = led->pdata; + mutex_init(&led->lock); + + if (p->num_leds == 1 && p->fleds[FLED1] && p->fleds[FLED2]) + led->iout_joint = true; + + ret = max77693_setup(led); + if (ret < 0) + goto err_setup; + + if (led->iout_joint || p->fleds[FLED1]) { + ret = max77693_register_led(led, FLED1); + if (ret < 0) + goto err_setup; + } + + if (!led->iout_joint && p->fleds[FLED2]) { + ret = max77693_register_led(led, FLED2); + if (ret < 0) + goto err_register_led2; + } + + return 0; + +err_register_led2: + if (!p->fleds[FLED1]) + goto err_setup; + led_classdev_flash_unregister(&sub_leds[FLED1].ldev); +err_setup: + mutex_destroy(&led->lock); + + return ret; +} + +static int max77693_led_remove(struct platform_device *pdev) +{ + struct max77693_led *led = platform_get_drvdata(pdev); + struct max77693_led_platform_data *p = led->pdata; + struct max77693_sub_led *sub_leds = led->sub_leds; + + if (led->iout_joint || p->fleds[FLED1]) { + led_classdev_flash_unregister(&sub_leds[FLED1].ldev); + cancel_work_sync(&sub_leds[FLED1].work_brightness_set); + } + + if (!led->iout_joint && p->fleds[FLED2]) { + led_classdev_flash_unregister(&sub_leds[FLED2].ldev); + cancel_work_sync(&sub_leds[FLED2].work_brightness_set); + } + + mutex_destroy(&led->lock); + + return 0; +} + +static struct of_device_id max77693_led_dt_match[] = { + {.compatible = "maxim,max77693-led"}, + {}, +}; + +static struct platform_driver max77693_led_driver = { + .probe = max77693_led_probe, + .remove = max77693_led_remove, + .driver = { + .name = "max77693-led", + .owner = THIS_MODULE, + .of_match_table = max77693_led_dt_match, + }, +}; + +module_platform_driver(max77693_led_driver); + +MODULE_AUTHOR("Jacek Anaszewski <j.anaszewski@xxxxxxxxxxx>"); +MODULE_AUTHOR("Andrzej Hajda <a.hajda@xxxxxxxxxxx>"); +MODULE_DESCRIPTION("Maxim MAX77693 led flash driver"); +MODULE_LICENSE("GPL");
Best Regards, Jacek Anaszewski -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
