From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751317AbcFVIz1 (ORCPT ); Wed, 22 Jun 2016 04:55:27 -0400 Received: from mailout1.w1.samsung.com ([210.118.77.11]:50770 "EHLO mailout1.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750799AbcFVIzX (ORCPT ); Wed, 22 Jun 2016 04:55:23 -0400 X-AuditID: cbfec7f4-f796c6d000001486-37-576a51ec8191 Message-id: <576A51EB.8090800@samsung.com> Date: Wed, 22 Jun 2016 10:52:59 +0200 From: Jacek Anaszewski User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130804 Thunderbird/17.0.8 MIME-version: 1.0 To: Florian Vaussard Cc: devicetree@vger.kernel.org, Richard Purdie , Rob Herring , Mark Rutland , linux-leds@vger.kernel.org, linux-kernel@vger.kernel.org, Florian Vaussard Subject: Re: [PATCH 1/2] leds: ncp5623: Add device tree binding documentation References: <1466494154-3786-1-git-send-email-florian.vaussard@heig-vd.ch> <1466494154-3786-2-git-send-email-florian.vaussard@heig-vd.ch> <57695D02.2000109@samsung.com> In-reply-to: <57695D02.2000109@samsung.com> Content-type: text/plain; charset=ISO-8859-1; format=flowed Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrGLMWRmVeSWpSXmKPExsVy+t/xK7pvA7PCDRZKW8w/co7VYvKhxewW r7ZbW1zeNYfNYuubdYwWS69fZLJo3XuE3WL3rqesDhwea+atYfTYOesuu8fTdf0sHptWdbJ5 7Jn/g9Xj8ya5ALYoLpuU1JzMstQifbsEroz2nsXsBXskKtq2b2FtYPwo1MXIySEhYCJx+d5X JghbTOLCvfVsXYxcHEICSxklps4+ww7hPGOUmLblNgtIFa+AlsT5pXNZQWwWAVWJhztvgnWz CRhK/HzxGswWFYiQ+HN6HytEvaDEj8n3wHpFgGoa3x4GG8os8JNRYtPka2BFwgK+Eh/fL2GG 2LaGUeLd7N2MIAlOAW2J70sng3UzC1hLrJy0jRHClpfYvOYt8wRGgVlIlsxCUjYLSdkCRuZV jKKppckFxUnpuYZ6xYm5xaV56XrJ+bmbGCGh/2UH4+JjVocYBTgYlXh4Gboyw4VYE8uKK3MP MUpwMCuJ8Br6ZIUL8aYkVlalFuXHF5XmpBYfYpTmYFES5527632IkEB6YklqdmpqQWoRTJaJ g1OqgTGtIN9ujrTaeU1JLRb7+lMNIYcitrxIffjM6UB43iNegbBk7YmCkY1HD4q+rr97/7R0 gXFyk8qMlU5Vcn8NFi2tdlrlu+HD4YMpAeG7L7g/2XTqmVxVptWUgt7JS2ucpA9t3Tj3wbq/ lQHyPZHtNZ3nAy/OX/vw0jybSzd2vXpwMijkPNPqcwpKLMUZiYZazEXFiQAjXWr+eQIAAA== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 06/21/2016 05:28 PM, Jacek Anaszewski wrote: > Hi Florian, > > Thanks for the patch. I have two remarks below. > > On 06/21/2016 09:29 AM, Florian Vaussard wrote: >> Add device tree binding documentation for On Semiconductor NCP5623 I2C >> LED driver. The driver can independently control the PWM of the 3 >> channels with 32 levels of intensity. >> >> The current delivered by the current source can be controlled using the >> led-max-microamp property. In order to control this value, it is also >> necessary to know the current on the Iref pin, hence the >> onnn,led-iref-microamp property. It is usually set using an external >> bias resistor, following Iref = Vref/Rbias with Vref=0.6V. >> >> Signed-off-by: Florian Vaussard >> --- >> .../devicetree/bindings/leds/leds-ncp5623.txt | 44 >> ++++++++++++++++++++++ >> 1 file changed, 44 insertions(+) >> create mode 100644 >> Documentation/devicetree/bindings/leds/leds-ncp5623.txt >> >> diff --git a/Documentation/devicetree/bindings/leds/leds-ncp5623.txt >> b/Documentation/devicetree/bindings/leds/leds-ncp5623.txt >> new file mode 100644 >> index 0000000..0dc8345 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/leds/leds-ncp5623.txt >> @@ -0,0 +1,44 @@ >> +* ON Semiconductor - NCP5623 3-Channel LED Driver >> + >> +The NCP5623 is a 3-channel I2C LED driver. The brightness of each >> +channel can be independently set using 32 levels. Each LED is >> represented >> +as a sub-node of the device. >> + >> +Required properties: >> + - compatible: Should be "onnn,ncp5623" >> + - reg: I2C slave address (fixed to 0x38) >> + - #address-cells: must be 1 >> + - #size-cells: must be 0 >> + - onnn,led-iref-microamp: Current on the Iref pin in microampere > > I think that you don't need this property. Just provide the formula for > calculating led-max-microamp value, similarly as you're doing that in > the commit message. > >> + - led-max-microamp: Desired maximum current for each LED in >> microampere >> + (maximum 30000uA) > > Please add instead of (maximum ...): > > Valid values: min - max, step by N (rounded {up|down}) > > E.g. > > Valid values: 10000 - 30000, step by 1000 (rounded down) > >> + >> +LED sub-node properties: >> + - reg : LED channel number (0..2) >> + - For other LED properties see: >> + Documentation/devicetree/bindings/leds/common.txt >> + >> +Example: >> + >> +led1: ncp5623@38 { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + compatible = "onnn,ncp5623"; >> + reg = <0x38>; >> + >> + led1_r@0 { >> + label = "ncp:power:red"; >> + reg = <0>; >> + linux,default-trigger = "default-on"; Please add also led-max-microamp to all child nodes in this example. >> + }; >> + >> + led1_b@1 { >> + label = "ncp:power:blue"; >> + reg = <1>; >> + }; >> + >> + led1_g@2 { >> + label = "ncp:power:green"; >> + reg = <2>; >> + }; >> +}; >> > > -- Best regards, Jacek Anaszewski