Devicetree
 help / color / mirror / Atom feed
From: Benjamin Larsson <benjamin.larsson@genexis.eu>
To: Ahmed Naseef <naseefkm@gmail.com>, linux-gpio@vger.kernel.org
Cc: Christian Marangi <ansuelsmth@gmail.com>,
	Conor Dooley <conor+dt@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Linus Walleij <linusw@kernel.org>,
	Lorenzo Bianconi <lorenzo@kernel.org>,
	Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>,
	Rob Herring <robh@kernel.org>,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-mediatek@lists.infradead.org
Subject: Re: [PATCH 1/3] pinctrl: airoha: limit GPIO interrupts to interrupt-capable pins
Date: Fri, 14 Aug 2026 21:38:22 +0200	[thread overview]
Message-ID: <1cd344b3-22a3-43d2-83b6-2eb01b3245d3@genexis.eu> (raw)
In-Reply-To: <5a7f97adbca73f2bd01393ce0ca840c2a6f43f76.1786262697.git.naseefkm@gmail.com>

Hi.

On 09/08/2026 10:04, Ahmed Naseef wrote:
> The driver assumes that every one of the AIROHA_NUM_PINS GPIOs can raise
> an interrupt. That holds for the SoCs supported so far, but not for every
> member of the family: on EN7528 only GPIO0-GPIO15 are wired to the
> interrupt controller.
> 
> Without this the driver hands out interrupts for GPIOs that can never
> fire, and the interrupt handler reads status registers that are not
> backed by any pin.
> 
> Add a num_irq_pins field to the per-SoC match data and use it to bound
> the interrupt callbacks and to size the loop in the interrupt handler.
> Feed it to gpiolib through gpio_irq_chip::init_valid_mask as well, so
> that gpiod_to_irq() fails for a pin that cannot be an interrupt source
> instead of deferring the failure to request_irq().
> 
> Signed-off-by: Ahmed Naseef <naseefkm@gmail.com>
> ---
>   drivers/pinctrl/airoha/airoha-common.h  |  3 +++
>   drivers/pinctrl/airoha/pinctrl-airoha.c | 33 +++++++++++++++++++++----
>   drivers/pinctrl/airoha/pinctrl-an7563.c |  1 +
>   drivers/pinctrl/airoha/pinctrl-an7581.c |  1 +
>   drivers/pinctrl/airoha/pinctrl-an7583.c |  1 +
>   drivers/pinctrl/airoha/pinctrl-en7523.c |  1 +
>   6 files changed, 35 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/pinctrl/airoha/airoha-common.h b/drivers/pinctrl/airoha/airoha-common.h
> index c1acbfb7426e..16f7abcdfb83 100644
> --- a/drivers/pinctrl/airoha/airoha-common.h
> +++ b/drivers/pinctrl/airoha/airoha-common.h
> @@ -127,6 +127,7 @@ struct airoha_pinctrl {
>   
>   	struct gpio_chip gpiochip;
>   	struct airoha_gpiochip_regs *gpio_regs;
> +	unsigned int num_irq_pins;
>   };
>   
>   struct airoha_pinctrl_match_data {
> @@ -140,6 +141,8 @@ struct airoha_pinctrl_match_data {
>   	const struct airoha_pinctrl_func *funcs;
>   	const unsigned int num_funcs;
>   	const struct airoha_pinctrl_confs_info confs_info[AIROHA_PINCTRL_CONFS_MAX];
> +	/* number of GPIOs wired to the interrupt controller */
> +	const unsigned int num_irq_pins;
>   };
>   
>   int airoha_pinctrl_probe(struct platform_device *pdev);
> diff --git a/drivers/pinctrl/airoha/pinctrl-airoha.c b/drivers/pinctrl/airoha/pinctrl-airoha.c
> index f505a3f69c5d..52a768c859b7 100644
> --- a/drivers/pinctrl/airoha/pinctrl-airoha.c
> +++ b/drivers/pinctrl/airoha/pinctrl-airoha.c
> @@ -213,7 +213,7 @@ static void airoha_irq_unmask(struct irq_data *data)
>   	u32 mask = GENMASK(2 * offset + 1, 2 * offset);
>   	u32 val = BIT(2 * offset);
>   
> -	if (WARN_ON_ONCE(data->hwirq >= AIROHA_NUM_PINS))
> +	if (WARN_ON_ONCE(data->hwirq >= pinctrl->num_irq_pins))
>   		return;
>   
>   	gpiochip_enable_irq(gc, irqd_to_hwirq(data));
> @@ -249,7 +249,7 @@ static void airoha_irq_mask(struct irq_data *data)
>   	u8 index = data->hwirq / AIROHA_REG_GPIOCTRL_NUM_PIN;
>   	u32 mask = GENMASK(2 * offset + 1, 2 * offset);
>   
> -	if (data->hwirq >= AIROHA_NUM_PINS)
> +	if (data->hwirq >= pinctrl->num_irq_pins)
>   		return;
>   
>   	regmap_clear_bits(pinctrl->regmap, gpio_regs->level[index], mask);
> @@ -265,7 +265,7 @@ static void airoha_irq_ack(struct irq_data *data)
>   	u8 offset = data->hwirq % AIROHA_PIN_BANK_SIZE;
>   	u8 index = data->hwirq / AIROHA_PIN_BANK_SIZE;
>   
> -	if (data->hwirq >= AIROHA_NUM_PINS)
> +	if (data->hwirq >= pinctrl->num_irq_pins)
>   		return;
>   
>   	regmap_write(pinctrl->regmap, gpio_regs->status[index], BIT(offset));
> @@ -273,7 +273,10 @@ static void airoha_irq_ack(struct irq_data *data)
>   
>   static int airoha_irq_type(struct irq_data *data, unsigned int type)
>   {
> -	if (data->hwirq >= AIROHA_NUM_PINS)
> +	struct gpio_chip *gc = irq_data_get_irq_chip_data(data);
> +	struct airoha_pinctrl *pinctrl = gpiochip_get_data(gc);
> +
> +	if (data->hwirq >= pinctrl->num_irq_pins)
>   		return -EINVAL;
>   
>   	if (type == IRQ_TYPE_NONE) {
> @@ -304,9 +307,11 @@ static irqreturn_t airoha_irq_handler(int irq, void *data)
>   {
>   	struct airoha_pinctrl *pinctrl = data;
>   	bool handled = false;
> +	unsigned int nbanks;
>   	int i;
>   
> -	for (i = 0; i < ARRAY_SIZE(irq_status_regs); i++) {
> +	nbanks = DIV_ROUND_UP(pinctrl->num_irq_pins, AIROHA_PIN_BANK_SIZE);
> +	for (i = 0; i < nbanks; i++) {
>   		struct gpio_irq_chip *girq = &pinctrl->gpiochip.irq;
>   		u32 regmap;
>   		unsigned long status;
> @@ -340,6 +345,22 @@ static const struct irq_chip airoha_gpio_irq_chip = {
>   	GPIOCHIP_IRQ_RESOURCE_HELPERS,
>   };
>   
> +/*
> + * Mark the GPIOs that are not wired to the interrupt controller as not
> + * valid, so that gpiod_to_irq() fails for them with -ENXIO instead of
> + * handing out an interrupt that can never fire.
> + */
> +static void airoha_gpio_init_valid_mask(struct gpio_chip *gc,
> +					unsigned long *valid_mask,
> +					unsigned int ngpios)
> +{
> +	struct airoha_pinctrl *pinctrl = gpiochip_get_data(gc);
> +	unsigned int num_irq_pins = pinctrl->num_irq_pins;
> +
> +	if (num_irq_pins < ngpios)
> +		bitmap_clear(valid_mask, num_irq_pins, ngpios - num_irq_pins);
> +}

I'm not sure if there is something that gives a sane error log if this 
trigger. Maybe make it explicit?

Either way:
Reviewed-by: Benjamin Larsson <benjamin.larsson@genexis.eu>

> +
>   static int airoha_pinctrl_add_gpiochip(struct airoha_pinctrl *pinctrl,
>   					struct platform_device *pdev)
>   {
> @@ -362,6 +383,7 @@ static int airoha_pinctrl_add_gpiochip(struct airoha_pinctrl *pinctrl,
>   
>   	girq->default_type = IRQ_TYPE_NONE;
>   	girq->handler = handle_bad_irq;
> +	girq->init_valid_mask = airoha_gpio_init_valid_mask;
>   	gpio_irq_chip_set_chip(girq, &airoha_gpio_irq_chip);
>   
>   	irq = platform_get_irq(pdev, 0);
> @@ -848,6 +870,7 @@ int airoha_pinctrl_probe(struct platform_device *pdev)
>   	pinctrl->grps = data->grps;
>   	pinctrl->funcs = data->funcs;
>   	pinctrl->confs_info = data->confs_info;
> +	pinctrl->num_irq_pins = data->num_irq_pins;
>   
>   	err = pinctrl_enable(pinctrl->ctrl);
>   	if (err)
> diff --git a/drivers/pinctrl/airoha/pinctrl-an7563.c b/drivers/pinctrl/airoha/pinctrl-an7563.c
> index 40cbbe90cc46..f011c6c9ccce 100644
> --- a/drivers/pinctrl/airoha/pinctrl-an7563.c
> +++ b/drivers/pinctrl/airoha/pinctrl-an7563.c
> @@ -1069,6 +1069,7 @@ static const struct airoha_pinctrl_match_data pinctrl_match_data = {
>   	.num_grps = ARRAY_SIZE(pinctrl_groups),
>   	.funcs = pinctrl_funcs,
>   	.num_funcs = ARRAY_SIZE(pinctrl_funcs),
> +	.num_irq_pins = AIROHA_NUM_PINS,
>   	.confs_info = {
>   		[AIROHA_PINCTRL_CONFS_PULLUP] = {
>   			.confs = pinctrl_pullup_conf,
> diff --git a/drivers/pinctrl/airoha/pinctrl-an7581.c b/drivers/pinctrl/airoha/pinctrl-an7581.c
> index 2fcf88106e11..bfb777594811 100644
> --- a/drivers/pinctrl/airoha/pinctrl-an7581.c
> +++ b/drivers/pinctrl/airoha/pinctrl-an7581.c
> @@ -1441,6 +1441,7 @@ static const struct airoha_pinctrl_match_data pinctrl_match_data = {
>   	.num_grps = ARRAY_SIZE(pinctrl_groups),
>   	.funcs = pinctrl_funcs,
>   	.num_funcs = ARRAY_SIZE(pinctrl_funcs),
> +	.num_irq_pins = AIROHA_NUM_PINS,
>   	.confs_info = {
>   		[AIROHA_PINCTRL_CONFS_PULLUP] = {
>   			.confs = pinctrl_pullup_conf,
> diff --git a/drivers/pinctrl/airoha/pinctrl-an7583.c b/drivers/pinctrl/airoha/pinctrl-an7583.c
> index 2c3a75c35915..1cd0f442ddc1 100644
> --- a/drivers/pinctrl/airoha/pinctrl-an7583.c
> +++ b/drivers/pinctrl/airoha/pinctrl-an7583.c
> @@ -1471,6 +1471,7 @@ static const struct airoha_pinctrl_match_data pinctrl_match_data = {
>   	.num_grps = ARRAY_SIZE(pinctrl_groups),
>   	.funcs = pinctrl_funcs,
>   	.num_funcs = ARRAY_SIZE(pinctrl_funcs),
> +	.num_irq_pins = AIROHA_NUM_PINS,
>   	.confs_info = {
>   		[AIROHA_PINCTRL_CONFS_PULLUP] = {
>   			.confs = pinctrl_pullup_conf,
> diff --git a/drivers/pinctrl/airoha/pinctrl-en7523.c b/drivers/pinctrl/airoha/pinctrl-en7523.c
> index 5aa39bacf460..b0c5e60f0aeb 100644
> --- a/drivers/pinctrl/airoha/pinctrl-en7523.c
> +++ b/drivers/pinctrl/airoha/pinctrl-en7523.c
> @@ -1113,6 +1113,7 @@ static const struct airoha_pinctrl_match_data pinctrl_match_data = {
>   	.num_grps = ARRAY_SIZE(pinctrl_groups),
>   	.funcs = pinctrl_funcs,
>   	.num_funcs = ARRAY_SIZE(pinctrl_funcs),
> +	.num_irq_pins = AIROHA_NUM_PINS,
>   	.confs_info = {
>   		[AIROHA_PINCTRL_CONFS_PULLUP] = {
>   			.confs = pinctrl_pullup_conf,


  parent reply	other threads:[~2026-08-14 19:38 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-09  8:04 [PATCH 0/3] pinctrl: airoha: add EcoNet EN7528 pin controller support Ahmed Naseef
2026-08-09  8:04 ` [PATCH 1/3] pinctrl: airoha: limit GPIO interrupts to interrupt-capable pins Ahmed Naseef
2026-08-09  8:18   ` sashiko-bot
2026-08-14 19:38   ` Benjamin Larsson [this message]
2026-08-18  5:17     ` Ahmed Naseef
2026-08-09  8:04 ` [PATCH 2/3] dt-bindings: pinctrl: Add EcoNet EN7528 pin controller Ahmed Naseef
2026-08-09  8:15   ` sashiko-bot
2026-08-28  7:02     ` Krzysztof Kozlowski
2026-08-28  6:59   ` Krzysztof Kozlowski
2026-08-28 14:33     ` Ahmed Naseef
2026-08-09  8:05 ` [PATCH 3/3] pinctrl: airoha: add support of en7528 SoC Ahmed Naseef
2026-08-10  7:23 ` [PATCH 0/3] pinctrl: airoha: add EcoNet EN7528 pin controller support Linus Walleij
2026-08-10  7:27   ` Christian Marangi (Ansuel)
2026-08-11  6:17     ` Linus Walleij

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=1cd344b3-22a3-43d2-83b6-2eb01b3245d3@genexis.eu \
    --to=benjamin.larsson@genexis.eu \
    --cc=ansuelsmth@gmail.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=linusw@kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=lorenzo@kernel.org \
    --cc=mikhail.kshevetskiy@iopsys.eu \
    --cc=naseefkm@gmail.com \
    --cc=robh@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