[PATCH v7 0/5] leds: mt6360: Add LED driver for MT6360

[PATCH v7 0/5] leds: mt6360: Add LED driver for MT6360

[Gene Chen]

In-Reply-To: 

Gene Chen (5)
 leds: flash: Add flash registration with undefined CONFIG_LEDS_CLASS_FLASH
 dt-bindings: leds: Add LED_COLOR_ID_MOONLIGHT definitions
 dt-bindings: leds: common: Increase LED_COLOR_ID_* maximum size
 dt-bindings: leds: Add bindings for MT6360 LED
 leds: mt6360: Add LED driver for MT6360

 Documentation/devicetree/bindings/leds/common.yaml      |    2 
 Documentation/devicetree/bindings/leds/leds-mt6360.yaml |  164 +++
 drivers/leds/Kconfig                                    |   13 
 drivers/leds/Makefile                                   |    1 
 drivers/leds/leds-mt6360.c                              |  808 ++++++++++++++++
 include/dt-bindings/leds/common.h                       |    3 
 include/linux/led-class-flash.h                         |   36 
 7 files changed, 1025 insertions(+), 2 deletions(-)

changelogs between v1 & v2
 - add led driver with mfd

changelogs between v2 & v3
 - independent add led driver
 - add dt-binding document
 - refactor macros definition for easy to debug
 - parse device tree by fwnode
 - use devm*ext to register led class device

changelogs between v3 & v4
 - fix binding document description
 - use GENMASK and add unit postfix to definition
 - isink register led class device

changelogs between v4 & v5
 - change rgb isink to multicolor control
 - add binding reference to mfd yaml

changelogs between v5 & v6
 - Use DT to decide RGB LED is multicolor device or indicator device only

changelogs between v6 & v7
 - Add binding multicolor device sample code
 - Add flash ops mutex lock
 - Remove V4L2 init with indicator device

[PATCH v7 1/5] leds: flash: Add flash registration with undefined CONFIG_LEDS_CLASS_FLASH

[Gene Chen]

From: Gene Chen <gene_chen@richtek.com>

Add flash registration with undefined CONFIG_LEDS_CLASS_FLASH

Signed-off-by: Gene Chen <gene_chen@richtek.com>
---
 include/linux/led-class-flash.h | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/include/linux/led-class-flash.h b/include/linux/led-class-flash.h
index 21a3358..4f56c28 100644
--- a/include/linux/led-class-flash.h
+++ b/include/linux/led-class-flash.h
@@ -85,6 +85,7 @@ static inline struct led_classdev_flash *lcdev_to_flcdev(
 	return container_of(lcdev, struct led_classdev_flash, led_cdev);
 }
 
+#if IS_ENABLED(CONFIG_LEDS_CLASS_FLASH)
 /**
  * led_classdev_flash_register_ext - register a new object of LED class with
  *				     init data and with support for flash LEDs
@@ -127,6 +128,41 @@ static inline int devm_led_classdev_flash_register(struct device *parent,
 void devm_led_classdev_flash_unregister(struct device *parent,
 					struct led_classdev_flash *fled_cdev);
 
+#else
+
+static inline int led_classdev_flash_register_ext(struct device *parent,
+				    struct led_classdev_flash *fled_cdev,
+				    struct led_init_data *init_data)
+{
+	return -EINVAL;
+}
+
+static inline int led_classdev_flash_register(struct device *parent,
+					   struct led_classdev_flash *fled_cdev)
+{
+	return led_classdev_flash_register_ext(parent, fled_cdev, NULL);
+}
+
+static inline void led_classdev_flash_unregister(struct led_classdev_flash *fled_cdev) {};
+static inline int devm_led_classdev_flash_register_ext(struct device *parent,
+				     struct led_classdev_flash *fled_cdev,
+				     struct led_init_data *init_data)
+{
+	return -EINVAL;
+}
+
+static inline int devm_led_classdev_flash_register(struct device *parent,
+				     struct led_classdev_flash *fled_cdev)
+{
+	return devm_led_classdev_flash_register_ext(parent, fled_cdev, NULL);
+}
+
+void devm_led_classdev_flash_unregister(struct device *parent,
+					struct led_classdev_flash *fled_cdev)
+{};
+
+#endif  /* IS_ENABLED(CONFIG_LEDS_CLASS_FLASH) */
+
 /**
  * led_set_flash_strobe - setup flash strobe
  * @fled_cdev: the flash LED to set strobe on
-- 
2.7.4

Re: [PATCH v7 1/5] leds: flash: Add flash registration with undefined CONFIG_LEDS_CLASS_FLASH

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2141 bytes --]

Hi Gene,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on pavel-linux-leds/for-next]
[also build test WARNING on robh/for-next linus/master v5.10-rc4 next-20201118]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Gene-Chen/leds-mt6360-Add-LED-driver-for-MT6360/20201118-185208
base:   git://git.kernel.org/pub/scm/linux/kernel/git/pavel/linux-leds.git for-next
config: nds32-randconfig-r001-20201118 (attached as .config)
compiler: nds32le-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/a088d9624f18b888ede6b0344163b8464db82f3b
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Gene-Chen/leds-mt6360-Add-LED-driver-for-MT6360/20201118-185208
        git checkout a088d9624f18b888ede6b0344163b8464db82f3b
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=nds32 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from drivers/staging/greybus/light.c:11:
>> include/linux/led-class-flash.h:160:6: warning: no previous prototype for 'devm_led_classdev_flash_unregister' [-Wmissing-prototypes]
     160 | void devm_led_classdev_flash_unregister(struct device *parent,
         |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

vim +/devm_led_classdev_flash_unregister +160 include/linux/led-class-flash.h

   159	
 > 160	void devm_led_classdev_flash_unregister(struct device *parent,
   161						struct led_classdev_flash *fled_cdev)
   162	{};
   163	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25091 bytes --]

Re: [PATCH v7 1/5] leds: flash: Add flash registration with undefined CONFIG_LEDS_CLASS_FLASH

[Jacek Anaszewski]

Hi Gene,

On 11/18/20 11:47 AM, Gene Chen wrote:
> From: Gene Chen <gene_chen@richtek.com>
> 
> Add flash registration with undefined CONFIG_LEDS_CLASS_FLASH
> 
> Signed-off-by: Gene Chen <gene_chen@richtek.com>
> ---
>   include/linux/led-class-flash.h | 36 ++++++++++++++++++++++++++++++++++++
>   1 file changed, 36 insertions(+)
> 
> diff --git a/include/linux/led-class-flash.h b/include/linux/led-class-flash.h
> index 21a3358..4f56c28 100644
> --- a/include/linux/led-class-flash.h
> +++ b/include/linux/led-class-flash.h
> @@ -85,6 +85,7 @@ static inline struct led_classdev_flash *lcdev_to_flcdev(
>   	return container_of(lcdev, struct led_classdev_flash, led_cdev);
>   }
>   
> +#if IS_ENABLED(CONFIG_LEDS_CLASS_FLASH)
>   /**
>    * led_classdev_flash_register_ext - register a new object of LED class with
>    *				     init data and with support for flash LEDs
> @@ -127,6 +128,41 @@ static inline int devm_led_classdev_flash_register(struct device *parent,
>   void devm_led_classdev_flash_unregister(struct device *parent,
>   					struct led_classdev_flash *fled_cdev);
>   
> +#else
> +
> +static inline int led_classdev_flash_register_ext(struct device *parent,
> +				    struct led_classdev_flash *fled_cdev,
> +				    struct led_init_data *init_data)
> +{
> +	return -EINVAL;

s/-EINVAL/0/

The goal here is to assure that client will not fail when using no-op.

> +}
> +
> +static inline int led_classdev_flash_register(struct device *parent,
> +					   struct led_classdev_flash *fled_cdev)
> +{
> +	return led_classdev_flash_register_ext(parent, fled_cdev, NULL);
> +}

This function should be placed after #ifdef block because its
shape is the same for both cases.

> +static inline void led_classdev_flash_unregister(struct led_classdev_flash *fled_cdev) {};
> +static inline int devm_led_classdev_flash_register_ext(struct device *parent,
> +				     struct led_classdev_flash *fled_cdev,
> +				     struct led_init_data *init_data)
> +{
> +	return -EINVAL;

/-EINVAL/0/

Please do the same fix in all no-ops in the led-class-multicolor.h,
as we've discussed.

> +}
> +
> +static inline int devm_led_classdev_flash_register(struct device *parent,
> +				     struct led_classdev_flash *fled_cdev)
> +{
> +	return devm_led_classdev_flash_register_ext(parent, fled_cdev, NULL);
> +}


This function should also be placed after #ifdef block.
Please make the same optimizations in the led-class-multicolor.h as you
are at it.

> +
> +void devm_led_classdev_flash_unregister(struct device *parent,

s/void/static inline void/

That's the reason why you got warning from buildbot.

> +					struct led_classdev_flash *fled_cdev)
> +{};
> +
> +#endif  /* IS_ENABLED(CONFIG_LEDS_CLASS_FLASH) */
> +
>   /**
>    * led_set_flash_strobe - setup flash strobe
>    * @fled_cdev: the flash LED to set strobe on
> 

-- 
Best regards,
Jacek Anaszewski

Re: [PATCH v7 1/5] leds: flash: Add flash registration with undefined CONFIG_LEDS_CLASS_FLASH

[Gene Chen]

Jacek Anaszewski <jacek.anaszewski@gmail.com> 於 2020年11月20日 週五 上午6:29寫道：
>
> Hi Gene,
>
> On 11/18/20 11:47 AM, Gene Chen wrote:
> > From: Gene Chen <gene_chen@richtek.com>
> >
> > Add flash registration with undefined CONFIG_LEDS_CLASS_FLASH
> >
> > Signed-off-by: Gene Chen <gene_chen@richtek.com>
> > ---
> >   include/linux/led-class-flash.h | 36 ++++++++++++++++++++++++++++++++++++
> >   1 file changed, 36 insertions(+)
> >
> > diff --git a/include/linux/led-class-flash.h b/include/linux/led-class-flash.h
> > index 21a3358..4f56c28 100644
> > --- a/include/linux/led-class-flash.h
> > +++ b/include/linux/led-class-flash.h
> > @@ -85,6 +85,7 @@ static inline struct led_classdev_flash *lcdev_to_flcdev(
> >       return container_of(lcdev, struct led_classdev_flash, led_cdev);
> >   }
> >
> > +#if IS_ENABLED(CONFIG_LEDS_CLASS_FLASH)
> >   /**
> >    * led_classdev_flash_register_ext - register a new object of LED class with
> >    *                               init data and with support for flash LEDs
> > @@ -127,6 +128,41 @@ static inline int devm_led_classdev_flash_register(struct device *parent,
> >   void devm_led_classdev_flash_unregister(struct device *parent,
> >                                       struct led_classdev_flash *fled_cdev);
> >
> > +#else
> > +
> > +static inline int led_classdev_flash_register_ext(struct device *parent,
> > +                                 struct led_classdev_flash *fled_cdev,
> > +                                 struct led_init_data *init_data)
> > +{
> > +     return -EINVAL;
>
> s/-EINVAL/0/
>
> The goal here is to assure that client will not fail when using no-op.
>
> > +}
> > +
> > +static inline int led_classdev_flash_register(struct device *parent,
> > +                                        struct led_classdev_flash *fled_cdev)
> > +{
> > +     return led_classdev_flash_register_ext(parent, fled_cdev, NULL);
> > +}
>
> This function should be placed after #ifdef block because its
> shape is the same for both cases.
>
> > +static inline void led_classdev_flash_unregister(struct led_classdev_flash *fled_cdev) {};
> > +static inline int devm_led_classdev_flash_register_ext(struct device *parent,
> > +                                  struct led_classdev_flash *fled_cdev,
> > +                                  struct led_init_data *init_data)
> > +{
> > +     return -EINVAL;
>
> /-EINVAL/0/
>
> Please do the same fix in all no-ops in the led-class-multicolor.h,
> as we've discussed.
>

I think return -EINVAL is correct, because I should register flash
light device if I define FLED in DTS node.

> > +}
> > +
> > +static inline int devm_led_classdev_flash_register(struct device *parent,
> > +                                  struct led_classdev_flash *fled_cdev)
> > +{
> > +     return devm_led_classdev_flash_register_ext(parent, fled_cdev, NULL);
> > +}
>
>
> This function should also be placed after #ifdef block.
> Please make the same optimizations in the led-class-multicolor.h as you
> are at it.
>
> > +
> > +void devm_led_classdev_flash_unregister(struct device *parent,
>
> s/void/static inline void/
>
> That's the reason why you got warning from buildbot.
>

ACK

> > +                                     struct led_classdev_flash *fled_cdev)
> > +{};
> > +
> > +#endif  /* IS_ENABLED(CONFIG_LEDS_CLASS_FLASH) */
> > +
> >   /**
> >    * led_set_flash_strobe - setup flash strobe
> >    * @fled_cdev: the flash LED to set strobe on
> >
>
> --
> Best regards,
> Jacek Anaszewski

Re: [PATCH v7 1/5] leds: flash: Add flash registration with undefined CONFIG_LEDS_CLASS_FLASH

[Jacek Anaszewski]

On 11/23/20 4:20 AM, Gene Chen wrote:
> Jacek Anaszewski <jacek.anaszewski@gmail.com> 於 2020年11月20日 週五 上午6:29寫道：
>>
>> Hi Gene,
>>
>> On 11/18/20 11:47 AM, Gene Chen wrote:
>>> From: Gene Chen <gene_chen@richtek.com>
>>>
>>> Add flash registration with undefined CONFIG_LEDS_CLASS_FLASH
>>>
>>> Signed-off-by: Gene Chen <gene_chen@richtek.com>
>>> ---
>>>    include/linux/led-class-flash.h | 36 ++++++++++++++++++++++++++++++++++++
>>>    1 file changed, 36 insertions(+)
>>>
>>> diff --git a/include/linux/led-class-flash.h b/include/linux/led-class-flash.h
>>> index 21a3358..4f56c28 100644
>>> --- a/include/linux/led-class-flash.h
>>> +++ b/include/linux/led-class-flash.h
>>> @@ -85,6 +85,7 @@ static inline struct led_classdev_flash *lcdev_to_flcdev(
>>>        return container_of(lcdev, struct led_classdev_flash, led_cdev);
>>>    }
>>>
>>> +#if IS_ENABLED(CONFIG_LEDS_CLASS_FLASH)
>>>    /**
>>>     * led_classdev_flash_register_ext - register a new object of LED class with
>>>     *                               init data and with support for flash LEDs
>>> @@ -127,6 +128,41 @@ static inline int devm_led_classdev_flash_register(struct device *parent,
>>>    void devm_led_classdev_flash_unregister(struct device *parent,
>>>                                        struct led_classdev_flash *fled_cdev);
>>>
>>> +#else
>>> +
>>> +static inline int led_classdev_flash_register_ext(struct device *parent,
>>> +                                 struct led_classdev_flash *fled_cdev,
>>> +                                 struct led_init_data *init_data)
>>> +{
>>> +     return -EINVAL;
>>
>> s/-EINVAL/0/
>>
>> The goal here is to assure that client will not fail when using no-op.
>>
>>> +}
>>> +
>>> +static inline int led_classdev_flash_register(struct device *parent,
>>> +                                        struct led_classdev_flash *fled_cdev)
>>> +{
>>> +     return led_classdev_flash_register_ext(parent, fled_cdev, NULL);
>>> +}
>>
>> This function should be placed after #ifdef block because its
>> shape is the same for both cases.
>>
>>> +static inline void led_classdev_flash_unregister(struct led_classdev_flash *fled_cdev) {};
>>> +static inline int devm_led_classdev_flash_register_ext(struct device *parent,
>>> +                                  struct led_classdev_flash *fled_cdev,
>>> +                                  struct led_init_data *init_data)
>>> +{
>>> +     return -EINVAL;
>>
>> /-EINVAL/0/
>>
>> Please do the same fix in all no-ops in the led-class-multicolor.h,
>> as we've discussed.
>>
> 
> I think return -EINVAL is correct, because I should register flash
> light device if I define FLED in DTS node.

I don't quite follow your logic here.

No-op function's purpose is to simplify the code on the caller's side.
Therefore it should report success.

Please return 0 from it.

-- 
Best regards,
Jacek Anaszewski

Re: [PATCH v7 1/5] leds: flash: Add flash registration with undefined CONFIG_LEDS_CLASS_FLASH

[Gene Chen]

Jacek Anaszewski <jacek.anaszewski@gmail.com> 於 2020年11月24日 週二 上午5:07寫道：
>
> On 11/23/20 4:20 AM, Gene Chen wrote:
> > Jacek Anaszewski <jacek.anaszewski@gmail.com> 於 2020年11月20日 週五 上午6:29寫道：
> >>
> >> Hi Gene,
> >>
> >> On 11/18/20 11:47 AM, Gene Chen wrote:
> >>> From: Gene Chen <gene_chen@richtek.com>
> >>>
> >>> Add flash registration with undefined CONFIG_LEDS_CLASS_FLASH
> >>>
> >>> Signed-off-by: Gene Chen <gene_chen@richtek.com>
> >>> ---
> >>>    include/linux/led-class-flash.h | 36 ++++++++++++++++++++++++++++++++++++
> >>>    1 file changed, 36 insertions(+)
> >>>
> >>> diff --git a/include/linux/led-class-flash.h b/include/linux/led-class-flash.h
> >>> index 21a3358..4f56c28 100644
> >>> --- a/include/linux/led-class-flash.h
> >>> +++ b/include/linux/led-class-flash.h
> >>> @@ -85,6 +85,7 @@ static inline struct led_classdev_flash *lcdev_to_flcdev(
> >>>        return container_of(lcdev, struct led_classdev_flash, led_cdev);
> >>>    }
> >>>
> >>> +#if IS_ENABLED(CONFIG_LEDS_CLASS_FLASH)
> >>>    /**
> >>>     * led_classdev_flash_register_ext - register a new object of LED class with
> >>>     *                               init data and with support for flash LEDs
> >>> @@ -127,6 +128,41 @@ static inline int devm_led_classdev_flash_register(struct device *parent,
> >>>    void devm_led_classdev_flash_unregister(struct device *parent,
> >>>                                        struct led_classdev_flash *fled_cdev);
> >>>
> >>> +#else
> >>> +
> >>> +static inline int led_classdev_flash_register_ext(struct device *parent,
> >>> +                                 struct led_classdev_flash *fled_cdev,
> >>> +                                 struct led_init_data *init_data)
> >>> +{
> >>> +     return -EINVAL;
> >>
> >> s/-EINVAL/0/
> >>
> >> The goal here is to assure that client will not fail when using no-op.
> >>
> >>> +}
> >>> +
> >>> +static inline int led_classdev_flash_register(struct device *parent,
> >>> +                                        struct led_classdev_flash *fled_cdev)
> >>> +{
> >>> +     return led_classdev_flash_register_ext(parent, fled_cdev, NULL);
> >>> +}
> >>
> >> This function should be placed after #ifdef block because its
> >> shape is the same for both cases.
> >>
> >>> +static inline void led_classdev_flash_unregister(struct led_classdev_flash *fled_cdev) {};
> >>> +static inline int devm_led_classdev_flash_register_ext(struct device *parent,
> >>> +                                  struct led_classdev_flash *fled_cdev,
> >>> +                                  struct led_init_data *init_data)
> >>> +{
> >>> +     return -EINVAL;
> >>
> >> /-EINVAL/0/
> >>
> >> Please do the same fix in all no-ops in the led-class-multicolor.h,
> >> as we've discussed.
> >>
> >
> > I think return -EINVAL is correct, because I should register flash
> > light device if I define FLED in DTS node.
>
> I don't quite follow your logic here.
>
> No-op function's purpose is to simplify the code on the caller's side.
> Therefore it should report success.
>
> Please return 0 from it.
>

Just like those functions in led-class-multicolor.h, caller may use
return value to check whether FLED is registered successfully or not.
For this case, is returning 0 a little bit misleading?

> --
> Best regards,
> Jacek Anaszewski

Re: [PATCH v7 1/5] leds: flash: Add flash registration with undefined CONFIG_LEDS_CLASS_FLASH

[Jacek Anaszewski]

On 11/24/20 7:08 AM, Gene Chen wrote:
[...]
>>>> This function should be placed after #ifdef block because its
>>>> shape is the same for both cases.
>>>>
>>>>> +static inline void led_classdev_flash_unregister(struct led_classdev_flash *fled_cdev) {};
>>>>> +static inline int devm_led_classdev_flash_register_ext(struct device *parent,
>>>>> +                                  struct led_classdev_flash *fled_cdev,
>>>>> +                                  struct led_init_data *init_data)
>>>>> +{
>>>>> +     return -EINVAL;
>>>>
>>>> /-EINVAL/0/
>>>>
>>>> Please do the same fix in all no-ops in the led-class-multicolor.h,
>>>> as we've discussed.
>>>>
>>>
>>> I think return -EINVAL is correct, because I should register flash
>>> light device if I define FLED in DTS node.

OK, I think I'm getting your concerns now. So you're only partially
correct - the driver should register flash LED device if there is
corresponding node in DT, but only if CONFIG_LEDS_CLASS_FLASH is 
enabled. In case it is disabled the no-op will come into play
and return 0, allowing the probe() to proceed as if the registration
succeeded.

 From the driver point of view nothing changes, except that flash LED
ops will not be called afterwards. This is common pattern. If in doubt
skim through the headers in include/linux.

>>
>> I don't quite follow your logic here.
>>
>> No-op function's purpose is to simplify the code on the caller's side.
>> Therefore it should report success.
>>
>> Please return 0 from it.
>>
> 
> Just like those functions in led-class-multicolor.h, caller may use
> return value to check whether FLED is registered successfully or not.
> For this case, is returning 0 a little bit misleading?

Please note that I've already admitted that led-class-multicolor.h
class is buggy and should also be fixed to return 0 from its no-ops.
Please apply the "s/-EINVAL/0/" fixes to it as well - your driver will
need that.

-- 
Best regards,
Jacek Anaszewski

[PATCH v7 2/5] dt-bindings: leds: Add LED_COLOR_ID_MOONLIGHT definitions

[Gene Chen]

From: Gene Chen <gene_chen@richtek.com>

Add LED_COLOR_ID_MOONLIGHT definitions

Signed-off-by: Gene Chen <gene_chen@richtek.com>
---
 include/dt-bindings/leds/common.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h
index 52b619d..1409e9a 100644
--- a/include/dt-bindings/leds/common.h
+++ b/include/dt-bindings/leds/common.h
@@ -33,7 +33,8 @@
 #define LED_COLOR_ID_MULTI	8	/* For multicolor LEDs */
 #define LED_COLOR_ID_RGB	9	/* For multicolor LEDs that can do arbitrary color,
 					   so this would include RGBW and similar */
-#define LED_COLOR_ID_MAX	10
+#define LED_COLOR_ID_MOONLIGHT	10
+#define LED_COLOR_ID_MAX	11
 
 /* Standard LED functions */
 /* Keyboard LEDs, usually it would be input4::capslock etc. */
-- 
2.7.4

Re: [PATCH v7 2/5] dt-bindings: leds: Add LED_COLOR_ID_MOONLIGHT definitions

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1080 bytes --]

Hi!

> From: Gene Chen <gene_chen@richtek.com>
> 
> Add LED_COLOR_ID_MOONLIGHT definitions

Why is moonlight a color? Camera flashes are usually white, no?

At least it needs a comment...

Best regards,
								Pavel
								


> Signed-off-by: Gene Chen <gene_chen@richtek.com>
> ---
>  include/dt-bindings/leds/common.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h
> index 52b619d..1409e9a 100644
> --- a/include/dt-bindings/leds/common.h
> +++ b/include/dt-bindings/leds/common.h
> @@ -33,7 +33,8 @@
>  #define LED_COLOR_ID_MULTI	8	/* For multicolor LEDs */
>  #define LED_COLOR_ID_RGB	9	/* For multicolor LEDs that can do arbitrary color,
>  					   so this would include RGBW and similar */
> -#define LED_COLOR_ID_MAX	10
> +#define LED_COLOR_ID_MOONLIGHT	10
> +#define LED_COLOR_ID_MAX	11
>  
>  /* Standard LED functions */
>  /* Keyboard LEDs, usually it would be input4::capslock etc. */

-- 
http://www.livejournal.com/~pavelmachek

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH v7 2/5] dt-bindings: leds: Add LED_COLOR_ID_MOONLIGHT definitions

[Gene Chen]

Pavel Machek <pavel@ucw.cz> 於 2020年11月19日 週四 上午5:37寫道：
>
> Hi!
>
> > From: Gene Chen <gene_chen@richtek.com>
> >
> > Add LED_COLOR_ID_MOONLIGHT definitions
>
> Why is moonlight a color? Camera flashes are usually white, no?
>
> At least it needs a comment...
>
> Best regards,
>                                                                 Pavel
>

Moonlight has more current level(150mA) from general RGB LED (24mA).
It can be used as night-light and usually use color AMBER.
Camera flashes are usually use two flash LED. One is YELLOW, and one is WHITE.

>
>
> > Signed-off-by: Gene Chen <gene_chen@richtek.com>
> > ---
> >  include/dt-bindings/leds/common.h | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h
> > index 52b619d..1409e9a 100644
> > --- a/include/dt-bindings/leds/common.h
> > +++ b/include/dt-bindings/leds/common.h
> > @@ -33,7 +33,8 @@
> >  #define LED_COLOR_ID_MULTI   8       /* For multicolor LEDs */
> >  #define LED_COLOR_ID_RGB     9       /* For multicolor LEDs that can do arbitrary color,
> >                                          so this would include RGBW and similar */
> > -#define LED_COLOR_ID_MAX     10
> > +#define LED_COLOR_ID_MOONLIGHT       10
> > +#define LED_COLOR_ID_MAX     11
> >
> >  /* Standard LED functions */
> >  /* Keyboard LEDs, usually it would be input4::capslock etc. */
>
> --
> http://www.livejournal.com/~pavelmachek

Re: [PATCH v7 2/5] dt-bindings: leds: Add LED_COLOR_ID_MOONLIGHT definitions

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 814 bytes --]

Hi!

> > > From: Gene Chen <gene_chen@richtek.com>
> > >
> > > Add LED_COLOR_ID_MOONLIGHT definitions
> >
> > Why is moonlight a color? Camera flashes are usually white, no?
> >
> > At least it needs a comment...
> >
> > Best regards,
> >                                                                 Pavel
> >
> 
> Moonlight has more current level(150mA) from general RGB LED (24mA).
> It can be used as night-light and usually use color AMBER.
> Camera flashes are usually use two flash LED. One is YELLOW, and one
>is WHITE.

From what I seen, night-lights are usually differetent "temperatures"
of white. Cool white + warm white.

I believe "warm white" would be easier to understand than
"moonlight"...

Best regards,
								Pavel

-- 
http://www.livejournal.com/~pavelmachek

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH v7 2/5] dt-bindings: leds: Add LED_COLOR_ID_MOONLIGHT definitions

[Gene Chen]

Pavel Machek <pavel@ucw.cz> 於 2020年11月19日 週四 下午3:44寫道：
>
> Hi!
>
> > > > From: Gene Chen <gene_chen@richtek.com>
> > > >
> > > > Add LED_COLOR_ID_MOONLIGHT definitions
> > >
> > > Why is moonlight a color? Camera flashes are usually white, no?
> > >
> > > At least it needs a comment...
> > >
> > > Best regards,
> > >                                                                 Pavel
> > >
> >
> > Moonlight has more current level(150mA) from general RGB LED (24mA).
> > It can be used as night-light and usually use color AMBER.
> > Camera flashes are usually use two flash LED. One is YELLOW, and one
> >is WHITE.
>
> From what I seen, night-lights are usually differetent "temperatures"
> of white. Cool white + warm white.
>
> I believe "warm white" would be easier to understand than
> "moonlight"...
>

ACK, I will change color "LED_COLOR_ID_WHITE"

> Best regards,
>                                                                 Pavel
>
> --
> http://www.livejournal.com/~pavelmachek

Re: [PATCH v7 2/5] dt-bindings: leds: Add LED_COLOR_ID_MOONLIGHT definitions

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 779 bytes --]

Hi!

> > > Moonlight has more current level(150mA) from general RGB LED (24mA).
> > > It can be used as night-light and usually use color AMBER.
> > > Camera flashes are usually use two flash LED. One is YELLOW, and one
> > >is WHITE.
> >
> > From what I seen, night-lights are usually differetent "temperatures"
> > of white. Cool white + warm white.
> >
> > I believe "warm white" would be easier to understand than
> > "moonlight"...
> 
> ACK, I will change color "LED_COLOR_ID_WHITE"

If you have two leds that differ only in one being "cool white" and
one being "warm white", you may need to introduce defines for one of
them.

If not, keeping "LED_COLOR_ID_WHITE" is enough.

Best regards,
								Pavel
-- 
http://www.livejournal.com/~pavelmachek

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH v7 2/5] dt-bindings: leds: Add LED_COLOR_ID_MOONLIGHT definitions

[Jacek Anaszewski]

Hi Pavel, Gene,

On 11/18/20 10:37 PM, Pavel Machek wrote:
> Hi!
> 
>> From: Gene Chen <gene_chen@richtek.com>
>>
>> Add LED_COLOR_ID_MOONLIGHT definitions
> 
> Why is moonlight a color? Camera flashes are usually white, no?
> 
> At least it needs a comment...

That's my fault, In fact I should have asked about adding
LED_FUNCTION_MOONLIGHT, it was evidently too late for me that evening...

-- 
Best regards,
Jacek Anaszewski

Re: [PATCH v7 2/5] dt-bindings: leds: Add LED_COLOR_ID_MOONLIGHT definitions

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 793 bytes --]

On Thu 2020-11-19 22:03:14, Jacek Anaszewski wrote:
> Hi Pavel, Gene,
> 
> On 11/18/20 10:37 PM, Pavel Machek wrote:
> >Hi!
> >
> >>From: Gene Chen <gene_chen@richtek.com>
> >>
> >>Add LED_COLOR_ID_MOONLIGHT definitions
> >
> >Why is moonlight a color? Camera flashes are usually white, no?
> >
> >At least it needs a comment...
> 
> That's my fault, In fact I should have asked about adding
> LED_FUNCTION_MOONLIGHT, it was evidently too late for me that evening...

Aha, that makes more sense.

But please let's call it "torch" if we do that, as that is already
used in kernel sources... and probably in the interface, too:

./arch/arm/mach-pxa/ezx.c:    	       	  .name = "a910::torch",

Best regards,
							Pavel

-- 
http://www.livejournal.com/~pavelmachek

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH v7 2/5] dt-bindings: leds: Add LED_COLOR_ID_MOONLIGHT definitions

[Jacek Anaszewski]

On 11/19/20 10:57 PM, Pavel Machek wrote:
> On Thu 2020-11-19 22:03:14, Jacek Anaszewski wrote:
>> Hi Pavel, Gene,
>>
>> On 11/18/20 10:37 PM, Pavel Machek wrote:
>>> Hi!
>>>
>>>> From: Gene Chen <gene_chen@richtek.com>
>>>>
>>>> Add LED_COLOR_ID_MOONLIGHT definitions
>>>
>>> Why is moonlight a color? Camera flashes are usually white, no?
>>>
>>> At least it needs a comment...
>>
>> That's my fault, In fact I should have asked about adding
>> LED_FUNCTION_MOONLIGHT, it was evidently too late for me that evening...
> 
> Aha, that makes more sense.
> 
> But please let's call it "torch" if we do that, as that is already
> used in kernel sources... and probably in the interface, too:

I'd say that torch is something different that moonlight,
but we would need more input from Gene to learn more about
the nature of light emitted by ML LED on his device.

Please note that torch is usually meant as the other mode of
flash LED (sometimes it is called "movie mode"), which is already
handled by brightness file of LED class flash device (i.e. its LED class
subset), and which also maps to v4l2-flash TORCH mode.

> ./arch/arm/mach-pxa/ezx.c:    	       	  .name = "a910::torch",
> 
> Best regards,
> 							Pavel
> 

-- 
Best regards,
Jacek Anaszewski

Re: [PATCH v7 2/5] dt-bindings: leds: Add LED_COLOR_ID_MOONLIGHT definitions

[Gene Chen]

Jacek Anaszewski <jacek.anaszewski@gmail.com> 於 2020年11月20日 週五 上午6:26寫道：
>
> On 11/19/20 10:57 PM, Pavel Machek wrote:
> > On Thu 2020-11-19 22:03:14, Jacek Anaszewski wrote:
> >> Hi Pavel, Gene,
> >>
> >> On 11/18/20 10:37 PM, Pavel Machek wrote:
> >>> Hi!
> >>>
> >>>> From: Gene Chen <gene_chen@richtek.com>
> >>>>
> >>>> Add LED_COLOR_ID_MOONLIGHT definitions
> >>>
> >>> Why is moonlight a color? Camera flashes are usually white, no?
> >>>
> >>> At least it needs a comment...
> >>
> >> That's my fault, In fact I should have asked about adding
> >> LED_FUNCTION_MOONLIGHT, it was evidently too late for me that evening...
> >
> > Aha, that makes more sense.
> >
> > But please let's call it "torch" if we do that, as that is already
> > used in kernel sources... and probably in the interface, too:
>
> I'd say that torch is something different that moonlight,
> but we would need more input from Gene to learn more about
> the nature of light emitted by ML LED on his device.
>
> Please note that torch is usually meant as the other mode of
> flash LED (sometimes it is called "movie mode"), which is already
> handled by brightness file of LED class flash device (i.e. its LED class
> subset), and which also maps to v4l2-flash TORCH mode.
>

It's used to front camera fill light.
More brightness than screen backlight, and more soft light than flash.
I think LED_ID_COLOR_WHITE is okay.

> > ./arch/arm/mach-pxa/ezx.c:                      .name = "a910::torch",
> >
> > Best regards,
> >                                                       Pavel
> >
>
> --
> Best regards,
> Jacek Anaszewski

Re: [PATCH v7 2/5] dt-bindings: leds: Add LED_COLOR_ID_MOONLIGHT definitions

[Jacek Anaszewski]

On 11/23/20 4:00 AM, Gene Chen wrote:
> Jacek Anaszewski <jacek.anaszewski@gmail.com> 於 2020年11月20日 週五 上午6:26寫道：
>>
>> On 11/19/20 10:57 PM, Pavel Machek wrote:
>>> On Thu 2020-11-19 22:03:14, Jacek Anaszewski wrote:
>>>> Hi Pavel, Gene,
>>>>
>>>> On 11/18/20 10:37 PM, Pavel Machek wrote:
>>>>> Hi!
>>>>>
>>>>>> From: Gene Chen <gene_chen@richtek.com>
>>>>>>
>>>>>> Add LED_COLOR_ID_MOONLIGHT definitions
>>>>>
>>>>> Why is moonlight a color? Camera flashes are usually white, no?
>>>>>
>>>>> At least it needs a comment...
>>>>
>>>> That's my fault, In fact I should have asked about adding
>>>> LED_FUNCTION_MOONLIGHT, it was evidently too late for me that evening...
>>>
>>> Aha, that makes more sense.
>>>
>>> But please let's call it "torch" if we do that, as that is already
>>> used in kernel sources... and probably in the interface, too:
>>
>> I'd say that torch is something different that moonlight,
>> but we would need more input from Gene to learn more about
>> the nature of light emitted by ML LED on his device.
>>
>> Please note that torch is usually meant as the other mode of
>> flash LED (sometimes it is called "movie mode"), which is already
>> handled by brightness file of LED class flash device (i.e. its LED class
>> subset), and which also maps to v4l2-flash TORCH mode.
>>
> 
> It's used to front camera fill light.
> More brightness than screen backlight, and more soft light than flash.
> I think LED_ID_COLOR_WHITE is okay.

So why in v6 you assigned LED_COLOR_ID_AMBER to it?

Regardless of that, now we're talking about LED function - you chose
LED_FUNCTION_INDICATOR for it, but inferring from your above description
- it certainly doesn't fit here.

Also register names, containing part "ML" indicate that this LED's
intended function is moonlinght, which your description somehow
corroborates.

Moonlight LEDs become ubiquitous nowadays so sooner or later we will
need to add this function anyway [0].

[0] 
https://landscapelightingoakville.com/what-is-moon-lighting-and-why-does-it-remain-so-popular/

-- 
Best regards,
Jacek Anaszewski

Re: [PATCH v7 2/5] dt-bindings: leds: Add LED_COLOR_ID_MOONLIGHT definitions

[Gene Chen]

Jacek Anaszewski <jacek.anaszewski@gmail.com> 於 2020年11月24日 週二 上午4:52寫道：
>
> On 11/23/20 4:00 AM, Gene Chen wrote:
> > Jacek Anaszewski <jacek.anaszewski@gmail.com> 於 2020年11月20日 週五 上午6:26寫道：
> >>
> >> On 11/19/20 10:57 PM, Pavel Machek wrote:
> >>> On Thu 2020-11-19 22:03:14, Jacek Anaszewski wrote:
> >>>> Hi Pavel, Gene,
> >>>>
> >>>> On 11/18/20 10:37 PM, Pavel Machek wrote:
> >>>>> Hi!
> >>>>>
> >>>>>> From: Gene Chen <gene_chen@richtek.com>
> >>>>>>
> >>>>>> Add LED_COLOR_ID_MOONLIGHT definitions
> >>>>>
> >>>>> Why is moonlight a color? Camera flashes are usually white, no?
> >>>>>
> >>>>> At least it needs a comment...
> >>>>
> >>>> That's my fault, In fact I should have asked about adding
> >>>> LED_FUNCTION_MOONLIGHT, it was evidently too late for me that evening...
> >>>
> >>> Aha, that makes more sense.
> >>>
> >>> But please let's call it "torch" if we do that, as that is already
> >>> used in kernel sources... and probably in the interface, too:
> >>
> >> I'd say that torch is something different that moonlight,
> >> but we would need more input from Gene to learn more about
> >> the nature of light emitted by ML LED on his device.
> >>
> >> Please note that torch is usually meant as the other mode of
> >> flash LED (sometimes it is called "movie mode"), which is already
> >> handled by brightness file of LED class flash device (i.e. its LED class
> >> subset), and which also maps to v4l2-flash TORCH mode.
> >>
> >
> > It's used to front camera fill light.
> > More brightness than screen backlight, and more soft light than flash.
> > I think LED_ID_COLOR_WHITE is okay.
>
> So why in v6 you assigned LED_COLOR_ID_AMBER to it?
>
> Regardless of that, now we're talking about LED function - you chose
> LED_FUNCTION_INDICATOR for it, but inferring from your above description
> - it certainly doesn't fit here.
>
> Also register names, containing part "ML" indicate that this LED's
> intended function is moonlinght, which your description somehow
> corroborates.
>
> Moonlight LEDs become ubiquitous nowadays so sooner or later we will
> need to add this function anyway [0].
>
> [0]
> https://landscapelightingoakville.com/what-is-moon-lighting-and-why-does-it-remain-so-popular/
>

We use term "Moonlight" as reference says
"When you are trying to imitate moonlight you need to use low voltage,
softer lighting. You don’t want something that’s too bright"
which is focus on brightness instead of color.

So we surpose Moonlight can be white or amber.

Should I add LED_FUNCTION_MOONLIGHT and set LED_COLOR_ID_WHITE?

> --
> Best regards,
> Jacek Anaszewski

Re: [PATCH v7 2/5] dt-bindings: leds: Add LED_COLOR_ID_MOONLIGHT definitions

[Gene Chen]

Fix Typo.

Gene Chen <gene.chen.richtek@gmail.com> 於 2020年11月24日 週二 下午3:33寫道：
>
> Jacek Anaszewski <jacek.anaszewski@gmail.com> 於 2020年11月24日 週二 上午4:52寫道：
> >
> > On 11/23/20 4:00 AM, Gene Chen wrote:
> > > Jacek Anaszewski <jacek.anaszewski@gmail.com> 於 2020年11月20日 週五 上午6:26寫道：
> > >>
> > >> On 11/19/20 10:57 PM, Pavel Machek wrote:
> > >>> On Thu 2020-11-19 22:03:14, Jacek Anaszewski wrote:
> > >>>> Hi Pavel, Gene,
> > >>>>
> > >>>> On 11/18/20 10:37 PM, Pavel Machek wrote:
> > >>>>> Hi!
> > >>>>>
> > >>>>>> From: Gene Chen <gene_chen@richtek.com>
> > >>>>>>
> > >>>>>> Add LED_COLOR_ID_MOONLIGHT definitions
> > >>>>>
> > >>>>> Why is moonlight a color? Camera flashes are usually white, no?
> > >>>>>
> > >>>>> At least it needs a comment...
> > >>>>
> > >>>> That's my fault, In fact I should have asked about adding
> > >>>> LED_FUNCTION_MOONLIGHT, it was evidently too late for me that evening...
> > >>>
> > >>> Aha, that makes more sense.
> > >>>
> > >>> But please let's call it "torch" if we do that, as that is already
> > >>> used in kernel sources... and probably in the interface, too:
> > >>
> > >> I'd say that torch is something different that moonlight,
> > >> but we would need more input from Gene to learn more about
> > >> the nature of light emitted by ML LED on his device.
> > >>
> > >> Please note that torch is usually meant as the other mode of
> > >> flash LED (sometimes it is called "movie mode"), which is already
> > >> handled by brightness file of LED class flash device (i.e. its LED class
> > >> subset), and which also maps to v4l2-flash TORCH mode.
> > >>
> > >
> > > It's used to front camera fill light.
> > > More brightness than screen backlight, and more soft light than flash.
> > > I think LED_ID_COLOR_WHITE is okay.
> >
> > So why in v6 you assigned LED_COLOR_ID_AMBER to it?
> >

we suppose Moonlight can be white or amber.

> > Regardless of that, now we're talking about LED function - you chose
> > LED_FUNCTION_INDICATOR for it, but inferring from your above description
> > - it certainly doesn't fit here.
> >
> > Also register names, containing part "ML" indicate that this LED's
> > intended function is moonlinght, which your description somehow
> > corroborates.
> >
> > Moonlight LEDs become ubiquitous nowadays so sooner or later we will
> > need to add this function anyway [0].
> >
> > [0]
> > https://landscapelightingoakville.com/what-is-moon-lighting-and-why-does-it-remain-so-popular/
> >
>
> According to reference,
> "When you are trying to imitate moonlight you need to use low voltage,
> softer lighting. You don’t want something that’s too bright"
> which is focus on brightness instead of color.
>
> So we suppose Moonlight can be white or amber.
>
> Should I add LED_FUNCTION_MOONLIGHT and set LED_COLOR_ID_WHITE?
>
> > --
> > Best regards,
> > Jacek Anaszewski

Re: [PATCH v7 2/5] dt-bindings: leds: Add LED_COLOR_ID_MOONLIGHT definitions

[Jacek Anaszewski]

On 11/24/20 8:33 AM, Gene Chen wrote:
> Jacek Anaszewski <jacek.anaszewski@gmail.com> 於 2020年11月24日 週二 上午4:52寫道：
>>
>> On 11/23/20 4:00 AM, Gene Chen wrote:
>>> Jacek Anaszewski <jacek.anaszewski@gmail.com> 於 2020年11月20日 週五 上午6:26寫道：
>>>>
>>>> On 11/19/20 10:57 PM, Pavel Machek wrote:
>>>>> On Thu 2020-11-19 22:03:14, Jacek Anaszewski wrote:
>>>>>> Hi Pavel, Gene,
>>>>>>
>>>>>> On 11/18/20 10:37 PM, Pavel Machek wrote:
>>>>>>> Hi!
>>>>>>>
>>>>>>>> From: Gene Chen <gene_chen@richtek.com>
>>>>>>>>
>>>>>>>> Add LED_COLOR_ID_MOONLIGHT definitions
>>>>>>>
>>>>>>> Why is moonlight a color? Camera flashes are usually white, no?
>>>>>>>
>>>>>>> At least it needs a comment...
>>>>>>
>>>>>> That's my fault, In fact I should have asked about adding
>>>>>> LED_FUNCTION_MOONLIGHT, it was evidently too late for me that evening...
>>>>>
>>>>> Aha, that makes more sense.
>>>>>
>>>>> But please let's call it "torch" if we do that, as that is already
>>>>> used in kernel sources... and probably in the interface, too:
>>>>
>>>> I'd say that torch is something different that moonlight,
>>>> but we would need more input from Gene to learn more about
>>>> the nature of light emitted by ML LED on his device.
>>>>
>>>> Please note that torch is usually meant as the other mode of
>>>> flash LED (sometimes it is called "movie mode"), which is already
>>>> handled by brightness file of LED class flash device (i.e. its LED class
>>>> subset), and which also maps to v4l2-flash TORCH mode.
>>>>
>>>
>>> It's used to front camera fill light.
>>> More brightness than screen backlight, and more soft light than flash.
>>> I think LED_ID_COLOR_WHITE is okay.
>>
>> So why in v6 you assigned LED_COLOR_ID_AMBER to it?
>>
>> Regardless of that, now we're talking about LED function - you chose
>> LED_FUNCTION_INDICATOR for it, but inferring from your above description
>> - it certainly doesn't fit here.
>>
>> Also register names, containing part "ML" indicate that this LED's
>> intended function is moonlinght, which your description somehow
>> corroborates.
>>
>> Moonlight LEDs become ubiquitous nowadays so sooner or later we will
>> need to add this function anyway [0].
>>
>> [0]
>> https://landscapelightingoakville.com/what-is-moon-lighting-and-why-does-it-remain-so-popular/
>>
> 
> We use term "Moonlight" as reference says
> "When you are trying to imitate moonlight you need to use low voltage,
> softer lighting. You don’t want something that’s too bright"
> which is focus on brightness instead of color.
> 
> So we surpose Moonlight can be white or amber.
> 
> Should I add LED_FUNCTION_MOONLIGHT and set LED_COLOR_ID_WHITE?

Regarding the function - yes, the reference backs that up.
Regarding the color - if you feel that it properly describes the
LED color then go for it.

-- 
Best regards,
Jacek Anaszewski

[PATCH v7 3/5] dt-bindings: leds: common: Increase LED_COLOR_ID_* maximum size

[Gene Chen]

From: Gene Chen <gene_chen@richtek.com>

Increase LED_COLOR_ID_* maximum size

Signed-off-by: Gene Chen <gene_chen@richtek.com>
---
 Documentation/devicetree/bindings/leds/common.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/leds/common.yaml b/Documentation/devicetree/bindings/leds/common.yaml
index a2a541b..f356b61 100644
--- a/Documentation/devicetree/bindings/leds/common.yaml
+++ b/Documentation/devicetree/bindings/leds/common.yaml
@@ -43,7 +43,7 @@ properties:
       LED_COLOR_ID available, add a new one.
     $ref: /schemas/types.yaml#definitions/uint32
     minimum: 0
-    maximum: 8
+    maximum: 10
 
   function-enumerator:
     description:
-- 
2.7.4

[PATCH v7 4/5] dt-bindings: leds: Add bindings for MT6360 LED

[Gene Chen]

From: Gene Chen <gene_chen@richtek.com>

Add bindings document for LED support on MT6360 PMIC

Signed-off-by: Gene Chen <gene_chen@richtek.com>
---
 .../devicetree/bindings/leds/leds-mt6360.yaml      | 164 +++++++++++++++++++++
 1 file changed, 164 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-mt6360.yaml

diff --git a/Documentation/devicetree/bindings/leds/leds-mt6360.yaml b/Documentation/devicetree/bindings/leds/leds-mt6360.yaml
new file mode 100644
index 0000000..9bd6cbe
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-mt6360.yaml
@@ -0,0 +1,164 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/leds-mt6360.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: LED driver for MT6360 PMIC from MediaTek Integrated.
+
+maintainers:
+  - Gene Chen <gene_chen@richtek.com>
+
+description: |
+  This module is part of the MT6360 MFD device.
+  see Documentation/devicetree/bindings/mfd/mt6360.yaml
+  Add MT6360 LED driver include 2-channel Flash LED with torch/strobe mode,
+  and 4-channel RGB LED support Register/Flash/Breath Mode
+
+properties:
+  compatible:
+    const: mediatek,mt6360-led
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+patternProperties:
+  "^led@[0-6]$":
+    type: object
+    $ref: common.yaml#
+    description:
+      Properties for a single LED.
+
+    properties:
+      reg:
+        description: Index of the LED.
+        enum:
+          - 0 # LED output INDICATOR1_RED
+          - 1 # LED output INDICATOR1_GREEN
+          - 2 # LED output INDICATOR1_BLUE
+          - 3 # LED output INDICATOR2_ML
+          - 4 # LED output FLED1
+          - 5 # LED output FLED2
+          - 6 # LED output MULTICOLOR
+
+unevaluatedProperties: false
+
+required:
+  - compatible
+  - "#address-cells"
+  - "#size-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+   #include <dt-bindings/leds/common.h>
+   led-controller {
+     compatible = "mediatek,mt6360-led";
+     #address-cells = <1>;
+     #size-cells = <0>;
+
+     led@3 {
+       reg = <3>;
+       function = LED_FUNCTION_INDICATOR;
+       color = <LED_COLOR_ID_MOONLIGHT>;
+       led-max-microamp = <150000>;
+     };
+     led@4 {
+       reg = <4>;
+       function = LED_FUNCTION_FLASH;
+       color = <LED_COLOR_ID_WHITE>;
+       function-enumerator = <1>;
+       led-max-microamp = <200000>;
+       flash-max-microamp = <500000>;
+       flash-max-timeout-us = <1024000>;
+     };
+     led@5 {
+       reg = <5>;
+       function = LED_FUNCTION_FLASH;
+       color = <LED_COLOR_ID_WHITE>;
+       function-enumerator = <2>;
+       led-max-microamp = <200000>;
+       flash-max-microamp = <500000>;
+       flash-max-timeout-us = <1024000>;
+     };
+     led@6 {
+       reg = <6>;
+       function = LED_FUNCTION_INDICATOR;
+       color = <LED_COLOR_ID_RGB>;
+       led-max-microamp = <24000>;
+       #address-cells = <1>;
+       #size-cells = <0>;
+
+       led@0 {
+         reg = <0>;
+         function = LED_FUNCTION_INDICATOR;
+         color = <LED_COLOR_ID_RED>;
+       };
+       led@1 {
+         reg = <1>;
+         function = LED_FUNCTION_INDICATOR;
+         color = <LED_COLOR_ID_GREEN>;
+       };
+       led@2 {
+         reg = <2>;
+         function = LED_FUNCTION_INDICATOR;
+         color = <LED_COLOR_ID_BLUE>;
+       };
+     };
+   };
+
+ - |
+
+   led-controller {
+     compatible = "mediatek,mt6360-led";
+     #address-cells = <1>;
+     #size-cells = <0>;
+
+     led@0 {
+       reg = <0>;
+       function = LED_FUNCTION_INDICATOR;
+       color = <LED_COLOR_ID_RED>;
+       led-max-microamp = <24000>;
+     };
+     led@1 {
+       reg = <1>;
+       function = LED_FUNCTION_INDICATOR;
+       color = <LED_COLOR_ID_GREEN>;
+       led-max-microamp = <24000>;
+     };
+     led@2 {
+       reg = <2>;
+       function = LED_FUNCTION_INDICATOR;
+       color = <LED_COLOR_ID_BLUE>;
+       led-max-microamp = <24000>;
+     };
+     led@3 {
+       reg = <3>;
+       function = LED_FUNCTION_INDICATOR;
+       color = <LED_COLOR_ID_MOONLIGHT>;
+       led-max-microamp = <150000>;
+     };
+     led@4 {
+       reg = <4>;
+       function = LED_FUNCTION_FLASH;
+       color = <LED_COLOR_ID_WHITE>;
+       function-enumerator = <1>;
+       led-max-microamp = <200000>;
+       flash-max-microamp = <500000>;
+       flash-max-timeout-us = <1024000>;
+     };
+     led@5 {
+       reg = <5>;
+       function = LED_FUNCTION_FLASH;
+       color = <LED_COLOR_ID_WHITE>;
+       function-enumerator = <2>;
+       led-max-microamp = <200000>;
+       flash-max-microamp = <500000>;
+       flash-max-timeout-us = <1024000>;
+     };
+   };
+...
-- 
2.7.4

Re: [PATCH v7 4/5] dt-bindings: leds: Add bindings for MT6360 LED

[Rob Herring]

On Wed, 18 Nov 2020 18:47:41 +0800, Gene Chen wrote:
> From: Gene Chen <gene_chen@richtek.com>
> 
> Add bindings document for LED support on MT6360 PMIC
> 
> Signed-off-by: Gene Chen <gene_chen@richtek.com>
> ---
>  .../devicetree/bindings/leds/leds-mt6360.yaml      | 164 +++++++++++++++++++++
>  1 file changed, 164 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/leds-mt6360.yaml
> 


My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:
./Documentation/devicetree/bindings/leds/leds-mt6360.yaml:57:2: [warning] wrong indentation: expected 2 but found 1 (indentation)

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/leds/leds-mt6360.example.dt.yaml: led-controller: led@3:color:0:0: 10 is greater than the maximum of 9
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/leds/leds-mt6360.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/leds/leds-mt6360.example.dt.yaml: led-controller: led@3:color:0:0: 10 is greater than the maximum of 9
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/leds/leds-mt6360.yaml


See https://patchwork.ozlabs.org/patch/1402193

The base for the patch is generally the last rc1. Any dependencies
should be noted.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [PATCH v7 4/5] dt-bindings: leds: Add bindings for MT6360 LED

[Gene Chen]

Rob Herring <robh@kernel.org> 於 2020年11月19日 週四 上午5:25寫道：
>
> On Wed, 18 Nov 2020 18:47:41 +0800, Gene Chen wrote:
> > From: Gene Chen <gene_chen@richtek.com>
> >
> > Add bindings document for LED support on MT6360 PMIC
> >
> > Signed-off-by: Gene Chen <gene_chen@richtek.com>
> > ---
> >  .../devicetree/bindings/leds/leds-mt6360.yaml      | 164 +++++++++++++++++++++
> >  1 file changed, 164 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/leds/leds-mt6360.yaml
> >
>
>
> My bot found errors running 'make dt_binding_check' on your patch:
>
> yamllint warnings/errors:
> ./Documentation/devicetree/bindings/leds/leds-mt6360.yaml:57:2: [warning] wrong indentation: expected 2 but found 1 (indentation)
>
> dtschema/dtc warnings/errors:
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/leds/leds-mt6360.example.dt.yaml: led-controller: led@3:color:0:0: 10 is greater than the maximum of 9
>         From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/leds/leds-mt6360.yaml
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/leds/leds-mt6360.example.dt.yaml: led-controller: led@3:color:0:0: 10 is greater than the maximum of 9
>         From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/leds/leds-mt6360.yaml
>
>

ACK, I will add description to this patch by
“this patch depends on patch [PATCH v7 3/5] dt-bindings: leds: common:
Increase LED_COLOR_ID_* maximum size"

> See https://patchwork.ozlabs.org/patch/1402193
>
> The base for the patch is generally the last rc1. Any dependencies
> should be noted.
>
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
>
> pip3 install dtschema --upgrade
>
> Please check and re-submit.
>

[PATCH v7 5/5] leds: mt6360: Add LED driver for MT6360

[Gene Chen]

From: Gene Chen <gene_chen@richtek.com>

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

Signed-off-by: Gene Chen <gene_chen@richtek.com>
---
 drivers/leds/Kconfig       |  13 +
 drivers/leds/Makefile      |   1 +
 drivers/leds/leds-mt6360.c | 808 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 822 insertions(+)
 create mode 100644 drivers/leds/leds-mt6360.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 1c181df..4f533bc 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -271,6 +271,19 @@ 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 && OF
+	depends on LEDS_CLASS_FLASH || !LEDS_CLASS_FLASH
+	depends on LEDS_CLASS_MULTICOLOR || !LEDS_CLASS_MULTICOLOR
+	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.
+	  Independent current sources supply for each flash LED support torch
+	  and strobe mode.
+
 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 c2c7d7a..5596427 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -66,6 +66,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..8432901
--- /dev/null
+++ b/drivers/leds/leds-mt6360.c
@@ -0,0 +1,808 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/bitops.h>
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/led-class-flash.h>
+#include <linux/led-class-multicolor.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <media/v4l2-flash-led-class.h>
+
+enum {
+	MT6360_LED_ISNK1 = 0,
+	MT6360_LED_ISNK2,
+	MT6360_LED_ISNK3,
+	MT6360_LED_ISNKML,
+	MT6360_LED_FLASH1,
+	MT6360_LED_FLASH2,
+	MT6360_LED_MULTICOLOR,
+	MT6360_MAX_LEDS = MT6360_LED_MULTICOLOR
+};
+
+#define MT6360_REG_RGBEN		0x380
+#define MT6360_REG_ISNK(_led_no)	(0x381 + (_led_no))
+#define MT6360_ISNK_ENMASK(_led_no)	BIT(7 - (_led_no))
+#define MT6360_ISNK_MASK		GENMASK(4, 0)
+#define MT6360_CHRINDSEL_MASK		BIT(3)
+
+#define MULTICOLOR_NUM_CHANNELS		3
+
+#define MT6360_REG_FLEDEN		0x37E
+#define MT6360_REG_STRBTO		0x373
+#define MT6360_REG_FLEDBASE(_id)	(0x372 + 4 * (_id - MT6360_LED_FLASH1))
+#define MT6360_REG_FLEDISTRB(_id)	(MT6360_REG_FLEDBASE(_id) + 2)
+#define MT6360_REG_FLEDITOR(_id)	(MT6360_REG_FLEDBASE(_id) + 3)
+#define MT6360_REG_CHGSTAT2		0x3E1
+#define MT6360_REG_FLEDSTAT1		0x3E9
+#define MT6360_ITORCH_MASK		GENMASK(4, 0)
+#define MT6360_ISTROBE_MASK		GENMASK(6, 0)
+#define MT6360_STRBTO_MASK		GENMASK(6, 0)
+#define MT6360_TORCHEN_MASK		BIT(3)
+#define MT6360_STROBEN_MASK		BIT(2)
+#define MT6360_FLCSEN_MASK(_id)		BIT(MT6360_LED_FLASH2 - _id)
+#define MT6360_FLEDCHGVINOVP_MASK	BIT(3)
+#define MT6360_FLED1STRBTO_MASK		BIT(11)
+#define MT6360_FLED2STRBTO_MASK		BIT(10)
+#define MT6360_FLED1STRB_MASK		BIT(9)
+#define MT6360_FLED2STRB_MASK		BIT(8)
+#define MT6360_FLED1SHORT_MASK		BIT(7)
+#define MT6360_FLED2SHORT_MASK		BIT(6)
+#define MT6360_FLEDLVF_MASK		BIT(3)
+
+#define MT6360_ISNKRGB_STEPUA		2000
+#define MT6360_ISNKRGB_MAXUA		24000
+#define MT6360_ISNKML_STEPUA		5000
+#define MT6360_ISNKML_MAXUA		150000
+
+#define MT6360_ITORCH_MINUA		25000
+#define MT6360_ITORCH_STEPUA		12500
+#define MT6360_ITORCH_MAXUA		400000
+#define MT6360_ISTRB_MINUA		50000
+#define MT6360_ISTRB_STEPUA		12500
+#define MT6360_ISTRB_MAXUA		1500000
+#define MT6360_STRBTO_MINUS		64000
+#define MT6360_STRBTO_STEPUS		32000
+#define MT6360_STRBTO_MAXUS		2432000
+
+#define STATE_OFF			0
+#define STATE_KEEP			1
+#define STATE_ON			2
+
+struct mt6360_led {
+	union {
+		struct led_classdev isnk;
+		struct led_classdev_mc mc;
+		struct led_classdev_flash flash;
+	};
+	struct v4l2_flash *v4l2_flash;
+	struct mt6360_priv *priv;
+	u32 led_no;
+	u32 default_state;
+};
+
+struct mt6360_priv {
+	struct device *dev;
+	struct regmap *regmap;
+	struct mutex lock;
+	unsigned int fled_strobe_used;
+	unsigned int fled_torch_used;
+	unsigned int leds_active;
+	unsigned int leds_count;
+	struct mt6360_led leds[];
+};
+
+static int mt6360_mc_brightness_set(struct led_classdev *lcdev, enum led_brightness level)
+{
+	struct led_classdev_mc *mccdev = lcdev_to_mccdev(lcdev);
+	struct mt6360_led *led = container_of(mccdev, struct mt6360_led, mc);
+	struct mt6360_priv *priv = led->priv;
+	u32 real_bright, enable_mask = 0, enable = 0;
+	int i, ret;
+
+	mutex_lock(&priv->lock);
+
+	led_mc_calc_color_components(mccdev, level);
+
+	for (i = 0; i < mccdev->num_colors; i++) {
+		struct mc_subled *subled = mccdev->subled_info + i;
+
+		real_bright = min(lcdev->max_brightness, subled->brightness);
+		ret = regmap_update_bits(priv->regmap, MT6360_REG_ISNK(i), MT6360_ISNK_MASK,
+					 real_bright);
+		if (ret)
+			goto out;
+
+		enable_mask |= MT6360_ISNK_ENMASK(subled->channel);
+		if (real_bright)
+			enable |= MT6360_ISNK_ENMASK(subled->channel);
+	}
+
+	ret = regmap_update_bits(priv->regmap, MT6360_REG_RGBEN, enable_mask, enable);
+
+out:
+	mutex_unlock(&priv->lock);
+	return ret;
+}
+
+static int mt6360_isnk_brightness_set(struct led_classdev *lcdev, enum led_brightness level)
+{
+	struct mt6360_led *led = container_of(lcdev, struct mt6360_led, isnk);
+	struct mt6360_priv *priv = led->priv;
+	u32 enable_mask = MT6360_ISNK_ENMASK(led->led_no);
+	u32 val = level ? MT6360_ISNK_ENMASK(led->led_no) : 0;
+	int ret;
+
+	mutex_lock(&priv->lock);
+
+	ret = regmap_update_bits(priv->regmap, MT6360_REG_ISNK(led->led_no), MT6360_ISNK_MASK,
+				 level);
+	if (ret)
+		goto out;
+
+	ret = regmap_update_bits(priv->regmap, MT6360_REG_RGBEN, enable_mask, val);
+
+out:
+	mutex_unlock(&priv->lock);
+	return ret;
+}
+
+static int mt6360_torch_brightness_set(struct led_classdev *lcdev, enum led_brightness level)
+{
+	struct mt6360_led *led = container_of(lcdev, struct mt6360_led, flash.led_cdev);
+	struct mt6360_priv *priv = led->priv;
+	u32 enable_mask = MT6360_TORCHEN_MASK | MT6360_FLCSEN_MASK(led->led_no);
+	u32 val = level ? MT6360_FLCSEN_MASK(led->led_no) : 0;
+	u32 prev = priv->fled_torch_used, curr;
+	int ret;
+
+	mutex_lock(&priv->lock);
+
+	/* Only one set of flash control logic, use the flag to avoid strobe is currently used */
+	if (priv->fled_strobe_used) {
+		dev_warn(lcdev->dev, "Please disable strobe first [%d]\n", priv->fled_strobe_used);
+		ret = -EBUSY;
+		goto unlock;
+	}
+
+	if (level)
+		curr = prev | BIT(led->led_no);
+	else
+		curr = prev & ~BIT(led->led_no);
+
+	if (curr)
+		val |= MT6360_TORCHEN_MASK;
+
+	if (level) {
+		ret = regmap_update_bits(priv->regmap, MT6360_REG_FLEDITOR(led->led_no),
+					 MT6360_ITORCH_MASK, level - 1);
+		if (ret)
+			goto unlock;
+	}
+
+	ret = regmap_update_bits(priv->regmap, MT6360_REG_FLEDEN, enable_mask, val);
+	if (ret)
+		goto unlock;
+
+	priv->fled_torch_used = curr;
+
+unlock:
+	mutex_unlock(&priv->lock);
+	return ret;
+}
+
+static int mt6360_flash_brightness_set(struct led_classdev_flash *fl_cdev, u32 brightness)
+{
+	/*
+	 * Due to the current spike when turning on flash, let brightness to be kept by framework.
+	 * This empty function is used to prevent led_classdev_flash register ops check failure.
+	 */
+	return 0;
+}
+
+static int _mt6360_flash_brightness_set(struct led_classdev_flash *fl_cdev, u32 brightness)
+{
+	struct mt6360_led *led = container_of(fl_cdev, struct mt6360_led, flash);
+	struct mt6360_priv *priv = led->priv;
+	struct led_flash_setting *s = &fl_cdev->brightness;
+	u32 val = (brightness - s->min) / s->step;
+
+	return regmap_update_bits(priv->regmap, MT6360_REG_FLEDISTRB(led->led_no),
+				 MT6360_ISTROBE_MASK, val);
+}
+
+static int mt6360_strobe_set(struct led_classdev_flash *fl_cdev, bool state)
+{
+	struct mt6360_led *led = container_of(fl_cdev, struct mt6360_led, flash);
+	struct mt6360_priv *priv = led->priv;
+	struct led_classdev *lcdev = &fl_cdev->led_cdev;
+	struct led_flash_setting *s = &fl_cdev->brightness;
+	u32 enable_mask = MT6360_STROBEN_MASK | MT6360_FLCSEN_MASK(led->led_no);
+	u32 val = state ? MT6360_FLCSEN_MASK(led->led_no) : 0;
+	u32 prev = priv->fled_strobe_used, curr;
+	int ret;
+
+	mutex_lock(&priv->lock);
+
+	/* Only one set of flash control logic, use the flag to avoid torch is currently used */
+	if (priv->fled_torch_used) {
+		dev_warn(lcdev->dev, "Please disable torch first [0x%x]\n", priv->fled_torch_used);
+		ret = -EBUSY;
+		goto unlock;
+	}
+
+	if (state)
+		curr = prev | BIT(led->led_no);
+	else
+		curr = prev & ~BIT(led->led_no);
+
+	if (curr)
+		val |= MT6360_STROBEN_MASK;
+
+	ret = regmap_update_bits(priv->regmap, MT6360_REG_FLEDEN, enable_mask, val);
+	if (ret) {
+		dev_err(lcdev->dev, "[%d] control current source %d fail\n", led->led_no, state);
+		goto unlock;
+	}
+
+	/*
+	 * If the flash need to be on, config the flash current ramping up to the setting value
+	 * Else, always recover back to the minimum one
+	 */
+	ret = _mt6360_flash_brightness_set(fl_cdev, state ? s->val : s->min);
+	if (ret)
+		goto unlock;
+
+	/* For the flash turn on/off, HW rampping up/down time is 5ms/500us, respectively */
+	if (!prev && curr)
+		usleep_range(5000, 6000);
+	else if (prev && !curr)
+		udelay(500);
+
+	priv->fled_strobe_used = curr;
+
+unlock:
+	mutex_unlock(&priv->lock);
+	return ret;
+}
+
+static int mt6360_strobe_get(struct led_classdev_flash *fl_cdev, bool *state)
+{
+	struct mt6360_led *led = container_of(fl_cdev, struct mt6360_led, flash);
+	struct mt6360_priv *priv = led->priv;
+
+	mutex_lock(&priv->lock);
+	*state = !!(priv->fled_strobe_used & BIT(led->led_no));
+	mutex_unlock(&priv->lock);
+
+	return 0;
+}
+
+static int mt6360_timeout_set(struct led_classdev_flash *fl_cdev, u32 timeout)
+{
+	struct mt6360_led *led = container_of(fl_cdev, struct mt6360_led, flash);
+	struct mt6360_priv *priv = led->priv;
+	struct led_flash_setting *s = &fl_cdev->timeout;
+	u32 val = (timeout - s->min) / s->step;
+	int ret;
+
+	mutex_lock(&priv->lock);
+	ret = regmap_update_bits(priv->regmap, MT6360_REG_STRBTO, MT6360_STRBTO_MASK, val);
+	mutex_unlock(&priv->lock);
+
+	return ret;
+}
+
+static int mt6360_fault_get(struct led_classdev_flash *fl_cdev, u32 *fault)
+{
+	struct mt6360_led *led = container_of(fl_cdev, struct mt6360_led, flash);
+	struct mt6360_priv *priv = led->priv;
+	u16 fled_stat;
+	unsigned int chg_stat, strobe_timeout_mask, fled_short_mask;
+	u32 rfault = 0;
+	int ret;
+
+	ret = regmap_read(priv->regmap, MT6360_REG_CHGSTAT2, &chg_stat);
+	if (ret)
+		return ret;
+
+	ret = regmap_raw_read(priv->regmap, MT6360_REG_FLEDSTAT1, &fled_stat, sizeof(fled_stat));
+	if (ret)
+		return ret;
+
+	if (led->led_no == MT6360_LED_FLASH1) {
+		strobe_timeout_mask = MT6360_FLED1STRBTO_MASK;
+		fled_short_mask = MT6360_FLED1SHORT_MASK;
+	} else {
+		strobe_timeout_mask = MT6360_FLED2STRBTO_MASK;
+		fled_short_mask = MT6360_FLED2SHORT_MASK;
+	}
+
+	if (chg_stat & MT6360_FLEDCHGVINOVP_MASK)
+		rfault |= LED_FAULT_INPUT_VOLTAGE;
+
+	if (fled_stat & strobe_timeout_mask)
+		rfault |= LED_FAULT_TIMEOUT;
+
+	if (fled_stat & fled_short_mask)
+		rfault |= LED_FAULT_SHORT_CIRCUIT;
+
+	if (fled_stat & MT6360_FLEDLVF_MASK)
+		rfault |= LED_FAULT_UNDER_VOLTAGE;
+
+	*fault = rfault;
+	return 0;
+}
+
+static const struct led_flash_ops mt6360_flash_ops = {
+	.flash_brightness_set = mt6360_flash_brightness_set,
+	.strobe_set = mt6360_strobe_set,
+	.strobe_get = mt6360_strobe_get,
+	.timeout_set = mt6360_timeout_set,
+	.fault_get = mt6360_fault_get,
+};
+
+static int mt6360_isnk_init_default_state(struct mt6360_led *led)
+{
+	struct mt6360_priv *priv = led->priv;
+	unsigned int regval;
+	u32 level;
+	int ret;
+
+	ret = regmap_read(priv->regmap, MT6360_REG_ISNK(led->led_no), &regval);
+	if (ret)
+		return ret;
+	level = regval & MT6360_ISNK_MASK;
+
+	ret = regmap_read(priv->regmap, MT6360_REG_RGBEN, &regval);
+	if (ret)
+		return ret;
+
+	if (!(regval & MT6360_ISNK_ENMASK(led->led_no)))
+		level = LED_OFF;
+
+	switch (led->default_state) {
+	case STATE_ON:
+		led->isnk.brightness = led->isnk.max_brightness;
+		break;
+	case STATE_KEEP:
+		led->isnk.brightness = min(level, led->isnk.max_brightness);
+		break;
+	default:
+		led->isnk.brightness = LED_OFF;
+	}
+
+	return mt6360_isnk_brightness_set(&led->isnk, led->isnk.brightness);
+}
+
+static int mt6360_flash_init_default_state(struct mt6360_led *led)
+{
+	struct led_classdev_flash *flash = &led->flash;
+	struct mt6360_priv *priv = led->priv;
+	u32 enable_mask = MT6360_TORCHEN_MASK | MT6360_FLCSEN_MASK(led->led_no);
+	u32 level;
+	unsigned int regval;
+	int ret;
+
+	ret = regmap_read(priv->regmap, MT6360_REG_FLEDITOR(led->led_no), &regval);
+	if (ret)
+		return ret;
+	level = regval & MT6360_ITORCH_MASK;
+
+	ret = regmap_read(priv->regmap, MT6360_REG_FLEDEN, &regval);
+	if (ret)
+		return ret;
+
+	if ((regval & enable_mask) == enable_mask)
+		level += 1;
+	else
+		level = LED_OFF;
+
+	switch (led->default_state) {
+	case STATE_ON:
+		flash->led_cdev.brightness = flash->led_cdev.max_brightness;
+		break;
+	case STATE_KEEP:
+		flash->led_cdev.brightness = min(level, flash->led_cdev.max_brightness);
+		break;
+	default:
+		flash->led_cdev.brightness = LED_OFF;
+	}
+
+	return mt6360_torch_brightness_set(&flash->led_cdev, flash->led_cdev.brightness);
+}
+
+#if IS_ENABLED(CONFIG_V4L2_FLASH_LED_CLASS)
+static int mt6360_flash_external_strobe_set(struct v4l2_flash *v4l2_flash, bool enable)
+{
+	struct led_classdev_flash *flash = v4l2_flash->fled_cdev;
+	struct mt6360_led *led = container_of(flash, struct mt6360_led, flash);
+	struct mt6360_priv *priv = led->priv;
+	u32 mask = MT6360_FLCSEN_MASK(led->led_no);
+	u32 val = enable ? mask : 0;
+	int ret;
+
+	mutex_lock(&priv->lock);
+
+	ret = regmap_update_bits(priv->regmap, MT6360_REG_FLEDEN, mask, val);
+	if (ret)
+		goto unlock;
+
+	if (enable)
+		priv->fled_strobe_used |= BIT(led->led_no);
+	else
+		priv->fled_strobe_used &= ~BIT(led->led_no);
+
+unlock:
+	mutex_unlock(&priv->lock);
+	return ret;
+}
+
+static const struct v4l2_flash_ops v4l2_flash_ops = {
+	.external_strobe_set = mt6360_flash_external_strobe_set,
+};
+
+static void mt6360_init_v4l2_flash_config(struct mt6360_led *led, struct v4l2_flash_config *config)
+{
+	struct led_classdev *lcdev;
+	struct led_flash_setting *s = &config->intensity;
+
+	lcdev = &led->flash.led_cdev;
+
+	s->min = MT6360_ITORCH_MINUA;
+	s->step = MT6360_ITORCH_STEPUA;
+	s->val = s->max = s->min + (lcdev->max_brightness - 1) * s->step;
+
+	config->has_external_strobe = 1;
+	strscpy(config->dev_name, lcdev->dev->kobj.name, sizeof(config->dev_name));
+
+	config->flash_faults = LED_FAULT_SHORT_CIRCUIT | LED_FAULT_TIMEOUT |
+			       LED_FAULT_INPUT_VOLTAGE | LED_FAULT_UNDER_VOLTAGE;
+}
+#else
+static const struct v4l2_flash_ops v4l2_flash_ops;
+static void mt6360_init_v4l2_flash_config(struct mt6360_led *led, struct v4l2_flash_config *config)
+{
+}
+#endif
+
+static int mt6360_led_register(struct device *parent, struct mt6360_led *led,
+				struct led_init_data *init_data)
+{
+	struct mt6360_priv *priv = led->priv;
+	struct v4l2_flash_config v4l2_config = {0};
+	int ret;
+
+	if ((led->led_no == MT6360_LED_ISNK1 || led->led_no == MT6360_LED_MULTICOLOR) &&
+		(priv->leds_active & BIT(MT6360_LED_ISNK1))) {
+		/* Change isink1 to SW control mode, disconnect it with charger state */
+		ret = regmap_update_bits(priv->regmap, MT6360_REG_RGBEN, MT6360_CHRINDSEL_MASK,
+					 MT6360_CHRINDSEL_MASK);
+		if (ret) {
+			dev_err(parent, "Failed to config ISNK1 to SW mode\n");
+			return ret;
+		}
+	}
+
+	switch (led->led_no) {
+	case MT6360_LED_MULTICOLOR:
+		ret = mt6360_mc_brightness_set(&led->mc.led_cdev, LED_OFF);
+		if (ret) {
+			dev_err(parent, "Failed to init multicolor brightness\n");
+			return ret;
+		}
+
+		ret = devm_led_classdev_multicolor_register_ext(parent, &led->mc, init_data);
+		if (ret) {
+			dev_err(parent, "Couldn't register multicolor\n");
+			return ret;
+		}
+		break;
+	case MT6360_LED_ISNK1 ... MT6360_LED_ISNKML:
+		ret = mt6360_isnk_init_default_state(led);
+		if (ret) {
+			dev_err(parent, "Failed to init %d isnk state\n", led->led_no);
+			return ret;
+		}
+
+		ret = devm_led_classdev_register_ext(parent, &led->isnk, init_data);
+		if (ret) {
+			dev_err(parent, "Couldn't register isink %d\n", led->led_no);
+			return ret;
+		}
+		break;
+	default:
+		ret = mt6360_flash_init_default_state(led);
+		if (ret) {
+			dev_err(parent, "Failed to init %d flash state\n", led->led_no);
+			return ret;
+		}
+
+		ret = devm_led_classdev_flash_register_ext(parent, &led->flash, init_data);
+		if (ret) {
+			dev_err(parent, "Couldn't register flash %d\n", led->led_no);
+			return ret;
+		}
+
+		mt6360_init_v4l2_flash_config(led, &v4l2_config);
+		led->v4l2_flash = v4l2_flash_init(parent, init_data->fwnode, &led->flash,
+						  &v4l2_flash_ops, &v4l2_config);
+		if (IS_ERR(led->v4l2_flash)) {
+			dev_err(parent, "Failed to register %d v4l2 sd\n", led->led_no);
+			return PTR_ERR(led->v4l2_flash);
+		}
+	}
+
+	return 0;
+}
+
+static u32 clamp_align(u32 val, u32 min, u32 max, u32 step)
+{
+	u32 retval;
+
+	retval = clamp_val(val, min, max);
+	if (step > 1)
+		retval = rounddown(retval - min, step) + min;
+
+	return retval;
+}
+
+static int mt6360_init_isnk_properties(struct mt6360_led *led, struct led_init_data *init_data)
+{
+	struct led_classdev *lcdev;
+	struct mt6360_priv *priv = led->priv;
+	struct fwnode_handle *child;
+	u32 step_uA = MT6360_ISNKRGB_STEPUA, max_uA = MT6360_ISNKRGB_MAXUA;
+	u32 val;
+	int num_color = 0, ret;
+
+	if (led->led_no == MT6360_LED_MULTICOLOR) {
+		struct mc_subled *sub_led;
+
+		sub_led = devm_kzalloc(priv->dev, sizeof(*sub_led) * MULTICOLOR_NUM_CHANNELS,
+				       GFP_KERNEL);
+		if (!sub_led)
+			return -ENOMEM;
+
+		fwnode_for_each_child_node(init_data->fwnode, child) {
+			u32 reg, color;
+
+			ret = fwnode_property_read_u32(child, "reg", &reg);
+			if (ret || reg > MT6360_LED_ISNK3 || priv->leds_active & BIT(reg))
+				return -EINVAL;
+
+			ret = fwnode_property_read_u32(child, "color", &color);
+			if (ret) {
+				dev_err(priv->dev, "led %d, no color specified\n", led->led_no);
+				return ret;
+			}
+
+			priv->leds_active |= BIT(reg);
+			sub_led[num_color].color_index = color;
+			sub_led[num_color].channel = reg;
+			num_color++;
+		}
+
+		if (num_color < 2) {
+			dev_err(priv->dev, "Multicolor must include 2 or more led channel\n");
+			return -EINVAL;
+		}
+
+		led->mc.num_colors = num_color;
+		led->mc.subled_info = sub_led;
+
+		lcdev = &led->mc.led_cdev;
+		lcdev->brightness_set_blocking = mt6360_mc_brightness_set;
+	} else {
+		if (led->led_no == MT6360_LED_ISNKML) {
+			step_uA = MT6360_ISNKML_STEPUA;
+			max_uA = MT6360_ISNKML_MAXUA;
+		}
+
+		lcdev = &led->isnk;
+		lcdev->brightness_set_blocking = mt6360_isnk_brightness_set;
+	}
+
+	ret = fwnode_property_read_u32(init_data->fwnode, "led-max-microamp", &val);
+	if (ret) {
+		dev_warn(priv->dev, "Not specified led-max-microamp, config to the minimum\n");
+		val = step_uA;
+	} else
+		val = clamp_align(val, 0, max_uA, step_uA);
+
+	lcdev->max_brightness = val / step_uA;
+
+	fwnode_property_read_string(init_data->fwnode, "linux,default-trigger",
+				    &lcdev->default_trigger);
+
+	return 0;
+}
+
+static int mt6360_init_flash_properties(struct mt6360_led *led, struct led_init_data *init_data)
+{
+	struct led_classdev_flash *flash = &led->flash;
+	struct led_classdev *lcdev = &flash->led_cdev;
+	struct mt6360_priv *priv = led->priv;
+	struct led_flash_setting *s;
+	u32 val;
+	int ret;
+
+	ret = fwnode_property_read_u32(init_data->fwnode, "led-max-microamp", &val);
+	if (ret) {
+		dev_warn(priv->dev, "Not specified led-max-microamp, config to the minimum\n");
+		val = MT6360_ITORCH_MINUA;
+	} else
+		val = clamp_align(val, MT6360_ITORCH_MINUA, MT6360_ITORCH_MAXUA,
+				  MT6360_ITORCH_STEPUA);
+
+	lcdev->max_brightness = (val - MT6360_ITORCH_MINUA) / MT6360_ITORCH_STEPUA + 1;
+	lcdev->brightness_set_blocking = mt6360_torch_brightness_set;
+	lcdev->flags |= LED_DEV_CAP_FLASH;
+
+	ret = fwnode_property_read_u32(init_data->fwnode, "flash-max-microamp", &val);
+	if (ret) {
+		dev_warn(priv->dev, "Not specified flash-max-microamp, config to the minimum\n");
+		val = MT6360_ISTRB_MINUA;
+	} else
+		val = clamp_align(val, MT6360_ISTRB_MINUA, MT6360_ISTRB_MAXUA, MT6360_ISTRB_STEPUA);
+
+	s = &flash->brightness;
+	s->min = MT6360_ISTRB_MINUA;
+	s->step = MT6360_ISTRB_STEPUA;
+	s->val = s->max = val;
+
+	/* Always configure as min level when off to prevent flash current spike */
+	ret = _mt6360_flash_brightness_set(flash, s->min);
+	if (ret)
+		return ret;
+
+	ret = fwnode_property_read_u32(init_data->fwnode, "flash-max-timeout-us", &val);
+	if (ret) {
+		dev_warn(priv->dev, "Not specified flash-max-timeout-us, config to the minimum\n");
+		val = MT6360_STRBTO_MINUS;
+	} else
+		val = clamp_align(val, MT6360_STRBTO_MINUS, MT6360_STRBTO_MAXUS,
+				  MT6360_STRBTO_STEPUS);
+
+	s = &flash->timeout;
+	s->min = MT6360_STRBTO_MINUS;
+	s->step = MT6360_STRBTO_STEPUS;
+	s->val = s->max = val;
+
+	flash->ops = &mt6360_flash_ops;
+
+	return 0;
+}
+
+static int mt6360_init_common_properties(struct mt6360_led *led, struct led_init_data *init_data)
+{
+	const char * const states[] = { "off", "keep", "on" };
+	const char *str;
+	int ret;
+
+	if (!fwnode_property_read_string(init_data->fwnode, "default-state", &str)) {
+		ret = match_string(states, ARRAY_SIZE(states), str);
+		if (ret < 0)
+			ret = STATE_OFF;
+
+		led->default_state = ret;
+	}
+
+	return 0;
+}
+
+static void mt6360_v4l2_flash_release(struct mt6360_priv *priv)
+{
+	int i;
+
+	for (i = 0; i < priv->leds_count; i++) {
+		struct mt6360_led *led = priv->leds + i;
+
+		if (led->v4l2_flash)
+			v4l2_flash_release(led->v4l2_flash);
+	}
+}
+
+static int mt6360_led_probe(struct platform_device *pdev)
+{
+	struct mt6360_priv *priv;
+	struct fwnode_handle *child;
+	size_t count;
+	int i = 0, ret;
+
+	count = device_get_child_node_count(&pdev->dev);
+	if (!count || count > MT6360_MAX_LEDS) {
+		dev_err(&pdev->dev, "No child node or node count over max led number %lu\n", count);
+		return -EINVAL;
+	}
+
+	priv = devm_kzalloc(&pdev->dev, struct_size(priv, leds, count), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->leds_count = count;
+	priv->dev = &pdev->dev;
+	mutex_init(&priv->lock);
+
+	priv->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+	if (!priv->regmap) {
+		dev_err(&pdev->dev, "Failed to get parent regmap\n");
+		return -ENODEV;
+	}
+
+	device_for_each_child_node(&pdev->dev, child) {
+		struct mt6360_led *led = priv->leds + i;
+		struct led_init_data init_data = { .fwnode = child, };
+		u32 reg;
+
+		ret = fwnode_property_read_u32(child, "reg", &reg);
+		if (ret)
+			goto out_flash_release;
+
+		if (reg > MT6360_MAX_LEDS || priv->leds_active & BIT(reg))
+			return -EINVAL;
+		priv->leds_active |= BIT(reg);
+
+		led->led_no = reg;
+		led->priv = priv;
+
+		ret = mt6360_init_common_properties(led, &init_data);
+		if (ret)
+			goto out_flash_release;
+
+		if (reg == MT6360_LED_MULTICOLOR ||
+			(reg >= MT6360_LED_ISNK1 && reg <= MT6360_LED_ISNKML))
+			ret = mt6360_init_isnk_properties(led, &init_data);
+		else
+			ret = mt6360_init_flash_properties(led, &init_data);
+
+		if (ret)
+			goto out_flash_release;
+
+		ret = mt6360_led_register(&pdev->dev, led, &init_data);
+		if (ret)
+			goto out_flash_release;
+
+		i++;
+	}
+
+	platform_set_drvdata(pdev, priv);
+	return 0;
+
+out_flash_release:
+	mt6360_v4l2_flash_release(priv);
+	return ret;
+}
+
+static int mt6360_led_remove(struct platform_device *pdev)
+{
+	struct mt6360_priv *priv = platform_get_drvdata(pdev);
+
+	mt6360_v4l2_flash_release(priv);
+	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",
+		.of_match_table = mt6360_led_of_id,
+	},
+	.probe = mt6360_led_probe,
+	.remove = mt6360_led_remove,
+};
+module_platform_driver(mt6360_led_driver);
+
+MODULE_AUTHOR("Gene Chen <gene_chen@richtek.com>");
+MODULE_DESCRIPTION("MT6360 LED Driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

Re: [PATCH v7 5/5] leds: mt6360: Add LED driver for MT6360

[Jacek Anaszewski]

Hi Gene,

On 11/18/20 11:47 AM, Gene Chen wrote:
> From: Gene Chen <gene_chen@richtek.com>
> 
> Add MT6360 LED driver include 2-channel Flash LED with torch/strobe mode,
> 3-channel RGB LED support Register/Flash/Breath Mode, and 1-channel for
> moonlight LED.
> 
> Signed-off-by: Gene Chen <gene_chen@richtek.com>
> ---
>   drivers/leds/Kconfig       |  13 +
>   drivers/leds/Makefile      |   1 +
>   drivers/leds/leds-mt6360.c | 808 +++++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 822 insertions(+)
>   create mode 100644 drivers/leds/leds-mt6360.c
> 
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 1c181df..4f533bc 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -271,6 +271,19 @@ 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 && OF
> +	depends on LEDS_CLASS_FLASH || !LEDS_CLASS_FLASH
> +	depends on LEDS_CLASS_MULTICOLOR || !LEDS_CLASS_MULTICOLOR
> +	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.
> +	  Independent current sources supply for each flash LED support torch
> +	  and strobe mode.
> +
>   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 c2c7d7a..5596427 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -66,6 +66,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..8432901
> --- /dev/null
> +++ b/drivers/leds/leds-mt6360.c
> @@ -0,0 +1,808 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
[...]
> +static int mt6360_fault_get(struct led_classdev_flash *fl_cdev, u32 *fault)
> +{
> +	struct mt6360_led *led = container_of(fl_cdev, struct mt6360_led, flash);
> +	struct mt6360_priv *priv = led->priv;
> +	u16 fled_stat;
> +	unsigned int chg_stat, strobe_timeout_mask, fled_short_mask;
> +	u32 rfault = 0;
> +	int ret;

You need mutex here as well because you're making two readouts and
you have to assure atomicity of this operation.

> +	ret = regmap_read(priv->regmap, MT6360_REG_CHGSTAT2, &chg_stat);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_raw_read(priv->regmap, MT6360_REG_FLEDSTAT1, &fled_stat, sizeof(fled_stat));
> +	if (ret)
> +		return ret;
> +
> +	if (led->led_no == MT6360_LED_FLASH1) {
> +		strobe_timeout_mask = MT6360_FLED1STRBTO_MASK;
> +		fled_short_mask = MT6360_FLED1SHORT_MASK;
> +	} else {
> +		strobe_timeout_mask = MT6360_FLED2STRBTO_MASK;
> +		fled_short_mask = MT6360_FLED2SHORT_MASK;
> +	}
> +
> +	if (chg_stat & MT6360_FLEDCHGVINOVP_MASK)
> +		rfault |= LED_FAULT_INPUT_VOLTAGE;
> +
> +	if (fled_stat & strobe_timeout_mask)
> +		rfault |= LED_FAULT_TIMEOUT;
> +
> +	if (fled_stat & fled_short_mask)
> +		rfault |= LED_FAULT_SHORT_CIRCUIT;
> +
> +	if (fled_stat & MT6360_FLEDLVF_MASK)
> +		rfault |= LED_FAULT_UNDER_VOLTAGE;
> +
> +	*fault = rfault;
> +	return 0;
> +}
> +
[...]

-- 
Best regards,
Jacek Anaszewski