On Tue, May 2, 2017 at 6:23 AM, Guillaume Tucker
[off-list ref] wrote:

Hi Rob,

On 28/04/17 20:27, Rob Herring wrote:

quoted

On Tue, Apr 25, 2017 at 02:16:16PM +0100, Guillaume Tucker wrote:

quoted

quoted

diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt

b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt
new file mode 100644
index 000000000000..547ddeceb498

--- /dev/null
+++ b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt

@@ -0,0 +1,82 @@
+ARM Mali Midgard GPU
+====================
+
+Required properties:
+
+- compatible :
+  * Must be one of the following:
+    + "arm,mali-t60x"
+    + "arm,mali-t62x"

Don't use wildcards.

Sure, old habits die hard...  I'll fix it in patch v5.

quoted

quoted

+    + "arm,mali-t720"
+    + "arm,mali-t760"
+    + "arm,mali-t820"
+    + "arm,mali-t830"
+    + "arm,mali-t860"
+    + "arm,mali-t880"
+  * And, optionally, one of the vendor specific compatible:

IMO, these should not be optional.

Well, vendor compatible strings are clearly optional for the
Utgard GPU series for which the bindings docs were recently
merged.  It seems that whether these should be optional or not,
the documentation should be consistent between at least all
similar types of devices like Midgard and Utgard GPUs.  They have
different architectures but from a device tree point of view,
they both have the same kind of SoC-specific integration (clocks,
irqs, regulators...).

Clocks should not vary by SoC. There is often variation because clocks
get driven by same source or are not s/w controlled, but really there
should not be that variation. I noticed Utgard has 2 clocks. So is
Midgard really just 1 clock? The DT should have all the clocks listed
in the TRMs.

So was this was overlooked in the Utgard case and should it
ideally be fixed there as well as non-optional?  Or, is it OK to
keep these optional on a second thought?

Probably should be required in the Utgard case as well.

Rob