| Message ID | 20180209024125.38862-1-aik@ozlabs.ru (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
Alexey Kardashevskiy <aik@ozlabs.ru> writes: > Commit 59f47eff03a0 ("powerpc/pci: Use of_irq_parse_and_map_pci() helper") > replaced of_irq_parse_pci() + irq_create_of_mapping() with > of_irq_parse_and_map_pci() but this change lost virq returned by > irq_create_of_mapping() so virq remained zero causing INTx > misconfiguration. > > This fixes pci_read_irq_line() not to loose a virq returned by > of_irq_parse_and_map_pci(). > > Fixes: 59f47eff03a0 "powerpc/pci: Use of_irq_parse_and_map_pci() helper" > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > --- > Changes: > v2: > * changed the condition from <=0 to !=0 as by design > of_irq_parse_and_map_pci() can only return 0 for an error and virq>0. It returns int, so you should store the result in an int, and check the result for <= 0. Otherwise if it starts returning a negative error value this code will break. If it's a problem that of_irq_parse_and_map_pci() might internally convert an unsigned virq to int then we need to fix that there, or make of_irq_parse_and_map_pci() return unsigned also. cheers > diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c > index ae2ede4..33580a9 100644 > --- a/arch/powerpc/kernel/pci-common.c > +++ b/arch/powerpc/kernel/pci-common.c > @@ -362,7 +362,7 @@ struct pci_controller* pci_find_hose_for_OF_device(struct device_node* node) > */ > static int pci_read_irq_line(struct pci_dev *pci_dev) > { > - unsigned int virq = 0; > + unsigned int virq; > > pr_debug("PCI: Try to map irq for %s...\n", pci_name(pci_dev)); > > @@ -370,7 +370,8 @@ static int pci_read_irq_line(struct pci_dev *pci_dev) > memset(&oirq, 0xff, sizeof(oirq)); > #endif > /* Try to get a mapping from the device-tree */ > - if (!of_irq_parse_and_map_pci(pci_dev, 0, 0)) { > + virq = of_irq_parse_and_map_pci(pci_dev, 0, 0); > + if (!virq) { > u8 line, pin; > > /* If that fails, lets fallback to what is in the config > -- > 2.11.0
On Thu, Feb 8, 2018 at 11:54 PM, Michael Ellerman <mpe@ellerman.id.au> wrote: > Alexey Kardashevskiy <aik@ozlabs.ru> writes: > >> Commit 59f47eff03a0 ("powerpc/pci: Use of_irq_parse_and_map_pci() helper") >> replaced of_irq_parse_pci() + irq_create_of_mapping() with >> of_irq_parse_and_map_pci() but this change lost virq returned by >> irq_create_of_mapping() so virq remained zero causing INTx >> misconfiguration. >> >> This fixes pci_read_irq_line() not to loose a virq returned by >> of_irq_parse_and_map_pci(). >> >> Fixes: 59f47eff03a0 "powerpc/pci: Use of_irq_parse_and_map_pci() helper" >> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> >> --- >> Changes: >> v2: >> * changed the condition from <=0 to !=0 as by design >> of_irq_parse_and_map_pci() can only return 0 for an error and virq>0. > > It returns int, so you should store the result in an int, and check the > result for <= 0. > > Otherwise if it starts returning a negative error value this code will > break. That won't happen until we're confident all callers expect NO_IRQ is only 0 and don't treat negative as NO_IRQ. I expect that to be never, but maybe of_irq_parse_and_map_pci calls can be audited more easily than of_irq_parse_and_map. We ended up with of_irq_get() because we need to return error codes. Rob
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c index ae2ede4..33580a9 100644 --- a/arch/powerpc/kernel/pci-common.c +++ b/arch/powerpc/kernel/pci-common.c @@ -362,7 +362,7 @@ struct pci_controller* pci_find_hose_for_OF_device(struct device_node* node) */ static int pci_read_irq_line(struct pci_dev *pci_dev) { - unsigned int virq = 0; + unsigned int virq; pr_debug("PCI: Try to map irq for %s...\n", pci_name(pci_dev)); @@ -370,7 +370,8 @@ static int pci_read_irq_line(struct pci_dev *pci_dev) memset(&oirq, 0xff, sizeof(oirq)); #endif /* Try to get a mapping from the device-tree */ - if (!of_irq_parse_and_map_pci(pci_dev, 0, 0)) { + virq = of_irq_parse_and_map_pci(pci_dev, 0, 0); + if (!virq) { u8 line, pin; /* If that fails, lets fallback to what is in the config
Commit 59f47eff03a0 ("powerpc/pci: Use of_irq_parse_and_map_pci() helper") replaced of_irq_parse_pci() + irq_create_of_mapping() with of_irq_parse_and_map_pci() but this change lost virq returned by irq_create_of_mapping() so virq remained zero causing INTx misconfiguration. This fixes pci_read_irq_line() not to loose a virq returned by of_irq_parse_and_map_pci(). Fixes: 59f47eff03a0 "powerpc/pci: Use of_irq_parse_and_map_pci() helper" Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> --- Changes: v2: * changed the condition from <=0 to !=0 as by design of_irq_parse_and_map_pci() can only return 0 for an error and virq>0. --- arch/powerpc/kernel/pci-common.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)