Re: [PATCH v3 6/8] arm64: dts: bst: add support for Black Sesame Technologies C1200 CDCU1.0 board

[Krzysztof Kozlowski <krzk@kernel.org>]

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@bst.ai>
> Signed-off-by: Albert Yang <yangzh0906@thundersoft.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@810000000 {
> +		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@5160000 {
> +			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@20008000 {
> +			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@22200000 {
> +			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@32800000 {
> +			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