Hi, On Tue, Mar 26, 2024 at 08:18:39AM +0100, Krzysztof Kozlowski wrote: > On 25/03/2024 22:45, Sebastian Reichel wrote: > > Convert the legacy txt binding to modern YAML and rename from > > client-devices to hsi-client. No semantic change. > > There is semantic change: missing example (which is reasonable for > shared schema) Right, I should have mentioned that. > but more importantly: some properties are now excluding each > other. I think that requirement was already there. > > Signed-off-by: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx> > > --- > > ... > > > diff --git a/Documentation/devicetree/bindings/hsi/hsi-client.yaml b/Documentation/devicetree/bindings/hsi/hsi-client.yaml > > new file mode 100644 > > index 000000000000..df6e1fdd2702 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/hsi/hsi-client.yaml > > @@ -0,0 +1,84 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/hsi/hsi-client.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: HSI bus peripheral > > + > > +description: > > + Each HSI port is supposed to have one child node, which > > + symbols the remote device connected to the HSI port. > > + > > +maintainers: > > + - Sebastian Reichel <sre@xxxxxxxxxx> > > + > > +properties: > > + $nodename: > > + const: hsi-client > > Why? Does anything depend on this? It breaks generic-node-name rule. It > seems you need it only to match the schema, but this just point to main > problem - missing bus schema. Ah, that's a good point. It makes a lot more sense to get the nodename from the actual client. I will work this over. > > + > > + hsi-channel-ids: > > + $ref: /schemas/types.yaml#/definitions/uint32-array > > + minItems: 1 > > + maxItems: 8 > > + > > + hsi-channel-names: > > + minItems: 1 > > + maxItems: 8 > > + > > + hsi-rx-mode: > > + enum: [stream, frame] > > + description: Receiver Bit transmission mode > > + > > + hsi-tx-mode: > > + enum: [stream, frame] > > + description: Transmitter Bit transmission mode > > + > > + hsi-mode: > > + enum: [stream, frame] > > + description: > > + May be used instead hsi-rx-mode and hsi-tx-mode if the > > + transmission mode is the same for receiver and transmitter. > > + > > + hsi-speed-kbps: > > + description: Max bit transmission speed in kbit/s > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + > > + hsi-flow: > > + enum: [synchronized, pipeline] > > + description: RX flow type > > + > > + hsi-arb-mode: > > + enum: [round-robin, priority] > > + description: Arbitration mode for TX frame > > + > > +additionalProperties: true > > + > > +required: > > + - compatible > > + - hsi-channel-ids > > + - hsi-speed-kbps > > + - hsi-flow > > + - hsi-arb-mode > > + > > +anyOf: > > + - required: > > + - hsi-mode > > + - required: > > + - hsi-rx-mode > > + - hsi-tx-mode > > + > > +allOf: > > + - if: > > + required: > > + - hsi-mode > > + then: > > + properties: > > + hsi-rx-mode: false > > + hsi-tx-mode: false > > I don't understand what you are trying to achieve here and with anyOf. > It looks like just oneOf. OTOH, old binding did not exclude these > properties. So the anyOf ensures, that either hsi-mode or hsi-rx-mode + hsi-tx-mode are specified. Those properties were previously listed as required and they are indeed mandatory by the Linux kernel implementation. The old binding also has this: hsi-mode: May be used ***instead*** hsi-rx-mode and hsi-tx-mode So it's either hsi-rx-mode + hsi-tx-mode OR hsi-mode, but not all properties at the same time. That's what the allOf ensures: if hsi-mode is specified, then hsi-rx-mode and hsi-tx-mode may not be specified. > > + - if: > > + required: > > + - hsi-rx-mode > > + then: > > + properties: > > + hsi-mode: false > > Thanks for the review, -- Sebastian
Attachment:
signature.asc
Description: PGP signature
