[U-Boot] [PATCH 3/4] efikamx: update to Efika MX Smarttop and Smartbook boards

[Matt Sealey]

On Fri, Aug 17, 2012 at 2:29 PM, Benoît Thébaudeau
<benoit.thebaudeau at advansee.com> wrote:
> Dear Matt Sealey,
>>
>> * Use iomux-mx51.h include to simplify board configuration.
>> * Remove LED toggle function as it had no real users.
>> * Red LED is now on for pre-reloc, Blue LED for "in U-Boot"
>> * Function renames for readability.
>> * Some board identification string changes
>> * Reduce CPU core voltage to 1.1V (per TO3 spec)
>> * Implicitly remove support for TO2 boards
>
> These are many unrelated changes that could be split.

Could be, but they're not going to be, the reasoning behind it being
that it increases work for us to test each individual patch works in
any particular order it may be committed to any tree, and increases
work for patch maintainers whereby there are too many inter-dependent
changes, extra fuzz, rebasing etc. which is just totally needless. In
essence this is a cleanup patch intended to get rid of some things
that made absolutely no sense but have persisted upstream for the
longest time. It also ports to iomux-mx51.h as previously sent since
that helps a lot on cleanup.

We tested every single part of this as a unit internally over 10
weeks, what's being sent to the list is the result. What isn't in the
patch (or more accurately, what got removed) was code that would make
more of a mess for very little functionality, code that was repeated
or overcomplicated for no good reason (led setup for example) or was
over-abstracted.

What went in to the process was the old code: functionality was tested
and verified against expectations. What you get from the new code:
almost exactly the same behavior (actually it works a little better as
some redundant things were removed).

The sign-off is not just me hitting "-s" on my git command line, it's
my explicit assurance that this is highly tested code that I am
willing to maintain and answer questions on. Marex has been asking me
to submit it for a long, long time and I refused because we were still
going through QA; that got finished, to a level I am happy with at
least (still a few weird things happening but they existed with the
old code too). However, QA got done, and the result is clear, and
splitting it up means going through QA again just to respin patches,
so I'm going to refuse on that basis alone. If we just consider it a
cleanup patch for the board and a reasonable example of the benefits
of the new iomux model code, does that make it easier to accept?

>> Signed-off-by: Matt Sealey <matt at genesi-usa.com>

[snip]

>> +     MX51_PAD_CSPI1_SS1__GPIO4_25,
>> +     MX51_PAD_GPIO1_6__GPIO1_6,
>> +};
>
> Why don't you merge all these pad settings to a single board pad array, with a
> single call to imx_iomux_v3_setup_multiple_pads()?

That has been done before (it was the way we did it in our ancient
Linux kernels, it was the way Freescale did it in their ancient Linux
kernels). We feel a huge board-specific array is hard to maintain and
ugly to read, and the few differences would end up in tiny, separate
arrays anyway. The U-Boot code was written that way in the first
place, it made a lot of sense to keep the status quo for now. My
preferred solution would be to split each part into a seperate source
file and call them from the board file (as efikamx-usb.c is done now,
but for MMC, ATA too) where such code would clean up the main board
file significantly, but I think that would be too big a change for a
lot of people. We can perform a more egregious cleanup later.

-- 
Matt Sealey <matt at genesi-usa.com>
Product Development Analyst, Genesi USA, Inc.