On Tue, Oct 06 2020 at 11:11, Marc Zyngier wrote:
> It appears that some HW is ugly enough that not all the interrupts
> connected to a particular interrupt controller end up with the same
> hierarchy repth (some of them are terminated early). This leaves
depth?
> the irqchip hacker with only two choices, both equally bad:
>
> - create discrete domain chains, one for each "hierarchy depth",
> which is very hard to maintain
>
> - create fake hierarchy levels for the shallow paths, leading
> to all kind of problems (what are the safe hwirq values for these
> fake levels?)
>
> Instead, let's offer the possibility to cut short a single interrupt
s/let's offer/implement/
> hierarchy, exactly representing the HW. This can only be done from
> the .alloc() callback, before mappings can be established.
>
> Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx>
> ---
> include/linux/irqdomain.h | 3 +++
> kernel/irq/irqdomain.c | 56 +++++++++++++++++++++++++++++++++++----
> 2 files changed, 54 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
> index b37350c4fe37..c6901c1bb981 100644
> --- a/include/linux/irqdomain.h
> +++ b/include/linux/irqdomain.h
> @@ -509,6 +509,9 @@ extern void irq_domain_free_irqs_parent(struct irq_domain *domain,
> unsigned int irq_base,
> unsigned int nr_irqs);
>
> +extern int irq_domain_trim_hierarchy(unsigned int virq,
> + struct irq_domain *domain);
> +
> static inline bool irq_domain_is_hierarchy(struct irq_domain *domain)
> {
> return domain->flags & IRQ_DOMAIN_FLAG_HIERARCHY;
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index 76cd7ebd1178..d0adaeea70b6 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -1136,6 +1136,17 @@ static struct irq_data *irq_domain_insert_irq_data(struct irq_domain *domain,
> return irq_data;
> }
>
> +static void __irq_domain_free_hierarchy(struct irq_data *irq_data)
> +{
> + struct irq_data *tmp;
> +
> + while (irq_data) {
> + tmp = irq_data;
> + irq_data = irq_data->parent_data;
> + kfree(tmp);
> + }
> +}
> +
> static void irq_domain_free_irq_data(unsigned int virq, unsigned int nr_irqs)
> {
> struct irq_data *irq_data, *tmp;
> @@ -1147,14 +1158,49 @@ static void irq_domain_free_irq_data(unsigned int virq, unsigned int nr_irqs)
> irq_data->parent_data = NULL;
> irq_data->domain = NULL;
>
> - while (tmp) {
> - irq_data = tmp;
> - tmp = tmp->parent_data;
> - kfree(irq_data);
> - }
> + __irq_domain_free_hierarchy(tmp);
> }
> }
>
> +/**
> + * irq_domain_trim_hierarchy - Trim the irq hierarchy from a particular
> + * irq domain
> + * @virq: IRQ number to trim where the hierarchy is to be trimmed
> + * @domain: domain from which the hierarchy gets discarded for this
> + * interrupt
> + *
> + * Drop the partial irq_data hierarchy from @domain (included) onward.
> + *
> + * This is only meant to be called from a .alloc() callback, when no
> + * actual mapping in the respective domains has been established yet.
> + * Its only use is to be able to trim levels of hierarchy that do not
> + * have any real meaning for this interrupt.
> + */
> +int irq_domain_trim_hierarchy(unsigned int virq, struct irq_domain *domain)
> +{
> + struct irq_data *tail, *irq_data = irq_get_irq_data(virq);
> +
> + /* It really needs to be a hierarchy, and not a single entry */
> + if (WARN_ON(!irq_data->parent_data))
> + return -EINVAL;
> +
> + /* Skip until we find the right domain */
> + while (irq_data->parent_data && irq_data->parent_data->domain != domain)
> + irq_data = irq_data->parent_data;
> +
> + /* The domain doesn't exist in the hierarchy, which is pretty bad */
> + if (WARN_ON(!irq_data->parent_data))
> + return -ENOENT;
> +
> + /* Sever the inner part of the hierarchy... */
> + tail = irq_data->parent_data;
> + irq_data->parent_data = NULL;
> + __irq_domain_free_hierarchy(tail);
This is butt ugly, really. Especially the use case where the tegra PMC
domain removes itself from the hierarchy from .alloc()
That said, I don't have a better idea either. Sigh...
Thanks,
tglx
