On Thu, 2021-03-11 at 08:19 +0000, Marc Zyngier wrote: > On 2021-03-11 00:05, Pali Rohár wrote: > > On Wednesday 24 February 2021 14:11:30 Jianjun Wang wrote: > >> +static int mtk_msi_bottom_domain_alloc(struct irq_domain *domain, > >> + unsigned int virq, unsigned int nr_irqs, > >> + void *arg) > >> +{ > >> + struct mtk_pcie_port *port = domain->host_data; > >> + struct mtk_msi_set *msi_set; > >> + int i, hwirq, set_idx; > >> + > >> + mutex_lock(&port->lock); > >> + > >> + hwirq = bitmap_find_free_region(port->msi_irq_in_use, > >> PCIE_MSI_IRQS_NUM, > >> + order_base_2(nr_irqs)); > >> + > >> + mutex_unlock(&port->lock); > >> + > >> + if (hwirq < 0) > >> + return -ENOSPC; > >> + > >> + set_idx = hwirq / PCIE_MSI_IRQS_PER_SET; > >> + msi_set = &port->msi_sets[set_idx]; > >> + > >> + for (i = 0; i < nr_irqs; i++) > >> + irq_domain_set_info(domain, virq + i, hwirq + i, > >> + &mtk_msi_bottom_irq_chip, msi_set, > >> + handle_edge_irq, NULL, NULL); > >> + > >> + return 0; > >> +} > >> + > >> +static void mtk_msi_bottom_domain_free(struct irq_domain *domain, > >> + unsigned int virq, unsigned int nr_irqs) > >> +{ > >> + struct mtk_pcie_port *port = domain->host_data; > >> + struct irq_data *data = irq_domain_get_irq_data(domain, virq); > >> + > >> + mutex_lock(&port->lock); > >> + > >> + bitmap_clear(port->msi_irq_in_use, data->hwirq, nr_irqs); > > > > Marc, should not be there bitmap_release_region() with order_base_2()? > > > > bitmap_release_region(port->msi_irq_in_use, data->hwirq, > > order_base_2(nr_irqs)); > > > > Because mtk_msi_bottom_domain_alloc() is allocating > > order_base_2(nr_irqs) interrupts, not only nr_irqs. > > Indeed, good catch. I will fix it in the next version, thanks for your review. > > Thanks, > > M.