On Wed, Jun 7, 2023 at 2:44 PM jerome Neanne <jneanne@xxxxxxxxxxxx> wrote:
...
> >> + enum {
> >> + MULTI_BUCK12,
> >> + MULTI_BUCK123,
> >> + MULTI_BUCK1234,
> >> + MULTI_BUCK12_34,
> >
> >> + MULTI_FIRST = MULTI_BUCK12,
> >> + MULTI_LAST = MULTI_BUCK12_34,
> >> + MULTI_NUM = MULTI_LAST - MULTI_FIRST + 1
> > MULT_NUM
> >
> > will suffice instead of all this.
(1)
> >> + };
> >
> > But why enum at all? See below.
> Just for the switch case readability.
> I have to iterate across the multiphases array for look up name into
> device tree and evaluate in that order.
>
> This can be reduced to:
> enum {
> MULTI_BUCK12,
> MULTI_BUCK123,
> MULTI_BUCK1234,
> MULTI_BUCK12_34,
> MULTI_NUM = MULTI_BUCK12_34 - MULTI_BUCK12 + 1
See (1) above.
> };
...
> >> + continue;
> >> + delta = strcmp(npname, multiphases[multi]);
> >> + if (!delta) {
> >> + switch (multi) {
> >> + case MULTI_BUCK12:
> >
> > This all looks like match_string() reinvention.
> I can go with match_string but this is not significantly changing the game:
>
> index = match_string(multiphases, ARRAY_SIZE(multiphases), npname);
> if (index >= 0) {
> switch (index) {
>
> No question on all your other feedback. Just wondering if I missed
> something with match_string use. Looks like a good idea indeed but this
> is not drastically changing the code as you seem to expect... Let me
> know if you think I'm doing it in a wrong way.
I guess the entire big for-loop can be optimized, but I haven't looked
at that. At least match_string() would help understanding what you are
trying to do,
In any case it seems Mark applied your version, so the follow ups can be made.
--
With Best Regards,
Andy Shevchenko
