Re: [PATCH v3 2/2] leds: add aw20xx driver

[Martin Kurbanov <mmkurbanov@sberdevices.ru>]

Hello Andy. Thank you for review.
I have fixed most of your comments. Please take a look below.

On 2023-03-01 00:51, Andy Shevchenko wrote:
>> +       /* The output current of each LED (see p.14 of datasheet for formula) */
>> +       return (duty * global_imax_microamp) / 1000U;
> 
> units.h ?

These constants are needed to improve the accuracy of calculations.
units.h doesn’t have any helpful definitions to use here.

>> +static int aw200xx_set_imax(const struct aw200xx *const chip,
>> +                           u32 led_imax_microamp)
>> +{
>> +       struct imax_global {
>> +               u32 regval;
>> +               u32 microamp;
>> +       } imaxs[] = {
>> +               { 8,  3300 },
>> +               { 9,  6700 },
>> +               { 0,  10000 },
>> +               { 11, 13300 },
>> +               { 1,  20000 },
>> +               { 13, 26700 },
>> +               { 2,  30000 },
>> +               { 3,  40000 },
>> +               { 15, 53300 },
>> +               { 4,  60000 },
>> +               { 5,  80000 },
>> +               { 6,  120000 },
>> +               { 7,  160000 },
> 
> This looks a bit random. Is there any pattern on how value is
> connected to the register value?

There is no ability to create any pattern here, because this table data
doesn’t have any regularity.


-- 
Best Regards,
Kurbanov Martin