Message-ID: <5cc644f8-7394-48f2-b62b-1e7cd5ce27d3@ieee.org>
Date: Thu, 19 Jun 2025 10:11:32 -0500
From: Alex Elder <elder@...e.org>
To: Vivian Wang <wangruikang@...as.ac.cn>, Yixun Lan <dlan@...too.org>,
 Guodong Xu <guodong@...cstar.com>, Ze Huang <huangze@...t.edu.cn>,
 spacemit@...ts.linux.dev
Cc: Vivian Wang <uwu@...m.page>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Paul Walmsley <paul.walmsley@...ive.com>,
 Palmer Dabbelt <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>,
 Alexandre Ghiti <alex@...ti.fr>, devicetree@...r.kernel.org,
 linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC] riscv: dts: spacemit: Add DMA translation buses for
 K1

On 6/17/25 12:21 AM, Vivian Wang wrote:
> The SpacemiT K1 has various static translations of DMA accesses. Add
> these as simple-bus nodes. Devices actually using these translation will
> be added in later patches.
> 
> The bus names are assigned according to consensus with SpacemiT [1].
> 
> [1] https://lore.kernel.org/all/CAH1PCMaC+imcMZCFYtRdmH6ge=dPgnANn_GqVfsGRS=+YhyJCw@mail.gmail.com/

So what you include here very closely matches what Guodong
said in the message above.  Yours differs from his proposal
and that makes it hard to compare them.  I have a few comments
on that below.

> Signed-off-by: Vivian Wang <wangruikang@...as.ac.cn>
> ---
> This is my concrete proposal for representing DMA translations for
> SpacemiT K1.

It's worth acknowledging that this is derived from what Guodong
proposed (it's not "your" proposal in that respect).  That said,
yours is a more complete and "formal" RFP than what he wrote.

> For context, memory on the SpacemiT K1 is split into two chunks:
> 
> - 0x0000_0000 to 0x8000_0000: First 2 GiB of memory
> - 0x1_0000_0000 above: Rest of memory
> 
> DMA-capable devices on the K1 all have access to the lower 2G of memory
> through an identity mapping. However, for the upper region of memory,
> each device falls under one of six different mappings. The mappings are
> provided in this patch as simple-bus nodes that device nodes should be
> added to.
> 
> This patch is an RFC because it is not meant to be applied, or at least,
> not certainly meant to be applied. Instead, this is an attempt to come
> to a consensus on how these bus nodes should look like.

I think the above is what Krzysztof might not have seen.  Perhaps
it could have been made more clear--maybe in the "main" description
section (above the ---) or even the subject line.

> More specifically, I propose that the process proceeds as follows:
> 
> - Firstly, relevant parties agree on these bus nodes given here.
> - After that, each time the first user of a bus appears, the series
>    should include a patch to add the bus required for that driver.
> - If a driver being submitted uses the same bus as another one that has
>    been submitted but hasn't yet landed, it can depend on the bus patch
>    from that previous series.

Getting agreement is good, but otherwise this is basically
the process Guodong was suggesting, right?

> For conventions regarding coding style, I propose that:
> 
> - #address-cells and #size-cells are 2 for consistency
> - These bus nodes are put at the end of /soc, inside /soc
> - These bus nodes are sorted alphabetically, not in vendor's order
> - Devices are added into *-bus nodes directly, not appended towards the
>    end with a label reference

I do like that you're trying to be more complete and explicit
on what you think needs agreement on.

> The K1 DMA translations are *not* interconnects, since they do not
> provide any configuration capabilities.
> 
> These bus nodes names and properties are provided compliant with
> "simple-bus" bindings, and should pass "make dtbs_check".
> 
> Remaining questions:
> 
> - Should storage-bus exist? Or should drivers under it simply specify
>    32-bit DMA?

Explicitly saying storage devices have one-to-one mapping
seems informative, to me.

> ---
>   arch/riscv/boot/dts/spacemit/k1.dtsi | 53 ++++++++++++++++++++++++++++++++++++
>   1 file changed, 53 insertions(+)

The short summary of what differs between your proposal
and what Guodong said is:
- You sort nodes alphabetically, Guodong did not
- You dropped the unit address
- You dropped the comments he had, which indicated which
   devices "belonged" to each mapping
- You added a compatible property to each ("simple-bus")
- You added an explicit (empty) ranges property to each
- You add #address-cells and #size-cells properties, both 2
- Your dma-ranges properties are identical to Guodong's,
   for all nodes

My comments:
- I agree with you that a unit address doesn't really make
   sense, because there is no meaningful ordering
- Sorting alphabetically also makes sense to me
- I think the comments about devices associated with each
   mapping were informative

I think Yixun should manage getting consensus on this
(i.e., he should sign off on the RFP somehow).

					-Alex

> diff --git a/arch/riscv/boot/dts/spacemit/k1.dtsi b/arch/riscv/boot/dts/spacemit/k1.dtsi
> index c0f8c5fca975d73b6ea6886da13fcf55289cb16c..efefed21b9fa1ab9c6ac3d24cd0cca8958b85184 100644
> --- a/arch/riscv/boot/dts/spacemit/k1.dtsi
> +++ b/arch/riscv/boot/dts/spacemit/k1.dtsi
> @@ -562,5 +562,58 @@ sec_uart1: serial@...12000 {
>   			reg-io-width = <4>;
>   			status = "reserved"; /* for TEE usage */
>   		};
> +
> +		camera-bus {
> +			compatible = "simple-bus";
> +			ranges;
> +			#address-cells = <2>;
> +			#size-cells = <2>;
> +			dma-ranges = <0x0 0x00000000 0x0 0x00000000 0x0 0x80000000>,
> +				     <0x0 0x80000000 0x1 0x00000000 0x1 0x80000000>;
> +		};
> +
> +		dma-bus {
> +			compatible = "simple-bus";
> +			ranges;
> +			#address-cells = <2>;
> +			#size-cells = <2>;
> +			dma-ranges = <0x0 0x00000000 0x0 0x00000000 0x0 0x80000000>,
> +				     <0x1 0x00000000 0x1 0x80000000 0x3 0x00000000>;
> +		};
> +
> +		multimedia-bus {
> +			compatible = "simple-bus";
> +			ranges;
> +			#address-cells = <2>;
> +			#size-cells = <2>;
> +			dma-ranges = <0x0 0x00000000 0x0 0x00000000 0x0 0x80000000>,
> +				     <0x0 0x80000000 0x1 0x00000000 0x3 0x80000000>;
> +		};
> +
> +		network-bus {
> +			compatible = "simple-bus";
> +			ranges;
> +			#address-cells = <2>;
> +			#size-cells = <2>;
> +			dma-ranges = <0x0 0x00000000 0x0 0x00000000 0x0 0x80000000>,
> +				     <0x0 0x80000000 0x1 0x00000000 0x0 0x80000000>;
> +		};
> +
> +		pcie-bus {
> +			compatible = "simple-bus";
> +			ranges;
> +			#address-cells = <2>;
> +			#size-cells = <2>;
> +			dma-ranges = <0x0 0x00000000 0x0 0x00000000 0x0 0x80000000>,
> +				     <0x0 0xb8000000 0x1 0x38000000 0x3 0x48000000>;
> +		};
> +
> +		storage-bus {
> +			compatible = "simple-bus";
> +			ranges;
> +			#address-cells = <2>;
> +			#size-cells = <2>;
> +			dma-ranges = <0x0 0x00000000 0x0 0x00000000 0x0 0x80000000>;
> +		};
>   	};
>   };
> 
> ---
> base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
> change-id: 20250616-k1-dma-buses-rfc-wip-3be7a01f47c8
> 
> Best regards,

Powered by blists - more mailing lists