Re: [Buildroot] [PATCH v1 1/1] configs/zynqmp_kria_kr260_defconfig: new defconfig

[Luca Ceresoli via buildroot <buildroot@buildroot.org>]

Hi Neal,

On Wed, 31 May 2023 12:16:02 +0100
Neal Frager <neal.frager@amd.com> wrote:

> This patch adds support for Xilinx Kria KR260 starter kit.
> 
> KR260 features can be found here:
> https://www.xilinx.com/products/som/kria/kr260-robotics-starter-kit.html
> 
> While the Kria SOM is based on a ZynqMP SoC, there are some key
> boot config differences from the other ZynqMP evaluation boards.
> 
> 1. There are no boot switches on Kria SOMs. The boot mode is thus
> hard configured for QSPI flash. A pre-programmed boot.bin comes
> with every Starter Kit. U-Boot can then find the Linux kernel and
> file system on the SD card.
> 
> Optional instructions for updating the boot.bin in the QSPI flash
> can be found in the readme.txt file and the link below.
> 
> https://xilinx-wiki.atlassian.net/wiki/spaces/A/pages/1641152513/Kria+K26+SOM
> 
> 2. Kria SOMs use UART1 for the console instead of UART0. For this
> reason, Kria Starter Kits will use a separate extlinux.conf file
> from other ZynqMP evaluation boards.
> 
> 3. The KR260 has a USB to SD card bridge, so the Linux kernel
> and file system are found on /dev/sda1 and /dev/sda2.
> 
> Signed-off-by: Neal Frager <neal.frager@amd.com>
> ---
>  DEVELOPERS                                    |   3 +-
>  board/zynqmp/kria/kr260/kr260.sh              |  12 +
>  ...inctrl-zynqmp-add-tri-state-controls.patch |  90 ++++
>  board/zynqmp/kria/kr260/pm_cfg_obj.c          | 496 ++++++++++++++++++
>  board/zynqmp/kria/readme.txt                  |  29 +-
>  configs/zynqmp_kria_kr260_defconfig           |  42 ++
>  6 files changed, 665 insertions(+), 7 deletions(-)
>  create mode 100755 board/zynqmp/kria/kr260/kr260.sh
>  create mode 100644 board/zynqmp/kria/kr260/patches/uboot/v1-0001-drivers-pinctrl-zynqmp-add-tri-state-controls.patch
>  create mode 100644 board/zynqmp/kria/kr260/pm_cfg_obj.c
>  create mode 100644 configs/zynqmp_kria_kr260_defconfig
> 
> diff --git a/DEVELOPERS b/DEVELOPERS
> index e76717e845..26bcf93b0a 100644
> --- a/DEVELOPERS
> +++ b/DEVELOPERS
> @@ -2152,9 +2152,10 @@ F:	board/zynqmp/
>  F:	board/zynqmp/kria/
>  F:	configs/versal_vck190_defconfig
>  F:	configs/zynq_zc706_defconfig
> +F:	configs/zynqmp_kria_kr260_defconfig
> +F:	configs/zynqmp_kria_kv260_defconfig
>  F:	configs/zynqmp_zcu102_defconfig
>  F:	configs/zynqmp_zcu106_defconfig
> -F:	configs/zynqmp_kria_kv260_defconfig
>  F:	package/bootgen/
>  F:	package/versal-firmware/
>  
> diff --git a/board/zynqmp/kria/kr260/kr260.sh b/board/zynqmp/kria/kr260/kr260.sh
> new file mode 100755
> index 0000000000..df8b0ccde5
> --- /dev/null
> +++ b/board/zynqmp/kria/kr260/kr260.sh
> @@ -0,0 +1,12 @@
> +#!/bin/sh
> +
> +# This is a temporary work around for generating kr260 u-boot.itb.
> +# The problem is there is no way to currently configure u-boot to apply
> +# the carrier board dtb overlay during build, so all kr260 carrier board
> +# drivers are missing.
> +# This will be removed when u-boot can build the kr260 u-boot.itb natively.
> +
> +UBOOT_DIR=$4
> +
> +fdtoverlay -o ${UBOOT_DIR}/fit-dtb.blob -i ${UBOOT_DIR}/arch/arm/dts/zynqmp-smk-k26-revA.dtb ${UBOOT_DIR}/arch/arm/dts/zynqmp-sck-kr-g-revB.dtbo
> +${UBOOT_DIR}/tools/mkimage -E -f ${UBOOT_DIR}/u-boot.its -B 0x8 ${BINARIES_DIR}/u-boot.itb
> diff --git a/board/zynqmp/kria/kr260/patches/uboot/v1-0001-drivers-pinctrl-zynqmp-add-tri-state-controls.patch b/board/zynqmp/kria/kr260/patches/uboot/v1-0001-drivers-pinctrl-zynqmp-add-tri-state-controls.patch

I know we have a similar script for the KV260 carrier, but I wonder
whether U-Boot would be able to load the base dtb and then apply the
overlay at runtime before loading Linux.

Another question is of course: is there any effort ongoing to fix this
upstream?

> new file mode 100644
> index 0000000000..422a5d091c
> --- /dev/null
> +++ b/board/zynqmp/kria/kr260/patches/uboot/v1-0001-drivers-pinctrl-zynqmp-add-tri-state-controls.patch
> @@ -0,0 +1,90 @@
> +From 560b2e0b6de66c4c9f4b261308263dd091c6002a Mon Sep 17 00:00:00 2001
> +From: Neal Frager <neal.frager@amd.com>
> +Date: Tue, 28 Mar 2023 14:42:59 +0100
> +Subject: [PATCH v1 1/1] drivers: pinctrl-zynqmp: add tri-state controls
> +
> +This patch enables u-boot pinctrl of the tri-state registers.  As it does not
> +check pmufw version, it assumes version 2 of the pmufw pinctrl driver which
> +comes only in pmufw 2023.1 or newer.

Why is this needed? Since you are using a vendor U-Boot, it feels
strange that you need further patches on top.

Is this patch being upstreamed?

As a general rule, every commit message should always explain _why_ the
patch is needed. Explaining _what_ the patch changes is often evident
from the diff and thus useless in the commit message.

Otherwise looks good.

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot