Aw: Re: [PATCH v4] board: rockchip: Add Bananapi R2Pro Board

Tue Sep 19 09:32:51 CEST 2023

Hi Jonas

> Gesendet: Montag, 18. September 2023 um 23:45 Uhr
> Von: "Jonas Karlman" <jonas at kwiboo.se>
> An: "Frank Wunderlich" <linux at fw-web.de>
> Cc: "Frank Wunderlich" <frank-w at public-files.de>, "Simon Glass" <sjg at chromium.org>, "Philipp Tomsich" <philipp.tomsich at vrull.eu>, "Kever Yang" <kever.yang at rock-chips.com>, u-boot at lists.denx.de
> Betreff: Re: [PATCH v4] board: rockchip: Add Bananapi R2Pro Board
>
> Hi Frank,
>
> On 2023-09-18 19:36, Frank Wunderlich wrote:
> > From: Frank Wunderlich <frank-w at public-files.de>
> >
> > Add Bananapi R2 Pro board.
> >
> > tested:
> > - sdcard
> > - both front usb-ports
> > - sata
> > - wan-port
> >
> > lan-ports are connected to mt7531 switch where driver needs to be separated from mtk ethernet-driver.
> >
> > Signed-off-by: Frank Wunderlich <frank-w at public-files.de>
> > ---
> > because iodomain is different to evb and now iodomain driver is sent as
> > patch we need to separate between EVB and R2Pro else board can be bricked.
> > ---
> > v4:
> > - add r2pro board to readme
> > - update r2pro dts to linux version
> > - remove switch node from linux dts
> > - disable gmac0 because switch driver does not work yet
> >   to solve timeout error:
> >   ethernet at fe2a0000 Waiting for PHY auto negotiation to complete......... TIMEOUT!
> >   phy_startup() failed: -110FAILED: -110ethernet at fe010000 Waiting for PHY auto nee
> > - cleanup r2pro u-boot.dtsi like jonas suggests
> > - update and reorder defconfig based on jonas suggestions
> > - dts: disable usb_host0_ohci because of error on usb-start
> >   scanning bus usb at fd840000 for devices...
> >   ERROR: USB-error: DEVICENOTRESPONDING: Device did not respond to token (IN) or did
> >   not provide a handshake (OUT) (5)
> >   unable to get device descriptor (error=-1)
> > - pcie is not yet working, so not adding these options
> >   rockchip_pcie3phy phy at fe8c0000: lock failed 0x6890000
> >   rockchip_pcie3phy phy at fe8c0000: PHY: Failed to init phy at fe8c0000: -110.
> >   pcie_dw_rockchip pcie at fe270000: failed to init phy (ret=-110)
> >   rockchip_pcie3phy phy at fe8c0000: lock failed 0x6890000
> >   rockchip_pcie3phy phy at fe8c0000: PHY: Failed to init phy at fe8c0000: -110.
> >   pcie_dw_rockchip pcie at fe280000: failed to init phy (ret=-110)
>
> Looks like this board use PCIe bifurcation and would depend on my rk35xx
> PCIe bifurcation series to at least have a chance to work. (looks like
> it is applied based on your log rows).

yes i have added your series to my tree...

https://github.com/frank-w/u-boot/commits/2023-10-bpi-r2pro_2

and yes, board uses bifurcation to split pcie3 to an mPCIe slot and an m.2 slot.

> Could there be an issue with a regulator not being enabled in time?
> Please use regulator cmd to enable all regulators prior to running the
> pci enum cmd, and see if that changes anything.

at least the output changes...

there are 3 pcie-regulators, first is for clock

vcc3v3_pcie          disabled      3300000          - -          0
vcc3v3_minipcie      disabled      3300000          - -          0
vcc3v3_ngff          disabled      3300000          - -          0

BPI-R2PRO> regulator dev vcc3v3_pcie
dev: vcc3v3_pcie @ vcc3v3-pi6c-05-regulator
BPI-R2PRO> regulator enable
BPI-R2PRO> regulator dev vcc3v3_minipcie
dev: vcc3v3_minipcie @ vcc3v3-minipcie-regulator
BPI-R2PRO> regulator enable

vcc3v3_pcie          enabled       3300000          - -          0
vcc3v3_minipcie      enabled       3300000          - -          0
vcc3v3_ngff          disabled      3300000          - -          0

BPI-R2PRO> pci enum
pcie_dw_rockchip pcie at fe270000: PCIe-0 Link Fail

(card is in mpcie-slot)

> > - emmc not tested as it is empty on my board because it breaks sdcard boot
> >
> > v3:
> > - disable gmac0 as switch-driver is not yet ready to attach to the mac
> >
> > v2:
> > - drop switch-node for now as u-boot driver works differently to linux
> > ---
> >  arch/arm/dts/Makefile                     |   3 +-
> >  arch/arm/dts/rk3568-bpi-r2pro-u-boot.dtsi |  28 +
> >  arch/arm/dts/rk3568-bpi-r2pro.dts         | 852 ++++++++++++++++++++++
> >  configs/bpi-r2pro-rk3568_defconfig        |  95 +++
> >  doc/board/rockchip/rockchip.rst           |   1 +
> >  5 files changed, 978 insertions(+), 1 deletion(-)
> >  create mode 100644 arch/arm/dts/rk3568-bpi-r2pro-u-boot.dtsi
> >  create mode 100644 arch/arm/dts/rk3568-bpi-r2pro.dts
>
> The DT file is named rk3568-bpi-r2-pro.dts in linux, please use same
> name in U-Boot. Same goes for defconfig-file, please name it
> bpi-r2-pro-rk3568_defconfig to match DT filename.
>
> >  create mode 100644 configs/bpi-r2pro-rk3568_defconfig
> >
> > diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> > index 85fd5b1157b1..71c557e87e9e 100644
> > --- a/arch/arm/dts/Makefile
> > +++ b/arch/arm/dts/Makefile
> > @@ -183,7 +183,8 @@ dtb-$(CONFIG_ROCKCHIP_RK3568) += \
> >  	rk3568-nanopi-r5s.dtb \
> >  	rk3568-odroid-m1.dtb \
> >  	rk3568-radxa-e25.dtb \
> > -	rk3568-rock-3a.dtb
> > +	rk3568-rock-3a.dtb \
> > +	rk3568-bpi-r2pro.dtb
>
> Please sort the dtb files alphabetically.

done in my tree

> >
> >  dtb-$(CONFIG_ROCKCHIP_RK3588) += \
> >  	rk3588-edgeble-neu6a-io.dtb \
> > diff --git a/arch/arm/dts/rk3568-bpi-r2pro-u-boot.dtsi b/arch/arm/dts/rk3568-bpi-r2pro-u-boot.dtsi
> > new file mode 100644
> > index 000000000000..089e54f8cece
> > --- /dev/null
> > +++ b/arch/arm/dts/rk3568-bpi-r2pro-u-boot.dtsi
>
> [...]
>
> > diff --git a/configs/bpi-r2pro-rk3568_defconfig b/configs/bpi-r2pro-rk3568_defconfig
> > new file mode 100644
> > index 000000000000..df4daed865f0
> > --- /dev/null
> > +++ b/configs/bpi-r2pro-rk3568_defconfig
> > @@ -0,0 +1,95 @@
> > +CONFIG_ARM=y
> > +CONFIG_SKIP_LOWLEVEL_INIT=y
> > +CONFIG_COUNTER_FREQUENCY=24000000
> > +CONFIG_ARCH_ROCKCHIP=y
> > +CONFIG_TEXT_BASE=0x00a00000
> > +CONFIG_SPL_LIBCOMMON_SUPPORT=y
> > +CONFIG_SPL_LIBGENERIC_SUPPORT=y
> > +CONFIG_NR_DRAM_BANKS=2
> > +CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
> > +CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0xc00000
> > +CONFIG_DEFAULT_DEVICE_TREE="rk3568-bpi-r2pro"
> > +CONFIG_SYS_PROMPT="BPI-R2PRO> "
> > +CONFIG_ROCKCHIP_RK3568=y
> > +CONFIG_SPL_ROCKCHIP_COMMON_BOARD=y
> > +CONFIG_SPL_SERIAL=y
> > +CONFIG_SPL_STACK_R_ADDR=0x600000
> > +CONFIG_SPL_STACK=0x400000
> > +CONFIG_DEBUG_UART_BASE=0xFE660000
> > +CONFIG_DEBUG_UART_CLOCK=24000000
> > +CONFIG_SYS_LOAD_ADDR=0xc00800
> > +CONFIG_DEBUG_UART=y
> > +CONFIG_AHCI=y
> > +CONFIG_FIT=y
> > +CONFIG_FIT_VERBOSE=y
> > +CONFIG_SPL_FIT_SIGNATURE=y
> > +CONFIG_SPL_LOAD_FIT=y
> > +CONFIG_BOOTSTD_FULL=y
> > +CONFIG_LEGACY_IMAGE_FORMAT=y
> > +CONFIG_DEFAULT_FDT_FILE="rockchip/rk3568-bpi-r2pro"
>
> Should be rockchip/rk3568-bpi-r2-pro.dtb to match linux.
done in my tree

> > +# CONFIG_DISPLAY_CPUINFO is not set
> > +CONFIG_DISPLAY_BOARDINFO_LATE=y
> > +CONFIG_SPL_MAX_SIZE=0x40000
> > +CONFIG_SPL_PAD_TO=0x7f8000
> > +CONFIG_SPL_HAS_BSS_LINKER_SECTION=y
> > +CONFIG_SPL_BSS_START_ADDR=0x4000000
> > +CONFIG_SPL_BSS_MAX_SIZE=0x4000
> > +# CONFIG_SPL_RAW_IMAGE_SUPPORT is not set
> > +# CONFIG_SPL_SHARES_INIT_SP_ADDR is not set
> > +CONFIG_SPL_STACK_R=y
> > +CONFIG_SPL_ATF=y
> > +CONFIG_CMD_GPIO=y
> > +CONFIG_CMD_GPT=y
> > +CONFIG_CMD_I2C=y
> > +CONFIG_CMD_MMC=y
> > +CONFIG_CMD_USB=y
> > +CONFIG_CMD_SYSBOOT=y
> > +CONFIG_CMD_PMIC=y
> > +CONFIG_CMD_REGULATOR=y
> > +# CONFIG_SPL_DOS_PARTITION is not set
> > +CONFIG_SPL_OF_CONTROL=y
> > +CONFIG_OF_LIVE=y
> > +CONFIG_OF_SPL_REMOVE_PROPS="pinctrl-0 pinctrl-names clock-names interrupt-parent assigned-clocks assigned-clock-rates assigned-clock-parents"
>
> Would recommend you to remove "pinctrl-0 pinctrl-names " from this
> and add CONFIG_SPL_DM_SEQ_ALIAS=y to match other rk3568 boards.

ok, i do

> > +CONFIG_SPL_REGMAP=y
> > +CONFIG_SPL_SYSCON=y
> > +CONFIG_DWC_AHCI=y
> > +CONFIG_SPL_CLK=y
> > +CONFIG_ROCKCHIP_GPIO=y
> > +CONFIG_SYS_I2C_ROCKCHIP=y
> > +CONFIG_MISC=y
> > +CONFIG_SUPPORT_EMMC_RPMB=y
> > +CONFIG_SUPPORT_EMMC_BOOT=y
>
> Not sure eMMC boot partitions are useful on Rockchip. It is not enabled
> on any other rk356x board. Should we enabled it for other rk356x boards?

have not done much with emmc and on my other boards (mediatek based) i need to flash
emmc boot0 partition to successful boot from emmc. So i added this option here to allow this.
If it is really not needed on rockchip i can drop it.

Problem is when i flah emmc, board always boot from emmc independ of sdcard is inserted and needs to
short the emmc...that makes it hard doing such tests.

> > +CONFIG_MMC_DW=y
> > +CONFIG_MMC_DW_ROCKCHIP=y
> > +CONFIG_MMC_SDHCI=y
> > +CONFIG_MMC_SDHCI_SDMA=y
> > +CONFIG_MMC_SDHCI_ROCKCHIP=y
> > +CONFIG_PHY_REALTEK=y
> > +CONFIG_DWC_ETH_QOS=y
> > +CONFIG_DWC_ETH_QOS_ROCKCHIP=y
>
> This Kconfig options is still in a pending series, please remove or
> mention that this patch has a dependency on my rk35xx GMAC series.

ok, i mention it in commit

> > +CONFIG_PHY_ROCKCHIP_INNO_USB2=y
> > +CONFIG_PHY_ROCKCHIP_NANENG_COMBOPHY=y
>
> Would recommend you to add CONFIG_SPL_PINCTRL=y here. It should not hurt
> to let SPL configure pinctrl for sd-card/emmc before it tries to load
> the FIT image for storage media.
>
> > +CONFIG_DM_PMIC=y
> > +CONFIG_PMIC_RK8XX=y
> > +CONFIG_REGULATOR_PWM=y
>
> I am not seeing any pwm regulator in the DT so this line can probably be
> removed.

ok, i drop it

> > +CONFIG_REGULATOR_RK8XX=y
> > +CONFIG_PWM_ROCKCHIP=y
> > +CONFIG_SPL_RAM=y
> > +CONFIG_SCSI=y
> > +CONFIG_DM_SCSI=y
> > +CONFIG_BAUDRATE=1500000
> > +CONFIG_DEBUG_UART_SHIFT=2
> > +CONFIG_SYS_NS16550_MEM32=y
> > +CONFIG_SYSRESET=y
> > +CONFIG_SYSRESET_PSCI=y
>
> Is this working/being used? Normal sysreset from cru driver should work.

not tested as i used an older version of evb defconfig as base

> > +CONFIG_USB=y
> > +CONFIG_USB_XHCI_HCD=y
> > +CONFIG_USB_EHCI_HCD=y
> > +CONFIG_USB_EHCI_GENERIC=y
> > +CONFIG_USB_OHCI_HCD=y
> > +CONFIG_USB_OHCI_GENERIC=y
> > +CONFIG_USB_UHCI_HCD=y
>
> Is this Kconfig options used for anything? Does not look like there is
> any other board using this, should be safe to remove this line.

the generic EHCI and OHCI have nodes in dts and imho are needed for usb2-support on the xhci controllers.

> > +CONFIG_USB_DWC3=y
> > +CONFIG_USB_DWC3_GENERIC=y
> > +CONFIG_ERRNO_STR=y
> > +# CONFIG_EFI_LOADER is not set
>
> I would recommend you to remove this line to allow EFI boot on this board.

done in my tree

> With EFI enabled users should be able to use a plain aarch64 distro
> installers to setup a system. Fedora, openSUSE, Ubuntu are a few distros
> that currently works with some minor quirks.
>
> For an incomplete guide on how a distro installer could be used, see:
> https://wiki.pine64.org/wiki/Quartz64_UEFI_with_U-Boot
>
> > diff --git a/doc/board/rockchip/rockchip.rst b/doc/board/rockchip/rockchip.rst
> > index de9fe8e642b1..a639215d904e 100644
> > --- a/doc/board/rockchip/rockchip.rst
> > +++ b/doc/board/rockchip/rockchip.rst
> > @@ -101,6 +101,7 @@ List of mainline supported Rockchip boards:
> >
> >  * rk3568
> >       - Rockchip Evb-RK3568 (evb-rk3568)
> > +     - BananaPi R2 Pro (bpi-r2pro-rk3568_defconfig)
>
> Please use the full market name, Banana Pi BPI-P2 Pro, if I am not
> mistaken, also remove _defconfig and only keep bpi-r2-pro-rk3568 in
> parentheses.

done in my tree

> >       - EmbedFire LubanCat 2 (lubancat-2-rk3568)
> >       - FriendlyElec NanoPi R5C (nanopi-r5c-rk3568)
> >       - FriendlyElec NanoPi R5S (nanopi-r5s-rk3568)
>
> An entry for this board should be added to a MAINTAINERS-file, probably
> board/rockchip/evb_rk3568/MAINTAINERS, because it is using that boards
> config.

done in my tree

> Regards,
> Jonas

regards Frank