[PATCH 1/3] dt-bindings: remoteproc: add Versal platform support

Tanmay Shah posted 3 patches 1 year, 11 months ago
[PATCH 1/3] dt-bindings: remoteproc: add Versal platform support
Posted by Tanmay Shah 1 year, 11 months ago
AMD-Xilinx Versal platform is successor of ZynqMP platform. Real-time
Processor Unit R5 cluster IP on Versal is same as of ZynqMP Platform.
Only difference is power-domains ID needed by power management firmware.
Hence, keeping the compatible property same as of zynqmp node.

Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>
---
 .../remoteproc/xlnx,zynqmp-r5fss.yaml         | 93 +++++++++++++++++++
 1 file changed, 93 insertions(+)

diff --git a/Documentation/devicetree/bindings/remoteproc/xlnx,zynqmp-r5fss.yaml b/Documentation/devicetree/bindings/remoteproc/xlnx,zynqmp-r5fss.yaml
index 629084a84ce6..711da0272250 100644
--- a/Documentation/devicetree/bindings/remoteproc/xlnx,zynqmp-r5fss.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/xlnx,zynqmp-r5fss.yaml
@@ -293,4 +293,97 @@ examples:
             };
         };
     };
+
+  - |
+    // Versal Split mode configuration
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        remoteproc@ffe00000 {
+            compatible = "xlnx,zynqmp-r5fss";
+            xlnx,cluster-mode = <0>;
+
+            #address-cells = <2>;
+            #size-cells = <2>;
+            ranges = <0x0 0x0 0x0 0xffe00000 0x0 0x10000>,
+                     <0x0 0x20000 0x0 0xffe20000 0x0 0x10000>,
+                     <0x1 0x0 0x0 0xffe90000 0x0 0x10000>,
+                     <0x1 0x20000 0x0 0xffeb0000 0x0 0x10000>;
+
+            r5f@0 {
+                compatible = "xlnx,zynqmp-r5f";
+                reg = <0x0 0x0 0x0 0x10000>, <0x0 0x20000 0x0 0x10000>;
+                reg-names = "atcm0", "btcm0";
+                power-domains = <&versal_firmware 0x18110005>,
+                                <&versal_firmware 0x1831800b>,
+                                <&versal_firmware 0x1831800c>;
+                memory-region = <&rproc_0_fw_image>, <&rpu0vdev0buffer>,
+                                <&rpu0vdev0vring0>, <&rpu0vdev0vring1>;
+                mboxes = <&ipi_mailbox_rpu0 0>, <&ipi_mailbox_rpu0 1>;
+                mbox-names = "tx", "rx";
+            };
+
+            r5f@1 {
+                compatible = "xlnx,zynqmp-r5f";
+                reg = <0x1 0x0 0x0 0x10000>, <0x1 0x20000 0x0 0x10000>;
+                reg-names = "atcm0", "btcm0";
+                power-domains = <&versal_firmware 0x18110006>,
+                                <&versal_firmware 0x1831800d>,
+                                <&versal_firmware 0x1831800e>;
+                memory-region = <&rproc_1_fw_image>, <&rpu1vdev0buffer>,
+                                <&rpu1vdev0vring0>, <&rpu1vdev0vring1>;
+                mboxes = <&ipi_mailbox_rpu1 0>, <&ipi_mailbox_rpu1 1>;
+                mbox-names = "tx", "rx";
+            };
+        };
+    };
+
+  - |
+    // Versal Lockstep configuration
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        remoteproc@ffe00000 {
+            compatible = "xlnx,zynqmp-r5fss";
+            xlnx,cluster-mode = <1>;
+
+            #address-cells = <2>;
+            #size-cells = <2>;
+            ranges = <0x0 0x0 0x0 0xffe00000 0x0 0x20000>,
+                     <0x0 0x20000 0x0 0xffe20000 0x0 0x20000>;
+
+            r5f@0 {
+                compatible = "xlnx,zynqmp-r5f";
+                reg = <0x0 0x0 0x0 0x10000>,
+                      <0x0 0x20000 0x0 0x10000>,
+                      <0x0 0x10000 0x0 0x10000>,
+                      <0x0 0x30000 0x0 0x10000>;
+                reg-names = "atcm0", "btcm0", "atcm1", "btcm1";
+                power-domains = <&versal_firmware 0x18110005>,
+                                <&versal_firmware 0x1831800b>,
+                                <&versal_firmware 0x1831800c>,
+                                <&versal_firmware 0x1831800d>,
+                                <&versal_firmware 0x1831800e>;
+                memory-region = <&rproc_0_fw_image>, <&rpu0vdev0buffer>,
+                                <&rpu0vdev0vring0>, <&rpu0vdev0vring1>;
+                mboxes = <&ipi_mailbox_rpu0 0>, <&ipi_mailbox_rpu0 1>;
+                mbox-names = "tx", "rx";
+            };
+
+            r5f@1 {
+                compatible = "xlnx,zynqmp-r5f";
+                reg = <0x1 0x0 0x0 0x10000>, <0x1 0x20000 0x0 0x10000>;
+                reg-names = "atcm0", "btcm0";
+                power-domains = <&versal_firmware 0x18110006>,
+                                <&versal_firmware 0x1831800d>,
+                                <&versal_firmware 0x1831800e>;
+                memory-region = <&rproc_1_fw_image>, <&rpu1vdev0buffer>,
+                                <&rpu1vdev0vring0>, <&rpu1vdev0vring1>;
+                mboxes = <&ipi_mailbox_rpu1 0>, <&ipi_mailbox_rpu1 1>;
+                mbox-names = "tx", "rx";
+            };
+        };
+    };
 ...
-- 
2.25.1
Re: [PATCH 1/3] dt-bindings: remoteproc: add Versal platform support
Posted by Krzysztof Kozlowski 1 year, 10 months ago
On 15/03/2024 22:15, Tanmay Shah wrote:
> AMD-Xilinx Versal platform is successor of ZynqMP platform. Real-time
> Processor Unit R5 cluster IP on Versal is same as of ZynqMP Platform.
> Only difference is power-domains ID needed by power management firmware.
> Hence, keeping the compatible property same as of zynqmp node.
> 
> Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>

There is no binding change, so NAK. Platform is not being added to
examples. You changed nothing in  Versal support...

Best regards,
Krzysztof
Re: [PATCH 1/3] dt-bindings: remoteproc: add Versal platform support
Posted by Tanmay Shah 1 year, 10 months ago

On 3/17/24 1:50 PM, Krzysztof Kozlowski wrote:
> On 15/03/2024 22:15, Tanmay Shah wrote:
>> AMD-Xilinx Versal platform is successor of ZynqMP platform. Real-time
>> Processor Unit R5 cluster IP on Versal is same as of ZynqMP Platform.
>> Only difference is power-domains ID needed by power management firmware.
>> Hence, keeping the compatible property same as of zynqmp node.
>> 
>> Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>
> 
> There is no binding change, so NAK. Platform is not being added to
> examples. You changed nothing in  Versal support...

Thanks for reviews.

Okay got it. That means I don't really need to add anything related to Versal.
I will get rid of patch that says "Versal support". Looks like it's not needed
at all.

Thanks.

> 
> Best regards,
> Krzysztof
>
Re: [PATCH 1/3] dt-bindings: remoteproc: add Versal platform support
Posted by Conor Dooley 1 year, 10 months ago
On Fri, Mar 15, 2024 at 02:15:31PM -0700, Tanmay Shah wrote:
> AMD-Xilinx Versal platform is successor of ZynqMP platform. Real-time
> Processor Unit R5 cluster IP on Versal is same as of ZynqMP Platform.

> Only difference is power-domains ID needed by power management firmware.
> Hence, keeping the compatible property same as of zynqmp node.

No, don't be lazy. Add a compatible with a fallback please.
This doesn't apply to linux-next either FYI.
Re: [PATCH 1/3] dt-bindings: remoteproc: add Versal platform support
Posted by Tanmay Shah 1 year, 10 months ago
Hello,

Thanks for reviews, please find my comments below.

On 3/17/24 9:50 AM, Conor Dooley wrote:
> On Fri, Mar 15, 2024 at 02:15:31PM -0700, Tanmay Shah wrote:
>> AMD-Xilinx Versal platform is successor of ZynqMP platform. Real-time
>> Processor Unit R5 cluster IP on Versal is same as of ZynqMP Platform.
> 
>> Only difference is power-domains ID needed by power management firmware.
>> Hence, keeping the compatible property same as of zynqmp node.
> 
> No, don't be lazy. Add a compatible with a fallback please.

It's same IP on different platform. I am not sure how adding compatible string
adds value. I will refactor this series based on other comments provided.

> This doesn't apply to linux-next either FYI.

Actually cover-letter contains dependent series link that is needed for this
patch. That series was acked recently but hasn't made to linux-next yet.

I may be missing something in the process. In such case there is no other way
to send patch except for waiting?

Thanks,
Tanmay
Re: [PATCH 1/3] dt-bindings: remoteproc: add Versal platform support
Posted by Krzysztof Kozlowski 1 year, 10 months ago
On 19/03/2024 01:37, Tanmay Shah wrote:
> Hello,
> 
> Thanks for reviews, please find my comments below.
> 
> On 3/17/24 9:50 AM, Conor Dooley wrote:
>> On Fri, Mar 15, 2024 at 02:15:31PM -0700, Tanmay Shah wrote:
>>> AMD-Xilinx Versal platform is successor of ZynqMP platform. Real-time
>>> Processor Unit R5 cluster IP on Versal is same as of ZynqMP Platform.
>>
>>> Only difference is power-domains ID needed by power management firmware.
>>> Hence, keeping the compatible property same as of zynqmp node.
>>
>> No, don't be lazy. Add a compatible with a fallback please.
> 
> It's same IP on different platform. I am not sure how adding compatible string
> adds value. I will refactor this series based on other comments provided.

Judging by your other thread, it would add value. Also writing bindings
asks you for this.

Best regards,
Krzysztof
Re: [PATCH 1/3] dt-bindings: remoteproc: add Versal platform support
Posted by Conor Dooley 1 year, 10 months ago
On Sun, Mar 17, 2024 at 02:50:27PM +0000, Conor Dooley wrote:
> On Fri, Mar 15, 2024 at 02:15:31PM -0700, Tanmay Shah wrote:
> > AMD-Xilinx Versal platform is successor of ZynqMP platform. Real-time
> > Processor Unit R5 cluster IP on Versal is same as of ZynqMP Platform.
> 
> > Only difference is power-domains ID needed by power management firmware.
> > Hence, keeping the compatible property same as of zynqmp node.
> 
> No, don't be lazy. Add a compatible with a fallback please.

> This doesn't apply to linux-next either FYI.

Ordinarily'd I'd not even bother applying a patch like this and I
wouldn't notice, but the diff for the versal-net patch is difficult to
read without colour-moved enabled and since I can't apply this I'm not
going to review that patch.