* [PATCH] of: property: do not create device links from *nr-gpios
@ 2021-04-05 3:14 Ilya Lipnitskiy
2021-04-05 20:00 ` Saravana Kannan
2021-04-05 22:25 ` [PATCH v2] of: property: fw_devlink: do not link ".*,nr-gpios" Ilya Lipnitskiy
0 siblings, 2 replies; 20+ messages in thread
From: Ilya Lipnitskiy @ 2021-04-05 3:14 UTC (permalink / raw)
To: Rob Herring, Frank Rowand, devicetree, linux-kernel
Cc: Ilya Lipnitskiy, Saravana Kannan, stable
[<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)
--
2.31.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] of: property: do not create device links from *nr-gpios
2021-04-05 3:14 [PATCH] of: property: do not create device links from *nr-gpios Ilya Lipnitskiy
@ 2021-04-05 20:00 ` Saravana Kannan
2021-04-05 20:10 ` Ilya Lipnitskiy
2021-04-05 22:25 ` [PATCH v2] of: property: fw_devlink: do not link ".*,nr-gpios" Ilya Lipnitskiy
1 sibling, 1 reply; 20+ messages in thread
From: Saravana Kannan @ 2021-04-05 20:00 UTC (permalink / raw)
To: Ilya Lipnitskiy
Cc: Rob Herring, Frank Rowand,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
stable
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
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] of: property: do not create device links from *nr-gpios
2021-04-05 20:00 ` Saravana Kannan
@ 2021-04-05 20:10 ` Ilya Lipnitskiy
2021-04-05 20:18 ` Saravana Kannan
0 siblings, 1 reply; 20+ messages in thread
From: Ilya Lipnitskiy @ 2021-04-05 20:10 UTC (permalink / raw)
To: Saravana Kannan
Cc: Rob Herring, Frank Rowand,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
stable
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] of: property: do not create device links from *nr-gpios
2021-04-05 20:10 ` Ilya Lipnitskiy
@ 2021-04-05 20:18 ` Saravana Kannan
2021-04-05 20:55 ` Ilya Lipnitskiy
2021-04-06 17:40 ` Rob Herring
0 siblings, 2 replies; 20+ messages in thread
From: Saravana Kannan @ 2021-04-05 20:18 UTC (permalink / raw)
To: Ilya Lipnitskiy
Cc: Rob Herring, Frank Rowand,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
stable
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-consumer.yaml#L20
-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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] of: property: do not create device links from *nr-gpios
2021-04-05 20:18 ` Saravana Kannan
@ 2021-04-05 20:55 ` Ilya Lipnitskiy
2021-04-06 17:40 ` Rob Herring
1 sibling, 0 replies; 20+ messages in thread
From: Ilya Lipnitskiy @ 2021-04-05 20:55 UTC (permalink / raw)
To: Saravana Kannan
Cc: Rob Herring, Frank Rowand,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
stable
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-consumer.yaml#L20
Thanks for linking the spec. I can re-spin the patch with ",nr-gpios"
as the special suffix to align with the spec.
Ilya
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2] of: property: fw_devlink: do not link ".*,nr-gpios"
2021-04-05 3:14 [PATCH] of: property: do not create device links from *nr-gpios Ilya Lipnitskiy
2021-04-05 20:00 ` Saravana Kannan
@ 2021-04-05 22:25 ` Ilya Lipnitskiy
2021-04-06 23:09 ` Saravana Kannan
1 sibling, 1 reply; 20+ messages in thread
From: Ilya Lipnitskiy @ 2021-04-05 22:25 UTC (permalink / raw)
To: Rob Herring, Frank Rowand, devicetree, linux-kernel
Cc: Ilya Lipnitskiy, Saravana Kannan, stable
[<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/cb53a16a1eb3e2169ce170c071e47940845ec26e/schemas/gpio/gpio-consumer.yaml#L20
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)
--
2.31.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] of: property: do not create device links from *nr-gpios
2021-04-05 20:18 ` Saravana Kannan
2021-04-05 20:55 ` Ilya Lipnitskiy
@ 2021-04-06 17:40 ` Rob Herring
2021-04-06 19:28 ` Ilya Lipnitskiy
1 sibling, 1 reply; 20+ messages in thread
From: Rob Herring @ 2021-04-06 17:40 UTC (permalink / raw)
To: Saravana Kannan
Cc: Ilya Lipnitskiy, Frank Rowand,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
stable
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] of: property: do not create device links from *nr-gpios
2021-04-06 17:40 ` Rob Herring
@ 2021-04-06 19:28 ` Ilya Lipnitskiy
2021-04-06 21:28 ` Saravana Kannan
0 siblings, 1 reply; 20+ messages in thread
From: Ilya Lipnitskiy @ 2021-04-06 19:28 UTC (permalink / raw)
To: Rob Herring
Cc: Saravana Kannan, Frank Rowand,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
stable
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] of: property: do not create device links from *nr-gpios
2021-04-06 19:28 ` Ilya Lipnitskiy
@ 2021-04-06 21:28 ` Saravana Kannan
2021-04-06 22:31 ` Rob Herring
0 siblings, 1 reply; 20+ messages in thread
From: Saravana Kannan @ 2021-04-06 21:28 UTC (permalink / raw)
To: Ilya Lipnitskiy
Cc: Rob Herring, Frank Rowand,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
stable
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] of: property: do not create device links from *nr-gpios
2021-04-06 21:28 ` Saravana Kannan
@ 2021-04-06 22:31 ` Rob Herring
0 siblings, 0 replies; 20+ messages in thread
From: Rob Herring @ 2021-04-06 22:31 UTC (permalink / raw)
To: Saravana Kannan
Cc: Ilya Lipnitskiy, Frank Rowand,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
stable
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:
> 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).
Yes, but (3) is not gated on (1) and (2). I'm applying v2.
Rob
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] of: property: fw_devlink: do not link ".*,nr-gpios"
2021-04-05 22:25 ` [PATCH v2] of: property: fw_devlink: do not link ".*,nr-gpios" Ilya Lipnitskiy
@ 2021-04-06 23:09 ` Saravana Kannan
2021-04-07 0:34 ` Rob Herring
0 siblings, 1 reply; 20+ messages in thread
From: Saravana Kannan @ 2021-04-06 23:09 UTC (permalink / raw)
To: Ilya Lipnitskiy
Cc: Rob Herring, Frank Rowand,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
stable
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/cb53a16a1eb3e2169ce170c071e47940845ec26e/schemas/gpio/gpio-consumer.yaml#L20
>
> 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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] of: property: fw_devlink: do not link ".*,nr-gpios"
2021-04-06 23:09 ` Saravana Kannan
@ 2021-04-07 0:34 ` Rob Herring
2021-04-07 0:45 ` Saravana Kannan
0 siblings, 1 reply; 20+ messages in thread
From: Rob Herring @ 2021-04-07 0:34 UTC (permalink / raw)
To: Saravana Kannan
Cc: Ilya Lipnitskiy, Frank Rowand,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
stable
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/cb53a16a1eb3e2169ce170c071e47940845ec26e/schemas/gpio/gpio-consumer.yaml#L20
> >
> > 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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] of: property: fw_devlink: do not link ".*,nr-gpios"
2021-04-07 0:34 ` Rob Herring
@ 2021-04-07 0:45 ` Saravana Kannan
2021-04-07 1:10 ` Rob Herring
0 siblings, 1 reply; 20+ messages in thread
From: Saravana Kannan @ 2021-04-07 0:45 UTC (permalink / raw)
To: Rob Herring
Cc: Ilya Lipnitskiy, Frank Rowand,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
stable
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/cb53a16a1eb3e2169ce170c071e47940845ec26e/schemas/gpio/gpio-consumer.yaml#L20
> > >
> > > 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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] of: property: fw_devlink: do not link ".*,nr-gpios"
2021-04-07 0:45 ` Saravana Kannan
@ 2021-04-07 1:10 ` Rob Herring
2021-04-07 1:24 ` Saravana Kannan
0 siblings, 1 reply; 20+ messages in thread
From: Rob Herring @ 2021-04-07 1:10 UTC (permalink / raw)
To: Saravana Kannan
Cc: Ilya Lipnitskiy, Frank Rowand,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
stable
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/cb53a16a1eb3e2169ce170c071e47940845ec26e/schemas/gpio/gpio-consumer.yaml#L20
> > > >
> > > > 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>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] of: property: fw_devlink: do not link ".*,nr-gpios"
2021-04-07 1:10 ` Rob Herring
@ 2021-04-07 1:24 ` Saravana Kannan
2021-04-07 20:44 ` Ilya Lipnitskiy
2021-04-10 8:56 ` Greg Kroah-Hartman
0 siblings, 2 replies; 20+ messages in thread
From: Saravana Kannan @ 2021-04-07 1:24 UTC (permalink / raw)
To: Rob Herring
Cc: Ilya Lipnitskiy, Frank Rowand,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
stable, Greg Kroah-Hartman
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/cb53a16a1eb3e2169ce170c071e47940845ec26e/schemas/gpio/gpio-consumer.yaml#L20
> > > > >
> > > > > 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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] of: property: fw_devlink: do not link ".*,nr-gpios"
2021-04-07 1:24 ` Saravana Kannan
@ 2021-04-07 20:44 ` Ilya Lipnitskiy
2021-04-09 19:26 ` Rob Herring
2021-04-10 8:56 ` Greg Kroah-Hartman
1 sibling, 1 reply; 20+ messages in thread
From: Ilya Lipnitskiy @ 2021-04-07 20:44 UTC (permalink / raw)
To: Saravana Kannan
Cc: Rob Herring, Frank Rowand,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
stable, Greg Kroah-Hartman
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/cb53a16a1eb3e2169ce170c071e47940845ec26e/schemas/gpio/gpio-consumer.yaml#L20
> > > > > >
> > > > > > 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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] of: property: fw_devlink: do not link ".*,nr-gpios"
2021-04-07 20:44 ` Ilya Lipnitskiy
@ 2021-04-09 19:26 ` Rob Herring
2021-04-09 19:36 ` Saravana Kannan
0 siblings, 1 reply; 20+ messages in thread
From: Rob Herring @ 2021-04-09 19:26 UTC (permalink / raw)
To: Ilya Lipnitskiy
Cc: Saravana Kannan, Frank Rowand,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
stable, Greg Kroah-Hartman
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/cb53a16a1eb3e2169ce170c071e47940845ec26e/schemas/gpio/gpio-consumer.yaml#L20
> > > > > > >
> > > > > > > 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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] of: property: fw_devlink: do not link ".*,nr-gpios"
2021-04-09 19:26 ` Rob Herring
@ 2021-04-09 19:36 ` Saravana Kannan
2021-04-10 12:08 ` Greg Kroah-Hartman
0 siblings, 1 reply; 20+ messages in thread
From: Saravana Kannan @ 2021-04-09 19:36 UTC (permalink / raw)
To: Rob Herring
Cc: Ilya Lipnitskiy, Frank Rowand,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
stable, Greg Kroah-Hartman
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/cb53a16a1eb3e2169ce170c071e47940845ec26e/schemas/gpio/gpio-consumer.yaml#L20
> > > > > > > >
> > > > > > > > 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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] of: property: fw_devlink: do not link ".*,nr-gpios"
2021-04-07 1:24 ` Saravana Kannan
2021-04-07 20:44 ` Ilya Lipnitskiy
@ 2021-04-10 8:56 ` Greg Kroah-Hartman
1 sibling, 0 replies; 20+ messages in thread
From: Greg Kroah-Hartman @ 2021-04-10 8:56 UTC (permalink / raw)
To: Saravana Kannan
Cc: Rob Herring, Ilya Lipnitskiy, Frank Rowand,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
stable
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/cb53a16a1eb3e2169ce170c071e47940845ec26e/schemas/gpio/gpio-consumer.yaml#L20
> > > > > >
> > > > > > 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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] of: property: fw_devlink: do not link ".*,nr-gpios"
2021-04-09 19:36 ` Saravana Kannan
@ 2021-04-10 12:08 ` Greg Kroah-Hartman
0 siblings, 0 replies; 20+ messages in thread
From: Greg Kroah-Hartman @ 2021-04-10 12:08 UTC (permalink / raw)
To: Saravana Kannan
Cc: Rob Herring, Ilya Lipnitskiy, Frank Rowand,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
stable
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/cb53a16a1eb3e2169ce170c071e47940845ec26e/schemas/gpio/gpio-consumer.yaml#L20
> > > > > > > > >
> > > > > > > > > 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}
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2021-04-10 12:08 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-05 3:14 [PATCH] of: property: do not create device links from *nr-gpios Ilya Lipnitskiy
2021-04-05 20:00 ` Saravana Kannan
2021-04-05 20:10 ` Ilya Lipnitskiy
2021-04-05 20:18 ` Saravana Kannan
2021-04-05 20:55 ` Ilya Lipnitskiy
2021-04-06 17:40 ` Rob Herring
2021-04-06 19:28 ` Ilya Lipnitskiy
2021-04-06 21:28 ` Saravana Kannan
2021-04-06 22:31 ` Rob Herring
2021-04-05 22:25 ` [PATCH v2] of: property: fw_devlink: do not link ".*,nr-gpios" Ilya Lipnitskiy
2021-04-06 23:09 ` Saravana Kannan
2021-04-07 0:34 ` Rob Herring
2021-04-07 0:45 ` Saravana Kannan
2021-04-07 1:10 ` Rob Herring
2021-04-07 1:24 ` Saravana Kannan
2021-04-07 20:44 ` Ilya Lipnitskiy
2021-04-09 19:26 ` Rob Herring
2021-04-09 19:36 ` Saravana Kannan
2021-04-10 12:08 ` Greg Kroah-Hartman
2021-04-10 8:56 ` Greg Kroah-Hartman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).