| Message ID | ZhqO-DEmh-6TeHrt@Z926fQmE5jqhFMgp6 (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Avoid error message on rk3328 use | expand |
On 13/04/2024 15:56, Etienne Buira wrote: > rockchip,rk3328-grf-gpio is handled as syscon, but syscon mandates syscon does not need such property. I see it in gpio-syscon, but not in syscon. > presence of gpio,syscon-dev node (or it will call dev_err() when probed). > Correct rk3328.dtsi and related documentation to follow syscon's > expectations. No, look at gpio-syscon driver. Parent is used. > > Signed-off-by: Etienne Buira <etienne.buira@free.fr> > --- > .../devicetree/bindings/gpio/rockchip,rk3328-grf-gpio.yaml | 2 ++ Please run scripts/checkpatch.pl and fix reported warnings. Then please run `scripts/checkpatch.pl --strict` and (probably) fix more warnings. Some warnings can be ignored, especially from --strict run, but the code here looks like it needs a fix. Feel free to get in touch if the warning is not clear. > arch/arm64/boot/dts/rockchip/rk3328.dtsi | 1 + > 2 files changed, 3 insertions(+) > > diff --git a/Documentation/devicetree/bindings/gpio/rockchip,rk3328-grf-gpio.yaml b/Documentation/devicetree/bindings/gpio/rockchip,rk3328-grf-gpio.yaml > index d8cce73ea0ae..2c878e7db900 100644 > --- a/Documentation/devicetree/bindings/gpio/rockchip,rk3328-grf-gpio.yaml > +++ b/Documentation/devicetree/bindings/gpio/rockchip,rk3328-grf-gpio.yaml > @@ -38,6 +38,7 @@ required: > - compatible > - gpio-controller > - "#gpio-cells" > + - gpio,syscon-dev No, not needed. And also incomplete - where is the property defined? It does not look like you tested the bindings, at least after quick look. Please run `make dt_binding_check` (see Documentation/devicetree/bindings/writing-schema.rst for instructions). Maybe you need to update your dtschema and yamllint. Best regards, Krzysztof
On Sat, 13 Apr 2024 15:56:08 +0200, Etienne Buira wrote: > rockchip,rk3328-grf-gpio is handled as syscon, but syscon mandates > presence of gpio,syscon-dev node (or it will call dev_err() when probed). > Correct rk3328.dtsi and related documentation to follow syscon's > expectations. > > Signed-off-by: Etienne Buira <etienne.buira@free.fr> > --- > .../devicetree/bindings/gpio/rockchip,rk3328-grf-gpio.yaml | 2 ++ > arch/arm64/boot/dts/rockchip/rk3328.dtsi | 1 + > 2 files changed, 3 insertions(+) > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/gpio/rockchip,rk3328-grf-gpio.example.dtb: gpio: 'gpio,syscon-dev' does not match any of the regexes: 'pinctrl-[0-9]+' from schema $id: http://devicetree.org/schemas/gpio/rockchip,rk3328-grf-gpio.yaml# doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/ZhqO-DEmh-6TeHrt@Z926fQmE5jqhFMgp6 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 Krzysztof, On Sat, Apr 13, 2024 at 05:09:33PM +0200, Krzysztof Kozlowski wrote: > On 13/04/2024 15:56, Etienne Buira wrote: > > rockchip,rk3328-grf-gpio is handled as syscon, but syscon mandates > > syscon does not need such property. I see it in gpio-syscon, but not in > syscon. gpio-syscon, indeed. > > presence of gpio,syscon-dev node (or it will call dev_err() when probed). > > Correct rk3328.dtsi and related documentation to follow syscon's > > expectations. > > No, look at gpio-syscon driver. Parent is used. Parent is used, but the next lines are: ret = of_property_read_u32_index(np, "gpio,syscon-dev", 1, &priv->dreg_offset); if (ret) dev_err(...) So if gpio,syscon-dev does not have at least 2 items (or is missing), dev_err will be called, 3 items for dev_dbg. Current tree displays a spurious "can't read the data register offset" message. > > > > Signed-off-by: Etienne Buira <etienne.buira@free.fr> > > --- > > .../devicetree/bindings/gpio/rockchip,rk3328-grf-gpio.yaml | 2 ++ > > Please run scripts/checkpatch.pl and fix reported warnings. Then please > run `scripts/checkpatch.pl --strict` and (probably) fix more warnings. > Some warnings can be ignored, especially from --strict run, but the code > here looks like it needs a fix. Feel free to get in touch if the warning > is not clear. Will do, if we agree on the interest of patch. > > arch/arm64/boot/dts/rockchip/rk3328.dtsi | 1 + > > 2 files changed, 3 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/gpio/rockchip,rk3328-grf-gpio.yaml b/Documentation/devicetree/bindings/gpio/rockchip,rk3328-grf-gpio.yaml > > index d8cce73ea0ae..2c878e7db900 100644 > > --- a/Documentation/devicetree/bindings/gpio/rockchip,rk3328-grf-gpio.yaml > > +++ b/Documentation/devicetree/bindings/gpio/rockchip,rk3328-grf-gpio.yaml > > @@ -38,6 +38,7 @@ required: > > - compatible > > - gpio-controller > > - "#gpio-cells" > > + - gpio,syscon-dev > > No, not needed. And also incomplete - where is the property defined? > > It does not look like you tested the bindings, at least after quick > look. Please run `make dt_binding_check` (see > Documentation/devicetree/bindings/writing-schema.rst for instructions). > Maybe you need to update your dtschema and yamllint. ditto Regards
On 13/04/2024 17:23, Etienne Buira wrote: >>> presence of gpio,syscon-dev node (or it will call dev_err() when probed). >>> Correct rk3328.dtsi and related documentation to follow syscon's >>> expectations. >> >> No, look at gpio-syscon driver. Parent is used. > > Parent is used, but the next lines are: > ret = of_property_read_u32_index(np, "gpio,syscon-dev", 1, &priv->dreg_offset); > if (ret) > dev_err(...) > > So if gpio,syscon-dev does not have at least 2 items (or is missing), > dev_err will be called, 3 items for dev_dbg. > Current tree displays a spurious "can't read the data register offset" > message. Hm, indeed, then I think driver, so aa1fdda8f7ebf83f678e8d3c2ab4f1638c94195f, should be fixed. Otherwise please say why binding is not correct and driver is good. Best regards, Krzysztof
On Sat, Apr 13, 2024 at 05:49:13PM +0200, Krzysztof Kozlowski wrote: > On 13/04/2024 17:23, Etienne Buira wrote: > >>> presence of gpio,syscon-dev node (or it will call dev_err() when probed). > >>> Correct rk3328.dtsi and related documentation to follow syscon's > >>> expectations. > >> > >> No, look at gpio-syscon driver. Parent is used. > > > > Parent is used, but the next lines are: > > ret = of_property_read_u32_index(np, "gpio,syscon-dev", 1, &priv->dreg_offset); > > if (ret) > > dev_err(...) > > > > So if gpio,syscon-dev does not have at least 2 items (or is missing), > > dev_err will be called, 3 items for dev_dbg. > > Current tree displays a spurious "can't read the data register offset" > > message. > > Hm, indeed, then I think driver, so > aa1fdda8f7ebf83f678e8d3c2ab4f1638c94195f, should be fixed. Otherwise > please say why binding is not correct and driver is good. I tried fixing the driver first, this was discussed here: https://lore.kernel.org/linux-gpio/ZhptEWb7tD5pummq@Z926fQmE5jqhFMgp6/T/#t you're welcome to comment. I have no strong opinion over what should be fixed, i just wished to shut a spurious error message, that i expected to be straightforward at first (hence good candidate for first kernel patch). Regards
On Saturday, 13 April 2024 17:23:50 CEST Etienne Buira wrote: > Current tree displays a spurious "can't read the data register offset" > message. Sounds useful to mention that (specific) error message as that did rang a bell.
diff --git a/Documentation/devicetree/bindings/gpio/rockchip,rk3328-grf-gpio.yaml b/Documentation/devicetree/bindings/gpio/rockchip,rk3328-grf-gpio.yaml index d8cce73ea0ae..2c878e7db900 100644 --- a/Documentation/devicetree/bindings/gpio/rockchip,rk3328-grf-gpio.yaml +++ b/Documentation/devicetree/bindings/gpio/rockchip,rk3328-grf-gpio.yaml @@ -38,6 +38,7 @@ required: - compatible - gpio-controller - "#gpio-cells" + - gpio,syscon-dev additionalProperties: false @@ -47,4 +48,5 @@ examples: compatible = "rockchip,rk3328-grf-gpio"; gpio-controller; #gpio-cells = <2>; + gpio,syscon-dev = <&grf 0 0>; }; diff --git a/arch/arm64/boot/dts/rockchip/rk3328.dtsi b/arch/arm64/boot/dts/rockchip/rk3328.dtsi index b6f045069ee2..fd25d5bee19f 100644 --- a/arch/arm64/boot/dts/rockchip/rk3328.dtsi +++ b/arch/arm64/boot/dts/rockchip/rk3328.dtsi @@ -296,6 +296,7 @@ grf_gpio: gpio { compatible = "rockchip,rk3328-grf-gpio"; gpio-controller; #gpio-cells = <2>; + gpio,syscon-dev = <&grf 0 0>; }; power: power-controller {
rockchip,rk3328-grf-gpio is handled as syscon, but syscon mandates presence of gpio,syscon-dev node (or it will call dev_err() when probed). Correct rk3328.dtsi and related documentation to follow syscon's expectations. Signed-off-by: Etienne Buira <etienne.buira@free.fr> --- .../devicetree/bindings/gpio/rockchip,rk3328-grf-gpio.yaml | 2 ++ arch/arm64/boot/dts/rockchip/rk3328.dtsi | 1 + 2 files changed, 3 insertions(+) base-commit: 20cb38a7af88dc40095da7c2c9094da3873fea23