Re: [yocto-patches] [meta-rockchip] [PATCH] SOQuartz: add

[Quentin Schulz <quentin.schulz@cherry.de>]

Hi Paul,

Please version your patches when sending (usually -v N with N being the 
proper version number should be enough), a vN will then appear next to 
[PATCH in the mail subject. That is necessary for maintainers and 
reviewers to not have to figure out which one mail is the latest and 
greatest patch you've sent and they should review or merge.

I can also recommend using b4 which does a lot of things automatically 
for you :)

See comment inline.

On 11/15/24 3:51 PM, Paul M. Bendixen via lists.yoctoproject.org wrote:
> [You don't often get email from pbe=trifork.com@lists.yoctoproject.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> The SOQuartz is a RK3566 based compute module and parts of Quartz64 series
> The Model-A base board is one possible board that supports it
> 
> Website:
>       https://pine64.org/devices/soquartz/
> Wiki:
>       https://wiki.pine64.org/wiki/SOQuartz
> 
> Specs:
> - Rockchip RK3566 Quad-core ARM Cortex-A55@1.8GHz
> - Mali-G52 2EE Bifrost GPU@800MHz
> - Raspberry Pi 4 CM form factor
> - RAM Memory Variants: 2GB, 4GB, 8GB LPDDR4.
> - optional eMMC from 8GB to 128GB
> - optional 128Mb SPI Flash
> - 10/100/1000Mbps Ethernet
> - WiFi 802.11 b/g/n/ac with Bluetooth 5.0
> 
> Exposed preripherals:
> - 1x HDMI
> - 2x DSI
> - 1x eDP
> - 1x LVDS
> - 1x CSI 4-line
> - 1x Ethernet
> - 1x USB 2.0 OTG
> - 1x SD
> - 1x PCIe 1-line
> - 28x GPIO
> 
> Model-A baseboard:
> - 1x microSD - bootable
> - 1x HDMI Port
> - 2x USB A 2.0 Host
> - 1x USB C 2.0 Host
> - 1x 5 pin USB expansion
> - 1x Ethernet w. PoE
> - 1x 40 pole Pi2 compatible GPIO
> - 1x MiPi-CSI 2 lanes
> - 1x MiPi-CSI 4 lanes
> - 1x MiPi-DSI 2 lanes
> - 1x MiPi-DSI 4 lanes
> - 1x PCIe open ended
> 
> Signed-off-by: Paul M. B. Bendixen <pbe@trifork.com>
> ---
>    README                                      |  1 +
>    conf/machine/include/soquartz.inc           | 15 +++++++++++++++
>    conf/machine/soquartz-model-a.conf          | 12 ++++++++++++
>    recipes-kernel/linux/linux-yocto_%.bbappend |  1 +
>    4 files changed, 29 insertions(+)
>    create mode 100644 conf/machine/include/soquartz.inc
>    create mode 100644 conf/machine/soquartz-model-a.conf
> 
> diff --git a/README b/README
> index 09f8691..85ab2ec 100644
> --- a/README
> +++ b/README
> @@ -45,6 +45,7 @@ Status of supported boards:
>                rock-pi-e
>                rock-pi-s
>                rock64
> +            soquartz (on model A base board)
> 
>        builds:
>            32-bit:
> diff --git a/conf/machine/include/soquartz.inc
> b/conf/machine/include/soquartz.inc
> new file mode 100644
> index 0000000..6714efc
> --- /dev/null
> +++ b/conf/machine/include/soquartz.inc
> @@ -0,0 +1,15 @@
> +#Copyright (C) Trifork A/S 2024
> +# Released under the MIT license (see COPYING.MIT for the terms)
> +
> +# Common override for basing a SOQuartz design on
> +# https://wiki.pine64.org/wiki/SOQuartz
> +
> +MACHINEOVERRIDES =. "soquartz:"
> +
> +require conf/machine/include/rk3566.inc
> +
> +# A machine using this baseboard could use these options or overload them

I think it is actually required to set those variables in a machine 
configuration file since nothing else defines a default? And since those 
are now commented out (as they should).

So I'm just nitpicking on the phrasing here. I wouldn't necessarily send 
a new version just for that, but maybe Trevor will have a different 
opinion (or fix it when merging).

Looks good to me otherwise!

Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>

Thanks!
Quentin