On 1/9/24 5:32 AM, Krzysztof Kozlowski wrote:
On 08/01/2024 19:32, Andrew Davis wrote:
The Imagination PowerVR Series5 "SGX" GPU is part of several SoCs from
multiple vendors. Describe how the SGX GPU is integrated in these SoC,
including register space and interrupts. Clocks, reset, and power domain
information is SoC specific.
Signed-off-by: Andrew Davis <afd@xxxxxx>
---
.../bindings/gpu/img,powervr-sgx.yaml | 124 ++++++++++++++++++
MAINTAINERS | 1 +
2 files changed, 125 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpu/img,powervr-sgx.yaml
diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-sgx.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-sgx.yaml
new file mode 100644
index 0000000000000..bb821e1184de9
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpu/img,powervr-sgx.yaml
@@ -0,0 +1,124 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (c) 2023 Imagination Technologies Ltd.
Your email has @TI domain, are you sure you attribute your copyrights to
Imagination?
The file started as a copy/paste from a IMG copyrighted file, even
though it is now almost completely re-written I've left their (c)
for good measure. I'll add an additional TI (c).
...
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ clocks: true
Missing min/maxItems
These are set in the allOf/if/then blocks below, seems
if I don't set them to at least something here then I get
a warning:
'clock-names', 'clocks' do not match any of the regexes: 'pinctrl-[0-9]+'
even if I define them in the allOf block below. I don't
know what the min/max should be until I check the compatible
in the allOf block.
+
+ clock-names:
+ minItems: 1
+ items:
+ - const: core
+ - const: mem
+ - const: sys
+
+ power-domains:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - interrupts
+
+additionalProperties: false
This goes after allOf: block.
ACK
+
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: ti,am6548-gpu
+ then:
+ required:
+ - power-domains
+ else:
+ properties:
+ power-domains: false
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - allwinner,sun6i-a31-gpu
+ - ingenic,jz4780-gpu
+ then:
+ allOf:
+ - if:
I don't understand why do you need to embed allOf inside another allOf.
The upper (outer) if:then: looks entirely useless.
It is so that both compatibles falls through to having
clock being required.
Logic in YAML always seems messy to me, here it is in pseudo C:
if (compatible == allwinner,sun6i-a31-gpu ||
compatible == ingenic,jz4780-gpu) {
if (compatible == allwinner,sun6i-a31-gpu)
clocks: ...
if (compatible == ingenic,jz4780-gpu)
clocks: ...
required:
- clocks
- clock-names
} else { /* disallow for all others */
properties:
clocks: false
clock-names: false
}
Now if I had an "else if" that didn't force the indention to keep
growing I would have used that. (does one exist?) I also cannot
simply add the clock properties only for the two compats need
them for the reasons above and so must add them unconditionally
before then explicitly disable them in a catch-all else path.
Andrew
+ properties:
+ compatible:
+ contains:
+ const: allwinner,sun6i-a31-gpu
+ then:
+ properties:
+ clocks:
+ minItems: 2
+ maxItems: 2
+ clock-names:
+ minItems: 2
+ maxItems: 2
Best regards,
Krzysztof
