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

[Albert Yang <yangzh0906@thundersoft.com>]

On Tue, Aug 12, 2025 at 06:16:08PM +0200, Krzysztof Kozlowski wrote:
> On 12/08/2025 14:31, Albert Yang wrote:

Thank you for the detailed review and your patience. Apologies for not addressing these properly in v1/v2.
I have fixed the issues in my local tree and will send v4. Below are my point-by-point replies.


> > +&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?
>
Thank you for pointing this out. I did not follow the DTS coding style correctly earlier. I have re-read the DTS coding style and the relevant dt-bindings documentation:
https://docs.kernel.org/devicetree/bindings/dts-coding-style.html


> > +           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.
>
I referenced the ARM CPU node binding: Documentation/devicetree/bindings/arm/cpus.yaml
Updated as follows
		cpu@0 {
			device_type = "cpu";
			compatible = "arm,cortex-a78";
			reg = <0x0>;
			enable-method = "psci";
			next-level-cache = <&l2_cache>;
		};


> > +           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?

Good catch. The device is not SoC-embedded; it is a board-level eMMC. Changes:
- Removed non-removable from the SoC dtsi
- Kept the controller disabled in the SoC dtsi
- Enabled the controller and set non-removable in the board DTS (&mmc0)
Updates:
in bstc1200.dtsi:
		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>;
			status = "disabled";
		};
in board bstc1200-cdcu1.0-adas_4c2g.dts:
&mmc0 {
	bus-width = <8>;
	memory-region = <&mmc0_reserved>;
	non-removable;
	status = "okay";
};


> > +           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...

Refs: arm,gic‑v3 binding
https://raw.githubusercontent.com/torvalds/linux/master/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml
Updated as follows:
		gic: interrupt-controller@32800000 {
			compatible = "arm,gic-v3";
			#interrupt-cells = <3>;
			#address-cells = <2>;
			#size-cells = <2>;
			ranges;
			interrupt-controller;
			reg = <0x0 0x32800000 0x0 0x10000>,
			      <0x0 0x32880000 0x0 0x100000>;
			interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_LOW>;
		};
  
Thanks again for the review. If you have no objections to the current modifications,
I’ll post v4 accordingly.

Best regards,
Albert