On Thu, Apr 25, 2013 at 4:34 PM, Kim, Milo <Milo.Kim@xxxxxx> wrote: >> -----Original Message----- >> From: linux-leds-owner@xxxxxxxxxxxxxxx [mailto:linux-leds- >> owner@xxxxxxxxxxxxxxx] On Behalf Of Bryan Wu >> Sent: Friday, April 26, 2013 7:26 AM >> To: Linus Walleij; Kim, Milo >> Cc: Richard Purdie; Linux LED Subsystem; Linus Walleij; Gabriel >> Fernandez >> Subject: Re: [PATCH] leds: lp55xx: add support for Device Tree bindings >> >> On Tue, Apr 23, 2013 at 4:52 AM, Linus Walleij >> <linus.walleij@xxxxxxxxxxxxxx> wrote: >> > From: Linus Walleij <linus.walleij@xxxxxxxxxx> >> > >> > This patch allows the lp5521 driver to be successfully probed and >> > initialised when Device Tree support is enabled. >> > >> > Based on a patch by Gabriel Fernandez, rewritten in accordance >> > with review feedback. >> > >> > Cc: Gabriel Fernandez <gabriel.fernandez@xxxxxxxxxxxxxx> >> > Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx> >> > --- >> > ChangeLog v1->v2: >> > - Move DT parsing into the lp55xx-common code. >> > - Implement probing for lp5523 as well. >> > - Handle errors from all of_property_read* functions. >> > - Rename num-chanel to num-channel >> > - Prefix all compatible strings with "national,*" like >> > "national,lp5521", a bit uncertain about this but seems >> > like the right thing to do. >> > - Since I have now written more than half the patch I have >> > set myself as author so as not to blame Gabriel for all my >> > mistakes. >> > --- >> > .../devicetree/bindings/leds/leds-lp55xx.txt | 21 +++++++++ >> > drivers/leds/leds-lp5521.c | 20 ++++++-- >> > drivers/leds/leds-lp5523.c | 19 ++++++-- >> > drivers/leds/leds-lp55xx-common.c | 54 >> ++++++++++++++++++++++ >> > drivers/leds/leds-lp55xx-common.h | 4 ++ >> > 5 files changed, 108 insertions(+), 10 deletions(-) >> > create mode 100644 Documentation/devicetree/bindings/leds/leds- >> lp55xx.txt >> > >> > diff --git a/Documentation/devicetree/bindings/leds/leds-lp55xx.txt >> b/Documentation/devicetree/bindings/leds/leds-lp55xx.txt >> > new file mode 100644 >> > index 0000000..348c88e >> > --- /dev/null >> > +++ b/Documentation/devicetree/bindings/leds/leds-lp55xx.txt >> > @@ -0,0 +1,21 @@ >> > +Binding for National Semiconductor LP55xx Led Drivers >> > + >> > +Required properties: >> > +- compatible: "national,lp5521" or "national,lp5523" >> > +- label: Used for naming LEDs >> > +- num-channel: Number of LED channels >> > +- led-cur: Current setting at each led channel (mA x10, 0 if led is >> not connected) >> > +- max-cur: Maximun current at each led channel. >> > +- clock-mode: Input clock mode, (0: automode, 1: internal, 2: >> external) >> > + >> > +example: >> > + >> > +lp5521@32 { >> > + compatible = "national,lp5521"; >> > + reg = <0x32>; >> > + label = "lp5521_pri"; >> > + num-channel = /bits/ 8 <3>; >> > + led-cur = /bits/ 8 <0x2f 0x2f 0x2f>; >> > + max-cur = /bits/ 8 <0x5f 0x5f 0x5f>; >> > + clock-mode = /bits/8 <2>; >> I guess we missed a space between '/bits/' and '8' here. >> >> And this patch looks fine to me. Milo, can you review this and try it >> on your hardware? > > It looks good to me, either. Just two minor points here. > One is the name of vendor. I would suggest name changes from 'national' to 'ti'. > The other is the size of arrays, current configuration. The size is fixed as 3. > LP5521 has max tree channels, so no problem with this device. > However, LP5523 has max 9 channels and LP5562 has 4 channels. > The size can be a problem. I can fix the DT later. > So, I add my ACK here. > > Acked-by: Milo Kim <milo.kim@xxxxxx> OK, I'm going to merge it into my -devel branch for 3.11 merge window. Thanks, -Bryan -- To unsubscribe from this list: send the line "unsubscribe linux-leds" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
