On 11/04/2015 04:00 PM, Steve Kipisz wrote: > Several TI EVMs have onboard EEPROM that contain board description > information. The onboard EEPROM on Beaglebone, Beaglebone Black, AM335x > EVM, AM43x EVM, AM57xx EVM, Beagleboard-x15 all share the same format. > > This series of patches introduces code which is generic among these > platforms. The boards can use the data for any operations they might > choose. > > Lokesh Vutla (1): > ARM: omap-common: Add standard access for board description EEPROM > > Steve Kipisz (4): > ARM: OMAP4/5: Centralize early clock initialization > ARM: OMAP4/5: Centralize gpi2c_init > ARM: OMAP4/5: Add generic board detection hook > board: ti: AM57xx: Add detection logic for AM57xx-evm > > arch/arm/cpu/armv7/omap-common/clocks-common.c | 21 +++- > arch/arm/cpu/armv7/omap-common/hwinit-common.c | 14 ++- > arch/arm/include/asm/arch-omap4/sys_proto.h | 4 +- > arch/arm/include/asm/arch-omap5/sys_proto.h | 4 +- > arch/arm/include/asm/omap_common.h | 8 +- > board/ti/am57xx/Makefile | 2 + > board/ti/am57xx/board.c | 52 +++++++++ > board/ti/common/board.c | 54 +++++++++ > board/ti/common/board.h | 117 +++++++++++++++++++ > board/ti/common/ti-i2c-eeprom.c | 148 > +++++++++++++++++++++++++ > include/configs/am57xx_evm.h | 4 + > 11 files changed, 419 insertions(+), 9 deletions(-) > create mode 100644 board/ti/common/board.c > create mode 100644 board/ti/common/board.h > create mode 100644 board/ti/common/ti-i2c-eeprom.c >
In the future, please: a) CC beagleboard-x15 mailing list (beagleboard-x15 <[email protected]>) b) CC previous reviewers: I think I dont see Igor ([email protected]) in CC list. it is a good practice to cc previous reviewers because, we dont always watch all emails coming to the list and you'd like to respect the time and effort reviewers put in when sending newer revisions. c) even though I see that you have provided in-context of each patch what the changes are (that is good), it is usually better to add to the covering letter what the v1,v2 links were, and also what the baseline for this series is - in this case, you do depend on https://patchwork.ozlabs.org/patch/538046/ - and should be highlighted in cover-letter and reiterated in the corresponding patch. d) provide links to test logs in cover-letter. most of folks do not possess x15 and evms - While I do realize that you have covered that data in the patch #5, personally, I dont usually review patches that are'nt even tested - and I personally respect the patches that tell me upfront in cover letter than the patches have been tested and links to the same. e) provide the baseline for the patches in cover letter, instead of repeating them over and over in each of the patches. it is a series, and patches following will depend on previous patches. For example, your patch #5 claims: "v3 Based on: master 83bf0057 arm: at91: reworked meesc board support" That is not really true - because you do depend on x15 rename patch. If I attempt to apply this series on current u-boot master, I will fail and that does not give me a good impression of the series overall. Apologies on ranting about this, but I'd thought it might help you give a better perspective of what, at least I, as a reviewer would like to see in a series. -- Regards, Nishanth Menon _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
