On 12/03/2022 09:07, niravkumar.l.rabara@xxxxxxxxx wrote:
> From: Niravkumar L Rabara <niravkumar.l.rabara@xxxxxxxxx>
>
> Add a dts file for the Silicom FPGA SmartNIC N6010/N6011,
> which is based on the Intel Agliex platform.
>
> Acked-by: Dinh Nguyen <dinguyen@xxxxxxxxxx>
It's a v1, so did Dinh ack it off-line?
> Signed-off-by: Niravkumar L Rabara <niravkumar.l.rabara@xxxxxxxxx>
> ---
> 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
