Re: [PATCH v3 05/11] spi: dw: Add Pensando Elba SoC SPI Controller bindings
From: Serge Semin <hidden>
Date: 2021-11-17 08:19:32
Also in:
linux-devicetree, linux-gpio, linux-mmc, linux-spi, lkml
On Tue, Nov 16, 2021 at 03:11:53PM -0800, Brad Larson wrote:
quoted hunk ↗ jump to hunk
Hi Sergey, On Tue, Nov 16, 2021 at 3:29 AM Serge Semin [off-list ref] wrote:quoted
quoted
quoted
AFAICS from the driver-part of the patchset it's not enough. You've also got the syscon phandle, which needs to be reflected in the bindings. That also makes me thinking that you didn't perform the "dtbs_check" on the dts-files you were going to submit, but for some reason discarded from this series (btw why?). If you did you would have got an error of an unevaluated property detection.I ran the checks below and didn't get errors. Rob provided some info and I found the server did not have yamllint installed (not flagged by tool). Also dt-schema was not the latest. I'm re-doing this and including "DT_CHECKER_FLAGS=-m" as that is new with v5.13.quoted
make ARCH=arm64 dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml make ARCH=arm64 dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml make ARCH=arm64 dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml make ARCH=arm64 dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/vendor-prefixes.yaml make ARCH=arm64 dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/arm/pensando,elba.yamlHmm, that's weird. Rob, does dtschema tool have the "unevaluatedProperties" property support? Brad, anyway you still need to add the syscon-property (pensando,*spics) requirement in the snps,dw-apb-ssi.yaml schema. See the way it's done there for instance for "baikal,bt1-sys-ssi" when it comes to the vendor-specific properties definition in the allOf composition block. You'll need to define a custom phandle property there in case if a DT-node is compatible with you SPI controller.Updating and adding only this bindings update to file snps,dw-apb-ssi.yaml in 5.16.0-rc1 (next-20211116):diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yamlb/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml index d7e08b03e204..99deb587a47b 100644--- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml@@ -37,6 +37,21 @@ allOf: else: required: - interrupts + - if: + properties:
+ compatible: + contains: + enum: + - pensando,elba-spics
I was wrong using that construction here (fixup patch would be very welcome) seeing the "snps,dw-apb-ssi" doesn't permit having a generic "snps,dw*" compatible string. So just const-compatible property should be enough: + compatible: + const: pensando,elba-spics
+ then: + properties: + pensando,spics: + $ref: /schemas/types.yaml#/definitions/phandle + description: + Phandle to the system control device node which provides access to + the spics control register + required:
+ - pensando,spics
Please note, I've asked to be more specific in this property naming. Something like this should be fine "pensando,elba-syscon-spics"/"pensando,syscon-spics".
quoted hunk ↗ jump to hunk
properties: compatible:@@ -73,6 +88,8 @@ properties: - renesas,r9a06g032-spi # RZ/N1D - renesas,r9a06g033-spi # RZ/N1S - const: renesas,rzn1-spi # RZ/N1 + - description: Pensando Elba SoC SPI Controller + const: pensando,elba-spics reg: minItems: 1$ make ARCH=arm64 defconfig ...
$ make DT_CHECKER_FLAGS=-m dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
I am kind of surprised that this command is still evaluating all the schemas. Compiling only the depended DT-schemas would much better... Anyway that's why you are getting unrelated to the snps,dw-apb-ssi.yaml errors.
... DTEX Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.example.dts DTC Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.example.dt.yaml CHECK Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.example.dt.yaml Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.example.dt.yaml:0:0: /example-0/spi@fff00000/flash@1: failed to match any schema with compatible: ['spi-nand']
That means there is no bindings defined for the "spi-nand"-compatible node. AFAICS it doesn't make the tool to fail though. Anyway fixing this part would be a subject of additional patches (which would be very appreciated). That would concern converting the Documentation/devicetree/bindings/mtd/spi-nand.txt legacy bindings to the DT-schema. It's not enough though. Seeing that file lacks of the NAND Flash specific DT-property description, you'd need to detach ones (described by the "^nand@[a-f0-9]$"-pattern property) from the Documentation/devicetree/bindings/mtd/nand-controller.yaml schema and place them into a separate DT-schema file for generic nand-flashes Documentation/devicetree/bindings/mtd/nand-flash.yaml in a framework of a pre-requisite patch. Than in the legacy bindings conversion patch you'd need to use it to correctly evaluate a generic NAND flash node. Adding some example properties to the DT-schema would be also required. -Sergey
The spi-nand schema match failure happens before I make any change. The tool also throws errors for these files which are unrelated Documentation/devicetree/bindings/net/qcom,ipa.yaml: ignoring, error in schema: properties: qcom,smem-state-names Documentation/devicetree/bindings/iio/adc/st,stm32-dfsdm-adc.yaml: ignoring, error in schema: patternProperties: ^filter@[0-9]+$: properties: st,adc-channel-names Documentation/devicetree/bindings/interconnect/qcom,rpmh.yaml: ignoring, error in schema: properties: qcom,bcm-voter-names Thanks, Brad
_______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel