Re: [RFCv2,2/2] i2c: add new dual Flash driver,LM3646

2014-01-29 Thread Sakari Ailus
Hi Daniel,

Thanks for the update. The driver is very nicely written in general btw. One
comment below.

On Tue, Jan 28, 2014 at 03:55:58PM +0900, Daniel Jeong wrote:
...
 +/*
 + * struct lm3646_flash
 + *
 + * @pdata: platform data
 + * @regmap: reg. map for i2c
 + * @lock: muxtex for serial access.
 + * @led_mode: V4L2 LED mode
 + * @ctrls_led: V4L2 contols
 + * @subdev_led: V4L2 subdev
 + */
 +struct lm3646_flash {
 + struct device *dev;
 + struct lm3646_platform_data *pdata;
 + struct regmap *regmap;
 +
 + enum v4l2_flash_led_mode led_mode;
 + struct v4l2_ctrl_handler ctrls_led;
 + struct v4l2_subdev subdev_led;
 +};
 +
 +#define to_lm3646_flash(_ctrl)   \
 + container_of(_ctrl-handler, struct lm3646_flash, ctrls_led)
 +
 +/* enable mode control */
 +static int lm3646_mode_ctrl(struct lm3646_flash *flash)
 +{
 + int rval = -EINVAL;
 +
 + switch (flash-led_mode) {
 + case V4L2_FLASH_LED_MODE_NONE:
 + rval = regmap_update_bits(flash-regmap,
 +   REG_ENABLE, MASK_ENABLE, MODE_SHDN);
 + break;
 + case V4L2_FLASH_LED_MODE_TORCH:
 + rval = regmap_update_bits(flash-regmap,
 +   REG_ENABLE, MASK_ENABLE, MODE_TORCH);
 + break;
 + case V4L2_FLASH_LED_MODE_FLASH:
 + rval = regmap_update_bits(flash-regmap,
 +   REG_ENABLE, MASK_ENABLE, MODE_FLASH);
 + break;
 + }
 + return rval;
 +}
 +
 +/* V4L2 controls  */
 +static int lm3646_get_ctrl(struct v4l2_ctrl *ctrl)
 +{
 + struct lm3646_flash *flash = to_lm3646_flash(ctrl);
 + int rval = -EINVAL;
 +
 + if (ctrl-id == V4L2_CID_FLASH_FAULT) {
 + s32 fault = 0;
 + unsigned int reg_val;
 + rval = regmap_read(flash-regmap, REG_FLAG, reg_val);
 + if (rval  0)
 + return rval;
 +
 + if (reg_val  FAULT_TIMEOUT)
 + fault |= V4L2_FLASH_FAULT_TIMEOUT;
 + if (reg_val  FAULT_SHORT_CIRCUIT)
 + fault |= V4L2_FLASH_FAULT_SHORT_CIRCUIT;
 + if (reg_val  FAULT_UVLO)
 + fault |= V4L2_FLASH_FAULT_UVLO;
 + if (reg_val  FAULT_IVFM)
 + fault |= V4L2_FLASH_FAULT_IVFM;
 + if (reg_val  FAULT_OCP)
 + fault |= V4L2_FLASH_FAULT_OVER_CURRENT;
 + if (reg_val  FAULT_OVERTEMP)
 + fault |= V4L2_FLASH_FAULT_OVER_TEMPERATURE;
 + if (reg_val  FAULT_NTC_TRIP)
 + fault |= V4L2_FLASH_FAULT_NTC_TRIP;
 + if (reg_val  FAULT_OVP)
 + fault |= V4L2_FLASH_FAULT_OVER_VOLTAGE;
 +
 + ctrl-cur.val = fault;
 + }
 +
 + return rval;
 +}
 +
 +static int lm3646_set_ctrl(struct v4l2_ctrl *ctrl)
 +{
 + struct lm3646_flash *flash = to_lm3646_flash(ctrl);
 + u8 bval;
 + int rval = -EINVAL;
 +
 + switch (ctrl-id) {
 + case V4L2_CID_FLASH_LED_MODE:
 + flash-led_mode = ctrl-val;

Do you need to keep led_mode in struct lm3646_flash? Could you access the
value in struct v4l2_ctrl directly? (See smiapp-core.c for an example.)

 + if (flash-led_mode != V4L2_FLASH_LED_MODE_FLASH)
 + rval = lm3646_mode_ctrl(flash);
 + break;
 +
 + case V4L2_CID_FLASH_STROBE_SOURCE:
 + rval = regmap_update_bits(flash-regmap,
 +   REG_STROBE_SRC, MASK_STROBE_SRC,
 +   (ctrl-val)  7);
 + break;
 +
 + case V4L2_CID_FLASH_STROBE:
 + if (flash-led_mode != V4L2_FLASH_LED_MODE_FLASH)
 + return rval;
 + rval = lm3646_mode_ctrl(flash);
 + break;
 +
 + case V4L2_CID_FLASH_STROBE_STOP:
 + if (flash-led_mode != V4L2_FLASH_LED_MODE_FLASH)
 + return rval;
 + flash-led_mode = V4L2_FLASH_LED_MODE_NONE;
 + rval = lm3646_mode_ctrl(flash);
 + break;
 +
 + case V4L2_CID_FLASH_TIMEOUT:
 + bval = LM3646_FLASH_TOUT_ms_TO_REG(ctrl-val);
 + rval = regmap_update_bits(flash-regmap,
 +   REG_FLASH_TOUT, MASK_FLASH_TOUT,
 +   bval);
 + break;
 +
 + case V4L2_CID_FLASH_INTENSITY:
 + bval = LM3646_TOTAL_FLASH_BRT_uA_TO_REG(ctrl-val);
 + rval = regmap_update_bits(flash-regmap,
 +   REG_FLASH_BR, MASK_FLASH_BR, bval);
 + break;
 +
 + case V4L2_CID_FLASH_TORCH_INTENSITY:
 + bval = LM3646_TOTAL_TORCH_BRT_uA_TO_REG(ctrl-val);
 + rval = regmap_update_bits(flash-regmap,
 +   REG_TORCH_BR, MASK_TORCH_BR,
 +   bval  4);
 + 

[RFCv2,2/2] i2c: add new dual Flash driver,LM3646

2014-01-27 Thread Daniel Jeong
Add new dual flash driver. 
LM3646 is a dual Flash LED Driver, LED1 and LED2, following the datasheet.
But there is no registers to contorl LED2 brightness.
LED2 brightness can be controlled by limiting max brightness.
LED2 brightness  = Total brightness - LED1 brightness
LED2 will be off if LED2 brightness is set equal to or bigger than Total 
brightness.
And the brightness step is very small, 1.46mA for Torch, 11.71mA for Flash.
If the step is changed to mA, maximum brightness cannot be reachable.

Signed-off-by: Daniel Jeong gshark.je...@gmail.com
---
 drivers/media/i2c/Kconfig  |9 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/lm3646.c |  400 
 include/media/lm3646.h |   87 ++
 4 files changed, 497 insertions(+)
 create mode 100644 drivers/media/i2c/lm3646.c
 create mode 100644 include/media/lm3646.h

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 842654d..654df46 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -630,6 +630,15 @@ config VIDEO_LM3560
  This is a driver for the lm3560 dual flash controllers. It controls
  flash, torch LEDs.
 
+config VIDEO_LM3646
+   tristate LM3646 dual flash driver support
+   depends on I2C  VIDEO_V4L2  MEDIA_CONTROLLER
+   depends on MEDIA_CAMERA_SUPPORT
+   select REGMAP_I2C
+   ---help---
+ This is a driver for the lm3646 dual flash controllers. It controls
+ flash, torch LEDs.
+
 comment Video improvement chips
 
 config VIDEO_UPD64031A
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index e03f177..a52cda6 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -71,6 +71,7 @@ obj-$(CONFIG_VIDEO_S5C73M3)   += s5c73m3/
 obj-$(CONFIG_VIDEO_ADP1653)+= adp1653.o
 obj-$(CONFIG_VIDEO_AS3645A)+= as3645a.o
 obj-$(CONFIG_VIDEO_LM3560) += lm3560.o
+obj-$(CONFIG_VIDEO_LM3646) += lm3646.o
 obj-$(CONFIG_VIDEO_SMIAPP_PLL) += smiapp-pll.o
 obj-$(CONFIG_VIDEO_AK881X) += ak881x.o
 obj-$(CONFIG_VIDEO_IR_I2C)  += ir-kbd-i2c.o
diff --git a/drivers/media/i2c/lm3646.c b/drivers/media/i2c/lm3646.c
new file mode 100644
index 000..4b025f2
--- /dev/null
+++ b/drivers/media/i2c/lm3646.c
@@ -0,0 +1,400 @@
+/*
+ * drivers/media/i2c/lm3646.c
+ * General device driver for TI lm3646, Dual FLASH LED Driver
+ *
+ * Copyright (C) 2014 Texas Instruments
+ *
+ * Contact: Daniel Jeong gshark.je...@gmail.com
+ * Ldd-Mlp ldd-...@list.ti.com
+ *
+ * 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 linux/delay.h
+#include linux/module.h
+#include linux/i2c.h
+#include linux/slab.h
+#include linux/regmap.h
+#include linux/videodev2.h
+#include media/lm3646.h
+#include media/v4l2-ctrls.h
+#include media/v4l2-device.h
+
+/* registers definitions */
+#define REG_ENABLE 0x01
+#define REG_TORCH_BR   0x05
+#define REG_FLASH_BR   0x05
+#define REG_FLASH_TOUT 0x04
+#define REG_FLAG   0x08
+#define REG_STROBE_SRC 0x06
+#define REG_LED1_FLASH_BR 0x06
+#define REG_LED1_TORCH_BR 0x07
+
+#define MASK_ENABLE0x03
+#define MASK_TORCH_BR  0x70
+#define MASK_FLASH_BR  0x0F
+#define MASK_FLASH_TOUT0x07
+#define MASK_FLAG  0xFF
+#define MASK_STROBE_SRC0x80
+
+/* Fault Mask */
+#define FAULT_TIMEOUT  (10)
+#define FAULT_SHORT_CIRCUIT(11)
+#define FAULT_UVLO (12)
+#define FAULT_IVFM (13)
+#define FAULT_OCP  (14)
+#define FAULT_OVERTEMP (15)
+#define FAULT_NTC_TRIP (16)
+#define FAULT_OVP  (17)
+
+enum led_mode {
+   MODE_SHDN = 0x0,
+   MODE_TORCH = 0x2,
+   MODE_FLASH = 0x3,
+};
+
+/*
+ * struct lm3646_flash
+ *
+ * @pdata: platform data
+ * @regmap: reg. map for i2c
+ * @lock: muxtex for serial access.
+ * @led_mode: V4L2 LED mode
+ * @ctrls_led: V4L2 contols
+ * @subdev_led: V4L2 subdev
+ */
+struct lm3646_flash {
+   struct device *dev;
+   struct lm3646_platform_data *pdata;
+   struct regmap *regmap;
+
+   enum v4l2_flash_led_mode led_mode;
+   struct v4l2_ctrl_handler ctrls_led;
+   struct v4l2_subdev subdev_led;
+};
+
+#define to_lm3646_flash(_ctrl) \
+   container_of(_ctrl-handler, struct lm3646_flash, ctrls_led)
+
+/* enable mode control */
+static int lm3646_mode_ctrl(struct lm3646_flash *flash)
+{
+   int rval = -EINVAL;
+
+   switch (flash-led_mode) {
+   case V4L2_FLASH_LED_MODE_NONE:
+   rval = regmap_update_bits(flash-regmap,
+ REG_ENABLE, MASK_ENABLE, MODE_SHDN);
+   break;
+   case V4L2_FLASH_LED_MODE_TORCH:
+   rval = regmap_update_bits(flash-regmap,
+ REG_ENABLE, MASK_ENABLE, MODE_TORCH);
+   break;
+ 

Re: [RFCv2,2/2] i2c: add new dual Flash driver,LM3646

2014-01-27 Thread Hans Verkuil
On 01/28/2014 07:55 AM, Daniel Jeong wrote:
 Add new dual flash driver. 
 LM3646 is a dual Flash LED Driver, LED1 and LED2, following the datasheet.
 But there is no registers to contorl LED2 brightness.
 LED2 brightness can be controlled by limiting max brightness.
 LED2 brightness  = Total brightness - LED1 brightness
 LED2 will be off if LED2 brightness is set equal to or bigger than Total 
 brightness.
 And the brightness step is very small, 1.46mA for Torch, 11.71mA for Flash.
 If the step is changed to mA, maximum brightness cannot be reachable.
 
 Signed-off-by: Daniel Jeong gshark.je...@gmail.com
 ---
  drivers/media/i2c/Kconfig  |9 +
  drivers/media/i2c/Makefile |1 +
  drivers/media/i2c/lm3646.c |  400 
 
  include/media/lm3646.h |   87 ++
  4 files changed, 497 insertions(+)
  create mode 100644 drivers/media/i2c/lm3646.c
  create mode 100644 include/media/lm3646.h
 
 diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
 index 842654d..654df46 100644
 --- a/drivers/media/i2c/Kconfig
 +++ b/drivers/media/i2c/Kconfig
 @@ -630,6 +630,15 @@ config VIDEO_LM3560
 This is a driver for the lm3560 dual flash controllers. It controls
 flash, torch LEDs.
  
 +config VIDEO_LM3646
 + tristate LM3646 dual flash driver support
 + depends on I2C  VIDEO_V4L2  MEDIA_CONTROLLER
 + depends on MEDIA_CAMERA_SUPPORT
 + select REGMAP_I2C
 + ---help---
 +   This is a driver for the lm3646 dual flash controllers. It controls
 +   flash, torch LEDs.
 +
  comment Video improvement chips
  
  config VIDEO_UPD64031A
 diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
 index e03f177..a52cda6 100644
 --- a/drivers/media/i2c/Makefile
 +++ b/drivers/media/i2c/Makefile
 @@ -71,6 +71,7 @@ obj-$(CONFIG_VIDEO_S5C73M3) += s5c73m3/
  obj-$(CONFIG_VIDEO_ADP1653)  += adp1653.o
  obj-$(CONFIG_VIDEO_AS3645A)  += as3645a.o
  obj-$(CONFIG_VIDEO_LM3560)   += lm3560.o
 +obj-$(CONFIG_VIDEO_LM3646)   += lm3646.o
  obj-$(CONFIG_VIDEO_SMIAPP_PLL)   += smiapp-pll.o
  obj-$(CONFIG_VIDEO_AK881X)   += ak881x.o
  obj-$(CONFIG_VIDEO_IR_I2C)  += ir-kbd-i2c.o
 diff --git a/drivers/media/i2c/lm3646.c b/drivers/media/i2c/lm3646.c
 new file mode 100644
 index 000..4b025f2
 --- /dev/null
 +++ b/drivers/media/i2c/lm3646.c
 @@ -0,0 +1,400 @@
 +/*
 + * drivers/media/i2c/lm3646.c
 + * General device driver for TI lm3646, Dual FLASH LED Driver
 + *
 + * Copyright (C) 2014 Texas Instruments
 + *
 + * Contact: Daniel Jeong gshark.je...@gmail.com
 + *   Ldd-Mlp ldd-...@list.ti.com
 + *
 + * 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 linux/delay.h
 +#include linux/module.h
 +#include linux/i2c.h
 +#include linux/slab.h
 +#include linux/regmap.h
 +#include linux/videodev2.h
 +#include media/lm3646.h
 +#include media/v4l2-ctrls.h
 +#include media/v4l2-device.h
 +
 +/* registers definitions */
 +#define REG_ENABLE   0x01
 +#define REG_TORCH_BR 0x05
 +#define REG_FLASH_BR 0x05
 +#define REG_FLASH_TOUT   0x04
 +#define REG_FLAG 0x08
 +#define REG_STROBE_SRC   0x06
 +#define REG_LED1_FLASH_BR 0x06
 +#define REG_LED1_TORCH_BR 0x07
 +
 +#define MASK_ENABLE  0x03
 +#define MASK_TORCH_BR0x70
 +#define MASK_FLASH_BR0x0F
 +#define MASK_FLASH_TOUT  0x07
 +#define MASK_FLAG0xFF
 +#define MASK_STROBE_SRC  0x80
 +
 +/* Fault Mask */
 +#define FAULT_TIMEOUT(10)
 +#define FAULT_SHORT_CIRCUIT  (11)
 +#define FAULT_UVLO   (12)
 +#define FAULT_IVFM   (13)
 +#define FAULT_OCP(14)
 +#define FAULT_OVERTEMP   (15)
 +#define FAULT_NTC_TRIP   (16)
 +#define FAULT_OVP(17)
 +
 +enum led_mode {
 + MODE_SHDN = 0x0,
 + MODE_TORCH = 0x2,
 + MODE_FLASH = 0x3,
 +};
 +
 +/*
 + * struct lm3646_flash
 + *
 + * @pdata: platform data
 + * @regmap: reg. map for i2c
 + * @lock: muxtex for serial access.
 + * @led_mode: V4L2 LED mode
 + * @ctrls_led: V4L2 contols
 + * @subdev_led: V4L2 subdev
 + */
 +struct lm3646_flash {
 + struct device *dev;
 + struct lm3646_platform_data *pdata;
 + struct regmap *regmap;
 +
 + enum v4l2_flash_led_mode led_mode;
 + struct v4l2_ctrl_handler ctrls_led;
 + struct v4l2_subdev subdev_led;
 +};
 +
 +#define to_lm3646_flash(_ctrl)   \
 + container_of(_ctrl-handler, struct lm3646_flash, ctrls_led)
 +
 +/* enable mode control */
 +static int lm3646_mode_ctrl(struct lm3646_flash *flash)
 +{
 + int rval = -EINVAL;
 +
 + switch (flash-led_mode) {
 + case V4L2_FLASH_LED_MODE_NONE:
 + rval = regmap_update_bits(flash-regmap,
 +   REG_ENABLE, MASK_ENABLE, MODE_SHDN);
 + break;
 + case V4L2_FLASH_LED_MODE_TORCH:
 +