On Thu, Nov 21, 2024 at 06:10:00PM +0100, Vicentiu Galanopulo wrote:
> On Mon, Nov 18, 2024 at 09:13:32PM +0100, Christophe JAILLET wrote:
> > Le 18/11/2024 à 15:54, Vicentiu Galanopulo a écrit :
> > > The output current can be adjusted separately for each channel by 8-bit
> > > analog (current sink input) and 12-bit digital (PWM) dimming control. The
> > > LED1202 implements 12 low-side current generators with independent dimming
> > > control.
> > > Internal volatile memory allows the user to store up to 8 different patterns,
> > > each pattern is a particular output configuration in terms of PWM
> > > duty-cycle (on 4096 steps). Analog dimming (on 256 steps) is per channel but
> > > common to all patterns. Each device tree LED node will have a corresponding
> > > entry in /sys/class/leds with the label name. The brightness property
> > > corresponds to the per channel analog dimming, while the patterns[1-8] to the
> > > PWM dimming control.
> > >
> > > Signed-off-by: Vicentiu Galanopulo <vicentiu.galanopulo@xxxxxxxxxxxxxxxxx>
> > ...
Sorry for resending this, I pasted wrong in the previous one and didn't notice
> > > +{
> > > + return value/ST1202_MILLIS_PATTERN_DUR_MIN - 1;
> >
> > Can value be 0?
> > If yes, should the return value be clamped?
>
No, it's called in st1202_duration_pattern_write, which is mostly a wrapper,
called in 2 places:
line 225: ret = st1202_duration_pattern_write(chip, patt, ST1202_MILLIS_PATTERN_DUR_MIN);
line 258: ret = st1202_duration_pattern_write(chip, patt, pattern[patt].delta_t);
for line 258, I make sure in line 250:
if (pattern[patt].delta_t < ST1202_MILLIS_PATTERN_DUR_MIN ||
pattern[patt].delta_t > ST1202_MILLIS_PATTERN_DUR_MAX)
return -EINVAL;
that it respects the range provided by the datasheet and supported by the chip.
>
> >
> > Is it needed?
> > (There is no i2c_get_clientdata())
>
Indeed, not needed so removed.
>
>
> > > +
> >
> > CJ
>
Thank you very much for thanking the time to review!
Kindest regards,
Vicentiu
