Devices can provide multiple interrupt lines. One reason for this is that
a device has multiple subfunctions, each providing its own interrupt line.
Another reason is that a device can be designed to be used (also) on a
system where some of the interrupts can be routed to another processor.
A line often further acts as a demultiplex for specific interrupts
and has it's respective set of interrupt (status, mask, ack, ...)
registers.
Regmap supports the handling of these registers and demultiplexing
interrupts, but interrupt domain code ends up assigning the same name for
the per interrupt line domains. This will cause a naming collision in the
debugFS code and can also lead to confusion, as /proc/interrupts would
show two separate interrupts with the same domain name and hardware
interrupt number.
Instead of adding a workaround in regmap or driver code, allow giving a
name suffix for the domain name when the domain is created.
Add support for name suffix in __irq_domain_add() and allow using it via
new irq_domain_create_linear_named() function. This is the only function
required by the regmap code to support such setups because no new
drivers are expected to be using legacy domains.
Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
---
Revision history:
v2 => v3:
- Improve the commit message
- Drop the new name-suffix API for legacy domains
Some devices which could use this are ROHM's BD96801, BD96802 and BD96811
PMICs. BD96801 is being upstreamed, others are on my TODO list - but I
believe this may be beneficial to any other devices as well. Especially
for those which provide IRQs that are clearly different from each
other.
Some discussion about this can be found from:
https://lore.kernel.org/all/Zjzt8mOW6dO_7XNV@finisterre.sirena.org.uk/
---
include/linux/irqdomain.h | 22 ++++++++++++++++------
kernel/irq/irqdomain.c | 37 +++++++++++++++++++++++++++++--------
2 files changed, 45 insertions(+), 14 deletions(-)
diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index 21ecf582a0fe..0d829495bf9e 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -260,7 +260,7 @@ void irq_domain_free_fwnode(struct fwnode_handle *fwnode);
struct irq_domain *__irq_domain_add(struct fwnode_handle *fwnode, unsigned int size,
irq_hw_number_t hwirq_max, int direct_max,
const struct irq_domain_ops *ops,
- void *host_data);
+ void *host_data, const char *name_suffix);
struct irq_domain *irq_domain_create_simple(struct fwnode_handle *fwnode,
unsigned int size,
unsigned int first_irq,
@@ -350,7 +350,8 @@ static inline struct irq_domain *irq_domain_add_linear(struct device_node *of_no
const struct irq_domain_ops *ops,
void *host_data)
{
- return __irq_domain_add(of_node_to_fwnode(of_node), size, size, 0, ops, host_data);
+ return __irq_domain_add(of_node_to_fwnode(of_node), size, size, 0, ops,
+ host_data, NULL);
}
#ifdef CONFIG_IRQ_DOMAIN_NOMAP
@@ -359,7 +360,8 @@ static inline struct irq_domain *irq_domain_add_nomap(struct device_node *of_nod
const struct irq_domain_ops *ops,
void *host_data)
{
- return __irq_domain_add(of_node_to_fwnode(of_node), 0, max_irq, max_irq, ops, host_data);
+ return __irq_domain_add(of_node_to_fwnode(of_node), 0, max_irq, max_irq,
+ ops, host_data, NULL);
}
extern unsigned int irq_create_direct_mapping(struct irq_domain *host);
@@ -369,7 +371,7 @@ static inline struct irq_domain *irq_domain_add_tree(struct device_node *of_node
const struct irq_domain_ops *ops,
void *host_data)
{
- return __irq_domain_add(of_node_to_fwnode(of_node), 0, ~0, 0, ops, host_data);
+ return __irq_domain_add(of_node_to_fwnode(of_node), 0, ~0, 0, ops, host_data, NULL);
}
static inline struct irq_domain *irq_domain_create_linear(struct fwnode_handle *fwnode,
@@ -377,14 +379,22 @@ static inline struct irq_domain *irq_domain_create_linear(struct fwnode_handle *
const struct irq_domain_ops *ops,
void *host_data)
{
- return __irq_domain_add(fwnode, size, size, 0, ops, host_data);
+ return __irq_domain_add(fwnode, size, size, 0, ops, host_data, NULL);
+}
+
+static inline struct irq_domain *irq_domain_create_linear_named(struct fwnode_handle *fwnode,
+ unsigned int size,
+ const struct irq_domain_ops *ops,
+ void *host_data, const char *name_suffix)
+{
+ return __irq_domain_add(fwnode, size, size, 0, ops, host_data, name_suffix);
}
static inline struct irq_domain *irq_domain_create_tree(struct fwnode_handle *fwnode,
const struct irq_domain_ops *ops,
void *host_data)
{
- return __irq_domain_add(fwnode, 0, ~0, 0, ops, host_data);
+ return __irq_domain_add(fwnode, 0, ~0, 0, ops, host_data, NULL);
}
extern void irq_domain_remove(struct irq_domain *host);
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index aadc8891cc16..c2a565aeada8 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -132,7 +132,8 @@ static struct irq_domain *__irq_domain_create(struct fwnode_handle *fwnode,
irq_hw_number_t hwirq_max,
int direct_max,
const struct irq_domain_ops *ops,
- void *host_data)
+ void *host_data,
+ const char *name_suffix)
{
struct irqchip_fwid *fwid;
struct irq_domain *domain;
@@ -150,6 +151,17 @@ static struct irq_domain *__irq_domain_create(struct fwnode_handle *fwnode,
return NULL;
if (is_fwnode_irqchip(fwnode)) {
+ /*
+ * The name_suffix is only intended to be used to avoid a name
+ * collison, when multiple domains are created for a single
+ * device and the name is picked using a real device node.
+ * (Typical use-case is regmap-IRQ controllers for devices
+ * providing more than one physical IRQ.) There should be no
+ * need to use name_suffix with irqchip-fwnode.
+ */
+ if (name_suffix)
+ return NULL;
+
fwid = container_of(fwnode, struct irqchip_fwid, fwnode);
switch (fwid->type) {
@@ -177,7 +189,11 @@ static struct irq_domain *__irq_domain_create(struct fwnode_handle *fwnode,
* unhappy about. Replace them with ':', which does
* the trick and is not as offensive as '\'...
*/
- name = kasprintf(GFP_KERNEL, "%pfw", fwnode);
+ if (!name_suffix)
+ name = kasprintf(GFP_KERNEL, "%pfw", fwnode);
+ else
+ name = kasprintf(GFP_KERNEL, "%pfw-%s", fwnode,
+ name_suffix);
if (!name) {
kfree(domain);
return NULL;
@@ -249,6 +265,8 @@ static void __irq_domain_publish(struct irq_domain *domain)
* direct mapping
* @ops: domain callbacks
* @host_data: Controller private data pointer
+ * @name_suffix: Optional name suffix to avoid collisions when multiple domains
+ * are added using same fwnode
*
* Allocates and initializes an irq_domain structure.
* Returns pointer to IRQ domain, or NULL on failure.
@@ -256,12 +274,12 @@ static void __irq_domain_publish(struct irq_domain *domain)
struct irq_domain *__irq_domain_add(struct fwnode_handle *fwnode, unsigned int size,
irq_hw_number_t hwirq_max, int direct_max,
const struct irq_domain_ops *ops,
- void *host_data)
+ void *host_data, const char *name_suffix)
{
struct irq_domain *domain;
domain = __irq_domain_create(fwnode, size, hwirq_max, direct_max,
- ops, host_data);
+ ops, host_data, name_suffix);
if (domain)
__irq_domain_publish(domain);
@@ -362,7 +380,7 @@ struct irq_domain *irq_domain_create_simple(struct fwnode_handle *fwnode,
{
struct irq_domain *domain;
- domain = __irq_domain_add(fwnode, size, size, 0, ops, host_data);
+ domain = __irq_domain_add(fwnode, size, size, 0, ops, host_data, NULL);
if (!domain)
return NULL;
@@ -418,7 +436,8 @@ struct irq_domain *irq_domain_create_legacy(struct fwnode_handle *fwnode,
{
struct irq_domain *domain;
- domain = __irq_domain_add(fwnode, first_hwirq + size, first_hwirq + size, 0, ops, host_data);
+ domain = __irq_domain_add(fwnode, first_hwirq + size, first_hwirq + size,
+ 0, ops, host_data, NULL);
if (domain)
irq_domain_associate_many(domain, first_irq, first_hwirq, size);
@@ -1147,9 +1166,11 @@ struct irq_domain *irq_domain_create_hierarchy(struct irq_domain *parent,
struct irq_domain *domain;
if (size)
- domain = __irq_domain_create(fwnode, size, size, 0, ops, host_data);
+ domain = __irq_domain_create(fwnode, size, size, 0, ops,
+ host_data, NULL);
else
- domain = __irq_domain_create(fwnode, 0, ~0, 0, ops, host_data);
+ domain = __irq_domain_create(fwnode, 0, ~0, 0, ops, host_data,
+ NULL);
if (domain) {
if (parent)
--
2.45.1
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
Matti!
On Tue, Jun 04 2024 at 10:55, Matti Vaittinen wrote:
> struct irq_domain *__irq_domain_add(struct fwnode_handle *fwnode, unsigned int size,
> irq_hw_number_t hwirq_max, int direct_max,
> const struct irq_domain_ops *ops,
> - void *host_data);
> + void *host_data, const char *name_suffix);
> struct irq_domain *irq_domain_create_simple(struct fwnode_handle *fwnode,
> unsigned int size,
> unsigned int first_irq,
> @@ -350,7 +350,8 @@ static inline struct irq_domain *irq_domain_add_linear(struct device_node *of_no
> const struct irq_domain_ops *ops,
> void *host_data)
> {
> - return __irq_domain_add(of_node_to_fwnode(of_node), size, size, 0, ops, host_data);
> + return __irq_domain_add(of_node_to_fwnode(of_node), size, size, 0, ops,
> + host_data, NULL);
....
Looking at the resulting amount of churn to add that argument, I'm not
really enthused. There is some other unrelated change required in this
area:
https://lore.kernel.org/all/8734pr5yq1.ffs@tglx
My suggestion to convert all of this mess into a template based
mechanism would nicely solve your problem too.
Can you please have a look and eventually team up with Herve (CC'ed) to
sort this out? I'm happy to help and give guidance.
Thanks,
tglx
Hello Thomas, Herve.
On 6/6/24 21:59, Thomas Gleixner wrote:
> Matti!
>
> On Tue, Jun 04 2024 at 10:55, Matti Vaittinen wrote:
>> struct irq_domain *__irq_domain_add(struct fwnode_handle *fwnode, unsigned int size,
>> irq_hw_number_t hwirq_max, int direct_max,
>> const struct irq_domain_ops *ops,
>> - void *host_data);
>> + void *host_data, const char *name_suffix);
>> struct irq_domain *irq_domain_create_simple(struct fwnode_handle *fwnode,
>> unsigned int size,
>> unsigned int first_irq,
>> @@ -350,7 +350,8 @@ static inline struct irq_domain *irq_domain_add_linear(struct device_node *of_no
>> const struct irq_domain_ops *ops,
>> void *host_data)
>> {
>> - return __irq_domain_add(of_node_to_fwnode(of_node), size, size, 0, ops, host_data);
>> + return __irq_domain_add(of_node_to_fwnode(of_node), size, size, 0, ops,
>> + host_data, NULL);
>
> ....
>
> Looking at the resulting amount of churn to add that argument, I'm not
> really enthused. There is some other unrelated change required in this
> area:
>
> https://lore.kernel.org/all/8734pr5yq1.ffs@tglx
>
> My suggestion to convert all of this mess into a template based
> mechanism would nicely solve your problem too.
I am not entirely sure what you mean by template based in this context.
My brains are somehow fixed to start thinking of C++ templates, or C
macro magic with typeof() and I just can't get past that.
Anyways, what I picked from discussion between you and Herve, is using
an initialization structure (struct irq_domain_info) for the new domain
creation function (irq_domain_instantiate()) instead of adding bunch of
functions with quite a few separate arguments. So, I assume you're
referring to a possibility to add the name-suffix in this initialization
structure? I hope I got this right.
I assume there is no intention to change the existing public
irq_domain_creat_foo() APIs to use the new irq_domain_info - and change
all the callers(?) But I think changing the internal
__irq_domain_create() to use this new info struct should be very much
doable - although, in my opinion, making existing callers of the
__irq_domain_create() to assign their parameters to this struct so they
can pass it to __irq_domain_create() does not seem so nice.
So, even though I am not really happy about the delay (I secretly hoped
to get the series merged before my summer vacations ;) ) - I admit your
suggested change looks cleaner (again, at least to me).
Herve, do you have any idea when you plan to do further sketching on
this? Do you want me to try seeing if I can add the struct
irq_domain_info and maybe use that in the __irq_domain_add() to get the
name-suffix added? I might be able to send one version out during next
week - but then I plan to be offline for couple of weeks ... so it may
be I am not much of a help here.
> Can you please have a look and eventually team up with Herve (CC'ed) to
> sort this out? I'm happy to help and give guidance.
I appreciate the guidance! Thanks Thomas.
Yours,
-- Matti
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
Hi Matti, On Fri, 7 Jun 2024 09:38:31 +0300 Matti Vaittinen <mazziesaccount@gmail.com> wrote: ... > Herve, do you have any idea when you plan to do further sketching on > this? Do you want me to try seeing if I can add the struct > irq_domain_info and maybe use that in the __irq_domain_add() to get the > name-suffix added? I might be able to send one version out during next > week - but then I plan to be offline for couple of weeks ... so it may > be I am not much of a help here. > On my side, I plan to work on it next week too. If you are off a couple of weeks after, I think I can start and move forward on this topic. Best regards, Hervé -- Hervé Codina, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
On 6/7/24 11:13, Herve Codina wrote: > Hi Matti, > > On Fri, 7 Jun 2024 09:38:31 +0300 > Matti Vaittinen <mazziesaccount@gmail.com> wrote: > > ... > >> Herve, do you have any idea when you plan to do further sketching on >> this? Do you want me to try seeing if I can add the struct >> irq_domain_info and maybe use that in the __irq_domain_add() to get the >> name-suffix added? I might be able to send one version out during next >> week - but then I plan to be offline for couple of weeks ... so it may >> be I am not much of a help here. >> > > On my side, I plan to work on it next week too. > If you are off a couple of weeks after, I think I can start and move forward > on this topic. Thanks for the prompt reply and thanks for working with this :) I'll leave it to you for now then, as I don't think it makes much sense to intentionally do conflicting changes. I'll see what you've been up to when I return to the keyboard :) I'd appreciated if you added me to CC when sending the irqdomain refactoring patches (but I can dig them up if you don't). Have fun! Yours, -- Matti -- Matti Vaittinen Linux kernel developer at ROHM Semiconductors Oulu Finland ~~ When things go utterly wrong vim users can always type :help! ~~
On Fri, 7 Jun 2024 11:49:07 +0300 Matti Vaittinen <mazziesaccount@gmail.com> wrote: > On 6/7/24 11:13, Herve Codina wrote: > > Hi Matti, > > > > On Fri, 7 Jun 2024 09:38:31 +0300 > > Matti Vaittinen <mazziesaccount@gmail.com> wrote: > > > > ... > > > >> Herve, do you have any idea when you plan to do further sketching on > >> this? Do you want me to try seeing if I can add the struct > >> irq_domain_info and maybe use that in the __irq_domain_add() to get the > >> name-suffix added? I might be able to send one version out during next > >> week - but then I plan to be offline for couple of weeks ... so it may > >> be I am not much of a help here. > >> > > > > On my side, I plan to work on it next week too. > > If you are off a couple of weeks after, I think I can start and move forward > > on this topic. > > Thanks for the prompt reply and thanks for working with this :) I'll > leave it to you for now then, as I don't think it makes much sense to > intentionally do conflicting changes. I'll see what you've been up to > when I return to the keyboard :) I'd appreciated if you added me to CC > when sending the irqdomain refactoring patches (but I can dig them up if > you don't). Sure, you will CC you. Also, I am not sure that I will perfectly take into account your use-case but it should not be a big deal to add it on top of my commits afterwards. > > Have fun! > Cheers, Hervé
On 6/7/24 12:23, Herve Codina wrote: > On Fri, 7 Jun 2024 11:49:07 +0300 > Matti Vaittinen <mazziesaccount@gmail.com> wrote: > >> On 6/7/24 11:13, Herve Codina wrote: >>> Hi Matti, >>> >>> On Fri, 7 Jun 2024 09:38:31 +0300 >>> Matti Vaittinen <mazziesaccount@gmail.com> wrote: >>> >>> ... >>> >>>> Herve, do you have any idea when you plan to do further sketching on >>>> this? Do you want me to try seeing if I can add the struct >>>> irq_domain_info and maybe use that in the __irq_domain_add() to get the >>>> name-suffix added? I might be able to send one version out during next >>>> week - but then I plan to be offline for couple of weeks ... so it may >>>> be I am not much of a help here. >>>> >>> >>> On my side, I plan to work on it next week too. >>> If you are off a couple of weeks after, I think I can start and move forward >>> on this topic. >> >> Thanks for the prompt reply and thanks for working with this :) I'll >> leave it to you for now then, as I don't think it makes much sense to >> intentionally do conflicting changes. I'll see what you've been up to >> when I return to the keyboard :) I'd appreciated if you added me to CC >> when sending the irqdomain refactoring patches (but I can dig them up if >> you don't). > > Sure, you will CC you. Thanks! > Also, I am not sure that I will perfectly take into account your use-case > but it should not be a big deal to add it on top of my commits afterwards. No problem! That's just fine :) -- Matti Vaittinen Linux kernel developer at ROHM Semiconductors Oulu Finland ~~ When things go utterly wrong vim users can always type :help! ~~
© 2016 - 2026 Red Hat, Inc.