* Re: [PATCH 0/2] pinctrl: mediatek: Enable MT8189 as loadable module
From: Linus Walleij @ 2026-06-08 19:29 UTC (permalink / raw)
To: Justin Yeh
Cc: Sean Wang, Matthias Brugger, AngeloGioacchino Del Regno,
Project_Global_Chrome_Upstream_Group, linux-mediatek, linux-gpio,
linux-kernel, linux-arm-kernel
In-Reply-To: <20260529100308.51271-1-justin.yeh@mediatek.com>
On Fri, May 29, 2026 at 12:04 PM Justin Yeh <justin.yeh@mediatek.com> wrote:
> This series enables PINCTRL_MT8189 to be built as a loadable kernel
> module, which is required for GKI (Generic Kernel Image) + vendor_dlkm
> deployments where vendor-specific drivers must be kept separate from
> the GKI vmlinux.
Maybe tell us that this is for Android?
> Patch 1 restores the tristate option that was recently changed to bool,
No the driver was merged with tristate.
See
commit a3fe1324c3c5c292ec79bd756497c1c44ff247d2 (tag: pinctrl-v6.17-1)
"pinctrl: mediatek: Add pinctrl driver for mt8189"
> preventing module builds. Patch 2 adds the missing MODULE_LICENSE macro
> that's required when building as a module.
So that should be squashed into one patch or it will not bisect.
> Together these changes allow MT8189 pinctrl to be properly packaged as
> a vendor kernel module while maintaining the existing built-in option.
So why do we change this only for MT8189 and not for all
Mediatek pinctrl drivers?
I would just add this tristate option and license to all of them while
you're at it or I will get like 20 identical patches and it's a waste
of time for me.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v2] gpio: mt7621: fix interrupt banks mapping on gpio chips
From: Vicente Bergas @ 2026-06-08 19:21 UTC (permalink / raw)
To: Sergio Paracuellos
Cc: Bartosz Golaszewski, linusw, tglx, grant.likely, linux-kernel,
linux-gpio
In-Reply-To: <CAMhs-H-dnR7KjOZYv+rSt=n4MwPd5O1g9kfoP99uA7TQASf0WQ@mail.gmail.com>
On Mon, Jun 8, 2026 at 4:27 PM Sergio Paracuellos
<sergio.paracuellos@gmail.com> wrote:
>
> On Mon, Jun 8, 2026 at 4:02 PM Bartosz Golaszewski <brgl@kernel.org> wrote:
> >
> > On Mon, 8 Jun 2026 11:40:57 +0200, Sergio Paracuellos
> > <sergio.paracuellos@gmail.com> said:
> > > Hi,
> > >
> > > On Mon, Jun 8, 2026 at 11:05 AM Bartosz Golaszewski <brgl@kernel.org> wrote:
> > >>
> > >> On Tue, 2 Jun 2026 16:25:13 +0200, Sergio Paracuellos
> > >> <sergio.paracuellos@gmail.com> said:
> > >> > The GPIO controller's registers are organized as sets of eight 32-bit
> > >> > registers with each set controlling a bank of up to 32 pins. A single
> > >> > interrupt is shared for all of the banks handled by the controller.
> > >> > The driver implements this using three gpio chip instances every one
> > >> > with its own irq chip. Every single pin can generate interrupts having
> > >> > a total of 96 possible interrupts here. It looks like there is a problem
> > >> > with interrupts being properly mapped to the gpio bank using this solution.
> > >> > This problem report is in the following lore's link [0].
> > >> >
> > >> > Device tree is using two cells for this, so only the interrupt pin and the
> > >> > interrupt type are described there. Changing to have three cells to setup
> > >> > also the bank and implement 'of_node_instance_match()' would also work but
> > >> > this would be an ABI breakage and also a bit incoherent since gpios itself
> > >> > are also using two cells and properly mapped in desired bank using through
> > >> > its pin number on 'of_xlate()'.
> > >> >
> > >> > That said, register a linear IRQ domain of the total of 96 interrupts shared
> > >> > with the three gpio chip instances so the bank and the interrupt is properly
> > >> > decoded and devices using gpio IRQs properly work.
> > >> >
> > >> > [0]: https://lore.kernel.org/linux-gpio/CAAMcf8C_A9dJ_v4QRKtb9eGNOpJ7BZNOGsFP4i2WFOZxOVBPnQ@mail.gmail.com/T/#u
> > >> >
> > >> > Fixes: 4ba9c3afda41 ("gpio: mt7621: Add a driver for MT7621")
> > >> > Co-developed-by: Vicente Bergas <vicencb@gmail.com>
> > >> > Signed-off-by: Vicente Bergas <vicencb@gmail.com>
> > >> > Tested-by: Vicente Bergas <vicencb@gmail.com>
> > >> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > >> > ---
> > >>
> > >> Hi!
> > >>
> > >> Can you look at the sashiko review? Especially the bit about tracking the
> > >> GPIOD_FLAG_IRQ_IS_ENABLED flag looks correct.
> > >
> > > I got rid of those two calls (gpiochip_enable_irq() and
> > > gpiochip_disable_irq()) because the driver "gpio-brcmstb" which is the
> > > one I based my changes on was not used them at all. We have not found
> > > anything weird related with that on testing. I do believe that since
> > > we are using our own callbacks for 'irq_request_resources()' and
> > > 'irq_release_resources()' we are safe here. Regarding the others I am
> > > not sure, but the introduction of the remove stuff for the irq domain
> > > is because there are no devm_* functions for that. Other resources in
> > > driver are using devm versions so I think the changes are ok as they
> > > are...
> > >
> >
> > It's about GPIO core: a GPIO that appears as "free" (users can request it) but
> > was earlier enabled for interrupts cannot be requested in output mode - only
> > input works. Without this flag set, gpiod_direction_output_nonotify() will allow
> > you to set direction to output.
>
> I see. I need Vicente to re-test without removing
> gpiochip_enable_irq() and gpiochip_disable_irq() to see if everything
> is still ok.
>
> Vicente, would you mind to test the following change on top of this v2 PATCH?
>
> diff --git a/drivers/gpio/gpio-mt7621.c b/drivers/gpio/gpio-mt7621.c
> index c36aa0abd0c6..a814885ccd5d 100644
> --- a/drivers/gpio/gpio-mt7621.c
> +++ b/drivers/gpio/gpio-mt7621.c
> @@ -144,6 +144,8 @@ mediatek_gpio_irq_unmask(struct irq_data *d)
> u32 mask = mt7621_gpio_hwirq_to_offset(d->hwirq, rg);
> u32 rise, fall, high, low;
>
> + gpiochip_enable_irq(gc, mask);
> +
> guard(gpio_generic_lock_irqsave)(&rg->chip);
>
> rise = mtk_gpio_r32(rg, GPIO_REG_REDGE);
> @@ -174,6 +176,8 @@ mediatek_gpio_irq_mask(struct irq_data *d)
> mtk_gpio_w32(rg, GPIO_REG_HLVL, high & ~BIT(mask));
> mtk_gpio_w32(rg, GPIO_REG_LLVL, low & ~BIT(mask));
> }
> +
> + gpiochip_disable_irq(gc, mask);
> }
>
> Thanks,
> Sergio Paracuellos
Hi Sergio, test successful.
I tested the buttons and touch and it still works.
I did _not_ test if the irqs are disallowed to be used as gpios in output mode.
Regards,
Vicente.
^ permalink raw reply
* Re: [PATCH RFC 1/2] pinctrl: qcom: eliza: Fix HDMI_RCV_DET function slot
From: Alexander Koskovich @ 2026-06-08 18:41 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Bjorn Andersson, Linus Walleij, linux-arm-msm, linux-gpio,
linux-kernel
In-Reply-To: <109ad54c-f17f-49ea-9d7f-54e13dd367da@oss.qualcomm.com>
On Monday, June 8th, 2026 at 7:55 AM, Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> wrote:
> On 5/28/26 7:24 PM, Alexander Koskovich wrote:
> > On Thursday, April 23rd, 2026 at 7:08 PM, Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> wrote:
> >
> >> On 4/23/26 6:51 AM, Alexander Koskovich wrote:
> >>> The hdmi_rcv_det function was placed at alt function slot 2, but the
> >>> correct mux value for this function on GPIO 19 is slot 3. Move it
> >>> accordingly and leave slot 2 unassigned.
> >>
> >> No, 2 is the desired one per docs
> >>
> >> 0 -> GPIO
> >> 1 -> QUP2_SE5_L3
> >> 2 -> HDMI_RCV_DET
> >
> > Is it possible that CQ7790S is a special case? The pin assignment I have for it
> > here is:
> >
> > 0 -> GPIO
> > 1 -> QUP2_SE5_L3
> > 2 -> N/A
> > 3 -> GP_PDM_MIRA[0]/HDMI_RCV_DET
>
> I was reassured that my source has the latest information
>
> For reference, does your reference doc have any 80-xxxx-xx-like number?
Yes, the document I am referencing is 80-97791-1A, though since you've double
checked likely just means this is is incorrect?
Thanks,
Alex
^ permalink raw reply
* Re: [PATCH v3 1/2] dt-bindings: pinctrl: Add UltraRISC DP1000 pinctrl controller
From: Conor Dooley @ 2026-06-08 17:17 UTC (permalink / raw)
To: wangjia
Cc: Linus Walleij, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bartosz Golaszewski, linux-gpio, devicetree, linux-kernel
In-Reply-To: <20260608-ultrarisc-pinctrl-v3-1-30a09ed74275@ultrarisc.com>
[-- Attachment #1: Type: text/plain, Size: 765 bytes --]
On Mon, Jun 08, 2026 at 03:50:48PM +0800, Jia Wang via B4 Relay wrote:
> From: Jia Wang <wangjia@ultrarisc.com>
>
> Add doc for the pinctrl controllers on the UltraRISC DP1000 RISC-V SoC.
>
> Signed-off-by: Jia Wang <wangjia@ultrarisc.com>
> ---
> .../bindings/pinctrl/ultrarisc,dp1000-pinctrl.yaml | 131 +++++++++++++++++++++
> MAINTAINERS | 6 +
> .../dt-bindings/pinctrl/ultrarisc,dp1000-pinctrl.h | 63 ++++++++++
The driver never uses this file, so just stick it as a header in
arch/riscv/boot/dts/ultrarisc instead of making it a binding.
Otherwise, I think this is okay. With the file moved,
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: changes-requested
Cheers,
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: pinctrl: tegra238: add missing AON pin groups
From: Conor Dooley @ 2026-06-08 17:08 UTC (permalink / raw)
To: Prathamesh Shete
Cc: Linus Walleij, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Thierry Reding, Jonathan Hunter, Arnd Bergmann, linux-gpio,
devicetree, linux-tegra, linux-kernel
In-Reply-To: <20260608094122.1245189-1-pshete@nvidia.com>
[-- Attachment #1: Type: text/plain, Size: 75 bytes --]
Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH 16/18] pinctrl: airoha: an7583: remove an7583 prefix from variable names
From: Bartosz Golaszewski @ 2026-06-08 16:17 UTC (permalink / raw)
To: Mikhail Kshevetskiy
Cc: Linus Walleij, Sean Wang, Lorenzo Bianconi, Matthias Brugger,
AngeloGioacchino Del Regno, Christian Marangi,
Bartosz Golaszewski, Benjamin Larsson, linux-kernel, linux-gpio,
linux-mediatek, linux-arm-kernel, Matheus Sampaio Queiroga,
Markus Gothe
In-Reply-To: <20260607001654.1439480-17-mikhail.kshevetskiy@iopsys.eu>
On Sun, 7 Jun 2026 02:16:52 +0200, Mikhail Kshevetskiy
<mikhail.kshevetskiy@iopsys.eu> said:
> We have only an7583 specific code in the pinctrl-an7583 kernel module,
> so 'an75831_' prefix is not necessary anymore. Remove it.
>
> Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
> ---
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
^ permalink raw reply
* Re: [PATCH 17/18] pinctrl: airoha: prepare for en7523 adding
From: Bartosz Golaszewski @ 2026-06-08 16:15 UTC (permalink / raw)
To: Mikhail Kshevetskiy
Cc: Linus Walleij, Sean Wang, Lorenzo Bianconi, Matthias Brugger,
AngeloGioacchino Del Regno, Christian Marangi,
Bartosz Golaszewski, Benjamin Larsson, linux-kernel, linux-gpio,
linux-mediatek, linux-arm-kernel, Matheus Sampaio Queiroga,
Markus Gothe
In-Reply-To: <20260607001654.1439480-18-mikhail.kshevetskiy@iopsys.eu>
On Sun, 7 Jun 2026 02:16:53 +0200, Mikhail Kshevetskiy
<mikhail.kshevetskiy@iopsys.eu> said:
> en7523 a bit differs from an7581/an7583. It has different register
> offsets and slightly different bitfield masks.
>
> Let's adapt common header and existing drivers for the future addition
> of en7523.
>
> Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
> ---
Looks good to me.
Acked-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
^ permalink raw reply
* Re: [PATCH 06/18] pinctrl: airoha: an7583: fix incorrect led mapping in phy4_led1 pin function
From: Bartosz Golaszewski @ 2026-06-08 16:13 UTC (permalink / raw)
To: Mikhail Kshevetskiy
Cc: Linus Walleij, Sean Wang, Lorenzo Bianconi, Matthias Brugger,
AngeloGioacchino Del Regno, Christian Marangi,
Bartosz Golaszewski, Benjamin Larsson, linux-kernel, linux-gpio,
linux-mediatek, linux-arm-kernel, Matheus Sampaio Queiroga,
Markus Gothe
In-Reply-To: <20260607001654.1439480-7-mikhail.kshevetskiy@iopsys.eu>
On Sun, 7 Jun 2026 02:16:42 +0200, Mikhail Kshevetskiy
<mikhail.kshevetskiy@iopsys.eu> said:
> phy4_led1 pin function maps led incorrectly. It uses the same map as
> phy3_led1. PHY{X} should map to LAN{N}_PHY_LED_MAP(X-1).
>
> Fixes: 3ffeb17a9a27 ("pinctrl: airoha: add support for Airoha AN7583 PINs")
> Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
> ---
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
^ permalink raw reply
* Re: [PATCH 12/18] pinctrl: airoha: move driver to separate directory
From: Bartosz Golaszewski @ 2026-06-08 14:55 UTC (permalink / raw)
To: Mikhail Kshevetskiy
Cc: Linus Walleij, Sean Wang, Lorenzo Bianconi, Matthias Brugger,
AngeloGioacchino Del Regno, Christian Marangi,
Bartosz Golaszewski, Benjamin Larsson, linux-kernel, linux-gpio,
linux-mediatek, linux-arm-kernel, Matheus Sampaio Queiroga,
Markus Gothe
In-Reply-To: <20260607001654.1439480-13-mikhail.kshevetskiy@iopsys.eu>
On Sun, 7 Jun 2026 02:16:48 +0200, Mikhail Kshevetskiy
<mikhail.kshevetskiy@iopsys.eu> said:
> Preparation step. Later the driver will be split on several SoC
> specific drivers and a common code. So it's better put them to
> a separate directory.
>
> No functional changes.
>
> Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
> ---
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
^ permalink raw reply
* Re: [PATCH 05/18] pinctrl: airoha: an7581: fix incorrect led mapping in phy4_led1 pin function
From: Bartosz Golaszewski @ 2026-06-08 14:52 UTC (permalink / raw)
To: Mikhail Kshevetskiy
Cc: Linus Walleij, Sean Wang, Lorenzo Bianconi, Matthias Brugger,
AngeloGioacchino Del Regno, Christian Marangi,
Bartosz Golaszewski, Benjamin Larsson, linux-kernel, linux-gpio,
linux-mediatek, linux-arm-kernel, Matheus Sampaio Queiroga,
Markus Gothe
In-Reply-To: <20260607001654.1439480-6-mikhail.kshevetskiy@iopsys.eu>
On Sun, 7 Jun 2026 02:16:41 +0200, Mikhail Kshevetskiy
<mikhail.kshevetskiy@iopsys.eu> said:
> phy4_led1 pin function maps led incorrectly. It uses the same map as
> phy3_led1. PHY{X} should map to LAN{N}_PHY_LED_MAP(X-1).
>
> Fixes: 579839c9548c ("pinctrl: airoha: convert PHY LED GPIO to macro")
> Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
> ---
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
^ permalink raw reply
* Re: [PATCH 04/18] pinctrl: airoha: an7583: fix misprint in gpio19 pinconf
From: Bartosz Golaszewski @ 2026-06-08 14:50 UTC (permalink / raw)
To: Mikhail Kshevetskiy
Cc: Linus Walleij, Sean Wang, Lorenzo Bianconi, Matthias Brugger,
AngeloGioacchino Del Regno, Christian Marangi,
Bartosz Golaszewski, Benjamin Larsson, linux-kernel, linux-gpio,
linux-mediatek, linux-arm-kernel, Matheus Sampaio Queiroga,
Markus Gothe
In-Reply-To: <20260607001654.1439480-5-mikhail.kshevetskiy@iopsys.eu>
On Sun, 7 Jun 2026 02:16:40 +0200, Mikhail Kshevetskiy
<mikhail.kshevetskiy@iopsys.eu> said:
> Pin 21 (gpio19) duplicate pinconf settings of pin 20. Fix it using
> a proper bit number in the configuration register.
>
> Fixes: 3ffeb17a9a27 ("pinctrl: airoha: add support for Airoha AN7583 PINs")
> Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
> ---
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
^ permalink raw reply
* Re: [PATCH 03/18] pinctrl: airoha: an7581: fix misprint in gpio19 pinconf
From: Bartosz Golaszewski @ 2026-06-08 14:50 UTC (permalink / raw)
To: Mikhail Kshevetskiy
Cc: Linus Walleij, Sean Wang, Lorenzo Bianconi, Matthias Brugger,
AngeloGioacchino Del Regno, Christian Marangi,
Bartosz Golaszewski, Benjamin Larsson, linux-kernel, linux-gpio,
linux-mediatek, linux-arm-kernel, Matheus Sampaio Queiroga,
Markus Gothe
In-Reply-To: <20260607001654.1439480-4-mikhail.kshevetskiy@iopsys.eu>
On Sun, 7 Jun 2026 02:16:39 +0200, Mikhail Kshevetskiy
<mikhail.kshevetskiy@iopsys.eu> said:
> Pin 32 (gpio19) duplicate pinconf settings of pin 31. Fix it using
> a proper bit number in the configuration register.
>
> Fixes: 1c8ace2d0725 ("pinctrl: airoha: Add support for EN7581 SoC")
> Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
> ---
I think the term we typically use is "typo" but I won't die on that hill.
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
^ permalink raw reply
* Re: [PATCH 02/18] pinctrl: airoha: an7583: add missed gpio32 pin group
From: Bartosz Golaszewski @ 2026-06-08 14:48 UTC (permalink / raw)
To: Mikhail Kshevetskiy
Cc: Linus Walleij, Sean Wang, Lorenzo Bianconi, Matthias Brugger,
AngeloGioacchino Del Regno, Christian Marangi,
Bartosz Golaszewski, Benjamin Larsson, linux-kernel, linux-gpio,
linux-mediatek, linux-arm-kernel, Matheus Sampaio Queiroga,
Markus Gothe
In-Reply-To: <20260607001654.1439480-3-mikhail.kshevetskiy@iopsys.eu>
On Sun, 7 Jun 2026 02:16:38 +0200, Mikhail Kshevetskiy
<mikhail.kshevetskiy@iopsys.eu> said:
> gpio32 pin group is missed for an7583 SoC. This patch add it.
>
> Fixes: 3ffeb17a9a27 ("pinctrl: airoha: add support for Airoha AN7583 PINs")
> Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
> ---
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
^ permalink raw reply
* Re: [PATCH 01/18] pinctrl: airoha: an7581: add missed gpio32 pin group
From: Bartosz Golaszewski @ 2026-06-08 14:48 UTC (permalink / raw)
To: Mikhail Kshevetskiy
Cc: Linus Walleij, Sean Wang, Lorenzo Bianconi, Matthias Brugger,
AngeloGioacchino Del Regno, Christian Marangi,
Bartosz Golaszewski, Benjamin Larsson, linux-kernel, linux-gpio,
linux-mediatek, linux-arm-kernel, Matheus Sampaio Queiroga,
Markus Gothe
In-Reply-To: <20260607001654.1439480-2-mikhail.kshevetskiy@iopsys.eu>
On Sun, 7 Jun 2026 02:16:37 +0200, Mikhail Kshevetskiy
<mikhail.kshevetskiy@iopsys.eu> said:
> gpio32 pin group is missed for an7581 SoC. This patch add it.
>
> Fixes: 1c8ace2d0725 ("pinctrl: airoha: Add support for EN7581 SoC")
> Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
> ---
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
^ permalink raw reply
* [PATCH libgpiod 5/5] tools: gpioset: store interactive sleep period in a long long
From: Bartosz Golaszewski @ 2026-06-08 14:47 UTC (permalink / raw)
To: Kent Gibson, Erik Wierich, Viresh Kumar, Vincent Fazio,
Linus Walleij
Cc: brgl, linux-gpio, Bartosz Golaszewski
In-Reply-To: <20260608-more-pre-2-3-fixes-v1-0-577a5ba426a5@oss.qualcomm.com>
The interactive 'sleep' command stored the result of parse_period() in
an int. parse_period() returns a long long number of microseconds, so a
period above INT_MAX microseconds (~36 minutes, e.g. "sleep 40m")
overflowed: it could wrap to a negative value and be wrongly rejected as
an invalid period, or to an incorrect positive value producing the wrong
sleep duration.
Declare period_us as long long to match parse_period()'s return type.
The error check (period_us < 0) and the sleep_us() call, which takes an
unsigned long long, both remain correct. The non-interactive hold and
toggle period paths already use unsigned long long and were unaffected.
Add a test that issues "sleep 40m" at the interactive prompt and verifies
the tool starts sleeping rather than emitting an "invalid period" error
Fixes: c34a572c5350 ("tools: add minutes as a new supported time unit")
Assisted-by: Claude Opus 4.8
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
tools/gpio-tools-test.bash | 21 +++++++++++++++++++++
tools/gpioset.c | 3 ++-
2 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/tools/gpio-tools-test.bash b/tools/gpio-tools-test.bash
index 2a39133045deb5f4f6afb3d039d0436ff8340778..f2b5ec7a69d46264dd10645d2af7275758866e0a 100755
--- a/tools/gpio-tools-test.bash
+++ b/tools/gpio-tools-test.bash
@@ -1320,6 +1320,27 @@ test_gpioset_interactive_sleep() {
dut_readable
}
+test_gpioset_interactive_sleep_with_long_period() {
+ gpiosim_chip sim0 num_lines=8 line_name=1:foo
+
+ dut_run gpioset --interactive foo=1
+
+ # clear the initial prompt
+ dut_flush
+
+ # A period longer than INT_MAX microseconds (~36 minutes) must be
+ # accepted, not overflow into a negative value and get rejected as an
+ # invalid period.
+ dut_write "sleep 40m"
+
+ # give the tool a moment to either start sleeping (correct) or print an
+ # error and re-prompt (the bug)
+ sleep 1
+
+ # nothing must be readable: the tool is sleeping, not reporting an error
+ assert_fail dut_readable
+}
+
test_gpioset_toggle_continuous() {
gpiosim_chip sim0 num_lines=8 line_name=1:foo line_name=4:bar \
line_name=7:baz
diff --git a/tools/gpioset.c b/tools/gpioset.c
index 5a68a7e9017cefd3661a3274bf1ba0bb194f132f..fce4a7de222f93121c99843a761c5308e021343a 100644
--- a/tools/gpioset.c
+++ b/tools/gpioset.c
@@ -739,9 +739,10 @@ static void interact(struct gpiod_line_request **requests,
unsigned int *offsets, enum gpiod_line_value *values,
bool unquoted)
{
- int num_words, num_lines, max_words, period_us, i;
+ int num_words, num_lines, max_words, i;
char *line, **words, *line_buf;
bool done, stdout_is_tty;
+ long long period_us;
stifle_history(20);
rl_attempted_completion_function = tab_completion;
--
2.47.3
^ permalink raw reply related
* [PATCH libgpiod 4/5] tools: reject trailing garbage in parse_period()
From: Bartosz Golaszewski @ 2026-06-08 14:47 UTC (permalink / raw)
To: Kent Gibson, Erik Wierich, Viresh Kumar, Vincent Fazio,
Linus Walleij
Cc: brgl, linux-gpio, Bartosz Golaszewski
In-Reply-To: <20260608-more-pre-2-3-fixes-v1-0-577a5ba426a5@oss.qualcomm.com>
parse_period() consumed an optional unit suffix ('us', 'ms', 's', 'm')
but never verified that the suffix was the end of the string. As a
result inputs with trailing garbage such as "100msx", "100usx" or
"100sx" were silently accepted and parsed as if the garbage were not
there, while inconsistently "100mx" was already rejected.
Require that the whole string be consumed by checking *end == '\0'
after the unit suffix has been processed, mirroring what parse_uint()
already does. All valid forms still leave end pointing at the
terminating NUL at that point, so only malformed input is rejected.
Add a test that passes an invalid suffix and verifies it's rejected.
Fixes: 6702eed1a5f2 ("tools: fix an integer overflow bug in parse_period()")
Assisted-by: Claude Opus 4.8
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
tools/gpio-tools-test.bash | 11 +++++++++++
tools/tools-common.c | 3 +++
2 files changed, 14 insertions(+)
diff --git a/tools/gpio-tools-test.bash b/tools/gpio-tools-test.bash
index 00739c5a4008c636b667cccd1cdaebb624ae0ac8..2a39133045deb5f4f6afb3d039d0436ff8340778 100755
--- a/tools/gpio-tools-test.bash
+++ b/tools/gpio-tools-test.bash
@@ -879,6 +879,17 @@ test_gpioget_with_invalid_hold_period() {
status_is 1
}
+test_gpioget_with_trailing_garbage_in_period() {
+ gpiosim_chip sim0 num_lines=8
+
+ # A period with trailing garbage after the unit suffix must be
+ # rejected. "100msx" must not be silently parsed as "100ms".
+ run_prog gpioget --hold-period=100msx --chip "${GPIOSIM_CHIP_NAME[sim0]}" 0
+
+ output_regex_match ".*invalid period.*"
+ status_is 1
+}
+
#
# gpioset test cases
#
diff --git a/tools/tools-common.c b/tools/tools-common.c
index 56ed90f639790ec54a3d6e9eb6ad594104474d7e..4482ea5d6a4c6ef3cad3af2fb960d230e1a542a0 100644
--- a/tools/tools-common.c
+++ b/tools/tools-common.c
@@ -153,6 +153,9 @@ long long parse_period(const char *option)
m = 1000;
}
+ if (*end != '\0')
+ return -1;
+
if (m != 0 && p > ULLONG_MAX / m)
return -1;
p *= m;
--
2.47.3
^ permalink raw reply related
* [PATCH libgpiod 3/5] tools: gpionotify: don't watch lines on the wrong chip
From: Bartosz Golaszewski @ 2026-06-08 14:47 UTC (permalink / raw)
To: Kent Gibson, Erik Wierich, Viresh Kumar, Vincent Fazio,
Linus Walleij
Cc: brgl, linux-gpio, Bartosz Golaszewski
In-Reply-To: <20260608-more-pre-2-3-fixes-v1-0-577a5ba426a5@oss.qualcomm.com>
Commit 0de77e2abf12 ("tools: gpionotify: don't leak info returned by
gpiod_chip_watch_line_info()") moved the call to
gpiod_chip_watch_line_info() out of the condition that guarded it. The
original code relied on the short-circuit evaluation of the logical AND
to only request a watch when the line actually belonged to the chip
currently being processed:
if ((resolver->lines[j].chip_num == i) &&
!gpiod_chip_watch_line_info(chip, resolver->lines[j].offset))
die_perror(...);
After the refactor the watch is requested unconditionally for every
resolved line on every chip, using the line's offset as-is. When lines
are spread across multiple chips this installs watches at foreign
offsets on the wrong chip. Such a watch typically succeeds (the offset
usually exists on the other chip too), persists - freeing the returned
gpiod_line_info does not remove it - and makes gpionotify report
spurious info events for lines the user never asked to watch.
Restore the guard so that each line is only ever watched on the chip it
belongs to, while keeping the gpiod_line_info_free() leak fix. As a side
effect a watch failure on the correct chip is now always treated as
fatal instead of only when it coincided with a matching chip_num.
Add a test-case that requests two lines with distinct offsets on two
different chips and then toggles one line's offset on the other chip. No
event must be reported for that, as the offset is not watched there.
Fixes: 0de77e2abf12 ("tools: gpionotify: don't leak info returned by gpiod_chip_watch_line_info()")
Assisted-by: Claude Opus 4.8
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
tools/gpio-tools-test.bash | 23 +++++++++++++++++++++++
tools/gpionotify.c | 5 ++++-
2 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/tools/gpio-tools-test.bash b/tools/gpio-tools-test.bash
index 3ec76dfac95ee276c78dac0c2f9792d8439f5418..00739c5a4008c636b667cccd1cdaebb624ae0ac8 100755
--- a/tools/gpio-tools-test.bash
+++ b/tools/gpio-tools-test.bash
@@ -2486,6 +2486,29 @@ test_gpionotify_multiple_lines_across_multiple_chips() {
assert_fail dut_readable
}
+test_gpionotify_do_not_watch_lines_on_wrong_chip() {
+ gpiosim_chip sim0 num_lines=4 line_name=2:foo
+ gpiosim_chip sim1 num_lines=8 line_name=4:bar
+
+ local sim0=${GPIOSIM_CHIP_NAME[sim0]}
+ local sim1=${GPIOSIM_CHIP_NAME[sim1]}
+
+ dut_run gpionotify --banner foo bar
+ dut_regex_match "Watching lines .*"
+
+ # Toggle sim1 offset 2 - this is foo's offset but on the wrong chip, so
+ # it must not be watched and must not produce any event. Doing it before
+ # the legitimate bar event guarantees that a spurious event, if emitted,
+ # would be read first from sim1's event buffer.
+ request_release_line "$sim1" 2
+
+ request_release_line "$sim1" 4
+ dut_regex_match "[0-9]+\.[0-9]+\\s+requested\\s+\"bar\""
+ dut_regex_match "[0-9]+\.[0-9]+\\s+released\\s+\"bar\""
+
+ assert_fail dut_readable
+}
+
test_gpionotify_exit_after_SIGTERM() {
gpiosim_chip sim0 num_lines=8
diff --git a/tools/gpionotify.c b/tools/gpionotify.c
index a45685f22ff73b1e138ddcd662f35fc8df5c0755..ea4b0ca79bb7b710c9090cb524c0c12b96f176e7 100644
--- a/tools/gpionotify.c
+++ b/tools/gpionotify.c
@@ -407,9 +407,12 @@ int main(int argc, char **argv)
resolver->chips[i].path);
for (j = 0; j < resolver->num_lines; j++) {
+ if (resolver->lines[j].chip_num != i)
+ continue;
+
info = gpiod_chip_watch_line_info(chip,
resolver->lines[j].offset);
- if ((resolver->lines[j].chip_num == i) && !info)
+ if (!info)
die_perror("unable to watch line on chip '%s'",
resolver->chips[i].path);
--
2.47.3
^ permalink raw reply related
* [PATCH libgpiod 2/5] core: edge-event: return 0 when zero events are requested
From: Bartosz Golaszewski @ 2026-06-08 14:47 UTC (permalink / raw)
To: Kent Gibson, Erik Wierich, Viresh Kumar, Vincent Fazio,
Linus Walleij
Cc: brgl, linux-gpio, Bartosz Golaszewski
In-Reply-To: <20260608-more-pre-2-3-fixes-v1-0-577a5ba426a5@oss.qualcomm.com>
gpiod_edge_event_buffer_read_fd() is documented to read at most
max_events events and to return the number of events read. When called
with max_events == 0 it nonetheless issued a read() for 0 bytes, which
returns 0, and the subsequent short-read check
((unsigned int)rd < sizeof(*event_data)) then treated this as an I/O
error, setting errno to EIO and returning -1.
Requesting zero events is not an error - it is a no-op that reads
nothing. Handle it explicitly by clearing num_events and returning 0
before attempting the read.
Add a test case verifying the behavior.
Fixes: b7ba732e6a93 ("treewide: libgpiod v2 implementation")
Assisted-by: Claude Opus 4.8
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
lib/edge-event.c | 5 +++++
tests/tests-edge-event.c | 35 +++++++++++++++++++++++++++++++++++
2 files changed, 40 insertions(+)
diff --git a/lib/edge-event.c b/lib/edge-event.c
index 8ec355fd59a280936ae348dfb6e418189f865153..be46dcecedd7ccc8dfd1d285724db7e38c2e64a2 100644
--- a/lib/edge-event.c
+++ b/lib/edge-event.c
@@ -184,6 +184,11 @@ int gpiod_edge_event_buffer_read_fd(int fd,
if (max_events > buffer->capacity)
max_events = buffer->capacity;
+ if (max_events == 0) {
+ buffer->num_events = 0;
+ return 0;
+ }
+
rd = read(fd, buffer->event_data,
max_events * sizeof(*buffer->event_data));
if (rd < 0) {
diff --git a/tests/tests-edge-event.c b/tests/tests-edge-event.c
index 6389455763ed4ee2215de6409d1b042169776902..6c7ce20ff81605e53a7e929f5591b9a3b43650ed 100644
--- a/tests/tests-edge-event.c
+++ b/tests/tests-edge-event.c
@@ -613,6 +613,41 @@ GPIOD_TEST_CASE(reading_more_events_than_the_queue_contains_doesnt_block)
gpiod_test_return_if_failed();
}
+GPIOD_TEST_CASE(reading_zero_events_is_a_noop)
+{
+ static const guint offset = 2;
+
+ g_autoptr(GPIOSimChip) sim = g_gpiosim_chip_new("num-lines", 8, NULL);
+ g_autoptr(struct_gpiod_chip) chip = NULL;
+ g_autoptr(struct_gpiod_line_settings) settings = NULL;
+ g_autoptr(struct_gpiod_line_config) line_cfg = NULL;
+ g_autoptr(struct_gpiod_line_request) request = NULL;
+ g_autoptr(struct_gpiod_edge_event_buffer) buffer = NULL;
+ gint ret;
+
+ chip = gpiod_test_open_chip_or_fail(g_gpiosim_chip_get_dev_path(sim));
+ settings = gpiod_test_create_line_settings_or_fail();
+ line_cfg = gpiod_test_create_line_config_or_fail();
+ buffer = gpiod_test_create_edge_event_buffer_or_fail(64);
+
+ gpiod_line_settings_set_direction(settings, GPIOD_LINE_DIRECTION_INPUT);
+ gpiod_line_settings_set_edge_detection(settings, GPIOD_LINE_EDGE_BOTH);
+
+ gpiod_test_line_config_add_line_settings_or_fail(line_cfg, &offset, 1,
+ settings);
+
+ request = gpiod_test_chip_request_lines_or_fail(chip, NULL, line_cfg);
+
+ g_gpiosim_chip_set_pull(sim, 2, G_GPIOSIM_PULL_UP);
+ g_usleep(500);
+
+ ret = gpiod_line_request_read_edge_events(request, buffer, 0);
+ g_assert_cmpint(ret, ==, 0);
+ gpiod_test_return_if_failed();
+
+ g_assert_cmpuint(gpiod_edge_event_buffer_get_num_events(buffer), ==, 0);
+}
+
GPIOD_TEST_CASE(null_buffer)
{
static const guint offset = 2;
--
2.47.3
^ permalink raw reply related
* [PATCH libgpiod 1/5] core: line-config: use gpiod_line_settings_free() to release settings
From: Bartosz Golaszewski @ 2026-06-08 14:47 UTC (permalink / raw)
To: Kent Gibson, Erik Wierich, Viresh Kumar, Vincent Fazio,
Linus Walleij
Cc: brgl, linux-gpio, Bartosz Golaszewski
In-Reply-To: <20260608-more-pre-2-3-fixes-v1-0-577a5ba426a5@oss.qualcomm.com>
When releasing a no-longer-referenced settings node in
gpiod_line_config_add_line_settings(), the embedded settings object was
freed with a raw free() instead of gpiod_line_settings_free(), unlike
every other release site (e.g. free_refs()).
This is currently harmless because gpiod_line_settings_free() is just a
free(), but bypassing the type's destructor is a latent leak should the
struct ever gain heap-allocated members. Use the proper destructor for
consistency.
Fixes: 2560a857ce43 ("core: keep memory usage of struct line_config in check")
Assisted-by: Claude Opus 4.8
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
lib/line-config.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/line-config.c b/lib/line-config.c
index c9e5cb0d9d2f27e441856ffa1b09a0e57a0db2ba..f00892e6e78d5b36a8b1433701f752e7d3fedc13 100644
--- a/lib/line-config.c
+++ b/lib/line-config.c
@@ -182,7 +182,7 @@ GPIOD_API int gpiod_line_config_add_line_settings(
old->prev->next = old->next;
if (old->next)
old->next->prev = old->prev;
- free(old->settings);
+ gpiod_line_settings_free(old->settings);
free(old);
}
}
--
2.47.3
^ permalink raw reply related
* [PATCH libgpiod 0/5] treewide: more AI-assisted pre-v2.3 fixes
From: Bartosz Golaszewski @ 2026-06-08 14:47 UTC (permalink / raw)
To: Kent Gibson, Erik Wierich, Viresh Kumar, Vincent Fazio,
Linus Walleij
Cc: brgl, linux-gpio, Bartosz Golaszewski
I got access to the more recent Claude Opus v4.8 model and it found even
more issues so here's another round of fixes.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
Bartosz Golaszewski (5):
core: line-config: use gpiod_line_settings_free() to release settings
core: edge-event: return 0 when zero events are requested
tools: gpionotify: don't watch lines on the wrong chip
tools: reject trailing garbage in parse_period()
tools: gpioset: store interactive sleep period in a long long
lib/edge-event.c | 5 +++++
lib/line-config.c | 2 +-
tests/tests-edge-event.c | 35 +++++++++++++++++++++++++++++
tools/gpio-tools-test.bash | 55 ++++++++++++++++++++++++++++++++++++++++++++++
tools/gpionotify.c | 5 ++++-
tools/gpioset.c | 3 ++-
tools/tools-common.c | 3 +++
7 files changed, 105 insertions(+), 3 deletions(-)
---
base-commit: 84700a6d5fe7f4499d6117b55e670bbda72b95d7
change-id: 20260608-more-pre-2-3-fixes-68f676b0ce03
Best regards,
--
Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
^ permalink raw reply
* Re: [PATCH v3 18/21] pinctrl: starfive: Add StarFive JHB100 per2pok controller driver
From: Bartosz Golaszewski @ 2026-06-08 14:45 UTC (permalink / raw)
To: Changhuang Liang
Cc: linux-gpio, linux-kernel, devicetree, linux-riscv,
Lianfeng Ouyang, Linus Walleij, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Emil Renner Berthing, Paul Walmsley, Albert Ou,
Palmer Dabbelt, Alexandre Ghiti, Philipp Zabel,
Bartosz Golaszewski
In-Reply-To: <20260603055347.66845-19-changhuang.liang@starfivetech.com>
On Wed, 3 Jun 2026 07:53:44 +0200, Changhuang Liang
<changhuang.liang@starfivetech.com> said:
> Add pinctrl driver for StarFive JHB100 SoC Peripheral-2 Power OK
> (per2pok) pinctrl controller.
>
> Co-developed-by: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
> Signed-off-by: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
> ---
> drivers/pinctrl/starfive/Kconfig | 12 +++
> drivers/pinctrl/starfive/Makefile | 1 +
> .../pinctrl-starfive-jhb100-per2pok.c | 97 +++++++++++++++++++
> 3 files changed, 110 insertions(+)
> create mode 100644 drivers/pinctrl/starfive/pinctrl-starfive-jhb100-per2pok.c
>
> diff --git a/drivers/pinctrl/starfive/Kconfig b/drivers/pinctrl/starfive/Kconfig
> index edc3b6d9c8d7..bf5915e0a5f2 100644
> --- a/drivers/pinctrl/starfive/Kconfig
> +++ b/drivers/pinctrl/starfive/Kconfig
> @@ -91,6 +91,18 @@ config PINCTRL_STARFIVE_JHB100_PER2
> peripherals supporting inputs, outputs, configuring pull-up/pull-down
> and interrupts on input changes.
>
> +config PINCTRL_STARFIVE_JHB100_PER2POK
> + tristate "StarFive JHB100 SoC Peripheral-2 Power OK pinctrl and GPIO driver"
> + depends on ARCH_STARFIVE || COMPILE_TEST
> + depends on OF
I don't see any real dependency on CONFIG_OF here, am I missing something? The
OF ID matching is available even with !CONFIG_OF.
Same elsewhere.
Bart
^ permalink raw reply
* Re: [PATCH v2] gpio: mt7621: fix interrupt banks mapping on gpio chips
From: Bartosz Golaszewski @ 2026-06-08 14:42 UTC (permalink / raw)
To: Sergio Paracuellos
Cc: linusw, tglx, grant.likely, anna-maria, vicencb, linux-kernel,
linux-gpio, Bartosz Golaszewski
In-Reply-To: <CAMhs-H-dnR7KjOZYv+rSt=n4MwPd5O1g9kfoP99uA7TQASf0WQ@mail.gmail.com>
On Mon, 8 Jun 2026 16:27:35 +0200, Sergio Paracuellos
<sergio.paracuellos@gmail.com> said:
> On Mon, Jun 8, 2026 at 4:02 PM Bartosz Golaszewski <brgl@kernel.org> wrote:
> >>
>> >> Hi!
>> >>
>> >> Can you look at the sashiko review? Especially the bit about tracking the
>> >> GPIOD_FLAG_IRQ_IS_ENABLED flag looks correct.
>> >
>> > I got rid of those two calls (gpiochip_enable_irq() and
>> > gpiochip_disable_irq()) because the driver "gpio-brcmstb" which is the
>> > one I based my changes on was not used them at all. We have not found
>> > anything weird related with that on testing. I do believe that since
>> > we are using our own callbacks for 'irq_request_resources()' and
>> > 'irq_release_resources()' we are safe here. Regarding the others I am
>> > not sure, but the introduction of the remove stuff for the irq domain
>> > is because there are no devm_* functions for that. Other resources in
>> > driver are using devm versions so I think the changes are ok as they
>> > are...
>> >
>>
>> It's about GPIO core: a GPIO that appears as "free" (users can request it) but
>> was earlier enabled for interrupts cannot be requested in output mode - only
>> input works. Without this flag set, gpiod_direction_output_nonotify() will allow
>> you to set direction to output.
>
> I see. I need Vicente to re-test without removing
> gpiochip_enable_irq() and gpiochip_disable_irq() to see if everything
> is still ok.
>
> Vicente, would you mind to test the following change on top of this v2 PATCH?
Thanks, sorry for the late notice but I was travelling last week. If this
misses v7.1, it will go in during the merge window and can be backported
Bart
^ permalink raw reply
* Re: [PATCH v3 2/7] gpio: regmap: add gpio_regmap_get_gpiochip() accessor
From: Michael Walle @ 2026-06-08 14:41 UTC (permalink / raw)
To: Bartosz Golaszewski, Andy Shevchenko
Cc: linusw@kernel.org, robh@kernel.org, krzk+dt@kernel.org,
conor+dt@kernel.org, afaerber@suse.com, wbg@kernel.org,
mathieu.dubois-briand@bootlin.com, lars@metafoo.de,
Michael.Hennerich@analog.com, jic23@kernel.org,
nuno.sa@analog.com, andy@kernel.org, dlechner@baylibre.com,
TY_Chang[張子逸], linux-gpio@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-realtek-soc@lists.infradead.org, linux-iio@vger.kernel.org,
CY_Huang[黃鉦晏],
Stanley Chang[昌育德],
James Tai [戴志峰],
Yu-Chun Lin [林祐君]
In-Reply-To: <CAMRc=MdA24z-tB_D8CTw68Di8e4OVQJ1QH4+rDskFzq=xjJ5BQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 4853 bytes --]
Hi,
On Mon Jun 8, 2026 at 4:10 PM CEST, Bartosz Golaszewski wrote:
> On Wed, 3 Jun 2026 02:34:40 +0200, Andy Shevchenko
> <andriy.shevchenko@intel.com> said:
>> On Mon, May 25, 2026 at 12:04:09PM +0000, Yu-Chun Lin [林祐君] wrote:
>>> > On Tue, May 12, 2026 at 11:33:12AM +0800, Yu-Chun Lin wrote:
>>> > > Expose an accessor function to retrieve the gpio_chip pointer from a
>>> > > gpio_regmap instance.
>>> > >
>>> > > This is needed by drivers that use gpio_regmap but also manage their
>>> > > own irq_chip, where gpiochip_enable_irq()/gpiochip_disable_irq() must
>>> > > be called with the gpio_chip pointer.
>>> > >
>>> > > Add gpio_regmap_get_gpiochip() to allow drivers with complex custom
>>> > > IRQ implementations.
>>> >
>>> > Hmm... Can't we rather add
>>> > gpio_regmap_enable_irq()/gpio_regmap_disable_irq()
>>> > that take regmap or GPIO regmap (whatever suits better for the purpose) and
>>> > do the magic inside GPIO regmap library code?
>>
>>> Thanks for the review! I apologize for the misleading commit message.
>>> The real reason I need the struct gpio_chip pointer is to properly set up a custom
>>> IRQ domain. Our SoC GPIO controller is quite complex. It routes different trigger
>>> types to multiple parent IRQs, which doesn't fit the generic regmap_irq framework.
>>> Therefore, we have to create our own irq_domain and pass it to
>>> gpio_regmap_config.irq_domain.
>>>
>>> The core problem occurs inside our custom irq_domain_ops.map() callback:
>>>
>>> static int rtd1625_gpio_irq_map(struct irq_domain *domain, unsigned int irq,
>>> irq_hw_number_t hwirq)
>>> {
>>> struct rtd1625_gpio *data = domain->host_data;
>>> struct gpio_chip *gc = data->gpio_chip;
>>>
>>> /*
>>> * The second argument MUST be struct gpio_chip *.
>>> * If we pass our custom data structure here, the kernel will panic later
>>> * in gpiochip_irq_reqres() when it calls irq_data_get_irq_chip_data()
>>> * and strictly expects it to be a gpio_chip.
>>> */
>>> irq_set_chip_data(irq, gc);
>>>
>>> irq_set_lockdep_class(irq, &rtd1625_gpio_irq_lock_class,
>>> &rtd1625_gpio_irq_request_class);
>>>
>>> irq_set_chip_and_handler(irq, &rtd1625_iso_gpio_irq_chip, handle_bad_irq);
>>> irq_set_noprobe(irq);
>>>
>>> return 0;
>>> }
>>>
>>> Without an accessor like gpio_regmap_get_gpiochip(), we cannot retrieve the
>>> gpio_chip instantiated inside gpio-regmap.c to fulfill these requirements in our
>>> map() function.
Why is gpiochip_irq_reqres() called in the first place? Isn't that
only called if the irq handling is set up via gc->irq.chip and not
via gpiochip_irqchip_add_domain() like in gpio-regmap?
>> This is all good and needs to be depicted in the cover-letter and/or commit message.
>>
>>> Before I send a v4, I see 3 possible paths:
>>>
>>> Option 1: Keep the accessor (Current v3 approach)
>>> We keep gpio_regmap_get_gpiochip() but I will completely rewrite the commit message
>>> to explain the custom irq_domain_ops.map and lockdep requirements.
>>>
>>> Option 2: Let gpiolib create the irq_domain via gpio_regmap_config
>>> Instead of creating the irq_domain in our driver, we add all necessary IRQ fields
>>> (irq_chip, irq_handler, irq_parents, etc.) into struct gpio_regmap_config. Then
>>> gpio-regmap.c populates the gpio_irq_chip structure before calling
>>> gpiochip_add_data(). This prevents an early return and allows the core gpiolib
>>> (gpiochip_add_irqchip()) to automatically create the irq_domain for us.
>>> Drawback: This adds a lot of fields to gpio_regmap_config and might violate the
>>> original design philosophy of gpio-regmap.c (commit ebe363197e52), which explicitly
>>> states that it does not implement its own IRQ chip and delegates it to the parent
>>> driver.
>>>
>>> Option 3: Drop gpio-regmap entirely (Revert to v2 approach)
>>> Currently, all drivers using gpio-regmap (mostly simple CPLDs and external I/O cards)
>>> use regmap-irq to get their domain. Since our SoC has a complex IRQ routing scheme
>>> with multiple parents, maybe gpio-regmap is simply not the right tool for this
>>> hardware, and we should just implement a standard GPIO driver directly using gpiolib.
>>>
>>> Which approach would you prefer upstream?
>>
>> This question to Bart, Linus, and poissibly gpio-regmap stakeholders. I'm not sure
>> that my personal opinion will be the best fit here.
>>
>
> My preference would be for #2 but I understand that this could risk getting
> stuck in endless bikeshedding so I'm fine with going #3 with potential for
> future refactoring if we have more similar users.
Yeah, I'd like to keep that stuff out of gpio-regmap. But I'm on the
same boat regarding the refactoring if we have more data and
potential users.
-michael
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]
^ permalink raw reply
* Re: [PATCH v2] gpio: mvebu: fix NULL pointer dereference in suspend/resume
From: Bartosz Golaszewski @ 2026-06-08 14:34 UTC (permalink / raw)
To: linusw, brgl, baruch, Yun Zhou
Cc: Bartosz Golaszewski, linux-gpio, linux-kernel
In-Reply-To: <20260608084334.2960803-1-yun.zhou@windriver.com>
On Mon, 08 Jun 2026 16:43:34 +0800, Yun Zhou wrote:
> mvebu_pwm_suspend() and mvebu_pwm_resume() are called for all GPIO
> banks during suspend/resume, but not all banks have PWM functionality.
> GPIO banks without PWM have mvchip->mvpwm set to NULL.
>
> Calling mvebu_pwm_suspend() with mvpwm == NULL causes a NULL pointer
> dereference when it tries to access mvpwm->blink_select.
>
> [...]
Applied, thanks!
[1/1] gpio: mvebu: fix NULL pointer dereference in suspend/resume
https://git.kernel.org/brgl/c/8c784f357364a5424ce1e45d193b0900bbc88fd6
Best regards,
--
Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
^ permalink raw reply
* Re: [PATCH v2] gpio: mt7621: fix interrupt banks mapping on gpio chips
From: Sergio Paracuellos @ 2026-06-08 14:27 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: linusw, tglx, grant.likely, anna-maria, vicencb, linux-kernel,
linux-gpio
In-Reply-To: <CAMRc=MdeyVNd9CZQid99sHoXQiGD=y9USCkUjMT=Sy3nERpAiw@mail.gmail.com>
On Mon, Jun 8, 2026 at 4:02 PM Bartosz Golaszewski <brgl@kernel.org> wrote:
>
> On Mon, 8 Jun 2026 11:40:57 +0200, Sergio Paracuellos
> <sergio.paracuellos@gmail.com> said:
> > Hi,
> >
> > On Mon, Jun 8, 2026 at 11:05 AM Bartosz Golaszewski <brgl@kernel.org> wrote:
> >>
> >> On Tue, 2 Jun 2026 16:25:13 +0200, Sergio Paracuellos
> >> <sergio.paracuellos@gmail.com> said:
> >> > The GPIO controller's registers are organized as sets of eight 32-bit
> >> > registers with each set controlling a bank of up to 32 pins. A single
> >> > interrupt is shared for all of the banks handled by the controller.
> >> > The driver implements this using three gpio chip instances every one
> >> > with its own irq chip. Every single pin can generate interrupts having
> >> > a total of 96 possible interrupts here. It looks like there is a problem
> >> > with interrupts being properly mapped to the gpio bank using this solution.
> >> > This problem report is in the following lore's link [0].
> >> >
> >> > Device tree is using two cells for this, so only the interrupt pin and the
> >> > interrupt type are described there. Changing to have three cells to setup
> >> > also the bank and implement 'of_node_instance_match()' would also work but
> >> > this would be an ABI breakage and also a bit incoherent since gpios itself
> >> > are also using two cells and properly mapped in desired bank using through
> >> > its pin number on 'of_xlate()'.
> >> >
> >> > That said, register a linear IRQ domain of the total of 96 interrupts shared
> >> > with the three gpio chip instances so the bank and the interrupt is properly
> >> > decoded and devices using gpio IRQs properly work.
> >> >
> >> > [0]: https://lore.kernel.org/linux-gpio/CAAMcf8C_A9dJ_v4QRKtb9eGNOpJ7BZNOGsFP4i2WFOZxOVBPnQ@mail.gmail.com/T/#u
> >> >
> >> > Fixes: 4ba9c3afda41 ("gpio: mt7621: Add a driver for MT7621")
> >> > Co-developed-by: Vicente Bergas <vicencb@gmail.com>
> >> > Signed-off-by: Vicente Bergas <vicencb@gmail.com>
> >> > Tested-by: Vicente Bergas <vicencb@gmail.com>
> >> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> >> > ---
> >>
> >> Hi!
> >>
> >> Can you look at the sashiko review? Especially the bit about tracking the
> >> GPIOD_FLAG_IRQ_IS_ENABLED flag looks correct.
> >
> > I got rid of those two calls (gpiochip_enable_irq() and
> > gpiochip_disable_irq()) because the driver "gpio-brcmstb" which is the
> > one I based my changes on was not used them at all. We have not found
> > anything weird related with that on testing. I do believe that since
> > we are using our own callbacks for 'irq_request_resources()' and
> > 'irq_release_resources()' we are safe here. Regarding the others I am
> > not sure, but the introduction of the remove stuff for the irq domain
> > is because there are no devm_* functions for that. Other resources in
> > driver are using devm versions so I think the changes are ok as they
> > are...
> >
>
> It's about GPIO core: a GPIO that appears as "free" (users can request it) but
> was earlier enabled for interrupts cannot be requested in output mode - only
> input works. Without this flag set, gpiod_direction_output_nonotify() will allow
> you to set direction to output.
I see. I need Vicente to re-test without removing
gpiochip_enable_irq() and gpiochip_disable_irq() to see if everything
is still ok.
Vicente, would you mind to test the following change on top of this v2 PATCH?
diff --git a/drivers/gpio/gpio-mt7621.c b/drivers/gpio/gpio-mt7621.c
index c36aa0abd0c6..a814885ccd5d 100644
--- a/drivers/gpio/gpio-mt7621.c
+++ b/drivers/gpio/gpio-mt7621.c
@@ -144,6 +144,8 @@ mediatek_gpio_irq_unmask(struct irq_data *d)
u32 mask = mt7621_gpio_hwirq_to_offset(d->hwirq, rg);
u32 rise, fall, high, low;
+ gpiochip_enable_irq(gc, mask);
+
guard(gpio_generic_lock_irqsave)(&rg->chip);
rise = mtk_gpio_r32(rg, GPIO_REG_REDGE);
@@ -174,6 +176,8 @@ mediatek_gpio_irq_mask(struct irq_data *d)
mtk_gpio_w32(rg, GPIO_REG_HLVL, high & ~BIT(mask));
mtk_gpio_w32(rg, GPIO_REG_LLVL, low & ~BIT(mask));
}
+
+ gpiochip_disable_irq(gc, mask);
}
Thanks,
Sergio Paracuellos
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox