* [PATCH RFC] riscv: dts: spacemit: Add DMA translation buses for K1
@ 2025-06-17 5:21 Vivian Wang
2025-06-17 6:21 ` Krzysztof Kozlowski
2025-06-19 15:11 ` Alex Elder
0 siblings, 2 replies; 13+ messages in thread
From: Vivian Wang @ 2025-06-17 5:21 UTC (permalink / raw)
To: Yixun Lan, Guodong Xu, Ze Huang, spacemit
Cc: Vivian Wang, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
devicetree, linux-riscv, linux-kernel, Vivian Wang
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/
Signed-off-by: Vivian Wang <wangruikang@iscas.ac.cn>
---
This is my concrete proposal for representing DMA translations for
SpacemiT K1.
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.
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.
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
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?
---
arch/riscv/boot/dts/spacemit/k1.dtsi | 53 ++++++++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)
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,
--
Vivian "dramforever" Wang
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH RFC] riscv: dts: spacemit: Add DMA translation buses for K1
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-19 15:11 ` Alex Elder
1 sibling, 1 reply; 13+ messages in thread
From: Krzysztof Kozlowski @ 2025-06-17 6:21 UTC (permalink / raw)
To: Vivian Wang, Yixun Lan, Guodong Xu, Ze Huang, spacemit
Cc: Vivian Wang, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
devicetree, linux-riscv, linux-kernel
On 17/06/2025 07:21, 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].
Read the feedback there:
"So, as you are submitting the first node(s) under network_bus: bus@5, you
should have this added into your patchset, instead of sending out with
none."
Plus simple bus within MMIO node needs unit address. IOW, don't mix MMIO
with non-MMIO. I also suspect this does not pass checks, so the tools
can do our review...
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH RFC] riscv: dts: spacemit: Add DMA translation buses for K1
2025-06-17 6:21 ` Krzysztof Kozlowski
@ 2025-06-17 8:48 ` Vivian Wang
2025-06-17 9:35 ` Krzysztof Kozlowski
0 siblings, 1 reply; 13+ messages in thread
From: Vivian Wang @ 2025-06-17 8:48 UTC (permalink / raw)
To: Krzysztof Kozlowski, Yixun Lan, Guodong Xu, Ze Huang, spacemit
Cc: Vivian Wang, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
devicetree, linux-riscv, linux-kernel
Hi Krzysztof,
On 6/17/25 14:21, Krzysztof Kozlowski wrote:
> On 17/06/2025 07:21, 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].
>
> Read the feedback there:
>
> "So, as you are submitting the first node(s) under network_bus: bus@5, you
> should have this added into your patchset, instead of sending out with
> none."
As mentioned in the patch extra message, this is an RFC meant for
achieving consensus on what the bus nodes should look like, not an
actual patch meant to be taken. I was hoping I was clear on that, but I
guess that paragraph was buried too deep. Well...
> Plus simple bus within MMIO node needs unit address. IOW, don't mix MMIO
> with non-MMIO. I also suspect this does not pass checks, so the tools
> can do our review...
This DT passes "make dtbs_check" fine, with only unrelated warnings on
sec_uart1 that was already there before:
DTC [C] arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dtb
.../arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dtb: serial@f0612000
(spacemit,k1-uart): 'clock-names' is a required property
from schema $id: http://devicetree.org/schemas/serial/8250.yaml#
DTC [C] arch/riscv/boot/dts/spacemit/k1-milkv-jupiter.dtb
.../arch/riscv/boot/dts/spacemit/k1-milkv-jupiter.dtb: serial@f0612000
(spacemit,k1-uart): 'clock-names' is a required property
from schema $id: http://devicetree.org/schemas/serial/8250.yaml#
To be honest, I don't understand what "within MMIO node" means here.
Should the buses be taken out of /soc and added as its siblings?
Thanks,
Vivian "dramforever" Wang
> Best regards,
> Krzysztof
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH RFC] riscv: dts: spacemit: Add DMA translation buses for K1
2025-06-17 8:48 ` Vivian Wang
@ 2025-06-17 9:35 ` Krzysztof Kozlowski
2025-06-17 10:10 ` Vivian Wang
0 siblings, 1 reply; 13+ messages in thread
From: Krzysztof Kozlowski @ 2025-06-17 9:35 UTC (permalink / raw)
To: Vivian Wang, Yixun Lan, Guodong Xu, Ze Huang, spacemit
Cc: Vivian Wang, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
devicetree, linux-riscv, linux-kernel
On 17/06/2025 10:48, Vivian Wang wrote:
> Hi Krzysztof,
>
> On 6/17/25 14:21, Krzysztof Kozlowski wrote:
>> On 17/06/2025 07:21, 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].
>>
>> Read the feedback there:
>>
>> "So, as you are submitting the first node(s) under network_bus: bus@5, you
>> should have this added into your patchset, instead of sending out with
>> none."
> As mentioned in the patch extra message, this is an RFC meant for
> achieving consensus on what the bus nodes should look like, not an
> actual patch meant to be taken. I was hoping I was clear on that, but I
> guess that paragraph was buried too deep. Well...
>> Plus simple bus within MMIO node needs unit address. IOW, don't mix MMIO
>> with non-MMIO. I also suspect this does not pass checks, so the tools
>> can do our review...
>
> This DT passes "make dtbs_check" fine, with only unrelated warnings on
> sec_uart1 that was already there before:
>
> DTC [C] arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dtb
> .../arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dtb: serial@f0612000
> (spacemit,k1-uart): 'clock-names' is a required property
> from schema $id: http://devicetree.org/schemas/serial/8250.yaml#
> DTC [C] arch/riscv/boot/dts/spacemit/k1-milkv-jupiter.dtb
> .../arch/riscv/boot/dts/spacemit/k1-milkv-jupiter.dtb: serial@f0612000
> (spacemit,k1-uart): 'clock-names' is a required property
> from schema $id: http://devicetree.org/schemas/serial/8250.yaml#
>
> To be honest, I don't understand what "within MMIO node" means here.
> Should the buses be taken out of /soc and added as its siblings?
These looks like children of simple-bus. If that's right: children of
simple-bus are supposed to have unit addresses (see also simple-bus in
the DT schema).
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH RFC] riscv: dts: spacemit: Add DMA translation buses for K1
2025-06-17 9:35 ` Krzysztof Kozlowski
@ 2025-06-17 10:10 ` Vivian Wang
0 siblings, 0 replies; 13+ messages in thread
From: Vivian Wang @ 2025-06-17 10:10 UTC (permalink / raw)
To: Krzysztof Kozlowski, Vivian Wang, Yixun Lan, Guodong Xu, Ze Huang,
spacemit
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Alexandre Ghiti, devicetree,
linux-riscv, linux-kernel
On 6/17/25 17:35, Krzysztof Kozlowski wrote:
> On 17/06/2025 10:48, Vivian Wang wrote:
>> Hi Krzysztof,
>>
>> On 6/17/25 14:21, Krzysztof Kozlowski wrote:
>>> On 17/06/2025 07:21, 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].
>>> Read the feedback there:
>>>
>>> "So, as you are submitting the first node(s) under network_bus: bus@5, you
>>> should have this added into your patchset, instead of sending out with
>>> none."
>> As mentioned in the patch extra message, this is an RFC meant for
>> achieving consensus on what the bus nodes should look like, not an
>> actual patch meant to be taken. I was hoping I was clear on that, but I
>> guess that paragraph was buried too deep. Well...
>>> Plus simple bus within MMIO node needs unit address. IOW, don't mix MMIO
>>> with non-MMIO. I also suspect this does not pass checks, so the tools
>>> can do our review...
>> This DT passes "make dtbs_check" fine, with only unrelated warnings on
>> sec_uart1 that was already there before:
>>
>> DTC [C] arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dtb
>> .../arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dtb: serial@f0612000
>> (spacemit,k1-uart): 'clock-names' is a required property
>> from schema $id: http://devicetree.org/schemas/serial/8250.yaml#
>> DTC [C] arch/riscv/boot/dts/spacemit/k1-milkv-jupiter.dtb
>> .../arch/riscv/boot/dts/spacemit/k1-milkv-jupiter.dtb: serial@f0612000
>> (spacemit,k1-uart): 'clock-names' is a required property
>> from schema $id: http://devicetree.org/schemas/serial/8250.yaml#
>>
>> To be honest, I don't understand what "within MMIO node" means here.
>> Should the buses be taken out of /soc and added as its siblings?
> These looks like children of simple-bus. If that's right: children of
> simple-bus are supposed to have unit addresses (see also simple-bus in
> the DT schema).
The history of the simple-bus.yaml bindings file suggests that a
simple-bus with empty ranges and no unit address as a child of a parent
simple-bus is intentionally allowed [1]. The commit message also
suggests that even in older versions of the same file it was not the
intention to disallow such nested buses.
[1]: https://github.com/devicetree-org/dt-schema/commit/ed9190d20f146d13e262cc9138506326f7d4da91
Regards,
Vivian "dramforever" Wang
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH RFC] riscv: dts: spacemit: Add DMA translation buses for K1
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-19 15:11 ` Alex Elder
2025-06-19 15:42 ` Vivian Wang
1 sibling, 1 reply; 13+ messages in thread
From: Alex Elder @ 2025-06-19 15:11 UTC (permalink / raw)
To: Vivian Wang, Yixun Lan, Guodong Xu, Ze Huang, spacemit
Cc: Vivian Wang, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
devicetree, linux-riscv, linux-kernel
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,
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH RFC] riscv: dts: spacemit: Add DMA translation buses for K1
2025-06-19 15:11 ` Alex Elder
@ 2025-06-19 15:42 ` Vivian Wang
2025-06-20 10:56 ` Yixun Lan
0 siblings, 1 reply; 13+ messages in thread
From: Vivian Wang @ 2025-06-19 15:42 UTC (permalink / raw)
To: Alex Elder, Vivian Wang, Yixun Lan, Guodong Xu, Ze Huang,
spacemit
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Alexandre Ghiti, devicetree,
linux-riscv, linux-kernel
Hi Alex,
Thank you for your comments on this.
On 6/19/25 23:11, Alex Elder wrote:
> 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.
>
I had thought that since the addresses were already there in vendor's DT
[2], and the names were provided by SpacemiT, anything other than the
names was "well-known information". In retrospect, I should have made
the chain of information of this clearer and make it explicit that this
was based on Guodong's note.
So, just to be clear, the information in my proposal is based on
Guodong's reply [1] (link the quoted text), which I had assumed, but not
explicitly confirmed, was based on already addresses in SpacemiT's DT
and names provided by SpacemiT.
[2]: https://github.com/spacemit-com/linux-k1x/blob/k1/arch/riscv/boot/dts/spacemit/k1-x.dtsi
>> 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.
>
Yeah, that's my mistake in organizing the paragraphs.
>> 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?
Hmm, actually re-reading the discussion now, I realized that I may have
come to this late and missed out on some previous discussions, which
were alluded to in Yixun's messages. (This is again thread around link
[1] in quoted text.) This led me to believe that some of these were not
really agreed upon.
I also realized I think one of the things I may have not yet made clear
is that I would like the bus node to be a *separate* patch. I think this
makes sense, because it's dealing with two different subsystems.
>
>> 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.
>
Being thorough was the main goal of this RFC. If there was previous
agreement on how dma-ranges should be done, I'm sorry for missing them,
but from my observations on the mailing list on how these ended up into
patches I really haven't seen much consistency. Maybe there was
misunderstanding, which I'm hoping to clear up.
(Although see my paragraph above, maybe I haven't been thorough enough.)
>> 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
>
That was a good summary. Thanks!
My main goal of organizing the bus this way is making it actually pass
"make dtbs_check". I'm not sure if Krzysztof still objects to my reading
of simple-bus.yaml though.
By the way, I don't think I will be making an RFC v2 of this. I think we
should get everything sorted under this one thread.
Thanks again for taking a look.
Vivian "dramforever" Wang
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH RFC] riscv: dts: spacemit: Add DMA translation buses for K1
2025-06-19 15:42 ` Vivian Wang
@ 2025-06-20 10:56 ` Yixun Lan
2025-06-20 14:10 ` Guodong Xu
0 siblings, 1 reply; 13+ messages in thread
From: Yixun Lan @ 2025-06-20 10:56 UTC (permalink / raw)
To: Vivian Wang
Cc: Alex Elder, Guodong Xu, Ze Huang, spacemit, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Alexandre Ghiti, devicetree, linux-riscv, linux-kernel
Hi Vivian, Alex,
On 23:42 Thu 19 Jun , Vivian Wang wrote:
> Hi Alex,
>
> Thank you for your comments on this.
>
> On 6/19/25 23:11, Alex Elder wrote:
> > 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.
> >
> I had thought that since the addresses were already there in vendor's DT
> [2], and the names were provided by SpacemiT, anything other than the
> names was "well-known information". In retrospect, I should have made
> the chain of information of this clearer and make it explicit that this
> was based on Guodong's note.
>
> So, just to be clear, the information in my proposal is based on
> Guodong's reply [1] (link the quoted text), which I had assumed, but not
> explicitly confirmed, was based on already addresses in SpacemiT's DT
> and names provided by SpacemiT.
>
> [2]: https://github.com/spacemit-com/linux-k1x/blob/k1/arch/riscv/boot/dts/spacemit/k1-x.dtsi
>
> >> 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.
> >
> Yeah, that's my mistake in organizing the paragraphs.
>
> >> 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?
>
> Hmm, actually re-reading the discussion now, I realized that I may have
> come to this late and missed out on some previous discussions, which
> were alluded to in Yixun's messages. (This is again thread around link
> [1] in quoted text.) This led me to believe that some of these were not
> really agreed upon.
>
> I also realized I think one of the things I may have not yet made clear
> is that I would like the bus node to be a *separate* patch. I think this
> makes sense, because it's dealing with two different subsystems.
>
> >
> >> 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.
> >
> Being thorough was the main goal of this RFC. If there was previous
> agreement on how dma-ranges should be done, I'm sorry for missing them,
> but from my observations on the mailing list on how these ended up into
> patches I really haven't seen much consistency. Maybe there was
> misunderstanding, which I'm hoping to clear up.
>
> (Although see my paragraph above, maybe I haven't been thorough enough.)
>
> >> 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.
sounds good to be explicit
> >
> >> ---
> >> 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
I'd agree with not adding unit number to the simple-bus
> > - You dropped the comments he had, which indicated which
> > devices "belonged" to each mapping
I went ahead and checked those comments, and found them all about
devices under specific bus, I'm not strongly against adding the
comments but feel it's kind of unnecessary, or even in worst cases,
it may bring extra confusions.. on the other hand, you can always
check device nodes under the bus to find what's there.
exmaple for dram4_range(vendor code)/dma_bus, the comments is
/* DMA controller, and users */
what's is 'users'? still have to check the dts, and find them -
uart, spi, i2c, qspi, hdmi, sounds..
If people really want to add comments and help others to understand
this patch, then I'd suggest to add explanation in commit message(better?)
to fully describe all the busses, or why choose this name? -
storage/multimedia/pcie/camera/dma/network_bus
pretty much in much high level perspective..
> > - 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
I think those all above already exist in Guodong's version which
align his idea
> >
> That was a good summary. Thanks!
>
> My main goal of organizing the bus this way is making it actually pass
> "make dtbs_check". I'm not sure if Krzysztof still objects to my reading
> of simple-bus.yaml though.
It would be great if DT maintainer can clarify, or give an ACK
>
> By the way, I don't think I will be making an RFC v2 of this. I think we
> should get everything sorted under this one thread.
>
Instead, from a SoC tree maintainer's perspective (whom taking care of
merging all the dts files), I'd rather perfer an independent or
separated patch for this given every party reached consesus, so we could
get this patch merged first and early, instead of getting them distributed all
over in different series, IMO, separated patches brings more dedependencies
if more than two series require one bus and result in more merge conflicts..
Besides, introducing new busses result in re-arrangement of previous nodes,
those like uart, i2c (even they have no DMA feature implemented currently)..
> Thanks again for taking a look.
>
> Vivian "dramforever" Wang
>
--
Yixun Lan (dlan)
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH RFC] riscv: dts: spacemit: Add DMA translation buses for K1
2025-06-20 10:56 ` Yixun Lan
@ 2025-06-20 14:10 ` Guodong Xu
2025-06-20 14:57 ` Yixun Lan
0 siblings, 1 reply; 13+ messages in thread
From: Guodong Xu @ 2025-06-20 14:10 UTC (permalink / raw)
To: Yixun Lan
Cc: Vivian Wang, Alex Elder, Ze Huang, spacemit, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Alexandre Ghiti, devicetree, linux-riscv, linux-kernel
On Fri, Jun 20, 2025 at 6:56 PM Yixun Lan <dlan@gentoo.org> wrote:
>
> Hi Vivian, Alex,
>
> On 23:42 Thu 19 Jun , Vivian Wang wrote:
> > Hi Alex,
> >
> > Thank you for your comments on this.
> >
> > On 6/19/25 23:11, Alex Elder wrote:
> > > 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.
> > >
> > I had thought that since the addresses were already there in vendor's DT
> > [2], and the names were provided by SpacemiT, anything other than the
> > names was "well-known information". In retrospect, I should have made
> > the chain of information of this clearer and make it explicit that this
> > was based on Guodong's note.
> >
> > So, just to be clear, the information in my proposal is based on
> > Guodong's reply [1] (link the quoted text), which I had assumed, but not
> > explicitly confirmed, was based on already addresses in SpacemiT's DT
> > and names provided by SpacemiT.
> >
> > [2]: https://github.com/spacemit-com/linux-k1x/blob/k1/arch/riscv/boot/dts/spacemit/k1-x.dtsi
> >
> > >> 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.
> > >
> > Yeah, that's my mistake in organizing the paragraphs.
> >
> > >> 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?
> >
> > Hmm, actually re-reading the discussion now, I realized that I may have
> > come to this late and missed out on some previous discussions, which
> > were alluded to in Yixun's messages. (This is again thread around link
> > [1] in quoted text.) This led me to believe that some of these were not
> > really agreed upon.
> >
> > I also realized I think one of the things I may have not yet made clear
> > is that I would like the bus node to be a *separate* patch. I think this
> > makes sense, because it's dealing with two different subsystems.
> >
> > >
> > >> 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.
> > >
> > Being thorough was the main goal of this RFC. If there was previous
> > agreement on how dma-ranges should be done, I'm sorry for missing them,
> > but from my observations on the mailing list on how these ended up into
> > patches I really haven't seen much consistency. Maybe there was
> > misunderstanding, which I'm hoping to clear up.
> >
> > (Although see my paragraph above, maybe I haven't been thorough enough.)
> >
> > >> 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.
> sounds good to be explicit
>
> > >
> > >> ---
> > >> 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
> I'd agree with not adding unit number to the simple-bus
>
> > > - You dropped the comments he had, which indicated which
> > > devices "belonged" to each mapping
> I went ahead and checked those comments, and found them all about
> devices under specific bus, I'm not strongly against adding the
> comments but feel it's kind of unnecessary, or even in worst cases,
> it may bring extra confusions.. on the other hand, you can always
> check device nodes under the bus to find what's there.
>
> exmaple for dram4_range(vendor code)/dma_bus, the comments is
> /* DMA controller, and users */
> what's is 'users'? still have to check the dts, and find them -
> uart, spi, i2c, qspi, hdmi, sounds..
>
> If people really want to add comments and help others to understand
> this patch, then I'd suggest to add explanation in commit message(better?)
> to fully describe all the busses, or why choose this name? -
> storage/multimedia/pcie/camera/dma/network_bus
> pretty much in much high level perspective..
>
> > > - 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
> I think those all above already exist in Guodong's version which
> align his idea
>
> > >
> > That was a good summary. Thanks!
> >
> > My main goal of organizing the bus this way is making it actually pass
> > "make dtbs_check". I'm not sure if Krzysztof still objects to my reading
> > of simple-bus.yaml though.
> It would be great if DT maintainer can clarify, or give an ACK
>
> >
> > By the way, I don't think I will be making an RFC v2 of this. I think we
> > should get everything sorted under this one thread.
> >
> Instead, from a SoC tree maintainer's perspective (whom taking care of
> merging all the dts files), I'd rather perfer an independent or
> separated patch for this given every party reached consesus, so we could
> get this patch merged first and early, instead of getting them distributed all
> over in different series, IMO, separated patches brings more dedependencies
> if more than two series require one bus and result in more merge conflicts..
> Besides, introducing new busses result in re-arrangement of previous nodes,
> those like uart, i2c (even they have no DMA feature implemented currently)..
>
Hi Yixun,
So, here is my proposed plan: I will submit two patches. The first
patch will introduce the dma-bus node and move the relevant (uart0, uart2
..uart9) device nodes under it. The second patch will then add the pdma0
node itself. Please let me know if you have a different approach in mind.
Maybe you want to see an independent patchset with just the first patch? This
way it can be merged early without waiting for the pdma0 series.
Let me know. Thanks.
On a side note, you mentioned I2C. I searched for upstream I2C DTS nodes
for the K1 but couldn't find any. I checked the for-next/dt-for-next
branches in the spacemit-com/linux.git repository. Did I miss something?
BR,
Guodong
> >
>
> --
> Yixun Lan (dlan)
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH RFC] riscv: dts: spacemit: Add DMA translation buses for K1
2025-06-20 14:10 ` Guodong Xu
@ 2025-06-20 14:57 ` Yixun Lan
2025-06-23 7:01 ` Yixun Lan
0 siblings, 1 reply; 13+ messages in thread
From: Yixun Lan @ 2025-06-20 14:57 UTC (permalink / raw)
To: Guodong Xu
Cc: Vivian Wang, Alex Elder, Ze Huang, spacemit, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Alexandre Ghiti, devicetree, linux-riscv, linux-kernel
Hi Guodong,
On 22:10 Fri 20 Jun , Guodong Xu wrote:
> On Fri, Jun 20, 2025 at 6:56 PM Yixun Lan <dlan@gentoo.org> wrote:
> >
> > Hi Vivian, Alex,
> >
> > On 23:42 Thu 19 Jun , Vivian Wang wrote:
> > > Hi Alex,
> > >
> > > Thank you for your comments on this.
> > >
> > > On 6/19/25 23:11, Alex Elder wrote:
> > > > 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.
> > > >
> > > I had thought that since the addresses were already there in vendor's DT
> > > [2], and the names were provided by SpacemiT, anything other than the
> > > names was "well-known information". In retrospect, I should have made
> > > the chain of information of this clearer and make it explicit that this
> > > was based on Guodong's note.
> > >
> > > So, just to be clear, the information in my proposal is based on
> > > Guodong's reply [1] (link the quoted text), which I had assumed, but not
> > > explicitly confirmed, was based on already addresses in SpacemiT's DT
> > > and names provided by SpacemiT.
> > >
> > > [2]: https://github.com/spacemit-com/linux-k1x/blob/k1/arch/riscv/boot/dts/spacemit/k1-x.dtsi
> > >
> > > >> 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.
> > > >
> > > Yeah, that's my mistake in organizing the paragraphs.
> > >
> > > >> 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?
> > >
> > > Hmm, actually re-reading the discussion now, I realized that I may have
> > > come to this late and missed out on some previous discussions, which
> > > were alluded to in Yixun's messages. (This is again thread around link
> > > [1] in quoted text.) This led me to believe that some of these were not
> > > really agreed upon.
> > >
> > > I also realized I think one of the things I may have not yet made clear
> > > is that I would like the bus node to be a *separate* patch. I think this
> > > makes sense, because it's dealing with two different subsystems.
> > >
> > > >
> > > >> 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.
> > > >
> > > Being thorough was the main goal of this RFC. If there was previous
> > > agreement on how dma-ranges should be done, I'm sorry for missing them,
> > > but from my observations on the mailing list on how these ended up into
> > > patches I really haven't seen much consistency. Maybe there was
> > > misunderstanding, which I'm hoping to clear up.
> > >
> > > (Although see my paragraph above, maybe I haven't been thorough enough.)
> > >
> > > >> 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.
> > sounds good to be explicit
> >
> > > >
> > > >> ---
> > > >> 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
> > I'd agree with not adding unit number to the simple-bus
> >
> > > > - You dropped the comments he had, which indicated which
> > > > devices "belonged" to each mapping
> > I went ahead and checked those comments, and found them all about
> > devices under specific bus, I'm not strongly against adding the
> > comments but feel it's kind of unnecessary, or even in worst cases,
> > it may bring extra confusions.. on the other hand, you can always
> > check device nodes under the bus to find what's there.
> >
> > exmaple for dram4_range(vendor code)/dma_bus, the comments is
> > /* DMA controller, and users */
> > what's is 'users'? still have to check the dts, and find them -
> > uart, spi, i2c, qspi, hdmi, sounds..
> >
> > If people really want to add comments and help others to understand
> > this patch, then I'd suggest to add explanation in commit message(better?)
> > to fully describe all the busses, or why choose this name? -
> > storage/multimedia/pcie/camera/dma/network_bus
> > pretty much in much high level perspective..
> >
> > > > - 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
> > I think those all above already exist in Guodong's version which
> > align his idea
> >
> > > >
> > > That was a good summary. Thanks!
> > >
> > > My main goal of organizing the bus this way is making it actually pass
> > > "make dtbs_check". I'm not sure if Krzysztof still objects to my reading
> > > of simple-bus.yaml though.
> > It would be great if DT maintainer can clarify, or give an ACK
> >
> > >
> > > By the way, I don't think I will be making an RFC v2 of this. I think we
> > > should get everything sorted under this one thread.
> > >
> > Instead, from a SoC tree maintainer's perspective (whom taking care of
> > merging all the dts files), I'd rather perfer an independent or
> > separated patch for this given every party reached consesus, so we could
> > get this patch merged first and early, instead of getting them distributed all
> > over in different series, IMO, separated patches brings more dedependencies
> > if more than two series require one bus and result in more merge conflicts..
> > Besides, introducing new busses result in re-arrangement of previous nodes,
> > those like uart, i2c (even they have no DMA feature implemented currently)..
> >
>
> Hi Yixun,
>
> So, here is my proposed plan: I will submit two patches. The first
> patch will introduce the dma-bus node and move the relevant (uart0, uart2
> ..uart9) device nodes under it. The second patch will then add the pdma0
> node itself. Please let me know if you have a different approach in mind.
>
..
> Maybe you want to see an independent patchset with just the first patch? This
> way it can be merged early without waiting for the pdma0 series.
> Let me know. Thanks.
yes, I prefer this way, this will also help other drivers - usb/emac,
since they all wait for those bus nodes..
please submit following two parts a) introduce bus b) move relevant nodes.
notice, I don't mind who (you or Vivian) doing the job, but keep in
mind don't duplicate the work..
>
> On a side note, you mentioned I2C. I searched for upstream I2C DTS nodes
> for the K1 but couldn't find any. I checked the for-next/dt-for-next
> branches in the spacemit-com/linux.git repository. Did I miss something?
>
you right here, the i2c driver is accepted, but not dts..
btw, the PMIC series do introduce i2c nodes at patch 3/6
> BR,
> Guodong
>
>
> > >
> >
> > --
> > Yixun Lan (dlan)
--
Yixun Lan (dlan)
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH RFC] riscv: dts: spacemit: Add DMA translation buses for K1
2025-06-20 14:57 ` Yixun Lan
@ 2025-06-23 7:01 ` Yixun Lan
2025-06-23 9:32 ` Guodong Xu
0 siblings, 1 reply; 13+ messages in thread
From: Yixun Lan @ 2025-06-23 7:01 UTC (permalink / raw)
To: Guodong Xu
Cc: Vivian Wang, Alex Elder, Ze Huang, spacemit, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Alexandre Ghiti, devicetree, linux-riscv, linux-kernel
Hi Guodong, Vivian
On 14:57 Fri 20 Jun , Yixun Lan wrote:
> Hi Guodong,
>
> On 22:10 Fri 20 Jun , Guodong Xu wrote:
> > On Fri, Jun 20, 2025 at 6:56 PM Yixun Lan <dlan@gentoo.org> wrote:
> > >
> > > Hi Vivian, Alex,
> > >
> > > On 23:42 Thu 19 Jun , Vivian Wang wrote:
> > > > Hi Alex,
> > > >
> > > > Thank you for your comments on this.
> > > >
> > > > On 6/19/25 23:11, Alex Elder wrote:
> > > > > 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.
> > > > >
> > > > I had thought that since the addresses were already there in vendor's DT
> > > > [2], and the names were provided by SpacemiT, anything other than the
> > > > names was "well-known information". In retrospect, I should have made
> > > > the chain of information of this clearer and make it explicit that this
> > > > was based on Guodong's note.
> > > >
> > > > So, just to be clear, the information in my proposal is based on
> > > > Guodong's reply [1] (link the quoted text), which I had assumed, but not
> > > > explicitly confirmed, was based on already addresses in SpacemiT's DT
> > > > and names provided by SpacemiT.
> > > >
> > > > [2]: https://github.com/spacemit-com/linux-k1x/blob/k1/arch/riscv/boot/dts/spacemit/k1-x.dtsi
> > > >
> > > > >> 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.
> > > > >
> > > > Yeah, that's my mistake in organizing the paragraphs.
> > > >
> > > > >> 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?
> > > >
> > > > Hmm, actually re-reading the discussion now, I realized that I may have
> > > > come to this late and missed out on some previous discussions, which
> > > > were alluded to in Yixun's messages. (This is again thread around link
> > > > [1] in quoted text.) This led me to believe that some of these were not
> > > > really agreed upon.
> > > >
> > > > I also realized I think one of the things I may have not yet made clear
> > > > is that I would like the bus node to be a *separate* patch. I think this
> > > > makes sense, because it's dealing with two different subsystems.
> > > >
> > > > >
> > > > >> 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.
> > > > >
> > > > Being thorough was the main goal of this RFC. If there was previous
> > > > agreement on how dma-ranges should be done, I'm sorry for missing them,
> > > > but from my observations on the mailing list on how these ended up into
> > > > patches I really haven't seen much consistency. Maybe there was
> > > > misunderstanding, which I'm hoping to clear up.
> > > >
> > > > (Although see my paragraph above, maybe I haven't been thorough enough.)
> > > >
> > > > >> 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.
> > > sounds good to be explicit
> > >
> > > > >
> > > > >> ---
> > > > >> 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
> > > I'd agree with not adding unit number to the simple-bus
> > >
> > > > > - You dropped the comments he had, which indicated which
> > > > > devices "belonged" to each mapping
> > > I went ahead and checked those comments, and found them all about
> > > devices under specific bus, I'm not strongly against adding the
> > > comments but feel it's kind of unnecessary, or even in worst cases,
> > > it may bring extra confusions.. on the other hand, you can always
> > > check device nodes under the bus to find what's there.
> > >
> > > exmaple for dram4_range(vendor code)/dma_bus, the comments is
> > > /* DMA controller, and users */
> > > what's is 'users'? still have to check the dts, and find them -
> > > uart, spi, i2c, qspi, hdmi, sounds..
> > >
> > > If people really want to add comments and help others to understand
> > > this patch, then I'd suggest to add explanation in commit message(better?)
> > > to fully describe all the busses, or why choose this name? -
> > > storage/multimedia/pcie/camera/dma/network_bus
> > > pretty much in much high level perspective..
> > >
> > > > > - 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
> > > I think those all above already exist in Guodong's version which
> > > align his idea
> > >
> > > > >
> > > > That was a good summary. Thanks!
> > > >
> > > > My main goal of organizing the bus this way is making it actually pass
> > > > "make dtbs_check". I'm not sure if Krzysztof still objects to my reading
> > > > of simple-bus.yaml though.
> > > It would be great if DT maintainer can clarify, or give an ACK
> > >
> > > >
> > > > By the way, I don't think I will be making an RFC v2 of this. I think we
> > > > should get everything sorted under this one thread.
> > > >
> > > Instead, from a SoC tree maintainer's perspective (whom taking care of
> > > merging all the dts files), I'd rather perfer an independent or
> > > separated patch for this given every party reached consesus, so we could
> > > get this patch merged first and early, instead of getting them distributed all
> > > over in different series, IMO, separated patches brings more dedependencies
> > > if more than two series require one bus and result in more merge conflicts..
> > > Besides, introducing new busses result in re-arrangement of previous nodes,
> > > those like uart, i2c (even they have no DMA feature implemented currently)..
> > >
> >
> > Hi Yixun,
> >
> > So, here is my proposed plan: I will submit two patches. The first
> > patch will introduce the dma-bus node and move the relevant (uart0, uart2
> > ..uart9) device nodes under it. The second patch will then add the pdma0
> > node itself. Please let me know if you have a different approach in mind.
> >
> ..
> > Maybe you want to see an independent patchset with just the first patch? This
> > way it can be merged early without waiting for the pdma0 series.
> > Let me know. Thanks.
> yes, I prefer this way, this will also help other drivers - usb/emac,
> since they all wait for those bus nodes..
>
> please submit following two parts a) introduce bus b) move relevant nodes.
> notice, I don't mind who (you or Vivian) doing the job, but keep in
> mind don't duplicate the work..
>
to make it clear, I'd like to see all relevant *bus nodes added in one
independent series, not only dma-bus, even some nodes currently not used.
the goal here is "do it once, and do it well"
in fact, I'd expect Vivian(or Guodong, whoever) to send a new version
of this patch without RFC prefix..
> >
> > On a side note, you mentioned I2C. I searched for upstream I2C DTS nodes
> > for the K1 but couldn't find any. I checked the for-next/dt-for-next
> > branches in the spacemit-com/linux.git repository. Did I miss something?
> >
> you right here, the i2c driver is accepted, but not dts..
> btw, the PMIC series do introduce i2c nodes at patch 3/6
>
> > BR,
> > Guodong
> >
> >
> > > >
> > >
> > > --
> > > Yixun Lan (dlan)
>
> --
> Yixun Lan (dlan)
--
Yixun Lan (dlan)
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH RFC] riscv: dts: spacemit: Add DMA translation buses for K1
2025-06-23 7:01 ` Yixun Lan
@ 2025-06-23 9:32 ` Guodong Xu
2025-06-23 9:42 ` Vivian Wang
0 siblings, 1 reply; 13+ messages in thread
From: Guodong Xu @ 2025-06-23 9:32 UTC (permalink / raw)
To: Yixun Lan
Cc: Vivian Wang, Alex Elder, Ze Huang, spacemit, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Alexandre Ghiti, devicetree, linux-riscv, linux-kernel
On Mon, Jun 23, 2025 at 3:01 PM Yixun Lan <dlan@gentoo.org> wrote:
>
> Hi Guodong, Vivian
>
> On 14:57 Fri 20 Jun , Yixun Lan wrote:
> > Hi Guodong,
> >
> > On 22:10 Fri 20 Jun , Guodong Xu wrote:
> > > On Fri, Jun 20, 2025 at 6:56 PM Yixun Lan <dlan@gentoo.org> wrote:
> > > >
> > > > Hi Vivian, Alex,
> > > >
> > > > On 23:42 Thu 19 Jun , Vivian Wang wrote:
> > > > > Hi Alex,
> > > > >
> > > > > Thank you for your comments on this.
> > > > >
> > > > > On 6/19/25 23:11, Alex Elder wrote:
> > > > > > 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.
> > > > > >
> > > > > I had thought that since the addresses were already there in vendor's DT
> > > > > [2], and the names were provided by SpacemiT, anything other than the
> > > > > names was "well-known information". In retrospect, I should have made
> > > > > the chain of information of this clearer and make it explicit that this
> > > > > was based on Guodong's note.
> > > > >
> > > > > So, just to be clear, the information in my proposal is based on
> > > > > Guodong's reply [1] (link the quoted text), which I had assumed, but not
> > > > > explicitly confirmed, was based on already addresses in SpacemiT's DT
> > > > > and names provided by SpacemiT.
> > > > >
> > > > > [2]: https://github.com/spacemit-com/linux-k1x/blob/k1/arch/riscv/boot/dts/spacemit/k1-x.dtsi
> > > > >
> > > > > >> 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.
> > > > > >
> > > > > Yeah, that's my mistake in organizing the paragraphs.
> > > > >
> > > > > >> 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?
> > > > >
> > > > > Hmm, actually re-reading the discussion now, I realized that I may have
> > > > > come to this late and missed out on some previous discussions, which
> > > > > were alluded to in Yixun's messages. (This is again thread around link
> > > > > [1] in quoted text.) This led me to believe that some of these were not
> > > > > really agreed upon.
> > > > >
> > > > > I also realized I think one of the things I may have not yet made clear
> > > > > is that I would like the bus node to be a *separate* patch. I think this
> > > > > makes sense, because it's dealing with two different subsystems.
> > > > >
> > > > > >
> > > > > >> 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.
> > > > > >
> > > > > Being thorough was the main goal of this RFC. If there was previous
> > > > > agreement on how dma-ranges should be done, I'm sorry for missing them,
> > > > > but from my observations on the mailing list on how these ended up into
> > > > > patches I really haven't seen much consistency. Maybe there was
> > > > > misunderstanding, which I'm hoping to clear up.
> > > > >
> > > > > (Although see my paragraph above, maybe I haven't been thorough enough.)
> > > > >
> > > > > >> 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.
> > > > sounds good to be explicit
> > > >
> > > > > >
> > > > > >> ---
> > > > > >> 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
> > > > I'd agree with not adding unit number to the simple-bus
> > > >
> > > > > > - You dropped the comments he had, which indicated which
> > > > > > devices "belonged" to each mapping
> > > > I went ahead and checked those comments, and found them all about
> > > > devices under specific bus, I'm not strongly against adding the
> > > > comments but feel it's kind of unnecessary, or even in worst cases,
> > > > it may bring extra confusions.. on the other hand, you can always
> > > > check device nodes under the bus to find what's there.
> > > >
> > > > exmaple for dram4_range(vendor code)/dma_bus, the comments is
> > > > /* DMA controller, and users */
> > > > what's is 'users'? still have to check the dts, and find them -
> > > > uart, spi, i2c, qspi, hdmi, sounds..
> > > >
> > > > If people really want to add comments and help others to understand
> > > > this patch, then I'd suggest to add explanation in commit message(better?)
> > > > to fully describe all the busses, or why choose this name? -
> > > > storage/multimedia/pcie/camera/dma/network_bus
> > > > pretty much in much high level perspective..
> > > >
> > > > > > - 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
> > > > I think those all above already exist in Guodong's version which
> > > > align his idea
> > > >
> > > > > >
> > > > > That was a good summary. Thanks!
> > > > >
> > > > > My main goal of organizing the bus this way is making it actually pass
> > > > > "make dtbs_check". I'm not sure if Krzysztof still objects to my reading
> > > > > of simple-bus.yaml though.
> > > > It would be great if DT maintainer can clarify, or give an ACK
> > > >
> > > > >
> > > > > By the way, I don't think I will be making an RFC v2 of this. I think we
> > > > > should get everything sorted under this one thread.
> > > > >
> > > > Instead, from a SoC tree maintainer's perspective (whom taking care of
> > > > merging all the dts files), I'd rather perfer an independent or
> > > > separated patch for this given every party reached consesus, so we could
> > > > get this patch merged first and early, instead of getting them distributed all
> > > > over in different series, IMO, separated patches brings more dedependencies
> > > > if more than two series require one bus and result in more merge conflicts..
> > > > Besides, introducing new busses result in re-arrangement of previous nodes,
> > > > those like uart, i2c (even they have no DMA feature implemented currently)..
> > > >
> > >
> > > Hi Yixun,
> > >
> > > So, here is my proposed plan: I will submit two patches. The first
> > > patch will introduce the dma-bus node and move the relevant (uart0, uart2
> > > ..uart9) device nodes under it. The second patch will then add the pdma0
> > > node itself. Please let me know if you have a different approach in mind.
> > >
> > ..
> > > Maybe you want to see an independent patchset with just the first patch? This
> > > way it can be merged early without waiting for the pdma0 series.
> > > Let me know. Thanks.
> > yes, I prefer this way, this will also help other drivers - usb/emac,
> > since they all wait for those bus nodes..
> >
> > please submit following two parts a) introduce bus b) move relevant nodes.
> > notice, I don't mind who (you or Vivian) doing the job, but keep in
> > mind don't duplicate the work..
> >
> to make it clear, I'd like to see all relevant *bus nodes added in one
> independent series, not only dma-bus, even some nodes currently not used.
> the goal here is "do it once, and do it well"
>
> in fact, I'd expect Vivian(or Guodong, whoever) to send a new version
> of this patch without RFC prefix..
>
I'm ok if Vivian can do that.
Thanks.
> > >
> > > On a side note, you mentioned I2C. I searched for upstream I2C DTS nodes
> > > for the K1 but couldn't find any. I checked the for-next/dt-for-next
> > > branches in the spacemit-com/linux.git repository. Did I miss something?
> > >
> > you right here, the i2c driver is accepted, but not dts..
> > btw, the PMIC series do introduce i2c nodes at patch 3/6
> >
> > > BR,
> > > Guodong
> > >
> > >
> > > > >
> > > >
> > > > --
> > > > Yixun Lan (dlan)
> >
> > --
> > Yixun Lan (dlan)
>
> --
> Yixun Lan (dlan)
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH RFC] riscv: dts: spacemit: Add DMA translation buses for K1
2025-06-23 9:32 ` Guodong Xu
@ 2025-06-23 9:42 ` Vivian Wang
0 siblings, 0 replies; 13+ messages in thread
From: Vivian Wang @ 2025-06-23 9:42 UTC (permalink / raw)
To: Guodong Xu, Yixun Lan
Cc: Alex Elder, Ze Huang, spacemit, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti, devicetree, linux-riscv, linux-kernel
Hi Guodong and Yixun,
>>>>> <snip>
>>>>>
>>>>>> By the way, I don't think I will be making an RFC v2 of this. I think we
>>>>>> should get everything sorted under this one thread.
>>>>>>
>>>>> Instead, from a SoC tree maintainer's perspective (whom taking care of
>>>>> merging all the dts files), I'd rather perfer an independent or
>>>>> separated patch for this given every party reached consesus, so we could
>>>>> get this patch merged first and early, instead of getting them distributed all
>>>>> over in different series, IMO, separated patches brings more dedependencies
>>>>> if more than two series require one bus and result in more merge conflicts..
>>>>> Besides, introducing new busses result in re-arrangement of previous nodes,
>>>>> those like uart, i2c (even they have no DMA feature implemented currently)..
>>>>>
>>>> Hi Yixun,
>>>>
>>>> So, here is my proposed plan: I will submit two patches. The first
>>>> patch will introduce the dma-bus node and move the relevant (uart0, uart2
>>>> ..uart9) device nodes under it. The second patch will then add the pdma0
>>>> node itself. Please let me know if you have a different approach in mind.
>>>>
>>> ..
>>>> Maybe you want to see an independent patchset with just the first patch? This
>>>> way it can be merged early without waiting for the pdma0 series.
>>>> Let me know. Thanks.
>>> yes, I prefer this way, this will also help other drivers - usb/emac,
>>> since they all wait for those bus nodes..
>>>
>>> please submit following two parts a) introduce bus b) move relevant nodes.
>>> notice, I don't mind who (you or Vivian) doing the job, but keep in
>>> mind don't duplicate the work..
>>>
>> to make it clear, I'd like to see all relevant *bus nodes added in one
>> independent series, not only dma-bus, even some nodes currently not used.
>> the goal here is "do it once, and do it well"
>>
>> in fact, I'd expect Vivian(or Guodong, whoever) to send a new version
>> of this patch without RFC prefix..
>>
> I'm ok if Vivian can do that.
> Thanks.
I will post v1 of this series soon.
Thanks,
Vivian "dramforever" Wang
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2025-06-23 9:42 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).