[U-Boot] [PATCH v3 3/4] rockchip: Add basic support for phyCORE-RK3288 SoM based carrier board

[Simon Glass]

On 12 June 2017 at 03:59, Wadim Egorov <w.egorov at phytec.de> wrote:
> The phyCORE-RK3288 is a SoM (System on Module) containing a RK3288 SoC.
> The module can be connected to different carrier boards.
> It can be also equipped with different RAM, SPI flash and eMMC variants.
> The Rapid Development Kit option is using the following setup:
>
>   - 1 GB DDR3 RAM (2 Banks)
>   - 1x 4 KB EEPROM
>   - DP83867 Gigabit Ethernet PHY
>   - 16 MB SPI Flash
>   - 4 GB eMMC Flash
>
> Add basic support for the PCM-947 carrier board, a RK3288 based development
> board made by PHYTEC. This board works in a combination with
> the phyCORE-RK3288 System on Module.
>
> Signed-off-by: Wadim Egorov <w.egorov at phytec.de>
> ---
> Changes in v3:
> - Added u-boot,dm-pre-reloc to regulators subnode
>   (PMIC failed to bind because of the missing regulators node)
> - Removed the hacky PMIC setup from phycore_init
> - Use CONFIG_TARGET_PHYCORE_RK3288 to cover board-specific stuff
>
>   Things get ugly when I use of_machine_is_compatible().
>   Boards whith CONFIG_SPL_OF_PLATDATA disabled would compile in
>   of_machine_is_compatible() which checks for my board.
>   phycore_init() calls rk818_spl_configure_*() which depends on
>   CONFIG_SPL_POWER_SUPPORT. So this will cause undefined reference linker
>   warnings for every other rockchip board.
>   That's the reason why I moved back to CONFIG_TARGET_PHYCORE_RK3288.
>
> ---
>  arch/arm/dts/Makefile                        |   1 +
>  arch/arm/dts/rk3288-phycore-rdk.dts          | 294 ++++++++++++++++
>  arch/arm/dts/rk3288-phycore-som.dtsi         | 506 +++++++++++++++++++++++++++
>  arch/arm/mach-rockchip/rk3288-board-spl.c    |  35 ++
>  arch/arm/mach-rockchip/rk3288/Kconfig        |  10 +
>  board/phytec/phycore_rk3288/Kconfig          |  15 +
>  board/phytec/phycore_rk3288/MAINTAINERS      |   6 +
>  board/phytec/phycore_rk3288/Makefile         |   8 +
>  board/phytec/phycore_rk3288/phycore-rk3288.c |   8 +
>  configs/phycore-rk3288_defconfig             |  70 ++++
>  include/configs/phycore_rk3288.h             |  23 ++
>  11 files changed, 976 insertions(+)
>  create mode 100644 arch/arm/dts/rk3288-phycore-rdk.dts
>  create mode 100644 arch/arm/dts/rk3288-phycore-som.dtsi
>  create mode 100644 board/phytec/phycore_rk3288/Kconfig
>  create mode 100644 board/phytec/phycore_rk3288/MAINTAINERS
>  create mode 100644 board/phytec/phycore_rk3288/Makefile
>  create mode 100644 board/phytec/phycore_rk3288/phycore-rk3288.c
>  create mode 100644 configs/phycore-rk3288_defconfig
>  create mode 100644 include/configs/phycore_rk3288.h

Reviewed-by: Simon Glass <sjg at chromium.org>

(assuming we need to set up the battery / charger in SPL)

But can you please check the compatible string to decide whether you
need to do the phytec stuff, instead of an #ifdef on the target? I
suppose if it increase code size it is OK to have both, but the #ifdef
should be a size optimisation, not the main method of selecting the
code Is there a way to describe what is needed in the DT?