[U-Boot] [PATCH v3 3/7] [REPOST-1] gpio: Add Multi-Function-Pin configuration driver for Marvell SoCs

[Wolfgang Denk]

Dear Prafulla Wadaskar,

In message <F766E4F80769BD478052FB6533FA745D19A9291DBC at SC-VEXCH4.marvell.com> you wrote:
> 
> > Since the mfp.c is for generic case, and should be made as generic at
> > the beginning...
> 
> Sure.. but I don't know how it will be on other SoCs?
> Apart from that what would be other impact?
> So in my opinion, let's keep it for future updates.

No, this is the wrong approach.

Already now you realize that this code does not reallys cale well with
future extensions, because you put all the logic in your code.

I think this is a bad approach here.

Instead, make the whole thing data driven - implement only minimal
code that waks through a table provided by the user.  Instead of
putting the logic in the code, try to put it in the data.

> With reference to your precise concern, I will create a macro in asm/arch/mfp.h
> moving below code there.
>
> +  if ( mfpr_no < 37)
> +   p_mfpr += (0x004c / 4) + mfpr_no;
> +  else if ( mfpr_no >= 56)
> +   p_mfpr += (0x00e0 / 4) + (mfpr_no - 56);
> +  else
> +   p_mfpr += (mfpr_no - 37)

No, please don't.  It's ugly, not readable and not maintainable.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
You might not be as stupid as you look. This is not hard. Let's think
about this. I mean ... I'll think about this, and  you  can  join  in
when you know the words.             - Terry Pratchett, _Men at Arms_