Re: [PATCH V3 7/8] ARM: dts: Add minimal Raspberry Pi 4 support

[Florian Fainelli <f.fainelli@gmail.com>]

On 9/28/2019 5:07 AM, Stefan Wahren wrote:
> This adds minimal support for the new Raspberry Pi 4 without the
> fancy stuff like GENET, PCIe, xHCI, 40 bit DMA and V3D. The RPi 4 is
> available in 3 different variants (1, 2 and 4 GB RAM), so leave the memory
> size to zero and let the bootloader take care of it. The DWC2 is still
> usable as peripheral via the USB-C port.

That comment is useful, and probably belongs where the memory node is
declared, see below.

> 
> Other differences to the Raspberry Pi 3:
> - additional GIC 400 Interrupt controller
> - new thermal IP and HWRNG
> - additional MMC interface (emmc2)
> - additional UART, I2C, SPI and PWM interfaces
> - clock stretching bug in I2C IP has been fixed
> 
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
> Acked-by: Eric Anholt <eric@anholt.net>
> ---

[snip]

> +/ {
> +	compatible = "raspberrypi,4-model-b", "brcm,bcm2711";
> +	model = "Raspberry Pi 4 Model B";
> +
> +	chosen {
> +		/* 8250 auxiliary UART instead of pl011 */
> +		stdout-path = "serial1:115200n8";
> +	};
> +

Might be worth a comment that the reg property of the memory node is
filed by the boot loader based on the populated amount of DRAM. You can
also go with a shorter format for the size (0)?

> +	memory@0 {
> +		device_type = "memory";
> +		reg = <0 0 0x00000000>;
> +	};
> +

[snip]

> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/soc/bcm2835-pm.h>
> +
> +/ {
> +	compatible = "brcm,bcm2711";
> +
> +	#address-cells = <2>;
> +	#size-cells = <1>;

Trying to see if we may need a #size-cells property value of 2 here, for
the 4GB model, I would assume that we would have to, unless we are fine
with supporting 4GB - 1byte of DRAM?

> +
> +	interrupt-parent = <&gicv2>;
> +
> +	soc {
> +		ranges = <0x7e000000  0x0 0xfe000000  0x01800000>,
> +			 <0x7c000000  0x0 0xfc000000  0x02000000>,
> +			 <0x40000000  0x0 0xff800000  0x00800000>;

Might be nice to get some comments about

> +		/* Emulate a contiguous 30-bit address range for DMA */
> +		dma-ranges = <0xc0000000  0x0 0x00000000  0x3c000000>;
> +
> +		local_intc: local_intc@40000000 {
> +			compatible = "brcm,bcm2836-l1-intc";
> +			reg = <0x40000000 0x100>;
> +		};

This deserves a comment to indicate that this node is the provider for
the enable-method for bringing up secondary cores. And no PSCI, still,
what year is this?

[snip]

> +		rng@7e104000 {
> +			interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>;
> +
> +			/* RNG is incompatible to brcm,bcm2835-rng */

Nit: s/to/with/

[snip]

> +		spi@7e204000 {
> +			reg = <0x7e204000 0x0200>;
> +			interrupts = <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>;
> +		};

Why is this SPI node incomplete, are you just overriding a previously
defined node here?

[snip]

> +
> +	timer {
> +		compatible = "arm,armv8-timer";
> +		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) |
> +					  IRQ_TYPE_LEVEL_LOW)>,
> +			     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) |
> +					  IRQ_TYPE_LEVEL_LOW)>,
> +			     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) |
> +					  IRQ_TYPE_LEVEL_LOW)>,
> +			     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) |
> +					  IRQ_TYPE_LEVEL_LOW)>;
> +		/* This only applies to the ARMv7 stub */
> +		arm,cpu-registers-not-fw-configured;
> +
> +		/* The ARM cores doesn't enter deep enough states */

Nit: s/doesn't/do not/
-- 
Florian

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel