lkml.org 
[lkml]   [2022]   [Jan]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
SubjectRe: [PATCH v2] drivers: irqchip: add irq-type-changer
From
 > Such HW can stay in the bin.

Huh? Industry is full of ugly hardware (that is still surprisingly useful to cover practical needs at
low cost - which ensures it will persist). Sure I'd prefer to spend time for something better than
negotiating workarounds to keep ugly hardware working. But after 20+ years in the industry, I can only
agree with the statement Linus wrote back in 2006 in [1]:

| For a driver writer, there is one rule above _all_ other rules:
|
| "Reality sucks, deal with it"
|
| That rule is inviolate, and no amount of "I wish", and "it _should_
| work this way" or "..but the documentation says" matters at all.

[1] https://lore.kernel.org/lkml/Pine.LNX.4.64.0604241156340.3701@g5.osdl.org/


Can we finally stop blaming and propose a practical way to solve the original issue?

If the only solution you are ready to accept is the code that supports inversion only, and uses node's
"interrupts" property to provide info for interrupt-grand-parent, please state that explicitly.


<skipping all the statements I don't agree with>


> (I really should put a WARN_ON() in the irqdomain
> code to catch this sort of things).

I do support this idea.

Is a WARN_ON() in irq_domain_set_mapping() checking that nothing is already assigned to this hwirq in
this domain good enough to serve this purpose?

I.e. for a linear domain (which shall be enough to catch the wl18xx-on-kf case where both parent domain
and irq-type-changer's domain are linear):

--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -521,9 +521,10 @@ static void irq_domain_set_mapping(struct irq_domain *domain,
return;

mutex_lock(&domain->revmap_mutex);
- if (hwirq < domain->revmap_size)
+ if (hwirq < domain->revmap_size) {
+ WARN_ON(domain->revmap[hwirq] != NULL);
rcu_assign_pointer(domain->revmap[hwirq], irq_data);
- else
+ } else
radix_tree_insert(&domain->revmap_tree, hwirq, irq_data);
mutex_unlock(&domain->revmap_mutex);
}
This does not hit on my code.

Could you please suggest a better location for WARN_ON() that will hit?


Nikita

\
 
 \ /
  Last update: 2022-01-25 20:35    [W:0.039 / U:0.240 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site