diff mbox series

[hyperv-next,v6,10/11] ACPI: irq: Introduce acpi_get_gsi_dispatcher()

Message ID 20250315001931.631210-11-romank@linux.microsoft.com (mailing list archive)
State Handled Elsewhere, archived
Headers show
Series arm64: hyperv: Support Virtual Trust Level Boot | expand

Commit Message

Roman Kisel March 15, 2025, 12:19 a.m. UTC 
Using acpi_irq_create_hierarchy() in the cases where the code
also handles OF leads to code duplication as the ACPI subsystem
doesn't provide means to compute the IRQ domain parent whereas
the OF does.

Introduce acpi_get_gsi_dispatcher() so that the drivers relying
on both ACPI and OF may use irq_domain_create_hierarchy() in the
common code paths.

No functional changes.

Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
---
 drivers/acpi/irq.c   | 15 +++++++++++++--
 include/linux/acpi.h |  5 ++++-
 2 files changed, 17 insertions(+), 3 deletions(-)

Comments

Rafael J. Wysocki March 26, 2025, 2:55 p.m. UTC | #1 
On Sat, Mar 15, 2025 at 1:19 AM Roman Kisel <romank@linux.microsoft.com> wrote:
>
> Using acpi_irq_create_hierarchy() in the cases where the code
> also handles OF leads to code duplication as the ACPI subsystem
> doesn't provide means to compute the IRQ domain parent whereas
> the OF does.
>
> Introduce acpi_get_gsi_dispatcher() so that the drivers relying
> on both ACPI and OF may use irq_domain_create_hierarchy() in the
> common code paths.
>
> No functional changes.
>
> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
> Reviewed-by: Michael Kelley <mhklinux@outlook.com>

This basically looks OK to me except for a couple of coding style
related nits below.

> ---
>  drivers/acpi/irq.c   | 15 +++++++++++++--
>  include/linux/acpi.h |  5 ++++-
>  2 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c
> index 1687483ff319..8eb09e45e5c5 100644
> --- a/drivers/acpi/irq.c
> +++ b/drivers/acpi/irq.c
> @@ -12,7 +12,7 @@
>
>  enum acpi_irq_model_id acpi_irq_model;
>
> -static struct fwnode_handle *(*acpi_get_gsi_domain_id)(u32 gsi);
> +static acpi_gsi_domain_disp_fn acpi_get_gsi_domain_id;
>  static u32 (*acpi_gsi_to_irq_fallback)(u32 gsi);
>
>  /**
> @@ -307,12 +307,23 @@ EXPORT_SYMBOL_GPL(acpi_irq_get);
>   *     for a given GSI
>   */
>  void __init acpi_set_irq_model(enum acpi_irq_model_id model,
> -                              struct fwnode_handle *(*fn)(u32))

Please retain the indentation here and analogously below.

> +       acpi_gsi_domain_disp_fn fn)
>  {
>         acpi_irq_model = model;
>         acpi_get_gsi_domain_id = fn;
>  }
>
> +/**
> + * acpi_get_gsi_dispatcher - Returns dispatcher function that
> + *                           computes the domain fwnode for a
> + *                           given GSI.
> + */

I would format this kerneldoc comment a bit differently:

/*
 * acpi_get_gsi_dispatcher() - Get the GSI dispatcher function
 *
 * Return the dispatcher function that computes the domain fwnode for
a given GSI.
 */

> +acpi_gsi_domain_disp_fn acpi_get_gsi_dispatcher(void)
> +{
> +       return acpi_get_gsi_domain_id;
> +}
> +EXPORT_SYMBOL_GPL(acpi_get_gsi_dispatcher);
> +
>  /**
>   * acpi_set_gsi_to_irq_fallback - Register a GSI transfer
>   * callback to fallback to arch specified implementation.
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 4e495b29c640..abc51288e867 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -336,8 +336,11 @@ int acpi_register_gsi (struct device *dev, u32 gsi, int triggering, int polarity
>  int acpi_gsi_to_irq (u32 gsi, unsigned int *irq);
>  int acpi_isa_irq_to_gsi (unsigned isa_irq, u32 *gsi);
>
> +typedef struct fwnode_handle *(*acpi_gsi_domain_disp_fn)(u32);
> +
>  void acpi_set_irq_model(enum acpi_irq_model_id model,
> -                       struct fwnode_handle *(*)(u32));
> +       acpi_gsi_domain_disp_fn fn);
> +acpi_gsi_domain_disp_fn acpi_get_gsi_dispatcher(void);
>  void acpi_set_gsi_to_irq_fallback(u32 (*)(u32));
>
>  struct irq_domain *acpi_irq_create_hierarchy(unsigned int flags,
> --

With the above addressed, please feel free to add

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

to the patch and route it along with the rest of the series.

Thanks!
Roman Kisel March 26, 2025, 3:32 p.m. UTC | #2 
On 3/26/2025 7:55 AM, Rafael J. Wysocki wrote:
> On Sat, Mar 15, 2025 at 1:19 AM Roman Kisel <romank@linux.microsoft.com> wrote:
[...]
> 
> This basically looks OK to me except for a couple of coding style
> related nits below.
> 

Appreciate taking time to review this very much! Will squash the nits in
the next version.

>> ---
>>   drivers/acpi/irq.c   | 15 +++++++++++++--
>>   include/linux/acpi.h |  5 ++++-
>>   2 files changed, 17 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c
>> index 1687483ff319..8eb09e45e5c5 100644
>> --- a/drivers/acpi/irq.c
>> +++ b/drivers/acpi/irq.c
>> @@ -12,7 +12,7 @@
>>
>>   enum acpi_irq_model_id acpi_irq_model;
>>
>> -static struct fwnode_handle *(*acpi_get_gsi_domain_id)(u32 gsi);
>> +static acpi_gsi_domain_disp_fn acpi_get_gsi_domain_id;
>>   static u32 (*acpi_gsi_to_irq_fallback)(u32 gsi);
>>
>>   /**
>> @@ -307,12 +307,23 @@ EXPORT_SYMBOL_GPL(acpi_irq_get);
>>    *     for a given GSI
>>    */
>>   void __init acpi_set_irq_model(enum acpi_irq_model_id model,
>> -                              struct fwnode_handle *(*fn)(u32))
> 
> Please retain the indentation here and analogously below.
> 
>> +       acpi_gsi_domain_disp_fn fn)
>>   {
>>          acpi_irq_model = model;
>>          acpi_get_gsi_domain_id = fn;
>>   }
>>
>> +/**
>> + * acpi_get_gsi_dispatcher - Returns dispatcher function that
>> + *                           computes the domain fwnode for a
>> + *                           given GSI.
>> + */
> 
> I would format this kerneldoc comment a bit differently:
> 
> /*
>   * acpi_get_gsi_dispatcher() - Get the GSI dispatcher function
>   *
>   * Return the dispatcher function that computes the domain fwnode for
> a given GSI.
>   */
> 
>> +acpi_gsi_domain_disp_fn acpi_get_gsi_dispatcher(void)
>> +{
>> +       return acpi_get_gsi_domain_id;
>> +}
>> +EXPORT_SYMBOL_GPL(acpi_get_gsi_dispatcher);
>> +
>>   /**
>>    * acpi_set_gsi_to_irq_fallback - Register a GSI transfer
>>    * callback to fallback to arch specified implementation.
>> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
>> index 4e495b29c640..abc51288e867 100644
>> --- a/include/linux/acpi.h
>> +++ b/include/linux/acpi.h
>> @@ -336,8 +336,11 @@ int acpi_register_gsi (struct device *dev, u32 gsi, int triggering, int polarity
>>   int acpi_gsi_to_irq (u32 gsi, unsigned int *irq);
>>   int acpi_isa_irq_to_gsi (unsigned isa_irq, u32 *gsi);
>>
>> +typedef struct fwnode_handle *(*acpi_gsi_domain_disp_fn)(u32);
>> +
>>   void acpi_set_irq_model(enum acpi_irq_model_id model,
>> -                       struct fwnode_handle *(*)(u32));
>> +       acpi_gsi_domain_disp_fn fn);
>> +acpi_gsi_domain_disp_fn acpi_get_gsi_dispatcher(void);
>>   void acpi_set_gsi_to_irq_fallback(u32 (*)(u32));
>>
>>   struct irq_domain *acpi_irq_create_hierarchy(unsigned int flags,
>> --
> 
> With the above addressed, please feel free to add
> 
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> to the patch and route it along with the rest of the series.
> 

Will do, thanks!

> Thanks!
diff mbox series

Patch

diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c
index 1687483ff319..8eb09e45e5c5 100644
--- a/drivers/acpi/irq.c
+++ b/drivers/acpi/irq.c
@@ -12,7 +12,7 @@ 
 
 enum acpi_irq_model_id acpi_irq_model;
 
-static struct fwnode_handle *(*acpi_get_gsi_domain_id)(u32 gsi);
+static acpi_gsi_domain_disp_fn acpi_get_gsi_domain_id;
 static u32 (*acpi_gsi_to_irq_fallback)(u32 gsi);
 
 /**
@@ -307,12 +307,23 @@  EXPORT_SYMBOL_GPL(acpi_irq_get);
  *	for a given GSI
  */
 void __init acpi_set_irq_model(enum acpi_irq_model_id model,
-			       struct fwnode_handle *(*fn)(u32))
+	acpi_gsi_domain_disp_fn fn)
 {
 	acpi_irq_model = model;
 	acpi_get_gsi_domain_id = fn;
 }
 
+/**
+ * acpi_get_gsi_dispatcher - Returns dispatcher function that
+ *                           computes the domain fwnode for a
+ *                           given GSI.
+ */
+acpi_gsi_domain_disp_fn acpi_get_gsi_dispatcher(void)
+{
+	return acpi_get_gsi_domain_id;
+}
+EXPORT_SYMBOL_GPL(acpi_get_gsi_dispatcher);
+
 /**
  * acpi_set_gsi_to_irq_fallback - Register a GSI transfer
  * callback to fallback to arch specified implementation.
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 4e495b29c640..abc51288e867 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -336,8 +336,11 @@  int acpi_register_gsi (struct device *dev, u32 gsi, int triggering, int polarity
 int acpi_gsi_to_irq (u32 gsi, unsigned int *irq);
 int acpi_isa_irq_to_gsi (unsigned isa_irq, u32 *gsi);
 
+typedef struct fwnode_handle *(*acpi_gsi_domain_disp_fn)(u32);
+
 void acpi_set_irq_model(enum acpi_irq_model_id model,
-			struct fwnode_handle *(*)(u32));
+	acpi_gsi_domain_disp_fn fn);
+acpi_gsi_domain_disp_fn acpi_get_gsi_dispatcher(void);
 void acpi_set_gsi_to_irq_fallback(u32 (*)(u32));
 
 struct irq_domain *acpi_irq_create_hierarchy(unsigned int flags,