diff mbox

[v2,5/7] acpi: gsi: Add acpi_set_irq_model to initialize the GSI layer

Message ID 1437656713-9677-6-git-send-email-marc.zyngier@arm.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Marc Zyngier July 23, 2015, 1:05 p.m. UTC
In order to start embrassing irqdomains at the GSI level, introduce
a new initializer:

void acpi_set_irq_model(enum acpi_irq_model_id model,
			unsigned long domain_token,
			int (*populate)(struct acpi_gsi_descriptor *,
					u32, unsigned int));

where:
- model is the value assigned to acpi_irq_model
- domain_token is the identifier for the irqdomain mapping
  GSI interrupts
- populate is a function provided by the interrupt controller,
  populating a struct acpi_gsi_descriptor based on a GSI and
  the interrupt trigger information

As nobody calls this code yet, the current code is left in place.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/acpi/gsi.c   | 32 ++++++++++++++++++++++++++++++++
 include/linux/acpi.h |  5 +++++
 2 files changed, 37 insertions(+)

Comments

Hanjun Guo July 24, 2015, 10 a.m. UTC | #1
On 07/23/2015 09:05 PM, Marc Zyngier wrote:
> In order to start embrassing irqdomains at the GSI level, introduce
> a new initializer:
>
> void acpi_set_irq_model(enum acpi_irq_model_id model,
> 			unsigned long domain_token,
> 			int (*populate)(struct acpi_gsi_descriptor *,
> 					u32, unsigned int));
>
> where:
> - model is the value assigned to acpi_irq_model
> - domain_token is the identifier for the irqdomain mapping
>    GSI interrupts
> - populate is a function provided by the interrupt controller,
>    populating a struct acpi_gsi_descriptor based on a GSI and
>    the interrupt trigger information
>
> As nobody calls this code yet, the current code is left in place.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>   drivers/acpi/gsi.c   | 32 ++++++++++++++++++++++++++++++++
>   include/linux/acpi.h |  5 +++++
>   2 files changed, 37 insertions(+)
>
> diff --git a/drivers/acpi/gsi.c b/drivers/acpi/gsi.c
> index a745880..e6f4a0c 100644
> --- a/drivers/acpi/gsi.c
> +++ b/drivers/acpi/gsi.c
> @@ -16,6 +16,9 @@ enum acpi_irq_model_id acpi_irq_model;
>
>   static void *acpi_gsi_domain_token;
>
> +static int (*acpi_gsi_descriptor_populate)(struct acpi_gsi_descriptor *data,
> +					   u32 gsi, unsigned int irq_type);
> +
>   static unsigned int acpi_gsi_get_irq_type(int trigger, int polarity)
>   {
>   	switch (polarity) {
> @@ -71,10 +74,20 @@ EXPORT_SYMBOL_GPL(acpi_gsi_to_irq);
>   int acpi_register_gsi(struct device *dev, u32 gsi, int trigger,
>   		      int polarity)
>   {
> +	int err;
>   	unsigned int irq;
>   	unsigned int irq_type = acpi_gsi_get_irq_type(trigger, polarity);
>   	struct irq_domain *d = irq_find_host(acpi_gsi_domain_token);
>
> +	if (acpi_gsi_descriptor_populate) {
> +		struct acpi_gsi_descriptor data;
> +		err = acpi_gsi_descriptor_populate(&data, gsi, irq_type);
> +		if (err)
> +			return err;
> +
> +		return irq_create_acpi_mapping(d, &data);
> +	}
> +
>   	irq = irq_create_mapping(d, gsi);
>   	if (!irq)
>   		return -EINVAL;
> @@ -99,3 +112,22 @@ void acpi_unregister_gsi(u32 gsi)
>   	irq_dispose_mapping(irq);
>   }
>   EXPORT_SYMBOL_GPL(acpi_unregister_gsi);
> +
> +/**
> + * acpi_set_irq_model - Setup the GSI irqdomain information
> + * @model: the value assigned to acpi_irq_model
> + * @domain_token: the irq_domain identifier for mapping and looking up
> + *                GSI interrupts
> + * @populate: provided by the interrupt controller, populating a
> + *            struct acpi_gsi_descriptor based on a GSI and
> + *            the interrupt trigger information
> + */
> +void acpi_set_irq_model(enum acpi_irq_model_id model,
> +			unsigned long domain_token,
> +			int (*populate)(struct acpi_gsi_descriptor *,
> +					u32, unsigned int))
> +{
> +	acpi_irq_model = model;
> +	acpi_gsi_domain_token = (void *)domain_token;

This will work perfectly fine with ARM as it has single GICD and
one irq domain, I'm not sure x86 is ok with it because x86 has
multi ioapics and each ioapic has a irqdomain if I understand
correctly, but x86 don't use functions in this file at all for now,
I think I'm over worried :)

Thanks
Hanjun
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marc Zyngier July 24, 2015, 12:27 p.m. UTC | #2
On 24/07/15 11:00, Hanjun Guo wrote:
> On 07/23/2015 09:05 PM, Marc Zyngier wrote:
>> In order to start embrassing irqdomains at the GSI level, introduce
>> a new initializer:
>>
>> void acpi_set_irq_model(enum acpi_irq_model_id model,
>> 			unsigned long domain_token,
>> 			int (*populate)(struct acpi_gsi_descriptor *,
>> 					u32, unsigned int));
>>
>> where:
>> - model is the value assigned to acpi_irq_model
>> - domain_token is the identifier for the irqdomain mapping
>>    GSI interrupts
>> - populate is a function provided by the interrupt controller,
>>    populating a struct acpi_gsi_descriptor based on a GSI and
>>    the interrupt trigger information
>>
>> As nobody calls this code yet, the current code is left in place.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>   drivers/acpi/gsi.c   | 32 ++++++++++++++++++++++++++++++++
>>   include/linux/acpi.h |  5 +++++
>>   2 files changed, 37 insertions(+)
>>
>> diff --git a/drivers/acpi/gsi.c b/drivers/acpi/gsi.c
>> index a745880..e6f4a0c 100644
>> --- a/drivers/acpi/gsi.c
>> +++ b/drivers/acpi/gsi.c
>> @@ -16,6 +16,9 @@ enum acpi_irq_model_id acpi_irq_model;
>>
>>   static void *acpi_gsi_domain_token;
>>
>> +static int (*acpi_gsi_descriptor_populate)(struct acpi_gsi_descriptor *data,
>> +					   u32 gsi, unsigned int irq_type);
>> +
>>   static unsigned int acpi_gsi_get_irq_type(int trigger, int polarity)
>>   {
>>   	switch (polarity) {
>> @@ -71,10 +74,20 @@ EXPORT_SYMBOL_GPL(acpi_gsi_to_irq);
>>   int acpi_register_gsi(struct device *dev, u32 gsi, int trigger,
>>   		      int polarity)
>>   {
>> +	int err;
>>   	unsigned int irq;
>>   	unsigned int irq_type = acpi_gsi_get_irq_type(trigger, polarity);
>>   	struct irq_domain *d = irq_find_host(acpi_gsi_domain_token);
>>
>> +	if (acpi_gsi_descriptor_populate) {
>> +		struct acpi_gsi_descriptor data;
>> +		err = acpi_gsi_descriptor_populate(&data, gsi, irq_type);
>> +		if (err)
>> +			return err;
>> +
>> +		return irq_create_acpi_mapping(d, &data);
>> +	}
>> +
>>   	irq = irq_create_mapping(d, gsi);
>>   	if (!irq)
>>   		return -EINVAL;
>> @@ -99,3 +112,22 @@ void acpi_unregister_gsi(u32 gsi)
>>   	irq_dispose_mapping(irq);
>>   }
>>   EXPORT_SYMBOL_GPL(acpi_unregister_gsi);
>> +
>> +/**
>> + * acpi_set_irq_model - Setup the GSI irqdomain information
>> + * @model: the value assigned to acpi_irq_model
>> + * @domain_token: the irq_domain identifier for mapping and looking up
>> + *                GSI interrupts
>> + * @populate: provided by the interrupt controller, populating a
>> + *            struct acpi_gsi_descriptor based on a GSI and
>> + *            the interrupt trigger information
>> + */
>> +void acpi_set_irq_model(enum acpi_irq_model_id model,
>> +			unsigned long domain_token,
>> +			int (*populate)(struct acpi_gsi_descriptor *,
>> +					u32, unsigned int))
>> +{
>> +	acpi_irq_model = model;
>> +	acpi_gsi_domain_token = (void *)domain_token;
> 
> This will work perfectly fine with ARM as it has single GICD and
> one irq domain, I'm not sure x86 is ok with it because x86 has
> multi ioapics and each ioapic has a irqdomain if I understand
> correctly, but x86 don't use functions in this file at all for now,
> I think I'm over worried :)

When (and if) that day comes, we'll have to either contextualize
acpi_register_gsi to take a domain token, or have a lookup table that
maps GSI ranges to domain tokens.

Either way, this doesn't invalidate what is being done here.

Thanks,

	M.
Lorenzo Pieralisi July 24, 2015, 5:32 p.m. UTC | #3
On Thu, Jul 23, 2015 at 02:05:11PM +0100, Marc Zyngier wrote:
> In order to start embrassing irqdomains at the GSI level, introduce

s/embrassing/embracing

[...]

> +
> +/**
> + * acpi_set_irq_model - Setup the GSI irqdomain information
> + * @model: the value assigned to acpi_irq_model
> + * @domain_token: the irq_domain identifier for mapping and looking up
> + *                GSI interrupts
> + * @populate: provided by the interrupt controller, populating a
> + *            struct acpi_gsi_descriptor based on a GSI and
> + *            the interrupt trigger information
> + */
> +void acpi_set_irq_model(enum acpi_irq_model_id model,

You can make it __init

> +			unsigned long domain_token,
> +			int (*populate)(struct acpi_gsi_descriptor *,
> +					u32, unsigned int))
> +{
> +	acpi_irq_model = model;
> +	acpi_gsi_domain_token = (void *)domain_token;
> +	acpi_gsi_descriptor_populate = populate;


> +}
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 2f23ab0..0820cb1 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -214,6 +214,11 @@ struct acpi_gsi_descriptor {
>  unsigned int irq_create_acpi_mapping(struct irq_domain *d,
>  				     struct acpi_gsi_descriptor *irq_data);
>  
> +void acpi_set_irq_model(enum acpi_irq_model_id model,
> +			unsigned long domain_token,
> +			int (*populate)(struct acpi_gsi_descriptor *,
> +					u32, unsigned int));
> +
>  #ifdef CONFIG_X86_IO_APIC
>  extern int acpi_get_override_irq(u32 gsi, int *trigger, int *polarity);
>  #else
> 

It makes sense and it removes GIC specific mapping from generic code:

Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hanjun Guo July 25, 2015, 6:57 a.m. UTC | #4
On 07/24/2015 08:27 PM, Marc Zyngier wrote:
> On 24/07/15 11:00, Hanjun Guo wrote:
>> On 07/23/2015 09:05 PM, Marc Zyngier wrote:
>>> In order to start embrassing irqdomains at the GSI level, introduce
>>> a new initializer:
>>>
>>> void acpi_set_irq_model(enum acpi_irq_model_id model,
>>> 			unsigned long domain_token,
>>> 			int (*populate)(struct acpi_gsi_descriptor *,
>>> 					u32, unsigned int));
>>>
>>> where:
>>> - model is the value assigned to acpi_irq_model
>>> - domain_token is the identifier for the irqdomain mapping
>>>     GSI interrupts
>>> - populate is a function provided by the interrupt controller,
>>>     populating a struct acpi_gsi_descriptor based on a GSI and
>>>     the interrupt trigger information
>>>
>>> As nobody calls this code yet, the current code is left in place.
>>>
>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>>> ---
>>>    drivers/acpi/gsi.c   | 32 ++++++++++++++++++++++++++++++++
>>>    include/linux/acpi.h |  5 +++++
>>>    2 files changed, 37 insertions(+)
>>>
>>> diff --git a/drivers/acpi/gsi.c b/drivers/acpi/gsi.c
>>> index a745880..e6f4a0c 100644
>>> --- a/drivers/acpi/gsi.c
>>> +++ b/drivers/acpi/gsi.c
>>> @@ -16,6 +16,9 @@ enum acpi_irq_model_id acpi_irq_model;
>>>
>>>    static void *acpi_gsi_domain_token;
>>>
>>> +static int (*acpi_gsi_descriptor_populate)(struct acpi_gsi_descriptor *data,
>>> +					   u32 gsi, unsigned int irq_type);
>>> +
>>>    static unsigned int acpi_gsi_get_irq_type(int trigger, int polarity)
>>>    {
>>>    	switch (polarity) {
>>> @@ -71,10 +74,20 @@ EXPORT_SYMBOL_GPL(acpi_gsi_to_irq);
>>>    int acpi_register_gsi(struct device *dev, u32 gsi, int trigger,
>>>    		      int polarity)
>>>    {
>>> +	int err;
>>>    	unsigned int irq;
>>>    	unsigned int irq_type = acpi_gsi_get_irq_type(trigger, polarity);
>>>    	struct irq_domain *d = irq_find_host(acpi_gsi_domain_token);
>>>
>>> +	if (acpi_gsi_descriptor_populate) {
>>> +		struct acpi_gsi_descriptor data;
>>> +		err = acpi_gsi_descriptor_populate(&data, gsi, irq_type);
>>> +		if (err)
>>> +			return err;
>>> +
>>> +		return irq_create_acpi_mapping(d, &data);
>>> +	}
>>> +
>>>    	irq = irq_create_mapping(d, gsi);
>>>    	if (!irq)
>>>    		return -EINVAL;
>>> @@ -99,3 +112,22 @@ void acpi_unregister_gsi(u32 gsi)
>>>    	irq_dispose_mapping(irq);
>>>    }
>>>    EXPORT_SYMBOL_GPL(acpi_unregister_gsi);
>>> +
>>> +/**
>>> + * acpi_set_irq_model - Setup the GSI irqdomain information
>>> + * @model: the value assigned to acpi_irq_model
>>> + * @domain_token: the irq_domain identifier for mapping and looking up
>>> + *                GSI interrupts
>>> + * @populate: provided by the interrupt controller, populating a
>>> + *            struct acpi_gsi_descriptor based on a GSI and
>>> + *            the interrupt trigger information
>>> + */
>>> +void acpi_set_irq_model(enum acpi_irq_model_id model,
>>> +			unsigned long domain_token,
>>> +			int (*populate)(struct acpi_gsi_descriptor *,
>>> +					u32, unsigned int))
>>> +{
>>> +	acpi_irq_model = model;
>>> +	acpi_gsi_domain_token = (void *)domain_token;
>>
>> This will work perfectly fine with ARM as it has single GICD and
>> one irq domain, I'm not sure x86 is ok with it because x86 has
>> multi ioapics and each ioapic has a irqdomain if I understand
>> correctly, but x86 don't use functions in this file at all for now,
>> I think I'm over worried :)
>
> When (and if) that day comes, we'll have to either contextualize
> acpi_register_gsi to take a domain token, or have a lookup table that
> maps GSI ranges to domain tokens.
>
> Either way, this doesn't invalidate what is being done here.

Agreed,

Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>

Thanks
Hanjun

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" 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/drivers/acpi/gsi.c b/drivers/acpi/gsi.c
index a745880..e6f4a0c 100644
--- a/drivers/acpi/gsi.c
+++ b/drivers/acpi/gsi.c
@@ -16,6 +16,9 @@  enum acpi_irq_model_id acpi_irq_model;
 
 static void *acpi_gsi_domain_token;
 
+static int (*acpi_gsi_descriptor_populate)(struct acpi_gsi_descriptor *data,
+					   u32 gsi, unsigned int irq_type);
+
 static unsigned int acpi_gsi_get_irq_type(int trigger, int polarity)
 {
 	switch (polarity) {
@@ -71,10 +74,20 @@  EXPORT_SYMBOL_GPL(acpi_gsi_to_irq);
 int acpi_register_gsi(struct device *dev, u32 gsi, int trigger,
 		      int polarity)
 {
+	int err;
 	unsigned int irq;
 	unsigned int irq_type = acpi_gsi_get_irq_type(trigger, polarity);
 	struct irq_domain *d = irq_find_host(acpi_gsi_domain_token);
 
+	if (acpi_gsi_descriptor_populate) {
+		struct acpi_gsi_descriptor data;
+		err = acpi_gsi_descriptor_populate(&data, gsi, irq_type);
+		if (err)
+			return err;
+
+		return irq_create_acpi_mapping(d, &data);
+	}
+
 	irq = irq_create_mapping(d, gsi);
 	if (!irq)
 		return -EINVAL;
@@ -99,3 +112,22 @@  void acpi_unregister_gsi(u32 gsi)
 	irq_dispose_mapping(irq);
 }
 EXPORT_SYMBOL_GPL(acpi_unregister_gsi);
+
+/**
+ * acpi_set_irq_model - Setup the GSI irqdomain information
+ * @model: the value assigned to acpi_irq_model
+ * @domain_token: the irq_domain identifier for mapping and looking up
+ *                GSI interrupts
+ * @populate: provided by the interrupt controller, populating a
+ *            struct acpi_gsi_descriptor based on a GSI and
+ *            the interrupt trigger information
+ */
+void acpi_set_irq_model(enum acpi_irq_model_id model,
+			unsigned long domain_token,
+			int (*populate)(struct acpi_gsi_descriptor *,
+					u32, unsigned int))
+{
+	acpi_irq_model = model;
+	acpi_gsi_domain_token = (void *)domain_token;
+	acpi_gsi_descriptor_populate = populate;
+}
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 2f23ab0..0820cb1 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -214,6 +214,11 @@  struct acpi_gsi_descriptor {
 unsigned int irq_create_acpi_mapping(struct irq_domain *d,
 				     struct acpi_gsi_descriptor *irq_data);
 
+void acpi_set_irq_model(enum acpi_irq_model_id model,
+			unsigned long domain_token,
+			int (*populate)(struct acpi_gsi_descriptor *,
+					u32, unsigned int));
+
 #ifdef CONFIG_X86_IO_APIC
 extern int acpi_get_override_irq(u32 gsi, int *trigger, int *polarity);
 #else