| Message ID | 20211012164145.14126-3-kabel@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | PCI: aardvark controller fixes BATCH 2 | expand |
On Tue, Oct 12, 2021 at 06:41:33PM +0200, Marek Behún wrote: > MSI domain callback .alloc() (implemented by advk_msi_irq_domain_alloc() > function) should return zero on success, since non-zero value indicates > failure. AFAICS the .alloc() method is called in: irq_domain_alloc_irqs_hierarchy() which in turn is called by: __irq_domain_alloc_irqs() -> that checks (ret < 0) irq_domain_push_irq() -> that checks for rv != 0 irq_domain_alloc_irqs_parent() called by many drivers and also by msi_domain_alloc() (that checks ret < 0) This patch is fine, I am just asking, given the above: - How did you detect it (given that aardvark would not fail ret < 0) ? - Should we consolidate the .alloc() return value handling ? Apologies if I missed something in the IRQ domain code. Lorenzo > When the driver was converted to generic MSI API in commit f21a8b1b6837 > ("PCI: aardvark: Move to MSI handling using generic MSI support"), it > was converted so that it returns hwirq number. > > Fix this. > > Fixes: f21a8b1b6837 ("PCI: aardvark: Move to MSI handling using generic MSI support") > Signed-off-by: Pali Rohár <pali@kernel.org> > Reviewed-by: Marek Behún <kabel@kernel.org> > Signed-off-by: Marek Behún <kabel@kernel.org> > Cc: stable@vger.kernel.org > --- > drivers/pci/controller/pci-aardvark.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c > index 10476c00b312..b45ff2911c80 100644 > --- a/drivers/pci/controller/pci-aardvark.c > +++ b/drivers/pci/controller/pci-aardvark.c > @@ -1138,7 +1138,7 @@ static int advk_msi_irq_domain_alloc(struct irq_domain *domain, > domain->host_data, handle_simple_irq, > NULL, NULL); > > - return hwirq; > + return 0; > } > > static void advk_msi_irq_domain_free(struct irq_domain *domain, > -- > 2.32.0 >
On Wednesday 27 October 2021 12:26:53 Lorenzo Pieralisi wrote: > On Tue, Oct 12, 2021 at 06:41:33PM +0200, Marek Behún wrote: > > MSI domain callback .alloc() (implemented by advk_msi_irq_domain_alloc() > > function) should return zero on success, since non-zero value indicates > > failure. > > AFAICS the .alloc() method is called in: > > irq_domain_alloc_irqs_hierarchy() > > which in turn is called by: > > __irq_domain_alloc_irqs() -> that checks (ret < 0) > > irq_domain_push_irq() -> that checks for rv != 0 > > irq_domain_alloc_irqs_parent() called by many drivers and also > by msi_domain_alloc() (that checks ret < 0) > > This patch is fine, I am just asking, given the above: > > - How did you detect it (given that aardvark would not fail ret < 0) ? Last year we have detected that Multi-MSI interrupts do not work correctly and we have found that return value is incorrect during debugging. Other drivers are returning 0. > - Should we consolidate the .alloc() return value handling ? > > Apologies if I missed something in the IRQ domain code. > > Lorenzo > > > When the driver was converted to generic MSI API in commit f21a8b1b6837 > > ("PCI: aardvark: Move to MSI handling using generic MSI support"), it > > was converted so that it returns hwirq number. > > > > Fix this. > > > > Fixes: f21a8b1b6837 ("PCI: aardvark: Move to MSI handling using generic MSI support") > > Signed-off-by: Pali Rohár <pali@kernel.org> > > Reviewed-by: Marek Behún <kabel@kernel.org> > > Signed-off-by: Marek Behún <kabel@kernel.org> > > Cc: stable@vger.kernel.org > > --- > > drivers/pci/controller/pci-aardvark.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c > > index 10476c00b312..b45ff2911c80 100644 > > --- a/drivers/pci/controller/pci-aardvark.c > > +++ b/drivers/pci/controller/pci-aardvark.c > > @@ -1138,7 +1138,7 @@ static int advk_msi_irq_domain_alloc(struct irq_domain *domain, > > domain->host_data, handle_simple_irq, > > NULL, NULL); > > > > - return hwirq; > > + return 0; > > } > > > > static void advk_msi_irq_domain_free(struct irq_domain *domain, > > -- > > 2.32.0 > >
diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c index 10476c00b312..b45ff2911c80 100644 --- a/drivers/pci/controller/pci-aardvark.c +++ b/drivers/pci/controller/pci-aardvark.c @@ -1138,7 +1138,7 @@ static int advk_msi_irq_domain_alloc(struct irq_domain *domain, domain->host_data, handle_simple_irq, NULL, NULL); - return hwirq; + return 0; } static void advk_msi_irq_domain_free(struct irq_domain *domain,
MSI domain callback .alloc() (implemented by advk_msi_irq_domain_alloc() function) should return zero on success, since non-zero value indicates failure. When the driver was converted to generic MSI API in commit f21a8b1b6837 ("PCI: aardvark: Move to MSI handling using generic MSI support"), it was converted so that it returns hwirq number. Fix this. Fixes: f21a8b1b6837 ("PCI: aardvark: Move to MSI handling using generic MSI support") Signed-off-by: Pali Rohár <pali@kernel.org> Reviewed-by: Marek Behún <kabel@kernel.org> Signed-off-by: Marek Behún <kabel@kernel.org> Cc: stable@vger.kernel.org --- drivers/pci/controller/pci-aardvark.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)