[<vendor>,]nr-gpios property is used by some GPIO drivers[0] to indicate the number of GPIOs present on a system, not define a GPIO. nr-gpios is not configured by #gpio-cells and can't be parsed along with other "*-gpios" properties.
scripts/dtc/checks.c also has a special case for nr-gpio{s}. However, nr-gpio is not really special, so we only need to fix nr-gpios suffix here.
[0]: nr-gpios is referenced in Documentation/devicetree/bindings/gpio: - gpio-adnp.txt - gpio-xgene-sb.txt - gpio-xlp.txt - snps,dw-apb-gpio.yaml
Fixes errors such as: OF: /palmbus@300000/gpio@600: could not find phandle
Call Trace: of_phandle_iterator_next+0x8c/0x16c __of_parse_phandle_with_args+0x38/0xb8 of_parse_phandle_with_args+0x28/0x3c parse_suffix_prop_cells+0x80/0xac parse_gpios+0x20/0x2c of_link_to_suppliers+0x18c/0x288 of_link_to_suppliers+0x1fc/0x288 device_add+0x4e0/0x734 of_platform_device_create_pdata+0xb8/0xfc of_platform_bus_create+0x170/0x214 of_platform_populate+0x88/0xf4 __dt_register_buses+0xbc/0xf0 plat_of_setup+0x1c/0x34
Fixes: 7f00be96f125 ("of: property: Add device link support for interrupt-parent, dmas and -gpio(s)") Signed-off-by: Ilya Lipnitskiy ilya.lipnitskiy@gmail.com Cc: Saravana Kannan saravanak@google.com Cc: stable@vger.kernel.org # 5.5.x --- drivers/of/property.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/of/property.c b/drivers/of/property.c index 2bb3158c9e43..24672c295603 100644 --- a/drivers/of/property.c +++ b/drivers/of/property.c @@ -1271,7 +1271,16 @@ DEFINE_SIMPLE_PROP(pinctrl8, "pinctrl-8", NULL) DEFINE_SIMPLE_PROP(remote_endpoint, "remote-endpoint", NULL) DEFINE_SUFFIX_PROP(regulators, "-supply", NULL) DEFINE_SUFFIX_PROP(gpio, "-gpio", "#gpio-cells") -DEFINE_SUFFIX_PROP(gpios, "-gpios", "#gpio-cells") + +static struct device_node *parse_gpios(struct device_node *np, + const char *prop_name, int index) +{ + if (!strcmp_suffix(prop_name, "nr-gpios")) + return NULL; + + return parse_suffix_prop_cells(np, prop_name, index, "-gpios", + "#gpio-cells"); +}
static struct device_node *parse_iommu_maps(struct device_node *np, const char *prop_name, int index)
On Sun, Apr 4, 2021 at 8:14 PM Ilya Lipnitskiy ilya.lipnitskiy@gmail.com wrote:
[<vendor>,]nr-gpios property is used by some GPIO drivers[0] to indicate the number of GPIOs present on a system, not define a GPIO. nr-gpios is not configured by #gpio-cells and can't be parsed along with other "*-gpios" properties.
scripts/dtc/checks.c also has a special case for nr-gpio{s}. However, nr-gpio is not really special, so we only need to fix nr-gpios suffix here.
The only example of this that I see is "snps,nr-gpios". I personally would like to deprecate such overlapping/ambiguous definitions.
Maybe fix up the DT? This warning is a nice reminder that the DT needs to be updated (if it can be). Outside of that, it's not causing any issues that I know of.
If they are, then we can pick up a patch similar to this. I'd also limit this fix to "snps,nr-gpios" so that future attempts to use -gpios for anything other than listing GPIOs triggers a warning.
Rob, thoughts?
Thanks, Saravana
[0]: nr-gpios is referenced in Documentation/devicetree/bindings/gpio:
- gpio-adnp.txt
- gpio-xgene-sb.txt
- gpio-xlp.txt
- snps,dw-apb-gpio.yaml
Fixes errors such as: OF: /palmbus@300000/gpio@600: could not find phandle
Call Trace: of_phandle_iterator_next+0x8c/0x16c __of_parse_phandle_with_args+0x38/0xb8 of_parse_phandle_with_args+0x28/0x3c parse_suffix_prop_cells+0x80/0xac parse_gpios+0x20/0x2c of_link_to_suppliers+0x18c/0x288 of_link_to_suppliers+0x1fc/0x288 device_add+0x4e0/0x734 of_platform_device_create_pdata+0xb8/0xfc of_platform_bus_create+0x170/0x214 of_platform_populate+0x88/0xf4 __dt_register_buses+0xbc/0xf0 plat_of_setup+0x1c/0x34
Fixes: 7f00be96f125 ("of: property: Add device link support for interrupt-parent, dmas and -gpio(s)") Signed-off-by: Ilya Lipnitskiy ilya.lipnitskiy@gmail.com Cc: Saravana Kannan saravanak@google.com Cc: stable@vger.kernel.org # 5.5.x
drivers/of/property.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/of/property.c b/drivers/of/property.c index 2bb3158c9e43..24672c295603 100644 --- a/drivers/of/property.c +++ b/drivers/of/property.c @@ -1271,7 +1271,16 @@ DEFINE_SIMPLE_PROP(pinctrl8, "pinctrl-8", NULL) DEFINE_SIMPLE_PROP(remote_endpoint, "remote-endpoint", NULL) DEFINE_SUFFIX_PROP(regulators, "-supply", NULL) DEFINE_SUFFIX_PROP(gpio, "-gpio", "#gpio-cells") -DEFINE_SUFFIX_PROP(gpios, "-gpios", "#gpio-cells")
+static struct device_node *parse_gpios(struct device_node *np,
const char *prop_name, int index)+{
if (!strcmp_suffix(prop_name, "nr-gpios"))return NULL;return parse_suffix_prop_cells(np, prop_name, index, "-gpios","#gpio-cells");+}
static struct device_node *parse_iommu_maps(struct device_node *np, const char *prop_name, int index) -- 2.31.1
Hi Saravana,
On Mon, Apr 5, 2021 at 1:01 PM Saravana Kannan saravanak@google.com wrote:
On Sun, Apr 4, 2021 at 8:14 PM Ilya Lipnitskiy ilya.lipnitskiy@gmail.com wrote:
[<vendor>,]nr-gpios property is used by some GPIO drivers[0] to indicate the number of GPIOs present on a system, not define a GPIO. nr-gpios is not configured by #gpio-cells and can't be parsed along with other "*-gpios" properties.
scripts/dtc/checks.c also has a special case for nr-gpio{s}. However, nr-gpio is not really special, so we only need to fix nr-gpios suffix here.
The only example of this that I see is "snps,nr-gpios".
arch/arm64/boot/dts/apm/apm-shadowcat.dtsi uses "apm,nr-gpios", with parsing code in drivers/gpio/gpio-xgene-sb.c. There is also code in drivers/gpio/gpio-adnp.c and drivers/gpio/gpio-mockup.c using "nr-gpios" without any vendor prefix.
I personally don't think causing regressions is good for any reason, so I think we need to fix this in stable releases. The patch can be reverted when nr-gpios is no longer special. The logic here should also be aligned with scripts/dtc/checks.c, I actually submitted a patch to warn about "nr-gpios" only and not "nr-gpio" in dtc as well: https://www.spinics.net/lists/devicetree-compiler/msg03619.html
Ilya
On Mon, Apr 5, 2021 at 1:10 PM Ilya Lipnitskiy ilya.lipnitskiy@gmail.com wrote:
Hi Saravana,
On Mon, Apr 5, 2021 at 1:01 PM Saravana Kannan saravanak@google.com wrote:
On Sun, Apr 4, 2021 at 8:14 PM Ilya Lipnitskiy ilya.lipnitskiy@gmail.com wrote:
[<vendor>,]nr-gpios property is used by some GPIO drivers[0] to indicate the number of GPIOs present on a system, not define a GPIO. nr-gpios is not configured by #gpio-cells and can't be parsed along with other "*-gpios" properties.
scripts/dtc/checks.c also has a special case for nr-gpio{s}. However, nr-gpio is not really special, so we only need to fix nr-gpios suffix here.
The only example of this that I see is "snps,nr-gpios".
arch/arm64/boot/dts/apm/apm-shadowcat.dtsi uses "apm,nr-gpios", with parsing code in drivers/gpio/gpio-xgene-sb.c. There is also code in drivers/gpio/gpio-adnp.c and drivers/gpio/gpio-mockup.c using "nr-gpios" without any vendor prefix.
Ah ok. I just grepped the DT files. I'm not sure what Rob's position is on supporting DT files not in upstream. Thanks for the clarification.
I personally don't think causing regressions is good for any reason,
I agree, but this is not a functional regression. Just a warning that's spit out. I don't have a strong opinion on the stack dump vs not, but I think we should at least reject future additions like this and limit the exceptions to exactly what's allowed today. nr-gpios (without any vendor prefix) is especially annoying to me.
Looks like even the DT spec has an exception only for vendor,nr and not just nr. https://github.com/devicetree-org/dt-schema/blob/master/schemas/gpio/gpio-co...
-Saravana
so I think we need to fix this in stable releases. The patch can be reverted when nr-gpios is no longer special. The logic here should also be aligned with scripts/dtc/checks.c, I actually submitted a patch to warn about "nr-gpios" only and not "nr-gpio" in dtc as well: https://www.spinics.net/lists/devicetree-compiler/msg03619.html
Ilya
On Mon, Apr 5, 2021 at 1:19 PM Saravana Kannan saravanak@google.com wrote:
On Mon, Apr 5, 2021 at 1:10 PM Ilya Lipnitskiy ilya.lipnitskiy@gmail.com wrote:
Hi Saravana,
On Mon, Apr 5, 2021 at 1:01 PM Saravana Kannan saravanak@google.com wrote:
On Sun, Apr 4, 2021 at 8:14 PM Ilya Lipnitskiy ilya.lipnitskiy@gmail.com wrote:
[<vendor>,]nr-gpios property is used by some GPIO drivers[0] to indicate the number of GPIOs present on a system, not define a GPIO. nr-gpios is not configured by #gpio-cells and can't be parsed along with other "*-gpios" properties.
scripts/dtc/checks.c also has a special case for nr-gpio{s}. However, nr-gpio is not really special, so we only need to fix nr-gpios suffix here.
The only example of this that I see is "snps,nr-gpios".
arch/arm64/boot/dts/apm/apm-shadowcat.dtsi uses "apm,nr-gpios", with parsing code in drivers/gpio/gpio-xgene-sb.c. There is also code in drivers/gpio/gpio-adnp.c and drivers/gpio/gpio-mockup.c using "nr-gpios" without any vendor prefix.
Ah ok. I just grepped the DT files. I'm not sure what Rob's position is on supporting DT files not in upstream. Thanks for the clarification.
For the offending drivers and docs that don't have any dts/dtsi files in-tree, can we just "sed -i 's/nr-gpios/ngpios'" and call it good?
Looks like even the DT spec has an exception only for vendor,nr and not just nr. https://github.com/devicetree-org/dt-schema/blob/master/schemas/gpio/gpio-co...
Thanks for linking the spec. I can re-spin the patch with ",nr-gpios" as the special suffix to align with the spec.
Ilya
On Mon, Apr 05, 2021 at 01:18:56PM -0700, Saravana Kannan wrote:
On Mon, Apr 5, 2021 at 1:10 PM Ilya Lipnitskiy ilya.lipnitskiy@gmail.com wrote:
Hi Saravana,
On Mon, Apr 5, 2021 at 1:01 PM Saravana Kannan saravanak@google.com wrote:
On Sun, Apr 4, 2021 at 8:14 PM Ilya Lipnitskiy ilya.lipnitskiy@gmail.com wrote:
[<vendor>,]nr-gpios property is used by some GPIO drivers[0] to indicate the number of GPIOs present on a system, not define a GPIO. nr-gpios is not configured by #gpio-cells and can't be parsed along with other "*-gpios" properties.
scripts/dtc/checks.c also has a special case for nr-gpio{s}. However, nr-gpio is not really special, so we only need to fix nr-gpios suffix here.
The only example of this that I see is "snps,nr-gpios".
arch/arm64/boot/dts/apm/apm-shadowcat.dtsi uses "apm,nr-gpios", with parsing code in drivers/gpio/gpio-xgene-sb.c. There is also code in drivers/gpio/gpio-adnp.c and drivers/gpio/gpio-mockup.c using "nr-gpios" without any vendor prefix.
Ah ok. I just grepped the DT files. I'm not sure what Rob's position is on supporting DT files not in upstream. Thanks for the clarification.
If it's something we had documented, then we have to support it (also conditioned on someone noticing). I'm hoping we can just delete APM and other defunct ARM server DTs soon, but we could update them to use 'ngpios' instead.
gpio-mockup doesn't have a binding, so no DT ABI. Hard to tell if gpio-adnp.c has any users.
Rob
On Tue, Apr 6, 2021 at 10:40 AM Rob Herring robh@kernel.org wrote:
On Mon, Apr 05, 2021 at 01:18:56PM -0700, Saravana Kannan wrote:
On Mon, Apr 5, 2021 at 1:10 PM Ilya Lipnitskiy ilya.lipnitskiy@gmail.com wrote:
Hi Saravana,
On Mon, Apr 5, 2021 at 1:01 PM Saravana Kannan saravanak@google.com wrote:
On Sun, Apr 4, 2021 at 8:14 PM Ilya Lipnitskiy ilya.lipnitskiy@gmail.com wrote:
[<vendor>,]nr-gpios property is used by some GPIO drivers[0] to indicate the number of GPIOs present on a system, not define a GPIO. nr-gpios is not configured by #gpio-cells and can't be parsed along with other "*-gpios" properties.
scripts/dtc/checks.c also has a special case for nr-gpio{s}. However, nr-gpio is not really special, so we only need to fix nr-gpios suffix here.
The only example of this that I see is "snps,nr-gpios".
arch/arm64/boot/dts/apm/apm-shadowcat.dtsi uses "apm,nr-gpios", with parsing code in drivers/gpio/gpio-xgene-sb.c. There is also code in drivers/gpio/gpio-adnp.c and drivers/gpio/gpio-mockup.c using "nr-gpios" without any vendor prefix.
Ah ok. I just grepped the DT files. I'm not sure what Rob's position is on supporting DT files not in upstream. Thanks for the clarification.
If it's something we had documented, then we have to support it
Do I read this correctly as a sort-of Ack of my proposed [PATCH v2] in this thread, since it aligns the code with the published DT schema?
Ilya
On Tue, Apr 6, 2021 at 12:28 PM Ilya Lipnitskiy ilya.lipnitskiy@gmail.com wrote:
On Tue, Apr 6, 2021 at 10:40 AM Rob Herring robh@kernel.org wrote:
On Mon, Apr 05, 2021 at 01:18:56PM -0700, Saravana Kannan wrote:
On Mon, Apr 5, 2021 at 1:10 PM Ilya Lipnitskiy ilya.lipnitskiy@gmail.com wrote:
Hi Saravana,
On Mon, Apr 5, 2021 at 1:01 PM Saravana Kannan saravanak@google.com wrote:
On Sun, Apr 4, 2021 at 8:14 PM Ilya Lipnitskiy ilya.lipnitskiy@gmail.com wrote:
[<vendor>,]nr-gpios property is used by some GPIO drivers[0] to indicate the number of GPIOs present on a system, not define a GPIO. nr-gpios is not configured by #gpio-cells and can't be parsed along with other "*-gpios" properties.
scripts/dtc/checks.c also has a special case for nr-gpio{s}. However, nr-gpio is not really special, so we only need to fix nr-gpios suffix here.
The only example of this that I see is "snps,nr-gpios".
arch/arm64/boot/dts/apm/apm-shadowcat.dtsi uses "apm,nr-gpios", with parsing code in drivers/gpio/gpio-xgene-sb.c. There is also code in drivers/gpio/gpio-adnp.c and drivers/gpio/gpio-mockup.c using "nr-gpios" without any vendor prefix.
Ah ok. I just grepped the DT files. I'm not sure what Rob's position is on supporting DT files not in upstream. Thanks for the clarification.
If it's something we had documented, then we have to support it
Do I read this correctly as a sort-of Ack of my proposed [PATCH v2] in this thread, since it aligns the code with the published DT schema?
He's talking about the DT binding documentation in the kernel.
I interpret Rob's reply as, you can do all of this: 1. Just fix up all drivers that use "*nr-gpios" that don't have binding documentation in the kernel. Change them to use ngpios. 2. Try to switch away old defunct ARM server DTs from nr-gpios to ngpios (both drivers and DT) and see if people notice. 3. Change the fw_devlink parsing code to have exceptions only for cases that are using nr-gpios after (1) and (2).
-Saravana
On Tue, Apr 6, 2021 at 4:28 PM Saravana Kannan saravanak@google.com wrote:
On Tue, Apr 6, 2021 at 12:28 PM Ilya Lipnitskiy ilya.lipnitskiy@gmail.com wrote:
On Tue, Apr 6, 2021 at 10:40 AM Rob Herring robh@kernel.org wrote:
On Mon, Apr 05, 2021 at 01:18:56PM -0700, Saravana Kannan wrote:
On Mon, Apr 5, 2021 at 1:10 PM Ilya Lipnitskiy ilya.lipnitskiy@gmail.com wrote:
Hi Saravana,
On Mon, Apr 5, 2021 at 1:01 PM Saravana Kannan saravanak@google.com wrote:
On Sun, Apr 4, 2021 at 8:14 PM Ilya Lipnitskiy ilya.lipnitskiy@gmail.com wrote: > > [<vendor>,]nr-gpios property is used by some GPIO drivers[0] to indicate > the number of GPIOs present on a system, not define a GPIO. nr-gpios is > not configured by #gpio-cells and can't be parsed along with other > "*-gpios" properties. > > scripts/dtc/checks.c also has a special case for nr-gpio{s}. However, > nr-gpio is not really special, so we only need to fix nr-gpios suffix > here.
The only example of this that I see is "snps,nr-gpios".
arch/arm64/boot/dts/apm/apm-shadowcat.dtsi uses "apm,nr-gpios", with parsing code in drivers/gpio/gpio-xgene-sb.c. There is also code in drivers/gpio/gpio-adnp.c and drivers/gpio/gpio-mockup.c using "nr-gpios" without any vendor prefix.
Ah ok. I just grepped the DT files. I'm not sure what Rob's position is on supporting DT files not in upstream. Thanks for the clarification.
If it's something we had documented, then we have to support it
Do I read this correctly as a sort-of Ack of my proposed [PATCH v2] in this thread, since it aligns the code with the published DT schema?
He's talking about the DT binding documentation in the kernel.
I interpret Rob's reply as, you can do all of this:
- Just fix up all drivers that use "*nr-gpios" that don't have
binding documentation in the kernel. Change them to use ngpios. 2. Try to switch away old defunct ARM server DTs from nr-gpios to ngpios (both drivers and DT) and see if people notice. 3. Change the fw_devlink parsing code to have exceptions only for cases that are using nr-gpios after (1) and (2).
Yes, but (3) is not gated on (1) and (2). I'm applying v2.
Rob
[<vendor>,]nr-gpios property is used by some GPIO drivers[0] to indicate the number of GPIOs present on a system, not define a GPIO. nr-gpios is not configured by #gpio-cells and can't be parsed along with other "*-gpios" properties.
nr-gpios without the "<vendor>," prefix is not allowed by the DT spec[1], so only add exception for the ",nr-gpios" suffix and let the error message continue being printed for non-compliant implementations.
[0]: nr-gpios is referenced in Documentation/devicetree/bindings/gpio: - gpio-adnp.txt - gpio-xgene-sb.txt - gpio-xlp.txt - snps,dw-apb-gpio.yaml
[1]: Link: https://github.com/devicetree-org/dt-schema/blob/cb53a16a1eb3e2169ce170c071e...
Fixes errors such as: OF: /palmbus@300000/gpio@600: could not find phandle
Fixes: 7f00be96f125 ("of: property: Add device link support for interrupt-parent, dmas and -gpio(s)") Signed-off-by: Ilya Lipnitskiy ilya.lipnitskiy@gmail.com Cc: Saravana Kannan saravanak@google.com Cc: stable@vger.kernel.org # 5.5.x --- drivers/of/property.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/of/property.c b/drivers/of/property.c index 2046ae311322..1793303e84ac 100644 --- a/drivers/of/property.c +++ b/drivers/of/property.c @@ -1281,7 +1281,16 @@ DEFINE_SIMPLE_PROP(pinctrl7, "pinctrl-7", NULL) DEFINE_SIMPLE_PROP(pinctrl8, "pinctrl-8", NULL) DEFINE_SUFFIX_PROP(regulators, "-supply", NULL) DEFINE_SUFFIX_PROP(gpio, "-gpio", "#gpio-cells") -DEFINE_SUFFIX_PROP(gpios, "-gpios", "#gpio-cells") + +static struct device_node *parse_gpios(struct device_node *np, + const char *prop_name, int index) +{ + if (!strcmp_suffix(prop_name, ",nr-gpios")) + return NULL; + + return parse_suffix_prop_cells(np, prop_name, index, "-gpios", + "#gpio-cells"); +}
static struct device_node *parse_iommu_maps(struct device_node *np, const char *prop_name, int index)
On Mon, Apr 5, 2021 at 3:26 PM Ilya Lipnitskiy ilya.lipnitskiy@gmail.com wrote:
[<vendor>,]nr-gpios property is used by some GPIO drivers[0] to indicate the number of GPIOs present on a system, not define a GPIO. nr-gpios is not configured by #gpio-cells and can't be parsed along with other "*-gpios" properties.
nr-gpios without the "<vendor>," prefix is not allowed by the DT spec[1], so only add exception for the ",nr-gpios" suffix and let the error message continue being printed for non-compliant implementations.
[0]: nr-gpios is referenced in Documentation/devicetree/bindings/gpio:
- gpio-adnp.txt
- gpio-xgene-sb.txt
- gpio-xlp.txt
- snps,dw-apb-gpio.yaml
[1]: Link: https://github.com/devicetree-org/dt-schema/blob/cb53a16a1eb3e2169ce170c071e...
Fixes errors such as: OF: /palmbus@300000/gpio@600: could not find phandle
Fixes: 7f00be96f125 ("of: property: Add device link support for interrupt-parent, dmas and -gpio(s)") Signed-off-by: Ilya Lipnitskiy ilya.lipnitskiy@gmail.com Cc: Saravana Kannan saravanak@google.com Cc: stable@vger.kernel.org # 5.5.x
drivers/of/property.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/of/property.c b/drivers/of/property.c index 2046ae311322..1793303e84ac 100644 --- a/drivers/of/property.c +++ b/drivers/of/property.c @@ -1281,7 +1281,16 @@ DEFINE_SIMPLE_PROP(pinctrl7, "pinctrl-7", NULL) DEFINE_SIMPLE_PROP(pinctrl8, "pinctrl-8", NULL) DEFINE_SUFFIX_PROP(regulators, "-supply", NULL) DEFINE_SUFFIX_PROP(gpio, "-gpio", "#gpio-cells") -DEFINE_SUFFIX_PROP(gpios, "-gpios", "#gpio-cells")
+static struct device_node *parse_gpios(struct device_node *np,
const char *prop_name, int index)+{
if (!strcmp_suffix(prop_name, ",nr-gpios"))return NULL;
Ah I somehow missed this patch. This gives a blanked exception for vendor,nr-gpios. I'd prefer explicit exceptions for all the instances of ",nr-gpios" we are grandfathering in. Any future additions should be rejected. Can we do that please?
Rob, you okay with making this list more explicit?
-Saravana
On Tue, Apr 06, 2021 at 04:09:10PM -0700, Saravana Kannan wrote:
On Mon, Apr 5, 2021 at 3:26 PM Ilya Lipnitskiy ilya.lipnitskiy@gmail.com wrote:
[<vendor>,]nr-gpios property is used by some GPIO drivers[0] to indicate the number of GPIOs present on a system, not define a GPIO. nr-gpios is not configured by #gpio-cells and can't be parsed along with other "*-gpios" properties.
nr-gpios without the "<vendor>," prefix is not allowed by the DT spec[1], so only add exception for the ",nr-gpios" suffix and let the error message continue being printed for non-compliant implementations.
[0]: nr-gpios is referenced in Documentation/devicetree/bindings/gpio:
- gpio-adnp.txt
- gpio-xgene-sb.txt
- gpio-xlp.txt
- snps,dw-apb-gpio.yaml
[1]: Link: https://github.com/devicetree-org/dt-schema/blob/cb53a16a1eb3e2169ce170c071e...
Fixes errors such as: OF: /palmbus@300000/gpio@600: could not find phandle
Fixes: 7f00be96f125 ("of: property: Add device link support for interrupt-parent, dmas and -gpio(s)") Signed-off-by: Ilya Lipnitskiy ilya.lipnitskiy@gmail.com Cc: Saravana Kannan saravanak@google.com Cc: stable@vger.kernel.org # 5.5.x
drivers/of/property.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/of/property.c b/drivers/of/property.c index 2046ae311322..1793303e84ac 100644 --- a/drivers/of/property.c +++ b/drivers/of/property.c @@ -1281,7 +1281,16 @@ DEFINE_SIMPLE_PROP(pinctrl7, "pinctrl-7", NULL) DEFINE_SIMPLE_PROP(pinctrl8, "pinctrl-8", NULL) DEFINE_SUFFIX_PROP(regulators, "-supply", NULL) DEFINE_SUFFIX_PROP(gpio, "-gpio", "#gpio-cells") -DEFINE_SUFFIX_PROP(gpios, "-gpios", "#gpio-cells")
+static struct device_node *parse_gpios(struct device_node *np,
const char *prop_name, int index)+{
if (!strcmp_suffix(prop_name, ",nr-gpios"))return NULL;Ah I somehow missed this patch. This gives a blanked exception for vendor,nr-gpios. I'd prefer explicit exceptions for all the instances of ",nr-gpios" we are grandfathering in. Any future additions should be rejected. Can we do that please?
Rob, you okay with making this list more explicit?
Not the kernel's job IMO. A schema is the right way to handle that.
Rob
On Tue, Apr 6, 2021 at 5:34 PM Rob Herring robh@kernel.org wrote:
On Tue, Apr 06, 2021 at 04:09:10PM -0700, Saravana Kannan wrote:
On Mon, Apr 5, 2021 at 3:26 PM Ilya Lipnitskiy ilya.lipnitskiy@gmail.com wrote:
[<vendor>,]nr-gpios property is used by some GPIO drivers[0] to indicate the number of GPIOs present on a system, not define a GPIO. nr-gpios is not configured by #gpio-cells and can't be parsed along with other "*-gpios" properties.
nr-gpios without the "<vendor>," prefix is not allowed by the DT spec[1], so only add exception for the ",nr-gpios" suffix and let the error message continue being printed for non-compliant implementations.
[0]: nr-gpios is referenced in Documentation/devicetree/bindings/gpio:
- gpio-adnp.txt
- gpio-xgene-sb.txt
- gpio-xlp.txt
- snps,dw-apb-gpio.yaml
[1]: Link: https://github.com/devicetree-org/dt-schema/blob/cb53a16a1eb3e2169ce170c071e...
Fixes errors such as: OF: /palmbus@300000/gpio@600: could not find phandle
Fixes: 7f00be96f125 ("of: property: Add device link support for interrupt-parent, dmas and -gpio(s)") Signed-off-by: Ilya Lipnitskiy ilya.lipnitskiy@gmail.com Cc: Saravana Kannan saravanak@google.com Cc: stable@vger.kernel.org # 5.5.x
drivers/of/property.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/of/property.c b/drivers/of/property.c index 2046ae311322..1793303e84ac 100644 --- a/drivers/of/property.c +++ b/drivers/of/property.c @@ -1281,7 +1281,16 @@ DEFINE_SIMPLE_PROP(pinctrl7, "pinctrl-7", NULL) DEFINE_SIMPLE_PROP(pinctrl8, "pinctrl-8", NULL) DEFINE_SUFFIX_PROP(regulators, "-supply", NULL) DEFINE_SUFFIX_PROP(gpio, "-gpio", "#gpio-cells") -DEFINE_SUFFIX_PROP(gpios, "-gpios", "#gpio-cells")
+static struct device_node *parse_gpios(struct device_node *np,
const char *prop_name, int index)+{
if (!strcmp_suffix(prop_name, ",nr-gpios"))return NULL;Ah I somehow missed this patch. This gives a blanked exception for vendor,nr-gpios. I'd prefer explicit exceptions for all the instances of ",nr-gpios" we are grandfathering in. Any future additions should be rejected. Can we do that please?
Rob, you okay with making this list more explicit?
Not the kernel's job IMO. A schema is the right way to handle that.
Ok, that's fine by me. Btw, let's land this in driver-core? I've made changes there and this might cause conflicts. Not sure.
-Saravana
On Tue, Apr 6, 2021 at 7:46 PM Saravana Kannan saravanak@google.com wrote:
On Tue, Apr 6, 2021 at 5:34 PM Rob Herring robh@kernel.org wrote:
On Tue, Apr 06, 2021 at 04:09:10PM -0700, Saravana Kannan wrote:
On Mon, Apr 5, 2021 at 3:26 PM Ilya Lipnitskiy ilya.lipnitskiy@gmail.com wrote:
[<vendor>,]nr-gpios property is used by some GPIO drivers[0] to indicate the number of GPIOs present on a system, not define a GPIO. nr-gpios is not configured by #gpio-cells and can't be parsed along with other "*-gpios" properties.
nr-gpios without the "<vendor>," prefix is not allowed by the DT spec[1], so only add exception for the ",nr-gpios" suffix and let the error message continue being printed for non-compliant implementations.
[0]: nr-gpios is referenced in Documentation/devicetree/bindings/gpio:
- gpio-adnp.txt
- gpio-xgene-sb.txt
- gpio-xlp.txt
- snps,dw-apb-gpio.yaml
[1]: Link: https://github.com/devicetree-org/dt-schema/blob/cb53a16a1eb3e2169ce170c071e...
Fixes errors such as: OF: /palmbus@300000/gpio@600: could not find phandle
Fixes: 7f00be96f125 ("of: property: Add device link support for interrupt-parent, dmas and -gpio(s)") Signed-off-by: Ilya Lipnitskiy ilya.lipnitskiy@gmail.com Cc: Saravana Kannan saravanak@google.com Cc: stable@vger.kernel.org # 5.5.x
drivers/of/property.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/of/property.c b/drivers/of/property.c index 2046ae311322..1793303e84ac 100644 --- a/drivers/of/property.c +++ b/drivers/of/property.c @@ -1281,7 +1281,16 @@ DEFINE_SIMPLE_PROP(pinctrl7, "pinctrl-7", NULL) DEFINE_SIMPLE_PROP(pinctrl8, "pinctrl-8", NULL) DEFINE_SUFFIX_PROP(regulators, "-supply", NULL) DEFINE_SUFFIX_PROP(gpio, "-gpio", "#gpio-cells") -DEFINE_SUFFIX_PROP(gpios, "-gpios", "#gpio-cells")
+static struct device_node *parse_gpios(struct device_node *np,
const char *prop_name, int index)+{
if (!strcmp_suffix(prop_name, ",nr-gpios"))return NULL;Ah I somehow missed this patch. This gives a blanked exception for vendor,nr-gpios. I'd prefer explicit exceptions for all the instances of ",nr-gpios" we are grandfathering in. Any future additions should be rejected. Can we do that please?
Rob, you okay with making this list more explicit?
Not the kernel's job IMO. A schema is the right way to handle that.
Ok, that's fine by me. Btw, let's land this in driver-core? I've made changes there and this might cause conflicts. Not sure.
It merges with linux-next fine. You'll need to resend this to Greg if you want to do that.
Reviewed-by: Rob Herring robh@kernel.org
On Tue, Apr 6, 2021 at 6:10 PM Rob Herring robh@kernel.org wrote:
On Tue, Apr 6, 2021 at 7:46 PM Saravana Kannan saravanak@google.com wrote:
On Tue, Apr 6, 2021 at 5:34 PM Rob Herring robh@kernel.org wrote:
On Tue, Apr 06, 2021 at 04:09:10PM -0700, Saravana Kannan wrote:
On Mon, Apr 5, 2021 at 3:26 PM Ilya Lipnitskiy ilya.lipnitskiy@gmail.com wrote:
[<vendor>,]nr-gpios property is used by some GPIO drivers[0] to indicate the number of GPIOs present on a system, not define a GPIO. nr-gpios is not configured by #gpio-cells and can't be parsed along with other "*-gpios" properties.
nr-gpios without the "<vendor>," prefix is not allowed by the DT spec[1], so only add exception for the ",nr-gpios" suffix and let the error message continue being printed for non-compliant implementations.
[0]: nr-gpios is referenced in Documentation/devicetree/bindings/gpio:
- gpio-adnp.txt
- gpio-xgene-sb.txt
- gpio-xlp.txt
- snps,dw-apb-gpio.yaml
[1]: Link: https://github.com/devicetree-org/dt-schema/blob/cb53a16a1eb3e2169ce170c071e...
Fixes errors such as: OF: /palmbus@300000/gpio@600: could not find phandle
Fixes: 7f00be96f125 ("of: property: Add device link support for interrupt-parent, dmas and -gpio(s)") Signed-off-by: Ilya Lipnitskiy ilya.lipnitskiy@gmail.com Cc: Saravana Kannan saravanak@google.com Cc: stable@vger.kernel.org # 5.5.x
drivers/of/property.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/of/property.c b/drivers/of/property.c index 2046ae311322..1793303e84ac 100644 --- a/drivers/of/property.c +++ b/drivers/of/property.c @@ -1281,7 +1281,16 @@ DEFINE_SIMPLE_PROP(pinctrl7, "pinctrl-7", NULL) DEFINE_SIMPLE_PROP(pinctrl8, "pinctrl-8", NULL) DEFINE_SUFFIX_PROP(regulators, "-supply", NULL) DEFINE_SUFFIX_PROP(gpio, "-gpio", "#gpio-cells") -DEFINE_SUFFIX_PROP(gpios, "-gpios", "#gpio-cells")
+static struct device_node *parse_gpios(struct device_node *np,
const char *prop_name, int index)+{
if (!strcmp_suffix(prop_name, ",nr-gpios"))return NULL;Ah I somehow missed this patch. This gives a blanked exception for vendor,nr-gpios. I'd prefer explicit exceptions for all the instances of ",nr-gpios" we are grandfathering in. Any future additions should be rejected. Can we do that please?
Rob, you okay with making this list more explicit?
Not the kernel's job IMO. A schema is the right way to handle that.
Ok, that's fine by me. Btw, let's land this in driver-core? I've made changes there and this might cause conflicts. Not sure.
It merges with linux-next fine. You'll need to resend this to Greg if you want to do that.
Reviewed-by: Rob Herring robh@kernel.org
Hi Greg,
Can you pull this into driver-core please? I touch this file a lot and might need to do so again if any fw_devlink=on issues come up. So trying to preemptively avoid conflicts.
-Saravana
On Tue, Apr 6, 2021 at 6:24 PM Saravana Kannan saravanak@google.com wrote:
On Tue, Apr 6, 2021 at 6:10 PM Rob Herring robh@kernel.org wrote:
On Tue, Apr 6, 2021 at 7:46 PM Saravana Kannan saravanak@google.com wrote:
On Tue, Apr 6, 2021 at 5:34 PM Rob Herring robh@kernel.org wrote:
On Tue, Apr 06, 2021 at 04:09:10PM -0700, Saravana Kannan wrote:
On Mon, Apr 5, 2021 at 3:26 PM Ilya Lipnitskiy ilya.lipnitskiy@gmail.com wrote:
[<vendor>,]nr-gpios property is used by some GPIO drivers[0] to indicate the number of GPIOs present on a system, not define a GPIO. nr-gpios is not configured by #gpio-cells and can't be parsed along with other "*-gpios" properties.
nr-gpios without the "<vendor>," prefix is not allowed by the DT spec[1], so only add exception for the ",nr-gpios" suffix and let the error message continue being printed for non-compliant implementations.
[0]: nr-gpios is referenced in Documentation/devicetree/bindings/gpio:
- gpio-adnp.txt
- gpio-xgene-sb.txt
- gpio-xlp.txt
- snps,dw-apb-gpio.yaml
[1]: Link: https://github.com/devicetree-org/dt-schema/blob/cb53a16a1eb3e2169ce170c071e...
Fixes errors such as: OF: /palmbus@300000/gpio@600: could not find phandle
Fixes: 7f00be96f125 ("of: property: Add device link support for interrupt-parent, dmas and -gpio(s)") Signed-off-by: Ilya Lipnitskiy ilya.lipnitskiy@gmail.com Cc: Saravana Kannan saravanak@google.com Cc: stable@vger.kernel.org # 5.5.x
drivers/of/property.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/of/property.c b/drivers/of/property.c index 2046ae311322..1793303e84ac 100644 --- a/drivers/of/property.c +++ b/drivers/of/property.c @@ -1281,7 +1281,16 @@ DEFINE_SIMPLE_PROP(pinctrl7, "pinctrl-7", NULL) DEFINE_SIMPLE_PROP(pinctrl8, "pinctrl-8", NULL) DEFINE_SUFFIX_PROP(regulators, "-supply", NULL) DEFINE_SUFFIX_PROP(gpio, "-gpio", "#gpio-cells") -DEFINE_SUFFIX_PROP(gpios, "-gpios", "#gpio-cells")
+static struct device_node *parse_gpios(struct device_node *np,
const char *prop_name, int index)+{
if (!strcmp_suffix(prop_name, ",nr-gpios"))return NULL;Ah I somehow missed this patch. This gives a blanked exception for vendor,nr-gpios. I'd prefer explicit exceptions for all the instances of ",nr-gpios" we are grandfathering in. Any future additions should be rejected. Can we do that please?
Rob, you okay with making this list more explicit?
Not the kernel's job IMO. A schema is the right way to handle that.
Ok, that's fine by me. Btw, let's land this in driver-core? I've made changes there and this might cause conflicts. Not sure.
It merges with linux-next fine. You'll need to resend this to Greg if you want to do that.
Reviewed-by: Rob Herring robh@kernel.org
Hi Greg,
Can you pull this into driver-core please?
Do you want me to re-spin on top of driver-core? The patch is currently based on dt/next in robh/linux.git
Ilya
On Wed, Apr 7, 2021 at 3:45 PM Ilya Lipnitskiy ilya.lipnitskiy@gmail.com wrote:
On Tue, Apr 6, 2021 at 6:24 PM Saravana Kannan saravanak@google.com wrote:
On Tue, Apr 6, 2021 at 6:10 PM Rob Herring robh@kernel.org wrote:
On Tue, Apr 6, 2021 at 7:46 PM Saravana Kannan saravanak@google.com wrote:
On Tue, Apr 6, 2021 at 5:34 PM Rob Herring robh@kernel.org wrote:
On Tue, Apr 06, 2021 at 04:09:10PM -0700, Saravana Kannan wrote:
On Mon, Apr 5, 2021 at 3:26 PM Ilya Lipnitskiy ilya.lipnitskiy@gmail.com wrote: > > [<vendor>,]nr-gpios property is used by some GPIO drivers[0] to indicate > the number of GPIOs present on a system, not define a GPIO. nr-gpios is > not configured by #gpio-cells and can't be parsed along with other > "*-gpios" properties. > > nr-gpios without the "<vendor>," prefix is not allowed by the DT > spec[1], so only add exception for the ",nr-gpios" suffix and let the > error message continue being printed for non-compliant implementations. > > [0]: nr-gpios is referenced in Documentation/devicetree/bindings/gpio: > - gpio-adnp.txt > - gpio-xgene-sb.txt > - gpio-xlp.txt > - snps,dw-apb-gpio.yaml > > [1]: > Link: https://github.com/devicetree-org/dt-schema/blob/cb53a16a1eb3e2169ce170c071e... > > Fixes errors such as: > OF: /palmbus@300000/gpio@600: could not find phandle > > Fixes: 7f00be96f125 ("of: property: Add device link support for interrupt-parent, dmas and -gpio(s)") > Signed-off-by: Ilya Lipnitskiy ilya.lipnitskiy@gmail.com > Cc: Saravana Kannan saravanak@google.com > Cc: stable@vger.kernel.org # 5.5.x > --- > drivers/of/property.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/drivers/of/property.c b/drivers/of/property.c > index 2046ae311322..1793303e84ac 100644 > --- a/drivers/of/property.c > +++ b/drivers/of/property.c > @@ -1281,7 +1281,16 @@ DEFINE_SIMPLE_PROP(pinctrl7, "pinctrl-7", NULL) > DEFINE_SIMPLE_PROP(pinctrl8, "pinctrl-8", NULL) > DEFINE_SUFFIX_PROP(regulators, "-supply", NULL) > DEFINE_SUFFIX_PROP(gpio, "-gpio", "#gpio-cells") > -DEFINE_SUFFIX_PROP(gpios, "-gpios", "#gpio-cells") > + > +static struct device_node *parse_gpios(struct device_node *np, > + const char *prop_name, int index) > +{ > + if (!strcmp_suffix(prop_name, ",nr-gpios")) > + return NULL;
Ah I somehow missed this patch. This gives a blanked exception for vendor,nr-gpios. I'd prefer explicit exceptions for all the instances of ",nr-gpios" we are grandfathering in. Any future additions should be rejected. Can we do that please?
Rob, you okay with making this list more explicit?
Not the kernel's job IMO. A schema is the right way to handle that.
Ok, that's fine by me. Btw, let's land this in driver-core? I've made changes there and this might cause conflicts. Not sure.
It merges with linux-next fine. You'll need to resend this to Greg if you want to do that.
Reviewed-by: Rob Herring robh@kernel.org
Hi Greg,
Can you pull this into driver-core please?
Do you want me to re-spin on top of driver-core? The patch is currently based on dt/next in robh/linux.git
I did say you need to resend the patch to Greg, but since there's no movement on this and I have other things to send upstream, I've applied it.
Rob
On Fri, Apr 9, 2021 at 12:26 PM Rob Herring robh@kernel.org wrote:
On Wed, Apr 7, 2021 at 3:45 PM Ilya Lipnitskiy ilya.lipnitskiy@gmail.com wrote:
On Tue, Apr 6, 2021 at 6:24 PM Saravana Kannan saravanak@google.com wrote:
On Tue, Apr 6, 2021 at 6:10 PM Rob Herring robh@kernel.org wrote:
On Tue, Apr 6, 2021 at 7:46 PM Saravana Kannan saravanak@google.com wrote:
On Tue, Apr 6, 2021 at 5:34 PM Rob Herring robh@kernel.org wrote:
On Tue, Apr 06, 2021 at 04:09:10PM -0700, Saravana Kannan wrote: > On Mon, Apr 5, 2021 at 3:26 PM Ilya Lipnitskiy > ilya.lipnitskiy@gmail.com wrote: > > > > [<vendor>,]nr-gpios property is used by some GPIO drivers[0] to indicate > > the number of GPIOs present on a system, not define a GPIO. nr-gpios is > > not configured by #gpio-cells and can't be parsed along with other > > "*-gpios" properties. > > > > nr-gpios without the "<vendor>," prefix is not allowed by the DT > > spec[1], so only add exception for the ",nr-gpios" suffix and let the > > error message continue being printed for non-compliant implementations. > > > > [0]: nr-gpios is referenced in Documentation/devicetree/bindings/gpio: > > - gpio-adnp.txt > > - gpio-xgene-sb.txt > > - gpio-xlp.txt > > - snps,dw-apb-gpio.yaml > > > > [1]: > > Link: https://github.com/devicetree-org/dt-schema/blob/cb53a16a1eb3e2169ce170c071e... > > > > Fixes errors such as: > > OF: /palmbus@300000/gpio@600: could not find phandle > > > > Fixes: 7f00be96f125 ("of: property: Add device link support for interrupt-parent, dmas and -gpio(s)") > > Signed-off-by: Ilya Lipnitskiy ilya.lipnitskiy@gmail.com > > Cc: Saravana Kannan saravanak@google.com > > Cc: stable@vger.kernel.org # 5.5.x > > --- > > drivers/of/property.c | 11 ++++++++++- > > 1 file changed, 10 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/of/property.c b/drivers/of/property.c > > index 2046ae311322..1793303e84ac 100644 > > --- a/drivers/of/property.c > > +++ b/drivers/of/property.c > > @@ -1281,7 +1281,16 @@ DEFINE_SIMPLE_PROP(pinctrl7, "pinctrl-7", NULL) > > DEFINE_SIMPLE_PROP(pinctrl8, "pinctrl-8", NULL) > > DEFINE_SUFFIX_PROP(regulators, "-supply", NULL) > > DEFINE_SUFFIX_PROP(gpio, "-gpio", "#gpio-cells") > > -DEFINE_SUFFIX_PROP(gpios, "-gpios", "#gpio-cells") > > + > > +static struct device_node *parse_gpios(struct device_node *np, > > + const char *prop_name, int index) > > +{ > > + if (!strcmp_suffix(prop_name, ",nr-gpios")) > > + return NULL; > > Ah I somehow missed this patch. This gives a blanked exception for > vendor,nr-gpios. I'd prefer explicit exceptions for all the instances > of ",nr-gpios" we are grandfathering in. Any future additions should > be rejected. Can we do that please? > > Rob, you okay with making this list more explicit?
Not the kernel's job IMO. A schema is the right way to handle that.
Ok, that's fine by me. Btw, let's land this in driver-core? I've made changes there and this might cause conflicts. Not sure.
It merges with linux-next fine. You'll need to resend this to Greg if you want to do that.
Reviewed-by: Rob Herring robh@kernel.org
Hi Greg,
Can you pull this into driver-core please?
Do you want me to re-spin on top of driver-core? The patch is currently based on dt/next in robh/linux.git
I did say you need to resend the patch to Greg, but since there's no movement on this and I have other things to send upstream, I've applied it.
:'(
If it's not too late, can we please drop it? I'm sure Greg would be okay with picking this up.
-Saravana
On Fri, Apr 09, 2021 at 12:36:36PM -0700, Saravana Kannan wrote:
On Fri, Apr 9, 2021 at 12:26 PM Rob Herring robh@kernel.org wrote:
On Wed, Apr 7, 2021 at 3:45 PM Ilya Lipnitskiy ilya.lipnitskiy@gmail.com wrote:
On Tue, Apr 6, 2021 at 6:24 PM Saravana Kannan saravanak@google.com wrote:
On Tue, Apr 6, 2021 at 6:10 PM Rob Herring robh@kernel.org wrote:
On Tue, Apr 6, 2021 at 7:46 PM Saravana Kannan saravanak@google.com wrote:
On Tue, Apr 6, 2021 at 5:34 PM Rob Herring robh@kernel.org wrote: > > On Tue, Apr 06, 2021 at 04:09:10PM -0700, Saravana Kannan wrote: > > On Mon, Apr 5, 2021 at 3:26 PM Ilya Lipnitskiy > > ilya.lipnitskiy@gmail.com wrote: > > > > > > [<vendor>,]nr-gpios property is used by some GPIO drivers[0] to indicate > > > the number of GPIOs present on a system, not define a GPIO. nr-gpios is > > > not configured by #gpio-cells and can't be parsed along with other > > > "*-gpios" properties. > > > > > > nr-gpios without the "<vendor>," prefix is not allowed by the DT > > > spec[1], so only add exception for the ",nr-gpios" suffix and let the > > > error message continue being printed for non-compliant implementations. > > > > > > [0]: nr-gpios is referenced in Documentation/devicetree/bindings/gpio: > > > - gpio-adnp.txt > > > - gpio-xgene-sb.txt > > > - gpio-xlp.txt > > > - snps,dw-apb-gpio.yaml > > > > > > [1]: > > > Link: https://github.com/devicetree-org/dt-schema/blob/cb53a16a1eb3e2169ce170c071e... > > > > > > Fixes errors such as: > > > OF: /palmbus@300000/gpio@600: could not find phandle > > > > > > Fixes: 7f00be96f125 ("of: property: Add device link support for interrupt-parent, dmas and -gpio(s)") > > > Signed-off-by: Ilya Lipnitskiy ilya.lipnitskiy@gmail.com > > > Cc: Saravana Kannan saravanak@google.com > > > Cc: stable@vger.kernel.org # 5.5.x > > > --- > > > drivers/of/property.c | 11 ++++++++++- > > > 1 file changed, 10 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/of/property.c b/drivers/of/property.c > > > index 2046ae311322..1793303e84ac 100644 > > > --- a/drivers/of/property.c > > > +++ b/drivers/of/property.c > > > @@ -1281,7 +1281,16 @@ DEFINE_SIMPLE_PROP(pinctrl7, "pinctrl-7", NULL) > > > DEFINE_SIMPLE_PROP(pinctrl8, "pinctrl-8", NULL) > > > DEFINE_SUFFIX_PROP(regulators, "-supply", NULL) > > > DEFINE_SUFFIX_PROP(gpio, "-gpio", "#gpio-cells") > > > -DEFINE_SUFFIX_PROP(gpios, "-gpios", "#gpio-cells") > > > + > > > +static struct device_node *parse_gpios(struct device_node *np, > > > + const char *prop_name, int index) > > > +{ > > > + if (!strcmp_suffix(prop_name, ",nr-gpios")) > > > + return NULL; > > > > Ah I somehow missed this patch. This gives a blanked exception for > > vendor,nr-gpios. I'd prefer explicit exceptions for all the instances > > of ",nr-gpios" we are grandfathering in. Any future additions should > > be rejected. Can we do that please? > > > > Rob, you okay with making this list more explicit? > > Not the kernel's job IMO. A schema is the right way to handle that.
Ok, that's fine by me. Btw, let's land this in driver-core? I've made changes there and this might cause conflicts. Not sure.
It merges with linux-next fine. You'll need to resend this to Greg if you want to do that.
Reviewed-by: Rob Herring robh@kernel.org
Hi Greg,
Can you pull this into driver-core please?
Do you want me to re-spin on top of driver-core? The patch is currently based on dt/next in robh/linux.git
I did say you need to resend the patch to Greg, but since there's no movement on this and I have other things to send upstream, I've applied it.
:'(
If it's not too late, can we please drop it? I'm sure Greg would be okay with picking this up.
It's in Linus's tree, why does it matter who sends it in?
{sigh}
On Tue, Apr 06, 2021 at 06:24:21PM -0700, Saravana Kannan wrote:
On Tue, Apr 6, 2021 at 6:10 PM Rob Herring robh@kernel.org wrote:
On Tue, Apr 6, 2021 at 7:46 PM Saravana Kannan saravanak@google.com wrote:
On Tue, Apr 6, 2021 at 5:34 PM Rob Herring robh@kernel.org wrote:
On Tue, Apr 06, 2021 at 04:09:10PM -0700, Saravana Kannan wrote:
On Mon, Apr 5, 2021 at 3:26 PM Ilya Lipnitskiy ilya.lipnitskiy@gmail.com wrote:
[<vendor>,]nr-gpios property is used by some GPIO drivers[0] to indicate the number of GPIOs present on a system, not define a GPIO. nr-gpios is not configured by #gpio-cells and can't be parsed along with other "*-gpios" properties.
nr-gpios without the "<vendor>," prefix is not allowed by the DT spec[1], so only add exception for the ",nr-gpios" suffix and let the error message continue being printed for non-compliant implementations.
[0]: nr-gpios is referenced in Documentation/devicetree/bindings/gpio:
- gpio-adnp.txt
- gpio-xgene-sb.txt
- gpio-xlp.txt
- snps,dw-apb-gpio.yaml
[1]: Link: https://github.com/devicetree-org/dt-schema/blob/cb53a16a1eb3e2169ce170c071e...
Fixes errors such as: OF: /palmbus@300000/gpio@600: could not find phandle
Fixes: 7f00be96f125 ("of: property: Add device link support for interrupt-parent, dmas and -gpio(s)") Signed-off-by: Ilya Lipnitskiy ilya.lipnitskiy@gmail.com Cc: Saravana Kannan saravanak@google.com Cc: stable@vger.kernel.org # 5.5.x
drivers/of/property.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/of/property.c b/drivers/of/property.c index 2046ae311322..1793303e84ac 100644 --- a/drivers/of/property.c +++ b/drivers/of/property.c @@ -1281,7 +1281,16 @@ DEFINE_SIMPLE_PROP(pinctrl7, "pinctrl-7", NULL) DEFINE_SIMPLE_PROP(pinctrl8, "pinctrl-8", NULL) DEFINE_SUFFIX_PROP(regulators, "-supply", NULL) DEFINE_SUFFIX_PROP(gpio, "-gpio", "#gpio-cells") -DEFINE_SUFFIX_PROP(gpios, "-gpios", "#gpio-cells")
+static struct device_node *parse_gpios(struct device_node *np,
const char *prop_name, int index)+{
if (!strcmp_suffix(prop_name, ",nr-gpios"))return NULL;Ah I somehow missed this patch. This gives a blanked exception for vendor,nr-gpios. I'd prefer explicit exceptions for all the instances of ",nr-gpios" we are grandfathering in. Any future additions should be rejected. Can we do that please?
Rob, you okay with making this list more explicit?
Not the kernel's job IMO. A schema is the right way to handle that.
Ok, that's fine by me. Btw, let's land this in driver-core? I've made changes there and this might cause conflicts. Not sure.
It merges with linux-next fine. You'll need to resend this to Greg if you want to do that.
Reviewed-by: Rob Herring robh@kernel.org
Hi Greg,
Can you pull this into driver-core please? I touch this file a lot and might need to do so again if any fw_devlink=on issues come up. So trying to preemptively avoid conflicts.
Pull what? I'm totally lost in this thread, sorry...
If you need me to apply something, you at least need to cc: me on it :)
thanks,
gre gk-h
linux-stable-mirror@lists.linaro.org
-
Greg Kroah-Hartman -
Ilya Lipnitskiy -
Rob Herring -
Saravana Kannan