diff mbox series

[v2,1/2] Documentation: dt-bindings: k3-r5f-rproc: Add new compatible for AM62 SoC family

Message ID 20221130134052.7513-2-devarsht@ti.com (mailing list archive)
State Superseded
Headers show
Series Add single core R5F IPC for AM62 SoC family | expand

Commit Message

Devarsh Thakkar Nov. 30, 2022, 1:40 p.m. UTC
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@ti.com>
---
V2: Avoid acronyms, use "Device Manager" instead of "DM"
---
 .../bindings/remoteproc/ti,k3-r5f-rproc.yaml  | 48 +++++++++++++------
 1 file changed, 34 insertions(+), 14 deletions(-)

Comments

Krzysztof Kozlowski Nov. 30, 2022, 3:03 p.m. UTC | #1
On 30/11/2022 14:40, 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@ti.com>
> ---
> V2: Avoid acronyms, use "Device Manager" instead of "DM"

Use subject prefixes matching the subsystem (git log --oneline -- ...).

> ---
>  .../bindings/remoteproc/ti,k3-r5f-rproc.yaml  | 48 +++++++++++++------
>  1 file changed, 34 insertions(+), 14 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..91357635025a 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,6 +31,9 @@ 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(@.*)?"
> @@ -38,6 +44,7 @@ properties:
>        - ti,j721e-r5fss
>        - ti,j7200-r5fss
>        - ti,am64-r5fss
> +      - ti,am62-r5fss

Some order? Alphabetical, so before am64? Same in other places.


>        - ti,j721s2-r5fss
>  
>    power-domains:
> @@ -80,7 +87,8 @@ 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,
> +      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
> @@ -104,6 +112,7 @@ patternProperties:
>            - ti,j721e-r5f
>            - ti,j7200-r5f
>            - ti,am64-r5f
> +          - ti,am62-r5f
>            - ti,j721s2-r5f
>  
>        reg:
> @@ -207,20 +216,31 @@ patternProperties:
>        - firmware-name
>  
>      unevaluatedProperties: false

Blank line.

> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - ti,am64-r5fss
> +    then:
> +      properties:
> +        ti,cluster-mode:
> +          enum: [0, 2]
> +
> +    else:
> +      properties:
> +        ti,cluster-mode:

It's not really valid anymore for ti,am62-r5fss, so this cannot be
simple "else".  Instead you need to list all compatibles.

> +          enum: [0, 1]
> +
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - ti,am62-r5fss
> +    then:
> +      properties:
> +        ti,cluster-mode: false
>  
> -if:
> -  properties:
> -    compatible:
> -      enum:
> -        - ti,am64-r5fss
> -then:
> -  properties:
> -    ti,cluster-mode:
> -      enum: [0, 2]
> -else:
> -  properties:
> -    ti,cluster-mode:
> -      enum: [0, 1]
>  
>  required:
>    - compatible

Best regards,
Krzysztof
Devarsh Thakkar Dec. 21, 2022, 7:42 a.m. UTC | #2
Hi Krzysztof,

Thanks for the review. Please find my response inline.

On 30/11/22 20:33, Krzysztof Kozlowski wrote:
> On 30/11/2022 14:40, 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@ti.com>
>> ---
>> V2: Avoid acronyms, use "Device Manager" instead of "DM"
> 
> Use subject prefixes matching the subsystem (git log --oneline -- ...).
Agreed, I will update the prefix as dt-bindings: remoteproc: k3-r5f: in V3.
> 
>> ---
>>  .../bindings/remoteproc/ti,k3-r5f-rproc.yaml  | 48 +++++++++++++------
>>  1 file changed, 34 insertions(+), 14 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..91357635025a 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,6 +31,9 @@ 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(@.*)?"
>> @@ -38,6 +44,7 @@ properties:
>>        - ti,j721e-r5fss
>>        - ti,j7200-r5fss
>>        - ti,am64-r5fss
>> +      - ti,am62-r5fss
> 
> Some order? Alphabetical, so before am64? Same in other places.
Agreed, I will update in V3 accordingly.
> 
> 
>>        - ti,j721s2-r5fss
>>  
>>    power-domains:
>> @@ -80,7 +87,8 @@ 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,
>> +      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
>> @@ -104,6 +112,7 @@ patternProperties:
>>            - ti,j721e-r5f
>>            - ti,j7200-r5f
>>            - ti,am64-r5f
>> +          - ti,am62-r5f
>>            - ti,j721s2-r5f
>>  
>>        reg:
>> @@ -207,20 +216,31 @@ patternProperties:
>>        - firmware-name
>>  
>>      unevaluatedProperties: false
> 
> Blank line.
Agreed, I will remove it in V3.
> 
>> +allOf:
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          enum:
>> +            - ti,am64-r5fss
>> +    then:
>> +      properties:
>> +        ti,cluster-mode:
>> +          enum: [0, 2]
>> +
>> +    else:
>> +      properties:
>> +        ti,cluster-mode:
> 
> It's not really valid anymore for ti,am62-r5fss, so this cannot be
> simple "else".  Instead you need to list all compatibles.
I agree that the else block is not valid for am62x, but my understanding is that since all the blocks under allOf are checked for validity,
I thought to add a separate if block only for am62x to set cluster-mode to false [1], which I believe would negate the effect of above else condition for am62x,
so that we don't have to list all compatibles under separate if blocks.

Just to verify this, I deliberately set cluster-mode=1 in am62x devicetree and then ran a dtbs-check and got below log : 
"linux-next/arch/arm64/boot/dts/ti/k3-am625-sk.dtb: r5fss@78000000: ti,cluster-mode: False schema does not allow [[1]]"

and above warning log goes away when i remove the cluster-mode node in am62x devicetree.
But please do let me know if I am missing something here or there is a better/more proper way to do this.

Best Regards,
Devarsh

> 
>> +          enum: [0, 1]
>> +
[1]
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          enum:
>> +            - ti,am62-r5fss
>> +    then:
>> +      properties:
>> +        ti,cluster-mode: false
>>  
>> -if:
>> -  properties:
>> -    compatible:
>> -      enum:
>> -        - ti,am64-r5fss
>> -then:
>> -  properties:
>> -    ti,cluster-mode:
>> -      enum: [0, 2]
>> -else:
>> -  properties:
>> -    ti,cluster-mode:
>> -      enum: [0, 1]
>>  
>>  required:
>>    - compatible
> 
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski Dec. 21, 2022, 9:36 a.m. UTC | #3
On 21/12/2022 08:42, Devarsh Thakkar wrote:
> Hi Krzysztof,
> 
> Thanks for the review. Please find my response inline.
> 
> On 30/11/22 20:33, Krzysztof Kozlowski wrote:
>> On 30/11/2022 14:40, 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@ti.com>
>>> ---
>>> V2: Avoid acronyms, use "Device Manager" instead of "DM"
>>
>> Use subject prefixes matching the subsystem (git log --oneline -- ...).
> Agreed, I will update the prefix as dt-bindings: remoteproc: k3-r5f: in V3.
>>
>>> ---
>>>  .../bindings/remoteproc/ti,k3-r5f-rproc.yaml  | 48 +++++++++++++------
>>>  1 file changed, 34 insertions(+), 14 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..91357635025a 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,6 +31,9 @@ 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(@.*)?"
>>> @@ -38,6 +44,7 @@ properties:
>>>        - ti,j721e-r5fss
>>>        - ti,j7200-r5fss
>>>        - ti,am64-r5fss
>>> +      - ti,am62-r5fss
>>
>> Some order? Alphabetical, so before am64? Same in other places.
> Agreed, I will update in V3 accordingly.
>>
>>
>>>        - ti,j721s2-r5fss
>>>  
>>>    power-domains:
>>> @@ -80,7 +87,8 @@ 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,
>>> +      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
>>> @@ -104,6 +112,7 @@ patternProperties:
>>>            - ti,j721e-r5f
>>>            - ti,j7200-r5f
>>>            - ti,am64-r5f
>>> +          - ti,am62-r5f
>>>            - ti,j721s2-r5f
>>>  
>>>        reg:
>>> @@ -207,20 +216,31 @@ patternProperties:
>>>        - firmware-name
>>>  
>>>      unevaluatedProperties: false
>>
>> Blank line.
> Agreed, I will remove it in V3.
>>
>>> +allOf:
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          enum:
>>> +            - ti,am64-r5fss
>>> +    then:
>>> +      properties:
>>> +        ti,cluster-mode:
>>> +          enum: [0, 2]
>>> +
>>> +    else:
>>> +      properties:
>>> +        ti,cluster-mode:
>>
>> It's not really valid anymore for ti,am62-r5fss, so this cannot be
>> simple "else".  Instead you need to list all compatibles.
> I agree that the else block is not valid for am62x, but my understanding is that since all the blocks under allOf are checked for validity,
> I thought to add a separate if block only for am62x to set cluster-mode to false [1], which I believe would negate the effect of above else condition for am62x,
> so that we don't have to list all compatibles under separate if blocks.
> 
> Just to verify this, I deliberately set cluster-mode=1 in am62x devicetree and then ran a dtbs-check and got below log : 
> "linux-next/arch/arm64/boot/dts/ti/k3-am625-sk.dtb: r5fss@78000000: ti,cluster-mode: False schema does not allow [[1]]"
> 
> and above warning log goes away when i remove the cluster-mode node in am62x devicetree.
> But please do let me know if I am missing something here or there is a better/more proper way to do this.

This was three weeks ago, so hundreds of patches ago, I don't remember
anymore.

Just look at your patch - it is clearly incorrect. You said in the patch
that for compatibles other than ti,am64-r5fss cluster mode is BOTH [0,
1] AND false.

I gave you the way to fix it. Feel free to fix it other ways if it gives
correct result.

Best regards,
Krzysztof
Devarsh Thakkar Dec. 21, 2022, 4:29 p.m. UTC | #4
Hi Krzysztof,

On 21/12/22 15:06, Krzysztof Kozlowski wrote:
> On 21/12/2022 08:42, Devarsh Thakkar wrote:
>> Hi Krzysztof,
>>
>> Thanks for the review. Please find my response inline.
>>
>> On 30/11/22 20:33, Krzysztof Kozlowski wrote:
>>> On 30/11/2022 14:40, 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@ti.com>
>>>> ---
>>>> V2: Avoid acronyms, use "Device Manager" instead of "DM"
>>>
>>> Use subject prefixes matching the subsystem (git log --oneline -- ...).
>> Agreed, I will update the prefix as dt-bindings: remoteproc: k3-r5f: in V3.
>>>
>>>> ---
>>>>  .../bindings/remoteproc/ti,k3-r5f-rproc.yaml  | 48 +++++++++++++------
>>>>  1 file changed, 34 insertions(+), 14 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..91357635025a 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,6 +31,9 @@ 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(@.*)?"
>>>> @@ -38,6 +44,7 @@ properties:
>>>>        - ti,j721e-r5fss
>>>>        - ti,j7200-r5fss
>>>>        - ti,am64-r5fss
>>>> +      - ti,am62-r5fss
>>>
>>> Some order? Alphabetical, so before am64? Same in other places.
>> Agreed, I will update in V3 accordingly.
>>>
>>>
>>>>        - ti,j721s2-r5fss
>>>>  
>>>>    power-domains:
>>>> @@ -80,7 +87,8 @@ 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,
>>>> +      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
>>>> @@ -104,6 +112,7 @@ patternProperties:
>>>>            - ti,j721e-r5f
>>>>            - ti,j7200-r5f
>>>>            - ti,am64-r5f
>>>> +          - ti,am62-r5f
>>>>            - ti,j721s2-r5f
>>>>  
>>>>        reg:
>>>> @@ -207,20 +216,31 @@ patternProperties:
>>>>        - firmware-name
>>>>  
>>>>      unevaluatedProperties: false
>>>
>>> Blank line.
>> Agreed, I will remove it in V3.
>>>
>>>> +allOf:
>>>> +  - if:
>>>> +      properties:
>>>> +        compatible:
>>>> +          enum:
>>>> +            - ti,am64-r5fss
>>>> +    then:
>>>> +      properties:
>>>> +        ti,cluster-mode:
>>>> +          enum: [0, 2]
>>>> +
>>>> +    else:
>>>> +      properties:
>>>> +        ti,cluster-mode:
>>>
>>> It's not really valid anymore for ti,am62-r5fss, so this cannot be
>>> simple "else".  Instead you need to list all compatibles.
>> I agree that the else block is not valid for am62x, but my understanding is that since all the blocks under allOf are checked for validity,
>> I thought to add a separate if block only for am62x to set cluster-mode to false [1], which I believe would negate the effect of above else condition for am62x,
>> so that we don't have to list all compatibles under separate if blocks.
>>
>> Just to verify this, I deliberately set cluster-mode=1 in am62x devicetree and then ran a dtbs-check and got below log : 
[2]
>> "linux-next/arch/arm64/boot/dts/ti/k3-am625-sk.dtb: r5fss@78000000: ti,cluster-mode: False schema does not allow [[1]]"
>>
>> and above warning log goes away when i remove the cluster-mode node in am62x devicetree.
>> But please do let me know if I am missing something here or there is a better/more proper way to do this.
> 
> This was three weeks ago, so hundreds of patches ago, I don't remember
> anymore.
My apologies for the delay.
> 
> Just look at your patch - it is clearly incorrect. You said in the patch
> that for compatibles other than ti,am64-r5fss cluster mode is BOTH [0,
> 1] AND false.

cluster-mode is BOTH [0,1] and false only in case of AM62x as per below snippet, but since it's under allOf the impact of latter will supersede, schema validation will fail even if cluster-mode set to 0 or 1 for am62x due to below snippet as shared in obesrvation log above [2]. 

"  - if:
      properties:
        compatible:
          enum:
            - ti,am62-r5fss
    then:
      properties:
        ti,cluster-mode: false"

Sorry for the back and forth, I just thought to describe more clearly what I was up-to as I thought above should be functionally fine and it also saves us from having separate if blocks for each compatible, but I am open to adding separate if blocks as you earlier suggested if that seems more cleaner solution.

Best Regards,
Devarsh
> 
> I gave you the way to fix it. Feel free to fix it other ways if it gives
> correct result.
> 
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski Dec. 21, 2022, 5:17 p.m. UTC | #5
On 21/12/2022 17:29, Devarsh Thakkar wrote:
>>
>> Just look at your patch - it is clearly incorrect. You said in the patch
>> that for compatibles other than ti,am64-r5fss cluster mode is BOTH [0,
>> 1] AND false.
> 
> cluster-mode is BOTH [0,1] and false only in case of AM62x as per below snippet

Yes, for that variant you have conflicting approach.

, but since it's under allOf the impact of latter will supersede, schema
validation will fail even if cluster-mode set to 0 or 1 for am62x due to
below snippet as shared in obesrvation log above [2].

Yeah, but the code is confusing. So again - you are saying with allOf
that both conditions are applicable. Your intentions of superseding do
not matter here - you said that allOf conditions must be taken into
account. These conditions can be reversed any time, don't you think?


> 
> "  - if:
>       properties:
>         compatible:
>           enum:
>             - ti,am62-r5fss
>     then:
>       properties:
>         ti,cluster-mode: false"
> 
> Sorry for the back and forth, I just thought to describe more clearly what I was up-to as I thought above should be functionally fine and it also saves us from having separate if blocks for each compatible, but I am open to adding separate if blocks as you earlier suggested if that seems more cleaner solution.

You need to fix your email client to properly wrap messages.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml
index fb9605f0655b..91357635025a 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,6 +31,9 @@  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(@.*)?"
@@ -38,6 +44,7 @@  properties:
       - ti,j721e-r5fss
       - ti,j7200-r5fss
       - ti,am64-r5fss
+      - ti,am62-r5fss
       - ti,j721s2-r5fss
 
   power-domains:
@@ -80,7 +87,8 @@  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,
+      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
@@ -104,6 +112,7 @@  patternProperties:
           - ti,j721e-r5f
           - ti,j7200-r5f
           - ti,am64-r5f
+          - ti,am62-r5f
           - ti,j721s2-r5f
 
       reg:
@@ -207,20 +216,31 @@  patternProperties:
       - firmware-name
 
     unevaluatedProperties: false
+allOf:
+  - if:
+      properties:
+        compatible:
+          enum:
+            - ti,am64-r5fss
+    then:
+      properties:
+        ti,cluster-mode:
+          enum: [0, 2]
+
+    else:
+      properties:
+        ti,cluster-mode:
+          enum: [0, 1]
+
+  - if:
+      properties:
+        compatible:
+          enum:
+            - ti,am62-r5fss
+    then:
+      properties:
+        ti,cluster-mode: false
 
-if:
-  properties:
-    compatible:
-      enum:
-        - ti,am64-r5fss
-then:
-  properties:
-    ti,cluster-mode:
-      enum: [0, 2]
-else:
-  properties:
-    ti,cluster-mode:
-      enum: [0, 1]
 
 required:
   - compatible