Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1653198 > unrolled thread

[PATCH V1 08/15] spmi: pmic_arb: use appropriate flow handler

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH V1 08/15] spmi: pmic_arb: use appropriate flow handler Kiran Gunda <kgunda@codeaurora.org> - 2017-05-30 14:50 +0200
    Re: [PATCH V1 08/15] spmi: pmic_arb: use appropriate flow handler Stephen Boyd <sboyd@codeaurora.org> - 2017-05-31 21:10 +0200
      Re: [PATCH V1 08/15] spmi: pmic_arb: use appropriate flow handler kgunda@codeaurora.org - 2017-06-06 13:00 +0200

#1653198 — [PATCH V1 08/15] spmi: pmic_arb: use appropriate flow handler

From: Abhijeet Dharmapurikar <adharmap@codeaurora.org>

The current code uses handle_level_irq flow handler even if the
trigger type of the interrupt is edge. This can lead to missing
of an edge transition that happens when the interrupt is being
handled. The level flow handler masks the interrupt while it is
being handled, so if an edge transition happens at that time,
that edge is lost.

Use an edge flow handler for edge type interrupts which ensures
that the interrupt stays enabled while being handled - at least
until it triggers at which point the flow handler sets the
IRQF_PENDING flag and only then masks the interrupt. That
IRQF_PENDING state indicates an edge transition happened while
the interrupt was being handled and the handler is called again.

Signed-off-by: Abhijeet Dharmapurikar <adharmap@codeaurora.org>
Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
---
 drivers/spmi/spmi-pmic-arb.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
index 1d23df0..ad34491 100644
--- a/drivers/spmi/spmi-pmic-arb.c
+++ b/drivers/spmi/spmi-pmic-arb.c
@@ -625,6 +625,12 @@ static int qpnpint_irq_set_type(struct irq_data *d, unsigned int flow_type)
 	}
 
 	qpnpint_spmi_write(d, QPNPINT_REG_SET_TYPE, &type, sizeof(type));
+
+	if (flow_type & IRQ_TYPE_EDGE_BOTH)
+		irq_set_handler_locked(d, handle_edge_irq);
+	else
+		irq_set_handler_locked(d, handle_level_irq);
+
 	return 0;
 }
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
--

[toc] | [next] | [standalone]

#1654555

On 05/30, Kiran Gunda wrote:
> From: Abhijeet Dharmapurikar <adharmap@codeaurora.org>
> 
> The current code uses handle_level_irq flow handler even if the
> trigger type of the interrupt is edge. This can lead to missing
> of an edge transition that happens when the interrupt is being
> handled. The level flow handler masks the interrupt while it is
> being handled, so if an edge transition happens at that time,
> that edge is lost.
> 
> Use an edge flow handler for edge type interrupts which ensures
> that the interrupt stays enabled while being handled - at least
> until it triggers at which point the flow handler sets the
> IRQF_PENDING flag and only then masks the interrupt. That
> IRQF_PENDING state indicates an edge transition happened while
> the interrupt was being handled and the handler is called again.
> 
> Signed-off-by: Abhijeet Dharmapurikar <adharmap@codeaurora.org>
> Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>

Do we have any edge interrupts in the tree right now? At least
RTC seems to be using edge... This should go back to stable with
a Fixes tag.

> ---
>  drivers/spmi/spmi-pmic-arb.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
> index 1d23df0..ad34491 100644
> --- a/drivers/spmi/spmi-pmic-arb.c
> +++ b/drivers/spmi/spmi-pmic-arb.c
> @@ -625,6 +625,12 @@ static int qpnpint_irq_set_type(struct irq_data *d, unsigned int flow_type)
>  	}
>  
>  	qpnpint_spmi_write(d, QPNPINT_REG_SET_TYPE, &type, sizeof(type));
> +
> +	if (flow_type & IRQ_TYPE_EDGE_BOTH)

IRQ_TYPE_EDGE_BOTH doesn't seem appropriate to use here. We're
really just testing to see if the type is an edge type, not if
it's BOTH edges.

> +		irq_set_handler_locked(d, handle_edge_irq);
> +	else
> +		irq_set_handler_locked(d, handle_level_irq);
> +

And we already have code that does that check:

	if (flow_type & (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING))

so just put the irq_set_handler_locked() calls in those if
statements please.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [prev] | [next] | [standalone]

#1658607

On 2017-06-01 00:33, Stephen Boyd wrote:
> On 05/30, Kiran Gunda wrote:
>> From: Abhijeet Dharmapurikar <adharmap@codeaurora.org>
>> 
>> The current code uses handle_level_irq flow handler even if the
>> trigger type of the interrupt is edge. This can lead to missing
>> of an edge transition that happens when the interrupt is being
>> handled. The level flow handler masks the interrupt while it is
>> being handled, so if an edge transition happens at that time,
>> that edge is lost.
>> 
>> Use an edge flow handler for edge type interrupts which ensures
>> that the interrupt stays enabled while being handled - at least
>> until it triggers at which point the flow handler sets the
>> IRQF_PENDING flag and only then masks the interrupt. That
>> IRQF_PENDING state indicates an edge transition happened while
>> the interrupt was being handled and the handler is called again.
>> 
>> Signed-off-by: Abhijeet Dharmapurikar <adharmap@codeaurora.org>
>> Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
> 
> Do we have any edge interrupts in the tree right now? At least
> RTC seems to be using edge... This should go back to stable with
> a Fixes tag.
> 
Yes. A bunch of internal drivers use edge interrupts.
>> ---
>>  drivers/spmi/spmi-pmic-arb.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>> 
>> diff --git a/drivers/spmi/spmi-pmic-arb.c 
>> b/drivers/spmi/spmi-pmic-arb.c
>> index 1d23df0..ad34491 100644
>> --- a/drivers/spmi/spmi-pmic-arb.c
>> +++ b/drivers/spmi/spmi-pmic-arb.c
>> @@ -625,6 +625,12 @@ static int qpnpint_irq_set_type(struct irq_data 
>> *d, unsigned int flow_type)
>>  	}
>> 
>>  	qpnpint_spmi_write(d, QPNPINT_REG_SET_TYPE, &type, sizeof(type));
>> +
>> +	if (flow_type & IRQ_TYPE_EDGE_BOTH)
> 
> IRQ_TYPE_EDGE_BOTH doesn't seem appropriate to use here. We're
> really just testing to see if the type is an edge type, not if
> it's BOTH edges.
> 
Ok. Agree. Will fix in the follow up patch.
>> +		irq_set_handler_locked(d, handle_edge_irq);
>> +	else
>> +		irq_set_handler_locked(d, handle_level_irq);
>> +
> 
> And we already have code that does that check:
> 
> 	if (flow_type & (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING))
> 
> so just put the irq_set_handler_locked() calls in those if
> statements please.
Sure. Will fix in the follow up patch.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web