On Thu, May 27, 2021 at 11:33:23AM +0100, Cristian Marussi wrote:

Hi Sudeep,

Some feedback down below.

On Wed, May 26, 2021 at 07:28:07PM +0100, Sudeep Holla wrote:

[...]

quoted

+patternProperties:
+  '^protocol@[0-9a-f]+$':
+    type: object
+    description: |
+      Each sub-node represents a protocol supported. If the platform
+      supports dedicated communication channel for a particular protocol,
+      then corresponding transport properties must be present.
+

Not sure if it's needed, but maybe a reference or an example to which
are the transport properties could be useful in this description.

Good point, I will try to add that example.

quoted

+    properties:
+      reg:
+        maxItems: 1
+

Shouldn't be expressed that reg is required for these protocol
patternProperties ? (no sure how though...:D)

Hmm, right again need to explore on that.

quoted

+      '#clock-cells':
+        const: 1
+
+      '#reset-cells':
+        const: 1
+
+      '#power-domain-cells':
+        const: 1
+
+      '#thermal-sensor-cells':
+        const: 1
+

Maybe it does not matter, but all the info present in the old .txt binding
about references to external std bindings like:

quoted

-This binding for the SCMI power domain providers uses the generic
power
-domain binding[2].

is no more reported in yaml. Is it fine ?

I think we can add it as $ref if there is yaml schema, I really don't
think old .txt based reference adds anything.

quoted

+required:
+  - compatible
+  - shmem

Indeed shmem is required by chance all the transports defined in this
binding, but it is not really something generally required, in fact
virtio transport won't require it.

But I'm not sure if it's better to move it now down under some kind of
if: arm,scmi|arm,scmi-smc or just do it later when virtio transport binding
will be defined/introduced.

Yes I was aware of that fact while I wrote this and expect it to be part of
virtio update.

--
Regards,
Sudeep