* Re: [PATCH] ARM: dts: Add Raspberry Pi Compute Module 4 CANOPi Board
2022-09-16 15:31 [PATCH] ARM: dts: Add Raspberry Pi Compute Module 4 CANOPi Board Ariel D'Alessandro
@ 2022-09-17 10:18 ` Stefan Wahren
2022-09-19 7:47 ` Alexander Dahl
2022-09-20 9:51 ` Krzysztof Kozlowski
2 siblings, 0 replies; 8+ messages in thread
From: Stefan Wahren @ 2022-09-17 10:18 UTC (permalink / raw)
To: Ariel D'Alessandro
Cc: arnd, f.fainelli, krzyszccf.kozlowski+dt, devicetree, olof,
robh+dt, soc, linux-arm-kernel, william.zhang,
bcm-kernel-feedback-list, linux-rpi-kernel, linux-kernel
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"
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] ARM: dts: Add Raspberry Pi Compute Module 4 CANOPi Board
2022-09-16 15:31 [PATCH] ARM: dts: Add Raspberry Pi Compute Module 4 CANOPi Board Ariel D'Alessandro
2022-09-17 10:18 ` Stefan Wahren
@ 2022-09-19 7:47 ` Alexander Dahl
2022-09-19 11:18 ` Stefan Wahren
2022-09-20 9:51 ` Krzysztof Kozlowski
2 siblings, 1 reply; 8+ messages in thread
From: Alexander Dahl @ 2022-09-19 7:47 UTC (permalink / raw)
To: Ariel D'Alessandro
Cc: bcm-kernel-feedback-list, devicetree, linux-arm-kernel,
linux-kernel, linux-rpi-kernel, arnd, f.fainelli,
krzyszccf.kozlowski+dt, nsaenz, olof, robh+dt, soc,
stefan.wahren, william.zhang
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
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] ARM: dts: Add Raspberry Pi Compute Module 4 CANOPi Board
2022-09-19 7:47 ` Alexander Dahl
@ 2022-09-19 11:18 ` Stefan Wahren
2022-09-20 8:31 ` Alexander Dahl
0 siblings, 1 reply; 8+ messages in thread
From: Stefan Wahren @ 2022-09-19 11:18 UTC (permalink / raw)
To: Ariel D'Alessandro, bcm-kernel-feedback-list, devicetree,
linux-arm-kernel, linux-kernel, linux-rpi-kernel, arnd,
f.fainelli, nsaenz, olof, robh+dt, soc, william.zhang,
Krzysztof Kozlowski
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
>>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] ARM: dts: Add Raspberry Pi Compute Module 4 CANOPi Board
2022-09-19 11:18 ` Stefan Wahren
@ 2022-09-20 8:31 ` Alexander Dahl
2022-09-20 15:41 ` Stefan Wahren
0 siblings, 1 reply; 8+ messages in thread
From: Alexander Dahl @ 2022-09-20 8:31 UTC (permalink / raw)
To: Stefan Wahren
Cc: Ariel D'Alessandro, bcm-kernel-feedback-list, devicetree,
linux-arm-kernel, linux-kernel, linux-rpi-kernel, arnd,
f.fainelli, nsaenz, olof, robh+dt, soc, william.zhang,
Krzysztof Kozlowski
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
> > >
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] ARM: dts: Add Raspberry Pi Compute Module 4 CANOPi Board
2022-09-20 8:31 ` Alexander Dahl
@ 2022-09-20 15:41 ` Stefan Wahren
0 siblings, 0 replies; 8+ messages in thread
From: Stefan Wahren @ 2022-09-20 15:41 UTC (permalink / raw)
To: Ariel D'Alessandro, bcm-kernel-feedback-list, devicetree,
linux-arm-kernel, linux-kernel, linux-rpi-kernel, arnd,
f.fainelli, nsaenz, olof, robh+dt, soc, william.zhang,
Krzysztof Kozlowski
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] ARM: dts: Add Raspberry Pi Compute Module 4 CANOPi Board
2022-09-16 15:31 [PATCH] ARM: dts: Add Raspberry Pi Compute Module 4 CANOPi Board Ariel D'Alessandro
2022-09-17 10:18 ` Stefan Wahren
2022-09-19 7:47 ` Alexander Dahl
@ 2022-09-20 9:51 ` Krzysztof Kozlowski
2022-10-20 12:16 ` Pavel Machek
2 siblings, 1 reply; 8+ messages in thread
From: Krzysztof Kozlowski @ 2022-09-20 9:51 UTC (permalink / raw)
To: Ariel D'Alessandro, bcm-kernel-feedback-list, devicetree,
linux-arm-kernel, linux-kernel, linux-rpi-kernel
Cc: arnd, f.fainelli, krzyszccf.kozlowski+dt, nsaenz, olof, robh+dt,
soc, stefan.wahren, william.zhang
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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] ARM: dts: Add Raspberry Pi Compute Module 4 CANOPi Board
2022-09-20 9:51 ` Krzysztof Kozlowski
@ 2022-10-20 12:16 ` Pavel Machek
0 siblings, 0 replies; 8+ messages in thread
From: Pavel Machek @ 2022-10-20 12:16 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Ariel D'Alessandro, bcm-kernel-feedback-list, devicetree,
linux-arm-kernel, linux-kernel, linux-rpi-kernel, arnd,
f.fainelli, krzyszccf.kozlowski+dt, nsaenz, olof, robh+dt, soc,
stefan.wahren, william.zhang
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.
^ permalink raw reply [flat|nested] 8+ messages in thread