Messages in this thread | | | From | Huacai Chen <> | Date | Mon, 16 Aug 2021 11:19:58 +0800 | Subject | Re: [PATCH 3/9] irqchip/loongson-pch-pic: Add ACPI init support |
| |
Hi, Marc,
On Thu, Aug 12, 2021 at 9:28 PM Marc Zyngier <maz@kernel.org> wrote: > > On Thu, 12 Aug 2021 13:23:27 +0100, > Huacai Chen <chenhuacai@gmail.com> wrote: > > > [...] > > > > > > +struct fwnode_handle *pch_pic_acpi_init(struct fwnode_handle *parent, > > > > > + struct acpi_madt_bio_pic *acpi_pchpic) > > > > > +{ > > > > > + int count; > > > > > + struct pch_pic *priv; > > > > > + struct irq_domain *parent_domain; > > > > > + > > > > > + priv = kzalloc(sizeof(*priv), GFP_KERNEL); > > > > > + if (!priv) > > > > > + return NULL; > > > > > + > > > > > + raw_spin_lock_init(&priv->pic_lock); > > > > > + priv->base = ioremap(acpi_pchpic->address, acpi_pchpic->size); > > > > > + if (!priv->base) > > > > > + goto free_priv; > > > > > + > > > > > + priv->domain_handle = irq_domain_alloc_fwnode(priv->base); > > > > > + if (!priv->domain_handle) { > > > > > + pr_err("Unable to allocate domain handle\n"); > > > > > + goto iounmap_base; > > > > > + } > > > > > + > > > > > + priv->ht_vec_base = acpi_pchpic->gsi_base; > > > > > + count = ((readq(priv->base) >> 48) & 0xff) + 1; > > > > > + parent_domain = irq_find_matching_fwnode(parent, DOMAIN_BUS_ANY); > > > > > + if (!parent_domain) { > > > > > + pr_err("Failed to find the parent domain\n"); > > > > > + goto iounmap_base; > > > > > + } > > > > > + > > > > > + priv->pic_domain = irq_domain_create_hierarchy(parent_domain, 0, > > > > > + count, priv->domain_handle, > > > > > + &pch_pic_domain_ops, priv); > > > > > + > > > > > + if (!priv->pic_domain) { > > > > > + pr_err("Failed to create IRQ domain\n"); > > > > > + goto iounmap_base; > > > > > + } > > > > > + > > > > > + pch_pic_reset(priv); > > > > > + pch_pic_priv[nr_pch_pics++] = priv; > > > > > + > > > > > + register_syscore_ops(&pch_pic_syscore_ops); > > > > > + > > > > > + return priv->domain_handle; > > > > > + > > > > > +iounmap_base: > > > > > + iounmap(priv->base); > > > > > +free_priv: > > > > > + kfree(priv); > > > > > + > > > > > + return NULL; > > > > > +} > > > > > + > > > > > +#endif > > > > > > > > A lot of this code is common with its OF counterpart. How about making > > > > this logic common? > > > OK, let me think about. > > Though pch_pic_acpi_init() is similar to pch_pic_of_init(), but it is > > difficult to make a common function, because we cannot prepare > > everything before the common function. For example, ioremap() can be > > the common part, but pch_pic_acpi_init() should get the vector count > > after ioremap(). If we use an argument to distinguish the caller in > > the common function, the complexity increases and seems no benefits. > > All firmware implementations allocate private data structures, irq > domains, map MMIO regions, etc. All that can be common. We even have > APIs that deal with both firmware interfaces. > > As for the 'read the vector count from the HW', what does it have to > do with driving the HW using DT or ACPI? The HW doesn't *know*. If you > are conflating HW changes and firmware interfaces, then you have > already lost. OK, I know, thanks.
Huacai > > M. > > -- > Without deviation from the norm, progress is not possible.
| |