Re: [meta-rockchip][PATCH v2] Rock64: add machine

["Trevor Woerner" <twoerner@gmail.com>]

On Tue 2021-06-15 @ 10:03:31 AM, Yann Dirson wrote:
> Le lun. 14 juin 2021 à 18:19, Trevor Woerner <twoerner@gmail.com> a écrit :
> >
> > Hi Yann,
> >
> > Thanks for the contribution!
> >
> > On Mon 2021-05-31 @ 04:00:58 PM, yann.dirson@blade-group.com wrote:
> > > From: Yann Dirson <yann@blade-group.com>
> > >
> > > This is a RK3328 board from Pine64.
> > > Board details at https://wiki.pine64.org/wiki/ROCK64.
> > >
> > > Default image is built to boot from SD-card.  Building an image for
> > > eMMC requires to set RK_BOOT_DEVICE="mmcblk0".
> > >
> > > Signed-off-by: Yann Dirson <yann@blade-group.com>
> > > ---
> > >
> > > This is just basic initial support without a kernel BSP.  As is the
> > > board boots with a serial console.
> > >
> > > Note I had to create the SoC definition for rk3328, and rather than
> > > setting serial at 115200 there just to have the board definition
> > > override it with rockchip-standard 1500000 I've set the latter right
> > > in rk3328.inc.
> >
> > Sounds good. I prefer the rockchip default of 1,500,000 anyway.
> >
> > >
> > > Changes in v2:
> > > - include Ayufan's patch for mmc aliases so presence of eMMC won't
> > > prevent booting from SD
> > >
> > >  conf/machine/include/rk3328.inc               | 25 ++++++++++++++++
> > >  conf/machine/rock64.conf                      | 30 +++++++++++++++++++
> > >  ...an-dtsi-rk3328-add-mmc0-mmc1-aliases.patch | 27 +++++++++++++++++
> > >  recipes-kernel/linux/linux-yocto%.bbappend    |  6 ++++
> > >  4 files changed, 88 insertions(+)
> > >  create mode 100644 conf/machine/include/rk3328.inc
> > >  create mode 100644 conf/machine/rock64.conf
> > >  create mode 100644 recipes-kernel/linux/files/0001-ayufan-dtsi-rk3328-add-mmc0-mmc1-aliases.patch
> > >
> > > diff --git a/conf/machine/include/rk3328.inc b/conf/machine/include/rk3328.inc
> > > new file mode 100644
> > > index 0000000..7d67627
> > > --- /dev/null
> > > +++ b/conf/machine/include/rk3328.inc
> > > @@ -0,0 +1,25 @@
> > > +# Copyright (C) 2020 Garmin Ltd. or its subsidaries
> >
> > Odd that you'd be assigning the copyright to Garmin ;-)
> 
> Oh, right, copypaste rules around here, so Garmin does have a role but
> something may be missing :)
> 
> >
> > > +# Released under the MIT license (see COPYING.MIT for the terms)
> > > +
> > > +SOC_FAMILY = "rk3328"
> > > +
> > > +DEFAULTTUNE ?= "cortexa53-crypto"
> > > +
> > > +require conf/machine/include/soc-family.inc
> > > +require conf/machine/include/tune-cortexa53.inc
> > > +require conf/machine/include/rockchip-defaults.inc
> > > +
> > > +KBUILD_DEFCONFIG ?= "defconfig"
> > > +KERNEL_CLASSES = "kernel-fitimage"
> > > +KERNEL_IMAGETYPE = "fitImage"
> > > +
> > > +TFA_PLATFORM = "rk3328"
> > > +TFA_BUILD_TARGET = "bl31"
> > > +
> > > +UBOOT_SUFFIX ?= "itb"
> > > +UBOOT_ENTRYPOINT ?= "0x06000000"
> > > +
> > > +SERIAL_CONSOLES = "1500000;ttyS2"
> > > +
> > > +PREFERRED_PROVIDER_virtual/bootloader ?= "u-boot"
> > > +SPL_BINARY ?= "idbloader.img"
> > > diff --git a/conf/machine/rock64.conf b/conf/machine/rock64.conf
> > > new file mode 100644
> > > index 0000000..38bc9fa
> > > --- /dev/null
> > > +++ b/conf/machine/rock64.conf
> > > @@ -0,0 +1,30 @@
> > > +# Copyright (C) 2021 Blade SAS
> >
> > Can you also specify an OSS-friendly licence too?
> >
> > > +
> > > +#@TYPE: Machine
> > > +#@NAME: Rock64
> > > +#@DESCRIPTION: Rock64 RK3328 board from Pine64
> > > +
> > > +require include/rk3328.inc
> > > +
> > > +MACHINE_FEATURES += "usbhost serial"
> > > +
> > > +UBOOT_MACHINE = "rock64-rk3328_defconfig"
> > > +KERNEL_DEVICETREE = "rockchip/rk3328-rock64.dtb"
> > > +
> > > +# set to mmcblk0 for booting from optional eMMC
> > > +RK_BOOT_DEVICE ?= "mmcblk1"
> > > +
> > > +WKS_FILE ?= "rock-pi-4.wks"
> >
> > Personally I'd prefer to see a rock64 wic file which includes an rk3328
> > default, even if it is a copy of the rock-pi-4 layout.
> 
> Right that was something I wondered how to deal with and forgot (and note that
> for the nanopi-m4 I used the same file).
> 
> My reading of [1] is that all rockchip APs are using the same
> partition table.  I see
> that the existing {rk3288,rk3399}-boot.wks only differ in the choice
> of u-boot image,
> and I'm wondering if using the SoC to distinguish between them is the
> right choice,
> as eg. upstream RK is not using the .itb format, and I suspect we
> could use it as well
> for rk3288 (I'm sure I have one in a drawer and could check that some day).  Now
> maybe the sate of things is different for 32bit SoCs, and I thought it
> could make sense to
> distinguish rockchip-32bit-boot.wks and rockchip-64bit-boot.wks, or maybe even
> name them using the format, as something like rockchip-legacy-boot.wks
> (well there
> is probably a more descriptive name for that format) and rockchip-itb-boot.wks ?
> 
> Similarly, the .wks for 3288-based boards and for 3399-based ones only
> differ by the
> console baudrate, and the 3288-based .wks are all identical except for
> some whitespace.
> And in fact, it is not unthinkable for a given project to use
> something else than a
> single-partition layout, so those files are indeed closer to a
> "default wks".  Maybe we
> could use more generic filenames there too (until we implement a way to avoid
> hardcoding the baudrate here)
> 
> [1] http://opensource.rock-chips.com/wiki_Boot_option

True. We could definitely use some cleanup in this area. If you want to
propose something that's going to work, I'll add it.

Also, when adding a new board, please update the top-level README.

Thanks!