| Message ID | 20240315211533.1996543-2-tanmay.shah@amd.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add Versal and Versal-NET platform support | expand |
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.
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.
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
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
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 >
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
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"; + }; + }; + }; ...
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(+)