On 02/11/2013 04:39 AM, Prafulla Wadaskar wrote:
-----Original Message-----
From: Sebastian Hesselbarth [mailto:sebastian.hesselba...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to