[PATCH] ARM: dts: Add Raspberry Pi Compute Module 4 CANOPi Board

[PATCH] ARM: dts: Add Raspberry Pi Compute Module 4 CANOPi Board

[Ariel D'Alessandro]

The Eclipse KUKSA CANOPi [0] is a baseboard for the Raspberry Compute
Module 4 (CM4). It contains a VIA VL805 4 Port USB controller and two
MCP251xFD based CAN-FD interfaces.

[0] https://github.com/boschresearch/kuksa.hardware

Signed-off-by: Ariel D'Alessandro <ariel.dalessandro@collabora.com>
---
 arch/arm/boot/dts/Makefile                    |   1 +
 arch/arm/boot/dts/bcm2711-rpi-cm4-canopi.dts  | 139 ++++++++++++++++++
 arch/arm64/boot/dts/broadcom/Makefile         |   1 +
 .../dts/broadcom/bcm2711-rpi-cm4-canopi.dts   |   2 +
 4 files changed, 143 insertions(+)
 create mode 100644 arch/arm/boot/dts/bcm2711-rpi-cm4-canopi.dts
 create mode 100644 arch/arm64/boot/dts/broadcom/bcm2711-rpi-cm4-canopi.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 05d8aef6e5d2..8930ab2c132c 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -98,6 +98,7 @@ dtb-$(CONFIG_ARCH_BCM2835) += \
 	bcm2837-rpi-zero-2-w.dtb \
 	bcm2711-rpi-400.dtb \
 	bcm2711-rpi-4-b.dtb \
+	bcm2711-rpi-cm4-canopi.dtb \
 	bcm2711-rpi-cm4-io.dtb \
 	bcm2835-rpi-zero.dtb \
 	bcm2835-rpi-zero-w.dtb
diff --git a/arch/arm/boot/dts/bcm2711-rpi-cm4-canopi.dts b/arch/arm/boot/dts/bcm2711-rpi-cm4-canopi.dts
new file mode 100644
index 000000000000..52ec5908883c
--- /dev/null
+++ b/arch/arm/boot/dts/bcm2711-rpi-cm4-canopi.dts
@@ -0,0 +1,139 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+#include "bcm2711-rpi-cm4.dtsi"
+
+/ {
+	model = "Raspberry Pi Compute Module 4 CANOPi Board";
+
+	clocks {
+		clk_mcp251xfd_osc: mcp251xfd-osc {
+			#clock-cells = <0>;
+			compatible = "fixed-clock";
+			clock-frequency = <20000000>;
+		};
+	};
+
+	leds {
+		led-act {
+			gpios = <&gpio 42 GPIO_ACTIVE_HIGH>;
+		};
+
+		led-pwr {
+			label = "PWR";
+			gpios = <&expgpio 2 GPIO_ACTIVE_LOW>;
+			default-state = "keep";
+			linux,default-trigger = "default-on";
+		};
+	};
+};
+
+&ddc0 {
+	status = "okay";
+};
+
+&ddc1 {
+	status = "okay";
+};
+
+&hdmi0 {
+	status = "okay";
+};
+
+&hdmi1 {
+	status = "okay";
+};
+
+&i2c0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c0_gpio44>;
+	status = "okay";
+	clock-frequency = <100000>;
+
+	pcf85063a@51 {
+		compatible = "nxp,pcf85063a";
+		reg = <0x51>;
+	};
+};
+
+&pcie0 {
+	pci@0,0 {
+		device_type = "pci";
+		#address-cells = <3>;
+		#size-cells = <2>;
+		ranges;
+
+		reg = <0 0 0 0 0>;
+
+		usb@0,0 {
+			reg = <0 0 0 0 0>;
+			resets = <&reset RASPBERRYPI_FIRMWARE_RESET_ID_USB>;
+		};
+	};
+};
+
+&pixelvalve0 {
+	status = "okay";
+};
+
+&pixelvalve1 {
+	status = "okay";
+};
+
+&pixelvalve2 {
+	status = "okay";
+};
+
+&pixelvalve4 {
+	status = "okay";
+};
+
+&spi {
+	status = "okay";
+	pinctrl-names = "default";
+	pinctrl-0 = <&spi0_gpio7>;
+	cs-gpios = <&gpio 8 1>, <&gpio 7 1>;
+	dmas = <&dma 6>, <&dma 7>;
+	dma-names = "tx", "rx";
+
+	mcp251xfd0: mcp251xfd@0 {
+		compatible = "microchip,mcp251xfd";
+		reg = <0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&mcp251xfd0_pins>;
+		spi-max-frequency = <20000000>;
+		interrupt-parent = <&gpio>;
+		interrupts = <27 IRQ_TYPE_LEVEL_LOW>;
+		clocks = <&clk_mcp251xfd_osc>;
+	};
+
+	mcp251xfd1: mcp251xfd@1 {
+		compatible = "microchip,mcp251xfd";
+		reg = <1>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&mcp251xfd1_pins>;
+		spi-max-frequency = <20000000>;
+		interrupt-parent = <&gpio>;
+		interrupts = <22 IRQ_TYPE_LEVEL_LOW>;
+		clocks = <&clk_mcp251xfd_osc>;
+	};
+};
+
+&gpio {
+	mcp251xfd0_pins: mcp251xfd0_pins {
+		brcm,pins = <27>;
+		brcm,function = <BCM2835_FSEL_GPIO_IN>;
+	};
+
+	mcp251xfd1_pins: mcp251xfd1_pins {
+		brcm,pins = <22>;
+		brcm,function = <BCM2835_FSEL_GPIO_IN>;
+	};
+};
+
+&vc4 {
+	status = "okay";
+};
+
+&vec {
+	status = "disabled";
+};
diff --git a/arch/arm64/boot/dts/broadcom/Makefile b/arch/arm64/boot/dts/broadcom/Makefile
index e8584d3b698f..7cd88b8c0345 100644
--- a/arch/arm64/boot/dts/broadcom/Makefile
+++ b/arch/arm64/boot/dts/broadcom/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 dtb-$(CONFIG_ARCH_BCM2835) += bcm2711-rpi-400.dtb \
 			      bcm2711-rpi-4-b.dtb \
+			      bcm2711-rpi-cm4-canopi.dtb \
 			      bcm2711-rpi-cm4-io.dtb \
 			      bcm2837-rpi-3-a-plus.dtb \
 			      bcm2837-rpi-3-b.dtb \
diff --git a/arch/arm64/boot/dts/broadcom/bcm2711-rpi-cm4-canopi.dts b/arch/arm64/boot/dts/broadcom/bcm2711-rpi-cm4-canopi.dts
new file mode 100644
index 000000000000..e9369aa0eb39
--- /dev/null
+++ b/arch/arm64/boot/dts/broadcom/bcm2711-rpi-cm4-canopi.dts
@@ -0,0 +1,2 @@
+// SPDX-License-Identifier: GPL-2.0
+#include "arm/bcm2711-rpi-cm4-canopi.dts"
-- 
2.37.2

Re: [PATCH] ARM: dts: Add Raspberry Pi Compute Module 4 CANOPi Board

[Stefan Wahren]

Hi Ariel,

Am 16.09.22 um 17:31 schrieb Ariel D'Alessandro:
> The Eclipse KUKSA CANOPi [0] is a baseboard for the Raspberry Compute
> Module 4 (CM4). It contains a VIA VL805 4 Port USB controller and two
> MCP251xFD based CAN-FD interfaces.

this is a cool piece of hardware :-)

Is it correct this baseboard is only intended for Compute Modules 
without Wifi/BT? Otherwise we get conflicts with UART0. The 
bcm2711-rpi-cm4.dtsi is currently written for all Compute Module 
variants. A possible solution is to use delete-node, another cleaner 
ones is to split bcm2711-rpi-cm4 into wifi and non-wifi variants and 
include the non-wifi one in your case.

>
> [0] https://github.com/boschresearch/kuksa.hardware
>
> Signed-off-by: Ariel D'Alessandro <ariel.dalessandro@collabora.com>
> ---
>   arch/arm/boot/dts/Makefile                    |   1 +
>   arch/arm/boot/dts/bcm2711-rpi-cm4-canopi.dts  | 139 ++++++++++++++++++
>   arch/arm64/boot/dts/broadcom/Makefile         |   1 +
>   .../dts/broadcom/bcm2711-rpi-cm4-canopi.dts   |   2 +
>   4 files changed, 143 insertions(+)
>   create mode 100644 arch/arm/boot/dts/bcm2711-rpi-cm4-canopi.dts
>   create mode 100644 arch/arm64/boot/dts/broadcom/bcm2711-rpi-cm4-canopi.dts
>
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 05d8aef6e5d2..8930ab2c132c 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -98,6 +98,7 @@ dtb-$(CONFIG_ARCH_BCM2835) += \
>   	bcm2837-rpi-zero-2-w.dtb \
>   	bcm2711-rpi-400.dtb \
>   	bcm2711-rpi-4-b.dtb \
> +	bcm2711-rpi-cm4-canopi.dtb \
>   	bcm2711-rpi-cm4-io.dtb \
>   	bcm2835-rpi-zero.dtb \
>   	bcm2835-rpi-zero-w.dtb
> diff --git a/arch/arm/boot/dts/bcm2711-rpi-cm4-canopi.dts b/arch/arm/boot/dts/bcm2711-rpi-cm4-canopi.dts
> new file mode 100644
> index 000000000000..52ec5908883c
> --- /dev/null
> +++ b/arch/arm/boot/dts/bcm2711-rpi-cm4-canopi.dts
> @@ -0,0 +1,139 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/dts-v1/;
> +#include "bcm2711-rpi-cm4.dtsi"
> +
> +/ {
> +	model = "Raspberry Pi Compute Module 4 CANOPi Board";
> +
> +	clocks {
> +		clk_mcp251xfd_osc: mcp251xfd-osc {
> +			#clock-cells = <0>;
> +			compatible = "fixed-clock";
> +			clock-frequency = <20000000>;
> +		};
> +	};
> +
> +	leds {
> +		led-act {
> +			gpios = <&gpio 42 GPIO_ACTIVE_HIGH>;
> +		};
> +
> +		led-pwr {
> +			label = "PWR";
> +			gpios = <&expgpio 2 GPIO_ACTIVE_LOW>;
> +			default-state = "keep";
> +			linux,default-trigger = "default-on";
> +		};
> +	};
are these LEDs really populated and wired to the BCM2711? The schematics 
suggests that they are connected to the STN2120.
> +};
> +
> +&ddc0 {
> +	status = "okay";
> +};
> +
> +&ddc1 {
> +	status = "okay";
> +};
> +
> +&hdmi0 {
> +	status = "okay";
> +};
> +
> +&hdmi1 {
> +	status = "okay";
> +};
I cannot see any graphical interface in the schematics. So why they are 
enabled?
> +
> +&i2c0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&i2c0_gpio44>;
> +	status = "okay";
> +	clock-frequency = <100000>;
> +
> +	pcf85063a@51 {
Please use the actual function for the node name like rtc@51
> +		compatible = "nxp,pcf85063a";
> +		reg = <0x51>;
> +	};
> +};
> +
> +&pcie0 {
> +	pci@0,0 {
> +		device_type = "pci";
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		reg = <0 0 0 0 0>;
> +
> +		usb@0,0 {
> +			reg = <0 0 0 0 0>;
> +			resets = <&reset RASPBERRYPI_FIRMWARE_RESET_ID_USB>;
> +		};
> +	};
> +};
> +
> +&pixelvalve0 {
> +	status = "okay";
> +};
> +
> +&pixelvalve1 {
> +	status = "okay";
> +};
> +
> +&pixelvalve2 {
> +	status = "okay";
> +};
> +
> +&pixelvalve4 {
> +	status = "okay";
> +};
Without a graphical interface they shouldn't be necessary?
> +
> +&spi {
> +	status = "okay";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&spi0_gpio7>;
> +	cs-gpios = <&gpio 8 1>, <&gpio 7 1>;
> +	dmas = <&dma 6>, <&dma 7>;
> +	dma-names = "tx", "rx";
> +
> +	mcp251xfd0: mcp251xfd@0 {
mcp251xfd0: can@0
> +		compatible = "microchip,mcp251xfd";
> +		reg = <0>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&mcp251xfd0_pins>;
> +		spi-max-frequency = <20000000>;

I wasn't good at physics, but having spi-max-frequency equal to the 
oscillator frequency seems wrong. Is it because of the hack in the 
downstream kernel which halves the SPI frequency?

Just guessing because imx6qp-prtwd3.dts uses 10 MHz.

> +		interrupt-parent = <&gpio>;
> +		interrupts = <27 IRQ_TYPE_LEVEL_LOW>;
> +		clocks = <&clk_mcp251xfd_osc>;
> +	};
> +
> +	mcp251xfd1: mcp251xfd@1 {
mcp251xfd1: can@1
> +		compatible = "microchip,mcp251xfd";
> +		reg = <1>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&mcp251xfd1_pins>;
> +		spi-max-frequency = <20000000>;
> +		interrupt-parent = <&gpio>;
> +		interrupts = <22 IRQ_TYPE_LEVEL_LOW>;
> +		clocks = <&clk_mcp251xfd_osc>;
> +	};
> +};
> +
> +&gpio {

In case there are any GPIOs which should be controlled via user space 
(like LTE or FAN control), it would be nice to describe them via 
gpio-line-names.

> +	mcp251xfd0_pins: mcp251xfd0_pins {
> +		brcm,pins = <27>;
> +		brcm,function = <BCM2835_FSEL_GPIO_IN>;
> +	};

The vendor specific pin properties are deprecated for BCM2711. We have 
generic ones for this:

mcp251xfd0_pins: mcp251xfd0_pins {
         pin-irq {
             pins = "gpio27";
             function = "gpio_in";
         };
     };

> +
> +	mcp251xfd1_pins: mcp251xfd1_pins {
> +		brcm,pins = <22>;
> +		brcm,function = <BCM2835_FSEL_GPIO_IN>;
> +	};
dito
> +};
> +
> +&vc4 {
> +	status = "okay";
> +};
I think this is also not necessary for a headless device.
> +
> +&vec {
> +	status = "disabled";
> +};
> diff --git a/arch/arm64/boot/dts/broadcom/Makefile b/arch/arm64/boot/dts/broadcom/Makefile
> index e8584d3b698f..7cd88b8c0345 100644
> --- a/arch/arm64/boot/dts/broadcom/Makefile
> +++ b/arch/arm64/boot/dts/broadcom/Makefile
> @@ -1,6 +1,7 @@
>   # SPDX-License-Identifier: GPL-2.0
>   dtb-$(CONFIG_ARCH_BCM2835) += bcm2711-rpi-400.dtb \
>   			      bcm2711-rpi-4-b.dtb \
> +			      bcm2711-rpi-cm4-canopi.dtb \
>   			      bcm2711-rpi-cm4-io.dtb \
>   			      bcm2837-rpi-3-a-plus.dtb \
>   			      bcm2837-rpi-3-b.dtb \
> diff --git a/arch/arm64/boot/dts/broadcom/bcm2711-rpi-cm4-canopi.dts b/arch/arm64/boot/dts/broadcom/bcm2711-rpi-cm4-canopi.dts
> new file mode 100644
> index 000000000000..e9369aa0eb39
> --- /dev/null
> +++ b/arch/arm64/boot/dts/broadcom/bcm2711-rpi-cm4-canopi.dts
> @@ -0,0 +1,2 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include "arm/bcm2711-rpi-cm4-canopi.dts"

Re: [PATCH] ARM: dts: Add Raspberry Pi Compute Module 4 CANOPi Board

[Alexander Dahl]

Hei hei,

Am Fri, Sep 16, 2022 at 12:31:56PM -0300 schrieb Ariel D'Alessandro:
> The Eclipse KUKSA CANOPi [0] is a baseboard for the Raspberry Compute
> Module 4 (CM4). It contains a VIA VL805 4 Port USB controller and two
> MCP251xFD based CAN-FD interfaces.
> 
> [0] https://github.com/boschresearch/kuksa.hardware
> 
> Signed-off-by: Ariel D'Alessandro <ariel.dalessandro@collabora.com>
> ---
>  arch/arm/boot/dts/Makefile                    |   1 +
>  arch/arm/boot/dts/bcm2711-rpi-cm4-canopi.dts  | 139 ++++++++++++++++++
>  arch/arm64/boot/dts/broadcom/Makefile         |   1 +
>  .../dts/broadcom/bcm2711-rpi-cm4-canopi.dts   |   2 +
>  4 files changed, 143 insertions(+)
>  create mode 100644 arch/arm/boot/dts/bcm2711-rpi-cm4-canopi.dts
>  create mode 100644 arch/arm64/boot/dts/broadcom/bcm2711-rpi-cm4-canopi.dts
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 05d8aef6e5d2..8930ab2c132c 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -98,6 +98,7 @@ dtb-$(CONFIG_ARCH_BCM2835) += \
>  	bcm2837-rpi-zero-2-w.dtb \
>  	bcm2711-rpi-400.dtb \
>  	bcm2711-rpi-4-b.dtb \
> +	bcm2711-rpi-cm4-canopi.dtb \
>  	bcm2711-rpi-cm4-io.dtb \
>  	bcm2835-rpi-zero.dtb \
>  	bcm2835-rpi-zero-w.dtb
> diff --git a/arch/arm/boot/dts/bcm2711-rpi-cm4-canopi.dts b/arch/arm/boot/dts/bcm2711-rpi-cm4-canopi.dts
> new file mode 100644
> index 000000000000..52ec5908883c
> --- /dev/null
> +++ b/arch/arm/boot/dts/bcm2711-rpi-cm4-canopi.dts
> @@ -0,0 +1,139 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/dts-v1/;
> +#include "bcm2711-rpi-cm4.dtsi"
> +
> +/ {
> +	model = "Raspberry Pi Compute Module 4 CANOPi Board";
> +
> +	clocks {
> +		clk_mcp251xfd_osc: mcp251xfd-osc {
> +			#clock-cells = <0>;
> +			compatible = "fixed-clock";
> +			clock-frequency = <20000000>;
> +		};
> +	};
> +
> +	leds {
> +		led-act {
> +			gpios = <&gpio 42 GPIO_ACTIVE_HIGH>;
> +		};
> +
> +		led-pwr {
> +			label = "PWR";
> +			gpios = <&expgpio 2 GPIO_ACTIVE_LOW>;
> +			default-state = "keep";
> +			linux,default-trigger = "default-on";
> +		};
> +	};

This looks like using the node name and the deprecated "label"
property for LED naming.  Please see
Documentation/devicetree/bindings/leds/common.yaml and use the
properties "function" and "color" instead.  Also check the node names
itself, see the example in that binding or the leds-gpio binding for
reference.

Greets
Alex

> +};
> +
> +&ddc0 {
> +	status = "okay";
> +};
> +
> +&ddc1 {
> +	status = "okay";
> +};
> +
> +&hdmi0 {
> +	status = "okay";
> +};
> +
> +&hdmi1 {
> +	status = "okay";
> +};
> +
> +&i2c0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&i2c0_gpio44>;
> +	status = "okay";
> +	clock-frequency = <100000>;
> +
> +	pcf85063a@51 {
> +		compatible = "nxp,pcf85063a";
> +		reg = <0x51>;
> +	};
> +};
> +
> +&pcie0 {
> +	pci@0,0 {
> +		device_type = "pci";
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		reg = <0 0 0 0 0>;
> +
> +		usb@0,0 {
> +			reg = <0 0 0 0 0>;
> +			resets = <&reset RASPBERRYPI_FIRMWARE_RESET_ID_USB>;
> +		};
> +	};
> +};
> +
> +&pixelvalve0 {
> +	status = "okay";
> +};
> +
> +&pixelvalve1 {
> +	status = "okay";
> +};
> +
> +&pixelvalve2 {
> +	status = "okay";
> +};
> +
> +&pixelvalve4 {
> +	status = "okay";
> +};
> +
> +&spi {
> +	status = "okay";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&spi0_gpio7>;
> +	cs-gpios = <&gpio 8 1>, <&gpio 7 1>;
> +	dmas = <&dma 6>, <&dma 7>;
> +	dma-names = "tx", "rx";
> +
> +	mcp251xfd0: mcp251xfd@0 {
> +		compatible = "microchip,mcp251xfd";
> +		reg = <0>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&mcp251xfd0_pins>;
> +		spi-max-frequency = <20000000>;
> +		interrupt-parent = <&gpio>;
> +		interrupts = <27 IRQ_TYPE_LEVEL_LOW>;
> +		clocks = <&clk_mcp251xfd_osc>;
> +	};
> +
> +	mcp251xfd1: mcp251xfd@1 {
> +		compatible = "microchip,mcp251xfd";
> +		reg = <1>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&mcp251xfd1_pins>;
> +		spi-max-frequency = <20000000>;
> +		interrupt-parent = <&gpio>;
> +		interrupts = <22 IRQ_TYPE_LEVEL_LOW>;
> +		clocks = <&clk_mcp251xfd_osc>;
> +	};
> +};
> +
> +&gpio {
> +	mcp251xfd0_pins: mcp251xfd0_pins {
> +		brcm,pins = <27>;
> +		brcm,function = <BCM2835_FSEL_GPIO_IN>;
> +	};
> +
> +	mcp251xfd1_pins: mcp251xfd1_pins {
> +		brcm,pins = <22>;
> +		brcm,function = <BCM2835_FSEL_GPIO_IN>;
> +	};
> +};
> +
> +&vc4 {
> +	status = "okay";
> +};
> +
> +&vec {
> +	status = "disabled";
> +};
> diff --git a/arch/arm64/boot/dts/broadcom/Makefile b/arch/arm64/boot/dts/broadcom/Makefile
> index e8584d3b698f..7cd88b8c0345 100644
> --- a/arch/arm64/boot/dts/broadcom/Makefile
> +++ b/arch/arm64/boot/dts/broadcom/Makefile
> @@ -1,6 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0
>  dtb-$(CONFIG_ARCH_BCM2835) += bcm2711-rpi-400.dtb \
>  			      bcm2711-rpi-4-b.dtb \
> +			      bcm2711-rpi-cm4-canopi.dtb \
>  			      bcm2711-rpi-cm4-io.dtb \
>  			      bcm2837-rpi-3-a-plus.dtb \
>  			      bcm2837-rpi-3-b.dtb \
> diff --git a/arch/arm64/boot/dts/broadcom/bcm2711-rpi-cm4-canopi.dts b/arch/arm64/boot/dts/broadcom/bcm2711-rpi-cm4-canopi.dts
> new file mode 100644
> index 000000000000..e9369aa0eb39
> --- /dev/null
> +++ b/arch/arm64/boot/dts/broadcom/bcm2711-rpi-cm4-canopi.dts
> @@ -0,0 +1,2 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include "arm/bcm2711-rpi-cm4-canopi.dts"
> -- 
> 2.37.2
> 

Re: [PATCH] ARM: dts: Add Raspberry Pi Compute Module 4 CANOPi Board

[Stefan Wahren]

Hi Alexander,

[fix address of Krzysztof]

Am 19.09.22 um 09:47 schrieb Alexander Dahl:
> Hei hei,
>
> Am Fri, Sep 16, 2022 at 12:31:56PM -0300 schrieb Ariel D'Alessandro:
>> The Eclipse KUKSA CANOPi [0] is a baseboard for the Raspberry Compute
>> Module 4 (CM4). It contains a VIA VL805 4 Port USB controller and two
>> MCP251xFD based CAN-FD interfaces.
>>
>> [0] https://github.com/boschresearch/kuksa.hardware
>>
>> Signed-off-by: Ariel D'Alessandro <ariel.dalessandro@collabora.com>
>> ---
>>   arch/arm/boot/dts/Makefile                    |   1 +
>>   arch/arm/boot/dts/bcm2711-rpi-cm4-canopi.dts  | 139 ++++++++++++++++++
>>   arch/arm64/boot/dts/broadcom/Makefile         |   1 +
>>   .../dts/broadcom/bcm2711-rpi-cm4-canopi.dts   |   2 +
>>   4 files changed, 143 insertions(+)
>>   create mode 100644 arch/arm/boot/dts/bcm2711-rpi-cm4-canopi.dts
>>   create mode 100644 arch/arm64/boot/dts/broadcom/bcm2711-rpi-cm4-canopi.dts
>>
>> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
>> index 05d8aef6e5d2..8930ab2c132c 100644
>> --- a/arch/arm/boot/dts/Makefile
>> +++ b/arch/arm/boot/dts/Makefile
>> @@ -98,6 +98,7 @@ dtb-$(CONFIG_ARCH_BCM2835) += \
>>   	bcm2837-rpi-zero-2-w.dtb \
>>   	bcm2711-rpi-400.dtb \
>>   	bcm2711-rpi-4-b.dtb \
>> +	bcm2711-rpi-cm4-canopi.dtb \
>>   	bcm2711-rpi-cm4-io.dtb \
>>   	bcm2835-rpi-zero.dtb \
>>   	bcm2835-rpi-zero-w.dtb
>> diff --git a/arch/arm/boot/dts/bcm2711-rpi-cm4-canopi.dts b/arch/arm/boot/dts/bcm2711-rpi-cm4-canopi.dts
>> new file mode 100644
>> index 000000000000..52ec5908883c
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/bcm2711-rpi-cm4-canopi.dts
>> @@ -0,0 +1,139 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/dts-v1/;
>> +#include "bcm2711-rpi-cm4.dtsi"
>> +
>> +/ {
>> +	model = "Raspberry Pi Compute Module 4 CANOPi Board";
>> +
>> +	clocks {
>> +		clk_mcp251xfd_osc: mcp251xfd-osc {
>> +			#clock-cells = <0>;
>> +			compatible = "fixed-clock";
>> +			clock-frequency = <20000000>;
>> +		};
>> +	};
>> +
>> +	leds {
>> +		led-act {
>> +			gpios = <&gpio 42 GPIO_ACTIVE_HIGH>;
>> +		};
>> +
>> +		led-pwr {
>> +			label = "PWR";
>> +			gpios = <&expgpio 2 GPIO_ACTIVE_LOW>;
>> +			default-state = "keep";
>> +			linux,default-trigger = "default-on";
>> +		};
>> +	};
> This looks like using the node name and the deprecated "label"
> property for LED naming.  Please see
> Documentation/devicetree/bindings/leds/common.yaml and use the
> properties "function" and "color" instead.  Also check the node names
> itself, see the example in that binding or the leds-gpio binding for
> reference.

Oops, i didn't noticed this.

Unfortunately the ACT-LED is already a little bit opaque defined in 
bcm2835-rpi.dtsi:

leds {
         compatible = "gpio-leds";

         led-act {
             label = "ACT";
             default-state = "keep";
             linux,default-trigger = "heartbeat";
         };
};

So a reference (currently missing) would have make it clear that the 
ACT-LED is common for all Raspberry Pi boards.

So you wish that this is fixed for the CANOPi board or all Raspberry Pi 
boards?

I'm asking because switching to function would change the sysfs path and 
breaking userspace ABI.

>
> Greets
> Alex
>
>> +};
>> +
>> +&ddc0 {
>> +	status = "okay";
>> +};
>> +
>> +&ddc1 {
>> +	status = "okay";
>> +};
>> +
>> +&hdmi0 {
>> +	status = "okay";
>> +};
>> +
>> +&hdmi1 {
>> +	status = "okay";
>> +};
>> +
>> +&i2c0 {
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&i2c0_gpio44>;
>> +	status = "okay";
>> +	clock-frequency = <100000>;
>> +
>> +	pcf85063a@51 {
>> +		compatible = "nxp,pcf85063a";
>> +		reg = <0x51>;
>> +	};
>> +};
>> +
>> +&pcie0 {
>> +	pci@0,0 {
>> +		device_type = "pci";
>> +		#address-cells = <3>;
>> +		#size-cells = <2>;
>> +		ranges;
>> +
>> +		reg = <0 0 0 0 0>;
>> +
>> +		usb@0,0 {
>> +			reg = <0 0 0 0 0>;
>> +			resets = <&reset RASPBERRYPI_FIRMWARE_RESET_ID_USB>;
>> +		};
>> +	};
>> +};
>> +
>> +&pixelvalve0 {
>> +	status = "okay";
>> +};
>> +
>> +&pixelvalve1 {
>> +	status = "okay";
>> +};
>> +
>> +&pixelvalve2 {
>> +	status = "okay";
>> +};
>> +
>> +&pixelvalve4 {
>> +	status = "okay";
>> +};
>> +
>> +&spi {
>> +	status = "okay";
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&spi0_gpio7>;
>> +	cs-gpios = <&gpio 8 1>, <&gpio 7 1>;
>> +	dmas = <&dma 6>, <&dma 7>;
>> +	dma-names = "tx", "rx";
>> +
>> +	mcp251xfd0: mcp251xfd@0 {
>> +		compatible = "microchip,mcp251xfd";
>> +		reg = <0>;
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&mcp251xfd0_pins>;
>> +		spi-max-frequency = <20000000>;
>> +		interrupt-parent = <&gpio>;
>> +		interrupts = <27 IRQ_TYPE_LEVEL_LOW>;
>> +		clocks = <&clk_mcp251xfd_osc>;
>> +	};
>> +
>> +	mcp251xfd1: mcp251xfd@1 {
>> +		compatible = "microchip,mcp251xfd";
>> +		reg = <1>;
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&mcp251xfd1_pins>;
>> +		spi-max-frequency = <20000000>;
>> +		interrupt-parent = <&gpio>;
>> +		interrupts = <22 IRQ_TYPE_LEVEL_LOW>;
>> +		clocks = <&clk_mcp251xfd_osc>;
>> +	};
>> +};
>> +
>> +&gpio {
>> +	mcp251xfd0_pins: mcp251xfd0_pins {
>> +		brcm,pins = <27>;
>> +		brcm,function = <BCM2835_FSEL_GPIO_IN>;
>> +	};
>> +
>> +	mcp251xfd1_pins: mcp251xfd1_pins {
>> +		brcm,pins = <22>;
>> +		brcm,function = <BCM2835_FSEL_GPIO_IN>;
>> +	};
>> +};
>> +
>> +&vc4 {
>> +	status = "okay";
>> +};
>> +
>> +&vec {
>> +	status = "disabled";
>> +};
>> diff --git a/arch/arm64/boot/dts/broadcom/Makefile b/arch/arm64/boot/dts/broadcom/Makefile
>> index e8584d3b698f..7cd88b8c0345 100644
>> --- a/arch/arm64/boot/dts/broadcom/Makefile
>> +++ b/arch/arm64/boot/dts/broadcom/Makefile
>> @@ -1,6 +1,7 @@
>>   # SPDX-License-Identifier: GPL-2.0
>>   dtb-$(CONFIG_ARCH_BCM2835) += bcm2711-rpi-400.dtb \
>>   			      bcm2711-rpi-4-b.dtb \
>> +			      bcm2711-rpi-cm4-canopi.dtb \
>>   			      bcm2711-rpi-cm4-io.dtb \
>>   			      bcm2837-rpi-3-a-plus.dtb \
>>   			      bcm2837-rpi-3-b.dtb \
>> diff --git a/arch/arm64/boot/dts/broadcom/bcm2711-rpi-cm4-canopi.dts b/arch/arm64/boot/dts/broadcom/bcm2711-rpi-cm4-canopi.dts
>> new file mode 100644
>> index 000000000000..e9369aa0eb39
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/broadcom/bcm2711-rpi-cm4-canopi.dts
>> @@ -0,0 +1,2 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +#include "arm/bcm2711-rpi-cm4-canopi.dts"
>> -- 
>> 2.37.2
>>

Re: [PATCH] ARM: dts: Add Raspberry Pi Compute Module 4 CANOPi Board

[Alexander Dahl]

Hello Stefan,

Am Mon, Sep 19, 2022 at 01:18:21PM +0200 schrieb Stefan Wahren:
> Hi Alexander,
> 
> [fix address of Krzysztof]
> 
> Am 19.09.22 um 09:47 schrieb Alexander Dahl:
> > Hei hei,
> > 
> > Am Fri, Sep 16, 2022 at 12:31:56PM -0300 schrieb Ariel D'Alessandro:
> > > The Eclipse KUKSA CANOPi [0] is a baseboard for the Raspberry Compute
> > > Module 4 (CM4). It contains a VIA VL805 4 Port USB controller and two
> > > MCP251xFD based CAN-FD interfaces.
> > > 
> > > [0] https://github.com/boschresearch/kuksa.hardware
> > > 
> > > Signed-off-by: Ariel D'Alessandro <ariel.dalessandro@collabora.com>
> > > ---
> > >   arch/arm/boot/dts/Makefile                    |   1 +
> > >   arch/arm/boot/dts/bcm2711-rpi-cm4-canopi.dts  | 139 ++++++++++++++++++
> > >   arch/arm64/boot/dts/broadcom/Makefile         |   1 +
> > >   .../dts/broadcom/bcm2711-rpi-cm4-canopi.dts   |   2 +
> > >   4 files changed, 143 insertions(+)
> > >   create mode 100644 arch/arm/boot/dts/bcm2711-rpi-cm4-canopi.dts
> > >   create mode 100644 arch/arm64/boot/dts/broadcom/bcm2711-rpi-cm4-canopi.dts
> > > 
> > > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> > > index 05d8aef6e5d2..8930ab2c132c 100644
> > > --- a/arch/arm/boot/dts/Makefile
> > > +++ b/arch/arm/boot/dts/Makefile
> > > @@ -98,6 +98,7 @@ dtb-$(CONFIG_ARCH_BCM2835) += \
> > >   	bcm2837-rpi-zero-2-w.dtb \
> > >   	bcm2711-rpi-400.dtb \
> > >   	bcm2711-rpi-4-b.dtb \
> > > +	bcm2711-rpi-cm4-canopi.dtb \
> > >   	bcm2711-rpi-cm4-io.dtb \
> > >   	bcm2835-rpi-zero.dtb \
> > >   	bcm2835-rpi-zero-w.dtb
> > > diff --git a/arch/arm/boot/dts/bcm2711-rpi-cm4-canopi.dts b/arch/arm/boot/dts/bcm2711-rpi-cm4-canopi.dts
> > > new file mode 100644
> > > index 000000000000..52ec5908883c
> > > --- /dev/null
> > > +++ b/arch/arm/boot/dts/bcm2711-rpi-cm4-canopi.dts
> > > @@ -0,0 +1,139 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/dts-v1/;
> > > +#include "bcm2711-rpi-cm4.dtsi"
> > > +
> > > +/ {
> > > +	model = "Raspberry Pi Compute Module 4 CANOPi Board";
> > > +
> > > +	clocks {
> > > +		clk_mcp251xfd_osc: mcp251xfd-osc {
> > > +			#clock-cells = <0>;
> > > +			compatible = "fixed-clock";
> > > +			clock-frequency = <20000000>;
> > > +		};
> > > +	};
> > > +
> > > +	leds {
> > > +		led-act {
> > > +			gpios = <&gpio 42 GPIO_ACTIVE_HIGH>;
> > > +		};
> > > +
> > > +		led-pwr {
> > > +			label = "PWR";
> > > +			gpios = <&expgpio 2 GPIO_ACTIVE_LOW>;
> > > +			default-state = "keep";
> > > +			linux,default-trigger = "default-on";
> > > +		};
> > > +	};
> > This looks like using the node name and the deprecated "label"
> > property for LED naming.  Please see
> > Documentation/devicetree/bindings/leds/common.yaml and use the
> > properties "function" and "color" instead.  Also check the node names
> > itself, see the example in that binding or the leds-gpio binding for
> > reference.
> 
> Oops, i didn't noticed this.
> 
> Unfortunately the ACT-LED is already a little bit opaque defined in
> bcm2835-rpi.dtsi:
> 
> leds {
>         compatible = "gpio-leds";
> 
>         led-act {
>             label = "ACT";
>             default-state = "keep";
>             linux,default-trigger = "heartbeat";
>         };
> };
> 
> So a reference (currently missing) would have make it clear that the ACT-LED
> is common for all Raspberry Pi boards.

Yes, a reference would probably good, would make it easier to spot
this is already defined in the dtsi.

> So you wish that this is fixed for the CANOPi board or all Raspberry Pi
> boards?
> 
> I'm asking because switching to function would change the sysfs path and
> breaking userspace ABI.

You're right, and the effective label should stay as is for existing
boards to not break userspace.  

Not sure what the policy is for baseboards with compute modules.  Are
those LEDs on the compute module?  Or does the CM just expose those
GPIOs?  Is there some policy all baseboards must use them for LEDs?
An what about additional LEDs on the baseboard?  Is this allowed?  
(I don't think there a generic rules for that, but maybe some best
practices for certain SoMs like the RPi CM?)

IMHO for new independent boards though, new LEDs should not be
introduced the old way. I thought this is the case here, but it seems
I was wrong due to that baseboard vs. SoM thing.

Greets
Alex

> 
> > 
> > Greets
> > Alex
> > 
> > > +};
> > > +
> > > +&ddc0 {
> > > +	status = "okay";
> > > +};
> > > +
> > > +&ddc1 {
> > > +	status = "okay";
> > > +};
> > > +
> > > +&hdmi0 {
> > > +	status = "okay";
> > > +};
> > > +
> > > +&hdmi1 {
> > > +	status = "okay";
> > > +};
> > > +
> > > +&i2c0 {
> > > +	pinctrl-names = "default";
> > > +	pinctrl-0 = <&i2c0_gpio44>;
> > > +	status = "okay";
> > > +	clock-frequency = <100000>;
> > > +
> > > +	pcf85063a@51 {
> > > +		compatible = "nxp,pcf85063a";
> > > +		reg = <0x51>;
> > > +	};
> > > +};
> > > +
> > > +&pcie0 {
> > > +	pci@0,0 {
> > > +		device_type = "pci";
> > > +		#address-cells = <3>;
> > > +		#size-cells = <2>;
> > > +		ranges;
> > > +
> > > +		reg = <0 0 0 0 0>;
> > > +
> > > +		usb@0,0 {
> > > +			reg = <0 0 0 0 0>;
> > > +			resets = <&reset RASPBERRYPI_FIRMWARE_RESET_ID_USB>;
> > > +		};
> > > +	};
> > > +};
> > > +
> > > +&pixelvalve0 {
> > > +	status = "okay";
> > > +};
> > > +
> > > +&pixelvalve1 {
> > > +	status = "okay";
> > > +};
> > > +
> > > +&pixelvalve2 {
> > > +	status = "okay";
> > > +};
> > > +
> > > +&pixelvalve4 {
> > > +	status = "okay";
> > > +};
> > > +
> > > +&spi {
> > > +	status = "okay";
> > > +	pinctrl-names = "default";
> > > +	pinctrl-0 = <&spi0_gpio7>;
> > > +	cs-gpios = <&gpio 8 1>, <&gpio 7 1>;
> > > +	dmas = <&dma 6>, <&dma 7>;
> > > +	dma-names = "tx", "rx";
> > > +
> > > +	mcp251xfd0: mcp251xfd@0 {
> > > +		compatible = "microchip,mcp251xfd";
> > > +		reg = <0>;
> > > +		pinctrl-names = "default";
> > > +		pinctrl-0 = <&mcp251xfd0_pins>;
> > > +		spi-max-frequency = <20000000>;
> > > +		interrupt-parent = <&gpio>;
> > > +		interrupts = <27 IRQ_TYPE_LEVEL_LOW>;
> > > +		clocks = <&clk_mcp251xfd_osc>;
> > > +	};
> > > +
> > > +	mcp251xfd1: mcp251xfd@1 {
> > > +		compatible = "microchip,mcp251xfd";
> > > +		reg = <1>;
> > > +		pinctrl-names = "default";
> > > +		pinctrl-0 = <&mcp251xfd1_pins>;
> > > +		spi-max-frequency = <20000000>;
> > > +		interrupt-parent = <&gpio>;
> > > +		interrupts = <22 IRQ_TYPE_LEVEL_LOW>;
> > > +		clocks = <&clk_mcp251xfd_osc>;
> > > +	};
> > > +};
> > > +
> > > +&gpio {
> > > +	mcp251xfd0_pins: mcp251xfd0_pins {
> > > +		brcm,pins = <27>;
> > > +		brcm,function = <BCM2835_FSEL_GPIO_IN>;
> > > +	};
> > > +
> > > +	mcp251xfd1_pins: mcp251xfd1_pins {
> > > +		brcm,pins = <22>;
> > > +		brcm,function = <BCM2835_FSEL_GPIO_IN>;
> > > +	};
> > > +};
> > > +
> > > +&vc4 {
> > > +	status = "okay";
> > > +};
> > > +
> > > +&vec {
> > > +	status = "disabled";
> > > +};
> > > diff --git a/arch/arm64/boot/dts/broadcom/Makefile b/arch/arm64/boot/dts/broadcom/Makefile
> > > index e8584d3b698f..7cd88b8c0345 100644
> > > --- a/arch/arm64/boot/dts/broadcom/Makefile
> > > +++ b/arch/arm64/boot/dts/broadcom/Makefile
> > > @@ -1,6 +1,7 @@
> > >   # SPDX-License-Identifier: GPL-2.0
> > >   dtb-$(CONFIG_ARCH_BCM2835) += bcm2711-rpi-400.dtb \
> > >   			      bcm2711-rpi-4-b.dtb \
> > > +			      bcm2711-rpi-cm4-canopi.dtb \
> > >   			      bcm2711-rpi-cm4-io.dtb \
> > >   			      bcm2837-rpi-3-a-plus.dtb \
> > >   			      bcm2837-rpi-3-b.dtb \
> > > diff --git a/arch/arm64/boot/dts/broadcom/bcm2711-rpi-cm4-canopi.dts b/arch/arm64/boot/dts/broadcom/bcm2711-rpi-cm4-canopi.dts
> > > new file mode 100644
> > > index 000000000000..e9369aa0eb39
> > > --- /dev/null
> > > +++ b/arch/arm64/boot/dts/broadcom/bcm2711-rpi-cm4-canopi.dts
> > > @@ -0,0 +1,2 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +#include "arm/bcm2711-rpi-cm4-canopi.dts"
> > > -- 
> > > 2.37.2
> > > 

Re: [PATCH] ARM: dts: Add Raspberry Pi Compute Module 4 CANOPi Board

[Krzysztof Kozlowski]

On 16/09/2022 17:31, Ariel D'Alessandro wrote:
> The Eclipse KUKSA CANOPi [0] is a baseboard for the Raspberry Compute
> Module 4 (CM4). It contains a VIA VL805 4 Port USB controller and two
> MCP251xFD based CAN-FD interfaces.
> 
> [0] https://github.com/boschresearch/kuksa.hardware
> 
> Signed-off-by: Ariel D'Alessandro <ariel.dalessandro@collabora.com>
> ---
>  arch/arm/boot/dts/Makefile                    |   1 +
>  arch/arm/boot/dts/bcm2711-rpi-cm4-canopi.dts  | 139 ++++++++++++++++++
>  arch/arm64/boot/dts/broadcom/Makefile         |   1 +
>  .../dts/broadcom/bcm2711-rpi-cm4-canopi.dts   |   2 +
>  4 files changed, 143 insertions(+)
>  create mode 100644 arch/arm/boot/dts/bcm2711-rpi-cm4-canopi.dts
>  create mode 100644 arch/arm64/boot/dts/broadcom/bcm2711-rpi-cm4-canopi.dts
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 05d8aef6e5d2..8930ab2c132c 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -98,6 +98,7 @@ dtb-$(CONFIG_ARCH_BCM2835) += \
>  	bcm2837-rpi-zero-2-w.dtb \
>  	bcm2711-rpi-400.dtb \
>  	bcm2711-rpi-4-b.dtb \
> +	bcm2711-rpi-cm4-canopi.dtb \
>  	bcm2711-rpi-cm4-io.dtb \
>  	bcm2835-rpi-zero.dtb \
>  	bcm2835-rpi-zero-w.dtb
> diff --git a/arch/arm/boot/dts/bcm2711-rpi-cm4-canopi.dts b/arch/arm/boot/dts/bcm2711-rpi-cm4-canopi.dts
> new file mode 100644
> index 000000000000..52ec5908883c
> --- /dev/null
> +++ b/arch/arm/boot/dts/bcm2711-rpi-cm4-canopi.dts
> @@ -0,0 +1,139 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/dts-v1/;
> +#include "bcm2711-rpi-cm4.dtsi"
> +
> +/ {
> +	model = "Raspberry Pi Compute Module 4 CANOPi Board";

Where is the compatible?

> +
> +	clocks {
> +		clk_mcp251xfd_osc: mcp251xfd-osc {

Node names should be generic.
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

> +			#clock-cells = <0>;
> +			compatible = "fixed-clock";
> +			clock-frequency = <20000000>;
> +		};
> +	};
> +
> +	leds {

This does not look valid.

Does not look like you tested the DTS against bindings. Please run `make
dtbs_check` (see Documentation/devicetree/bindings/writing-schema.rst
for instructions).


> +		led-act {
> +			gpios = <&gpio 42 GPIO_ACTIVE_HIGH>;

What about the rest? Why only gpios? Does it pass dtbs_check?

> +		};
> +
> +		led-pwr {
> +			label = "PWR";
> +			gpios = <&expgpio 2 GPIO_ACTIVE_LOW>;
> +			default-state = "keep";
> +			linux,default-trigger = "default-on";
> +		};
> +	};
> +};
> +
> +&ddc0 {
> +	status = "okay";
> +};
> +
> +&ddc1 {
> +	status = "okay";
> +};
> +
> +&hdmi0 {
> +	status = "okay";
> +};
> +
> +&hdmi1 {
> +	status = "okay";
> +};
> +
> +&i2c0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&i2c0_gpio44>;
> +	status = "okay";
> +	clock-frequency = <100000>;
> +
> +	pcf85063a@51 {

Node names should be generic.
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

> +		compatible = "nxp,pcf85063a";
> +		reg = <0x51>;
> +	};
> +};
> +
> +&pcie0 {
> +	pci@0,0 {
> +		device_type = "pci";
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		reg = <0 0 0 0 0>;
> +
> +		usb@0,0 {
> +			reg = <0 0 0 0 0>;
> +			resets = <&reset RASPBERRYPI_FIRMWARE_RESET_ID_USB>;
> +		};
> +	};
> +};
> +
> +&pixelvalve0 {
> +	status = "okay";
> +};
> +
> +&pixelvalve1 {
> +	status = "okay";
> +};
> +
> +&pixelvalve2 {
> +	status = "okay";
> +};
> +
> +&pixelvalve4 {
> +	status = "okay";
> +};
> +
> +&spi {
> +	status = "okay";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&spi0_gpio7>;
> +	cs-gpios = <&gpio 8 1>, <&gpio 7 1>;

Use GPIO flags/defines. This applies everywhere.


> +	dmas = <&dma 6>, <&dma 7>;
> +	dma-names = "tx", "rx";
> +
> +	mcp251xfd0: mcp251xfd@0 {

Node names should be generic.
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

> +		compatible = "microchip,mcp251xfd";
> +		reg = <0>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&mcp251xfd0_pins>;
> +		spi-max-frequency = <20000000>;
> +		interrupt-parent = <&gpio>;
> +		interrupts = <27 IRQ_TYPE_LEVEL_LOW>;
> +		clocks = <&clk_mcp251xfd_osc>;
> +	};
> +
> +	mcp251xfd1: mcp251xfd@1 {

Node names should be generic.
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

> +		compatible = "microchip,mcp251xfd";
> +		reg = <1>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&mcp251xfd1_pins>;
> +		spi-max-frequency = <20000000>;
> +		interrupt-parent = <&gpio>;
> +		interrupts = <22 IRQ_TYPE_LEVEL_LOW>;
> +		clocks = <&clk_mcp251xfd_osc>;
> +	};
> +};
> +
> +&gpio {
> +	mcp251xfd0_pins: mcp251xfd0_pins {

No underscores in node names.

> +		brcm,pins = <27>;
> +		brcm,function = <BCM2835_FSEL_GPIO_IN>;
> +	};
> +
> +	mcp251xfd1_pins: mcp251xfd1_pins {

Ditto

> +		brcm,pins = <22>;
> +		brcm,function = <BCM2835_FSEL_GPIO_IN>;
> +	};
> +};
> +
> +&vc4 {
> +	status = "okay";
> +};
> +
> +&vec {
> +	status = "disabled";
> +};


Best regards,
Krzysztof

Re: [PATCH] ARM: dts: Add Raspberry Pi Compute Module 4 CANOPi Board

[Stefan Wahren]

Hi Alexander,

Am 20.09.22 um 10:31 schrieb Alexander Dahl:
> Hello Stefan,
>
> Am Mon, Sep 19, 2022 at 01:18:21PM +0200 schrieb Stefan Wahren:
>> Hi Alexander,
>>
>> [fix address of Krzysztof]
>>
>> Am 19.09.22 um 09:47 schrieb Alexander Dahl:
>>> Hei hei,
>>>
>>> Am Fri, Sep 16, 2022 at 12:31:56PM -0300 schrieb Ariel D'Alessandro:
>>>> The Eclipse KUKSA CANOPi [0] is a baseboard for the Raspberry Compute
>>>> Module 4 (CM4). It contains a VIA VL805 4 Port USB controller and two
>>>> MCP251xFD based CAN-FD interfaces.
>>>>
>>>> [0] https://github.com/boschresearch/kuksa.hardware
>>>>
>>>> Signed-off-by: Ariel D'Alessandro <ariel.dalessandro@collabora.com>
>>>> ---
>>>>    arch/arm/boot/dts/Makefile                    |   1 +
>>>>    arch/arm/boot/dts/bcm2711-rpi-cm4-canopi.dts  | 139 ++++++++++++++++++
>>>>    arch/arm64/boot/dts/broadcom/Makefile         |   1 +
>>>>    .../dts/broadcom/bcm2711-rpi-cm4-canopi.dts   |   2 +
>>>>    4 files changed, 143 insertions(+)
>>>>    create mode 100644 arch/arm/boot/dts/bcm2711-rpi-cm4-canopi.dts
>>>>    create mode 100644 arch/arm64/boot/dts/broadcom/bcm2711-rpi-cm4-canopi.dts
>>>>
>>>> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
>>>> index 05d8aef6e5d2..8930ab2c132c 100644
>>>> --- a/arch/arm/boot/dts/Makefile
>>>> +++ b/arch/arm/boot/dts/Makefile
>>>> @@ -98,6 +98,7 @@ dtb-$(CONFIG_ARCH_BCM2835) += \
>>>>    	bcm2837-rpi-zero-2-w.dtb \
>>>>    	bcm2711-rpi-400.dtb \
>>>>    	bcm2711-rpi-4-b.dtb \
>>>> +	bcm2711-rpi-cm4-canopi.dtb \
>>>>    	bcm2711-rpi-cm4-io.dtb \
>>>>    	bcm2835-rpi-zero.dtb \
>>>>    	bcm2835-rpi-zero-w.dtb
>>>> diff --git a/arch/arm/boot/dts/bcm2711-rpi-cm4-canopi.dts b/arch/arm/boot/dts/bcm2711-rpi-cm4-canopi.dts
>>>> new file mode 100644
>>>> index 000000000000..52ec5908883c
>>>> --- /dev/null
>>>> +++ b/arch/arm/boot/dts/bcm2711-rpi-cm4-canopi.dts
>>>> @@ -0,0 +1,139 @@
>>>> +// SPDX-License-Identifier: GPL-2.0
>>>> +/dts-v1/;
>>>> +#include "bcm2711-rpi-cm4.dtsi"
>>>> +
>>>> +/ {
>>>> +	model = "Raspberry Pi Compute Module 4 CANOPi Board";
>>>> +
>>>> +	clocks {
>>>> +		clk_mcp251xfd_osc: mcp251xfd-osc {
>>>> +			#clock-cells = <0>;
>>>> +			compatible = "fixed-clock";
>>>> +			clock-frequency = <20000000>;
>>>> +		};
>>>> +	};
>>>> +
>>>> +	leds {
>>>> +		led-act {
>>>> +			gpios = <&gpio 42 GPIO_ACTIVE_HIGH>;
>>>> +		};
>>>> +
>>>> +		led-pwr {
>>>> +			label = "PWR";
>>>> +			gpios = <&expgpio 2 GPIO_ACTIVE_LOW>;
>>>> +			default-state = "keep";
>>>> +			linux,default-trigger = "default-on";
>>>> +		};
>>>> +	};
>>> This looks like using the node name and the deprecated "label"
>>> property for LED naming.  Please see
>>> Documentation/devicetree/bindings/leds/common.yaml and use the
>>> properties "function" and "color" instead.  Also check the node names
>>> itself, see the example in that binding or the leds-gpio binding for
>>> reference.
>> Oops, i didn't noticed this.
>>
>> Unfortunately the ACT-LED is already a little bit opaque defined in
>> bcm2835-rpi.dtsi:
>>
>> leds {
>>          compatible = "gpio-leds";
>>
>>          led-act {
>>              label = "ACT";
>>              default-state = "keep";
>>              linux,default-trigger = "heartbeat";
>>          };
>> };
>>
>> So a reference (currently missing) would have make it clear that the ACT-LED
>> is common for all Raspberry Pi boards.
> Yes, a reference would probably good, would make it easier to spot
> this is already defined in the dtsi.
I will take care of this.
>
>> So you wish that this is fixed for the CANOPi board or all Raspberry Pi
>> boards?
>>
>> I'm asking because switching to function would change the sysfs path and
>> breaking userspace ABI.
> You're right, and the effective label should stay as is for existing
> boards to not break userspace.
>
> Not sure what the policy is for baseboards with compute modules.  Are
> those LEDs on the compute module?  Or does the CM just expose those
> GPIOs?
These are GPIOs expose by the Compute Module. Since these are 
initialized by the VC4 firmware, it's not the best idea to use them for 
other functions.
>    Is there some policy all baseboards must use them for LEDs?
> An what about additional LEDs on the baseboard?  Is this allowed?
Definitely
> (I don't think there a generic rules for that, but maybe some best
> practices for certain SoMs like the RPi CM?)
I think we should for Ariel's reponse.
> IMHO for new independent boards though, new LEDs should not be
> introduced the old way. I thought this is the case here, but it seems
> I was wrong due to that baseboard vs. SoM thing.

Without your comment i hadn't noticed this :-)

I'm thinking of a dtsi file in order to encapsulate the deprecated LED 
stuff, remove the global ACT-LED from bcm2835-rpi.dtsi and include the 
dtsi from all board files.

Best regards

Re: [PATCH] ARM: dts: Add Raspberry Pi Compute Module 4 CANOPi Board

[Pavel Machek]

Hi!

> > +		led-pwr {
> > +			label = "PWR";
> > +			gpios = <&expgpio 2 GPIO_ACTIVE_LOW>;
> > +			default-state = "keep";
> > +			linux,default-trigger = "default-on";
> > +		};

NAK on the label, see/modify well-known-leds.txt.