All of lore.kernel.org
 help / color / mirror / Atom feed
From: Krzysztof Kozlowski <krzk@kernel.org>
To: Harshit Shah <hshah@axiado.com>, Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	Bartosz Golaszewski <brgl@bgdev.pl>,
	Arnd Bergmann <arnd@arndb.de>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>
Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, linux-gpio@vger.kernel.org,
	soc@lists.linux.dev
Subject: Re: [PATCH v2 4/6] arm64: dts: axiado: Add initial support for AX3000 SoC and eval board
Date: Mon, 16 Jun 2025 08:09:36 +0200	[thread overview]
Message-ID: <6ef92d1a-39cc-409f-8ebe-d28ad2006988@kernel.org> (raw)
In-Reply-To: <20250615-axiado-ax3000-soc-and-evaluation-board-support-v2-4-341502d38618@axiado.com>

On 16/06/2025 06:31, Harshit Shah wrote:
> Add initial device tree support for the AX3000 SoC and its evaluation
> platform. The AX3000 is a multi-core SoC featuring 4 Cortex-A53 cores,
> Secure Vault, AI Engine and Firewall.
> 
> This commit adds support for Cortex-A53 CPUs, timer, UARTs, and I3C
> controllers on the AX3000 evaluation board.
> 
> Signed-off-by: Harshit Shah <hshah@axiado.com>
> ---
>  arch/arm64/boot/dts/Makefile              |   1 +
>  arch/arm64/boot/dts/axiado/Makefile       |   2 +
>  arch/arm64/boot/dts/axiado/ax3000.dtsi    | 584 ++++++++++++++++++++++++++++++
>  arch/arm64/boot/dts/axiado/ax3000_evk.dts |  72 ++++
>  4 files changed, 659 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/Makefile b/arch/arm64/boot/dts/Makefile
> index 79b73a21ddc22b17308554e502f8207392935b45..47dd8a1a7960d179ee28969a1d6750bfa0d73da1 100644
> --- a/arch/arm64/boot/dts/Makefile
> +++ b/arch/arm64/boot/dts/Makefile
> @@ -9,6 +9,7 @@ subdir-y += amlogic
>  subdir-y += apm
>  subdir-y += apple
>  subdir-y += arm
> +subdir-y += axiado
>  subdir-y += bitmain
>  subdir-y += blaize
>  subdir-y += broadcom
> diff --git a/arch/arm64/boot/dts/axiado/Makefile b/arch/arm64/boot/dts/axiado/Makefile
> new file mode 100644
> index 0000000000000000000000000000000000000000..eb5e08ba0f39c32cdbfd586d982849a80da30160
> --- /dev/null
> +++ b/arch/arm64/boot/dts/axiado/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0
> +dtb-$(CONFIG_ARCH_AXIADO) += ax3000_evk.dtb
> diff --git a/arch/arm64/boot/dts/axiado/ax3000.dtsi b/arch/arm64/boot/dts/axiado/ax3000.dtsi
> new file mode 100644
> index 0000000000000000000000000000000000000000..d5d84986d18efe9dfbb446ceee42fc4e4dbf95d0
> --- /dev/null
> +++ b/arch/arm64/boot/dts/axiado/ax3000.dtsi
> @@ -0,0 +1,584 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2021-25 Axiado Corporation (or its affiliates). All rights reserved.
> + */
> +
> +/dts-v1/;
> +
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +/memreserve/ 0x3c0013a0 0x00000008;	/* cpu-release-addr */
> +/ {
> +	compatible = "axiado,ax3000";
> +	interrupt-parent = <&gic500>;
> +
> +	aliases {
> +		i3c0 = &i3c0;
> +		i3c1 = &i3c1;
> +		i3c2 = &i3c2;
> +		i3c3 = &i3c3;
> +		i3c4 = &i3c4;
> +		i3c5 = &i3c5;
> +		i3c6 = &i3c6;
> +		i3c7 = &i3c7;
> +		i3c8 = &i3c8;
> +		i3c9 = &i3c9;
> +		i3c10 = &i3c10;
> +		i3c11 = &i3c11;
> +		i3c12 = &i3c12;
> +		i3c13 = &i3c13;
> +		i3c14 = &i3c14;
> +		i3c15 = &i3c15;
> +		i3c16 = &i3c16;
> +		serial0 = &uart0;
> +		serial1 = &uart1;
> +		serial2 = &uart2;
> +		serial3 = &uart3;

None of these are properties of SoC, but board. Move respective aliases
to the board files.

> +	};
> +
> +	cpus {
> +		#address-cells = <2>;
> +		#size-cells = <0>;
> +
> +		cpu0: cpu@0 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a53";
> +			reg = <0x0 0x0>;
> +			enable-method = "spin-table";
> +			cpu-release-addr = <0x0 0x3c0013a0>;
> +			d-cache-size = <0x8000>;
> +			d-cache-line-size = <64>;
> +			d-cache-sets = <128>;
> +			i-cache-size = <0x8000>;
> +			i-cache-line-size = <64>;
> +			i-cache-sets = <256>;
> +			next-level-cache = <&l2>;
> +		};
> +
> +		cpu1: cpu@1 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a53";
> +			reg = <0x0 0x1>;
> +			enable-method = "spin-table";
> +			cpu-release-addr = <0x0 0x3c0013a0>;
> +			d-cache-size = <0x8000>;
> +			d-cache-line-size = <64>;
> +			d-cache-sets = <128>;
> +			i-cache-size = <0x8000>;
> +			i-cache-line-size = <64>;
> +			i-cache-sets = <256>;
> +			next-level-cache = <&l2>;
> +		};
> +
> +		cpu2: cpu@2 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a53";
> +			reg = <0x0 0x2>;
> +			enable-method = "spin-table";
> +			cpu-release-addr = <0x0 0x3c0013a0>;
> +			d-cache-size = <0x8000>;
> +			d-cache-line-size = <64>;
> +			d-cache-sets = <128>;
> +			i-cache-size = <0x8000>;
> +			i-cache-line-size = <64>;
> +			i-cache-sets = <256>;
> +			next-level-cache = <&l2>;
> +		};
> +
> +		cpu3: cpu@3 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a53";
> +			reg = <0x0 0x3>;
> +			enable-method = "spin-table";
> +			cpu-release-addr = <0x0 0x3c0013a0>;
> +			d-cache-size = <0x8000>;
> +			d-cache-line-size = <64>;
> +			d-cache-sets = <128>;
> +			i-cache-size = <0x8000>;
> +			i-cache-line-size = <64>;
> +			i-cache-sets = <256>;
> +			next-level-cache = <&l2>;
> +		};
> +
> +		l2: l2-cache0 {
> +			compatible = "cache";
> +			cache-size = <0x100000>;
> +			cache-unified;
> +			cache-line-size = <64>;
> +			cache-sets = <1024>;
> +			cache-level = <2>;
> +		};
> +	};
> +
> +	timer:timer {

Missing space before node name, but anyway label is unused.

> +		compatible = "arm,armv8-timer";
> +		interrupt-parent = <&gic500>;
> +		interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_HIGH>,
> +			   <GIC_PPI 14 IRQ_TYPE_LEVEL_HIGH>,
> +			   <GIC_PPI 11 IRQ_TYPE_LEVEL_HIGH>,
> +			   <GIC_PPI 10 IRQ_TYPE_LEVEL_HIGH>;
> +		arm,cpu-registers-not-fw-configured;
> +	};
> +
> +	clocks {

Keep proper sorting of nodes, see DTS coding style.

> +		refclk: refclk {
> +			compatible = "fixed-clock";
> +			#clock-cells = <0>;
> +			clock-frequency = <125000000>;
> +		};
> +
> +		ref_clk: ref_clk {

This makes no sense. You have refclk and ref_clk. These ARE THE SAME.
Please use name for all fixed clocks which matches current format
recommendation: 'clock-<freq>' (see also the pattern in the binding for
any other options).

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/clock/fixed-clock.yaml?h=v6.11-rc1

> +			compatible = "fixed-clock";
> +			#clock-cells = <0>;
> +			clock-frequency = <1>;
> +		};
> +
> +		clk_ahb: clk_ahb {
> +			compatible = "fixed-clock";
> +			#clock-cells = <0>;
> +			clock-frequency = <200000000>;
> +			clock-output-names = "clk_ahb";
> +		};
> +
> +		clk_xin: clk_xin {
> +			compatible = "fixed-clock";
> +			#clock-cells = <0>;
> +			clock-frequency = <200000000>;
> +			clock-output-names = "clk_xin";
> +		};
> +
> +		clk_mali: clk_mali {
> +			compatible = "fixed-clock";
> +			#clock-cells = <0>;
> +			clock-frequency = <500000000>;
> +			clock-output-names = "clk_mali";
> +		};
> +
> +		clk_pclk: clk_pclk {
> +			compatible = "fixed-clock";
> +			#clock-cells = <0>;
> +			clock-frequency = <74250000>;
> +			clock-output-names = "clk_pclk";
> +		};
> +
> +		spi_clk: spi_clk {
> +			compatible = "fixed-clock";
> +			#clock-cells = <0>;
> +			clock-frequency = <25000000>;
> +		};
> +
> +		apb_pclk: apb_pclk {

No underscores in node names, but all these look incorrect - don't you
have clock controller?

> +			compatible = "fixed-clock";
> +			#clock-cells = <0>;
> +			clock-frequency = <25000000>;
> +		};
> +	};
> +
> +	soc {
> +		compatible = "simple-bus";
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		interrupt-parent = <&gic500>;
> +		ranges;
> +
> +		gic500: interrupt-controller@80300000 {
> +			compatible = "arm,gic-v3";
> +			#address-cells = <2>;
> +			#size-cells = <2>;
> +			ranges;
> +			#interrupt-cells = <3>;
> +			interrupt-controller;
> +			#redistributor-regions = <1>;
> +			reg = <0x00 0x80300000 0x00 0x10000>,
> +				  <0x00 0x80380000 0x00 0x80000>;

DTS coding style, incorrect order.

> +			interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
> +
> +		};
> +
> +		uart0: serial@80520000 {
> +			compatible = "xlnx,zynqmp-uart", "cdns,uart-r1p12";
> +			interrupt-parent = <&gic500>;
> +			interrupts = <GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH>;
> +			reg = <0x00 0x80520000 0x00 0x100>;

DTS coding style.

> +			clock-names = "uart_clk", "pclk";
> +			clocks = <&refclk &refclk>;
> +			status = "disabled";
> +		};
> +
> +		uart1: serial@805a0000 {
> +			compatible = "xlnx,zynqmp-uart", "cdns,uart-r1p12";
> +			interrupt-parent = <&gic500>;
> +			interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH>;
> +			reg = <0x00 0x805A0000 0x00 0x100>;
> +			clock-names = "uart_clk", "pclk";
> +			clocks = <&refclk &refclk>;
> +			status = "disabled";
> +		};
> +
> +		uart2: serial@80620000 {
> +			compatible = "xlnx,zynqmp-uart", "cdns,uart-r1p12";
> +			interrupt-parent = <&gic500>;
> +			interrupts = <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>;
> +			reg = <0x00 0x80620000 0x00 0x100>;
> +			clock-names = "uart_clk", "pclk";
> +			clocks = <&refclk &refclk>;
> +			status = "disabled";
> +		};
> +
> +		uart3: serial@80520800 {
> +			compatible = "xlnx,zynqmp-uart", "cdns,uart-r1p12";
> +			interrupt-parent = <&gic500>;
> +			interrupts = <GIC_SPI 170 IRQ_TYPE_LEVEL_HIGH>;
> +			reg = <0x00 0x80520800 0x00 0x100>;
> +			clock-names = "uart_clk", "pclk";
> +			clocks = <&refclk &refclk>;
> +			status = "disabled";
> +		};
> +
> +		/* GPIO Controller banks 0 - 7 */
> +		gpio0: gpio-controller@80500000 {
> +			compatible = "cdns,gpio-r1p02";
> +			clocks = <&refclk>;
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			interrupt-parent = <&gic500>;
> +			interrupts = <GIC_SPI 183 IRQ_TYPE_LEVEL_HIGH>;
> +			reg = <0x00 0x80500000 0x00  0x400>;

DTS coding style.


> +			interrupt-controller;
> +			#interrupt-cells = <2>;
> +			status = "disabled";
> +		};
> +


...

> +		i3c14: i3c@80880400 {
> +			compatible = "cdns,i3c-master";
> +			clocks = <&refclk &clk_xin>;
> +			clock-names = "pclk", "sysclk";
> +			interrupt-parent = <&gic500>;
> +			interrupts = <GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH>;
> +			i2c-scl-hz = <100000>;
> +			i3c-scl-hz = <400000>;
> +			reg = <0x00 0x80880400 0x00 0x400>;
> +			#address-cells = <3>;
> +			#size-cells = <0>;
> +			status = "disabled";
> +		};
> +
> +		i3c15: i3c@80880800 {
> +			compatible = "cdns,i3c-master";
> +			clocks = <&refclk &clk_xin>;
> +			clock-names = "pclk", "sysclk";
> +			interrupt-parent = <&gic500>;
> +			interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_HIGH>;
> +			i2c-scl-hz = <100000>;
> +			i3c-scl-hz = <400000>;
> +			reg = <0x00 0x80880800 0x00 0x400>;
> +			#address-cells = <3>;
> +			#size-cells = <0>;
> +			status = "disabled";
> +		};
> +
> +		i3c16: i3c@80620400 {
> +			compatible = "cdns,i3c-master";
> +			clocks = <&refclk &clk_xin>;
> +			clock-names = "pclk", "sysclk";
> +			interrupt-parent = <&gic500>;
> +			interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_HIGH>;
> +			i2c-scl-hz = <100000>;
> +			i3c-scl-hz = <400000>;
> +			reg = <0x00 0x80620400 0x00 0x400>;
> +			#address-cells = <3>;
> +			#size-cells = <0>;
> +			status = "disabled";
> +		};
> +

Drop stray blank lines.

> +	};
> +};
> diff --git a/arch/arm64/boot/dts/axiado/ax3000_evk.dts b/arch/arm64/boot/dts/axiado/ax3000_evk.dts
> new file mode 100644
> index 0000000000000000000000000000000000000000..0a183695e857a3a1e722ea6b7bee388bf650f0a3
> --- /dev/null
> +++ b/arch/arm64/boot/dts/axiado/ax3000_evk.dts
> @@ -0,0 +1,72 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2021-25 Axiado Corporation (or its affiliates). All rights reserved.
> + */
> +
> +/dts-v1/;
> +
> +#include "ax3000.dtsi"
> +
> +/ {
> +	model = "Axiado AX3000 EVK";
> +	compatible = "axiado,ax3000_evk", "axiado,ax3000";
> +	#address-cells = <2>;
> +	#size-cells = <2>;
> +
> +	chosen {
> +		bootargs = "console=ttyPS3,115200 earlyprintk nr_cpus=4 earlycon";

Drop bootargs. Not needed and not suitable for mainline. earlycon (not
earlyprintk!) is debugging tool, not wide mainline usage.


Best regards,
Krzysztof


  reply	other threads:[~2025-06-16  6:12 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-16  4:31 [PATCH v2 0/6] Axiado AX3000 SoC and Evaluation Board Support Harshit Shah
2025-06-16  4:31 ` [PATCH v2 1/6] dt-bindings: vendor-prefixes: Add Axiado Corporation Harshit Shah
2025-06-16  4:31 ` [PATCH v2 2/6] dt-bindings: arm: axiado: add AX3000 EVK compatible strings Harshit Shah
2025-06-16  6:05   ` Krzysztof Kozlowski
2025-06-16 21:13     ` Harshit Shah
2025-06-17  6:08       ` Krzysztof Kozlowski
2025-06-16  4:31 ` [PATCH v2 3/6] dt-bindings: gpio: gpio-cdns: convert to YAML Harshit Shah
2025-06-16  6:05   ` Krzysztof Kozlowski
2025-06-18 21:09     ` Harshit Shah
2025-06-16  4:31 ` [PATCH v2 4/6] arm64: dts: axiado: Add initial support for AX3000 SoC and eval board Harshit Shah
2025-06-16  6:09   ` Krzysztof Kozlowski [this message]
2025-06-19 22:41     ` Harshit Shah
2025-06-20  6:08       ` Krzysztof Kozlowski
2025-06-20 21:18         ` Harshit Shah
2025-06-22  9:49           ` Krzysztof Kozlowski
2025-06-23  5:56             ` Harshit Shah
2025-06-23  6:04               ` Krzysztof Kozlowski
2025-06-16  6:10   ` Krzysztof Kozlowski
2025-06-19 23:52     ` Harshit Shah
2025-06-16 13:57   ` Rob Herring
2025-06-19 23:09     ` Harshit Shah
2025-06-16  4:31 ` [PATCH v2 5/6] arm64: add support for ARCH_AXIADO Harshit Shah
2025-06-16  4:31 ` [PATCH v2 6/6] MAINTAINERS: Add entry for AXIADO Harshit Shah

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6ef92d1a-39cc-409f-8ebe-d28ad2006988@kernel.org \
    --to=krzk@kernel.org \
    --cc=arnd@arndb.de \
    --cc=brgl@bgdev.pl \
    --cc=catalin.marinas@arm.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=hshah@axiado.com \
    --cc=krzk+dt@kernel.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robh@kernel.org \
    --cc=soc@lists.linux.dev \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.