[1/2] dt-bindings: riscv: sifive-l2: add a PolarFire SoC compatible

Commit Message

Conor Dooley Aug. 25, 2022, 6:04 p.m. UTC

From: Conor Dooley <[e-mail]>

The l2 cache on PolarFire SoC is cross between that of the fu540 and
the fu740. It has the extra interrupt from the fu740 but the lower
number of cache-sets. Add a specific compatible to avoid the likes
of:

mpfs-polarberry.dtb: cache-controller@2010000: interrupts: [[1], [3], [4], [2]] is too long

Fixes: 34fc9cc3aebe ("riscv: dts: microchip: correct L2 cache interrupts")
Signed-off-by: Conor Dooley <[e-mail]>
---
 .../bindings/riscv/sifive-l2-cache.yaml       | 79 ++++++++++++-------
 1 file changed, 49 insertions(+), 30 deletions(-)

Comments

Heinrich Schuchardt Aug. 25, 2022, 6:36 p.m. UTC | #1

On 8/25/22 20:04, Conor Dooley wrote:
> From: Conor Dooley <[e-mail]>
> 
> The l2 cache on PolarFire SoC is cross between that of the fu540 and
> the fu740. It has the extra interrupt from the fu740 but the lower
> number of cache-sets. Add a specific compatible to avoid the likes
> of:
> 
> mpfs-polarberry.dtb: cache-controller@2010000: interrupts: [[1], [3], [4], [2]] is too long

Where is such a message written? I couldn't find the string in 
next-20220825 (git grep -n 'is too long"').

Why should a different number of cache sets require an extra compatible 
string. cache-size is simply a parameter going with the existing 
compatible strings.

I would assume that you only need an extra compatible string if there is 
a functional difference that can not be expressed with the existing 
parameters.

> 
> Fixes: 34fc9cc3aebe ("riscv: dts: microchip: correct L2 cache interrupts")
> Signed-off-by: Conor Dooley <[e-mail]>
> ---
>   .../bindings/riscv/sifive-l2-cache.yaml       | 79 ++++++++++++-------
>   1 file changed, 49 insertions(+), 30 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/riscv/sifive-l2-cache.yaml b/Documentation/devicetree/bindings/riscv/sifive-l2-cache.yaml
> index 69cdab18d629..ca3b9be58058 100644
> --- a/Documentation/devicetree/bindings/riscv/sifive-l2-cache.yaml
> +++ b/Documentation/devicetree/bindings/riscv/sifive-l2-cache.yaml
> @@ -17,9 +17,6 @@ description:
>     acts as directory-based coherency manager.
>     All the properties in ePAPR/DeviceTree specification applies for this platform.
>   
> -allOf:
> -  - $ref: /schemas/cache-controller.yaml#
> -
>   select:
>     properties:
>       compatible:
> @@ -33,11 +30,16 @@ select:
>   
>   properties:
>     compatible:
> -    items:
> -      - enum:
> -          - sifive,fu540-c000-ccache
> -          - sifive,fu740-c000-ccache

Why can't you simply add microchip,mpfs-ccache here?

> -      - const: cache
> +    oneOf:
> +      - items:
> +          - enum:
> +              - sifive,fu540-c000-ccache
> +              - sifive,fu740-c000-ccache
> +          - const: cache
> +      - items:
> +          - const: microchip,mpfs-ccache
> +          - const: sifive,fu540-c000-ccache

Why do we need 'sifive,fu540-c000-ccache' twice?

Best regards

Heinrich

> +          - const: cache
>   
>     cache-block-size:
>       const: 64
> @@ -72,29 +74,46 @@ properties:
>         The reference to the reserved-memory for the L2 Loosely Integrated Memory region.
>         The reserved memory node should be defined as per the bindings in reserved-memory.txt.
>   
> -if:
> -  properties:
> -    compatible:
> -      contains:
> -        const: sifive,fu540-c000-ccache
> +allOf:
> +  - $ref: /schemas/cache-controller.yaml#
>   
> -then:
> -  properties:
> -    interrupts:
> -      description: |
> -        Must contain entries for DirError, DataError and DataFail signals.
> -      maxItems: 3
> -    cache-sets:
> -      const: 1024
> -
> -else:
> -  properties:
> -    interrupts:
> -      description: |
> -        Must contain entries for DirError, DataError, DataFail, DirFail signals.
> -      minItems: 4
> -    cache-sets:
> -      const: 2048
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - sifive,fu740-c000-ccache
> +              - microchip,mpfs-ccache
> +
> +    then:
> +      properties:
> +        interrupts:
> +          description: |
> +            Must contain entries for DirError, DataError, DataFail, DirFail signals.
> +          minItems: 4
> +
> +    else:
> +      properties:
> +        interrupts:
> +          description: |
> +            Must contain entries for DirError, DataError and DataFail signals.
> +          maxItems: 3
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: sifive,fu740-c000-ccache
> +
> +    then:
> +      properties:
> +        cache-sets:
> +          const: 2048
> +
> +    else:
> +      properties:
> +        cache-sets:
> +          const: 1024
>   
>   additionalProperties: false
>

Conor Dooley Aug. 25, 2022, 6:56 p.m. UTC | #2

On 25/08/2022 19:36, Heinrich Schuchardt wrote:
> On 8/25/22 20:04, Conor Dooley wrote:
>> From: Conor Dooley <[e-mail]>
>>
>> The l2 cache on PolarFire SoC is cross between that of the fu540 and
>> the fu740. It has the extra interrupt from the fu740 but the lower
>> number of cache-sets. Add a specific compatible to avoid the likes
>> of:
>>
>> mpfs-polarberry.dtb: cache-controller@2010000: interrupts: [[1], [3], [4], [2]] is too long
> 
> Where is such a message written? I couldn't find the string in
> next-20220825 (git grep -n 'is too long"').

dtbs_check on next-20220825 (with dt-schema v22.08 FWIW):
mpfs-polarberry.dtb: cache-controller@2010000: interrupts: [[1], [3], [4], [2]] is too long
mpfs-icicle-kit.dtb: cache-controller@2010000: interrupts: [[1], [3], [4], [2]] is too long

I should have caught this before applying, but I got distracted
by the unusable system.

> 
> Why should a different number of cache sets require an extra
> compatible string. cache-size is simply a parameter going with> the existing compatible strings.

s/cache sets/interrupts
Because the correct value for the fu540 is 3 & this is regulated by
the binding. The alternative would be relaxing the binding to not
regulate the number of interrupts.

> 
> I would assume that you only need an extra compatible string if
> there is a functional difference that can not be expressed with
> the existing parameters.
> 
>>
>> Fixes: 34fc9cc3aebe ("riscv: dts: microchip: correct L2 cache interrupts")
>> Signed-off-by: Conor Dooley <[e-mail]>
>> ---
>>   .../bindings/riscv/sifive-l2-cache.yaml       | 79 ++++++++++++-------
>>   1 file changed, 49 insertions(+), 30 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/riscv/sifive-l2-cache.yaml b/Documentation/devicetree/bindings/riscv/sifive-l2-cache.yaml
>> index 69cdab18d629..ca3b9be58058 100644
>> --- a/Documentation/devicetree/bindings/riscv/sifive-l2-cache.yaml
>> +++ b/Documentation/devicetree/bindings/riscv/sifive-l2-cache.yaml
>> @@ -17,9 +17,6 @@ description:
>>     acts as directory-based coherency manager.
>>     All the properties in ePAPR/DeviceTree specification applies for this platform.
>>   -allOf:
>> -  - $ref: /schemas/cache-controller.yaml#
>> -
>>   select:
>>     properties:
>>       compatible:
>> @@ -33,11 +30,16 @@ select:
>>     properties:
>>     compatible:
>> -    items:
>> -      - enum:
>> -          - sifive,fu540-c000-ccache
>> -          - sifive,fu740-c000-ccache
> 
> Why can't you simply add microchip,mpfs-ccache here?

I *could* but I opted not to because the fu540 supports a compatible
subset of the features & keeping the compatible for it allows systems
with a newer dts to work with an older kernel.

> 
>> -      - const: cache
>> +    oneOf:
>> +      - items:
>> +          - enum:
>> +              - sifive,fu540-c000-ccache
>> +              - sifive,fu740-c000-ccache
>> +          - const: cache
>> +      - items:
>> +          - const: microchip,mpfs-ccache
>> +          - const: sifive,fu540-c000-ccache
> 
> Why do we need 'sifive,fu540-c000-ccache' twice?

Is there a better way to write it given the above caveat?

Thanks,
Conor.


> 
>> +          - const: cache
>>       cache-block-size:
>>       const: 64
>> @@ -72,29 +74,46 @@ properties:
>>         The reference to the reserved-memory for the L2 Loosely Integrated Memory region.
>>         The reserved memory node should be defined as per the bindings in reserved-memory.txt.
>>   -if:
>> -  properties:
>> -    compatible:
>> -      contains:
>> -        const: sifive,fu540-c000-ccache
>> +allOf:
>> +  - $ref: /schemas/cache-controller.yaml#
>>   -then:
>> -  properties:
>> -    interrupts:
>> -      description: |
>> -        Must contain entries for DirError, DataError and DataFail signals.
>> -      maxItems: 3
>> -    cache-sets:
>> -      const: 1024
>> -
>> -else:
>> -  properties:
>> -    interrupts:
>> -      description: |
>> -        Must contain entries for DirError, DataError, DataFail, DirFail signals.
>> -      minItems: 4
>> -    cache-sets:
>> -      const: 2048
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - sifive,fu740-c000-ccache
>> +              - microchip,mpfs-ccache
>> +
>> +    then:
>> +      properties:
>> +        interrupts:
>> +          description: |
>> +            Must contain entries for DirError, DataError, DataFail, DirFail signals.
>> +          minItems: 4
>> +
>> +    else:
>> +      properties:
>> +        interrupts:
>> +          description: |
>> +            Must contain entries for DirError, DataError and DataFail signals.
>> +          maxItems: 3
>> +
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: sifive,fu740-c000-ccache
>> +
>> +    then:
>> +      properties:
>> +        cache-sets:
>> +          const: 2048
>> +
>> +    else:
>> +      properties:
>> +        cache-sets:
>> +          const: 1024
>>     additionalProperties: false
>>

Heinrich Schuchardt Aug. 25, 2022, 7:49 p.m. UTC | #3

On 8/25/22 20:56, [e-mail] wrote:
> On 25/08/2022 19:36, Heinrich Schuchardt wrote:
>> On 8/25/22 20:04, Conor Dooley wrote:
>>> From: Conor Dooley <[e-mail]>
>>>
>>> The l2 cache on PolarFire SoC is cross between that of the fu540 and
>>> the fu740. It has the extra interrupt from the fu740 but the lower
>>> number of cache-sets. Add a specific compatible to avoid the likes
>>> of:
>>>
>>> mpfs-polarberry.dtb: cache-controller@2010000: interrupts: [[1], [3], [4], [2]] is too long
>>
>> Where is such a message written? I couldn't find the string in
>> next-20220825 (git grep -n 'is too long"').
> 
> dtbs_check on next-20220825 (with dt-schema v22.08 FWIW):
> mpfs-polarberry.dtb: cache-controller@2010000: interrupts: [[1], [3], [4], [2]] is too long
> mpfs-icicle-kit.dtb: cache-controller@2010000: interrupts: [[1], [3], [4], [2]] is too long
> 
> I should have caught this before applying, but I got distracted
> by the unusable system.
> 
>>
>> Why should a different number of cache sets require an extra
>> compatible string. cache-size is simply a parameter going with> the existing compatible strings.
> 
> s/cache sets/interrupts
> Because the correct value for the fu540 is 3 & this is regulated by
> the binding. The alternative would be relaxing the binding to not
> regulate the number of interrupts.
> 
>>
>> I would assume that you only need an extra compatible string if
>> there is a functional difference that can not be expressed with
>> the existing parameters.
>>
>>>
>>> Fixes: 34fc9cc3aebe ("riscv: dts: microchip: correct L2 cache interrupts")
>>> Signed-off-by: Conor Dooley <[e-mail]>
>>> ---
>>>    .../bindings/riscv/sifive-l2-cache.yaml       | 79 ++++++++++++-------
>>>    1 file changed, 49 insertions(+), 30 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/riscv/sifive-l2-cache.yaml b/Documentation/devicetree/bindings/riscv/sifive-l2-cache.yaml
>>> index 69cdab18d629..ca3b9be58058 100644
>>> --- a/Documentation/devicetree/bindings/riscv/sifive-l2-cache.yaml
>>> +++ b/Documentation/devicetree/bindings/riscv/sifive-l2-cache.yaml
>>> @@ -17,9 +17,6 @@ description:
>>>      acts as directory-based coherency manager.
>>>      All the properties in ePAPR/DeviceTree specification applies for this platform.
>>>    -allOf:
>>> -  - $ref: /schemas/cache-controller.yaml#
>>> -
>>>    select:
>>>      properties:
>>>        compatible:
>>> @@ -33,11 +30,16 @@ select:
>>>      properties:
>>>      compatible:
>>> -    items:
>>> -      - enum:
>>> -          - sifive,fu540-c000-ccache
>>> -          - sifive,fu740-c000-ccache
>>
>> Why can't you simply add microchip,mpfs-ccache here?
> 
> I *could* but I opted not to because the fu540 supports a compatible
> subset of the features & keeping the compatible for it allows systems
> with a newer dts to work with an older kernel.

That makes it clearer.

> 
>>
>>> -      - const: cache
>>> +    oneOf:
>>> +      - items:
>>> +          - enum:
>>> +              - sifive,fu540-c000-ccache
>>> +              - sifive,fu740-c000-ccache
>>> +          - const: cache
>>> +      - items:
>>> +          - const: microchip,mpfs-ccache
>>> +          - const: sifive,fu540-c000-ccache
>>
>> Why do we need 'sifive,fu540-c000-ccache' twice?
> 
> Is there a better way to write it given the above caveat?
> 
> Thanks,
> Conor.
> 
> 
>>
>>> +          - const: cache
>>>        cache-block-size:
>>>        const: 64
>>> @@ -72,29 +74,46 @@ properties:
>>>          The reference to the reserved-memory for the L2 Loosely Integrated Memory region.
>>>          The reserved memory node should be defined as per the bindings in reserved-memory.txt.
>>>    -if:
>>> -  properties:
>>> -    compatible:
>>> -      contains:
>>> -        const: sifive,fu540-c000-ccache
>>> +allOf:
>>> +  - $ref: /schemas/cache-controller.yaml#
>>>    -then:
>>> -  properties:
>>> -    interrupts:
>>> -      description: |
>>> -        Must contain entries for DirError, DataError and DataFail signals.
>>> -      maxItems: 3
>>> -    cache-sets:
>>> -      const: 1024
>>> -
>>> -else:
>>> -  properties:
>>> -    interrupts:
>>> -      description: |
>>> -        Must contain entries for DirError, DataError, DataFail, DirFail signals.
>>> -      minItems: 4
>>> -    cache-sets:
>>> -      const: 2048
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            enum:
>>> +              - sifive,fu740-c000-ccache
>>> +              - microchip,mpfs-ccache
>>> +
>>> +    then:
>>> +      properties:
>>> +        interrupts:
>>> +          description: |
>>> +            Must contain entries for DirError, DataError, DataFail, DirFail signals.
>>> +          minItems: 4

Above you indicated that you want strict limits for the interrupt count. 
You expect exactly 4 items here. Having 5 entries would not be correct.
Please, add 'maxItems: 4'.

>>> +
>>> +    else:
>>> +      properties:
>>> +        interrupts:
>>> +          description: |
>>> +            Must contain entries for DirError, DataError and DataFail signals.
>>> +          maxItems: 3

The item count should be exactly 3. Having 2 entries would not be correct.
Please, add 'minItems: 3'.

Best regards

Heinrich

>>> +
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            const: sifive,fu740-c000-ccache
>>> +
>>> +    then:
>>> +      properties:
>>> +        cache-sets:
>>> +          const: 2048
>>> +
>>> +    else:
>>> +      properties:
>>> +        cache-sets:
>>> +          const: 1024
>>>      additionalProperties: false
>>>

Conor Dooley Aug. 25, 2022, 8:03 p.m. UTC | #4

On 25/08/2022 20:49, Heinrich Schuchardt wrote:
> On 8/25/22 20:56, [e-mail] wrote:
>> On 25/08/2022 19:36, Heinrich Schuchardt wrote:
>>> On 8/25/22 20:04, Conor Dooley wrote:
>>>> From: Conor Dooley <[e-mail]>

>>>> +allOf:
>>>> +  - $ref: /schemas/cache-controller.yaml#
>>>>    -then:
>>>> -  properties:
>>>> -    interrupts:
>>>> -      description: |
>>>> -        Must contain entries for DirError, DataError and DataFail signals.
>>>> -      maxItems: 3
>>>> -    cache-sets:
>>>> -      const: 1024
>>>> -
>>>> -else:
>>>> -  properties:
>>>> -    interrupts:
>>>> -      description: |
>>>> -        Must contain entries for DirError, DataError, DataFail, DirFail signals.
>>>> -      minItems: 4
>>>> -    cache-sets:
>>>> -      const: 2048
>>>> +  - if:
>>>> +      properties:
>>>> +        compatible:
>>>> +          contains:
>>>> +            enum:
>>>> +              - sifive,fu740-c000-ccache
>>>> +              - microchip,mpfs-ccache
>>>> +
>>>> +    then:
>>>> +      properties:
>>>> +        interrupts:
>>>> +          description: |
>>>> +            Must contain entries for DirError, DataError, DataFail, DirFail signals.
>>>> +          minItems: 4
> 
> Above you indicated that you want strict limits for the interrupt count.
> You expect exactly 4 items here. Having 5 entries would not be correct.
> Please, add 'maxItems: 4'.

Outside of this diff, because of how the particular binding was
structured, there is:
  interrupts:
    minItems: 3
    items:
      - description: DirError interrupt
      - description: DataError interrupt
      - description: DataFail interrupt
      - description: DirFail interrupt

AFAIU, "maxItems: 4" is redundant because all possible items are listed.

> 
>>>> +
>>>> +    else:
>>>> +      properties:
>>>> +        interrupts:
>>>> +          description: |
>>>> +            Must contain entries for DirError, DataError and DataFail signals.
>>>> +          maxItems: 3
> 
> The item count should be exactly 3. Having 2 entries would not be correct.
> Please, add 'minItems: 3'.

Again, this is set by the section I pasted above - although this time
explicitly.

Hope that explains things, not the easiest binding to understand from
a diff alone. Possibly I should have passed a "-U" argument while
creating the patches to get an easier-to-follow diff.

Thanks for your (prompt) reviews,
Conor.

>>>> +
>>>> +  - if:
>>>> +      properties:
>>>> +        compatible:
>>>> +          contains:
>>>> +            const: sifive,fu740-c000-ccache
>>>> +
>>>> +    then:
>>>> +      properties:
>>>> +        cache-sets:
>>>> +          const: 2048
>>>> +
>>>> +    else:
>>>> +      properties:
>>>> +        cache-sets:
>>>> +          const: 1024
>>>>      additionalProperties: false
>>>>

Rob Herring Aug. 30, 2022, 8:57 p.m. UTC | #5

On Thu, Aug 25, 2022 at 1:36 PM Heinrich Schuchardt
<[e-mail]> wrote:
>
> On 8/25/22 20:04, Conor Dooley wrote:
> > From: Conor Dooley <[e-mail]>
> >
> > The l2 cache on PolarFire SoC is cross between that of the fu540 and
> > the fu740. It has the extra interrupt from the fu740 but the lower
> > number of cache-sets. Add a specific compatible to avoid the likes
> > of:
> >
> > mpfs-polarberry.dtb: cache-controller@2010000: interrupts: [[1], [3], [4], [2]] is too long
>
> Where is such a message written? I couldn't find the string in
> next-20220825 (git grep -n 'is too long"').
>
> Why should a different number of cache sets require an extra compatible
> string. cache-size is simply a parameter going with the existing
> compatible strings.
>
> I would assume that you only need an extra compatible string if there is
> a functional difference that can not be expressed with the existing
> parameters.

Correct, but you have to account for unknown functional differences
aka errata as well. Otherwise, we need firmware updates to enable the
OS to handle errata.

> > Fixes: 34fc9cc3aebe ("riscv: dts: microchip: correct L2 cache interrupts")
> > Signed-off-by: Conor Dooley <[e-mail]>
> > ---
> >   .../bindings/riscv/sifive-l2-cache.yaml       | 79 ++++++++++++-------
> >   1 file changed, 49 insertions(+), 30 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/riscv/sifive-l2-cache.yaml b/Documentation/devicetree/bindings/riscv/sifive-l2-cache.yaml
> > index 69cdab18d629..ca3b9be58058 100644
> > --- a/Documentation/devicetree/bindings/riscv/sifive-l2-cache.yaml
> > +++ b/Documentation/devicetree/bindings/riscv/sifive-l2-cache.yaml
> > @@ -17,9 +17,6 @@ description:
> >     acts as directory-based coherency manager.
> >     All the properties in ePAPR/DeviceTree specification applies for this platform.
> >
> > -allOf:
> > -  - $ref: /schemas/cache-controller.yaml#
> > -
> >   select:
> >     properties:
> >       compatible:
> > @@ -33,11 +30,16 @@ select:
> >
> >   properties:
> >     compatible:
> > -    items:
> > -      - enum:
> > -          - sifive,fu540-c000-ccache
> > -          - sifive,fu740-c000-ccache
>
> Why can't you simply add microchip,mpfs-ccache here?
>
> > -      - const: cache
> > +    oneOf:
> > +      - items:
> > +          - enum:
> > +              - sifive,fu540-c000-ccache
> > +              - sifive,fu740-c000-ccache
> > +          - const: cache
> > +      - items:
> > +          - const: microchip,mpfs-ccache
> > +          - const: sifive,fu540-c000-ccache
>
> Why do we need 'sifive,fu540-c000-ccache' twice?

Because it is in 2 different positions. While we can express that the
last N entries in a list are optional, there is no way in json-schema
to express entries at the beginning or in the middle are optional.

Rob

Rob Herring Aug. 30, 2022, 9:59 p.m. UTC | #6

On Thu, 25 Aug 2022 19:04:17 +0100, Conor Dooley wrote:
> From: Conor Dooley <[e-mail]>
> 
> The l2 cache on PolarFire SoC is cross between that of the fu540 and
> the fu740. It has the extra interrupt from the fu740 but the lower
> number of cache-sets. Add a specific compatible to avoid the likes
> of:
> 
> mpfs-polarberry.dtb: cache-controller@2010000: interrupts: [[1], [3], [4], [2]] is too long
> 
> Fixes: 34fc9cc3aebe ("riscv: dts: microchip: correct L2 cache interrupts")
> Signed-off-by: Conor Dooley <[e-mail]>
> ---
>  .../bindings/riscv/sifive-l2-cache.yaml       | 79 ++++++++++++-------
>  1 file changed, 49 insertions(+), 30 deletions(-)
> 

Reviewed-by: Rob Herring <[e-mail]>

Patch

diff --git a/Documentation/devicetree/bindings/riscv/sifive-l2-cache.yaml b/Documentation/devicetree/bindings/riscv/sifive-l2-cache.yaml
index 69cdab18d629..ca3b9be58058 100644
--- a/Documentation/devicetree/bindings/riscv/sifive-l2-cache.yaml
+++ b/Documentation/devicetree/bindings/riscv/sifive-l2-cache.yaml
@@ -17,9 +17,6 @@  description:
   acts as directory-based coherency manager.
   All the properties in ePAPR/DeviceTree specification applies for this platform.
 
-allOf:
-  - $ref: /schemas/cache-controller.yaml#
-
 select:
   properties:
     compatible:
@@ -33,11 +30,16 @@  select:
 
 properties:
   compatible:
-    items:
-      - enum:
-          - sifive,fu540-c000-ccache
-          - sifive,fu740-c000-ccache
-      - const: cache
+    oneOf:
+      - items:
+          - enum:
+              - sifive,fu540-c000-ccache
+              - sifive,fu740-c000-ccache
+          - const: cache
+      - items:
+          - const: microchip,mpfs-ccache
+          - const: sifive,fu540-c000-ccache
+          - const: cache
 
   cache-block-size:
     const: 64
@@ -72,29 +74,46 @@  properties:
       The reference to the reserved-memory for the L2 Loosely Integrated Memory region.
       The reserved memory node should be defined as per the bindings in reserved-memory.txt.
 
-if:
-  properties:
-    compatible:
-      contains:
-        const: sifive,fu540-c000-ccache
+allOf:
+  - $ref: /schemas/cache-controller.yaml#
 
-then:
-  properties:
-    interrupts:
-      description: |
-        Must contain entries for DirError, DataError and DataFail signals.
-      maxItems: 3
-    cache-sets:
-      const: 1024
-
-else:
-  properties:
-    interrupts:
-      description: |
-        Must contain entries for DirError, DataError, DataFail, DirFail signals.
-      minItems: 4
-    cache-sets:
-      const: 2048
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - sifive,fu740-c000-ccache
+              - microchip,mpfs-ccache
+
+    then:
+      properties:
+        interrupts:
+          description: |
+            Must contain entries for DirError, DataError, DataFail, DirFail signals.
+          minItems: 4
+
+    else:
+      properties:
+        interrupts:
+          description: |
+            Must contain entries for DirError, DataError and DataFail signals.
+          maxItems: 3
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: sifive,fu740-c000-ccache
+
+    then:
+      properties:
+        cache-sets:
+          const: 2048
+
+    else:
+      properties:
+        cache-sets:
+          const: 1024
 
 additionalProperties: false