On 04/10/21 12:09PM, Rob Herring wrote: > On Fri, Sep 24, 2021 at 11:37:05PM +0530, Pratyush Yadav wrote: > > Many SPI controllers need to add properties to slave devices. This could > > be the delay in clock or data lines, etc. These properties are > > controller specific but need to be defined in the slave node because > > they are per-slave and there can be multiple slaves attached to a > > controller. > > > > If these properties are not added to the slave binding, then the dtbs > > check emits a warning. But these properties do not make much sense in > > the slave binding because they are controller-specific and they will > > just pollute every slave binding. > > > > One option is to add a separate schema that collects all such properties > > from all such controllers. Slave bindings can simply refer to this > > binding and they should be rid of the warnings. > > > > There are some limitations with this approach: > > > > 1. There is no way to specify required properties. The schema would > > contain properties for all controllers and there would be no way to know > > which controller is being used. > > > > 2. There would be no way to restrict additional properties. Since this > > schema will be used with an allOf operator, additionalProperties would > > need to be true. In addition, the slave schema will have to set > > unevaluatedProperties: false. > > I don't see what is the problem. If there's a $ref, then > unevaluatedProperties is what should be used. > > > > > A much simpler option would be to make controller schemas specify those > > properties in patternProperties and set unevaluatedProperties to false > > on slave schemas, which is done in the previous approach anyway. This > > approach would have the same limitations as the 2nd limitation in the > > previous approach. But it does not have the 1st limitation since the > > properties of all controllers are not collected in a single schema, but > > instead reside in the same schema as the controller. It also has the > > added benefit of being much simpler. > > > > The SPI NOR binding is taken as the first one to follow this. Other > > bindings like SPI NAND can follow in later patches. > > > > Signed-off-by: Pratyush Yadav <p.yadav@xxxxxx> > > > > --- > > I sent the first approach mentioned in the commit message some time ago > > [0]. When re-rolling this series I realized that if we are going to use > > unevaluatedProperties: false, then it would be much simpler to just keep > > everything else as-is. This has clear positives with no negatives > > relative to [0], as explained in the commit message. > > > > [0] https://lore.kernel.org/all/20210609111707.9555-1-p.yadav@xxxxxx/T/#u > > > > 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 only works until unevaluatedProperties support is actually > implemented. Then it's back to the same warnings. In the mean time, we'd > be allowing any extra random properties to be added for everyone. Ok, I didn't know that. I don't understand the validation frameworks all that well. I will go back to the method you suggested. Thanks. > > Rob -- Regards, Pratyush Yadav Texas Instruments Inc.
