Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753403AbaLDJjY (ORCPT ); Thu, 4 Dec 2014 04:39:24 -0500 Received: from nblzone-211-213.nblnetworks.fi ([83.145.211.213]:47326 "EHLO hillosipuli.retiisi.org.uk" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753019AbaLDJjR (ORCPT ); Thu, 4 Dec 2014 04:39:17 -0500 Date: Thu, 4 Dec 2014 11:39:06 +0200 From: Sakari Ailus To: Jacek Anaszewski Cc: linux-leds@vger.kernel.org, linux-media@vger.kernel.org, linux-kernel@vger.kernel.org, kyungmin.park@samsung.com, b.zolnierkie@samsung.com, pavel@ucw.cz, cooloney@gmail.com, rpurdie@rpsys.net, s.nawrocki@samsung.com, robh+dt@kernel.org, pawel.moll@arm.com, mark.rutland@arm.com, ijc+devicetree@hellion.org.uk, galak@codeaurora.org, Andrzej Hajda , Lee Jones , Chanwoo Choi Subject: Re: [PATCH/RFC v9 05/19] leds: Add support for max77693 mfd flash cell Message-ID: <20141204093906.GO14746@valkosipuli.retiisi.org.uk> References: <1417622814-10845-1-git-send-email-j.anaszewski@samsung.com> <1417622814-10845-6-git-send-email-j.anaszewski@samsung.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1417622814-10845-6-git-send-email-j.anaszewski@samsung.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > Signed-off-by: Andrzej Hajda > Acked-by: Kyungmin Park > Cc: Bryan Wu > Cc: Richard Purdie > Cc: Lee Jones > Cc: Chanwoo Choi > --- > 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 > + * Andrzej Hajda > + * > + * 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 > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#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)) > +#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? > + 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? > +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. > + > + 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. > + /* 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. > + 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. > + 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. > +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. > + --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? > + > + 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? > +{ > + 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. > +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 "); > +MODULE_AUTHOR("Andrzej Hajda "); > +MODULE_DESCRIPTION("Maxim MAX77693 led flash driver"); > +MODULE_LICENSE("GPL"); -- Kind regards, Sakari Ailus e-mail: sakari.ailus@iki.fi XMPP: sailus@retiisi.org.uk -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/