| Message ID | 20240315211533.1996543-3-tanmay.shah@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add Versal and Versal-NET platform support | expand |
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@amd.com> > --- > .../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. > + then: > + patternProperties: > + "^r5f@[0-9a-f]+$": > + type: object > + > + properties: > + reg: > + minItems: 1 > + items: > + - description: ATCM internal memory > + - description: BTCM internal memory > + - description: extra ATCM memory in lockstep mode > + - description: extra BTCM memory in lockstep mode > + > + reg-names: > + minItems: 1 > + items: > + - const: atcm0 > + - const: btcm0 > + - const: atcm1 > + - const: btcm1 > + > + power-domains: > + minItems: 2 > + items: > + - description: RPU core power domain > + - description: ATCM power domain > + - description: BTCM power domain > + - description: second ATCM power domain > + - description: second BTCM 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 > > additionalProperties: false > > @@ -386,4 +427,111 @@ examples: > }; > }; > }; > + > + - | > + // Versal-NET split configuration > + soc { > + #address-cells = <2>; > + #size-cells = <2>; > + Don't add examples per each platform. Best regards, Krzysztof
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@amd.com> >> --- >> .../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. Thanks, Tanmay > >> + then: >> + patternProperties: >> + "^r5f@[0-9a-f]+$": >> + type: object >> + >> + properties: >> + reg: >> + minItems: 1 >> + items: >> + - description: ATCM internal memory >> + - description: BTCM internal memory >> + - description: extra ATCM memory in lockstep mode >> + - description: extra BTCM memory in lockstep mode >> + >> + reg-names: >> + minItems: 1 >> + items: >> + - const: atcm0 >> + - const: btcm0 >> + - const: atcm1 >> + - const: btcm1 >> + >> + power-domains: >> + minItems: 2 >> + items: >> + - description: RPU core power domain >> + - description: ATCM power domain >> + - description: BTCM power domain >> + - description: second ATCM power domain >> + - description: second BTCM 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 >> >> additionalProperties: false >> >> @@ -386,4 +427,111 @@ examples: >> }; >> }; >> }; >> + >> + - | >> + // Versal-NET split configuration >> + soc { >> + #address-cells = <2>; >> + #size-cells = <2>; >> + > > Don't add examples per each platform. Noted. If we go with different file for Versal-NET, then example should be included? > > Best regards, > Krzysztof >
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@amd.com> >>> --- >>> .../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? Best regards, Krzysztof
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@amd.com> >>>> --- >>>> .../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 >
On 19/03/2024 15:42, Tanmay Shah wrote: > > > 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@amd.com> >>>>> --- >>>>> .../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. You cannot have same programming model with different memory mappings. Anyway, please follow writing bindings rules: all of your different devices must have dedicated compatible. I really though we talked about two IPs on same SoC... Best regards, Krzysztof
On 3/20/24 2:40 AM, Krzysztof Kozlowski wrote: > On 19/03/2024 15:42, Tanmay Shah wrote: >> >> >> 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@amd.com> >>>>>> --- >>>>>> .../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. > > You cannot have same programming model with different memory mappings. > Anyway, please follow writing bindings rules: all of your different > devices must have dedicated compatible. I really though we talked about > two IPs on same SoC... I agree that Versal compatible should be added, I will do that in next revision. For ZynqMP case, it is two IPs on same SOC. In lockstep mode and split mode, same SOC is configuring TCM differently. How this should be resolved for Versal-NET ? Driver avoids such TCM configuration for Versal-NET. If you would like to see v14, before further discussion please let me know. Thanks, Tanmay > > > Best regards, > Krzysztof >
On 20/03/2024 16:14, Tanmay Shah wrote: > > > On 3/20/24 2:40 AM, Krzysztof Kozlowski wrote: >> On 19/03/2024 15:42, Tanmay Shah wrote: >>> >>> >>> 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@amd.com> >>>>>>> --- >>>>>>> .../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. >> >> You cannot have same programming model with different memory mappings. >> Anyway, please follow writing bindings rules: all of your different >> devices must have dedicated compatible. I really though we talked about >> two IPs on same SoC... > > I agree that Versal compatible should be added, I will do that in next revision. > > For ZynqMP case, it is two IPs on same SOC. In lockstep mode and split mode, > same SOC is configuring TCM differently. > > How this should be resolved for Versal-NET ? Driver avoids such TCM configuration > for Versal-NET. Binding should describe the hardware, not what driver is doing currently, so the question is: does your device have such properties or not? Anyway, you need compatible per each variant and each SoC implementation. Best regards, Krzysztof
On 3/21/24 2:39 AM, Krzysztof Kozlowski wrote: > On 20/03/2024 16:14, Tanmay Shah wrote: >> >> >> On 3/20/24 2:40 AM, Krzysztof Kozlowski wrote: >>> On 19/03/2024 15:42, Tanmay Shah wrote: >>>> >>>> >>>> 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@amd.com> >>>>>>>> --- >>>>>>>> .../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. >>> >>> You cannot have same programming model with different memory mappings. >>> Anyway, please follow writing bindings rules: all of your different >>> devices must have dedicated compatible. I really though we talked about >>> two IPs on same SoC... >> >> I agree that Versal compatible should be added, I will do that in next revision. >> >> For ZynqMP case, it is two IPs on same SOC. In lockstep mode and split mode, >> same SOC is configuring TCM differently. >> >> How this should be resolved for Versal-NET ? Driver avoids such TCM configuration >> for Versal-NET. > > Binding should describe the hardware, not what driver is doing > currently, so the question is: does your device have such properties or > not? Anyway, you need compatible per each variant and each SoC > implementation. Thanks for reviews. Okay in that case I believe I should add one more property to current bindings for TCM configuration. From our discussion I conclude to following next steps: 1) I will send Versal and Versal-NET support as part of previous series (v14) so we get bigger picture in the first place. 2) Add separate compatible for versal platform. Use device compatible string to maintain backward compatibility and not machine (root node) compatible string. 3) Add tcm,mode property in bindings and each device must configure TCM based on that property only and not based on compatible string. 4) Versal-NET will disallow tcm,mode property in bindings as no such configuration is possible for that platform. I hope I got everything. I will test and send v14 of previous series accordingly. Tanmay > > Best regards, > Krzysztof >
On 21/03/2024 16:13, Tanmay Shah wrote: > > > On 3/21/24 2:39 AM, Krzysztof Kozlowski wrote: >> On 20/03/2024 16:14, Tanmay Shah wrote: >>> >>> >>> On 3/20/24 2:40 AM, Krzysztof Kozlowski wrote: >>>> On 19/03/2024 15:42, Tanmay Shah wrote: >>>>> >>>>> >>>>> 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@amd.com> >>>>>>>>> --- >>>>>>>>> .../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. >>>> >>>> You cannot have same programming model with different memory mappings. >>>> Anyway, please follow writing bindings rules: all of your different >>>> devices must have dedicated compatible. I really though we talked about >>>> two IPs on same SoC... >>> >>> I agree that Versal compatible should be added, I will do that in next revision. >>> >>> For ZynqMP case, it is two IPs on same SOC. In lockstep mode and split mode, >>> same SOC is configuring TCM differently. >>> >>> How this should be resolved for Versal-NET ? Driver avoids such TCM configuration >>> for Versal-NET. >> >> Binding should describe the hardware, not what driver is doing >> currently, so the question is: does your device have such properties or >> not? Anyway, you need compatible per each variant and each SoC >> implementation. > > Thanks for reviews. > > Okay in that case I believe I should add one more property to current bindings for TCM > configuration. > I am not sure if you understand how IRC works... You sent me message on IRC about this topic and shortly after you quit. So how am I supposed to send reply? IRC does not work like that... > From our discussion I conclude to following next steps: > > 1) I will send Versal and Versal-NET support as part of previous series (v14) so we get > bigger picture in the first place. > > 2) Add separate compatible for versal platform. > Use device compatible string to maintain > backward compatibility and not machine (root node) compatible string. > > 3) Add tcm,mode property in bindings and each device must configure TCM based on that > property only and not based on compatible string. > > 4) Versal-NET will disallow tcm,mode property in bindings as no such configuration is > possible for that platform. I really don't know your SoCs. What about Zynq? You keep using here names all over the place, but I am not Xilinx maintainer. Best regards, Krzysztof
On 3/22/24 12:44 AM, Krzysztof Kozlowski wrote: > On 21/03/2024 16:13, Tanmay Shah wrote: >> >> >> On 3/21/24 2:39 AM, Krzysztof Kozlowski wrote: >>> On 20/03/2024 16:14, Tanmay Shah wrote: >>>> >>>> >>>> On 3/20/24 2:40 AM, Krzysztof Kozlowski wrote: >>>>> On 19/03/2024 15:42, Tanmay Shah wrote: >>>>>> >>>>>> >>>>>> 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@amd.com> >>>>>>>>>> --- >>>>>>>>>> .../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. >>>>> >>>>> You cannot have same programming model with different memory mappings. >>>>> Anyway, please follow writing bindings rules: all of your different >>>>> devices must have dedicated compatible. I really though we talked about >>>>> two IPs on same SoC... >>>> >>>> I agree that Versal compatible should be added, I will do that in next revision. >>>> >>>> For ZynqMP case, it is two IPs on same SOC. In lockstep mode and split mode, >>>> same SOC is configuring TCM differently. >>>> >>>> How this should be resolved for Versal-NET ? Driver avoids such TCM configuration >>>> for Versal-NET. >>> >>> Binding should describe the hardware, not what driver is doing >>> currently, so the question is: does your device have such properties or >>> not? Anyway, you need compatible per each variant and each SoC >>> implementation. >> >> Thanks for reviews. >> >> Okay in that case I believe I should add one more property to current bindings for TCM >> configuration. >> > > I am not sure if you understand how IRC works... You sent me message on > IRC about this topic and shortly after you quit. So how am I supposed to > send reply? IRC does not work like that... > Yeah, I am referring related documentation on IRC. >> From our discussion I conclude to following next steps: >> >> 1) I will send Versal and Versal-NET support as part of previous series (v14) so we get >> bigger picture in the first place. >> >> 2) Add separate compatible for versal platform. >> Use device compatible string to maintain >> backward compatibility and not machine (root node) compatible string. >> >> 3) Add tcm,mode property in bindings and each device must configure TCM based on that >> property only and not based on compatible string. >> >> 4) Versal-NET will disallow tcm,mode property in bindings as no such configuration is >> possible for that platform. > > I really don't know your SoCs. What about Zynq? You keep using here > names all over the place, but I am not Xilinx maintainer. > Zynq doesn't have Cortex-R IP so this driver isn't needed on that. > > Best regards, > Krzysztof >
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 + 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 + then: + patternProperties: + "^r5f@[0-9a-f]+$": + type: object + + properties: + reg: + minItems: 1 + items: + - description: ATCM internal memory + - description: BTCM internal memory + - description: extra ATCM memory in lockstep mode + - description: extra BTCM memory in lockstep mode + + reg-names: + minItems: 1 + items: + - const: atcm0 + - const: btcm0 + - const: atcm1 + - const: btcm1 + + power-domains: + minItems: 2 + items: + - description: RPU core power domain + - description: ATCM power domain + - description: BTCM power domain + - description: second ATCM power domain + - description: second BTCM 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 additionalProperties: false @@ -386,4 +427,111 @@ examples: }; }; }; + + - | + // Versal-NET split configuration + soc { + #address-cells = <2>; + #size-cells = <2>; + + remoteproc@eba00000 { + compatible = "xlnx,versal-net-r52fss"; + xlnx,cluster-mode = <0>; + #address-cells = <2>; + #size-cells = <2>; + ranges = <0x0 0x0 0x0 0xeba00000 0x0 0x10000>, + <0x0 0x10000 0x0 0xeba10000 0x0 0x8000>, + <0x0 0x18000 0x0 0xeba20000 0x0 0x8000>, + <0x1 0x0 0x0 0xeba40000 0x0 0x10000>, + <0x1 0x10000 0x0 0xeba50000 0x0 0x8000>, + <0x1 0x18000 0x0 0xeba60000 0x0 0x8000>; + r5f@0 { + compatible = "xlnx,versal-net-r52f"; + reg = <0x0 0x0 0x0 0x10000>, + <0x0 0x10000 0x0 0x8000>, + <0x0 0x18000 0x0 0x8000>; + reg-names = "atcm", "btcm", "ctcm"; + power-domains = <&versal_net_firmware 0x181100BF>, + <&versal_net_firmware 0x183180CB>, + <&versal_net_firmware 0x183180CC>, + <&versal_net_firmware 0x183180CD>; + 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,versal-net-r52f"; + reg = <0x1 0x0 0x0 0x10000>, <0x1 0x10000 0x0 0x8000>, + <0x1 0x18000 0x0 0x8000>; + reg-names = "atcm", "btcm", "ctcm"; + power-domains = <&versal_net_firmware 0x181100C0>, + <&versal_net_firmware 0x183180CE>, + <&versal_net_firmware 0x183180CF>, + <&versal_net_firmware 0x183180D0>; + memory-region = <&rproc_1_fw_image>, <&rpu1vdev0buffer>, + <&rpu1vdev0vring0>, <&rpu1vdev0vring1>; + mboxes = <&ipi_mailbox_rpu1 0>, <&ipi_mailbox_rpu1 1>; + mbox-names = "tx", "rx"; + }; + }; + }; + + - | + // Versal-NET lockstep configuration + soc { + #address-cells = <2>; + #size-cells = <2>; + + remoteproc@eba00000 { + compatible = "xlnx,versal-net-r52fss"; + xlnx,cluster-mode = <1>; + #address-cells = <2>; + #size-cells = <2>; + ranges = <0x0 0x0 0x0 0xeba00000 0x0 0x10000>, + <0x0 0x10000 0x0 0xeba10000 0x0 0x8000>, + <0x0 0x18000 0x0 0xeba20000 0x0 0x8000>, + <0x1 0x0 0x0 0xeba40000 0x0 0x10000>, + <0x1 0x10000 0x0 0xeba50000 0x0 0x8000>, + <0x1 0x18000 0x0 0xeba60000 0x0 0x8000>; + + r5f@0 { + compatible = "xlnx,versal-net-r52f"; + reg = <0x0 0x0 0x0 0x10000>, + <0x0 0x10000 0x0 0x8000>, + <0x0 0x18000 0x0 0x8000>; + + reg-names = "atcm", "btcm", "ctcm"; + + power-domains = <&versal_net_firmware 0x181100BF>, + <&versal_net_firmware 0x183180CB>, + <&versal_net_firmware 0x183180CC>, + <&versal_net_firmware 0x183180CD>; + + 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,versal-net-r52f"; + reg = <0x1 0x0 0x0 0x10000>, + <0x1 0x10000 0x0 0x8000>, + <0x1 0x18000 0x0 0x8000>; + reg-names = "atcm", "btcm", "ctcm"; + power-domains = <&versal_net_firmware 0x181100C0>, + <&versal_net_firmware 0x183180CE>, + <&versal_net_firmware 0x183180CF>, + <&versal_net_firmware 0x183180D0>; + 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-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@amd.com> --- .../remoteproc/xlnx,zynqmp-r5fss.yaml | 220 +++++++++++++++--- 1 file changed, 184 insertions(+), 36 deletions(-)