> -----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> -- 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
