Re: [PATCH 1/3] dt-bindings: remoteproc: add Versal platform support
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
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 > > 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
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
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 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
On Sun, Mar 17, 2024 at 02:50:27PM +, 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. signature.asc Description: PGP signature
Re: [PATCH 1/3] dt-bindings: remoteproc: add Versal platform support
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. signature.asc Description: PGP signature
