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@xxxxxxxxxx>
> > Reviewed-by: Marek Behún <kabel@xxxxxxxxxx>
> > Signed-off-by: Marek Behún <kabel@xxxxxxxxxx>
> > Cc: stable@xxxxxxxxxxxxxxx
> > ---
> > 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
> >
