On Fri, Oct 30, 2015 at 12:10 PM, York Sun yorksun@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@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