On 26/12/22 17:48, Krzysztof Kozlowski wrote: > On 23/12/2022 12:56, Devarsh Thakkar wrote: >> AM62 family of devices don't have a R5F cluster, instead >> they have single core DM R5F. >> Add new compatible string ti,am62-r5fss to support this scenario. >> >> When this new compatible is used don't allow cluster-mode >> property usage in device-tree as this implies that there >> is no R5F cluster available and only single R5F core >> is present. >> >> Signed-off-by: Devarsh Thakkar <devarsht@xxxxxx> >> --- >> V2: Avoid acronyms, use "Device Manager" instead of "DM" >> V3: >> - Use separate if block for each compatible for ti,cluster-mode property >> - Rearrange compatibles as per alphabatical order >> --- >> .../bindings/remoteproc/ti,k3-r5f-rproc.yaml | 68 +++++++++++++------ >> 1 file changed, 47 insertions(+), 21 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml >> index fb9605f0655b..e8a861179bd9 100644 >> --- a/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml >> +++ b/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml >> @@ -21,6 +21,9 @@ description: | >> called "Single-CPU" mode, where only Core0 is used, but with ability to use >> Core1's TCMs as well. >> >> + AM62 SoC family support a single R5F core only which runs Device Manager >> + firmware and can also be used as a remote processor with IPC communication. >> + >> Each Dual-Core R5F sub-system is represented as a single DTS node >> representing the cluster, with a pair of child DT nodes representing >> the individual R5F cores. Each node has a number of required or optional >> @@ -28,16 +31,20 @@ description: | >> the device management of the remote processor and to communicate with the >> remote processor. >> >> + Since AM62 SoC family only support a single core, there is no cluster-mode >> + property setting required for it. >> + >> properties: >> $nodename: >> pattern: "^r5fss(@.*)?" >> >> compatible: >> enum: >> + - ti,am62-r5fss >> + - ti,am64-r5fss >> - ti,am654-r5fss >> - - ti,j721e-r5fss >> - ti,j7200-r5fss >> - - ti,am64-r5fss >> + - ti,j721e-r5fss >> - ti,j721s2-r5fss >> >> power-domains: >> @@ -80,7 +87,9 @@ patternProperties: >> node representing a TI instantiation of the Arm Cortex R5F core. There >> are some specific integration differences for the IP like the usage of >> a Region Address Translator (RAT) for translating the larger SoC bus >> - addresses into a 32-bit address space for the processor. >> + addresses into a 32-bit address space for the processor. For AM62x, >> + the R5F Sub-System device node should only define one R5F child node >> + as it has only one core available. >> >> Each R5F core has an associated 64 KB of Tightly-Coupled Memory (TCM) >> internal memories split between two banks - TCMA and TCMB (further >> @@ -100,11 +109,12 @@ patternProperties: >> properties: >> compatible: >> enum: >> - - ti,am654-r5f >> - - ti,j721e-r5f >> - - ti,j7200-r5f >> - - ti,am64-r5f >> - - ti,j721s2-r5f >> + - ti,am62-r5fss >> + - ti,am64-r5fss >> + - ti,am654-r5fss >> + - ti,j7200-r5fss >> + - ti,j721e-r5fss >> + - ti,j721s2-r5fss >> There is a problem here, the compatibles still need to be "-r5f" I will correct it in V4. >> reg: >> items: >> @@ -208,19 +218,35 @@ patternProperties: >> >> unevaluatedProperties: false >> >> -if: >> - properties: >> - compatible: >> - enum: >> - - ti,am64-r5fss >> -then: >> - properties: >> - ti,cluster-mode: >> - enum: [0, 2] >> -else: >> - properties: >> - ti,cluster-mode: >> - enum: [0, 1] >> +allOf: >> + - if: >> + properties: >> + compatible: >> + enum: >> + - ti,am64-r5fss >> + then: >> + properties: >> + ti,cluster-mode: >> + enum: [0, 2] >> + >> + - if: >> + properties: >> + compatible: >> + enum: ["ti,am654-r5fss", "ti,j7200-r5fss", "ti,j721e-r5fss", "ti,j721s2-r5fss"] > > That's not how enums are spelled for such cases. Git grep for examples - > this should be a enum with each item in new entry, no quotes. Yeah, that was my initial thought but then I looked at section 4.9.3 of https://json-schema.org/understanding-json-schema/UnderstandingJSONSchema.pdf which had below example and based on that did this change thus avoiding separate entries for each enum. Example: { "enum": ["red", "amber", "green", null, 42] } To confirm whether the change works fine, I deliberately modified cluster-mode values for each of the SoC's beyond acceptable ranges as seen in https://gist.github.com/devarsht/1956063c8e39f1bdbad3574ea96b95a3 and then ran "make dtbs_check" and it was able to catch the inappropriate values of cluster-mode as seen in below logs : https://gist.github.com/devarsht/bc8cfb82c55b75d85649585d3fd536a0#file-gistfile1-txt-L392 https://gist.github.com/devarsht/bc8cfb82c55b75d85649585d3fd536a0#file-gistfile1-txt-L500 https://gist.github.com/devarsht/bc8cfb82c55b75d85649585d3fd536a0#file-gistfile1-txt-L712 https://gist.github.com/devarsht/bc8cfb82c55b75d85649585d3fd536a0#file-gistfile1-txt-L741 https://gist.github.com/devarsht/bc8cfb82c55b75d85649585d3fd536a0#file-gistfile1-txt-L750 https://gist.github.com/devarsht/bc8cfb82c55b75d85649585d3fd536a0#file-gistfile1-txt-L766 https://gist.github.com/devarsht/bc8cfb82c55b75d85649585d3fd536a0#file-gistfile1-txt-L773 Kindly let me know if you see some issues with this approach. Best Regards, Devarsh > > Best regards, > Krzysztof >
