diff mbox series

[v2,2/5] dt-bindings: mtd: spi-nor: Allow external properties

Message ID 20211126163450.394861-3-miquel.raynal@bootlin.com (mailing list archive)
State Superseded
Headers show
Series Stacked/parallel memories bindings | expand

Commit Message

Miquel Raynal Nov. 26, 2021, 4:34 p.m. UTC
Setting "additionalProperties: false" will refuse any generic SPI
property while they should be of course authorized. In practice it looks
like many people used compatibles different than "jedec,spi-nor" in
order to workaround this limitation because otherwise no SPI property
could be used in the examples. Use "unevaluatedProperties: false"
instead to allow defined properties to be used. It is likely that at the
time of the conversion to yaml of the jedec file, the unevaluated
keyword was not yet introduced.

Fixes: 3ff9ee2a8890 ("dt-bindings: mtd: spi-nor: Convert to DT schema format")
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Rob Herring Dec. 2, 2021, 12:05 a.m. UTC | #1
On Fri, Nov 26, 2021 at 05:34:47PM +0100, Miquel Raynal wrote:
> Setting "additionalProperties: false" will refuse any generic SPI
> property while they should be of course authorized. In practice it looks
> like many people used compatibles different than "jedec,spi-nor" in
> order to workaround this limitation because otherwise no SPI property
> could be used in the examples. Use "unevaluatedProperties: false"
> instead to allow defined properties to be used. It is likely that at the
> time of the conversion to yaml of the jedec file, the unevaluated
> keyword was not yet introduced.
> 
> Fixes: 3ff9ee2a8890 ("dt-bindings: mtd: spi-nor: Convert to DT schema format")
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> index ed590d7c6e37..81be0620b264 100644
> --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> @@ -88,7 +88,7 @@ patternProperties:
>    "^otp(-[0-9]+)?$":
>      type: object
>  
> -additionalProperties: false
> +unevaluatedProperties: false

This has no effect unless you have referenced some other schema here. 
The series I referenced will solve what your trying to solve I think.

Rob
Miquel Raynal Dec. 2, 2021, 7:25 a.m. UTC | #2
Hi Rob,

robh@kernel.org wrote on Wed, 1 Dec 2021 18:05:41 -0600:

> On Fri, Nov 26, 2021 at 05:34:47PM +0100, Miquel Raynal wrote:
> > Setting "additionalProperties: false" will refuse any generic SPI
> > property while they should be of course authorized. In practice it looks
> > like many people used compatibles different than "jedec,spi-nor" in
> > order to workaround this limitation because otherwise no SPI property
> > could be used in the examples. Use "unevaluatedProperties: false"
> > instead to allow defined properties to be used. It is likely that at the
> > time of the conversion to yaml of the jedec file, the unevaluated
> > keyword was not yet introduced.
> > 
> > Fixes: 3ff9ee2a8890 ("dt-bindings: mtd: spi-nor: Convert to DT schema format")
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> > index ed590d7c6e37..81be0620b264 100644
> > --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> > +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> > @@ -88,7 +88,7 @@ patternProperties:
> >    "^otp(-[0-9]+)?$":
> >      type: object
> >  
> > -additionalProperties: false
> > +unevaluatedProperties: false  
> 
> This has no effect unless you have referenced some other schema here. 
> The series I referenced will solve what your trying to solve I think.

Maybe this is not the right fix indeed, but my understanding of json
reaches its bounds here.

Without this change, any example in this file that references a
spi-controller.yaml property (which is correctly defined) will throw an
error. The fact is, all the examples out there with a spi-nor flash
using the jedec,spi-nor compatible *cannot* contain any
spi-controller.yaml property, otherwise the tooling errors out. This is
a real issue.

I will give Pratyush's series a try.

Thanks,
Miquèl
Rob Herring Dec. 2, 2021, 3:51 p.m. UTC | #3
On Thu, Dec 2, 2021 at 1:25 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Rob,
>
> robh@kernel.org wrote on Wed, 1 Dec 2021 18:05:41 -0600:
>
> > On Fri, Nov 26, 2021 at 05:34:47PM +0100, Miquel Raynal wrote:
> > > Setting "additionalProperties: false" will refuse any generic SPI
> > > property while they should be of course authorized. In practice it looks
> > > like many people used compatibles different than "jedec,spi-nor" in
> > > order to workaround this limitation because otherwise no SPI property
> > > could be used in the examples. Use "unevaluatedProperties: false"
> > > instead to allow defined properties to be used. It is likely that at the
> > > time of the conversion to yaml of the jedec file, the unevaluated
> > > keyword was not yet introduced.
> > >
> > > Fixes: 3ff9ee2a8890 ("dt-bindings: mtd: spi-nor: Convert to DT schema format")
> > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > ---
> > >  Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> > > index ed590d7c6e37..81be0620b264 100644
> > > --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> > > +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> > > @@ -88,7 +88,7 @@ patternProperties:
> > >    "^otp(-[0-9]+)?$":
> > >      type: object
> > >
> > > -additionalProperties: false
> > > +unevaluatedProperties: false
> >
> > This has no effect unless you have referenced some other schema here.
> > The series I referenced will solve what your trying to solve I think.
>
> Maybe this is not the right fix indeed, but my understanding of json
> reaches its bounds here.
>
> Without this change, any example in this file that references a
> spi-controller.yaml property (which is correctly defined) will throw an
> error. The fact is, all the examples out there with a spi-nor flash
> using the jedec,spi-nor compatible *cannot* contain any
> spi-controller.yaml property, otherwise the tooling errors out. This is
> a real issue.

That's probably because unevaluatedProperties is unimplemented in the
last dtschema release, but now supported in main branch. I'll be
tagging a release soon. So once the newer version is used, you should
be back to the same error.

> I will give Pratyush's series a try.

Referencing spi-peripheral.yaml with 'unevaluatedProperties: false'
should give you what you need.

Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
index ed590d7c6e37..81be0620b264 100644
--- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
+++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
@@ -88,7 +88,7 @@  patternProperties:
   "^otp(-[0-9]+)?$":
     type: object
 
-additionalProperties: false
+unevaluatedProperties: false
 
 examples:
   - |