[PATCH kernel] powerpc/pci: Fix broken INTx configuration via OF

Bjorn Helgaas helgaas at kernel.org
Fri Feb 9 09:42:05 AEDT 2018


On Fri, Feb 09, 2018 at 09:21:43AM +1100, Benjamin Herrenschmidt wrote:
> On Thu, 2018-02-08 at 15:39 -0600, Bjorn Helgaas wrote:
> > I don't understand how this fix works.  We used to check the result of
> > of_irq_parse_and_map_pci() and entered the block if it was zero.
> > 
> > Now you enter the block if it is zero or less than zero, but:
> > 
> >   static int pci_read_irq_line(...)
> >   {
> >     unsigned int virq = 0;     /* unnecessarily initialized, BTW */
> > 
> >     virq = of_irq_parse_and_map_pci(pci_dev, 0, 0);
> >     if (virq <= 0) {
> >       ...
> > 
> > virq is unsigned, so "virq < 0" can never be true.  So how does this
> > change anything?
> 
> Yes it does:
> 
> So the unsigned thing is a second bug in the original patch that Alexey
> isn't fixing, we need to fix it too.
> 
> However, the actual bug Alexey is fixing is that we lost the actual
> value of virq. IE, without his fix, we test it for 0 but we don't
> actually return it if it's positive.

Ah, I see, the bug is that we discarded the non-zero virq value when
we actually need it.  I'm going to wait for a new patch with a
changelog that says that and doesn't test an unsigned value for < 0.

> So he fixes the normal case but there's still a bug in the error case,
> we need to make virq signed.

I looked through the of_irq_parse_and_map_pci() path and I do not see
a case where it can return a negative value.  It either returns zero
or one of these:

  virq = irq_find_mapping(...)
  virq = irq_create_mapping(...)

Both of these functions return unsigned values.


More information about the Linuxppc-dev mailing list