Hi Pavel, On Wed, Mar 07, 2018 at 11:07:25PM +0100, Pavel Machek wrote: > Hi! > > > The datasheet is freely available: > > https://www.alliedelec.com/m/d/a9d2b3ee87c2d1a535a41dd747b1c247.pdf > > > + To compile this driver as a module, choose M here: the module > > + will be called leds-an30259a. > > leds-an30259a.ko? The vast majority of Kconfig options omit the .ko - a treewide grep shows only 40 including the .ko. > > > @@ -0,0 +1,382 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > I'd prefer 2.0+, but... your choice. I really don't mind. > > > +/* > > + * Driver for Panasonic AN30259A 3-channel LED driver > > + * > > + * Copyright (c) 2018 Simon Shields <simon@xxxxxxxxxxxxx> > > + * > > + * Datasheet: > > + * https://www.alliedelec.com/m/d/a9d2b3ee87c2d1a535a41dd747b1c247.pdf > > + */ > > +#include <linux/i2c.h> > > +#include <linux/leds.h> > > I'd do empty line after the comment. > > > +static u8 an30259a_get_dutymax(u8 brightness) > > +{ > > + u8 duty_max, floor, ceil; > > + > > + /* squash 8 bit number into 7-bit PWM range */ > > + duty_max = brightness >> 1; > > We should really start using max_brightness, so that userspace knows > what When outputting constant brightness, the IC supports 8-bit PWM. For sloped output mode, it's limited to 7 bits, and the bottom 3 bits are always set. > > > + /* bottom 3 bits are always set for DUTYMAX > > + * figure out the closest value > > + */ > > /* > * Multiline comments like this. > */ > Oops. > > + /* slope time controls on/off cycle length */ > > + ret = regmap_write(led->chip->regmap, REG_SLOPE(num), > > + LED_SLOPETIME1(off) | LED_SLOPETIME2(on)); > > + if (ret) > > + return ret; > > + > > + /* Finally, enable slope mode. */ > > And yes, having sentences in all the comments would be nice. Or maybe > nowhere. But consistent ;-). > > Since all I'm commenting is whitespace... > > Acked-by: Pavel Machek <pavel@xxxxxx> > Thanks for the review! > -- > (english) http://www.livejournal.com/~pavelmachek > (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html Cheers, Simon -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
