Re: [PATCH 2/2] leds: Add NCP5623 multi-led driver

[Abdel Alkuor]

On Thu, Feb 08, 2024 at 01:01:15PM +0000, Lee Jones wrote:
> On Sat, 03 Feb 2024, Abdel Alkuor wrote:
>
Hi Lee,

Please check the inline comment. All other comments will be addressed
in v2.

> > +What:		/sys/class/leds/<led>/dim_step
> 
> The step principle seems a bit arbitrary.
> 
> Why not provide the time directly?
> 
> dim_step_delay?
> 
> I already see documentation for risetime and falltime.
> 
> Perhaps that will omit the need for both direction and step?
>
I'm going to drop off both and use risetime and falltime. That being
said, the documented risetime and falltime for lm3533 use steps instead of
entering the time directly. This is my first time doing this, should I document
risetime/falltime in sysfs-class-led-multicolor-driver-ncp5623? or should
I update risetime/falltime in sysfs-class-led-driver-lm3533 to reflect
risetime/falltime for ncp5623?
> > +Date:		Feb 2024
> > +KernelVersion:	6.8
> > +Contact:	Abdel Alkuor <alkuor@xxxxxxxxx>
> > +Description:
> > +		Set gradual dimming time.
> > +
> > +		==== ======== ==== ======== ==== ========
> > +		Step Time(ms) Step Time(ms) Step Time(ms)
> > +		0     0       11   88       22   176
> > +		1     8       12   96       23   184
> > +		2     16      13   104      24   192
> > +		3     24      14   112      25   200
> > +		4     32      15   120      26   208
> > +		5     40      16   128      27   216
> > +		6     48      17   136      28   224
> > +		7     56      18   144      29   232
> > +		8     64      19   152      30   240
> > +		9     72      20   160      31   248
> > +		10    80      21   168
> > +		==== ======== ==== ======== ==== ========

Thanks,
Abdel