lkml.org 
[lkml]   [2023]   [Jan]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v2 2/2] leds: lp55xx: configure internal charge pump
From

>>
>> + pdata->charge_pump_mode = LP55XX_CP_AUTO;
>> + ret = of_property_read_string(np, "ti,charge-pump-mode", &pm);
>> + if (!ret) {
>> + for (cp_mode = LP55XX_CP_OFF;
>> + cp_mode < ARRAY_SIZE(charge_pump_modes);
>> + cp_mode++) {
>> + if (!strcasecmp(pm, charge_pump_modes[cp_mode])) {
>> + pdata->charge_pump_mode = cp_mode;
>> + break;
>> + }
>> + }
>> + }
> A little over-engineered, no?
>
> Why not make the property a numerical value, then simply:
>
> ret = of_property_read_u32(np, "ti,charge-pump-mode", &pdata->charge_pump_mode);
> if (ret)
> data->charge_pump_mode = LP55XX_CP_AUTO;
>
> Elevates the requirement for the crumby indexed array of strings above.
>
> Remainder looks sane enough.

Thanks for your feedback.

I won't argue that your implementation isn't far more simple. The idea
was to have an elaborate and clear and obvious devicetree, but that can
also be achieved by moving constants into
/includes/dt-bindings/leds/leds-lp55xx.h. Would that be more acceptable?

Maarten

\
 
 \ /
  Last update: 2023-03-26 23:49    [W:0.030 / U:0.924 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site