On 3/19/24 12:30 AM, Krzysztof Kozlowski wrote: > On 19/03/2024 01:51, Tanmay Shah wrote: >> Hello Krzysztof, >> >> Thanks for reviews. Please find my comments below. >> >> On 3/17/24 1:53 PM, Krzysztof Kozlowski wrote: >>> On 15/03/2024 22:15, Tanmay Shah wrote: >>>> AMD-Xilinx Versal-NET platform is successor of Versal platform. It >>>> contains multiple clusters of cortex-R52 real-time processing units. >>>> Each cluster contains two cores of cortex-R52 processors. Each cluster >>>> can be configured in lockstep mode or split mode. >>>> >>>> Each R52 core is assigned 128KB of TCM memory. ATCM memory is 64KB, BTCM >>>> and CTCM memoreis are 32KB each. Each TCM memory has its own dedicated >>>> power-domain that needs to be requested before using it. >>>> >>>> Signed-off-by: Tanmay Shah <tanmay.shah@xxxxxxx> >>>> --- >>>> .../remoteproc/xlnx,zynqmp-r5fss.yaml | 220 +++++++++++++++--- >>>> 1 file changed, 184 insertions(+), 36 deletions(-) >>>> >>>> diff --git a/Documentation/devicetree/bindings/remoteproc/xlnx,zynqmp-r5fss.yaml b/Documentation/devicetree/bindings/remoteproc/xlnx,zynqmp-r5fss.yaml >>>> index 711da0272250..55654ee02eef 100644 >>>> --- a/Documentation/devicetree/bindings/remoteproc/xlnx,zynqmp-r5fss.yaml >>>> +++ b/Documentation/devicetree/bindings/remoteproc/xlnx,zynqmp-r5fss.yaml >>>> @@ -18,7 +18,9 @@ description: | >>>> >>>> properties: >>>> compatible: >>>> - const: xlnx,zynqmp-r5fss >>>> + enum: >>>> + - xlnx,zynqmp-r5fss >>>> + - xlnx,versal-net-r52fss >>>> >>>> "#address-cells": >>>> const: 2 >>>> @@ -64,7 +66,9 @@ patternProperties: >>>> >>>> properties: >>>> compatible: >>>> - const: xlnx,zynqmp-r5f >>>> + enum: >>>> + - xlnx,zynqmp-r5f >>>> + - xlnx,versal-net-r52f >>>> >>>> reg: >>>> minItems: 1 >>>> @@ -135,9 +139,11 @@ required: >>>> allOf: >>>> - if: >>>> properties: >>>> - xlnx,cluster-mode: >>>> - enum: >>>> - - 1 >>>> + compatible: >>>> + contains: >>>> + enum: >>>> + - xlnx,versal-net-r52fss >>> >>> Why do you touch these lines? >>> >>>> + >>>> then: >>>> patternProperties: >>>> "^r5f@[0-9a-f]+$": >>>> @@ -149,16 +155,14 @@ allOf: >>>> items: >>>> - description: ATCM internal memory >>>> - description: BTCM internal memory >>>> - - description: extra ATCM memory in lockstep mode >>>> - - description: extra BTCM memory in lockstep mode >>>> + - description: CTCM internal memory >>>> >>>> reg-names: >>>> minItems: 1 >>>> items: >>>> - - const: atcm0 >>>> - - const: btcm0 >>>> - - const: atcm1 >>>> - - const: btcm1 >>>> + - const: atcm >>>> + - const: btcm >>>> + - const: ctcm >>>> >>>> power-domains: >>>> minItems: 2 >>>> @@ -166,33 +170,70 @@ allOf: >>>> - description: RPU core power domain >>>> - description: ATCM power domain >>>> - description: BTCM power domain >>>> - - description: second ATCM power domain >>>> - - description: second BTCM power domain >>>> + - description: CTCM power domain >>>> >>>> else: >>>> - patternProperties: >>>> - "^r5f@[0-9a-f]+$": >>>> - type: object >>>> - >>>> - properties: >>>> - reg: >>>> - minItems: 1 >>>> - items: >>>> - - description: ATCM internal memory >>>> - - description: BTCM internal memory >>>> - >>>> - reg-names: >>>> - minItems: 1 >>>> - items: >>>> - - const: atcm0 >>>> - - const: btcm0 >>>> - >>>> - power-domains: >>>> - minItems: 2 >>>> - items: >>>> - - description: RPU core power domain >>>> - - description: ATCM power domain >>>> - - description: BTCM power domain >>>> + allOf: >>>> + - if: >>>> + properties: >>>> + xlnx,cluster-mode: >>>> + enum: >>>> + - 1 >>> >>> Whatever you did here, is not really readable. You have now multiple >>> if:then:if:then embedded. >> >> For ZynqMP platform, TCM can be configured differently in lockstep mode >> and split mode. >> >> For Versal-NET no such configuration is available, but new CTCM memory >> is added. >> >> So, I am trying to achieve following representation of TCM for both: >> >> if: versal-net compatible >> then: >> ATCM - 64KB >> BTCM - 32KB >> CTCM - 32KB >> >> else: (ZynqMP compatible) >> if: >> xlnx,cluster-mode (lockstep mode) >> then: >> ATCM0 - 64KB >> BTCM0 - 64KB >> ATCM1 - 64KB >> BTCM1 - 64KB >> else: (split mode) >> ATCM0 - 64KB >> BTCM0 - 64KB >> >> >> If bindings are getting complicated, does it make sense to introduce >> new file for Versal-NET bindings? Let me know how you would like me >> to proceed. > > All this is broken in your previous patchset, but now we nicely see. > > No, this does not work like this. You do not have entirely different > programming models in one device, don't you? > I don't understand what do you mean? Programming model is same. Only number of TCMs are changing based on configuration and platform. I can certainly list different compatible for different platforms as requested. But other than that not sure what needs to be fixed. > > Best regards, > Krzysztof >
