[PATCH] pinctrl: qcom: msm: mark certain pins as invalid for interrupts

[PATCH] pinctrl: qcom: msm: mark certain pins as invalid for interrupts

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

When requesting pins whose intr_detection_width setting is not 1 or 2
for interrupts (for example by running `gpiomon -c 0 113` on RB2), we'll
hit a BUG() in msm_gpio_irq_set_type(). Potentially crashing the kernel
due to an invalid request from user-space is not optimal, so let's go
through the pins and mark those that would fail the check as invalid for
the irq chip as we should not even register them as available irqs.

This function can be extended if we determine that there are more
corner-cases like this.

Fixes: f365be092572 ("pinctrl: Add Qualcomm TLMM driver")
Cc: stable@vger.kernel.org
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/pinctrl/qcom/pinctrl-msm.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
index f012ea88aa22c..77e0c2f023455 100644
--- a/drivers/pinctrl/qcom/pinctrl-msm.c
+++ b/drivers/pinctrl/qcom/pinctrl-msm.c
@@ -1038,6 +1038,24 @@ static bool msm_gpio_needs_dual_edge_parent_workaround(struct irq_data *d,
 	       test_bit(d->hwirq, pctrl->skip_wake_irqs);
 }
 
+static void msm_gpio_irq_init_valid_mask(struct gpio_chip *gc,
+					 unsigned long *valid_mask,
+					 unsigned int ngpios)
+{
+	struct msm_pinctrl *pctrl = gpiochip_get_data(gc);
+	const struct msm_pingroup *g;
+	int i;
+
+	bitmap_fill(valid_mask, ngpios);
+
+	for (i = 0; i < ngpios; i++) {
+		g = &pctrl->soc->groups[i];
+		if (g->intr_detection_width != 1 &&
+		    g->intr_detection_width != 2)
+			clear_bit(i, valid_mask);
+	}
+}
+
 static int msm_gpio_irq_set_type(struct irq_data *d, unsigned int type)
 {
 	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
@@ -1441,6 +1459,7 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl)
 	girq->default_type = IRQ_TYPE_NONE;
 	girq->handler = handle_bad_irq;
 	girq->parents[0] = pctrl->irq;
+	girq->init_valid_mask = msm_gpio_irq_init_valid_mask;
 
 	ret = gpiochip_add_data(&pctrl->chip, pctrl);
 	if (ret) {
-- 
2.48.1

Re: [PATCH] pinctrl: qcom: msm: mark certain pins as invalid for interrupts

[Bjorn Andersson]

On Wed, Jun 11, 2025 at 04:39:11PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> When requesting pins whose intr_detection_width setting is not 1 or 2
> for interrupts (for example by running `gpiomon -c 0 113` on RB2), we'll
> hit a BUG() in msm_gpio_irq_set_type(). Potentially crashing the kernel
> due to an invalid request from user-space is not optimal, so let's go
> through the pins and mark those that would fail the check as invalid for
> the irq chip as we should not even register them as available irqs.
> 

I had to go dig into the code to understand why there is a problem with
GPIO 113 on RB2 (i.e. UFS_RESET on SM6115)... I think it would have been
better to document the actual reason for the problem, which is:

"The UFS_RESET pin doesn't have interrupt logic, but is registered as a
GPIO. Requesting the interrupt of this pin hits a BUG() in
msm_gpio_irq_set_type() because intr_detection_width is invalid"

> This function can be extended if we determine that there are more
> corner-cases like this.
> 

Reviewed-by: Bjorn Andersson <andersson@kernel.org>

Regards,
Bjorn

> Fixes: f365be092572 ("pinctrl: Add Qualcomm TLMM driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
>  drivers/pinctrl/qcom/pinctrl-msm.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
> index f012ea88aa22c..77e0c2f023455 100644
> --- a/drivers/pinctrl/qcom/pinctrl-msm.c
> +++ b/drivers/pinctrl/qcom/pinctrl-msm.c
> @@ -1038,6 +1038,24 @@ static bool msm_gpio_needs_dual_edge_parent_workaround(struct irq_data *d,
>  	       test_bit(d->hwirq, pctrl->skip_wake_irqs);
>  }
>  
> +static void msm_gpio_irq_init_valid_mask(struct gpio_chip *gc,
> +					 unsigned long *valid_mask,
> +					 unsigned int ngpios)
> +{
> +	struct msm_pinctrl *pctrl = gpiochip_get_data(gc);
> +	const struct msm_pingroup *g;
> +	int i;
> +
> +	bitmap_fill(valid_mask, ngpios);
> +
> +	for (i = 0; i < ngpios; i++) {
> +		g = &pctrl->soc->groups[i];
> +		if (g->intr_detection_width != 1 &&
> +		    g->intr_detection_width != 2)
> +			clear_bit(i, valid_mask);
> +	}
> +}
> +
>  static int msm_gpio_irq_set_type(struct irq_data *d, unsigned int type)
>  {
>  	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> @@ -1441,6 +1459,7 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl)
>  	girq->default_type = IRQ_TYPE_NONE;
>  	girq->handler = handle_bad_irq;
>  	girq->parents[0] = pctrl->irq;
> +	girq->init_valid_mask = msm_gpio_irq_init_valid_mask;
>  
>  	ret = gpiochip_add_data(&pctrl->chip, pctrl);
>  	if (ret) {
> -- 
> 2.48.1
> 

Re: [PATCH] pinctrl: qcom: msm: mark certain pins as invalid for interrupts

[Bartosz Golaszewski]

On Wed, Jun 11, 2025 at 5:03 PM Bjorn Andersson <andersson@kernel.org> wrote:
>
> On Wed, Jun 11, 2025 at 04:39:11PM +0200, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > When requesting pins whose intr_detection_width setting is not 1 or 2
> > for interrupts (for example by running `gpiomon -c 0 113` on RB2), we'll
> > hit a BUG() in msm_gpio_irq_set_type(). Potentially crashing the kernel
> > due to an invalid request from user-space is not optimal, so let's go
> > through the pins and mark those that would fail the check as invalid for
> > the irq chip as we should not even register them as available irqs.
> >
>
> I had to go dig into the code to understand why there is a problem with
> GPIO 113 on RB2 (i.e. UFS_RESET on SM6115)... I think it would have been
> better to document the actual reason for the problem, which is:
>
> "The UFS_RESET pin doesn't have interrupt logic, but is registered as a
> GPIO. Requesting the interrupt of this pin hits a BUG() in
> msm_gpio_irq_set_type() because intr_detection_width is invalid"

The UFS_RESET() case is the one I figured out initially but then
realized that the issue will be the same for other non-PINGROUP()
macros which set all the interrupt related fields to -1 so I made the
message more generic. I will include the above in v2.

Bart