public inbox for devicetree@vger.kernel.org
 help / color / mirror / Atom feed
From: Alex Elder <elder@ieee.org>
To: Vivian Wang <wangruikang@iscas.ac.cn>,
	Yixun Lan <dlan@gentoo.org>, Guodong Xu <guodong@riscstar.com>,
	Ze Huang <huangze@whut.edu.cn>,
	spacemit@lists.linux.dev
Cc: Vivian Wang <uwu@dram.page>, Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Alexandre Ghiti <alex@ghiti.fr>,
	devicetree@vger.kernel.org, linux-riscv@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH RFC] riscv: dts: spacemit: Add DMA translation buses for K1
Date: Thu, 19 Jun 2025 10:11:32 -0500	[thread overview]
Message-ID: <5cc644f8-7394-48f2-b62b-1e7cd5ce27d3@ieee.org> (raw)
In-Reply-To: <20250617-k1-dma-buses-rfc-wip-v1-1-c8ec192fbf58@iscas.ac.cn>

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@iscas.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@f0612000 {
>   			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,


  parent reply	other threads:[~2025-06-19 15:11 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-17  5:21 [PATCH RFC] riscv: dts: spacemit: Add DMA translation buses for K1 Vivian Wang
2025-06-17  6:21 ` Krzysztof Kozlowski
2025-06-17  8:48   ` Vivian Wang
2025-06-17  9:35     ` Krzysztof Kozlowski
2025-06-17 10:10       ` Vivian Wang
2025-06-19 15:11 ` Alex Elder [this message]
2025-06-19 15:42   ` Vivian Wang
2025-06-20 10:56     ` Yixun Lan
2025-06-20 14:10       ` Guodong Xu
2025-06-20 14:57         ` Yixun Lan
2025-06-23  7:01           ` Yixun Lan
2025-06-23  9:32             ` Guodong Xu
2025-06-23  9:42               ` Vivian Wang

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=5cc644f8-7394-48f2-b62b-1e7cd5ce27d3@ieee.org \
    --to=elder@ieee.org \
    --cc=alex@ghiti.fr \
    --cc=aou@eecs.berkeley.edu \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dlan@gentoo.org \
    --cc=guodong@riscstar.com \
    --cc=huangze@whut.edu.cn \
    --cc=krzk+dt@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    --cc=robh@kernel.org \
    --cc=spacemit@lists.linux.dev \
    --cc=uwu@dram.page \
    --cc=wangruikang@iscas.ac.cn \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox