Re: [PATCH v2 2/2] rockchip: Add initial support for the PINE64 Pinephone Pro

3 Jan 2023

Hi Peter,
On Sat, 31 Dec 2022 at 03:24, Peter Robinson pbrobinson@gmail.com wrote:
...
The Pinephone Pro is another device by PINE64. It's closely related
to the Pinebook Pro of which this initial support is derived from.
Specification:

A variant of the Rockchip RK3399
A 6 inch 720*1440 DSI display
Front and rear cameras
Type-C interface with alt mode display (DP 1.2) and PD charging
4GB LPDDR4 RAM
128GB eMMC
mSD card slot
An AP6255 module for 802.11ac WiFi and Bluetooth 5
Quectel EG25-G 4G/LTE modem

Signed-off-by: Peter Robinson pbrobinson@gmail.com
arch/arm/dts/rk3399-pinephone-pro-u-boot.dtsi |  31 ++++++
 arch/arm/mach-rockchip/rk3399/Kconfig         |   8 ++
 board/pine64/pinephone-pro-rk3399/Kconfig     |  15 +++
 board/pine64/pinephone-pro-rk3399/MAINTAINERS |   8 ++
 board/pine64/pinephone-pro-rk3399/Makefile    |   1 +
 .../pinephone-pro-rk3399.c                    |  76 +++++++++++++
 configs/pinephone-pro-rk3399_defconfig        | 104 ++++++++++++++++++
 include/configs/pinephone-pro-rk3399.h        |  19 ++++
 8 files changed, 262 insertions(+)
 create mode 100644 arch/arm/dts/rk3399-pinephone-pro-u-boot.dtsi
 create mode 100644 board/pine64/pinephone-pro-rk3399/Kconfig
 create mode 100644 board/pine64/pinephone-pro-rk3399/MAINTAINERS
 create mode 100644 board/pine64/pinephone-pro-rk3399/Makefile
 create mode 100644 board/pine64/pinephone-pro-rk3399/pinephone-pro-rk3399.c
 create mode 100644 configs/pinephone-pro-rk3399_defconfig
 create mode 100644 include/configs/pinephone-pro-rk3399.h
Reviewed-by: Simon Glass sjg@chromium.org
Just a nit below
[..]
...
new file mode 100644
index 00000000000..8d9203053e5
--- /dev/null
+++ b/board/pine64/pinephone-pro-rk3399/Makefile
@@ -0,0 +1 @@
+obj-y  += pinephone-pro-rk3399.o

diff --git a/board/pine64/pinephone-pro-rk3399/pinephone-pro-rk3399.c b/board/pine64/pinephone-pro-rk3399/pinephone-pro-rk3399.c
new file mode 100644
index 00000000000..eb639cd0d07
--- /dev/null
+++ b/board/pine64/pinephone-pro-rk3399/pinephone-pro-rk3399.c
@@ -0,0 +1,76 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*


(C) Copyright 2016 Rockchip Electronics Co., Ltd



(C) Copyright 2022 Peter Robinson <pbrobinson at gmail.com>


*/


+#include <common.h>
+#include <dm.h>
+#include <init.h>
+#include <syscon.h>
+#include <asm/io.h>
+#include <asm/arch-rockchip/clock.h>
+#include <asm/arch-rockchip/grf_rk3399.h>
+#include <asm/arch-rockchip/hardware.h>
+#include <asm/arch-rockchip/misc.h>
+#include <power/regulator.h>



+#define GRF_IO_VSEL_BT565_SHIFT 0
+#define PMUGRF_CON0_VSEL_SHIFT 8



+#ifndef CONFIG_SPL_BUILD
You should be able to use C code inside the function
if (spl_phase() == PHASE_SPL) {
..
...
+int board_early_init_f(void)
+{

  struct udevice *regulator;


  int ret;



  ret = regulator_get_by_platname("vcc5v0_usb", &regulator);


  if (ret) {


          pr_debug("%s vcc5v0_usb init fail! ret %d\n", __func__, ret);


          goto out;


  }



  ret = regulator_set_enable(regulator, true);


  if (ret)


          pr_debug("%s vcc5v0-host-en-gpio set fail! ret %d\n", __func__, ret);




+out:

  return 0;



+}
+#endif



+#ifdef CONFIG_MISC_INIT_R
Won't the compiler drop this anyway if it is not needed? Can you drop
the #ifdef ?
...
+static void setup_iodomain(void)
+{

  struct rk3399_grf_regs *grf =


     syscon_get_first_range(ROCKCHIP_SYSCON_GRF);


  struct rk3399_pmugrf_regs *pmugrf =


     syscon_get_first_range(ROCKCHIP_SYSCON_PMUGRF);



  /* BT565 is in 1.8v domain */


  rk_setreg(&grf->io_vsel, 1 << GRF_IO_VSEL_BT565_SHIFT);



  /* Set GPIO1 1.8v/3.0v source select to PMU1830_VOL */


  rk_setreg(&pmugrf->soc_con0, 1 << PMUGRF_CON0_VSEL_SHIFT);



+}



+int misc_init_r(void)
+{

  const u32 cpuid_offset = 0x7;


  const u32 cpuid_length = 0x10;


  u8 cpuid[cpuid_length];


  int ret;



  setup_iodomain();



  ret = rockchip_cpuid_from_efuse(cpuid_offset, cpuid_length, cpuid);


  if (ret)


          return ret;



  ret = rockchip_cpuid_set(cpuid, cpuid_length);


  if (ret)


          return ret;



  return ret;



+}
+#endif
diff --git a/configs/pinephone-pro-rk3399_defconfig b/configs/pinephone-pro-rk3399_defconfig
new file mode 100644
index 00000000000..eb979f6c051
--- /dev/null
+++ b/configs/pinephone-pro-rk3399_defconfig
@@ -0,0 +1,104 @@
+CONFIG_ARM=y
+CONFIG_SKIP_LOWLEVEL_INIT=y
+CONFIG_COUNTER_FREQUENCY=24000000
+CONFIG_ARCH_ROCKCHIP=y
+CONFIG_TEXT_BASE=0x00200000
+CONFIG_NR_DRAM_BANKS=1
+CONFIG_ENV_SIZE=0x8000
+CONFIG_ENV_OFFSET=0x3F8000
+CONFIG_DEFAULT_DEVICE_TREE="rk3399-pinephone-pro"
+CONFIG_ROCKCHIP_RK3399=y
+CONFIG_TARGET_PINEPHONE_PRO_RK3399=y
+CONFIG_DEBUG_UART_BASE=0xFF1A0000
+CONFIG_DEBUG_UART_CLOCK=24000000
+CONFIG_SPL_SPI_FLASH_SUPPORT=y
+CONFIG_SPL_SPI=y
+CONFIG_SYS_LOAD_ADDR=0x800800
+CONFIG_DEBUG_UART=y
+CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
+CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x300000
+CONFIG_BOOTDELAY=3
+CONFIG_USE_PREBOOT=y
+CONFIG_DEFAULT_FDT_FILE="rockchip/rk3399-pinephone-pro.dtb"
+CONFIG_DISPLAY_BOARDINFO_LATE=y
+CONFIG_MISC_INIT_R=y
+CONFIG_SPL_MAX_SIZE=0x2e000
+CONFIG_SPL_PAD_TO=0x7f8000
+CONFIG_SPL_HAS_BSS_LINKER_SECTION=y
+CONFIG_SPL_BSS_START_ADDR=0x400000
+CONFIG_SPL_BSS_MAX_SIZE=0x2000
+# CONFIG_SPL_RAW_IMAGE_SUPPORT is not set
+# CONFIG_SPL_SHARES_INIT_SP_ADDR is not set
+CONFIG_SPL_STACK=0x400000
+CONFIG_SPL_STACK_R=y
+CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x10000
+CONFIG_SPL_SPI_LOAD=y
+CONFIG_TPL=y
+CONFIG_CMD_BOOTZ=y
+CONFIG_CMD_GPIO=y
+CONFIG_CMD_GPT=y
+CONFIG_CMD_I2C=y
+CONFIG_CMD_MMC=y
+CONFIG_CMD_PCI=y
+CONFIG_CMD_USB=y
+# CONFIG_CMD_SETEXPR is not set
+CONFIG_CMD_TIME=y
+CONFIG_CMD_PMIC=y
+CONFIG_CMD_REGULATOR=y
+CONFIG_SPL_OF_CONTROL=y
+CONFIG_OF_SPL_REMOVE_PROPS="pinctrl-0 pinctrl-names clock-names interrupt-parent assigned-clocks assigned-clock-rates assigned-clock-parents"
+CONFIG_ENV_IS_IN_SPI_FLASH=y
+CONFIG_SYS_RELOC_GD_ENV_ADDR=y
+CONFIG_SPL_DM_SEQ_ALIAS=y
+CONFIG_ROCKCHIP_GPIO=y
+CONFIG_SYS_I2C_ROCKCHIP=y
+CONFIG_DM_KEYBOARD=y
+CONFIG_LED=y
+CONFIG_LED_GPIO=y
+CONFIG_MISC=y
+CONFIG_ROCKCHIP_EFUSE=y
+CONFIG_MMC_DW=y
+CONFIG_MMC_DW_ROCKCHIP=y
+CONFIG_MMC_SDHCI=y
+CONFIG_MMC_SDHCI_SDMA=y
+CONFIG_MMC_SDHCI_ROCKCHIP=y
+CONFIG_SF_DEFAULT_BUS=1
+CONFIG_SF_DEFAULT_SPEED=20000000
+CONFIG_SPI_FLASH_GIGADEVICE=y
+CONFIG_SPI_FLASH_WINBOND=y
+CONFIG_PHY_ROCKCHIP_INNO_USB2=y
+CONFIG_PHY_ROCKCHIP_TYPEC=y
+CONFIG_DM_PMIC_FAN53555=y
+CONFIG_DM_REGULATOR_FAN53555=y
+CONFIG_PMIC_RK8XX=y
+CONFIG_REGULATOR_PWM=y
+CONFIG_REGULATOR_RK8XX=y
+CONFIG_PWM_ROCKCHIP=y
+CONFIG_RAM_RK3399_LPDDR4=y
+CONFIG_DM_RESET=y
+CONFIG_DM_RNG=y
+CONFIG_RNG_ROCKCHIP=y
+CONFIG_BAUDRATE=1500000
+CONFIG_DEBUG_UART_SHIFT=2
+CONFIG_ROCKCHIP_SPI=y
+CONFIG_SYSRESET=y
+CONFIG_USB=y
+CONFIG_USB_XHCI_HCD=y
+CONFIG_USB_XHCI_DWC3=y
+CONFIG_USB_EHCI_HCD=y
+CONFIG_USB_EHCI_GENERIC=y
+CONFIG_USB_OHCI_HCD=y
+CONFIG_USB_OHCI_GENERIC=y
+CONFIG_USB_DWC3=y
+CONFIG_USB_DWC3_GENERIC=y
+CONFIG_USB_KEYBOARD=y
+CONFIG_SYS_USB_EVENT_POLL_VIA_INT_QUEUE=y
+CONFIG_USB_HOST_ETHER=y
+CONFIG_USB_ETHER_ASIX=y
+CONFIG_USB_ETHER_RTL8152=y
+CONFIG_DISPLAY=y
+CONFIG_VIDEO=y
+CONFIG_VIDEO_ROCKCHIP=y
+CONFIG_DISPLAY_ROCKCHIP_EDP=y
+CONFIG_SPL_TINY_MEMSET=y
+CONFIG_ERRNO_STR=y
[..]
Regards,
Simon

    