linux-kernel - Re: [PATCH 1/3] dt-bindings: hsi: hsi-client: convert to YAML

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5z2b74h2zvo7fwc4624hy5vegvlkyfdflijr35byqeemoiwd6l@vfebii4m2mim>
Date: Tue, 26 Mar 2024 16:15:03 +0100
From: Sebastian Reichel <sebastian.reichel@...labora.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc: Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>, Conor Dooley <conor+dt@...nel.org>, 
	Tony Lindgren <tony@...mide.com>, devicetree@...r.kernel.org, linux-omap@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] dt-bindings: hsi: hsi-client: convert to YAML

Hi,

On Tue, Mar 26, 2024 at 01:56:22PM +0100, Krzysztof Kozlowski wrote:
> >>> +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
> 
> Not entirely. anyOf should succeed also when none of them are present,
> which is not what you want in such case.

Right, this should be oneOf instead of anyOf. I fixed that for v2.

> > 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.
> 
> Then wouldn't this work for you:
> https://elixir.bootlin.com/linux/v5.17-rc2/source/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml#L91

I suppose you mean using "then: not: required: PROPERTY" instead of
"then: PROPERTY: false"? The variant using "PROPERTY: false" is what
is being used in example-schema.yaml:

https://elixir.bootlin.com/linux/v6.8/source/Documentation/devicetree/bindings/example-schema.yaml#L225

IMHO the "not: required: property" is harder to understand. I would
expect that to mean "the property is not required (i.e. optional)"
instead of "the property is not allowed".

-- Sebastian

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)