2018-07-20 16:42:54

by Liu Xiang

[permalink] [raw]
Subject: [PATCH] of: irq: Add a helper function for irq_of_parse_and_map

Implement a resource managed irq_of_parse_and_map function.

Signed-off-by: Liu Xiang <[email protected]>
---
drivers/of/irq.c | 38 ++++++++++++++++++++++++++++++++++++++
include/linux/of_irq.h | 7 +++++++
2 files changed, 45 insertions(+)

diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index 02ad93a..947fe41 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -45,6 +45,44 @@ unsigned int irq_of_parse_and_map(struct device_node *dev, int index)
}
EXPORT_SYMBOL_GPL(irq_of_parse_and_map);

+static void devm_irq_dispose_mapping(struct device *dev, void *res)
+{
+ irq_dispose_mapping(*(unsigned int *)res);
+}
+
+/**
+ * devm_irq_of_parse_and_map - Resource-managed irq_of_parse_and_map
+ * @dev: Device whose interrupt is to be mapped
+ * @index: Index of the interrupt to map
+ *
+ * Managed irq_of_parse_and_map. Irq mapping created by this function is
+ * automatically disposed on driver detach.
+ *
+ * RETURNS:
+ * Returns a linux irq number on success, 0 on failure.
+ */
+unsigned int devm_irq_of_parse_and_map(struct device *dev, int index)
+{
+ struct device_node *np = dev->of_node;
+ unsigned int *dr = NULL;
+ unsigned int rc = 0;
+
+ dr = devres_alloc(devm_irq_dispose_mapping, sizeof(unsigned int),
+ GFP_KERNEL);
+ if (!dr)
+ return 0;
+
+ rc = irq_of_parse_and_map(np, index);
+ if (rc > 0) {
+ *dr = rc;
+ devres_add(dev, dr);
+ } else
+ devres_free(dr);
+
+ return rc;
+}
+EXPORT_SYMBOL_GPL(devm_irq_of_parse_and_map);
+
/**
* of_irq_find_parent - Given a device node, find its interrupt parent node
* @child: pointer to device node
diff --git a/include/linux/of_irq.h b/include/linux/of_irq.h
index 1214cab..1ac26ff 100644
--- a/include/linux/of_irq.h
+++ b/include/linux/of_irq.h
@@ -55,6 +55,7 @@ extern struct irq_domain *of_msi_map_get_device_domain(struct device *dev,
u32 rid);
extern void of_msi_configure(struct device *dev, struct device_node *np);
u32 of_msi_map_rid(struct device *dev, struct device_node *msi_np, u32 rid_in);
+extern unsigned int devm_irq_of_parse_and_map(struct device *dev, int index);
#else
static inline int of_irq_count(struct device_node *dev)
{
@@ -97,6 +98,12 @@ static inline u32 of_msi_map_rid(struct device *dev,
{
return rid_in;
}
+
+static inline unsigned int devm_irq_of_parse_and_map(struct device *dev,
+ int index)
+{
+ return 0;
+}
#endif

#if defined(CONFIG_OF_IRQ) || defined(CONFIG_SPARC)
--
1.9.1



2018-07-23 15:30:53

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH] of: irq: Add a helper function for irq_of_parse_and_map

On Sat, Jul 21, 2018 at 9:38 AM Liu Xiang <[email protected]> wrote:
>
> Implement a resource managed irq_of_parse_and_map function.

That is obvious from the diff. Why do you need this?

Anything that is a platform device should use platform_get_irq. The
mapping is created by the core Also, of_irq_get() is preferred over
irq_of_parse_and_map() for new users as it has better error return
handling.

>
> Signed-off-by: Liu Xiang <[email protected]>
> ---
> drivers/of/irq.c | 38 ++++++++++++++++++++++++++++++++++++++
> include/linux/of_irq.h | 7 +++++++
> 2 files changed, 45 insertions(+)
>
> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
> index 02ad93a..947fe41 100644
> --- a/drivers/of/irq.c
> +++ b/drivers/of/irq.c
> @@ -45,6 +45,44 @@ unsigned int irq_of_parse_and_map(struct device_node *dev, int index)
> }
> EXPORT_SYMBOL_GPL(irq_of_parse_and_map);
>
> +static void devm_irq_dispose_mapping(struct device *dev, void *res)
> +{
> + irq_dispose_mapping(*(unsigned int *)res);

We don't ref count creating mappings, so this would break for any shared irq.

Rob

2018-07-24 13:20:47

by Liu Xiang

[permalink] [raw]
Subject: Re:Re: [PATCH] of: irq: Add a helper function for irq_of_parse_and_map


I found in probe function some drivers use irq_of_parse_and_map
and do not call irq_dispose_mapping in err path, while other drivers do.
So I think a resource managed irq_of_parse_and_map function can
be better to finish this job.

At 2018-07-23 23:28:52, "Rob Herring" <[email protected]> wrote:
>On Sat, Jul 21, 2018 at 9:38 AM Liu Xiang <[email protected]> wrote:
>>
>> Implement a resource managed irq_of_parse_and_map function.
>
>That is obvious from the diff. Why do you need this?
>
>Anything that is a platform device should use platform_get_irq. The
>mapping is created by the core Also, of_irq_get() is preferred over
>irq_of_parse_and_map() for new users as it has better error return
>handling.
>
>>
>> Signed-off-by: Liu Xiang <[email protected]>
>> ---
>> drivers/of/irq.c | 38 ++++++++++++++++++++++++++++++++++++++
>> include/linux/of_irq.h | 7 +++++++
>> 2 files changed, 45 insertions(+)
>>
>> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
>> index 02ad93a..947fe41 100644
>> --- a/drivers/of/irq.c
>> +++ b/drivers/of/irq.c
>> @@ -45,6 +45,44 @@ unsigned int irq_of_parse_and_map(struct device_node *dev, int index)
>> }
>> EXPORT_SYMBOL_GPL(irq_of_parse_and_map);
>>
>> +static void devm_irq_dispose_mapping(struct device *dev, void *res)
>> +{
>> + irq_dispose_mapping(*(unsigned int *)res);
>
>We don't ref count creating mappings, so this would break for any shared irq.
>
>Rob