[PATCH] soc/tegra: pmc: Fix unsafe generic_handle_irq() call

Prathamesh Shete posted 1 patch 1 month ago
drivers/soc/tegra/pmc.c | 106 ++++++++++++++++++++++++++++------------
1 file changed, 76 insertions(+), 30 deletions(-)
[PATCH] soc/tegra: pmc: Fix unsafe generic_handle_irq() call
Posted by Prathamesh Shete 1 month ago
Currently, when resuming from system suspend on Tegra platforms,
the following warning is observed:

WARNING: CPU: 0 PID: 14459 at kernel/irq/irqdesc.c:666
Call trace:
 handle_irq_desc+0x20/0x58 (P)
 tegra186_pmc_wake_syscore_resume+0xe4/0x15c
 syscore_resume+0x3c/0xb8
 suspend_devices_and_enter+0x510/0x540
 pm_suspend+0x16c/0x1d8

The warning occurs because generic_handle_irq() is being called from
a non-interrupt context which is considered as unsafe.

Fix this warning by deferring generic_handle_irq() call to an IRQ work
which gets executed in hard IRQ context where generic_handle_irq()
can be called safely.

When PREEMPT_RT kernels are used, regular IRQ work (initialized with
init_irq_work) is deferred to run in per-CPU kthreads in preemptible
context rather than hard IRQ context. Hence, use the IRQ_WORK_INIT_HARD
variant so that with PREEMPT_RT kernels, the IRQ work is processed in
hardirq context instead of being deferred to a thread which is required
for calling generic_handle_irq().

On non-PREEMPT_RT kernels, both init_irq_work() and IRQ_WORK_INIT_HARD()
execute in IRQ context, so this change has no functional impact for
standard kernel configurations.

Signed-off-by: Petlozu Pravareshwar <petlozup@nvidia.com>
Signed-off-by: Prathamesh Shete <pshete@nvidia.com>
---
 drivers/soc/tegra/pmc.c | 106 ++++++++++++++++++++++++++++------------
 1 file changed, 76 insertions(+), 30 deletions(-)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index f3760a3b3026..a0cc276e235a 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -28,6 +28,7 @@
 #include <linux/iopoll.h>
 #include <linux/irqdomain.h>
 #include <linux/irq.h>
+#include <linux/irq_work.h>
 #include <linux/kernel.h>
 #include <linux/of_address.h>
 #include <linux/of_clk.h>
@@ -336,6 +337,8 @@ struct tegra_wake_event {
 		},					\
 	}
 
+#define TEGRA_PMC_MAX_WAKE_VECTORS	4
+
 struct tegra_pmc_soc {
 	unsigned int num_powergates;
 	const char *const *powergates;
@@ -468,6 +471,10 @@ struct tegra_pmc {
 	unsigned long *wake_sw_status_map;
 	unsigned long *wake_cntrl_level_map;
 	struct syscore syscore;
+
+	/* Pending wake IRQ processing */
+	u32 pending_wake_status[TEGRA_PMC_MAX_WAKE_VECTORS];
+	struct irq_work pending_wake_irq_work;
 };
 
 static struct tegra_pmc *pmc = &(struct tegra_pmc) {
@@ -1905,8 +1912,59 @@ static int tegra_pmc_parse_dt(struct tegra_pmc *pmc, struct device_node *np)
 	return 0;
 }
 
+/* translate sc7 wake sources back into IRQs to catch edge triggered wakeups */
+static void tegra186_pmc_wake_irq_work_handler(struct irq_work *work)
+{
+	struct tegra_pmc *pmc = container_of(work, struct tegra_pmc,
+					      pending_wake_irq_work);
+	unsigned int i, wake;
+	unsigned long pending_wake_status;
+
+	for (i = 0; i < pmc->soc->max_wake_vectors; i++) {
+		pending_wake_status = pmc->pending_wake_status[i];
+
+		for_each_set_bit(wake, &pending_wake_status, 32) {
+			irq_hw_number_t hwirq = wake + (i * 32);
+			struct irq_desc *desc;
+			unsigned int irq;
+
+			irq = irq_find_mapping(pmc->domain, hwirq);
+			if (!irq) {
+				dev_warn(pmc->dev, "No IRQ found for WAKE%lu!\n",
+					 hwirq);
+				continue;
+			}
+
+			dev_dbg(pmc->dev,
+				"Resume caused by WAKE%lu mapped to IRQ %d\n",
+				hwirq, irq);
+
+			desc = irq_to_desc(irq);
+			if (!desc) {
+				dev_warn(pmc->dev,
+					 "No descriptor found for IRQ %d\n",
+					irq);
+				continue;
+			}
+
+			if (!desc->action || !desc->action->name)
+				continue;
+
+			generic_handle_irq(irq);
+		}
+
+		pmc->pending_wake_status[i] = 0;
+	}
+}
+
 static int tegra_pmc_init(struct tegra_pmc *pmc)
 {
+	if (pmc->soc->max_wake_vectors > TEGRA_PMC_MAX_WAKE_VECTORS) {
+		dev_err(pmc->dev, "max_wake_vectors (%u) exceeds maximum (%u)\n",
+			pmc->soc->max_wake_vectors, TEGRA_PMC_MAX_WAKE_VECTORS);
+		return -EINVAL;
+	}
+
 	if (pmc->soc->max_wake_events > 0) {
 		pmc->wake_type_level_map = bitmap_zalloc(pmc->soc->max_wake_events, GFP_KERNEL);
 		if (!pmc->wake_type_level_map)
@@ -1923,6 +1981,12 @@ static int tegra_pmc_init(struct tegra_pmc *pmc)
 		pmc->wake_cntrl_level_map = bitmap_zalloc(pmc->soc->max_wake_events, GFP_KERNEL);
 		if (!pmc->wake_cntrl_level_map)
 			return -ENOMEM;
+
+		/* Initialize IRQ work for processing wake IRQs
+		 * Must use HARD_IRQ variant to run in hard IRQ context on PREEMPT_RT
+		 * because we call generic_handle_irq() which requires hard IRQ context.
+		 */
+		pmc->pending_wake_irq_work = IRQ_WORK_INIT_HARD(tegra186_pmc_wake_irq_work_handler);
 	}
 
 	if (pmc->soc->init)
@@ -3129,47 +3193,29 @@ static void wke_clear_wake_status(struct tegra_pmc *pmc)
 	}
 }
 
-/* translate sc7 wake sources back into IRQs to catch edge triggered wakeups */
-static void tegra186_pmc_process_wake_events(struct tegra_pmc *pmc, unsigned int index,
-					     unsigned long status)
-{
-	unsigned int wake;
-
-	dev_dbg(pmc->dev, "Wake[%d:%d]  status=%#lx\n", (index * 32) + 31, index * 32, status);
-
-	for_each_set_bit(wake, &status, 32) {
-		irq_hw_number_t hwirq = wake + 32 * index;
-		struct irq_desc *desc;
-		unsigned int irq;
-
-		irq = irq_find_mapping(pmc->domain, hwirq);
-
-		desc = irq_to_desc(irq);
-		if (!desc || !desc->action || !desc->action->name) {
-			dev_dbg(pmc->dev, "Resume caused by WAKE%ld, IRQ %d\n", hwirq, irq);
-			continue;
-		}
-
-		dev_dbg(pmc->dev, "Resume caused by WAKE%ld, %s\n", hwirq, desc->action->name);
-		generic_handle_irq(irq);
-	}
-}
-
 static void tegra186_pmc_wake_syscore_resume(void *data)
 {
-	u32 status, mask;
+	u32 mask;
 	unsigned int i;
 
 	for (i = 0; i < pmc->soc->max_wake_vectors; i++) {
 		mask = readl(pmc->wake + WAKE_AOWAKE_TIER2_ROUTING(i));
-		status = readl(pmc->wake + WAKE_AOWAKE_STATUS_R(i)) & mask;
-
-		tegra186_pmc_process_wake_events(pmc, i, status);
+		pmc->pending_wake_status[i] = readl(pmc->wake + WAKE_AOWAKE_STATUS_R(i)) & mask;
 	}
+	/* Schedule IRQ work to process wake IRQs (if any) */
+	irq_work_queue(&pmc->pending_wake_irq_work);
 }
 
 static int tegra186_pmc_wake_syscore_suspend(void *data)
 {
+	unsigned int i;
+
+	/* Check if there are unhandled wake IRQs */
+	for (i = 0; i < pmc->soc->max_wake_vectors; i++)
+		if (pmc->pending_wake_status[i])
+			dev_warn(pmc->dev,
+				 "Unhandled wake IRQs pending vector[%u]: 0x%x\n",
+				 i, pmc->pending_wake_status[i]);
 	wke_read_sw_wake_status(pmc);
 
 	/* flip the wakeup trigger for dual-edge triggered pads
-- 
2.25.1
Re: [PATCH] soc/tegra: pmc: Fix unsafe generic_handle_irq() call
Posted by Jon Hunter 4 weeks ago

On 08/01/2026 05:01, Prathamesh Shete wrote:
> Currently, when resuming from system suspend on Tegra platforms,
> the following warning is observed:
> 
> WARNING: CPU: 0 PID: 14459 at kernel/irq/irqdesc.c:666
> Call trace:
>   handle_irq_desc+0x20/0x58 (P)
>   tegra186_pmc_wake_syscore_resume+0xe4/0x15c
>   syscore_resume+0x3c/0xb8
>   suspend_devices_and_enter+0x510/0x540
>   pm_suspend+0x16c/0x1d8
> 
> The warning occurs because generic_handle_irq() is being called from
> a non-interrupt context which is considered as unsafe.
> 
> Fix this warning by deferring generic_handle_irq() call to an IRQ work
> which gets executed in hard IRQ context where generic_handle_irq()
> can be called safely.
> 
> When PREEMPT_RT kernels are used, regular IRQ work (initialized with
> init_irq_work) is deferred to run in per-CPU kthreads in preemptible
> context rather than hard IRQ context. Hence, use the IRQ_WORK_INIT_HARD
> variant so that with PREEMPT_RT kernels, the IRQ work is processed in
> hardirq context instead of being deferred to a thread which is required
> for calling generic_handle_irq().
> 
> On non-PREEMPT_RT kernels, both init_irq_work() and IRQ_WORK_INIT_HARD()
> execute in IRQ context, so this change has no functional impact for
> standard kernel configurations.
> 
> Signed-off-by: Petlozu Pravareshwar <petlozup@nvidia.com>
> Signed-off-by: Prathamesh Shete <pshete@nvidia.com>
> ---
>   drivers/soc/tegra/pmc.c | 106 ++++++++++++++++++++++++++++------------
>   1 file changed, 76 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
> index f3760a3b3026..a0cc276e235a 100644
> --- a/drivers/soc/tegra/pmc.c
> +++ b/drivers/soc/tegra/pmc.c
> @@ -28,6 +28,7 @@
>   #include <linux/iopoll.h>
>   #include <linux/irqdomain.h>
>   #include <linux/irq.h>
> +#include <linux/irq_work.h>
>   #include <linux/kernel.h>
>   #include <linux/of_address.h>
>   #include <linux/of_clk.h>
> @@ -336,6 +337,8 @@ struct tegra_wake_event {
>   		},					\
>   	}
>   
> +#define TEGRA_PMC_MAX_WAKE_VECTORS	4
> +
>   struct tegra_pmc_soc {
>   	unsigned int num_powergates;
>   	const char *const *powergates;
> @@ -468,6 +471,10 @@ struct tegra_pmc {
>   	unsigned long *wake_sw_status_map;
>   	unsigned long *wake_cntrl_level_map;
>   	struct syscore syscore;
> +
> +	/* Pending wake IRQ processing */
> +	u32 pending_wake_status[TEGRA_PMC_MAX_WAKE_VECTORS];
> +	struct irq_work pending_wake_irq_work;
>   };
>   
>   static struct tegra_pmc *pmc = &(struct tegra_pmc) {
> @@ -1905,8 +1912,59 @@ static int tegra_pmc_parse_dt(struct tegra_pmc *pmc, struct device_node *np)
>   	return 0;
>   }
>   
> +/* translate sc7 wake sources back into IRQs to catch edge triggered wakeups */
> +static void tegra186_pmc_wake_irq_work_handler(struct irq_work *work)
> +{
> +	struct tegra_pmc *pmc = container_of(work, struct tegra_pmc,
> +					      pending_wake_irq_work);
> +	unsigned int i, wake;
> +	unsigned long pending_wake_status;
> +
> +	for (i = 0; i < pmc->soc->max_wake_vectors; i++) {
> +		pending_wake_status = pmc->pending_wake_status[i];
> +
> +		for_each_set_bit(wake, &pending_wake_status, 32) {
> +			irq_hw_number_t hwirq = wake + (i * 32);
> +			struct irq_desc *desc;
> +			unsigned int irq;
> +
> +			irq = irq_find_mapping(pmc->domain, hwirq);
> +			if (!irq) {
> +				dev_warn(pmc->dev, "No IRQ found for WAKE%lu!\n",
> +					 hwirq);
> +				continue;
> +			}
> +
> +			dev_dbg(pmc->dev,
> +				"Resume caused by WAKE%lu mapped to IRQ %d\n",
> +				hwirq, irq);
> +
> +			desc = irq_to_desc(irq);
> +			if (!desc) {
> +				dev_warn(pmc->dev,
> +					 "No descriptor found for IRQ %d\n",
> +					irq);
> +				continue;
> +			}
> +
> +			if (!desc->action || !desc->action->name)
> +				continue;
> +
> +			generic_handle_irq(irq);
> +		}
> +
> +		pmc->pending_wake_status[i] = 0;
> +	}
> +}
> +
>   static int tegra_pmc_init(struct tegra_pmc *pmc)
>   {
> +	if (pmc->soc->max_wake_vectors > TEGRA_PMC_MAX_WAKE_VECTORS) {
> +		dev_err(pmc->dev, "max_wake_vectors (%u) exceeds maximum (%u)\n",
> +			pmc->soc->max_wake_vectors, TEGRA_PMC_MAX_WAKE_VECTORS);
> +		return -EINVAL;
> +	}
> +
>   	if (pmc->soc->max_wake_events > 0) {
>   		pmc->wake_type_level_map = bitmap_zalloc(pmc->soc->max_wake_events, GFP_KERNEL);
>   		if (!pmc->wake_type_level_map)
> @@ -1923,6 +1981,12 @@ static int tegra_pmc_init(struct tegra_pmc *pmc)
>   		pmc->wake_cntrl_level_map = bitmap_zalloc(pmc->soc->max_wake_events, GFP_KERNEL);
>   		if (!pmc->wake_cntrl_level_map)
>   			return -ENOMEM;
> +
> +		/* Initialize IRQ work for processing wake IRQs
> +		 * Must use HARD_IRQ variant to run in hard IRQ context on PREEMPT_RT
> +		 * because we call generic_handle_irq() which requires hard IRQ context.
> +		 */
> +		pmc->pending_wake_irq_work = IRQ_WORK_INIT_HARD(tegra186_pmc_wake_irq_work_handler);
>   	}
>   
>   	if (pmc->soc->init)
> @@ -3129,47 +3193,29 @@ static void wke_clear_wake_status(struct tegra_pmc *pmc)
>   	}
>   }
>   
> -/* translate sc7 wake sources back into IRQs to catch edge triggered wakeups */
> -static void tegra186_pmc_process_wake_events(struct tegra_pmc *pmc, unsigned int index,
> -					     unsigned long status)
> -{
> -	unsigned int wake;
> -
> -	dev_dbg(pmc->dev, "Wake[%d:%d]  status=%#lx\n", (index * 32) + 31, index * 32, status);
> -
> -	for_each_set_bit(wake, &status, 32) {
> -		irq_hw_number_t hwirq = wake + 32 * index;
> -		struct irq_desc *desc;
> -		unsigned int irq;
> -
> -		irq = irq_find_mapping(pmc->domain, hwirq);
> -
> -		desc = irq_to_desc(irq);
> -		if (!desc || !desc->action || !desc->action->name) {
> -			dev_dbg(pmc->dev, "Resume caused by WAKE%ld, IRQ %d\n", hwirq, irq);
> -			continue;
> -		}
> -
> -		dev_dbg(pmc->dev, "Resume caused by WAKE%ld, %s\n", hwirq, desc->action->name);
> -		generic_handle_irq(irq);
> -	}
> -}
> -
>   static void tegra186_pmc_wake_syscore_resume(void *data)
>   {
> -	u32 status, mask;
> +	u32 mask;
>   	unsigned int i;
>   
>   	for (i = 0; i < pmc->soc->max_wake_vectors; i++) {
>   		mask = readl(pmc->wake + WAKE_AOWAKE_TIER2_ROUTING(i));
> -		status = readl(pmc->wake + WAKE_AOWAKE_STATUS_R(i)) & mask;
> -
> -		tegra186_pmc_process_wake_events(pmc, i, status);
> +		pmc->pending_wake_status[i] = readl(pmc->wake + WAKE_AOWAKE_STATUS_R(i)) & mask;
>   	}

Missing a newline here.

> +	/* Schedule IRQ work to process wake IRQs (if any) */
> +	irq_work_queue(&pmc->pending_wake_irq_work);
>   }
>   
>   static int tegra186_pmc_wake_syscore_suspend(void *data)
>   {
> +	unsigned int i;
> +
> +	/* Check if there are unhandled wake IRQs */
> +	for (i = 0; i < pmc->soc->max_wake_vectors; i++)
> +		if (pmc->pending_wake_status[i])
> +			dev_warn(pmc->dev,
> +				 "Unhandled wake IRQs pending vector[%u]: 0x%x\n",
> +				 i, pmc->pending_wake_status[i]);

Missing a newline here.

>   	wke_read_sw_wake_status(pmc);
>   
>   	/* flip the wakeup trigger for dual-edge triggered pads


Thierry may be able to fix up the missing newlines when applying. With 
that ...

Reviewed-by: Jon Hunter <jonathanh@nvidia.com>
Tested-by: Jon Hunter <jonathanh@nvidia.com>

Thanks!
Jon

-- 
nvpublic