public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Bug Fixes for PRUSS irqchip driver
@ 2023-09-19  6:18 MD Danish Anwar
  2023-09-19  6:18 ` [PATCH 1/3] irqchip/irq-pruss-intc: Fix enabling of intc events MD Danish Anwar
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: MD Danish Anwar @ 2023-09-19  6:18 UTC (permalink / raw)
  To: Grzegorz Jaszczyk, Suman Anna, David Lechner, Roger Quadros,
	Andrew F. Davis, Marc Zyngier, Thomas Gleixner
  Cc: linux-kernel, srk, danishanwar, r-gunasekaran, vigneshr

Hi All,
This series adds some bug fixes for PRUSS irqchip driver.

Thanks and Regards,
MD Danish Anwar

Grygorii Strashko (1):
  irqchip/irq-pruss-intc: Fix listed IRQ type in /proc/interrupts

Suman Anna (2):
  irqchip/irq-pruss-intc: Fix enabling of intc events
  irqchip/irq-pruss-intc: Fix processing of IEP interrupts

 drivers/irqchip/irq-pruss-intc.c | 47 ++++++++++++++++++++++++++++----
 1 file changed, 41 insertions(+), 6 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/3] irqchip/irq-pruss-intc: Fix enabling of intc events
  2023-09-19  6:18 [PATCH 0/3] Bug Fixes for PRUSS irqchip driver MD Danish Anwar
@ 2023-09-19  6:18 ` MD Danish Anwar
  2023-09-19  6:18 ` [PATCH 2/3] irqchip/irq-pruss-intc: Fix listed IRQ type in /proc/interrupts MD Danish Anwar
  2023-09-19  6:19 ` [PATCH 3/3] irqchip/irq-pruss-intc: Fix processing of IEP interrupts MD Danish Anwar
  2 siblings, 0 replies; 9+ messages in thread
From: MD Danish Anwar @ 2023-09-19  6:18 UTC (permalink / raw)
  To: Grzegorz Jaszczyk, Suman Anna, David Lechner, Roger Quadros,
	Andrew F. Davis, Marc Zyngier, Thomas Gleixner
  Cc: linux-kernel, srk, danishanwar, r-gunasekaran, vigneshr

From: Suman Anna <s-anna@ti.com>

PRUSS INTC events are enabled by default once IRQ events are mapped to
channel:host pair. This may cause issues with undesirable IRQs triggering
even before a PRU IRQ is requested which are silently processed by
pruss_intc_irq_handler().

Fix it by masking all events by default except those which are routed to
various PRU cores (Host IRQs 0, 1; 10 through 19 on K3 SoCs), and any
other reserved IRQs routed to other processors. The unmasking of IRQs is
the responsibility of Linux IRQ core when IRQ is actually requested.

Fixes: 04e2d1e06978 ("irqchip/irq-pruss-intc: Add a PRUSS irqchip driver for PRUSS interrupts")

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
Signed-off-by: Suman Anna <s-anna@ti.com>
Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
---
 drivers/irqchip/irq-pruss-intc.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/irqchip/irq-pruss-intc.c b/drivers/irqchip/irq-pruss-intc.c
index 0f64ecb9b1f4..7415817d4c6c 100644
--- a/drivers/irqchip/irq-pruss-intc.c
+++ b/drivers/irqchip/irq-pruss-intc.c
@@ -101,6 +101,7 @@ struct pruss_intc_match_data {
  * @soc_config: cached PRUSS INTC IP configuration data
  * @dev: PRUSS INTC device pointer
  * @lock: mutex to serialize interrupts mapping
+ * @irqs_reserved: bit-mask of reserved host interrupts
  */
 struct pruss_intc {
 	struct pruss_intc_map_record event_channel[MAX_PRU_SYS_EVENTS];
@@ -111,6 +112,7 @@ struct pruss_intc {
 	const struct pruss_intc_match_data *soc_config;
 	struct device *dev;
 	struct mutex lock; /* PRUSS INTC lock */
+	u8 irqs_reserved;
 };
 
 /**
@@ -178,6 +180,7 @@ static void pruss_intc_update_hmr(struct pruss_intc *intc, u8 ch, u8 host)
 static void pruss_intc_map(struct pruss_intc *intc, unsigned long hwirq)
 {
 	struct device *dev = intc->dev;
+	bool enable_hwirq = false;
 	u8 ch, host, reg_idx;
 	u32 val;
 
@@ -187,6 +190,9 @@ static void pruss_intc_map(struct pruss_intc *intc, unsigned long hwirq)
 
 	ch = intc->event_channel[hwirq].value;
 	host = intc->channel_host[ch].value;
+	enable_hwirq = (host < FIRST_PRU_HOST_INT ||
+			host >= FIRST_PRU_HOST_INT + MAX_NUM_HOST_IRQS ||
+			intc->irqs_reserved & BIT(host - FIRST_PRU_HOST_INT));
 
 	pruss_intc_update_cmr(intc, hwirq, ch);
 
@@ -194,8 +200,10 @@ static void pruss_intc_map(struct pruss_intc *intc, unsigned long hwirq)
 	val = BIT(hwirq  % 32);
 
 	/* clear and enable system event */
-	pruss_intc_write_reg(intc, PRU_INTC_ESR(reg_idx), val);
 	pruss_intc_write_reg(intc, PRU_INTC_SECR(reg_idx), val);
+	/* unmask only events going to various PRU and other cores by default */
+	if (enable_hwirq)
+		pruss_intc_write_reg(intc, PRU_INTC_ESR(reg_idx), val);
 
 	if (++intc->channel_host[ch].ref_count == 1) {
 		pruss_intc_update_hmr(intc, ch, host);
@@ -204,7 +212,8 @@ static void pruss_intc_map(struct pruss_intc *intc, unsigned long hwirq)
 		pruss_intc_write_reg(intc, PRU_INTC_HIEISR, host);
 	}
 
-	dev_dbg(dev, "mapped system_event = %lu channel = %d host = %d",
+	dev_dbg(dev, "mapped%s system_event = %lu channel = %d host = %d",
+		enable_hwirq ? " and enabled" : "",
 		hwirq, ch, host);
 
 	mutex_unlock(&intc->lock);
@@ -268,11 +277,14 @@ static void pruss_intc_init(struct pruss_intc *intc)
 
 	/*
 	 * configure polarity (SIPR register) to active high and
-	 * type (SITR register) to level interrupt for all system events
+	 * type (SITR register) to level interrupt for all system events,
+	 * and disable and clear all the system events
 	 */
 	for (i = 0; i < num_event_type_regs; i++) {
 		pruss_intc_write_reg(intc, PRU_INTC_SIPR(i), 0xffffffff);
 		pruss_intc_write_reg(intc, PRU_INTC_SITR(i), 0);
+		pruss_intc_write_reg(intc, PRU_INTC_ECR(i), 0xffffffff);
+		pruss_intc_write_reg(intc, PRU_INTC_SECR(i), 0xffffffff);
 	}
 
 	/* clear all interrupt channel map registers, 4 events per register */
@@ -521,7 +533,7 @@ static int pruss_intc_probe(struct platform_device *pdev)
 	struct pruss_intc *intc;
 	struct pruss_host_irq_data *host_data;
 	int i, irq, ret;
-	u8 max_system_events, irqs_reserved = 0;
+	u8 max_system_events;
 
 	data = of_device_get_match_data(dev);
 	if (!data)
@@ -542,7 +554,7 @@ static int pruss_intc_probe(struct platform_device *pdev)
 		return PTR_ERR(intc->base);
 
 	ret = of_property_read_u8(dev->of_node, "ti,irqs-reserved",
-				  &irqs_reserved);
+				  &intc->irqs_reserved);
 
 	/*
 	 * The irqs-reserved is used only for some SoC's therefore not having
@@ -561,7 +573,7 @@ static int pruss_intc_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	for (i = 0; i < MAX_NUM_HOST_IRQS; i++) {
-		if (irqs_reserved & BIT(i))
+		if (intc->irqs_reserved & BIT(i))
 			continue;
 
 		irq = platform_get_irq_byname(pdev, irq_names[i]);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/3] irqchip/irq-pruss-intc: Fix listed IRQ type in /proc/interrupts
  2023-09-19  6:18 [PATCH 0/3] Bug Fixes for PRUSS irqchip driver MD Danish Anwar
  2023-09-19  6:18 ` [PATCH 1/3] irqchip/irq-pruss-intc: Fix enabling of intc events MD Danish Anwar
@ 2023-09-19  6:18 ` MD Danish Anwar
  2023-09-19  6:19 ` [PATCH 3/3] irqchip/irq-pruss-intc: Fix processing of IEP interrupts MD Danish Anwar
  2 siblings, 0 replies; 9+ messages in thread
From: MD Danish Anwar @ 2023-09-19  6:18 UTC (permalink / raw)
  To: Grzegorz Jaszczyk, Suman Anna, David Lechner, Roger Quadros,
	Andrew F. Davis, Marc Zyngier, Thomas Gleixner
  Cc: linux-kernel, srk, danishanwar, r-gunasekaran, vigneshr

From: Grygorii Strashko <grygorii.strashko@ti.com>

The PRUSS INTC driver doesn't have .irq_set_type() callback implemented and
supports only IRQ_TYPE_LEVEL_HIGH. This resulted in the IRQ properties not
being updated properly and the PRUSS INTC IRQs were listed incorrectly in
/proc/interrupts as Edge.

Example:
  218:          0  4b220000.interrupt-controller  26 Edge      pru10

Fix this by adding a simple .irq_set_type() implementation which checks the
requested IRQ triggering type.

Fixes: 04e2d1e06978 ("irqchip/irq-pruss-intc: Add a PRUSS irqchip driver for PRUSS interrupts")

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
Signed-off-by: Suman Anna <s-anna@ti.com>
Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
---
 drivers/irqchip/irq-pruss-intc.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/irqchip/irq-pruss-intc.c b/drivers/irqchip/irq-pruss-intc.c
index 7415817d4c6c..3cf684ede564 100644
--- a/drivers/irqchip/irq-pruss-intc.c
+++ b/drivers/irqchip/irq-pruss-intc.c
@@ -373,6 +373,14 @@ static int pruss_intc_irq_set_irqchip_state(struct irq_data *data,
 	return 0;
 }
 
+static int pruss_intc_irq_irq_set_type(struct irq_data *data, unsigned int type)
+{
+	if (type != IRQ_TYPE_LEVEL_HIGH)
+		return -EINVAL;
+
+	return 0;
+}
+
 static struct irq_chip pruss_irqchip = {
 	.name			= "pruss-intc",
 	.irq_ack		= pruss_intc_irq_ack,
@@ -382,6 +390,7 @@ static struct irq_chip pruss_irqchip = {
 	.irq_release_resources	= pruss_intc_irq_relres,
 	.irq_get_irqchip_state	= pruss_intc_irq_get_irqchip_state,
 	.irq_set_irqchip_state	= pruss_intc_irq_set_irqchip_state,
+	.irq_set_type		= pruss_intc_irq_irq_set_type,
 };
 
 static int pruss_intc_validate_mapping(struct pruss_intc *intc, int event,
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/3] irqchip/irq-pruss-intc: Fix processing of IEP interrupts
  2023-09-19  6:18 [PATCH 0/3] Bug Fixes for PRUSS irqchip driver MD Danish Anwar
  2023-09-19  6:18 ` [PATCH 1/3] irqchip/irq-pruss-intc: Fix enabling of intc events MD Danish Anwar
  2023-09-19  6:18 ` [PATCH 2/3] irqchip/irq-pruss-intc: Fix listed IRQ type in /proc/interrupts MD Danish Anwar
@ 2023-09-19  6:19 ` MD Danish Anwar
  2023-09-19  7:32   ` Marc Zyngier
  2 siblings, 1 reply; 9+ messages in thread
From: MD Danish Anwar @ 2023-09-19  6:19 UTC (permalink / raw)
  To: Grzegorz Jaszczyk, Suman Anna, David Lechner, Roger Quadros,
	Andrew F. Davis, Marc Zyngier, Thomas Gleixner
  Cc: linux-kernel, srk, danishanwar, r-gunasekaran, vigneshr

From: Suman Anna <s-anna@ti.com>

It was discovered that IEP capture/compare IRQs (event #7 on all SoCs
and event #56 on K3 SoCs) are always triggered twice when PPS is
generated and CMP hit event detected by IEP.

An example of the problem is:
  pruss_intc_irq_handler
   generic_handle_irq
    handle_level_irq
      mask_ack_irq -> IRQ 7 masked and asked in INTC,
                      but it's not yet cleared on HW level
      handle_irq_event()
        <threaded on RT>
           icss_iep_cap_cmp_handler() -> IRQ 7 is actually processed in HW
        irq_finalize_oneshot()
         unmask_irq()
           pruss_intc_irq_unmask() -> IRQ 7 status is still observed as set

The solution is to actually ack these IRQs from pruss_intc_irq_unmask()
after the IRQ source is cleared in HW.

No public errata available for this yet.

Fixes: 04e2d1e06978 ("irqchip/irq-pruss-intc: Add a PRUSS irqchip driver for PRUSS interrupts")

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
Signed-off-by: Suman Anna <s-anna@ti.com>
Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
---
 drivers/irqchip/irq-pruss-intc.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/irqchip/irq-pruss-intc.c b/drivers/irqchip/irq-pruss-intc.c
index 3cf684ede564..9907847dbda8 100644
--- a/drivers/irqchip/irq-pruss-intc.c
+++ b/drivers/irqchip/irq-pruss-intc.c
@@ -70,6 +70,8 @@
 #define MAX_PRU_SYS_EVENTS 160
 #define MAX_PRU_CHANNELS 20
 
+#define MAX_PRU_INT_EVENTS	64
+
 /**
  * struct pruss_intc_map_record - keeps track of actual mapping state
  * @value: The currently mapped value (channel or host)
@@ -85,10 +87,13 @@ struct pruss_intc_map_record {
  * @num_system_events: number of input system events handled by the PRUSS INTC
  * @num_host_events: number of host events (which is equal to number of
  *		     channels) supported by the PRUSS INTC
+ * @quirky_events: bitmask of events that need quirky IRQ handling (limited to
+ *		   (internal sources only for now, so 64 bits suffice)
  */
 struct pruss_intc_match_data {
 	u8 num_system_events;
 	u8 num_host_events;
+	u64 quirky_events;
 };
 
 /**
@@ -304,6 +309,10 @@ static void pruss_intc_irq_ack(struct irq_data *data)
 	struct pruss_intc *intc = irq_data_get_irq_chip_data(data);
 	unsigned int hwirq = data->hwirq;
 
+	if (hwirq < MAX_PRU_INT_EVENTS &&
+	    intc->soc_config->quirky_events & BIT_ULL(hwirq))
+		return;
+
 	pruss_intc_write_reg(intc, PRU_INTC_SICR, hwirq);
 }
 
@@ -320,6 +329,9 @@ static void pruss_intc_irq_unmask(struct irq_data *data)
 	struct pruss_intc *intc = irq_data_get_irq_chip_data(data);
 	unsigned int hwirq = data->hwirq;
 
+	if (hwirq < MAX_PRU_INT_EVENTS &&
+	    intc->soc_config->quirky_events & BIT_ULL(hwirq))
+		pruss_intc_write_reg(intc, PRU_INTC_SICR, hwirq);
 	pruss_intc_write_reg(intc, PRU_INTC_EISR, hwirq);
 }
 
@@ -644,11 +656,13 @@ static int pruss_intc_remove(struct platform_device *pdev)
 static const struct pruss_intc_match_data pruss_intc_data = {
 	.num_system_events = 64,
 	.num_host_events = 10,
+	.quirky_events = BIT_ULL(7), /* IEP capture/compare event */
 };
 
 static const struct pruss_intc_match_data icssg_intc_data = {
 	.num_system_events = 160,
 	.num_host_events = 20,
+	.quirky_events = BIT_ULL(7) | BIT_ULL(56), /* IEP{0,1} capture/compare events */
 };
 
 static const struct of_device_id pruss_intc_of_match[] = {
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/3] irqchip/irq-pruss-intc: Fix processing of IEP interrupts
  2023-09-19  6:19 ` [PATCH 3/3] irqchip/irq-pruss-intc: Fix processing of IEP interrupts MD Danish Anwar
@ 2023-09-19  7:32   ` Marc Zyngier
  2026-02-18  7:08     ` Meghana Malladi
  0 siblings, 1 reply; 9+ messages in thread
From: Marc Zyngier @ 2023-09-19  7:32 UTC (permalink / raw)
  To: MD Danish Anwar
  Cc: Grzegorz Jaszczyk, Suman Anna, David Lechner, Roger Quadros,
	Andrew F. Davis, Thomas Gleixner, linux-kernel, srk,
	r-gunasekaran, vigneshr

On Tue, 19 Sep 2023 07:19:00 +0100,
MD Danish Anwar <danishanwar@ti.com> wrote:
> 
> From: Suman Anna <s-anna@ti.com>
> 
> It was discovered that IEP capture/compare IRQs (event #7 on all SoCs
> and event #56 on K3 SoCs) are always triggered twice when PPS is
> generated and CMP hit event detected by IEP.
> 
> An example of the problem is:
>   pruss_intc_irq_handler
>    generic_handle_irq
>     handle_level_irq
>       mask_ack_irq -> IRQ 7 masked and asked in INTC,
>                       but it's not yet cleared on HW level
>       handle_irq_event()
>         <threaded on RT>
>            icss_iep_cap_cmp_handler() -> IRQ 7 is actually processed in HW
>         irq_finalize_oneshot()
>          unmask_irq()
>            pruss_intc_irq_unmask() -> IRQ 7 status is still observed as set
> 
> The solution is to actually ack these IRQs from pruss_intc_irq_unmask()
> after the IRQ source is cleared in HW.

What you don't explain is whether the interrupt is level or edge
triggered? If it is level, then the "quirk" is that the interrupt
controller is slow to recognise that the level has changed. If it is
edge, this is a guaranteed recipe to lose interrupts.

Even if it is level, what happens if you issue a mask/unmask sequence
outside of the interrupt handling and that such an interrupt becomes
pending in between? Does this spurious ack have an effect on the now
pending interrupt?

> 
> No public errata available for this yet.
> 
> Fixes: 04e2d1e06978 ("irqchip/irq-pruss-intc: Add a PRUSS irqchip driver for PRUSS interrupts")
> 
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>

Nit: drop the empty line after Fixes:.

> Signed-off-by: Suman Anna <s-anna@ti.com>
> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
> ---
>  drivers/irqchip/irq-pruss-intc.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/irqchip/irq-pruss-intc.c b/drivers/irqchip/irq-pruss-intc.c
> index 3cf684ede564..9907847dbda8 100644
> --- a/drivers/irqchip/irq-pruss-intc.c
> +++ b/drivers/irqchip/irq-pruss-intc.c
> @@ -70,6 +70,8 @@
>  #define MAX_PRU_SYS_EVENTS 160
>  #define MAX_PRU_CHANNELS 20
>  
> +#define MAX_PRU_INT_EVENTS	64
> +
>  /**
>   * struct pruss_intc_map_record - keeps track of actual mapping state
>   * @value: The currently mapped value (channel or host)
> @@ -85,10 +87,13 @@ struct pruss_intc_map_record {
>   * @num_system_events: number of input system events handled by the PRUSS INTC
>   * @num_host_events: number of host events (which is equal to number of
>   *		     channels) supported by the PRUSS INTC
> + * @quirky_events: bitmask of events that need quirky IRQ handling (limited to
> + *		   (internal sources only for now, so 64 bits suffice)
>   */
>  struct pruss_intc_match_data {
>  	u8 num_system_events;
>  	u8 num_host_events;
> +	u64 quirky_events;

Why limit this to the first 64 interrupts, while the intc can deal
with 160? What makes you confident that this is solely limited to this
particular source? And why this source only?

	M.

-- 
Without deviation from the norm, progress is not possible.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/3] irqchip/irq-pruss-intc: Fix processing of IEP interrupts
  2023-09-19  7:32   ` Marc Zyngier
@ 2026-02-18  7:08     ` Meghana Malladi
  2026-02-18 11:04       ` Marc Zyngier
  0 siblings, 1 reply; 9+ messages in thread
From: Meghana Malladi @ 2026-02-18  7:08 UTC (permalink / raw)
  To: Marc Zyngier, MD Danish Anwar
  Cc: Grzegorz Jaszczyk, Suman Anna, David Lechner, Roger Quadros,
	Andrew F. Davis, Thomas Gleixner, linux-kernel, srk,
	r-gunasekaran, vigneshr

Hi Marc,

On 9/19/23 13:02, Marc Zyngier wrote:
> On Tue, 19 Sep 2023 07:19:00 +0100,
> MD Danish Anwar <danishanwar@ti.com> wrote:
>>
>> From: Suman Anna <s-anna@ti.com>
>>
>> It was discovered that IEP capture/compare IRQs (event #7 on all SoCs
>> and event #56 on K3 SoCs) are always triggered twice when PPS is
>> generated and CMP hit event detected by IEP.
>>
>> An example of the problem is:
>>    pruss_intc_irq_handler
>>     generic_handle_irq
>>      handle_level_irq
>>        mask_ack_irq -> IRQ 7 masked and asked in INTC,
>>                        but it's not yet cleared on HW level
>>        handle_irq_event()
>>          <threaded on RT>
>>             icss_iep_cap_cmp_handler() -> IRQ 7 is actually processed in HW
>>          irq_finalize_oneshot()
>>           unmask_irq()
>>             pruss_intc_irq_unmask() -> IRQ 7 status is still observed as set
>>
>> The solution is to actually ack these IRQs from pruss_intc_irq_unmask()
>> after the IRQ source is cleared in HW.
> 
> What you don't explain is whether the interrupt is level or edge
> triggered? If it is level, then the "quirk" is that the interrupt
> controller is slow to recognise that the level has changed. If it is
> edge, this is a guaranteed recipe to lose interrupts.
> 

These are level IRQs, but INTC has latency detecting the source 
deassertion, causing the double delivery (The hardware keeps the event 
asserted until cleared in IEP) - I will add more details about this fix 
in the commit message for v2.

> Even if it is level, what happens if you issue a mask/unmask sequence
> outside of the interrupt handling and that such an interrupt becomes
> pending in between? Does this spurious ack have an effect on the now
> pending interrupt?
> 

This ack will not result in any event loss outside IRQ context because,
even if a new event arrives between mask/unmask, it is not lost - as the 
level stays asserted until the driver's IEP handler clears it.

>>
>> No public errata available for this yet.
>>
>> Fixes: 04e2d1e06978 ("irqchip/irq-pruss-intc: Add a PRUSS irqchip driver for PRUSS interrupts")
>>
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> 
> Nit: drop the empty line after Fixes:.
> 
>> Signed-off-by: Suman Anna <s-anna@ti.com>
>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
>> ---
>>   drivers/irqchip/irq-pruss-intc.c | 14 ++++++++++++++
>>   1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/irqchip/irq-pruss-intc.c b/drivers/irqchip/irq-pruss-intc.c
>> index 3cf684ede564..9907847dbda8 100644
>> --- a/drivers/irqchip/irq-pruss-intc.c
>> +++ b/drivers/irqchip/irq-pruss-intc.c
>> @@ -70,6 +70,8 @@
>>   #define MAX_PRU_SYS_EVENTS 160
>>   #define MAX_PRU_CHANNELS 20
>>   
>> +#define MAX_PRU_INT_EVENTS	64
>> +
>>   /**
>>    * struct pruss_intc_map_record - keeps track of actual mapping state
>>    * @value: The currently mapped value (channel or host)
>> @@ -85,10 +87,13 @@ struct pruss_intc_map_record {
>>    * @num_system_events: number of input system events handled by the PRUSS INTC
>>    * @num_host_events: number of host events (which is equal to number of
>>    *		     channels) supported by the PRUSS INTC
>> + * @quirky_events: bitmask of events that need quirky IRQ handling (limited to
>> + *		   (internal sources only for now, so 64 bits suffice)
>>    */
>>   struct pruss_intc_match_data {
>>   	u8 num_system_events;
>>   	u8 num_host_events;
>> +	u64 quirky_events;
> 
> Why limit this to the first 64 interrupts, while the intc can deal
> with 160? What makes you confident that this is solely limited to this
> particular source? And why this source only?
>

This quirk only applies to internal IEP-generated events (IEP0 CMP/CAP & 
IEP1 CMP/CAP) and there are no known external events with this behavior. 
If future SoCs need quirks beyond 64, the field can be widened, as there 
is no known case today.


> 	M.
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/3] irqchip/irq-pruss-intc: Fix processing of IEP interrupts
  2026-02-18  7:08     ` Meghana Malladi
@ 2026-02-18 11:04       ` Marc Zyngier
  2026-02-18 11:26         ` Meghana Malladi
  0 siblings, 1 reply; 9+ messages in thread
From: Marc Zyngier @ 2026-02-18 11:04 UTC (permalink / raw)
  To: Meghana Malladi
  Cc: MD Danish Anwar, Grzegorz Jaszczyk, Suman Anna, David Lechner,
	Roger Quadros, Andrew F. Davis, Thomas Gleixner, linux-kernel,
	srk, r-gunasekaran, vigneshr

On Wed, 18 Feb 2026 07:08:45 +0000,
Meghana Malladi <m-malladi@ti.com> wrote:
> 
> Hi Marc,
> 
> On 9/19/23 13:02, Marc Zyngier wrote:
> > On Tue, 19 Sep 2023 07:19:00 +0100,
> > MD Danish Anwar <danishanwar@ti.com> wrote:
> >> 
> >> From: Suman Anna <s-anna@ti.com>
> >> 
> >> It was discovered that IEP capture/compare IRQs (event #7 on all SoCs
> >> and event #56 on K3 SoCs) are always triggered twice when PPS is
> >> generated and CMP hit event detected by IEP.
> >> 
> >> An example of the problem is:
> >>    pruss_intc_irq_handler
> >>     generic_handle_irq
> >>      handle_level_irq
> >>        mask_ack_irq -> IRQ 7 masked and asked in INTC,
> >>                        but it's not yet cleared on HW level
> >>        handle_irq_event()
> >>          <threaded on RT>
> >>             icss_iep_cap_cmp_handler() -> IRQ 7 is actually processed in HW
> >>          irq_finalize_oneshot()
> >>           unmask_irq()
> >>             pruss_intc_irq_unmask() -> IRQ 7 status is still observed as set
> >> 
> >> The solution is to actually ack these IRQs from pruss_intc_irq_unmask()
> >> after the IRQ source is cleared in HW.
> > 
> > What you don't explain is whether the interrupt is level or edge
> > triggered? If it is level, then the "quirk" is that the interrupt
> > controller is slow to recognise that the level has changed. If it is
> > edge, this is a guaranteed recipe to lose interrupts.
> > 
> 
> These are level IRQs, but INTC has latency detecting the source
> deassertion, causing the double delivery (The hardware keeps the event
> asserted until cleared in IEP) - I will add more details about this
> fix in the commit message for v2.

I'm sorry, but this comes almost 3 years late, and I've long forgotten
about all of this. I can only conclude that if you (TI, not you
personally) waited for this long to address my comments, then this is
probably not something we really need to care about.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/3] irqchip/irq-pruss-intc: Fix processing of IEP interrupts
  2026-02-18 11:04       ` Marc Zyngier
@ 2026-02-18 11:26         ` Meghana Malladi
  2026-02-22 20:59           ` Thomas Gleixner
  0 siblings, 1 reply; 9+ messages in thread
From: Meghana Malladi @ 2026-02-18 11:26 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: MD Danish Anwar, Grzegorz Jaszczyk, Suman Anna, David Lechner,
	Roger Quadros, Andrew F. Davis, Thomas Gleixner, linux-kernel,
	srk, r-gunasekaran, vigneshr

Hi Marc,

On 2/18/26 16:34, Marc Zyngier wrote:
> On Wed, 18 Feb 2026 07:08:45 +0000,
> Meghana Malladi <m-malladi@ti.com> wrote:
>>
>> Hi Marc,
>>
>> On 9/19/23 13:02, Marc Zyngier wrote:
>>> On Tue, 19 Sep 2023 07:19:00 +0100,
>>> MD Danish Anwar <danishanwar@ti.com> wrote:
>>>>
>>>> From: Suman Anna <s-anna@ti.com>
>>>>
>>>> It was discovered that IEP capture/compare IRQs (event #7 on all SoCs
>>>> and event #56 on K3 SoCs) are always triggered twice when PPS is
>>>> generated and CMP hit event detected by IEP.
>>>>
>>>> An example of the problem is:
>>>>     pruss_intc_irq_handler
>>>>      generic_handle_irq
>>>>       handle_level_irq
>>>>         mask_ack_irq -> IRQ 7 masked and asked in INTC,
>>>>                         but it's not yet cleared on HW level
>>>>         handle_irq_event()
>>>>           <threaded on RT>
>>>>              icss_iep_cap_cmp_handler() -> IRQ 7 is actually processed in HW
>>>>           irq_finalize_oneshot()
>>>>            unmask_irq()
>>>>              pruss_intc_irq_unmask() -> IRQ 7 status is still observed as set
>>>>
>>>> The solution is to actually ack these IRQs from pruss_intc_irq_unmask()
>>>> after the IRQ source is cleared in HW.
>>>
>>> What you don't explain is whether the interrupt is level or edge
>>> triggered? If it is level, then the "quirk" is that the interrupt
>>> controller is slow to recognise that the level has changed. If it is
>>> edge, this is a guaranteed recipe to lose interrupts.
>>>
>>
>> These are level IRQs, but INTC has latency detecting the source
>> deassertion, causing the double delivery (The hardware keeps the event
>> asserted until cleared in IEP) - I will add more details about this
>> fix in the commit message for v2.
> 
> I'm sorry, but this comes almost 3 years late, and I've long forgotten
> about all of this. I can only conclude that if you (TI, not you
> personally) waited for this long to address my comments, then this is
> probably not something we really need to care about.
> 

Thanks for your reply. I understand the concern about the delay and the 
difficulty in revisiting something after such a long gap.

This work resurfaced now because we are in the process of migrating to 
the 6.18 LTS kernel, and while validating the migration we encountered 
these issues again. For our platforms, these fixes are required to get 
PPS and timestamp functionality working correctly, and for ensuring 
long-term stability.

I realize this comes a long time after the original discussion, but I 
would like to get the patches into the proper shape and aligned with 
your feedback. Thanks again for your time and guidance.

Best,
Meghana


> Thanks,
> 
> 	M.
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/3] irqchip/irq-pruss-intc: Fix processing of IEP interrupts
  2026-02-18 11:26         ` Meghana Malladi
@ 2026-02-22 20:59           ` Thomas Gleixner
  0 siblings, 0 replies; 9+ messages in thread
From: Thomas Gleixner @ 2026-02-22 20:59 UTC (permalink / raw)
  To: Meghana Malladi, Marc Zyngier
  Cc: MD Danish Anwar, Grzegorz Jaszczyk, Suman Anna, David Lechner,
	Roger Quadros, Andrew F. Davis, linux-kernel, srk, r-gunasekaran,
	vigneshr

On Wed, Feb 18 2026 at 16:56, Meghana Malladi wrote:
> On 2/18/26 16:34, Marc Zyngier wrote:
>> I'm sorry, but this comes almost 3 years late, and I've long forgotten
>> about all of this. I can only conclude that if you (TI, not you
>> personally) waited for this long to address my comments, then this is
>> probably not something we really need to care about.
>> 
>
> Thanks for your reply. I understand the concern about the delay and the 
> difficulty in revisiting something after such a long gap.
>
> This work resurfaced now because we are in the process of migrating to 
> the 6.18 LTS kernel, and while validating the migration we encountered 
> these issues again. For our platforms, these fixes are required to get 
> PPS and timestamp functionality working correctly, and for ensuring 
> long-term stability.

For ensuring long-term stability you should work with the upstream
community when changes happen and not wait until you finally catch up
with reality in production.

That's not the way it works. If you ignore feedback for 3 years then you
can't expect that everything is dropped to please you now.

> I realize this comes a long time after the original discussion, but I 
> would like to get the patches into the proper shape and aligned with 
> your feedback. Thanks again for your time and guidance.

Marc told you what he's concerned about and that should be sufficient to
resubmit the patches with proper change logs and comments in the source
code explaining the issue and the solution properly, no?

While at it please fix up the broken S-O-B chain in your submission.
Documentation/process/ explains it neatly. While at it read the rest of
it too and act accordingly.

Thanks,

        tglx

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-02-22 20:59 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-19  6:18 [PATCH 0/3] Bug Fixes for PRUSS irqchip driver MD Danish Anwar
2023-09-19  6:18 ` [PATCH 1/3] irqchip/irq-pruss-intc: Fix enabling of intc events MD Danish Anwar
2023-09-19  6:18 ` [PATCH 2/3] irqchip/irq-pruss-intc: Fix listed IRQ type in /proc/interrupts MD Danish Anwar
2023-09-19  6:19 ` [PATCH 3/3] irqchip/irq-pruss-intc: Fix processing of IEP interrupts MD Danish Anwar
2023-09-19  7:32   ` Marc Zyngier
2026-02-18  7:08     ` Meghana Malladi
2026-02-18 11:04       ` Marc Zyngier
2026-02-18 11:26         ` Meghana Malladi
2026-02-22 20:59           ` Thomas Gleixner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox