Re: [PATCH v2 4/4] arm64: dts: renesas: add support for solidrun rzg2lc som and hb-iiot evb

Am 12.05.26 um 05:18 schrieb [email protected]:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [Low] Missing unit address for reserved-memory node with a `reg` property.
> - [Low] Inconsistent use of `+=` instead of `:=` for Kbuild composite DTB definitions.
> - [Low] Redundant addition of shared `.dtbo` overlay targets to the Makefile variable.
> --
>
> commit 8d9d2cbc7a77754ee19950ca31a17d0916b4b627
> Author: Josua Mayer <[email protected]>
> arm64: dts: renesas: add support for solidrun rzg2lc som and hb-iiot evb
>
> Add support for the SolidRun RZ/G2LC SoM on Hummingboard IIoT, which
> features various peripherals including Ethernet, eMMC, and WiFi.
> The RZ/G2LC SoM is pin-compatible with the G2L SoM with a slightly
> reduced feature set.
>
> Link: https://www.solid-run.com/embedded-industrial-iot/renesas-rz-family/rz-g2lc-som/
> Link: https://www.solid-run.com/embedded-industrial-iot/renesas-rz-family/hummingboard-rz-series-sbcs/hummingboard-rz-g2l-iot-sbc/
>
>> diff --git a/arch/arm64/boot/dts/renesas/Makefile b/arch/arm64/boot/dts/renesas/Makefile
>> --- a/arch/arm64/boot/dts/renesas/Makefile
>> +++ b/arch/arm64/boot/dts/renesas/Makefile
>> @@ -164,6 +164,17 @@ dtb-$(CONFIG_ARCH_R9A07G043) += r9a07g043u11-smarc-du-adv7513.dtb
>>  r9a07g043u11-smarc-pmod-dtbs := r9a07g043u11-smarc.dtb r9a07g043-smarc-pmod.dtbo
>>  dtb-$(CONFIG_ARCH_R9A07G043) += r9a07g043u11-smarc-pmod.dtb
>>  
>> +dtb-$(CONFIG_ARCH_R9A07G044) += r9a07g044c2-hummingboard-iiot.dtb
>> +r9a07g044c2-hummingboard-iiot-emmc-dtbs += r9a07g044c2-hummingboard-iiot.dtb rzg2l-sr-som-emmc.dtbo
> This isn't a bug, but should this use := instead of += for consistency with
> standard Kbuild practice and other composite DTB definitions in this Makefile?
Queued vor v3.
>
>> +dtb-$(CONFIG_ARCH_R9A07G044) += r9a07g044c2-hummingboard-iiot-emmc.dtb
>> +r9a07g044c2-hummingboard-iiot-microsd-dtbs += r9a07g044c2-hummingboard-iiot.dtb rzg2l-hummingboard-iiot-microsd.dtbo
>> +dtb-$(CONFIG_ARCH_R9A07G044) += r9a07g044c2-hummingboard-iiot-microsd.dtb
>> +dtb-$(CONFIG_ARCH_R9A07G044) += rzg2l-hummingboard-iiot-rs485-a.dtbo
> This isn't a bug, but since rzg2l-hummingboard-iiot-rs485-a.dtbo and
> rzg2l-hummingboard-iiot-rs485-b.dtbo are already appended to
> dtb-$(CONFIG_ARCH_R9A07G044) for the shared r9a07g044l2 SoC variant, is it
> redundant to include them here?

Yes, redundant as both G2L and G2LC are CONFIG_ARCH_R9A07G044.

Queued vor v3.

>
>> +r9a07g044c2-hummingboard-iiot-rs485-a-dtbs += r9a07g044c2-hummingboard-iiot.dtb rzg2l-hummingboard-iiot-rs485-a.dtbo
>> +dtb-$(CONFIG_ARCH_R9A07G044) += r9a07g044c2-hummingboard-iiot-rs485-a.dtb
>> +dtb-$(CONFIG_ARCH_R9A07G044) += rzg2l-hummingboard-iiot-rs485-b.dtbo
> [ ... ]
>
>> diff --git a/arch/arm64/boot/dts/renesas/rzg2lc-sr-som.dtsi b/arch/arm64/boot/dts/renesas/rzg2lc-sr-som.dtsi
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/renesas/rzg2lc-sr-som.dtsi
>> @@ -0,0 +1,373 @@
> [ ... ]
>
>> +	reserved-memory {
>> +		ranges;
>> +		#address-cells = <2>;
>> +		#size-cells = <2>;
>> +
>> +		mmp_reserved: linux,multimedia {
>> +			compatible = "shared-dma-pool";
>> +			reg = <0x0 0x68000000 0x0 0x8000000>;
>> +			reusable;
>> +		};
> Does this node need a unit address matching the first address of the reg block
> (e.g., linux,multimedia@68000000) to prevent a unit_address_vs_reg DTC
> compiler warning?
Probably, but the usage of pattern "linux,.+@" (i.e. with address suffix) is extremely rare
in existing device-tree, on v7.1-rc1:

❯ grep -r -E "linux,.+@" arch/arm64/boot/dts/
❯ grep -r -E "linux,.+@" arch/arm/boot/dts/
arch/arm/boot/dts/nvidia/tegra114-asus-tf701t.dts:              linux,cma@80000000 {
arch/arm/boot/dts/nvidia/tegra20-acer-a500-picasso.dts:         linux,cma@30000000 {
arch/arm/boot/dts/nvidia/tegra30-asus-nexus7-grouper-common.dtsi:               linux,cma@80000000 {
arch/arm/boot/dts/nvidia/tegra30-asus-p1801-t.dts:              linux,cma@80000000 {
arch/arm/boot/dts/nvidia/tegra30-asus-transformer-common.dtsi:          linux,cma@80000000 {
arch/arm/boot/dts/nvidia/tegra30-ouya.dts:              linux,cma@80000000 {
arch/arm/boot/dts/nvidia/tegra30-pegatron-chagall.dts:          linux,cma@80000000 {
arch/arm/boot/dts/nvidia/tegra20-asus-transformer-common.dtsi:          linux,cma@30000000 {
arch/arm/boot/dts/nvidia/tegra30-asus-tf600t.dts:               linux,cma@80000000 {
arch/arm/boot/dts/nvidia/tegra30-lg-x3.dtsi:            linux,cma@80000000 {

No objection to adding the address suffix to linux,multimedia node though.

>
>> +
>> +		global_cma: linux,cma@58000000 {
>> +			compatible = "shared-dma-pool";
>> +			reg = <0x0 0x58000000 0x0 0x10000000>;
>> +			reusable;