On Sat, May 21, 2022 at 12:22:48PM +0300, Serge Semin wrote: > On Tue, May 17, 2022 at 01:58:41PM -0500, Rob Herring wrote: > > On Thu, May 12, 2022 at 02:17:48AM +0300, Serge Semin wrote: > > > It's redundant to have the 'dma-coherent' property explicitly specified in > > > the DT schema because it's a generic property described in the core > > > DT-schema by which the property is always evaluated. > > > > > It is not redundant. > > > > The core schema defines the property (as a boolean), but this schema > > defines it being used in this binding. Otherwise, it won't be allowed. > > I thought that the generic properties like ranges, dma-ranges, etc > including the dma-coherent one due to being defined in the dt-core > schema are always evaluated. As such seeing the unevaluatedProperties > property is set to false here, they can be used in the DT-nodes with > no need to be explicitly specified in the DT node bindings. In > addition to that I tested this assumption by dropping the dma-coherent > property definition from the AHCI-common schema and executed the > DT-bindings check procedure. No error has been spotted: Those common properties are always applied, but not at the same time as a device binding. IOW, it's 2 schemas that are applied to an instance (node) independently. For things like 'reg', the common schema does type checks and the device schema does size (number of entries) checks. There a few things always allowed like 'status', and those are added to the device schema by the tools. > > > [fancer@mobilestation] kernel $ cat Documentation/devicetree/bindings/ata/snps,dwc-ahci.yaml | grep dma-coherent > > dma-coherent; > > [fancer@mobilestation] kernel $ make -j8 DT_SCHEMA_FILES=Documentation/devicetree/bindings/ata/snps,dwc-ahci.yaml dt_binding_check > > LINT Documentation/devicetree/bindings > > DTEX Documentation/devicetree/bindings/ata/snps,dwc-ahci.example.dts > > CHKDT Documentation/devicetree/bindings/processed-schema.json > > SCHEMA Documentation/devicetree/bindings/processed-schema.json > > DTC Documentation/devicetree/bindings/ata/snps,dwc-ahci.example.dtb > > CHECK Documentation/devicetree/bindings/ata/snps,dwc-ahci.example.dtb > > [fancer@mobilestation] kernel $ cat Documentation/devicetree/bindings/ata/snps,dwc-ahci.example.dts | grep dma-coherent > > dma-coherent; > > [fancer@mobilestation] kernel $ echo $? > > 0 > Due to that here are a few backward questions: > 1) Am I doing something wrong in the framework of the DT-bindings > evaluation? Really I even tried to specify unknown property in the > DT-bindings example like "bla-bla-bla;" and no evaluation error was > printed. Anyway If what you are saying was correct I would have got an > error during the DT-bindings evaluation, but as you can see there was > none. I think this is a known issue which has a pending fix. If a referenced schema has 'additionalProperties: true' in it, then the referring schema never has any unevaluated properties. The fix is pending because all the schema examples that start failing have to be fixed and in a base that people work on (i.e. rc1). > 2) Am I wrong in thinking that the unevaluatedProperties setting > concerns the generic properties defined in the DT-core schema? You are wrong as explained above. > If it > doesn't concern the generic properties then does it work for the > $ref'ed schemas only? Yes, except for the issue making it not work. > Getting back to the patch topic. We need to drop the dma-coherent > property from the schema anyway. AHCI-specification doesn't > regulate the DMA operations coherency. The dma-coherent property is > more specific to the particular controller implementation mainly > dependent on the platform settings. So I'll change the patch log, but > get to keep the patch in the series. What do you think? Intel wrote the spec, so they probably assume coherent. In DT, PPC is default coherent and Arm is default non-coherent. You'll need to add it to whatever specific device schemas need it if you remove it. Personally, I think it is fine where it is. dma-coherent is valid on any DMA capable device and it's not really a property of the device, but the system. If we could generically identify DMA capable devices, then dma-coherent would be allowed on them automatically. Rob
