On 02/11/2013 04:39 AM, Prafulla Wadaskar wrote:
-----Original Message----- From: Sebastian Hesselbarth [mailto:sebastian.hesselbarth@gmail.com] Sent: 17 January 2013 00:55 To: Sebastian Hesselbarth Cc: u-boot@lists.denx.de; Rabeeh Khoury; Albert Aribaud; Prafulla Wadaskar; Andy Fleming; Joe Hershberger; Daniel Stodden; Luka Perkov Subject: [PATCH v3 00/10] Add Marvell Dove and SolidRun CuBox
This patch set add support for the Marvell Dove 88AP510 SoC and the SolidRun CuBox board based on that SoC. The patch set is divided into the four following sections:
Dear Sabastian First of all I express my thanks for initiating this Soc Support in the u-boot. Secondly I express my apology since I could not review then immediately. Please find my comments for entire patch series
Prafulla,
thanks for the detailled review! I will quickly give some comments about some of your review questions and will do a full re-review later.
* About the orion/dove/kirkwood naming convention in linux kernel (that this patch set picks up):
When I started hacking on linux and marvell SoCs in particular there was support for orion5x, mv78xx0, kirkwood, and dove. As those SoCs share a bunch of internal peripherals (you might know better) the drivers shared between those SoCs were prepended with "orion_" that should reflect the family of SoCs with that peripherals. I personally find the naming collision with orion5x "unlucky" but that is the way it is.
Then there came some effort to also mainline Armada 370/XP series that of course also share "orion" drivers. The final name for all SoCs (including "orion") was mvebu as it is the only common naming scheme that fit (mvebu stands for some business unit within Marvell).
Plans for using just "marvell" or "mv" were rejected because there are also pxa SoCs from marvell sitting in the kernel tree. They likely share some of the internal peripherals but that is not reflected within kernel tree, as there are two different hacking communities I guess.
New drivers that fit all SoCs (namely pinctrl and gpio, because linux API changes required new drivers from scratch) get mvebu_, drivers that still fit in "orion" get orion_, new soc specific ones get either dove_, kirkwood_, armada_370_, or whatever the code name of that SoC is. Old driver names remain untouched even if they are reused on Armada 370/XP.
Whenever I used orion_ in this patch set this refers to orion as in SoC family, not orion5x that you might have been irritated with.
* About the kwboot/kwbimage patches: u-boot has kwboot for kirkwood since ages, but the general functionality to boot through UART boot mode also applies to above SoCs. I took kwbimage to reflect that kwboot should be used for booting this image. IMHO introducing new abbreviated image names like dvbimage will just distract people from using the correct tool to boot it. kwboot will not be renamed, will it?
* About the ordering of patches/patch sets:
(1) Patches 1-5:
I have suggestions to break down your patches as mentioned below
Add support for the Dove SoC and related drivers. Where possible drivers from Marvell Kirkwood are reused (mvsata, mvgbe), or forked to allow more generic usage (SPI, GPIO). The SDHCI driver is different and a new driver is added for it. The forked drivers can also be reused on Kirkwood but that would have required patching existing boards.
(2) Patches 6-8: Allow mvgbe to use the phylib API, add support for 88E1310 PHY and allow Dove to use the driver.
(3) Patch 9 Add the SolidRun CuBox as the first board based on Marvell Dove SoC.
(4) Patch 10 Add support for different UART boot mode found on Dove.
Changelog: v1->v2:
- respect review comments by Luka Perkov
- fix commenting styles and typos
- add MAINTAINERS entry
- also update kwboot.1 manpage
v2->v3:
- integrate kwboot patch from Daniel Stodden
- rebase on release v2013.01
Sebastian Hesselbarth (10): ARM: dove: add support for Marvell Dove SoC GPIO: add gpio driver for Orion SoCs MMC: sdhci: Add support for dove sdhci SPI: Add Orion SPI driver block: mvsata: add dove include NET: phy: add 88E1310 PHY initialization NET: mvgbe: add phylib support NET: mvgbe: add support for Dove Boards: Add support for SolidRun CuBox tools: Add support for Dove to kwboot
MAINTAINERS | 4 + arch/arm/cpu/armv7/dove/Makefile | 49 +++++ arch/arm/cpu/armv7/dove/cpu.c | 266 ++++++++++++++++++++++++++ arch/arm/cpu/armv7/dove/dram.c | 118 ++++++++++++ arch/arm/cpu/armv7/dove/lowlevel_init.S | 83 ++++++++ arch/arm/cpu/armv7/dove/mpp.c | 318 +++++++++++++++++++++++++++++++ arch/arm/cpu/armv7/dove/timer.c | 176 +++++++++++++++++ arch/arm/cpu/armv7/dove/usb.c | 101 ++++++++++ arch/arm/include/asm/arch-dove/config.h | 153 +++++++++++++++ arch/arm/include/asm/arch-dove/cpu.h | 204 ++++++++++++++++++++ arch/arm/include/asm/arch-dove/dove.h | 93 +++++++++ arch/arm/include/asm/arch-dove/gpio.h | 35 ++++ arch/arm/include/asm/arch-dove/mpp.h | 283
Basic Dove support (one patch series)
Ok. Just to make sure, you want me to leave driver specific stuff, e.g. orion_spi_* out of this patch set and re-add the calls within the corresponding driver patches?
drivers/gpio/Makefile | 1 + include/dove or mv_gpio.h | 64 +++++++ drivers/gpio/dove or mv_gpio.c | 167 ++++++++++++++++
GPIO driver support for Dove
With the above comments about SoC naming, I'd like to stick with either orion_ or mvebu_ if the basic driver framework can be reused on Armada 370/XP (when they get mainline support in u-boot).
drivers/spi/Makefile | 1 + drivers/spi/dove_spi.c | 217
Spi driver support for dove
Ditto.
drivers/block/mvsata_ide.c | 2 + drivers/mmc/Makefile | 1 + drivers/mmc/dove_sdhci.c | 101 ++++++++++ drivers/net/mvgbe.c | 70 ++++++- drivers/net/mvgbe.h | 7 + drivers/net/phy/marvell.c | 48 +++++
Other driver supports for dove (explain supported drivers)
Ack.
+++++++++++++++++++++++++++ board/solidrun/cubox/Makefile | 45 +++++ board/solidrun/cubox/cubox.c | 141 ++++++++++++++ board/solidrun/cubox/kwbimage.cfg | 76 ++++++++ boards.cfg | 1 + include/configs/cubox.h | 175 +++++++++++++++++
Board support patches
Ack.
doc/kwboot.1 | 13 +-
+++++++++++++++++++++
tools/Makefile | 2 + tools/kwboot.c | 25 ++-
Add dove support for boot tool.
Ack.
FYI: I have gone through all patches in this series
Thanks for that again! I will flip through all the individual patches later and give detailled comments for sure.
Regards, Sebastian