From mboxrd@z Thu Jan  1 00:00:00 1970
From: Simon Glass <sjg@chromium.org>
Date: Tue, 23 May 2017 18:44:40 -0600
Subject: [U-Boot] [PATCH v2 1/2] rockchip: Add basic support for
 phyCORE-RK3288 SoM based carrier board
In-Reply-To: <1494857976-23069-1-git-send-email-w.egorov@phytec.de>
References: <1494857976-23069-1-git-send-email-w.egorov@phytec.de>
Message-ID: <CAPnjgZ3AxOmgYPrinXbYp_JZzwT7TDBcwCVJ6VuxeXb=Hdv1kQ@mail.gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Wadim,

On 15 May 2017 at 08:19, Wadim Egorov <w.egorov@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@phytec.de>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
> Changes in v2:
> - Move phycore initialization to an own function
> - Use of_machine_is_compatible() instead of #ifdef
> - Drop board_boot_order() and use spl_boot_device()
> - Added Reviewed-by: Simon Glass <sjg@chromium.org>
>
> ---
>  arch/arm/dts/Makefile                        |   1 +
>  arch/arm/dts/rk3288-phycore-rdk.dts          | 294 ++++++++++++++++
>  arch/arm/dts/rk3288-phycore-som.dtsi         | 503 +++++++++++++++++++++++++++
>  arch/arm/mach-rockchip/rk3288-board-spl.c    |  37 ++
>  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             |  69 ++++
>  include/configs/phycore_rk3288.h             |  23 ++
>  11 files changed, 974 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

This unfortunately causes build errors on various rockchip boards.

Try 'buildman rockchip' to see it.

Please see below.

[...]

> diff --git a/arch/arm/mach-rockchip/rk3288-board-spl.c b/arch/arm/mach-rockchip/rk3288-board-spl.c
> index 74f3379..724dcb4 100644
> --- a/arch/arm/mach-rockchip/rk3288-board-spl.c
> +++ b/arch/arm/mach-rockchip/rk3288-board-spl.c
> @@ -8,6 +8,7 @@
>  #include <debug_uart.h>
>  #include <dm.h>
>  #include <fdtdec.h>
> +#include <i2c.h>
>  #include <led.h>
>  #include <malloc.h>
>  #include <ram.h>
> @@ -25,6 +26,7 @@
>  #include <dm/test.h>
>  #include <dm/util.h>
>  #include <power/regulator.h>
> +#include <power/rk8xx_pmic.h>
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> @@ -157,6 +159,38 @@ static int configure_emmc(struct udevice *pinctrl)
>  }
>  #endif
>
> +void phycore_init(void)
> +{
> +       struct udevice *dev;
> +       uint reg;
> +       int ret;
> +
> +       ret = uclass_get_device(UCLASS_I2C, 0, &dev);
> +       if (ret) {
> +               debug("I2C init failed: %d\n", ret);
> +               return;
> +       }
> +
> +       ret = i2c_get_chip(dev, 0x1c, 1, &dev);

We should not be hard-coding the bus address here.

See veyron_init() for an example of another way to do this.

> +       if (ret) {
> +               debug("Cannot find RK818: %d\n", ret);
> +               return;
> +       }
> +
> +       reg = dm_i2c_reg_read(dev, REG_USB_CTRL);
> +
> +       /*
> +        * Increase USB input current selection to 2A and close charger
> +        * when usb lower then 3.4V.
> +        */
> +       reg |= 0x77;
> +       ret = dm_i2c_reg_write(dev, REG_USB_CTRL, reg);

Here you are hacking registers in the PMIC. This should go in the PMIC
driver. See rk8xx_spl_configure_buck() for how to do this in SPL
without bringing in the whole regulator framework.

> +       if (ret) {
> +               debug("Unable to set RK818 REG_USB_CTRL: %d\n", ret);
> +               return;
> +       }
> +}
> +

Regards,
Simon