[U-Boot] [PATCH 2/2] mpc85xx: Add support for the Varisys Cyrus board

[Andy Fleming]

On Fri, Oct 30, 2015 at 12:10 PM, York Sun <yorksun at freescale.com> wrote:
>
>
> On 10/21/2015 04:59 PM, Andy Fleming wrote:
>> This board runs a P5020 or P5040 chip, and utilizes
>> an EEPROM with similar formatting to the Freescale P5020DS.
>>
>> Large amounts of this code were developed by
>> Adrian Cox <adrian at humboldt dot co dot uk>
>>
>> Signed-off-by: Andy Fleming <afleming at gmail.com>
>> ---
>>  arch/powerpc/cpu/mpc85xx/Kconfig     |   4 +
>>  board/varisys/common/Makefile        |  23 ++
>>  board/varisys/common/eeprom.h        |   6 +
>>  board/varisys/common/sys_eeprom.c    | 605 +++++++++++++++++++++++++++++++++++
>>  board/varisys/cyrus/Kconfig          |  13 +
>>  board/varisys/cyrus/Makefile         |   8 +
>>  board/varisys/cyrus/README           |  21 ++
>>  board/varisys/cyrus/cyrus.c          | 116 +++++++
>>  board/varisys/cyrus/cyrus.h          |  11 +
>>  board/varisys/cyrus/ddr.c            | 188 +++++++++++
>>  board/varisys/cyrus/eth.c            | 100 ++++++
>>  board/varisys/cyrus/law.c            |  27 ++
>>  board/varisys/cyrus/pbi.cfg          |  35 ++
>>  board/varisys/cyrus/pci.c            |  23 ++
>>  board/varisys/cyrus/rcw_p5020_v2.cfg |  11 +
>>  board/varisys/cyrus/rcw_p5040.cfg    |  11 +
>>  board/varisys/cyrus/tlb.c            | 106 ++++++
>>  configs/Cyrus_P5020_defconfig        |   9 +
>>  configs/Cyrus_P5040_defconfig        |   9 +
>>  include/configs/cyrus.h              | 590 ++++++++++++++++++++++++++++++++++
>>  20 files changed, 1916 insertions(+)
>>  create mode 100644 board/varisys/common/Makefile
>>  create mode 100644 board/varisys/common/eeprom.h
>>  create mode 100644 board/varisys/common/sys_eeprom.c
>>  create mode 100644 board/varisys/cyrus/Kconfig
>>  create mode 100644 board/varisys/cyrus/Makefile
>>  create mode 100644 board/varisys/cyrus/README
>>  create mode 100644 board/varisys/cyrus/cyrus.c
>>  create mode 100644 board/varisys/cyrus/cyrus.h
>>  create mode 100644 board/varisys/cyrus/ddr.c
>>  create mode 100644 board/varisys/cyrus/eth.c
>>  create mode 100644 board/varisys/cyrus/law.c
>>  create mode 100644 board/varisys/cyrus/pbi.cfg
>>  create mode 100644 board/varisys/cyrus/pci.c
>>  create mode 100644 board/varisys/cyrus/rcw_p5020_v2.cfg
>>  create mode 100644 board/varisys/cyrus/rcw_p5040.cfg
>>  create mode 100644 board/varisys/cyrus/tlb.c
>>  create mode 100644 configs/Cyrus_P5020_defconfig
>>  create mode 100644 configs/Cyrus_P5040_defconfig
>>  create mode 100644 include/configs/cyrus.h
>>
>
> Andy,
>
> I presume you have examined the difference between
> board/varisys/common/sys_eeprom.c and the original file
> board/freescale/common/sys_eeprom.c. Is it possible to reuse the existing file?
> This is a 600+ lines copy-n-paste.

This code presents some challenges. The code is nearly identical to
the Freescale code, and quite unnecessarily so. If I were working at
Varisys, I would recommend that we pare down this code to only what we
need. As I am not, and as I presume that some amount of eeprom
programming may be done in manufacturing, I don't feel comfortable
changing this code (other than to perhaps remove the MPC85xx-specific
stuff I missed).

Sadly, while it is nearly identical to the Freescale code, there are a
number of differences. This code uses NXID version 0, which has been
removed from the FSL code. It also supports an alternate mechanism for
reading the eeprom (mac_read_from_generic_eeprom).

I can imagine some ways I might refactor the code to ensure that it
works for both FSL and Varisys systems, but this raises the issue that
these files implement two completely separate APIs which are only
nearly-identical in current implementation. Freescale could decide to
change the format of their eeproms, and provide scripts to any
affected customers to shift them to the new format (they have before).
Freescale could discover that manufacturing made a mistake in the
formatting, and submit code to accommodate this. Likewise, Varisys
could decide that the current format is more than they need, or that
it's redundant with some other board mechanism. In such cases,
unifying this code would actually make code maintenance *more*
difficult for the two companies.

I did initially intend to refactor the code, but changed my mind when
I realized how difficult it would be to make sure I hadn't broken
anyone else's board. If you think it's necessary, I would probably
just try to strip down the varisys version of the file to the bare
essentials. I'm just not sure which parts *are* essential at the
moment.

Andy