From: Gene Chen <gene.chen.richtek@gmail.com>
To: Jacek Anaszewski <jacek.anaszewski@gmail.com>
Cc: Pavel Machek <pavel@ucw.cz>, Rob Herring <robh+dt@kernel.org>,
Matthias Brugger <matthias.bgg@gmail.com>,
Dan Murphy <dmurphy@ti.com>,
Linux LED Subsystem <linux-leds@vger.kernel.org>,
devicetree <devicetree@vger.kernel.org>,
linux-arm Mailing List <linux-arm-kernel@lists.infradead.org>,
"moderated list:ARM/Mediatek SoC support"
<linux-mediatek@lists.infradead.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Gene Chen <gene_chen@richtek.com>,
Wilma.Wu@mediatek.com, shufan_lee@richtek.com,
cy_huang@richtek.com, benjamin.chao@mediatek.com
Subject: Re: [PATCH v6 1/2] dt-bindings: leds: Add bindings for MT6360 LED
Date: Wed, 18 Nov 2020 18:38:47 +0800 [thread overview]
Message-ID: <CAE+NS345g6gokjpBn0G9uR2azA9p2ZCgujC2cM5pWmrmmwqYjA@mail.gmail.com> (raw)
In-Reply-To: <ff327630-d919-c26c-30ca-066ad8e826bb@gmail.com>
Jacek Anaszewski <jacek.anaszewski@gmail.com> 於 2020年11月18日 週三 上午6:24寫道:
>
> Hi Gene,
>
> Thank you for the patch.
>
> On 11/17/20 11:55 AM, 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 | 114 +++++++++++++++++++++
> > 1 file changed, 114 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..871db4d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/leds/leds-mt6360.yaml
> > @@ -0,0 +1,114 @@
> > +# 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
>
> What actually is the Register mode?
>
Register mode is equal to S/W mode.
Red LED indicator ic default is H/W mode. LED enable when charger plug in.
> > +
> > +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@0 {
> > + reg = <0>;
> > + function = LED_FUNCTION_INDICATOR;
> > + color = <LED_COLOR_ID_RED>;
> > + led-max-microamp = <24000>;
> > + };
> > + led@3 {
> > + reg = <3>;
> > + function = LED_FUNCTION_INDICATOR;
> > + color = <LED_COLOR_ID_AMBER>;
>
> You should really have here LED_COLOR_ID_MOONLIGHT if this is
> a moonlight LED. You'll need to add it to dt-bindings/leds/common.h.
>
> > + 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_MULTI>;
> > + led-max-microamp = <24000>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + 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>;
> > + };
> > + };
>
> It is of little avail to have multicolor LED with only two LEDs.
> I propose not to allow such setup - i.e. you should have either
> one multicolor LED comprising three sub-LEDs (regs: 0, 1, 2),
> and with main color property set to LED_COLOR_ID_RGB, or three separate
> LEDs.
>
> Effectively, you should have two separate DT examples here to make it
> clear: one for the case with three LED class devices and one with
> LED multicolor class device.
>
ACK
> > + };
> > +...
> >
>
> --
> Best regards,
> Jacek Anaszewski
WARNING: multiple messages have this Message-ID (diff)
From: Gene Chen <gene.chen.richtek@gmail.com>
To: Jacek Anaszewski <jacek.anaszewski@gmail.com>
Cc: linux-arm Mailing List <linux-arm-kernel@lists.infradead.org>,
devicetree <devicetree@vger.kernel.org>,
cy_huang@richtek.com,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Gene Chen <gene_chen@richtek.com>,
benjamin.chao@mediatek.com, Rob Herring <robh+dt@kernel.org>,
"moderated list:ARM/Mediatek SoC support"
<linux-mediatek@lists.infradead.org>, Dan Murphy <dmurphy@ti.com>,
Pavel Machek <pavel@ucw.cz>,
Matthias Brugger <matthias.bgg@gmail.com>,
Wilma.Wu@mediatek.com,
Linux LED Subsystem <linux-leds@vger.kernel.org>,
shufan_lee@richtek.com
Subject: Re: [PATCH v6 1/2] dt-bindings: leds: Add bindings for MT6360 LED
Date: Wed, 18 Nov 2020 18:38:47 +0800 [thread overview]
Message-ID: <CAE+NS345g6gokjpBn0G9uR2azA9p2ZCgujC2cM5pWmrmmwqYjA@mail.gmail.com> (raw)
In-Reply-To: <ff327630-d919-c26c-30ca-066ad8e826bb@gmail.com>
Jacek Anaszewski <jacek.anaszewski@gmail.com> 於 2020年11月18日 週三 上午6:24寫道:
>
> Hi Gene,
>
> Thank you for the patch.
>
> On 11/17/20 11:55 AM, 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 | 114 +++++++++++++++++++++
> > 1 file changed, 114 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..871db4d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/leds/leds-mt6360.yaml
> > @@ -0,0 +1,114 @@
> > +# 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
>
> What actually is the Register mode?
>
Register mode is equal to S/W mode.
Red LED indicator ic default is H/W mode. LED enable when charger plug in.
> > +
> > +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@0 {
> > + reg = <0>;
> > + function = LED_FUNCTION_INDICATOR;
> > + color = <LED_COLOR_ID_RED>;
> > + led-max-microamp = <24000>;
> > + };
> > + led@3 {
> > + reg = <3>;
> > + function = LED_FUNCTION_INDICATOR;
> > + color = <LED_COLOR_ID_AMBER>;
>
> You should really have here LED_COLOR_ID_MOONLIGHT if this is
> a moonlight LED. You'll need to add it to dt-bindings/leds/common.h.
>
> > + 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_MULTI>;
> > + led-max-microamp = <24000>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + 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>;
> > + };
> > + };
>
> It is of little avail to have multicolor LED with only two LEDs.
> I propose not to allow such setup - i.e. you should have either
> one multicolor LED comprising three sub-LEDs (regs: 0, 1, 2),
> and with main color property set to LED_COLOR_ID_RGB, or three separate
> LEDs.
>
> Effectively, you should have two separate DT examples here to make it
> clear: one for the case with three LED class devices and one with
> LED multicolor class device.
>
ACK
> > + };
> > +...
> >
>
> --
> Best regards,
> Jacek Anaszewski
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
WARNING: multiple messages have this Message-ID (diff)
From: Gene Chen <gene.chen.richtek@gmail.com>
To: Jacek Anaszewski <jacek.anaszewski@gmail.com>
Cc: linux-arm Mailing List <linux-arm-kernel@lists.infradead.org>,
devicetree <devicetree@vger.kernel.org>,
cy_huang@richtek.com,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Gene Chen <gene_chen@richtek.com>,
benjamin.chao@mediatek.com, Rob Herring <robh+dt@kernel.org>,
"moderated list:ARM/Mediatek SoC support"
<linux-mediatek@lists.infradead.org>, Dan Murphy <dmurphy@ti.com>,
Pavel Machek <pavel@ucw.cz>,
Matthias Brugger <matthias.bgg@gmail.com>,
Wilma.Wu@mediatek.com,
Linux LED Subsystem <linux-leds@vger.kernel.org>,
shufan_lee@richtek.com
Subject: Re: [PATCH v6 1/2] dt-bindings: leds: Add bindings for MT6360 LED
Date: Wed, 18 Nov 2020 18:38:47 +0800 [thread overview]
Message-ID: <CAE+NS345g6gokjpBn0G9uR2azA9p2ZCgujC2cM5pWmrmmwqYjA@mail.gmail.com> (raw)
In-Reply-To: <ff327630-d919-c26c-30ca-066ad8e826bb@gmail.com>
Jacek Anaszewski <jacek.anaszewski@gmail.com> 於 2020年11月18日 週三 上午6:24寫道:
>
> Hi Gene,
>
> Thank you for the patch.
>
> On 11/17/20 11:55 AM, 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 | 114 +++++++++++++++++++++
> > 1 file changed, 114 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..871db4d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/leds/leds-mt6360.yaml
> > @@ -0,0 +1,114 @@
> > +# 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
>
> What actually is the Register mode?
>
Register mode is equal to S/W mode.
Red LED indicator ic default is H/W mode. LED enable when charger plug in.
> > +
> > +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@0 {
> > + reg = <0>;
> > + function = LED_FUNCTION_INDICATOR;
> > + color = <LED_COLOR_ID_RED>;
> > + led-max-microamp = <24000>;
> > + };
> > + led@3 {
> > + reg = <3>;
> > + function = LED_FUNCTION_INDICATOR;
> > + color = <LED_COLOR_ID_AMBER>;
>
> You should really have here LED_COLOR_ID_MOONLIGHT if this is
> a moonlight LED. You'll need to add it to dt-bindings/leds/common.h.
>
> > + 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_MULTI>;
> > + led-max-microamp = <24000>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + 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>;
> > + };
> > + };
>
> It is of little avail to have multicolor LED with only two LEDs.
> I propose not to allow such setup - i.e. you should have either
> one multicolor LED comprising three sub-LEDs (regs: 0, 1, 2),
> and with main color property set to LED_COLOR_ID_RGB, or three separate
> LEDs.
>
> Effectively, you should have two separate DT examples here to make it
> clear: one for the case with three LED class devices and one with
> LED multicolor class device.
>
ACK
> > + };
> > +...
> >
>
> --
> Best regards,
> Jacek Anaszewski
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-11-18 10:39 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-17 10:55 [PATCH v6 0/2] leds: mt6360: Add LED driver for MT6360 Gene Chen
2020-11-17 10:55 ` Gene Chen
2020-11-17 10:55 ` Gene Chen
2020-11-17 10:55 ` [PATCH v6 1/2] dt-bindings: leds: Add bindings for MT6360 LED Gene Chen
2020-11-17 10:55 ` Gene Chen
2020-11-17 10:55 ` Gene Chen
2020-11-17 22:24 ` Jacek Anaszewski
2020-11-17 22:24 ` Jacek Anaszewski
2020-11-17 22:24 ` Jacek Anaszewski
2020-11-18 10:38 ` Gene Chen [this message]
2020-11-18 10:38 ` Gene Chen
2020-11-18 10:38 ` Gene Chen
2020-11-17 10:55 ` [PATCH v6 2/2] leds: mt6360: Add LED driver for MT6360 Gene Chen
2020-11-17 10:55 ` Gene Chen
2020-11-17 10:55 ` Gene Chen
2020-11-17 22:25 ` Jacek Anaszewski
2020-11-17 22:25 ` Jacek Anaszewski
2020-11-17 22:25 ` Jacek Anaszewski
2020-11-18 10:18 ` Gene Chen
2020-11-18 10:18 ` Gene Chen
2020-11-18 10:18 ` Gene Chen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAE+NS345g6gokjpBn0G9uR2azA9p2ZCgujC2cM5pWmrmmwqYjA@mail.gmail.com \
--to=gene.chen.richtek@gmail.com \
--cc=Wilma.Wu@mediatek.com \
--cc=benjamin.chao@mediatek.com \
--cc=cy_huang@richtek.com \
--cc=devicetree@vger.kernel.org \
--cc=dmurphy@ti.com \
--cc=gene_chen@richtek.com \
--cc=jacek.anaszewski@gmail.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-leds@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=matthias.bgg@gmail.com \
--cc=pavel@ucw.cz \
--cc=robh+dt@kernel.org \
--cc=shufan_lee@richtek.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.