On Wed, 24 Apr 2024 22:47:10 +0300
Andy Shevchenko <andy@xxxxxxxxxx> wrote:
> On Wed, Apr 24, 2024 at 08:51:23PM +0200, Marek Behún wrote:
> > On Wed, 24 Apr 2024 21:33:44 +0300
> > Andy Shevchenko <andy@xxxxxxxxxx> wrote:
> > > On Wed, Apr 24, 2024 at 07:38:05PM +0200, Marek Behún wrote:
>
> ...
>
> > > > +static void omnia_irq_mapping_drop(void *res)
> > > > +{
> > > > + irq_dispose_mapping((unsigned int)(unsigned long)res);
> > > > +}
> > >
> > > Leftover?
> >
> > What do you mean? I dropped the devm-helpers.h changes, now I do
> > devm_add_action_or_reset() manually, with this function as the action.
>
> But why?
>
> ...
>
> > > > + irq_idx = omnia_int_to_gpio_idx[__bf_shf(INT_TRNG)];
> > > > + irq = gpiod_to_irq(gpiochip_get_desc(&mcu->gc, irq_idx));
> > > > + if (irq < 0)
> > > > + return dev_err_probe(dev, irq, "Cannot get TRNG IRQ\n");
>
> > > > + err = devm_add_action_or_reset(dev, omnia_irq_mapping_drop,
> > > > + (void *)(unsigned long)irq);
> > > > + if (err)
> > > > + return err;
> > >
> > > Are you sure it's correct now?
> >
> > Yes, why wouldn't it?
>
> For what purpose? I don't see drivers doing that. Are you expecting that
> the same IRQ mapping will be reused for something else? Can you elaborate
> how? (I can imagine one theoretical / weird case how to achieve that,
> but impractical.)
I do a lot of binding/unbinding of that driver. I was under the
impression that all resources should be dropped on driver unbind.
> Besides above, this is asymmetrical call to gpiod_to_irq(). If we really care
> about this, it should be provided by GPIO library.
>
Something like the following?
>From 5aac93d55f6fb750726f7e879672142956981a4c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marek=20Beh=C3=BAn?= <kabel@xxxxxxxxxx>
Date: Thu, 25 Apr 2024 11:33:33 +0200
Subject: [PATCH] gpiolib: devres: Add resource managed version of
gpiod_to_irq()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Add devm_gpiod_to_irq(), a resource managed version of gpiod_to_irq().
The release function calls irq_dispose_mapping().
Signed-off-by: Marek Behún <kabel@xxxxxxxxxx>
---
drivers/gpio/gpiolib-devres.c | 27 +++++++++++++++++++++++++++
include/linux/gpio/consumer.h | 10 ++++++++++
2 files changed, 37 insertions(+)
diff --git a/drivers/gpio/gpiolib-devres.c b/drivers/gpio/gpiolib-devres.c
index 4987e62dcb3d..98a40492e596 100644
--- a/drivers/gpio/gpiolib-devres.c
+++ b/drivers/gpio/gpiolib-devres.c
@@ -12,6 +12,7 @@
#include <linux/gpio/consumer.h>
#include <linux/device.h>
#include <linux/gfp.h>
+#include <linux/irqdomain.h>
#include "gpiolib.h"
@@ -427,3 +428,29 @@ int devm_gpiochip_add_data_with_key(struct device *dev, struct gpio_chip *gc, vo
return devm_add_action_or_reset(dev, devm_gpio_chip_release, gc);
}
EXPORT_SYMBOL_GPL(devm_gpiochip_add_data_with_key);
+
+static void devm_gpiod_irq_release(void *data)
+{
+ irq_dispose_mapping((unsigned int)(unsigned long)data);
+}
+
+/**
+ * devm_gpiod_to_irq() - Resource managed devm_gpiod_to_irq()
+ * @dev: pointer to the device that gpio_chip belongs to.
+ * @desc: gpio whose IRQ will be returned
+ *
+ * Return the IRQ corresponding to the passed GPIO, or an error code in case of
+ * error.
+ */
+int devm_gpiod_to_irq(struct device *dev, const struct gpio_desc *desc)
+{
+ int virq;
+
+ virq = gpiod_to_irq(desc);
+ if (virq < 0)
+ return virq;
+
+ return devm_add_action_or_reset(dev, devm_gpiod_irq_release,
+ (void *)(unsigned long)virq);
+}
+EXPORT_SYMBOL_GPL(devm_gpiod_to_irq);
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index db2dfbae8edb..e8f4829538f6 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -165,6 +165,8 @@ int gpiod_is_active_low(const struct gpio_desc *desc);
int gpiod_cansleep(const struct gpio_desc *desc);
int gpiod_to_irq(const struct gpio_desc *desc);
+int devm_gpiod_to_irq(struct device *dev, const struct gpio_desc *desc);
+
int gpiod_set_consumer_name(struct gpio_desc *desc, const char *name);
/* Convert between the old gpio_ and new gpiod_ interfaces */
@@ -519,6 +521,14 @@ static inline int gpiod_to_irq(const struct gpio_desc *desc)
return -EINVAL;
}
+static inline int devm_gpiod_to_irq(struct device *dev,
+ const struct gpio_desc *desc)
+{
+ /* GPIO can never have been requested */
+ WARN_ON(desc);
+ return -EINVAL;
+}
+
static inline int gpiod_set_consumer_name(struct gpio_desc *desc,
const char *name)
{
--
2.43.2
