From: Geert Uytterhoeven <geert@linux-m68k.org>
To: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Cc: Linus Walleij <linusw@kernel.org>,
Bartosz Golaszewski <brgl@kernel.org>,
Frank Rowand <frowand.list@gmail.com>,
Mika Westerberg <westeri@kernel.org>,
Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
Aaro Koskinen <aaro.koskinen@iki.fi>,
Janusz Krzysztofik <jmkrzyszt@gmail.com>,
Tony Lindgren <tony@atomide.com>,
Russell King <linux@armlinux.org.uk>,
Jonathan Corbet <corbet@lwn.net>,
Shuah Khan <skhan@linuxfoundation.org>,
linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-acpi@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-omap@vger.kernel.org, linux-doc@vger.kernel.org,
Mika Westerberg <mika.westerberg@linux.intel.com>
Subject: Re: [PATCH v2 2/6] gpio: move hogs into GPIO core
Date: Tue, 24 Mar 2026 17:16:19 +0100 [thread overview]
Message-ID: <CAMuHMdX6RuZXAozrF5m625ZepJTVVr4pcyKczSk12MedWvoejw@mail.gmail.com> (raw)
In-Reply-To: <20260309-gpio-hog-fwnode-v2-2-4e61f3dbf06a@oss.qualcomm.com>
Hi Bartosz,
On Mon, 9 Mar 2026 at 13:43, Bartosz Golaszewski
<bartosz.golaszewski@oss.qualcomm.com> wrote:
> Refactor line hogging code by moving the parts duplicated in
> gpiolib-acpi-core.c and gpiolib-of.c into gpiolib.c, leaving just the
> OF-specific bits in the latter.
>
> This makes fwnode the primary API for setting up hogs and allows to use
> software nodes in addition to ACPI and OF nodes.
>
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Reviewed-by: Linus Walleij <linusw@kernel.org>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Thanks for your patch, which is now commit d1d564ec4992945d ("gpio:
move hogs into GPIO core") in gpio/gpio/for-next.
This breaks GPIO on the Renesas Marzen (R-Car H1) board:
-gpio_rcar ffc40000.gpio: driving 32 GPIOs
+gpiochip_add_data_with_key: GPIOs 512..543 (ffc40000.gpio) failed
to register, -22
+gpio_rcar ffc40000.gpio: failed to add GPIO controller
+gpio_rcar ffc40000.gpio: probe with driver gpio_rcar failed with error -22
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -960,6 +960,98 @@ static void machine_gpiochip_add(struct gpio_chip *gc)
> }
> }
>
> +int gpiochip_add_hog(struct gpio_chip *gc, struct fwnode_handle *fwnode)
> +{
> + struct fwnode_handle *gc_node = dev_fwnode(&gc->gpiodev->dev);
> + struct fwnode_reference_args gpiospec;
> + enum gpiod_flags dflags;
> + struct gpio_desc *desc;
> + unsigned long lflags;
> + const char *name;
> + int ret, argc;
> + u32 gpios[3]; /* We support up to three-cell bindings. */
> + u32 cells;
> +
> + lflags = GPIO_LOOKUP_FLAGS_DEFAULT;
> + dflags = GPIOD_ASIS;
> + name = NULL;
> +
> + argc = fwnode_property_count_u32(fwnode, "gpios");
> + if (argc < 0)
> + return argc;
> + if (argc > 3)
> + return -EINVAL;
The GPIO hog at [1] has a gpios property with two entries:
gpios = <17 GPIO_ACTIVE_LOW>, <18 GPIO_ACTIVE_LOW>;
hence argc = 4, which is considered invalid.
The check should take into account the actual value of #gpio-cells,
and handle the presence of multiple GPIOs.
"git grep -Ww gpio-hog -- arch/*/boot/dts/ | grep 'gpios\s*=.*>,'" finds
several other places where multiple GPIOs are specified.
> +
> + ret = fwnode_property_read_u32_array(fwnode, "gpios", gpios, argc);
> + if (ret < 0)
> + return ret;
> +
> + if (is_of_node(fwnode)) {
> + /*
> + * OF-nodes need some additional special handling for
> + * translating of devicetree flags.
> + */
> + ret = fwnode_property_read_u32(gc_node, "#gpio-cells", &cells);
> + if (ret)
> + return ret;
> + if (!ret && argc != cells)
> + return -EINVAL;
> +
> + memset(&gpiospec, 0, sizeof(gpiospec));
> + gpiospec.fwnode = fwnode;
> + gpiospec.nargs = argc;
> +
> + for (int i = 0; i < argc; i++)
> + gpiospec.args[i] = gpios[i];
> +
> + ret = of_gpiochip_get_lflags(gc, &gpiospec, &lflags);
> + if (ret)
> + return ret;
> + } else {
> + /*
> + * GPIO_ACTIVE_LOW is currently the only lookup flag
> + * supported for non-OF firmware nodes.
> + */
> + if (gpios[1])
> + lflags |= GPIO_ACTIVE_LOW;
> + }
> +
> + if (fwnode_property_present(fwnode, "input"))
> + dflags |= GPIOD_IN;
> + else if (fwnode_property_present(fwnode, "output-low"))
> + dflags |= GPIOD_OUT_LOW;
> + else if (fwnode_property_present(fwnode, "output-high"))
> + dflags |= GPIOD_OUT_HIGH;
> + else
> + return -EINVAL;
> +
> + fwnode_property_read_string(fwnode, "line-name", &name);
> +
> + desc = gpiochip_get_desc(gc, gpios[0]);
> + if (IS_ERR(desc))
> + return PTR_ERR(desc);
> +
> + return gpiod_hog(desc, name, lflags, dflags);
> +}
[1] https://elixir.bootlin.com/linux/v6.19.9/source/arch/arm/boot/dts/renesas/r8a7779-marzen.dts#L196
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
next prev parent reply other threads:[~2026-03-24 16:16 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-09 12:42 [PATCH v2 0/6] gpiolib: unify gpio-hog code Bartosz Golaszewski
2026-03-09 12:42 ` [PATCH v2 1/6] gpio: of: clear OF_POPULATED on hog nodes in remove path Bartosz Golaszewski
2026-03-09 12:42 ` [PATCH v2 2/6] gpio: move hogs into GPIO core Bartosz Golaszewski
2026-03-24 16:16 ` Geert Uytterhoeven [this message]
2026-03-09 12:42 ` [PATCH v2 3/6] gpio: sim: use fwnode-based GPIO hogs Bartosz Golaszewski
2026-03-09 12:42 ` [PATCH v2 4/6] ARM: omap1: ams-delta: convert GPIO hogs to using firmware nodes Bartosz Golaszewski
2026-03-09 12:42 ` [PATCH v2 5/6] gpio: remove machine hogs Bartosz Golaszewski
2026-03-09 12:42 ` [PATCH v2 6/6] gpio: sim: allow to define the active-low setting of a simulated hog Bartosz Golaszewski
2026-03-16 9:11 ` [PATCH v2 0/6] gpiolib: unify gpio-hog code Bartosz Golaszewski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAMuHMdX6RuZXAozrF5m625ZepJTVVr4pcyKczSk12MedWvoejw@mail.gmail.com \
--to=geert@linux-m68k.org \
--cc=aaro.koskinen@iki.fi \
--cc=andriy.shevchenko@linux.intel.com \
--cc=bartosz.golaszewski@oss.qualcomm.com \
--cc=brgl@kernel.org \
--cc=corbet@lwn.net \
--cc=frowand.list@gmail.com \
--cc=jmkrzyszt@gmail.com \
--cc=linusw@kernel.org \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-omap@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=mika.westerberg@linux.intel.com \
--cc=skhan@linuxfoundation.org \
--cc=tony@atomide.com \
--cc=westeri@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).