[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ce1285be-5a3f-4d4b-bca1-e31f56573597@kernel.org>
Date: Tue, 12 Aug 2025 18:16:08 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Albert Yang <yangzh0906@...ndersoft.com>, robh@...nel.org,
krzk+dt@...nel.org, conor+dt@...nel.org, ulf.hansson@...aro.org,
catalin.marinas@....com, will@...nel.org, arnd@...db.de,
adrian.hunter@...el.com, robin.murphy@....com, ding.wang@....ai,
gordon.ge@....ai
Cc: bst-upstream@...ai.top, linux-arm-kernel@...ts.infradead.org,
devicetree@...r.kernel.org, linux-mmc@...r.kernel.org, soc@...ts.linux.dev,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 6/8] arm64: dts: bst: add support for Black Sesame
Technologies C1200 CDCU1.0 board
On 12/08/2025 14:31, Albert Yang wrote:
> Add device tree support for the Black Sesame Technologies (BST) C1200
> CDCU1.0 ADAS 4C2G platform. This platform is based on the BST C1200 SoC
> family.
>
> The changes include:
> - Adding a new BST device tree directory
> - Adding Makefile entries to build the BST platform device trees
> - Adding the device tree for the BST C1200 CDCU1.0 ADAS 4C2G board
>
> This board features a quad-core Cortex-A78 CPU, and various peripherals
> including UART, MMC, watchdog timer, and interrupt controller.
>
> Signed-off-by: Ge Gordon <gordon.ge@....ai>
> Signed-off-by: Albert Yang <yangzh0906@...ndersoft.com>
> ---
> Changes for v3:
> - Split defconfig enablement out into a dedicated defconfig patch
> - Refine memory description: consolidate ranges in memory node and
> delele unused memory ranges
> - Adjust the order of nodes
> - remove mask of gic
>
> Changes for v2:
> 1. Reorganized memory map into discrete regions
> 2. Updated MMC controller definition:
> - Split into core/CRM register regions
> - Removed deprecated properties
> - Updated compatible string
> 3. Standardized interrupt definitions and numeric formats
> 4. Removed reserved-memory node (superseded by bounce buffers)
> 5. Added root compatible string for platform identification
> 6. Add soc defconfig
> ---
> arch/arm64/boot/dts/Makefile | 1 +
> arch/arm64/boot/dts/bst/Makefile | 2 +
> .../dts/bst/bstc1200-cdcu1.0-adas_4c2g.dts | 42 +++++++
> arch/arm64/boot/dts/bst/bstc1200.dtsi | 117 ++++++++++++++++++
> 4 files changed, 162 insertions(+)
> create mode 100644 arch/arm64/boot/dts/bst/Makefile
> create mode 100644 arch/arm64/boot/dts/bst/bstc1200-cdcu1.0-adas_4c2g.dts
> create mode 100644 arch/arm64/boot/dts/bst/bstc1200.dtsi
>
> diff --git a/arch/arm64/boot/dts/Makefile b/arch/arm64/boot/dts/Makefile
> index 79b73a21ddc2..a39b6cafb644 100644
> --- a/arch/arm64/boot/dts/Makefile
> +++ b/arch/arm64/boot/dts/Makefile
> @@ -12,6 +12,7 @@ subdir-y += arm
> subdir-y += bitmain
> subdir-y += blaize
> subdir-y += broadcom
> +subdir-y += bst
> subdir-y += cavium
> subdir-y += exynos
> subdir-y += freescale
> diff --git a/arch/arm64/boot/dts/bst/Makefile b/arch/arm64/boot/dts/bst/Makefile
> new file mode 100644
> index 000000000000..4c1b8b4cdad8
> --- /dev/null
> +++ b/arch/arm64/boot/dts/bst/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0
> +dtb-$(CONFIG_ARCH_BST) += bstc1200-cdcu1.0-adas_4c2g.dtb
> diff --git a/arch/arm64/boot/dts/bst/bstc1200-cdcu1.0-adas_4c2g.dts b/arch/arm64/boot/dts/bst/bstc1200-cdcu1.0-adas_4c2g.dts
> new file mode 100644
> index 000000000000..d8fb07b0bc80
> --- /dev/null
> +++ b/arch/arm64/boot/dts/bst/bstc1200-cdcu1.0-adas_4c2g.dts
> @@ -0,0 +1,42 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/dts-v1/;
> +
> +#include "bstc1200.dtsi"
> +
> +/ {
> + model = "BST C1200-96 CDCU1.0 4C2G";
> + compatible = "bst,c1200-cdcu1.0-adas-4c2g", "bst,c1200";
> +
> + chosen {
> + stdout-path = "serial0:115200n8";
> + };
> +
> + memory@...000000 {
> + device_type = "memory";
> + reg = <0x8 0x10000000 0x0 0x30000000>,
> + <0x8 0xc0000000 0x1 0x0>,
> + <0xc 0x00000000 0x0 0x40000000>;
> + };
> +
> + reserved-memory {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> +
> + mmc0_reserved: mmc0-reserved@...0000 {
> + compatible = "shared-dma-pool";
> + reg = <0x0 0x5160000 0x0 0x10000>;
> + no-map;
> + };
> + };
> +};
> +
> +&uart0 {
> + status = "okay";
> +};
> +
> +&mmc0 {
This is none of the two approved ordering styles from DTS coding style.
What sort of coding style are you using?
> + status = "okay";
> + memory-region = <&mmc0_reserved>;
> +};
> +
> diff --git a/arch/arm64/boot/dts/bst/bstc1200.dtsi b/arch/arm64/boot/dts/bst/bstc1200.dtsi
> new file mode 100644
> index 000000000000..5e9ca0ee17cf
> --- /dev/null
> +++ b/arch/arm64/boot/dts/bst/bstc1200.dtsi
> @@ -0,0 +1,117 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/interrupt-controller/irq.h>
> +
> +/ {
> + compatible = "bst,c1200";
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + clk_mmc: clock-4000000 {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <4000000>;
> + };
> +
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + cpu@0 {
> + compatible = "arm,cortex-a78";
> + device_type = "cpu";
> + enable-method = "psci";
> + next-level-cache = <&l2_cache>;
> + reg = <0>;
0x0
And why reg is the last? Please follow DTS coding style.
I already asked for this at v1. How did you resolve that comment?
Then I asked about this at v2:
"Nothing improved. I asked to follow DTS coding style in ordering."
So can you please respond to comments? You keep sending the same - third
time - and this is waste of our time.
> + };
> +
> + cpu@1 {
> + compatible = "arm,cortex-a78";
> + device_type = "cpu";
> + enable-method = "psci";
> + next-level-cache = <&l2_cache>;
> + reg = <0x100>;
> + };
> +
> + cpu@2 {
> + compatible = "arm,cortex-a78";
> + device_type = "cpu";
> + enable-method = "psci";
> + next-level-cache = <&l2_cache>;
> + reg = <0x200>;
> + };
> +
> + cpu@3 {
> + compatible = "arm,cortex-a78";
> + device_type = "cpu";
> + enable-method = "psci";
> + next-level-cache = <&l2_cache>;
> + reg = <0x300>;
> + };
> +
> + l2_cache: l2-cache {
> + compatible = "cache";
> + cache-level = <2>;
> + cache-unified;
> + };
> + };
> +
> + psci {
> + compatible = "arm,psci-1.0";
> + method = "smc";
> + };
> +
> + soc {
> + compatible = "simple-bus";
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> + interrupt-parent = <&gic>;
> +
> + uart0: serial@...08000 {
> + compatible = "snps,dw-apb-uart";
> + reg = <0x0 0x20008000 0x0 0x1000>;
> + interrupts = <GIC_SPI 211 IRQ_TYPE_LEVEL_HIGH>;
> + clock-frequency = <25000000>;
> + reg-shift = <2>;
> + reg-io-width = <4>;
> + status = "disabled";
> + };
> +
> + mmc0: mmc@...00000 {
> + compatible = "bst,c1200-dwcmshc-sdhci";
> + reg = <0x0 0x22200000 0x0 0x1000>,
> + <0x0 0x23006000 0x0 0x1000>;
> + interrupts = <GIC_SPI 144 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clk_mmc>;
> + clock-names = "core";
> + max-frequency = <200000000>;
> + bus-width = <8>;
> + non-removable;
Hm, this is odd to see in SoC. Are you saying that your SoC (!) has MMC
memory embedded?
> + dma-coherent;
> + status = "disabled";
If so, why is it disabled?
> + };
> +
> + gic: interrupt-controller@...00000 {
> + compatible = "arm,gic-v3";
> + #interrupt-cells = <3>;
> + #address-cells = <2>;
> + #size-cells = <2>;
> + interrupt-controller;
> + ranges;
> + reg = <0x0 0x32800000 0x0 0x10000>,
> + <0x0 0x32880000 0x0 0x100000>;
Random order...
Best regards,
Krzysztof
Powered by blists - more mailing lists