Since we have generic function to count GPIO controller nodes
under a given device, there is no need to open code it. Replace
custom code by gpiochip_node_count() call.
Signed-off-by: Andy Shevchenko <[email protected]>
Reviewed-by: Neil Armstrong <[email protected]>
---
drivers/pinctrl/meson/pinctrl-meson.c | 28 ++++++++++++---------------
1 file changed, 12 insertions(+), 16 deletions(-)
diff --git a/drivers/pinctrl/meson/pinctrl-meson.c b/drivers/pinctrl/meson/pinctrl-meson.c
index 5b46a0979db7..1b078da81523 100644
--- a/drivers/pinctrl/meson/pinctrl-meson.c
+++ b/drivers/pinctrl/meson/pinctrl-meson.c
@@ -49,6 +49,7 @@
#include <linux/pinctrl/pinctrl.h>
#include <linux/pinctrl/pinmux.h>
#include <linux/platform_device.h>
+#include <linux/property.h>
#include <linux/regmap.h>
#include <linux/seq_file.h>
@@ -662,27 +663,22 @@ static struct regmap *meson_map_resource(struct meson_pinctrl *pc,
return devm_regmap_init_mmio(pc->dev, base, &meson_regmap_config);
}
-static int meson_pinctrl_parse_dt(struct meson_pinctrl *pc,
- struct device_node *node)
+static int meson_pinctrl_parse_dt(struct meson_pinctrl *pc)
{
- struct device_node *np, *gpio_np = NULL;
+ struct device_node *gpio_np;
+ unsigned int chips;
- for_each_child_of_node(node, np) {
- if (!of_find_property(np, "gpio-controller", NULL))
- continue;
- if (gpio_np) {
- dev_err(pc->dev, "multiple gpio nodes\n");
- of_node_put(np);
- return -EINVAL;
- }
- gpio_np = np;
- }
-
- if (!gpio_np) {
+ chips = gpiochip_node_count(pc->dev);
+ if (!chips) {
dev_err(pc->dev, "no gpio node found\n");
return -EINVAL;
}
+ if (chips > 1) {
+ dev_err(pc->dev, "multiple gpio nodes\n");
+ return -EINVAL;
+ }
+ gpio_np = to_of_node(device_get_named_child_node(pc->dev, "gpio-controller"));
pc->of_node = gpio_np;
pc->reg_mux = meson_map_resource(pc, gpio_np, "mux");
@@ -751,7 +747,7 @@ int meson_pinctrl_probe(struct platform_device *pdev)
pc->dev = dev;
pc->data = (struct meson_pinctrl_data *) of_device_get_match_data(dev);
- ret = meson_pinctrl_parse_dt(pc, dev->of_node);
+ ret = meson_pinctrl_parse_dt(pc);
if (ret)
return ret;
--
2.35.1
On Thu, Apr 14, 2022 at 07:06:21PM +0300, Andy Shevchenko wrote:
> On Thu, Apr 14, 2022 at 6:32 PM Martin Blumenstingl
> <[email protected]> wrote:
> > On Thu, Apr 14, 2022 at 3:51 PM Andy Shevchenko
> > <[email protected]> wrote:
> > [...]
> > > > This patch landed in linux next-20220413 as commit 88834c75cae5
> > > > ("pinctrl: meson: Replace custom code by gpiochip_node_count() call").
> > > > Unfortunately it breaks booting of all my Amlogic-based test boards
> > > > (Odroid C4, N2, Khadas VIM3, VIM3l). MMC driver is no longer probed and
> > > > boards are unable to mount rootfs. Reverting this patch on top of
> > > > linux-next fixes the issue.
> > >
> > > Thank you for letting me know, I'll withdraw it and investigate.
> > If needed I can investigate further later today/tomorrow. I think the
> > problem is that our node name doesn't follow the .dts recommendation.
> >
> > For GXL (arch/arm64/boot/dts/amlogic/meson-gxl.dtsi) the GPIO
> > controller nodes are for example:
> > gpio: bank@4b0 {
> > ...
> > }
> > and
> > gpio_ao: bank@14 {
> > ...
> > }
> >
> > See also:
> > $ git grep -C6 gpio-controller arch/arm64/boot/dts/amlogic/*.dtsi
> >
> > Marek did not state which error he's getting but I suspect it fails
> > with "no gpio node found".
>
> Would be interesting to know that, yeah.
>
> The subtle difference between the patched and unpatched version is
> that the former uses only available nodes, it means that node is not
> available by some reason and then the error would be the one you
> guessed.
Looking into the difference between iterating via available nodes I have found
nothing suspicious. Your DTSes do not have status property, so it assumes the
node is available.
I'm quite puzzled what's going on there. Because I can't see what the logical
difference the patch brought in.
P.S. In any case it's withdrawn now and shouldn't appear in the next Linux
Next. But I would really appreciate more input on this.
--
With Best Regards,
Andy Shevchenko
Hi
On 01.04.2022 12:36, Andy Shevchenko wrote:
> Since we have generic function to count GPIO controller nodes
> under a given device, there is no need to open code it. Replace
> custom code by gpiochip_node_count() call.
>
> Signed-off-by: Andy Shevchenko <[email protected]>
> Reviewed-by: Neil Armstrong <[email protected]>
This patch landed in linux next-20220413 as commit 88834c75cae5
("pinctrl: meson: Replace custom code by gpiochip_node_count() call").
Unfortunately it breaks booting of all my Amlogic-based test boards
(Odroid C4, N2, Khadas VIM3, VIM3l). MMC driver is no longer probed and
boards are unable to mount rootfs. Reverting this patch on top of
linux-next fixes the issue.
> ---
> drivers/pinctrl/meson/pinctrl-meson.c | 28 ++++++++++++---------------
> 1 file changed, 12 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/pinctrl/meson/pinctrl-meson.c b/drivers/pinctrl/meson/pinctrl-meson.c
> index 5b46a0979db7..1b078da81523 100644
> --- a/drivers/pinctrl/meson/pinctrl-meson.c
> +++ b/drivers/pinctrl/meson/pinctrl-meson.c
> @@ -49,6 +49,7 @@
> #include <linux/pinctrl/pinctrl.h>
> #include <linux/pinctrl/pinmux.h>
> #include <linux/platform_device.h>
> +#include <linux/property.h>
> #include <linux/regmap.h>
> #include <linux/seq_file.h>
>
> @@ -662,27 +663,22 @@ static struct regmap *meson_map_resource(struct meson_pinctrl *pc,
> return devm_regmap_init_mmio(pc->dev, base, &meson_regmap_config);
> }
>
> -static int meson_pinctrl_parse_dt(struct meson_pinctrl *pc,
> - struct device_node *node)
> +static int meson_pinctrl_parse_dt(struct meson_pinctrl *pc)
> {
> - struct device_node *np, *gpio_np = NULL;
> + struct device_node *gpio_np;
> + unsigned int chips;
>
> - for_each_child_of_node(node, np) {
> - if (!of_find_property(np, "gpio-controller", NULL))
> - continue;
> - if (gpio_np) {
> - dev_err(pc->dev, "multiple gpio nodes\n");
> - of_node_put(np);
> - return -EINVAL;
> - }
> - gpio_np = np;
> - }
> -
> - if (!gpio_np) {
> + chips = gpiochip_node_count(pc->dev);
> + if (!chips) {
> dev_err(pc->dev, "no gpio node found\n");
> return -EINVAL;
> }
> + if (chips > 1) {
> + dev_err(pc->dev, "multiple gpio nodes\n");
> + return -EINVAL;
> + }
>
> + gpio_np = to_of_node(device_get_named_child_node(pc->dev, "gpio-controller"));
> pc->of_node = gpio_np;
>
> pc->reg_mux = meson_map_resource(pc, gpio_np, "mux");
> @@ -751,7 +747,7 @@ int meson_pinctrl_probe(struct platform_device *pdev)
> pc->dev = dev;
> pc->data = (struct meson_pinctrl_data *) of_device_get_match_data(dev);
>
> - ret = meson_pinctrl_parse_dt(pc, dev->of_node);
> + ret = meson_pinctrl_parse_dt(pc);
> if (ret)
> return ret;
>
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
Hi Andy,
On Thu, Apr 14, 2022 at 3:51 PM Andy Shevchenko
<[email protected]> wrote:
[...]
> > This patch landed in linux next-20220413 as commit 88834c75cae5
> > ("pinctrl: meson: Replace custom code by gpiochip_node_count() call").
> > Unfortunately it breaks booting of all my Amlogic-based test boards
> > (Odroid C4, N2, Khadas VIM3, VIM3l). MMC driver is no longer probed and
> > boards are unable to mount rootfs. Reverting this patch on top of
> > linux-next fixes the issue.
>
> Thank you for letting me know, I'll withdraw it and investigate.
If needed I can investigate further later today/tomorrow. I think the
problem is that our node name doesn't follow the .dts recommendation.
For GXL (arch/arm64/boot/dts/amlogic/meson-gxl.dtsi) the GPIO
controller nodes are for example:
gpio: bank@4b0 {
...
}
and
gpio_ao: bank@14 {
...
}
See also:
$ git grep -C6 gpio-controller arch/arm64/boot/dts/amlogic/*.dtsi
Marek did not state which error he's getting but I suspect it fails
with "no gpio node found".
Best regards,
Martin
On Thu, Apr 14, 2022 at 6:32 PM Martin Blumenstingl
<[email protected]> wrote:
> On Thu, Apr 14, 2022 at 3:51 PM Andy Shevchenko
> <[email protected]> wrote:
> [...]
> > > This patch landed in linux next-20220413 as commit 88834c75cae5
> > > ("pinctrl: meson: Replace custom code by gpiochip_node_count() call").
> > > Unfortunately it breaks booting of all my Amlogic-based test boards
> > > (Odroid C4, N2, Khadas VIM3, VIM3l). MMC driver is no longer probed and
> > > boards are unable to mount rootfs. Reverting this patch on top of
> > > linux-next fixes the issue.
> >
> > Thank you for letting me know, I'll withdraw it and investigate.
> If needed I can investigate further later today/tomorrow. I think the
> problem is that our node name doesn't follow the .dts recommendation.
>
> For GXL (arch/arm64/boot/dts/amlogic/meson-gxl.dtsi) the GPIO
> controller nodes are for example:
> gpio: bank@4b0 {
> ...
> }
> and
> gpio_ao: bank@14 {
> ...
> }
>
> See also:
> $ git grep -C6 gpio-controller arch/arm64/boot/dts/amlogic/*.dtsi
>
> Marek did not state which error he's getting but I suspect it fails
> with "no gpio node found".
Would be interesting to know that, yeah.
The subtle difference between the patched and unpatched version is
that the former uses only available nodes, it means that node is not
available by some reason and then the error would be the one you
guessed.
--
With Best Regards,
Andy Shevchenko
On Thu, Apr 14, 2022 at 12:44 PM Marek Szyprowski
<[email protected]> wrote:
> On 01.04.2022 12:36, Andy Shevchenko wrote:
> > Since we have generic function to count GPIO controller nodes
> > under a given device, there is no need to open code it. Replace
> > custom code by gpiochip_node_count() call.
...
> This patch landed in linux next-20220413 as commit 88834c75cae5
> ("pinctrl: meson: Replace custom code by gpiochip_node_count() call").
> Unfortunately it breaks booting of all my Amlogic-based test boards
> (Odroid C4, N2, Khadas VIM3, VIM3l). MMC driver is no longer probed and
> boards are unable to mount rootfs. Reverting this patch on top of
> linux-next fixes the issue.
Thank you for letting me know, I'll withdraw it and investigate.
--
With Best Regards,
Andy Shevchenko
On Thu, Apr 14, 2022 at 09:28:30PM +0300, Andy Shevchenko wrote:
> On Thu, Apr 14, 2022 at 07:06:21PM +0300, Andy Shevchenko wrote:
> > On Thu, Apr 14, 2022 at 6:32 PM Martin Blumenstingl
> > <[email protected]> wrote:
> > > On Thu, Apr 14, 2022 at 3:51 PM Andy Shevchenko
> > > <[email protected]> wrote:
> > > [...]
> > > > > This patch landed in linux next-20220413 as commit 88834c75cae5
> > > > > ("pinctrl: meson: Replace custom code by gpiochip_node_count() call").
> > > > > Unfortunately it breaks booting of all my Amlogic-based test boards
> > > > > (Odroid C4, N2, Khadas VIM3, VIM3l). MMC driver is no longer probed and
> > > > > boards are unable to mount rootfs. Reverting this patch on top of
> > > > > linux-next fixes the issue.
> > > >
> > > > Thank you for letting me know, I'll withdraw it and investigate.
> > > If needed I can investigate further later today/tomorrow. I think the
> > > problem is that our node name doesn't follow the .dts recommendation.
> > >
> > > For GXL (arch/arm64/boot/dts/amlogic/meson-gxl.dtsi) the GPIO
> > > controller nodes are for example:
> > > gpio: bank@4b0 {
> > > ...
> > > }
> > > and
> > > gpio_ao: bank@14 {
> > > ...
> > > }
> > >
> > > See also:
> > > $ git grep -C6 gpio-controller arch/arm64/boot/dts/amlogic/*.dtsi
> > >
> > > Marek did not state which error he's getting but I suspect it fails
> > > with "no gpio node found".
> >
> > Would be interesting to know that, yeah.
> >
> > The subtle difference between the patched and unpatched version is
> > that the former uses only available nodes, it means that node is not
> > available by some reason and then the error would be the one you
> > guessed.
>
> Looking into the difference between iterating via available nodes I have found
> nothing suspicious. Your DTSes do not have status property, so it assumes the
> node is available.
>
> I'm quite puzzled what's going on there. Because I can't see what the logical
> difference the patch brought in.
>
> P.S. In any case it's withdrawn now and shouldn't appear in the next Linux
> Next. But I would really appreciate more input on this.
Okay, now I got it. The "name" of the node is not the same as containing the
property with a given name. So, the faulting line of the code is this one:
gpio_np = to_of_node(device_get_named_child_node(pc->dev, "gpio-controller"));
--
With Best Regards,
Andy Shevchenko