From: Jacek Anaszewski <jacek.anaszewski@gmail.com>
To: Simon Shields <simon@lineageos.org>, linux-leds@vger.kernel.org
Cc: Richard Purdie <rpurdie@rpsys.net>, Pavel Machek <pavel@ucw.cz>,
devicetree@vger.kernel.org, Rob Herring <robh+dt@kernel.org>
Subject: Re: [PATCH 1/2] dt-bindings: leds: document Panasonic AN30259A bindings
Date: Wed, 21 Feb 2018 20:35:22 +0100 [thread overview]
Message-ID: <e0e0f999-bb14-6f08-4cc8-2c4936b04ed0@gmail.com> (raw)
In-Reply-To: <affb0dcc-05a0-20f5-4caf-528453b59fb6@gmail.com>
On 02/20/2018 10:28 PM, Jacek Anaszewski wrote:
> Hi Simon,
>
> Thank you for the patch. I have a few comments below.
>
> On 02/20/2018 01:54 AM, Simon Shields wrote:
>> Signed-off-by: Simon Shields <simon@lineageos.org>
>> ---
>> .../devicetree/bindings/leds/leds-an30259a.txt | 41 ++++++++++++++++++++++
>> 1 file changed, 41 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/leds/leds-an30259a.txt
>>
>> diff --git a/Documentation/devicetree/bindings/leds/leds-an30259a.txt b/Documentation/devicetree/bindings/leds/leds-an30259a.txt
>> new file mode 100644
>> index 000000000000..d3586ce71eef
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/leds/leds-an30259a.txt
>> @@ -0,0 +1,41 @@
>> +* Panasonic AN30259A 3-channel LED driver
>> +
>> +The AN30259A is a LED controller capable of driving three LEDs independently. It supports
>> +constant current output and sloping current output modes. The chip is connected over I2C.
>> +
>> +Required properties:
>> + - compatible : must be "panasonic,an30259a"
>
> s/must/Must/ and please put a dot in the end.
>
>> + - reg - I2C slave address
>> +
>> +Each led is represented as a sub-node of the panasonic,an30259a node.
>
> s/led/LED/
>
>> +
>> +Optional sub-node properties:
>> + - label: see Documentation/devicetree/bindings/leds/common.txt
>> + - linux,default-trigger: see Documentation/devicetree/bindings/leds/common.txt
>> + - led-sources: 0 if LED is connected to LED1 pin,
>
> led-sources is applicable in e bit different arrangements.
> Here please use "reg" property instead. In this case you will
> need also below in the "Required properties":
>
> - #address-cells: Must be 1.
> - #size-cells: Must be 0.
>
>
>> + 1 if LED is connected to LED2 pin, or 2 if LED is connected to LED3 pin.
>> +
>> +Example:
>> +an30259a@30 {
>
> According to the lesson from Rob [0] it should be:
>
> led-controller@30 {
> #address-cells = <1>;
> #size-cells = <0>
> reg = <30>;
>
> led0 {
> reg = <0>;
> label = "red";
> };
>
> green@1 {
> reg = <1>;
> label = "green";
> };
>
> blue@2 {
> reg = <2>;
> label = "blue";
> };
> };
Of course s/green@1/led@1/ and s/blue@2/led@2/
Moreover, label should describe LED function, color
is only an extra info. Please refer to the other LED bindings
for a reference.
>
>> + compatible = "panasonic,an30259a";
>> + reg = <0x30>;
>> +
>> + red {
>> + led-sources = <0>;
>> + linux,default-trigger = "heartbeat";
>> + label = "red";
>> + };
>> +
>> + green {
>> + led-sources = <1>;
>> + label = "green";
>> + };
>> +
>> + blue {
>> + led-sources = <2>;
>> + label = "blue";
>> + };
>> +};
>> +
>> +For more information please see the link below:
>> +https://www.alliedelec.com/m/d/a9d2b3ee87c2d1a535a41dd747b1c247.pdf
>>
>
> [0] https://patchwork.kernel.org/patch/10093757/
>
--
Best regards,
Jacek Anaszewski
next prev parent reply other threads:[~2018-02-21 19:35 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-20 0:54 [PATCH 0/2] Panasonic AN30259A support Simon Shields
2018-02-20 0:54 ` [PATCH 1/2] dt-bindings: leds: document Panasonic AN30259A bindings Simon Shields
2018-02-20 21:28 ` Jacek Anaszewski
2018-02-21 19:35 ` Jacek Anaszewski [this message]
2018-02-21 12:25 ` Pavel Machek
2018-02-20 0:54 ` [PATCH 2/2] leds: add Panasonic AN30259A support Simon Shields
2018-02-20 21:51 ` Jacek Anaszewski
2018-02-21 14:18 ` Simon Shields
2018-02-21 19:55 ` Jacek Anaszewski
2018-02-21 12:22 ` Pavel Machek
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=e0e0f999-bb14-6f08-4cc8-2c4936b04ed0@gmail.com \
--to=jacek.anaszewski@gmail.com \
--cc=devicetree@vger.kernel.org \
--cc=linux-leds@vger.kernel.org \
--cc=pavel@ucw.cz \
--cc=robh+dt@kernel.org \
--cc=rpurdie@rpsys.net \
--cc=simon@lineageos.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).