2022-03-12 01:50:32

by Rabara, Niravkumar L

[permalink] [raw]
Subject: [PATCH] arm64: dts: intel: add device tree for n6010

From: Niravkumar L Rabara <[email protected]>

Add a dts file for the Silicom FPGA SmartNIC N6010/N6011,
which is based on the Intel Agliex platform.

Acked-by: Dinh Nguyen <[email protected]>
Signed-off-by: Niravkumar L Rabara <[email protected]>
---
arch/arm64/boot/dts/intel/Makefile | 1 +
.../boot/dts/intel/socfpga_agilex_n6010.dts | 83 +++++++++++++++++++
2 files changed, 84 insertions(+)
create mode 100644 arch/arm64/boot/dts/intel/socfpga_agilex_n6010.dts

diff --git a/arch/arm64/boot/dts/intel/Makefile b/arch/arm64/boot/dts/intel/Makefile
index 0b5477442263..a05a610a4006 100644
--- a/arch/arm64/boot/dts/intel/Makefile
+++ b/arch/arm64/boot/dts/intel/Makefile
@@ -1,5 +1,6 @@
# SPDX-License-Identifier: GPL-2.0-only
dtb-$(CONFIG_ARCH_INTEL_SOCFPGA) += socfpga_agilex_socdk.dtb \
socfpga_agilex_socdk_nand.dtb \
+ socfpga_agilex_n6010.dtb \
socfpga_n5x_socdk.dtb
dtb-$(CONFIG_ARCH_KEEMBAY) += keembay-evm.dtb
diff --git a/arch/arm64/boot/dts/intel/socfpga_agilex_n6010.dts b/arch/arm64/boot/dts/intel/socfpga_agilex_n6010.dts
new file mode 100644
index 000000000000..c105f0621eb4
--- /dev/null
+++ b/arch/arm64/boot/dts/intel/socfpga_agilex_n6010.dts
@@ -0,0 +1,83 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2022, Intel Corporation
+ */
+#include "socfpga_agilex.dtsi"
+
+/ {
+ model = "SoCFPGA Agilex n6010";
+
+ aliases {
+ serial0 = &uart0;
+ ethernet0 = &gmac0;
+ ethernet1 = &gmac1;
+ ethernet2 = &gmac2;
+ };
+
+ chosen {
+ stdout-path = "serial0:115200n8";
+ };
+
+ memory {
+ device_type = "memory";
+ /* We expect the bootloader to fill in the reg */
+ reg = <0 0 0 0>;
+ };
+
+ soc {
+ clocks {
+ osc1 {
+ clock-frequency = <25000000>;
+ };
+ };
+ agilex_hps_bridges: bridge@80000000 {
+ compatible = "simple-bus";
+ reg = <0x80000000 0x60000000>,
+ <0xf9000000 0x00100000>;
+ reg-names = "axi_h2f", "axi_h2f_lw";
+ #address-cells = <0x2>;
+ #size-cells = <0x1>;
+ ranges = <0x00000000 0x00000000 0xf9000000 0x00001000>,
+ <0x00000001 0x02001000 0x82001000 0x00000800>,
+ <0x00000001 0x02080000 0x82080000 0x00004000>,
+ <0x00000001 0x02100000 0x82100000 0x00080000>,
+ <0x00000001 0x02000040 0x82000040 0x00000020>,
+ <0x00000001 0x02000800 0x82000800 0x00000020>,
+ <0x00000001 0x02000820 0x82000820 0x00000020>,
+ <0x00000001 0x02000900 0x82000900 0x00000020>,
+ <0x00000001 0x02000920 0x82000920 0x00000020>,
+ <0x00000001 0x02000940 0x82000940 0x00000020>,
+ <0x00000001 0x00000300 0xf9000300 0x00000010>,
+ <0x00000001 0x02000000 0x82000000 0x00000010>;
+
+ uio_cp_eng@0xf9000000 {
+ compatible = "generic-uio";
+ reg = <0x00000000 0x00000000 0x00001000>;
+ status = "okay";
+ };
+ };
+ };
+};
+
+&uart0 {
+ status = "okay";
+};
+
+&spi0 {
+ status = "okay";
+
+ spidev: spidev@0 {
+ status = "okay";
+ compatible = "linux,spidev";
+ spi-max-frequency = <25000000>;
+ reg = <0>;
+ };
+};
+
+&watchdog0 {
+ status = "okay";
+};
+
+&fpga_mgr {
+ status = "disabled";
+};
--
2.25.1


2022-03-12 23:17:56

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: intel: add device tree for n6010

On 12/03/2022 09:07, [email protected] wrote:
> From: Niravkumar L Rabara <[email protected]>
>
> Add a dts file for the Silicom FPGA SmartNIC N6010/N6011,
> which is based on the Intel Agliex platform.
>
> Acked-by: Dinh Nguyen <[email protected]>

It's a v1, so did Dinh ack it off-line?

> Signed-off-by: Niravkumar L Rabara <[email protected]>
> ---
> arch/arm64/boot/dts/intel/Makefile | 1 +
> .../boot/dts/intel/socfpga_agilex_n6010.dts | 83 +++++++++++++++++++
> 2 files changed, 84 insertions(+)
> create mode 100644 arch/arm64/boot/dts/intel/socfpga_agilex_n6010.dts
>
> diff --git a/arch/arm64/boot/dts/intel/Makefile b/arch/arm64/boot/dts/intel/Makefile
> index 0b5477442263..a05a610a4006 100644
> --- a/arch/arm64/boot/dts/intel/Makefile
> +++ b/arch/arm64/boot/dts/intel/Makefile
> @@ -1,5 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0-only
> dtb-$(CONFIG_ARCH_INTEL_SOCFPGA) += socfpga_agilex_socdk.dtb \
> socfpga_agilex_socdk_nand.dtb \
> + socfpga_agilex_n6010.dtb \

Put the names in alphabetical order.

> socfpga_n5x_socdk.dtb
> dtb-$(CONFIG_ARCH_KEEMBAY) += keembay-evm.dtb
> diff --git a/arch/arm64/boot/dts/intel/socfpga_agilex_n6010.dts b/arch/arm64/boot/dts/intel/socfpga_agilex_n6010.dts
> new file mode 100644
> index 000000000000..c105f0621eb4
> --- /dev/null
> +++ b/arch/arm64/boot/dts/intel/socfpga_agilex_n6010.dts
> @@ -0,0 +1,83 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2022, Intel Corporation
> + */
> +#include "socfpga_agilex.dtsi"
> +
> +/ {
> + model = "SoCFPGA Agilex n6010";

You missed here compatible and respective bindings change.

> +
> + aliases {
> + serial0 = &uart0;
> + ethernet0 = &gmac0;
> + ethernet1 = &gmac1;
> + ethernet2 = &gmac2;
> + };
> +
> + chosen {
> + stdout-path = "serial0:115200n8";
> + };
> +
> + memory {
> + device_type = "memory";
> + /* We expect the bootloader to fill in the reg */
> + reg = <0 0 0 0>;
> + };
> +
> + soc {
> + clocks {
> + osc1 {

Override with labels, not by full path. See other code as examples. Try
coding in similar way how it is in Linux kernel - it is a great way to
learn.

> + clock-frequency = <25000000>;


> + };
> + };
> + agilex_hps_bridges: bridge@80000000 {

Weird indentation.

> + compatible = "simple-bus";
> + reg = <0x80000000 0x60000000>,
> + <0xf9000000 0x00100000>;
> + reg-names = "axi_h2f", "axi_h2f_lw";
> + #address-cells = <0x2>;
> + #size-cells = <0x1>;

No need for hex numbers

> + ranges = <0x00000000 0x00000000 0xf9000000 0x00001000>,
> + <0x00000001 0x02001000 0x82001000 0x00000800>,
> + <0x00000001 0x02080000 0x82080000 0x00004000>,
> + <0x00000001 0x02100000 0x82100000 0x00080000>,
> + <0x00000001 0x02000040 0x82000040 0x00000020>,
> + <0x00000001 0x02000800 0x82000800 0x00000020>,
> + <0x00000001 0x02000820 0x82000820 0x00000020>,
> + <0x00000001 0x02000900 0x82000900 0x00000020>,
> + <0x00000001 0x02000920 0x82000920 0x00000020>,
> + <0x00000001 0x02000940 0x82000940 0x00000020>,
> + <0x00000001 0x00000300 0xf9000300 0x00000010>,
> + <0x00000001 0x02000000 0x82000000 0x00000010>;
> +
> + uio_cp_eng@0xf9000000 {
> + compatible = "generic-uio";
> + reg = <0x00000000 0x00000000 0x00001000>;
> + status = "okay";
> + };
> + };
> + };
> +};
> +
> +&uart0 {
> + status = "okay";
> +};
> +
> +&spi0 {
> + status = "okay";
> +
> + spidev: spidev@0 {
> + status = "okay";
> + compatible = "linux,spidev";

No, for multiple reasons:
1. Did you see anywhere such usage?
2. Did you see compatible documented?
3. Did you see the warning when booting the kernel?

> + spi-max-frequency = <25000000>;
> + reg = <0>;
> + };
> +};
> +
> +&watchdog0 {
> + status = "okay";
> +};
> +
> +&fpga_mgr {
> + status = "disabled";
> +};


Best regards,
Krzysztof