[4/4] dt-bindings: cache: Add docs for StarFive StarLink-500 cache controller

Commit Message

Joshua Yeong March 14, 2024, 6:12 a.m. UTC

Add DT binding documentation used by StarFive's
Starlink-500 cache controller.

Signed-off-by: Joshua Yeong <[e-mail]>
---
 .../cache/starfive,starlink-500-cache.yaml    | 62 +++++++++++++++++++
 1 file changed, 62 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/cache/starfive,starlink-500-cache.yaml

Comments

Rob Herring (Arm) March 15, 2024, 4:36 p.m. UTC | #1

On Thu, Mar 14, 2024 at 02:12:05PM +0800, Joshua Yeong wrote:
> Add DT binding documentation used by StarFive's
> Starlink-500 cache controller.
> 
> Signed-off-by: Joshua Yeong <[e-mail]>
> ---
>  .../cache/starfive,starlink-500-cache.yaml    | 62 +++++++++++++++++++
>  1 file changed, 62 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/cache/starfive,starlink-500-cache.yaml
> 
> diff --git a/Documentation/devicetree/bindings/cache/starfive,starlink-500-cache.yaml b/Documentation/devicetree/bindings/cache/starfive,starlink-500-cache.yaml
> new file mode 100644
> index 000000000000..97ddf7db39e9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/cache/starfive,starlink-500-cache.yaml
> @@ -0,0 +1,62 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/cache/starfive,starlink-500-cache.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: StarFive StarLink-500 Cache Controller
> +
> +maintainers:
> +  - Joshua Yeong <[e-mail]>
> +
> +description:
> +  StarFive's StarLink-500 controller manages cache shared between clusters of
> +  CPU cores. The cache driver provides mechanism to perform invalidate and write
> +  back functionality to the shared memory.

What a driver does is not relevant to the binding. You could describe 
what the registers control though.

> +
> +allOf:
> +  - $ref: /schemas/cache-controller.yaml#
> +
> +# We need a select here so we don't match all nodes with 'cache'
> +select:
> +  properties:
> +    compatible:
> +      contains:
> +        enum:
> +          - starfive,starlink-500-cache
> +
> +  required:
> +    - compatible
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: starfive,starlink-500-cache
> +      - const: cache
> +
> +  reg:
> +    maxItems: 1
> +
> +  cache-size: true
> +  cache-sets: true
> +  cache-block-size: true
> +  cache-level: true
> +  cache-unified: true

Drop these and...

> +
> +additionalProperties: false

... use 'unevaluatedProperties' instead.

> +
> +required:
> +  - compatible
> +  - reg

cache-unified should be required as I imagine the cache is always 
unified. Perhaps other properties too (cache-level already is)?

> +
> +examples:
> +  - |
> +      cache-controller@15000000 {
> +        compatible = "starfive,starlink-500-cache", "cache";
> +        reg = <0x15000000 0x278>;
> +        cache-block-size = <64>;
> +        cache-level = <3>;
> +        cache-sets = <8192>;
> +        cache-size = <0x400000>;
> +        cache-unified;
> +      };
> -- 
> 2.25.1
>

Conor Dooley March 17, 2024, 2:58 p.m. UTC | #2

On Fri, Mar 15, 2024 at 10:36:07AM -0600, Rob Herring wrote:
> On Thu, Mar 14, 2024 at 02:12:05PM +0800, Joshua Yeong wrote:
> > Add DT binding documentation used by StarFive's
> > Starlink-500 cache controller.
> > 
> > Signed-off-by: Joshua Yeong <[e-mail]>
> > ---
> >  .../cache/starfive,starlink-500-cache.yaml    | 62 +++++++++++++++++++
> >  1 file changed, 62 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/cache/starfive,starlink-500-cache.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/cache/starfive,starlink-500-cache.yaml b/Documentation/devicetree/bindings/cache/starfive,starlink-500-cache.yaml
> > new file mode 100644
> > index 000000000000..97ddf7db39e9
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/cache/starfive,starlink-500-cache.yaml
> > @@ -0,0 +1,62 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/cache/starfive,starlink-500-cache.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: StarFive StarLink-500 Cache Controller
> > +
> > +maintainers:
> > +  - Joshua Yeong <[e-mail]>
> > +
> > +description:
> > +  StarFive's StarLink-500 controller manages cache shared between clusters of
> > +  CPU cores. The cache driver provides mechanism to perform invalidate and write
> > +  back functionality to the shared memory.
> 
> What a driver does is not relevant to the binding. You could describe 
> what the registers control though.
> 
> > +
> > +allOf:
> > +  - $ref: /schemas/cache-controller.yaml#
> > +
> > +# We need a select here so we don't match all nodes with 'cache'
> > +select:
> > +  properties:
> > +    compatible:
> > +      contains:
> > +        enum:
> > +          - starfive,starlink-500-cache
> > +
> > +  required:
> > +    - compatible
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - const: starfive,starlink-500-cache
> > +      - const: cache

You're also missing a soc-specific compatible here as far as I can tell,
as one of the other patches in this series specifically calls out the
JH8100 as where this cache controller is. AFAICT, "starlink-500" is the
name of the IP or of the coreplex - it's not the name of the SoC.

> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  cache-size: true
> > +  cache-sets: true
> > +  cache-block-size: true
> > +  cache-level: true
> > +  cache-unified: true
> 
> Drop these and...
> 
> > +
> > +additionalProperties: false
> 
> ... use 'unevaluatedProperties' instead.
> 
> > +
> > +required:
> > +  - compatible
> > +  - reg
> 
> cache-unified should be required as I imagine the cache is always 
> unified. Perhaps other properties too (cache-level already is)?

And I also suspect that a load of the values can be restricted to
specific values for that jh8100 compatible.

> 
> > +
> > +examples:
> > +  - |
> > +      cache-controller@15000000 {
> > +        compatible = "starfive,starlink-500-cache", "cache";
> > +        reg = <0x15000000 0x278>;
> > +        cache-block-size = <64>;
> > +        cache-level = <3>;
> > +        cache-sets = <8192>;
> > +        cache-size = <0x400000>;
> > +        cache-unified;
> > +      };
> > -- 
> > 2.25.1
> >

Patch

diff --git a/Documentation/devicetree/bindings/cache/starfive,starlink-500-cache.yaml b/Documentation/devicetree/bindings/cache/starfive,starlink-500-cache.yaml
new file mode 100644
index 000000000000..97ddf7db39e9
--- /dev/null
+++ b/Documentation/devicetree/bindings/cache/starfive,starlink-500-cache.yaml
@@ -0,0 +1,62 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/cache/starfive,starlink-500-cache.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: StarFive StarLink-500 Cache Controller
+
+maintainers:
+  - Joshua Yeong <[e-mail]>
+
+description:
+  StarFive's StarLink-500 controller manages cache shared between clusters of
+  CPU cores. The cache driver provides mechanism to perform invalidate and write
+  back functionality to the shared memory.
+
+allOf:
+  - $ref: /schemas/cache-controller.yaml#
+
+# We need a select here so we don't match all nodes with 'cache'
+select:
+  properties:
+    compatible:
+      contains:
+        enum:
+          - starfive,starlink-500-cache
+
+  required:
+    - compatible
+
+properties:
+  compatible:
+    items:
+      - const: starfive,starlink-500-cache
+      - const: cache
+
+  reg:
+    maxItems: 1
+
+  cache-size: true
+  cache-sets: true
+  cache-block-size: true
+  cache-level: true
+  cache-unified: true
+
+additionalProperties: false
+
+required:
+  - compatible
+  - reg
+
+examples:
+  - |
+      cache-controller@15000000 {
+        compatible = "starfive,starlink-500-cache", "cache";
+        reg = <0x15000000 0x278>;
+        cache-block-size = <64>;
+        cache-level = <3>;
+        cache-sets = <8192>;
+        cache-size = <0x400000>;
+        cache-unified;
+      };