Re: [PATCH/RFC v9 05/19] leds: Add support for max77693 mfd flash cell

2014-12-11 Thread Jacek Anaszewski

Hi Sakari,

On 12/09/2014 02:11 PM, Sakari Ailus wrote:

Hi Jacek,

On Thu, Dec 04, 2014 at 12:06:59PM +0100, Jacek Anaszewski wrote:

Hi Sakari,

Thanks for the review.


You're welcome! :-)


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.anaszew...@samsung.com
Signed-off-by: Andrzej Hajda a.ha...@samsung.com
Acked-by: Kyungmin Park kyungmin.p...@samsung.com
Cc: Bryan Wu coolo...@gmail.com
Cc: Richard Purdie rpur...@rpsys.net
Cc: Lee Jones lee.jo...@linaro.org
Cc: Chanwoo Choi cw00.c...@samsung.com
---
  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 000..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.anaszew...@samsung.com
+ *  Andrzej Hajda a.ha...@samsung.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 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 {


This could then be renamed as max77693_led; up to you.


It will be better to rename the max77693_led structure to
max77693_led_device instead of max77693_device like you proposed. This
is because there is already max77693_dev structure in the mfd driver
for max77693 and this is only a driver for the led part of a device.

Taking above into account I'd rather leave struct max77693_sub_led
name unchanged.


+   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;
+   

Re: [PATCH/RFC v9 05/19] leds: Add support for max77693 mfd flash cell

2014-12-11 Thread Sakari Ailus
Hi Jacek,

On Thu, Dec 11, 2014 at 02:53:37PM +0100, Jacek Anaszewski wrote:
 Hi Sakari,
 
 On 12/09/2014 02:11 PM, Sakari Ailus wrote:
 Hi Jacek,
 
 On Thu, Dec 04, 2014 at 12:06:59PM +0100, Jacek Anaszewski wrote:
 Hi Sakari,
 
 Thanks for the review.
 
 You're welcome! :-)
 
 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.anaszew...@samsung.com
 Signed-off-by: Andrzej Hajda a.ha...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Bryan Wu coolo...@gmail.com
 Cc: Richard Purdie rpur...@rpsys.net
 Cc: Lee Jones lee.jo...@linaro.org
 Cc: Chanwoo Choi cw00.c...@samsung.com
 ---
   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 000..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.anaszew...@samsung.com
 + *Andrzej Hajda a.ha...@samsung.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 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 {
 
 This could then be renamed as max77693_led; up to you.
 
 It will be better to rename the max77693_led structure to
 max77693_led_device instead of max77693_device like you proposed. This
 is because there is already max77693_dev structure in the mfd driver
 for max77693 and this is only a driver for the led part of a device.
 
 Taking above into account I'd rather leave struct max77693_sub_led
 name unchanged.

Fine for me.

 + 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 

Re: [PATCH/RFC v9 05/19] leds: Add support for max77693 mfd flash cell

2014-12-09 Thread Sakari Ailus
Hi Jacek,

On Thu, Dec 04, 2014 at 12:06:59PM +0100, Jacek Anaszewski wrote:
 Hi Sakari,
 
 Thanks for the review.

You're welcome! :-)

 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.anaszew...@samsung.com
 Signed-off-by: Andrzej Hajda a.ha...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Bryan Wu coolo...@gmail.com
 Cc: Richard Purdie rpur...@rpsys.net
 Cc: Lee Jones lee.jo...@linaro.org
 Cc: Chanwoo Choi cw00.c...@samsung.com
 ---
   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 000..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.anaszew...@samsung.com
 + *  Andrzej Hajda a.ha...@samsung.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 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 {

This could then be renamed as max77693_led; up to you.

 +   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 

Re: [PATCH/RFC v9 05/19] leds: Add support for max77693 mfd flash cell

2014-12-04 Thread Sakari Ailus
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.anaszew...@samsung.com
 Signed-off-by: Andrzej Hajda a.ha...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Bryan Wu coolo...@gmail.com
 Cc: Richard Purdie rpur...@rpsys.net
 Cc: Lee Jones lee.jo...@linaro.org
 Cc: Chanwoo Choi cw00.c...@samsung.com
 ---
  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 000..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.anaszew...@samsung.com
 + *Andrzej Hajda a.ha...@samsung.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 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))

 +#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 

Re: [PATCH/RFC v9 05/19] leds: Add support for max77693 mfd flash cell

2014-12-04 Thread Jacek Anaszewski

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.anaszew...@samsung.com
Signed-off-by: Andrzej Hajda a.ha...@samsung.com
Acked-by: Kyungmin Park kyungmin.p...@samsung.com
Cc: Bryan Wu coolo...@gmail.com
Cc: Richard Purdie rpur...@rpsys.net
Cc: Lee Jones lee.jo...@linaro.org
Cc: Chanwoo Choi cw00.c...@samsung.com
---
  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 000..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.anaszew...@samsung.com
+ *  Andrzej Hajda a.ha...@samsung.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 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 - 

[PATCH/RFC v9 05/19] leds: Add support for max77693 mfd flash cell

2014-12-03 Thread Jacek Anaszewski
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.anaszew...@samsung.com
Signed-off-by: Andrzej Hajda a.ha...@samsung.com
Acked-by: Kyungmin Park kyungmin.p...@samsung.com
Cc: Bryan Wu coolo...@gmail.com
Cc: Richard Purdie rpur...@rpsys.net
Cc: Lee Jones lee.jo...@linaro.org
Cc: Chanwoo Choi cw00.c...@samsung.com
---
 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 000..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.anaszew...@samsung.com
+ *  Andrzej Hajda a.ha...@samsung.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 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)
+
+#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 {
+   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,
+