Hi Gene,

Thank you for the patch. Please find my comments in the code below.

On 6/4/20 8:26 AM, Gene Chen wrote:
From: Gene Chen <gene_c...@richtek.com>

Add MT6360 LED driver include 2-channel Flash LED with torch/strobe mode,
and 3-channel RGB LED support Register/Flash/Breath Mode

Signed-off-by: Gene Chen <gene_c...@richtek.com>
base-commit: 098c4adf249c198519a4abebe482b1e6b8c50e47
---
  drivers/leds/Kconfig       |   11 +
  drivers/leds/Makefile      |    1 +
  drivers/leds/leds-mt6360.c | 1061 ++++++++++++++++++++++++++++++++++++++++++++
  include/linux/mfd/mt6360.h |    6 +-
  4 files changed, 1078 insertions(+), 1 deletion(-)
  create mode 100644 drivers/leds/leds-mt6360.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index c664d84..c47be91 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -229,6 +229,17 @@ config LEDS_MT6323
          This option enables support for on-chip LED drivers found on
          Mediatek MT6323 PMIC.
+config LEDS_MT6360
+       tristate "LED Support for Mediatek MT6360 PMIC"
+       depends on LEDS_CLASS_FLASH && OF

I can't find DT bindings for this driver. Neither in this patch,
nor in mfd bindings.

+       depends on V4L2_FLASH_LED_CLASS || !V4L2_FLASH_LED_CLASS
+       depends on MFD_MT6360
+       help
+         This option enables support for dual Flash LED drivers found on
+         Mediatek MT6360 PMIC.
+         Support Torch and Strobe mode independently current source.
+         Include Low-VF and short protection.
+
  config LEDS_S3C24XX
        tristate "LED Support for Samsung S3C24XX GPIO LEDs"
        depends on LEDS_CLASS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 45235d5..2883b4d 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -63,6 +63,7 @@ obj-$(CONFIG_LEDS_MIKROTIK_RB532)     += leds-rb532.o
  obj-$(CONFIG_LEDS_MLXCPLD)            += leds-mlxcpld.o
  obj-$(CONFIG_LEDS_MLXREG)             += leds-mlxreg.o
  obj-$(CONFIG_LEDS_MT6323)             += leds-mt6323.o
+obj-$(CONFIG_LEDS_MT6360)              += leds-mt6360.o
  obj-$(CONFIG_LEDS_NET48XX)            += leds-net48xx.o
  obj-$(CONFIG_LEDS_NETXBIG)            += leds-netxbig.o
  obj-$(CONFIG_LEDS_NIC78BX)            += leds-nic78bx.o
diff --git a/drivers/leds/leds-mt6360.c b/drivers/leds/leds-mt6360.c
new file mode 100644
index 0000000..3e62547
--- /dev/null
+++ b/drivers/leds/leds-mt6360.c
@@ -0,0 +1,1061 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2020 MediaTek Inc.
+ *
+ * Author: Gene Chen <gene_c...@richtek.com>
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/regmap.h>
+#include <linux/platform_device.h>
+#include <linux/interrupt.h>
+#include <linux/of.h>
+#include <linux/led-class-flash.h>
+#include <media/v4l2-flash-led-class.h>
+
+#include <linux/mfd/mt6360.h>
+
+enum {
+       MT6360_LED_ISINK1 = 0,
+       MT6360_LED_ISINK2,
+       MT6360_LED_ISINK3,
+       MT6360_LED_ISINK4,
+       MT6360_LED_MAX,
+};
+
+enum {
+       MT6360_LEDMODE_PWM = 0,
+       MT6360_LEDMODE_BREATH,
+       MT6360_LEDMODE_CC,
+       MT6360_LEDMODE_MAX,
+};
+
+enum {
+       MT6360_FLED_CH1 = 0,
+       MT6360_FLED_CH2,
+       MT6360_FLED_MAX,
+};
+
+/* ILED setting/reg */
+#define MT6360_SINKCUR_MAX1    (0x0d)
+#define MT6360_SINKCUR_MAX2    (0x0d)
+#define MT6360_SINKCUR_MAX3    (0x0d)
+#define MT6360_SINKCUR_MAX4    (0x1f)

There is much redundancy and needless code obscurity in this
approach. I propose to provide macros for calculating particular
register value basing on passed argument.

e.g.

#define MT6360_SINKCUR_MAX(val) (((val) == 4) ? 0x1f : 0x0d)

+#define MT6360_CURRSEL_REG1    (MT6360_PMU_RGB1_ISNK)
+#define MT6360_CURRSEL_REG2    (MT6360_PMU_RGB2_ISNK)
+#define MT6360_CURRSEL_REG3    (MT6360_PMU_RGB3_ISNK)
+#define MT6360_CURRSEL_REG4    (MT6360_PMU_RGB_ML_ISNK)
+#define MT6360_CURRSEL_MASK1   (0x0f)
+#define MT6360_CURRSEL_MASK2   (0x0f)
+#define MT6360_CURRSEL_MASK3   (0x0f)
+#define MT6360_CURRSEL_MASK4   (0x1f)
+#define MT6360_LEDMODE_REG1    (MT6360_PMU_RGB1_ISNK)
+#define MT6360_LEDMODE_REG2    (MT6360_PMU_RGB2_ISNK)
+#define MT6360_LEDMODE_REG3    (MT6360_PMU_RGB3_ISNK)
+#define MT6360_LEDMODE_REG4    (0)
+#define MT6360_LEDMODE_MASK1   (0xc0)
+#define MT6360_LEDMODE_MASK2   (0xc0)
+#define MT6360_LEDMODE_MASK3   (0xc0)
+#define MT6360_LEDMODE_MASK4   (0)
+#define MT6360_PWMDUTY_REG1    (MT6360_PMU_RGB1_DIM)
+#define MT6360_PWMDUTY_REG2    (MT6360_PMU_RGB2_DIM)
+#define MT6360_PWMDUTY_REG3    (MT6360_PMU_RGB3_DIM)
+#define MT6360_PWMDUTY_REG4    (0)
+#define MT6360_PWMDUTY_MASK1   (0xff)
+#define MT6360_PWMDUTY_MASK2   (0xff)
+#define MT6360_PWMDUTY_MASK3   (0xff)
+#define MT6360_PWMDUTY_MASK4   (0)
+#define MT6360_PWMFREQ_REG1    (MT6360_PMU_RGB12_Freq)
+#define MT6360_PWMFREQ_REG2    (MT6360_PMU_RGB12_Freq)
+#define MT6360_PWMFREQ_REG3    (MT6360_PMU_RGB34_Freq)
+#define MT6360_PWMFREQ_REG4    (0)
+#define MT6360_PWMFREQ_MASK1   (0xe0)
+#define MT6360_PWMFREQ_MASK2   (0x1c)
+#define MT6360_PWMFREQ_MASK3   (0xe0)
+#define MT6360_PWMFREQ_MASK4   (0)
+#define MT6360_BREATH_REGBASE1 (MT6360_PMU_RGB1_Tr)
+#define MT6360_BREATH_REGBASE2 (MT6360_PMU_RGB2_Tr)
+#define MT6360_BREATH_REGBASE3 (MT6360_PMU_RGB3_Tr)
+#define MT6360_BREATH_REGBASE4 (0)
+#define MT6360_LEDEN_MASK1     (0x80)
+#define MT6360_LEDEN_MASK2     (0x40)
+#define MT6360_LEDEN_MASK3     (0x20)
+#define MT6360_LEDEN_MASK4     (0x10)
+#define MT6360_LEDEN_REG       (MT6360_PMU_RGB_EN)
+#define MT6360_LEDALLEN_MASK   (0xf0)
+
+#define MT6360_CHRIND_MASK     (0x08)
+
+/* pattern order -> toff, tr1, tr2, ton, tf1, tf2 */
+#define MT6360_BRPATTERN_NUM   (6)
+#define MT6360_BREATHREG_NUM   (3)
+
+/* FLED setting */
+#define MT6360_CSENABLE_REG1   (MT6360_PMU_FLED_EN)
+#define MT6360_CSENABLE_MASK1  (0x02)
+#define MT6360_CSENABLE_REG2   (MT6360_PMU_FLED_EN)
+#define MT6360_CSENABLE_MASK2  (0x01)
+#define MT6360_TORBRIGHT_MAX1  (0x1f)
+#define MT6360_TORBRIGHT_MAX2  (0x1f)
+#define MT6360_TORBRIGHT_REG1  (MT6360_PMU_FLED1_TOR_CTRL)
+#define MT6360_TORBRIGHT_MASK1 (0x1f)
+#define MT6360_STRBRIGHT_REG1  (MT6360_PMU_FLED1_STRB_CTRL2)
+#define MT6360_STRBRIGHT_MASK1 (0x7f)
+#define MT6360_TORBRIGHT_REG2  (MT6360_PMU_FLED2_TOR_CTRL)
+#define MT6360_TORBRIGHT_MASK2 (0x1f)
+#define MT6360_STRBRIGHT_REG2  (MT6360_PMU_FLED2_STRB_CTRL2)
+#define MT6360_STRBRIGHT_MASK2 (0x7f)
+#define MT6360_TORENABLE_REG1  (MT6360_PMU_FLED_EN)
+#define MT6360_TORENABLE_MASK1 (0x08)
+#define MT6360_TORENABLE_REG2  (MT6360_PMU_FLED_EN)
+#define MT6360_TORENABLE_MASK2 (0x08)
+#define MT6360_STRBENABLE_REG1 (MT6360_PMU_FLED_EN)
+#define MT6360_STRBENABLE_MASK1 (0x06)
+#define MT6360_STRBENABLE_REG2 (MT6360_PMU_FLED_EN)
+#define MT6360_STRBENABLE_MASK2 (0x04)
+#define MT6360_STRBTIMEOUT_REG (MT6360_PMU_FLED_STRB_CTRL)
+#define MT6360_STRBTIMEOUT_MASK        (0x7f)
+#define MT6360_TORCHCUR_MIN    (25000)
+#define MT6360_TORCHCUR_STEP   (12500)
+#define MT6360_TORCHCUR_MAX    (400000)
+#define MT6360_STROBECUR_MIN   (50000)
+#define MT6360_STROBECUR_STEP  (12500)
+#define MT6360_STROBECUR_MAX   (1500000)
+#define MT6360_STRBTIMEOUT_MIN (64000)
+#define MT6360_STRBTIMEOUT_STEP        (32000)
+#define MT6360_STRBTIMEOUT_MAX (2432000)
+
+#define MT6360_MASK_ULTRA_STROBE       (0x80)
+#define MT6360_SHIFT_ULTRA_STROBE      (7)
+
+#define MT6360_FLEDSUPPORT_FAULTS      (LED_FAULT_UNDER_VOLTAGE |\
+                                        LED_FAULT_SHORT_CIRCUIT |\
+                                        LED_FAULT_INPUT_VOLTAGE |\
+                                        LED_FAULT_TIMEOUT)
+
+struct mt6360_led_platform_data {
+       u32 rgbon_sync;
+       u32 fled1_ultraistrb;
+       u32 fled2_ultraistrb;
+};
+
+struct breath_element_cfg {
+       /* base, step in ms */
+       unsigned int base;
+       unsigned int step;
+       unsigned int maxval;
+       unsigned int reg_offset;
+       unsigned int reg_mask;

With the change of approach you could avoid keeping these
values like this.

+};
+
+struct mt6360_led_classdev {
+       struct led_classdev cdev;
+       int index;
+       struct device_node *np;
+       unsigned int currsel_reg;
+       unsigned int currsel_mask;
+       unsigned int enable_mask;
+       unsigned int mode_reg;
+       unsigned int mode_mask;
+       unsigned int pwmduty_reg;
+       unsigned int pwmduty_mask;
+       unsigned int pwmfreq_reg;
+       unsigned int pwmfreq_mask;
+       unsigned int breath_regbase;

Ditto.

+};
+
+struct mt6360_fled_classdev {
+       struct led_classdev_flash fl_cdev;
+       int index;
+       struct v4l2_flash *v4l2_flash;
+       struct device_node *np;
+       unsigned int cs_enable_reg;
+       unsigned int cs_enable_mask;
+       unsigned int torch_bright_reg;
+       unsigned int torch_bright_mask;
+       unsigned int torch_enable_reg;
+       unsigned int torch_enable_mask;
+       unsigned int strobe_bright_reg;
+       unsigned int strobe_bright_mask;
+       unsigned int strobe_enable_reg;
+       unsigned int strobe_enable_mask;
+       unsigned int strobe_external_reg;
+       unsigned int strobe_external_mask;

Ditto.

+       u32 faults;
+};
+
+struct mt6360_led_data {
+       struct device *dev;
+       struct mt6360_led_platform_data *pdata;
+       struct regmap *regmap;
+       struct mt6360_led_classdev mtled_cdev[MT6360_LED_MAX];
+       struct mt6360_fled_classdev mtfled_cdev[MT6360_FLED_MAX];
+       unsigned long fl_torch_flags;
+       unsigned long fl_strobe_flags;
+};
+
+static const struct mt6360_led_platform_data def_platform_data = {
+       .rgbon_sync = 0,
+       .fled1_ultraistrb = 1,
+       .fled2_ultraistrb = 1,
+};
+
+static int mt6360_led_brightness_set(struct led_classdev *cdev,
+                                     enum led_brightness brightness)
+{
+       struct mt6360_led_classdev *mtled_cdev =
+                                            (struct mt6360_led_classdev *)cdev;
+       struct mt6360_led_data *mld = dev_get_drvdata(cdev->dev->parent);
+       int shift, sync_regval = 0, ret;
+
+       /* if isink1 user control, set chrind function to sw mode */
+       if (mtled_cdev->index == MT6360_LED_ISINK1) {
+               ret = regmap_update_bits(mld->regmap,
+                                  MT6360_PMU_RGB_EN, MT6360_CHRIND_MASK, 0xff);
+               if (ret < 0)
+                       dev_err(cdev->dev, "disable chrind func fail\n");
+       }

This may be personal preference, but I would prefer adding newlines
at the end of logical blocks of code.

E.g. like here.

+       if (brightness == LED_OFF) {
+               ret = regmap_update_bits(mld->regmap,
+                                 MT6360_LEDEN_REG, mtled_cdev->enable_mask, 0);
+               if (ret < 0)
+                       return ret;
+               if (mtled_cdev->mode_reg == 0)
+                       goto out_bright_set;
+               /* if off, force config to cc_mode */
+               shift = ffs(mtled_cdev->mode_mask) - 1;
+               ret = regmap_update_bits(mld->regmap, mtled_cdev->mode_reg,
+                            mtled_cdev->mode_mask, MT6360_LEDMODE_CC << shift);
+               if (ret < 0)
+                       dev_err(cdev->dev, "config cc mode fail\n");
+               goto out_bright_set;
+       }

Here.

+       shift = ffs(mtled_cdev->currsel_mask) - 1;
+       brightness -= 1;
+       ret = regmap_update_bits(mld->regmap, mtled_cdev->currsel_reg,
+                                mtled_cdev->currsel_mask, brightness << shift);
+       if (ret < 0)
+               return ret;

Here.

And so on for all similar occurrences.

+       if (mld->pdata->rgbon_sync) {
+               ret = regmap_read(mld->regmap, MT6360_LEDEN_REG,  &sync_regval);
+               if (ret < 0)
+                       goto out_bright_set;
+               ret = regmap_update_bits(mld->regmap,
+                                    MT6360_LEDEN_REG, MT6360_LEDALLEN_MASK, 0);
+               if (ret < 0)
+                       goto out_bright_set;
+               sync_regval |= mtled_cdev->enable_mask;
+               ret = regmap_update_bits(mld->regmap, MT6360_LEDEN_REG,
+                                        MT6360_LEDALLEN_MASK, sync_regval);
+       } else {
+               ret = regmap_update_bits(mld->regmap, MT6360_LEDEN_REG,
+                                        mtled_cdev->enable_mask, 0xff);
+       }
+out_bright_set:
+       return ret;
+}
+
+static enum led_brightness mt6360_led_brightness_get(struct led_classdev *cdev)
+{
+       struct mt6360_led_classdev *mtled_cdev =
+                                            (struct mt6360_led_classdev *)cdev;
+       struct mt6360_led_data *mld = dev_get_drvdata(cdev->dev->parent);
+       unsigned int regval = 0;
+       int shift = ffs(mtled_cdev->currsel_mask) - 1, ret;
+
+       ret = regmap_read(mld->regmap, MT6360_LEDEN_REG, &regval);
+       if (ret < 0) {
+               dev_err(cdev->dev, "%s: get enable fail\n", __func__);
+               return LED_OFF;
+       }
+       if (!(regval & mtled_cdev->enable_mask))
+               return LED_OFF;
+       ret = regmap_read(mld->regmap, mtled_cdev->currsel_reg, &regval);
+       if (ret < 0) {
+               dev_err(cdev->dev, "%s: get isink fail\n", __func__);
+               return LED_OFF;
+       }
+       regval &= mtled_cdev->currsel_mask;
+       regval >>= shift;
+       return (regval + 1);
+}
+
+static const unsigned int dim_freqs[] = {
+       4, 8, 250, 500, 1000, 2000, 4000, 8000,
+};
+
+static int mt6360_led_blink_set(struct led_classdev *cdev,
+                            unsigned long *delay_on,  unsigned long *delay_off)
+{
+       struct mt6360_led_classdev *mtled_cdev =
+                                            (struct mt6360_led_classdev *)cdev;
+       struct mt6360_led_data *mld = dev_get_drvdata(cdev->dev->parent);
+       int freq, duty, shift, sum, ret;
+
+       if (mtled_cdev->mode_reg == 0)
+               return -ENOTSUPP;
+       if (*delay_on == 0 && *delay_off == 0)
+               *delay_on = *delay_off = 500;
+       sum = *delay_on + *delay_off;
+       for (freq = 0; freq < ARRAY_SIZE(dim_freqs); freq++) {
+               if (sum <= dim_freqs[freq])
+                       break;
+       }
+       if (freq == ARRAY_SIZE(dim_freqs)) {
+               dev_err(cdev->dev, "exceed pwm frequency max\n");
+               return -EINVAL;
+       }
+       /* invert */
+       freq = ARRAY_SIZE(dim_freqs) - 1 - freq;
+       shift = ffs(mtled_cdev->pwmfreq_mask) - 1;
+       ret = regmap_update_bits(mld->regmap, mtled_cdev->pwmfreq_reg,
+                                mtled_cdev->pwmfreq_mask, freq << shift);
+       if (ret < 0) {
+               dev_err(cdev->dev, "Failed to set pwmfreq\n");
+               return ret;
+       }
+       duty = 255 * (*delay_on) / sum;
+       shift = ffs(mtled_cdev->pwmduty_mask) - 1;
+       ret = regmap_update_bits(mld->regmap, mtled_cdev->pwmduty_reg,
+                                mtled_cdev->pwmduty_mask, duty << shift);
+       if (ret < 0) {
+               dev_err(cdev->dev, "Failed to set pwmduty\n");
+               return ret;
+       }
+       shift = ffs(mtled_cdev->mode_mask) - 1;
+       ret = regmap_update_bits(mld->regmap, mtled_cdev->mode_reg,
+                           mtled_cdev->mode_mask, MT6360_LEDMODE_PWM << shift);
+       return ret;
+}
+
+#define MT6360_LED_DESC(_id)  {                                                
\
+       .cdev = {                                                       \
+               .name = "mt6360_isink" #_id,                          \
+               .max_brightness = MT6360_SINKCUR_MAX##_id,              \
+               .brightness_set_blocking = mt6360_led_brightness_set,   \
+               .brightness_get = mt6360_led_brightness_get,            \
+               .blink_set = mt6360_led_blink_set,                      \
+       },                                                              \
+       .index = MT6360_LED_ISINK##_id,                                 \
+       .currsel_reg = MT6360_CURRSEL_REG##_id,                         \
+       .currsel_mask = MT6360_CURRSEL_MASK##_id,                       \
+       .enable_mask = MT6360_LEDEN_MASK##_id,                          \
+       .mode_reg = MT6360_LEDMODE_REG##_id,                            \
+       .mode_mask = MT6360_LEDMODE_MASK##_id,                          \
+       .pwmduty_reg = MT6360_PWMDUTY_REG##_id,                         \
+       .pwmduty_mask = MT6360_PWMDUTY_MASK##_id,                       \
+       .pwmfreq_reg = MT6360_PWMFREQ_REG##_id,                         \
+       .pwmfreq_mask = MT6360_PWMFREQ_MASK##_id,                       \
+       .breath_regbase = MT6360_BREATH_REGBASE##_id,                   \
+}

Let's better avoid code generation via macros. It horribly hinders code
analysis and debugging.

+
+/* ISINK 1/2/3 for RGBLED, ISINK4 for MoonLight */
+static const struct mt6360_led_classdev def_led_classdev[MT6360_LED_MAX] = {
+       MT6360_LED_DESC(1),
+       MT6360_LED_DESC(2),
+       MT6360_LED_DESC(3),
+       MT6360_LED_DESC(4),
+};
+
+static inline bool mt6360_fled_check_flags_if_any(unsigned long *flags)
+{
+       return (*flags) ? true : false;

Just do "return *flags" and it will be cast to bool automatically.

Although I think that the function is redundant.

+}
+
+static int mt6360_fled_strobe_brightness_set(
+                          struct led_classdev_flash *fled_cdev, u32 brightness)
+{
+       struct led_classdev *led_cdev = &fled_cdev->led_cdev;
+       struct mt6360_led_data *mld = dev_get_drvdata(led_cdev->dev->parent);
+       struct led_flash_setting *fs = &fled_cdev->brightness;
+       struct mt6360_fled_classdev *mtfled_cdev = (void *)fled_cdev;
+       int shift;
+       u32 val;
+
+       val = brightness;
+       val = (val - fs->min) / fs->step;
+       shift = ffs(mtfled_cdev->strobe_bright_mask) - 1;
+       return regmap_update_bits(mld->regmap, mtfled_cdev->strobe_bright_reg,
+                                mtfled_cdev->strobe_bright_mask, val << shift);
+}
+
+static int mt6360_fled_strobe_brightness_get(
+                         struct led_classdev_flash *fled_cdev, u32 *brightness)
+{
+       struct led_classdev *led_cdev = &fled_cdev->led_cdev;
+       struct mt6360_led_data *mld = dev_get_drvdata(led_cdev->dev->parent);
+       struct led_flash_setting *fs = &fled_cdev->brightness;
+       struct mt6360_fled_classdev *mtfled_cdev = (void *)fled_cdev;
+       int shift, ret;
+       u32 regval = 0;
+
+       ret = regmap_read(mld->regmap, mtfled_cdev->strobe_bright_reg, &regval);
+       if (ret < 0)
+               return ret;
+       regval &= mtfled_cdev->strobe_bright_mask;
+       shift = ffs(mtfled_cdev->strobe_bright_mask) - 1;
+       regval >>= shift;
+       /* convert to microamp value */
+       *brightness = regval * fs->step + fs->min;
+       return 0;
+}
+
+static int mt6360_fled_strobe_set(
+                              struct led_classdev_flash *fled_cdev, bool state)
+{
+       struct led_classdev *led_cdev = &fled_cdev->led_cdev;
+       struct mt6360_led_data *mld = dev_get_drvdata(led_cdev->dev->parent);
+       struct mt6360_fled_classdev *mtfled_cdev = (void *)fled_cdev;
+       int id = mtfled_cdev->index, ret;
+
+       if (!(state ^ test_bit(id, &mld->fl_strobe_flags)))
+               return 0;
+       if (mt6360_fled_check_flags_if_any(&mld->fl_torch_flags)) {
+               dev_err(led_cdev->dev,
+                       "Disable all leds torch [%lu]\n", mld->fl_torch_flags);
+               return -EINVAL;
+       }
+       ret = regmap_update_bits(mld->regmap, mtfled_cdev->cs_enable_reg,
+                                mtfled_cdev->cs_enable_mask, state ? 0xff : 0);
+       if (ret < 0) {
+               dev_err(led_cdev->dev, "Fail to set cs enable [%d]\n", state);
+               goto out_strobe_set;
+       }
+       ret = regmap_update_bits(mld->regmap, mtfled_cdev->strobe_enable_reg,
+                            mtfled_cdev->strobe_enable_mask, state ? 0xff : 0);
+       if (ret < 0) {
+               dev_err(led_cdev->dev, "Fail to set strb enable [%d]\n", state);
+               goto out_strobe_set;
+       }
+       if (state) {
+               if (!mt6360_fled_check_flags_if_any(&mld->fl_strobe_flags))
+                       usleep_range(5000, 6000);
+               set_bit(id, &mld->fl_strobe_flags);
+               mtfled_cdev->faults = 0;
+       } else {
+               clear_bit(id, &mld->fl_strobe_flags);
+               if (!mt6360_fled_check_flags_if_any(&mld->fl_strobe_flags))
+                       usleep_range(400, 500);
+       }
+out_strobe_set:
+       return ret;
+}
+
+static int mt6360_fled_strobe_get(
+                             struct led_classdev_flash *fled_cdev, bool *state)
+{
+       struct led_classdev *led_cdev = &fled_cdev->led_cdev;
+       struct mt6360_led_data *mld = dev_get_drvdata(led_cdev->dev->parent);
+       struct mt6360_fled_classdev *mtfled_cdev = (void *)fled_cdev;
+       int id = mtfled_cdev->index;
+
+       *state = test_bit(id, &mld->fl_strobe_flags) ? true : false;
+       return 0;
+}
+
+static int mt6360_fled_strobe_timeout_set(
+                             struct led_classdev_flash *fled_cdev, u32 timeout)
+{
+       struct led_classdev *led_cdev = &fled_cdev->led_cdev;
+       struct mt6360_led_data *mld = dev_get_drvdata(led_cdev->dev->parent);
+       struct led_flash_setting *ts = &fled_cdev->timeout;
+       int shift, ret;
+       u32 regval;
+
+       regval = (timeout - ts->min) / ts->step;
+       shift = ffs(MT6360_STRBTIMEOUT_MASK) - 1;
+       ret = regmap_update_bits(mld->regmap, MT6360_STRBTIMEOUT_REG,
+                                MT6360_STRBTIMEOUT_MASK, regval << shift);
+       return ret;
+}
+
+static int mt6360_fled_strobe_fault_get(
+                              struct led_classdev_flash *fled_cdev, u32 *fault)
+{
+       struct mt6360_fled_classdev *mtfled_cdev = (void *)fled_cdev;
+
+       *fault = mtfled_cdev->faults;
+       return 0;
+}
+
+static const struct led_flash_ops mt6360_fled_ops = {
+       .flash_brightness_set = mt6360_fled_strobe_brightness_set,
+       .flash_brightness_get = mt6360_fled_strobe_brightness_get,
+       .strobe_set = mt6360_fled_strobe_set,
+       .strobe_get = mt6360_fled_strobe_get,
+       .timeout_set = mt6360_fled_strobe_timeout_set,
+       .fault_get = mt6360_fled_strobe_fault_get,
+};
+
+static int mt6360_fled_brightness_set(struct led_classdev *led_cdev,
+                                     enum led_brightness brightness)
+{
+       struct led_classdev_flash *lcf = lcdev_to_flcdev(led_cdev);
+       struct mt6360_led_data *mld = dev_get_drvdata(led_cdev->dev->parent);
+       struct mt6360_fled_classdev *mtfled_cdev = (void *)lcf;
+       int id = mtfled_cdev->index, shift, keep, ret;
+
+       if (mt6360_fled_check_flags_if_any(&mld->fl_strobe_flags)) {
+               dev_err(led_cdev->dev,
+                      "Disable all leds strobe [%lu]\n", mld->fl_strobe_flags);
+               return -EINVAL;
+       }
+       if (brightness == LED_OFF) {
+               clear_bit(id, &mld->fl_torch_flags);
+               keep = mt6360_fled_check_flags_if_any(&mld->fl_torch_flags);
+               ret = regmap_update_bits(mld->regmap,
+                                        mtfled_cdev->torch_enable_reg,
+                                        mtfled_cdev->torch_enable_mask,
+                                        keep ? 0xff : 0);
+               if (ret < 0) {
+                       dev_err(led_cdev->dev, "Fail to set torch disable\n");
+                       goto out_bright_set;
+               }
+               ret = regmap_update_bits(mld->regmap,
+                                        mtfled_cdev->cs_enable_reg,
+                                        mtfled_cdev->cs_enable_mask, 0);
+               if (ret < 0)
+                       dev_err(led_cdev->dev, "Fail to set torch disable\n");
+               goto out_bright_set;
+       }
+       shift = ffs(mtfled_cdev->torch_bright_mask) - 1;
+       brightness -= 1;
+       ret = regmap_update_bits(mld->regmap, mtfled_cdev->torch_bright_reg,
+                          mtfled_cdev->torch_bright_mask, brightness << shift);
+       if (ret < 0) {
+               dev_err(led_cdev->dev,
+                       "Fail to set torch bright [%d]\n", brightness);
+               goto out_bright_set;
+       }
+       ret = regmap_update_bits(mld->regmap, mtfled_cdev->cs_enable_reg,
+                                mtfled_cdev->cs_enable_mask, 0xff);
+       if (ret < 0) {
+               dev_err(led_cdev->dev, "Fail to set cs enable\n");
+               goto out_bright_set;
+       }
+       ret = regmap_update_bits(mld->regmap, mtfled_cdev->torch_enable_reg,
+                                mtfled_cdev->torch_enable_mask, 0xff);
+       set_bit(id, &mld->fl_torch_flags);
+out_bright_set:
+       return ret;
+}
+
+static enum led_brightness mt6360_fled_brightness_get(
+                                                 struct led_classdev *led_cdev)
+{
+       struct led_classdev_flash *lcf = lcdev_to_flcdev(led_cdev);
+       struct mt6360_led_data *mld = dev_get_drvdata(led_cdev->dev->parent);
+       struct mt6360_fled_classdev *mtfled_cdev = (void *)lcf;
+       int id = mtfled_cdev->index, shift, ret;
+       u32 regval = 0;
+
+       if (!test_bit(id, &mld->fl_torch_flags))
+               return LED_OFF;
+       ret = regmap_read(mld->regmap, mtfled_cdev->torch_bright_reg, &regval);
+       if (ret < 0) {
+               dev_err(led_cdev->dev, "%s: Fail to get torb reg\n", __func__);
+               return LED_OFF;
+       }
+       shift = ffs(mtfled_cdev->torch_bright_mask) - 1;
+       regval &= mtfled_cdev->torch_bright_mask;
+       regval >>= shift;
+       return (regval + 1);
+}
+
+#define MT6360_FLED_DESC(_id)  {                                       \
+       .fl_cdev = {                                                    \
+        .led_cdev = {                                                  \
+               .name = "mt6360_fled_ch" #_id,                                \
+               .max_brightness = MT6360_TORBRIGHT_MAX##_id,            \
+               .brightness_set_blocking =  mt6360_fled_brightness_set, \
+               .brightness_get = mt6360_fled_brightness_get,           \
+               .flags = LED_DEV_CAP_FLASH,                             \
+        },                                                             \
+        .brightness = {                                                \
+               .min = MT6360_STROBECUR_MIN,                            \
+               .step = MT6360_STROBECUR_STEP,                          \
+               .max = MT6360_STROBECUR_MAX,                            \
+               .val = MT6360_STROBECUR_MIN,                            \
+        },                                                             \
+        .timeout = {                                                   \
+               .min = MT6360_STRBTIMEOUT_MIN,                          \
+               .step = MT6360_STRBTIMEOUT_STEP,                        \
+               .max = MT6360_STRBTIMEOUT_MAX,                          \
+               .val = MT6360_STRBTIMEOUT_MIN,                          \
+        },                                                             \
+        .ops = &mt6360_fled_ops,                                   \
+       },                                                              \
+       .index = MT6360_FLED_CH##_id,                                   \
+       .cs_enable_reg = MT6360_CSENABLE_REG##_id,                      \
+       .cs_enable_mask = MT6360_CSENABLE_MASK##_id,                    \
+       .torch_bright_reg = MT6360_TORBRIGHT_REG##_id,                  \
+       .torch_bright_mask = MT6360_TORBRIGHT_MASK##_id,                \
+       .torch_enable_reg = MT6360_TORENABLE_REG##_id,                  \
+       .torch_enable_mask = MT6360_TORENABLE_MASK##_id,                \
+       .strobe_bright_reg = MT6360_STRBRIGHT_REG##_id,                 \
+       .strobe_bright_mask = MT6360_STRBRIGHT_MASK##_id,               \
+       .strobe_enable_reg = MT6360_STRBENABLE_REG##_id,                \
+       .strobe_enable_mask = MT6360_STRBENABLE_MASK##_id,              \
+}
+
+static const struct mt6360_fled_classdev def_fled_classdev[MT6360_FLED_MAX] = {
+       MT6360_FLED_DESC(1),
+       MT6360_FLED_DESC(2),
+};
+
+#if IS_ENABLED(CONFIG_V4L2_FLASH_LED_CLASS)
+static int mt6360_fled_external_strobe_set(
+                                    struct v4l2_flash *v4l2_flash, bool enable)
+{
+       struct led_classdev_flash *lcf = v4l2_flash->fled_cdev;
+       struct led_classdev *led_cdev = &lcf->led_cdev;
+       struct mt6360_led_data *mld = dev_get_drvdata(led_cdev->dev->parent);
+       struct mt6360_fled_classdev *mtfled_cdev = (void *)lcf;
+       int id = mtfled_cdev->index, ret;
+
+       if (!(enable ^ test_bit(id, &mld->fl_strobe_flags)))
+               return 0;
+       if (mt6360_fled_check_flags_if_any(&mld->fl_torch_flags)) {
+               dev_err(led_cdev->dev,
+                       "Disable all leds torch [%lu]\n", mld->fl_torch_flags);
+               return -EINVAL;
+       }
+       ret = regmap_update_bits(mld->regmap, mtfled_cdev->cs_enable_reg,
+                         mtfled_cdev->cs_enable_mask, enable ? 0xff : 0);
+       if (enable) {
+               set_bit(id, &mld->fl_strobe_flags);
+               mtfled_cdev->faults = 0;
+       } else
+               clear_bit(id, &mld->fl_strobe_flags);
+       return ret;
+}
+
+static const struct v4l2_flash_ops v4l2_flash_ops = {
+       .external_strobe_set = mt6360_fled_external_strobe_set,
+};
+
+static void mt6360_init_v4l2_flash_config(
+                                      struct mt6360_fled_classdev *mtfled_cdev,
+                                      struct v4l2_flash_config *config)
+{
+       struct led_flash_setting *torch_intensity = &config->intensity;
+       struct led_classdev *led_cdev = &(mtfled_cdev->fl_cdev.led_cdev);
+       s32 val;
+
+       snprintf(config->dev_name, sizeof(config->dev_name),
+                "%s", mtfled_cdev->fl_cdev.led_cdev.name);
+       torch_intensity->min = MT6360_TORCHCUR_MIN;
+       torch_intensity->step = MT6360_TORCHCUR_STEP;
+       val = MT6360_TORCHCUR_MIN;
+       val += ((led_cdev->max_brightness - 1) * MT6360_TORCHCUR_STEP);
+       torch_intensity->val = torch_intensity->max = val;
+       config->flash_faults |= MT6360_FLEDSUPPORT_FAULTS;
+       config->has_external_strobe = 1;
+}
+#else
+static const struct v4l2_flash_ops v4l2_flash_ops;
+
+static void mt6360_init_v4l2_flash_config(
+                                      struct mt6360_fled_classdev *mtfled_cdev,
+                                      struct v4l2_flash_config *config)
+{
+}
+#endif /* IS_ENABLED(CONFIG_V4L2_FLASH_LED_CLASS) */
+
+static irqreturn_t mt6360_pmu_fled_lvf_evt_handler(int irq, void *data)
+{
+       struct mt6360_led_data *mld = data;
+
+       dev_err(mld->dev, "%s\n", __func__);
+       mld->mtfled_cdev[MT6360_FLED_CH1].faults |= LED_FAULT_UNDER_VOLTAGE;
+       mld->mtfled_cdev[MT6360_FLED_CH2].faults |= LED_FAULT_UNDER_VOLTAGE;
+       return IRQ_HANDLED;
+}
+
+static irqreturn_t mt6360_pmu_fled2_short_evt_handler(int irq, void *data)
+{
+       struct mt6360_led_data *mld = data;
+
+       dev_err(mld->dev, "%s\n", __func__);
+       mld->mtfled_cdev[MT6360_FLED_CH2].faults |= LED_FAULT_SHORT_CIRCUIT;
+       return IRQ_HANDLED;
+}
+
+static irqreturn_t mt6360_pmu_fled1_short_evt_handler(int irq, void *data)
+{
+       struct mt6360_led_data *mld = data;
+
+       dev_err(mld->dev, "%s\n", __func__);
+       mld->mtfled_cdev[MT6360_FLED_CH1].faults |= LED_FAULT_SHORT_CIRCUIT;
+       return IRQ_HANDLED;
+}
+
+static irqreturn_t mt6360_pmu_fled2_strb_to_evt_handler(int irq, void *data)
+{
+       struct mt6360_led_data *mld = data;
+
+       mld->mtfled_cdev[MT6360_FLED_CH2].faults |= LED_FAULT_TIMEOUT;
+       return IRQ_HANDLED;
+}
+
+static irqreturn_t mt6360_pmu_fled1_strb_to_evt_handler(int irq, void *data)
+{
+       struct mt6360_led_data *mld = data;
+
+       mld->mtfled_cdev[MT6360_FLED_CH1].faults |= LED_FAULT_TIMEOUT;
+       return IRQ_HANDLED;
+}
+
+static irqreturn_t mt6360_pmu_fled_chg_vinovp_evt_handler(int irq, void *data)
+{
+       struct mt6360_led_data *mld = data;
+
+       dev_warn(mld->dev, "%s\n", __func__);
+       mld->mtfled_cdev[MT6360_FLED_CH1].faults |= LED_FAULT_INPUT_VOLTAGE;
+       mld->mtfled_cdev[MT6360_FLED_CH2].faults |= LED_FAULT_INPUT_VOLTAGE;
+       return IRQ_HANDLED;
+}
+
+static struct mt6360_pmu_irq_desc mt6360_pmu_fled_irq_desc[] = {
+       { "fled_chg_vinovp_evt",  mt6360_pmu_fled_chg_vinovp_evt_handler },
+       { "fled_lvf_evt", mt6360_pmu_fled_lvf_evt_handler },
+       { "fled2_short_evt", mt6360_pmu_fled2_short_evt_handler },
+       { "fled1_short_evt", mt6360_pmu_fled1_short_evt_handler },
+       { "fled2_strb_to_evt", mt6360_pmu_fled2_strb_to_evt_handler },
+       { "fled1_strb_to_evt", mt6360_pmu_fled1_strb_to_evt_handler },
+};
+
+static int mt6360_fled_irq_register(struct platform_device *pdev)
+{
+       struct mt6360_pmu_irq_desc *irq_desc;
+       int i, irq, ret;
+
+       for (i = 0; i < ARRAY_SIZE(mt6360_pmu_fled_irq_desc); i++) {
+               irq_desc = mt6360_pmu_fled_irq_desc + i;
+               if (unlikely(!irq_desc->name))
+                       continue;
+               irq = platform_get_irq_byname(pdev, irq_desc->name);
+               if (irq < 0)
+                       continue;
+               ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
+                                               irq_desc->irq_handler,
+                                               IRQF_TRIGGER_FALLING,
+                                               irq_desc->name,
+                                               platform_get_drvdata(pdev));
+               if (ret < 0) {
+                       dev_err(&pdev->dev,
+                               "request %s irq fail\n", irq_desc->name);
+                       return ret;
+               }
+       }
+       return 0;
+}
+
+static int mt6360_iled_parse_dt(struct device *dev,
+                               struct mt6360_led_data *mld)
+{
+       struct device_node *iled_np, *child;
+       struct mt6360_led_classdev *mtled_cdev;
+       u32 val;
+       int ret;
+
+       if (!dev->of_node)
+               return 0;
+       iled_np = of_find_node_by_name(dev->of_node, "iled");
+       if (!iled_np)
+               return 0;
+       for_each_available_child_of_node(iled_np, child) {
+               ret = of_property_read_u32(child, "reg", &val);
+               if (ret) {
+                       dev_err(dev, "Fail to read reg property\n");
+                       continue;
+               }
+               if (val >= MT6360_LED_MAX) {
+                       dev_err(dev, "Invalid iled reg [%u]\n", val);
+                       ret = -EINVAL;
+                       goto out_iled_dt;
+               }
+               mtled_cdev = mld->mtled_cdev + val;
+
+               of_property_read_string(child,
+                                       "label", &(mtled_cdev->cdev.name));

If you used new "*ext" LED registration API the LED core would do LED name parsing for you. You'd have to use 'function' and 'color' DT
properties instead of label then.

+               of_property_read_string(child, "linux,default-trigger",
+                                       &(mtled_cdev->cdev.default_trigger));
+               mtled_cdev->np = child;
+       }
+       return 0;
+out_iled_dt:
+       of_node_put(child);
+       return ret;
+}
+
+static int mt6360_fled_parse_dt(struct device *dev,
+                               struct mt6360_led_data *mld)
+{
+       struct device_node *fled_np, *child;
+       struct mt6360_fled_classdev *mtfled_cdev;
+       struct led_classdev *led_cdev;
+       struct led_flash_setting *fs;
+       u32 val;
+       int ret;
+
+       if (!dev->of_node)
+               return 0;
+       fled_np = of_find_node_by_name(dev->of_node, "fled");
+       if (!fled_np)
+               return 0;
+       for_each_available_child_of_node(fled_np, child) {
+               ret = of_property_read_u32(child, "reg", &val);
+               if (ret) {
+                       dev_err(dev, "Fail to read reg property\n");
+                       continue;
+               }
+               if (val >= MT6360_FLED_MAX) {
+                       dev_err(dev, "Invalid fled reg [%u]\n", val);
+                       ret = -EINVAL;
+                       goto out_fled_dt;
+               }
+               mtfled_cdev = mld->mtfled_cdev + val;
+
+               of_property_read_string(child, "label",
+                                       &(mtfled_cdev->fl_cdev.led_cdev.name));

Ditto.

+               ret = of_property_read_u32(child, "led-max-microamp", &val);
+               if (ret) {
+                       dev_warn(dev, "led-max-microamp property missing\n");
+                       val = MT6360_TORCHCUR_MIN;
+               }
+               if (val < MT6360_TORCHCUR_MIN)
+                       val = MT6360_TORCHCUR_MIN;
+               val = (val - MT6360_TORCHCUR_MIN) / MT6360_TORCHCUR_STEP + 1;
+               led_cdev = &(mtfled_cdev->fl_cdev.led_cdev);
+               led_cdev->max_brightness = min(led_cdev->max_brightness, val);
+               ret = of_property_read_u32(child, "flash-max-microamp", &val);
+               if (ret) {
+                       dev_warn(dev, "flash-max-microamp property missing\n");
+                       val = MT6360_STROBECUR_MIN;
+               }
+               if (val < MT6360_STROBECUR_MIN)
+                       val = MT6360_STROBECUR_MIN;
+               fs = &(mtfled_cdev->fl_cdev.brightness);
+               fs->val = fs->max = min(fs->max, val);
+               ret = of_property_read_u32(child, "flash-max-timeout", &val);
+               if (ret) {
+                       dev_warn(dev, "flash-max-timeout property missing\n");
+                       val = MT6360_STRBTIMEOUT_MIN;
+               }
+               if (val < MT6360_STRBTIMEOUT_MIN)
+                       val = MT6360_STRBTIMEOUT_MIN;
+               fs = &(mtfled_cdev->fl_cdev.timeout);
+               fs->val = fs->max = min(fs->max, val);
+               mtfled_cdev->np = child;
+       }
+       return 0;
+out_fled_dt:
+       of_node_put(child);
+       return ret;
+}
+
+static void mt6360_led_parse_dt_data(struct device *dev,
+                                    struct mt6360_led_platform_data *pdata)
+{
+       struct device_node *np = dev->of_node;
+       int i, ret;
+       const struct {
+               const char *name;
+               u32 *val_ptr;
+       } u32_opts[] = {
+               { "rgbon_sync", &pdata->rgbon_sync },
+               { "fled1_ultraistrb", &pdata->fled1_ultraistrb },
+               { "fled2_ultraistrb", &pdata->fled2_ultraistrb },
+       };

Whereas it may be thought of as an optimization, I'd prefer to stick
to the sequential one-by-one property parsing.

Introducing all of a sudden the iteration like below you introduce
unnecessary confusion.

Also, the function name in not telling too much. And again - we don't
seem to have bindings.

+       memcpy(pdata, &def_platform_data, sizeof(*pdata));
+       for (i = 0; i < ARRAY_SIZE(u32_opts); i++) {
+               ret = of_property_read_u32(np, u32_opts[i].name,
+                                          u32_opts[i].val_ptr);
+               if (ret)
+                       dev_err(dev, "error reading '%s'\n", u32_opts[i].name);
+       }
+}
+
+static int mt6360_led_apply_pdata(struct mt6360_led_data *mld,
+                                  struct mt6360_led_platform_data *pdata)

This function seems to affect only fleds, so its name should reflect
that.

+{
+       int i, ret;
+       const struct {
+               u32 *val_ptr;
+               u8 reg;
+               u8 mask;
+               u8 shift;
+       } sel_props[] = {
+               {
+                       &pdata->fled1_ultraistrb, MT6360_PMU_FLED1_STRB_CTRL2,
+                       MT6360_MASK_ULTRA_STROBE, MT6360_SHIFT_ULTRA_STROBE,
+               },
+               {
+                       &pdata->fled2_ultraistrb, MT6360_PMU_FLED2_STRB_CTRL2,
+                       MT6360_MASK_ULTRA_STROBE, MT6360_SHIFT_ULTRA_STROBE,
+               },
+       };
+
+       for (i = 0; i < ARRAY_SIZE(sel_props); i++) {
+               ret = regmap_update_bits(mld->regmap,
+                                        sel_props[i].reg,
+                                        sel_props[i].mask,
+                                        *sel_props[i].val_ptr <<
+                                               sel_props[i].shift);
+               if (ret < 0)
+                       return ret;
+       }
+       return 0;
+}
+
+static int mt6360_led_probe(struct platform_device *pdev)
+{
+       struct mt6360_led_platform_data *pdata = dev_get_platdata(&pdev->dev);
+       struct mt6360_led_data *mld;
+       struct mt6360_led_classdev *mtled_cdev;
+       struct mt6360_fled_classdev *mtfled_cdev;
+       struct v4l2_flash_config v4l2_config;
+       int i, ret;
+
+       mld = devm_kzalloc(&pdev->dev, sizeof(*mld), GFP_KERNEL);
+       if (!mld)
+               return -ENOMEM;
+
+       if (pdev->dev.of_node) {
+               pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+               if (!pdata)
+                       return -ENOMEM;
+               mt6360_led_parse_dt_data(&pdev->dev, pdata);
+       }
+
+       if (!pdata) {
+               dev_err(&pdev->dev, "no platform data specified\n");
+               return -EINVAL;
+       }
+
+       mld->dev = &pdev->dev;
+       mld->pdata = pdata;
+       platform_set_drvdata(pdev, mld);
+
+       mld->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+       if (!mld->regmap) {
+               dev_err(&pdev->dev, "Failed to get parent regmap\n");
+               return -ENODEV;
+       }
+
+       ret = mt6360_led_apply_pdata(mld, pdata);
+       if (ret < 0) {
+               dev_err(&pdev->dev, "apply pdata fail\n");
+               return ret;
+       }
+
+       memcpy(mld->mtled_cdev, def_led_classdev, sizeof(def_led_classdev));
+       ret = mt6360_iled_parse_dt(&pdev->dev, mld);
+       if (ret < 0) {
+               dev_err(&pdev->dev, "Failed to parse iled dt\n");
+               return ret;
+       }
+
+       for (i = 0; i < MT6360_LED_MAX; i++) {
+               mtled_cdev = mld->mtled_cdev + i;

You don't seem to check how many LEDs were defined in DT.

+               ret = devm_led_classdev_register(&pdev->dev,
+                                                &(mtled_cdev->cdev));

Please use devm*ext() API.

+               if (ret < 0) {
+                       dev_err(&pdev->dev, "Failed to register led[%d]\n", i);
+                       return ret;
+               }
+               mtled_cdev->cdev.dev->of_node = mtled_cdev->np;
+       }
+
+       memcpy(mld->mtfled_cdev, def_fled_classdev, sizeof(def_fled_classdev));
+       ret = mt6360_fled_parse_dt(&pdev->dev, mld);
+       if (ret < 0) {
+               dev_err(&pdev->dev, "Fail to parse fled dt\n");
+               return ret;
+       }
+
+       for (i = 0; i < MT6360_FLED_MAX; ++) {
+               mtfled_cdev = mld->mtfled_cdev + i;

You don't seem to check how many LEDs were defined in DT.

+               ret = led_classdev_flash_register(&pdev->dev,
+                                                 &mtfled_cdev->fl_cdev);

Please use devm*ext() API.

+               if (ret < 0) {
+                       dev_err(&pdev->dev, "Failed to register fled[%d]\n", i);
+                       goto out_fled_cdev;
+               }
+       }
+
+       for (i = 0; i < MT6360_FLED_MAX; i++) {
+               mtfled_cdev = mld->mtfled_cdev + i;
+               memset(&v4l2_config, 0, sizeof(v4l2_config));
+               mt6360_init_v4l2_flash_config(mtfled_cdev, &v4l2_config);
+               mtfled_cdev->v4l2_flash = v4l2_flash_init(&pdev->dev,
+                                             of_fwnode_handle(mtfled_cdev->np),
+                                             &mtfled_cdev->fl_cdev,
+                                             &v4l2_flash_ops, &v4l2_config);
+               if (IS_ERR(mtfled_cdev->v4l2_flash)) {
+                       dev_err(&pdev->dev, "Failed to register v4l2_sd\n");
+                       ret = PTR_ERR(mtfled_cdev->v4l2_flash);
+                       goto out_v4l2_sd;
+               }
+       }
+
+       ret = mt6360_fled_irq_register(pdev);
+       if (ret < 0) {
+               dev_err(&pdev->dev, "Failed to register irqs\n");
+               goto out_v4l2_sd;
+       }
+
+       return 0;
+out_v4l2_sd:
+       while (--i >= 0) {
+               mtfled_cdev = mld->mtfled_cdev + i;
+               v4l2_flash_release(mtfled_cdev->v4l2_flash);
+       }
+       i = MT6360_FLED_MAX;
+out_fled_cdev:
+       while (--i >= 0) {
+               mtfled_cdev = mld->mtfled_cdev + i;
+               led_classdev_flash_unregister(&mtfled_cdev->fl_cdev);
+       }
+       return ret;
+}
+
+static int mt6360_led_remove(struct platform_device *pdev)
+{
+       struct mt6360_led_data *mld = platform_get_drvdata(pdev);
+       struct mt6360_fled_classdev *mtfled_cdev;
+       int i;
+
+       for (i = 0; i < MT6360_FLED_MAX; i++) {
+               mtfled_cdev = mld->mtfled_cdev + i;
+               v4l2_flash_release(mtfled_cdev->v4l2_flash);
+               led_classdev_flash_unregister(&mtfled_cdev->fl_cdev);
+       }
+
+       return 0;
+}
+
+static const struct of_device_id __maybe_unused mt6360_led_of_id[] = {
+       { .compatible = "mediatek,mt6360_led", },
+       {},
+};
+MODULE_DEVICE_TABLE(of, mt6360_led_of_id);
+
+static struct platform_driver mt6360_led_driver = {
+       .driver = {
+               .name = "mt6360_led",
+               .owner = THIS_MODULE,
+               .of_match_table = of_match_ptr(mt6360_led_of_id),
+       },
+       .probe = mt6360_led_probe,
+       .remove = mt6360_led_remove,
+};
+module_platform_driver(mt6360_led_driver);
+
+MODULE_AUTHOR("Gene Chen <gene_c...@richtek.com>");
+MODULE_DESCRIPTION("MT6360 Led Driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mfd/mt6360.h b/include/linux/mfd/mt6360.h
index ea13040..2b81ab5 100644
--- a/include/linux/mfd/mt6360.h
+++ b/include/linux/mfd/mt6360.h
@@ -29,6 +29,11 @@ struct mt6360_pmu_data {
        unsigned int chip_rev;
  };
+struct mt6360_pmu_irq_desc {
+       const char *name;
+       irq_handler_t irq_handler;
+};
+
  /* PMU register defininition */
  #define MT6360_PMU_DEV_INFO                   (0x00)
  #define MT6360_PMU_CORE_CTRL1                 (0x01)
@@ -236,5 +241,4 @@ struct mt6360_pmu_data {
  #define CHIP_VEN_MASK                         (0xF0)
  #define CHIP_VEN_MT6360                               (0x50)
  #define CHIP_REV_MASK                         (0x0F)
-
  #endif /* __MT6360_H__ */


--
Best regards,
Jacek Anaszewski

Reply via email to