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: [...] > > +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. > > + 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. > > + '#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: > > > -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. > > +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
