diff mbox

[07/15] irqdomain: Don't set type when mapping an IRQ

Message ID 1458224359-32665-8-git-send-email-jonathanh@nvidia.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jon Hunter March 17, 2016, 2:19 p.m. UTC
Some IRQ chips, such as GPIO controllers or secondary level interrupt
controllers, may require require additional runtime power management
control to ensure they are accessible. For such IRQ chips, it makes sense
to enable the IRQ chip when interrupts are requested and disabled them
again once all interrupts have been freed.

When mapping an IRQ, the IRQ type settings are read and then programmed.
The mapping of the IRQ happens before the IRQ is requested and so the
programming of the type settings occurs before the IRQ is requested. This
is a problem for IRQ chips that require additional power management
control because they may not be accessible yet. Therefore, when mapping
the IRQ, don't program the type settings, just save them and then program
these saved settings when the IRQ is requested (so long as if they are not
overridden via the call to request the IRQ).

Add a stub function for irq_domain_free_irqs() to avoid any compilation
errors when CONFIG_IRQ_DOMAIN_HIERARCHY is not selected.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 include/linux/irqdomain.h |  3 +++
 kernel/irq/irqdomain.c    | 18 ++++++++++++++----
 kernel/irq/manage.c       |  7 +++++++
 3 files changed, 24 insertions(+), 4 deletions(-)

Comments

Thomas Gleixner March 17, 2016, 2:55 p.m. UTC | #1
On Thu, 17 Mar 2016, Jon Hunter wrote:
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -1117,6 +1117,13 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new)
>  	new->irq = irq;
>  
>  	/*
> +	 * If the trigger type is not specified by the caller,
> +	 * then use the default for this interrupt.
> +	 */
> +	if (!(new->flags & IRQF_TRIGGER_MASK))
> +		new->flags |= irqd_get_trigger_type(&desc->irq_data);
> +

This change is independent of the irqdomain part and should be in a seperate
patch because it affects all callers.

Thanks,

	tglx


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jon Hunter March 17, 2016, 3:06 p.m. UTC | #2
On 17/03/16 14:55, Thomas Gleixner wrote:
> On Thu, 17 Mar 2016, Jon Hunter wrote:
>> --- a/kernel/irq/manage.c
>> +++ b/kernel/irq/manage.c
>> @@ -1117,6 +1117,13 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new)
>>  	new->irq = irq;
>>  
>>  	/*
>> +	 * If the trigger type is not specified by the caller,
>> +	 * then use the default for this interrupt.
>> +	 */
>> +	if (!(new->flags & IRQF_TRIGGER_MASK))
>> +		new->flags |= irqd_get_trigger_type(&desc->irq_data);
>> +
> 
> This change is independent of the irqdomain part and should be in a seperate
> patch because it affects all callers.

Ok, I will put this into a separate patch.

Cheers
Jon
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jon Hunter March 17, 2016, 6:19 p.m. UTC | #3
On 17/03/16 14:19, Jon Hunter wrote:
> Some IRQ chips, such as GPIO controllers or secondary level interrupt
> controllers, may require require additional runtime power management
> control to ensure they are accessible. For such IRQ chips, it makes sense
> to enable the IRQ chip when interrupts are requested and disabled them
> again once all interrupts have been freed.
> 
> When mapping an IRQ, the IRQ type settings are read and then programmed.
> The mapping of the IRQ happens before the IRQ is requested and so the
> programming of the type settings occurs before the IRQ is requested. This
> is a problem for IRQ chips that require additional power management
> control because they may not be accessible yet. Therefore, when mapping
> the IRQ, don't program the type settings, just save them and then program
> these saved settings when the IRQ is requested (so long as if they are not
> overridden via the call to request the IRQ).
> 
> Add a stub function for irq_domain_free_irqs() to avoid any compilation
> errors when CONFIG_IRQ_DOMAIN_HIERARCHY is not selected.
> 
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
>  include/linux/irqdomain.h |  3 +++
>  kernel/irq/irqdomain.c    | 18 ++++++++++++++----
>  kernel/irq/manage.c       |  7 +++++++
>  3 files changed, 24 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
> index 2aed04396210..fc66876d1965 100644
> --- a/include/linux/irqdomain.h
> +++ b/include/linux/irqdomain.h
> @@ -440,6 +440,9 @@ static inline int irq_domain_alloc_irqs(struct irq_domain *domain,
>  	return -1;
>  }
>  
> +static inline void irq_domain_free_irqs(unsigned int virq,
> +					unsigned int nr_irqs) { }
> +
>  static inline bool irq_domain_is_hierarchy(struct irq_domain *domain)
>  {
>  	return false;
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index 0ea285baa619..332bee2205e0 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -576,6 +576,7 @@ static void of_phandle_args_to_fwspec(struct of_phandle_args *irq_data,
>  unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec)
>  {
>  	struct irq_domain *domain;
> +	struct irq_data *irq_data;
>  	irq_hw_number_t hwirq;
>  	unsigned int cur_type = IRQ_TYPE_NONE;
>  	unsigned int type = IRQ_TYPE_NONE;
> @@ -638,10 +639,19 @@ unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec)
>  		return 0;
>  	}
>  
> -	/* Set type if specified and different than the current one */
> -	if (type != IRQ_TYPE_NONE &&
> -	    type != irq_get_trigger_type(virq))
> -		irq_set_irq_type(virq, type);
> +	irq_data = irq_get_irq_data(virq);
> +	if (!irq_data) {
> +		if (irq_domain_is_hierarchy(domain))
> +			irq_domain_free_irqs(virq, 1);
> +		else
> +			irq_dispose_mapping(virq);
> +
> +		return 0;
> +	}

I should only free/dispose here if we actually created it! Will fix this
as well :-(

Jon
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index 2aed04396210..fc66876d1965 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -440,6 +440,9 @@  static inline int irq_domain_alloc_irqs(struct irq_domain *domain,
 	return -1;
 }
 
+static inline void irq_domain_free_irqs(unsigned int virq,
+					unsigned int nr_irqs) { }
+
 static inline bool irq_domain_is_hierarchy(struct irq_domain *domain)
 {
 	return false;
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 0ea285baa619..332bee2205e0 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -576,6 +576,7 @@  static void of_phandle_args_to_fwspec(struct of_phandle_args *irq_data,
 unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec)
 {
 	struct irq_domain *domain;
+	struct irq_data *irq_data;
 	irq_hw_number_t hwirq;
 	unsigned int cur_type = IRQ_TYPE_NONE;
 	unsigned int type = IRQ_TYPE_NONE;
@@ -638,10 +639,19 @@  unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec)
 		return 0;
 	}
 
-	/* Set type if specified and different than the current one */
-	if (type != IRQ_TYPE_NONE &&
-	    type != irq_get_trigger_type(virq))
-		irq_set_irq_type(virq, type);
+	irq_data = irq_get_irq_data(virq);
+	if (!irq_data) {
+		if (irq_domain_is_hierarchy(domain))
+			irq_domain_free_irqs(virq, 1);
+		else
+			irq_dispose_mapping(virq);
+
+		return 0;
+	}
+
+	/* Store trigger type */
+	irqd_set_trigger_type(irq_data, type);
+
 	return virq;
 }
 EXPORT_SYMBOL_GPL(irq_create_fwspec_mapping);
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index cc1cc641d653..b2a93a37f772 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -1117,6 +1117,13 @@  __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new)
 	new->irq = irq;
 
 	/*
+	 * If the trigger type is not specified by the caller,
+	 * then use the default for this interrupt.
+	 */
+	if (!(new->flags & IRQF_TRIGGER_MASK))
+		new->flags |= irqd_get_trigger_type(&desc->irq_data);
+
+	/*
 	 * Check whether the interrupt nests into another interrupt
 	 * thread.
 	 */