Hi Lirenzo, On Thu, 14 Feb 2019 10:31:38 +0000, Lorenzo Pieralisi <lorenzo.pieralisi@xxxxxxx> wrote: > > [CC'ed MarcZ] > > On Thu, Feb 14, 2019 at 10:27:19AM +0530, Kishon Vijay Abraham I wrote: > > Hi Lorenzo, > > > > On 13/02/19 10:27 PM, Lorenzo Pieralisi wrote: > > > On Wed, Feb 13, 2019 at 06:56:23PM +0530, Kishon Vijay Abraham I wrote: > > >> ks_pcie_legacy_irq_handler() uses 'virq' to get the IRQ number offset. > > >> This offset is used to get the correct IRQ_STATUS register > > >> corresponding to the IRQ line that raised the interrupt. > > >> There is no guarantee that 'virq' assigned for consecutive hardware > > >> IRQ will be contiguous. And this might get us an incorrect IRQ number > > >> offset. > > >> > > >> Fix it here by using 'hwirq' to get the IRQ number offset. > > >> > > >> Link: https://lkml.kernel.org/r/bb081d21-7c03-0357-4294-7e92d95d838c@xxxxxxx > > >> Signed-off-by: Kishon Vijay Abraham I <kishon@xxxxxx> > > >> --- > > >> drivers/pci/controller/dwc/pci-keystone.c | 17 +++++++++++++---- > > >> 1 file changed, 13 insertions(+), 4 deletions(-) > > >> > > >> diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c > > >> index e8b1d8eca78e..d35ac712a9f8 100644 > > >> --- a/drivers/pci/controller/dwc/pci-keystone.c > > >> +++ b/drivers/pci/controller/dwc/pci-keystone.c > > >> @@ -87,7 +87,7 @@ struct keystone_pcie { > > >> struct dw_pcie *pci; > > >> /* PCI Device ID */ > > >> u32 device_id; > > >> - int legacy_host_irqs[PCI_NUM_INTX]; > > >> + int legacy_host_irq; > > >> struct device_node *legacy_intc_np; > > >> > > >> int msi_host_irqs[MAX_MSI_HOST_IRQS]; > > >> @@ -582,11 +582,11 @@ static void ks_pcie_msi_irq_handler(struct irq_desc *desc) > > >> */ > > >> static void ks_pcie_legacy_irq_handler(struct irq_desc *desc) > > >> { > > >> - unsigned int irq = irq_desc_get_irq(desc); > > >> + unsigned int irq = desc->irq_data.hwirq; > > >> struct keystone_pcie *ks_pcie = irq_desc_get_handler_data(desc); > > >> struct dw_pcie *pci = ks_pcie->pci; > > >> struct device *dev = pci->dev; > > >> - u32 irq_offset = irq - ks_pcie->legacy_host_irqs[0]; > > >> + u32 irq_offset = irq - ks_pcie->legacy_host_irq; > > > > > > I think you should use the plain hwirq number (that if I understand > > > correctly range in [0,3]) and drop legacy_host_irq. > > > > The hwirq is [80, 83] for Keystone. We store legacy_host_irq (for Keystone it > > is 80) to get the correct offset in the range [0, 3]. > > IIUC the _parent_ hw_irq number should be what you need here, Marc (if > he has time) can correct me if I am wrong (or what I am suggesting is > a violation of IRQ domain bstraction usage). It isn't pretty, but it is something we already do in some cases (see irq-gic-v3-mbi.c::mbi_compose_msi_msg as an infamous example). Thanks, M. -- Jazz is not dead, it just smell funny.
