| Message ID | 20230522125129.526604-2-festevam@gmail.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [v5,1/4] dt-bindings: display: bridge: ldb: Adjust imx6sx entries | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dt-meta-schema | fail | build log |
On 5/22/23 14:51, Fabio Estevam wrote: > From: Fabio Estevam <festevam@denx.de> > > The i.MX6SX General Purpose Registers is a set of register that serves > various different purposes and in particular, IOMUXC_GPR_GPR6, at > offset 0x18, can be used to configure the LDB block. > > Signed-off-by: Fabio Estevam <festevam@denx.de> > --- > Changes since v4: > - Renamed to syscon@20e4000 (Conor). > > .../bindings/soc/imx/fsl,imx6sx-gpr.yaml | 84 +++++++++++++++++++ > 1 file changed, 84 insertions(+) > create mode 100644 Documentation/devicetree/bindings/soc/imx/fsl,imx6sx-gpr.yaml > > diff --git a/Documentation/devicetree/bindings/soc/imx/fsl,imx6sx-gpr.yaml b/Documentation/devicetree/bindings/soc/imx/fsl,imx6sx-gpr.yaml > new file mode 100644 > index 000000000000..22777ecfb56b > --- /dev/null > +++ b/Documentation/devicetree/bindings/soc/imx/fsl,imx6sx-gpr.yaml > @@ -0,0 +1,84 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/soc/imx/fsl,imx6sx-gpr.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: NXP i.MX6SX General Purpose Register > + > +maintainers: > + - Fabio Estevam <festevam@denx.de> > + > +description: > + The i.MX6SX General Purpose Registers is a set of register that serves > + for various purposes and in particular, IOMUXC_GPR_GPR6, at offset 0x18, > + can be used to configure the LDB block. > + > +properties: > + compatible: > + items: > + - const: fsl,imx6sx-iomuxc-gpr > + - const: fsl,imx6q-iomuxc-gpr > + - const: syscon Take a look at MX6Q and notice how the iomuxc and GPR register sets share the same base address . That's different on MX6SX where they are separate. So I think this binding should be specific to MX6SX ONLY and for MX6Q the subnode probing should be handled in the IOMUXC driver instead , i.e. drop the fsl,imx6q-iomuxc-gpr here and in imx6sx.dtsi . $ git grep -A 2 @20e0000 arch/arm/boot/dts/imx6qdl.dtsi arch/arm/boot/dts/imx6qdl.dtsi: gpr: iomuxc-gpr@20e0000 { arch/arm/boot/dts/imx6qdl.dtsi- compatible = "fsl,imx6q-iomuxc-gpr", "syscon", "simple-mfd"; arch/arm/boot/dts/imx6qdl.dtsi- reg = <0x20e0000 0x38>; -- arch/arm/boot/dts/imx6qdl.dtsi: iomuxc: pinctrl@20e0000 { arch/arm/boot/dts/imx6qdl.dtsi- compatible = "fsl,imx6dl-iomuxc", "fsl,imx6q-iomuxc"; arch/arm/boot/dts/imx6qdl.dtsi- reg = <0x20e0000 0x4000>;
On Mon, 22 May 2023 09:51:27 -0300, Fabio Estevam wrote: > From: Fabio Estevam <festevam@denx.de> > > The i.MX6SX General Purpose Registers is a set of register that serves > various different purposes and in particular, IOMUXC_GPR_GPR6, at > offset 0x18, can be used to configure the LDB block. > > Signed-off-by: Fabio Estevam <festevam@denx.de> > --- > Changes since v4: > - Renamed to syscon@20e4000 (Conor). > > .../bindings/soc/imx/fsl,imx6sx-gpr.yaml | 84 +++++++++++++++++++ > 1 file changed, 84 insertions(+) > create mode 100644 Documentation/devicetree/bindings/soc/imx/fsl,imx6sx-gpr.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/imx/fsl,imx6sx-gpr.example.dtb: syscon@20e4000: bridge@18:compatible:0: 'fsl,imx6sx-ldb' is not one of ['fsl,imx8mp-ldb', 'fsl,imx93-ldb'] From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/imx/fsl,imx6sx-gpr.yaml /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/imx/fsl,imx6sx-gpr.example.dtb: syscon@20e4000: bridge@18:reg: [[24, 4]] is too short From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/imx/fsl,imx6sx-gpr.yaml /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/imx/fsl,imx6sx-gpr.example.dtb: syscon@20e4000: bridge@18:reg-names: ['ldb'] is too short From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/imx/fsl,imx6sx-gpr.yaml /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/imx/fsl,imx6sx-gpr.example.dtb: syscon@20e4000: bridge@18: Unevaluated properties are not allowed ('compatible', 'reg', 'reg-names' were unexpected) From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/imx/fsl,imx6sx-gpr.yaml Documentation/devicetree/bindings/soc/imx/fsl,imx6sx-gpr.example.dtb: /example-0/syscon@20e4000/bridge@18: failed to match any schema with compatible: ['fsl,imx6sx-ldb'] doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230522125129.526604-2-festevam@gmail.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
Hi Marek, On 22/05/2023 10:20, Marek Vasut wrote: > Take a look at MX6Q and notice how the iomuxc and GPR register sets > share the same base address . That's different on MX6SX where they are > separate. So I think this binding should be specific to MX6SX ONLY and > for MX6Q the subnode probing should be handled in the IOMUXC driver > instead , i.e. drop the fsl,imx6q-iomuxc-gpr here and in imx6sx.dtsi . > > $ git grep -A 2 @20e0000 arch/arm/boot/dts/imx6qdl.dtsi > arch/arm/boot/dts/imx6qdl.dtsi: gpr: iomuxc-gpr@20e0000 > { > arch/arm/boot/dts/imx6qdl.dtsi- compatible = > "fsl,imx6q-iomuxc-gpr", "syscon", "simple-mfd"; > arch/arm/boot/dts/imx6qdl.dtsi- reg = > <0x20e0000 0x38>; > -- > arch/arm/boot/dts/imx6qdl.dtsi: iomuxc: pinctrl@20e0000 > { > arch/arm/boot/dts/imx6qdl.dtsi- compatible = > "fsl,imx6dl-iomuxc", "fsl,imx6q-iomuxc"; > arch/arm/boot/dts/imx6qdl.dtsi- reg = > <0x20e0000 0x4000>; Removing fsl,imx6q-iomuxc-gpr causes PCI probe issue on imx6sx as the driver searches for "fsl,imx6q-iomuxc-gpr": static const struct imx6_pcie_drvdata drvdata[] = { .... [IMX6SX] = { .variant = IMX6SX, .flags = IMX6_PCIE_FLAG_IMX6_PHY | IMX6_PCIE_FLAG_IMX6_SPEED_CHANGE | IMX6_PCIE_FLAG_SUPPORTS_SUSPEND, .gpr = "fsl,imx6q-iomuxc-gpr", # dmesg | grep pci [ 0.260212] imx6q-pcie 8ffc000.pcie: unable to find iomuxc registers
On 5/22/23 16:18, Fabio Estevam wrote: > Hi Marek, > > On 22/05/2023 10:20, Marek Vasut wrote: > >> Take a look at MX6Q and notice how the iomuxc and GPR register sets >> share the same base address . That's different on MX6SX where they are >> separate. So I think this binding should be specific to MX6SX ONLY and >> for MX6Q the subnode probing should be handled in the IOMUXC driver >> instead , i.e. drop the fsl,imx6q-iomuxc-gpr here and in imx6sx.dtsi . >> >> $ git grep -A 2 @20e0000 arch/arm/boot/dts/imx6qdl.dtsi >> arch/arm/boot/dts/imx6qdl.dtsi: gpr: iomuxc-gpr@20e0000 { >> arch/arm/boot/dts/imx6qdl.dtsi- compatible = >> "fsl,imx6q-iomuxc-gpr", "syscon", "simple-mfd"; >> arch/arm/boot/dts/imx6qdl.dtsi- reg = >> <0x20e0000 0x38>; >> -- >> arch/arm/boot/dts/imx6qdl.dtsi: iomuxc: pinctrl@20e0000 { >> arch/arm/boot/dts/imx6qdl.dtsi- compatible = >> "fsl,imx6dl-iomuxc", "fsl,imx6q-iomuxc"; >> arch/arm/boot/dts/imx6qdl.dtsi- reg = >> <0x20e0000 0x4000>; > > Removing fsl,imx6q-iomuxc-gpr causes PCI probe issue on imx6sx > as the driver searches for "fsl,imx6q-iomuxc-gpr": > > static const struct imx6_pcie_drvdata drvdata[] = { > .... > [IMX6SX] = { > .variant = IMX6SX, > .flags = IMX6_PCIE_FLAG_IMX6_PHY | > IMX6_PCIE_FLAG_IMX6_SPEED_CHANGE | > IMX6_PCIE_FLAG_SUPPORTS_SUSPEND, > .gpr = "fsl,imx6q-iomuxc-gpr", > > > # dmesg | grep pci > [ 0.260212] imx6q-pcie 8ffc000.pcie: unable to find iomuxc registers I would say, this should use the mx6sx compatible string for the GPR look up (or even use phandle ?)
diff --git a/Documentation/devicetree/bindings/soc/imx/fsl,imx6sx-gpr.yaml b/Documentation/devicetree/bindings/soc/imx/fsl,imx6sx-gpr.yaml new file mode 100644 index 000000000000..22777ecfb56b --- /dev/null +++ b/Documentation/devicetree/bindings/soc/imx/fsl,imx6sx-gpr.yaml @@ -0,0 +1,84 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/soc/imx/fsl,imx6sx-gpr.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: NXP i.MX6SX General Purpose Register + +maintainers: + - Fabio Estevam <festevam@denx.de> + +description: + The i.MX6SX General Purpose Registers is a set of register that serves + for various purposes and in particular, IOMUXC_GPR_GPR6, at offset 0x18, + can be used to configure the LDB block. + +properties: + compatible: + items: + - const: fsl,imx6sx-iomuxc-gpr + - const: fsl,imx6q-iomuxc-gpr + - const: syscon + + reg: + maxItems: 1 + + '#address-cells': + const: 1 + + '#size-cells': + const: 1 + + bridge@18: + type: object + $ref: /schemas/display/bridge/fsl,ldb.yaml# + unevaluatedProperties: false + +required: + - compatible + - reg + - '#address-cells' + - '#size-cells' + +additionalProperties: false + +examples: + - | + #include <dt-bindings/clock/imx6sx-clock.h> + + syscon@20e4000 { + compatible = "fsl,imx6sx-iomuxc-gpr", "fsl,imx6q-iomuxc-gpr", "syscon"; + reg = <0x020e4000 0x4000>; + #address-cells = <1>; + #size-cells = <1>; + + bridge@18 { + compatible = "fsl,imx6sx-ldb"; + reg = <0x18 0x4>; + reg-names = "ldb"; + clocks = <&clks IMX6SX_CLK_LDB_DI0>; + clock-names = "ldb"; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + + ldb_from_lcdif1: endpoint { + remote-endpoint = <&lcdif1_to_ldb>; + }; + }; + + port@1 { + reg = <1>; + + ldb_lvds_ch0: endpoint { + }; + }; + }; + }; + }; +...