diff mbox series

[v6,06/11] devm-helpers: Add resource managed version of irq_create_mapping()

Message ID 20240418121116.22184-7-kabel@kernel.org (mailing list archive)
State Superseded
Delegated to: Arnd Bergmann
Headers show
Series Turris Omnia MCU driver | expand

Commit Message

Marek Behún April 18, 2024, 12:11 p.m. UTC
Add resource managed version of irq_create_mapping(), to help drivers
automatically dispose a linux irq mapping when driver is detached.

Signed-off-by: Marek Behún <kabel@kernel.org>
---
 include/linux/devm-helpers.h | 54 ++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

Comments

Matti Vaittinen April 23, 2024, 12:32 p.m. UTC | #1
On 4/18/24 15:11, Marek Behún wrote:
> Add resource managed version of irq_create_mapping(), to help drivers
> automatically dispose a linux irq mapping when driver is detached.
> 
> Signed-off-by: Marek Behún <kabel@kernel.org>
> ---
>   include/linux/devm-helpers.h | 54 ++++++++++++++++++++++++++++++++++++
>   1 file changed, 54 insertions(+)
> 
> diff --git a/include/linux/devm-helpers.h b/include/linux/devm-helpers.h
> index 74891802200d..fe62b28baf03 100644
> --- a/include/linux/devm-helpers.h
> +++ b/include/linux/devm-helpers.h
> @@ -24,6 +24,8 @@
>    */
>   
>   #include <linux/device.h>
> +#include <linux/kconfig.h>
> +#include <linux/irqdomain.h>

I still think stuffing all unrelated APIs in one header will lead to 
problems because of the sheer amount of unrelated stuff which gets 
included when only some APIs from this file are used.

But as I said during V5 review, I don't want to block this if no one 
else thinks it is a real problem.

Reviewed-by: Matti Vaittinen <mazziesaccount@gmail.com>

>   #include <linux/workqueue.h>
>   
>   static inline void devm_delayed_work_drop(void *res)
> @@ -76,4 +78,56 @@ static inline int devm_work_autocancel(struct device *dev,
>   	return devm_add_action(dev, devm_work_drop, w);
>   }
>   
> +/**
> + * devm_irq_mapping_drop - devm action for disposing an irq mapping
> + * @res:	linux irq number cast to the void * type
> + *
> + * devm_irq_mapping_drop() can be used as an action parameter for the
> + * devm_add_action_or_reset() function in order to automatically dispose
> + * a linux irq mapping when a device driver is detached.
> + */
> +static inline void devm_irq_mapping_drop(void *res)
> +{
> +	irq_dispose_mapping((unsigned int)(unsigned long)res);
> +}
> +
> +/**
> + * devm_irq_create_mapping - Resource managed version of irq_create_mapping()
> + * @dev:	Device which lifetime the mapping is bound to
> + * @domain:	domain owning this hardware interrupt or NULL for default domain
> + * @hwirq:	hardware irq number in that domain space
> + *
> + * Create an irq mapping to linux irq space which is automatically disposed when
> + * the driver is detached.
> + * devm_irq_create_mapping() can be used to omit the explicit
> + * irq_dispose_mapping() call when driver is detached.
> + *
> + * Returns a linux irq number on success, -ENXIO if the mapping could not be
> + * created, or a negative error number if devm action could not be added.
> + */
> +static inline int devm_irq_create_mapping(struct device *dev,
> +					  struct irq_domain *domain,
> +					  irq_hw_number_t hwirq)
> +{
> +	unsigned int virq = irq_create_mapping(domain, hwirq);
> +
> +	if (!virq)
> +		return -ENXIO;
> +
> +	/*
> +	 * irq_dispose_mapping() is an empty function if CONFIG_IRQ_DOMAIN is
> +	 * disabled. No need to register an action in that case.
> +	 */
> +	if (IS_ENABLED(CONFIG_IRQ_DOMAIN)) {
> +		int err;
> +
> +		err = devm_add_action_or_reset(dev, devm_irq_mapping_drop,
> +					       (void *)(unsigned long)virq);
> +		if (err)
> +			return err;
> +	}
> +
> +	return virq;
> +}
> +
>   #endif
Andy Shevchenko April 23, 2024, 12:46 p.m. UTC | #2
On Thu, Apr 18, 2024 at 02:11:11PM +0200, Marek Behún wrote:
> Add resource managed version of irq_create_mapping(), to help drivers
> automatically dispose a linux irq mapping when driver is detached.

AFAICS this is a dead code.
Please, drop it from the next version.
Marek Behún April 23, 2024, 2:21 p.m. UTC | #3
On Tue, Apr 23, 2024 at 03:46:15PM +0300, Andy Shevchenko wrote:
> On Thu, Apr 18, 2024 at 02:11:11PM +0200, Marek Behún wrote:
> > Add resource managed version of irq_create_mapping(), to help drivers
> > automatically dispose a linux irq mapping when driver is detached.
> 
> AFAICS this is a dead code.
> Please, drop it from the next version.

What do you mean by dead code?
Marek Behún April 23, 2024, 3:14 p.m. UTC | #4
On Tue, 23 Apr 2024 15:46:15 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Thu, Apr 18, 2024 at 02:11:11PM +0200, Marek Behún wrote:
> > Add resource managed version of irq_create_mapping(), to help drivers
> > automatically dispose a linux irq mapping when driver is detached.  
> 
> AFAICS this is a dead code.
> Please, drop it from the next version.

Do you mean that it is not used when this patch is applied? Because it
is used in the very next patch. Should I merge these two patches into
one?

Marek
Andy Shevchenko April 23, 2024, 3:56 p.m. UTC | #5
On Tue, Apr 23, 2024 at 05:14:58PM +0200, Marek Behún wrote:
> On Tue, 23 Apr 2024 15:46:15 +0300
> Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> > On Thu, Apr 18, 2024 at 02:11:11PM +0200, Marek Behún wrote:
> > > Add resource managed version of irq_create_mapping(), to help drivers
> > > automatically dispose a linux irq mapping when driver is detached.  
> > 
> > AFAICS this is a dead code.
> > Please, drop it from the next version.

> Do you mean that it is not used when this patch is applied? 

No.

> Because it is used in the very next patch.

I missed that, sorry.

> Should I merge these two patches into one?

No.
diff mbox series

Patch

diff --git a/include/linux/devm-helpers.h b/include/linux/devm-helpers.h
index 74891802200d..fe62b28baf03 100644
--- a/include/linux/devm-helpers.h
+++ b/include/linux/devm-helpers.h
@@ -24,6 +24,8 @@ 
  */
 
 #include <linux/device.h>
+#include <linux/kconfig.h>
+#include <linux/irqdomain.h>
 #include <linux/workqueue.h>
 
 static inline void devm_delayed_work_drop(void *res)
@@ -76,4 +78,56 @@  static inline int devm_work_autocancel(struct device *dev,
 	return devm_add_action(dev, devm_work_drop, w);
 }
 
+/**
+ * devm_irq_mapping_drop - devm action for disposing an irq mapping
+ * @res:	linux irq number cast to the void * type
+ *
+ * devm_irq_mapping_drop() can be used as an action parameter for the
+ * devm_add_action_or_reset() function in order to automatically dispose
+ * a linux irq mapping when a device driver is detached.
+ */
+static inline void devm_irq_mapping_drop(void *res)
+{
+	irq_dispose_mapping((unsigned int)(unsigned long)res);
+}
+
+/**
+ * devm_irq_create_mapping - Resource managed version of irq_create_mapping()
+ * @dev:	Device which lifetime the mapping is bound to
+ * @domain:	domain owning this hardware interrupt or NULL for default domain
+ * @hwirq:	hardware irq number in that domain space
+ *
+ * Create an irq mapping to linux irq space which is automatically disposed when
+ * the driver is detached.
+ * devm_irq_create_mapping() can be used to omit the explicit
+ * irq_dispose_mapping() call when driver is detached.
+ *
+ * Returns a linux irq number on success, -ENXIO if the mapping could not be
+ * created, or a negative error number if devm action could not be added.
+ */
+static inline int devm_irq_create_mapping(struct device *dev,
+					  struct irq_domain *domain,
+					  irq_hw_number_t hwirq)
+{
+	unsigned int virq = irq_create_mapping(domain, hwirq);
+
+	if (!virq)
+		return -ENXIO;
+
+	/*
+	 * irq_dispose_mapping() is an empty function if CONFIG_IRQ_DOMAIN is
+	 * disabled. No need to register an action in that case.
+	 */
+	if (IS_ENABLED(CONFIG_IRQ_DOMAIN)) {
+		int err;
+
+		err = devm_add_action_or_reset(dev, devm_irq_mapping_drop,
+					       (void *)(unsigned long)virq);
+		if (err)
+			return err;
+	}
+
+	return virq;
+}
+
 #endif