Re: [PATCH] PCI: microchip: add missing chained_irq enter/exit calls

From: <Conor.Dooley@microchip.com>
To: <lorenzo.pieralisi@arm.com>
Cc: <helgaas@kernel.org>, <Daire.McNamara@microchip.com>,
	<bhelgaas@google.com>, <Cyril.Jean@microchip.com>,
	<david.abdurachmanov@gmail.com>, <linux-pci@vger.kernel.org>,
	<maz@kernel.org>, <robh@kernel.org>
Subject: Re: [PATCH] PCI: microchip: add missing chained_irq enter/exit calls
Date: Wed, 11 May 2022 12:52:25 +0000	[thread overview]
Message-ID: <565cdcee-bae9-48d8-2a8d-b5c54e0905d2@microchip.com> (raw)
In-Reply-To: <Ynuw2jiLB7+ZSd3z@lpieralisi>

On 11/05/2022 13:49, Lorenzo Pieralisi wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> "PCI: microchip: Add missing chained_irq_enter()/exit() calls"
> 
> Always capitalize the first word in the commit subject sentence
> and add brackets to functions calls.
> 
> On Wed, May 11, 2022 at 10:55:05AM +0100, Conor Dooley wrote:
>> Bjorn spotted that two of the chained irq handlers were missing their
> 
> It is clear in the Link/reported-by that Bjorn spotted it, it is a nit
> but I'd prefer the commit log to just explain what you are fixing
> rather than telling the background story that is already there in
> the Link: provided.
> 
> I can make these changes for you, just let me know.

Hey Lorenzo,
If you wish - go for it.
Thanks,
Conor.

> 
> Thanks,
> Lorenzo
> 
>> chained_irq_enter()/chained_irq_exit() calls, so add them in to avoid
>> potentially lost interrupts.
>>
>> Reported by: Bjorn Helgaas <bhelgaas@google.com>
>> Link: https://lore.kernel.org/linux-pci/87h76b8nxc.wl-maz@kernel.org
>> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
>> ---
>>   drivers/pci/controller/pcie-microchip-host.c | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/pci/controller/pcie-microchip-host.c b/drivers/pci/controller/pcie-microchip-host.c
>> index 29d8e81e4181..8175abed0f05 100644
>> --- a/drivers/pci/controller/pcie-microchip-host.c
>> +++ b/drivers/pci/controller/pcie-microchip-host.c
>> @@ -406,6 +406,7 @@ static void mc_pcie_enable_msi(struct mc_pcie *port, void __iomem *base)
>>   static void mc_handle_msi(struct irq_desc *desc)
>>   {
>>        struct mc_pcie *port = irq_desc_get_handler_data(desc);
>> +     struct irq_chip *chip = irq_desc_get_chip(desc);
>>        struct device *dev = port->dev;
>>        struct mc_msi *msi = &port->msi;
>>        void __iomem *bridge_base_addr =
>> @@ -414,6 +415,8 @@ static void mc_handle_msi(struct irq_desc *desc)
>>        u32 bit;
>>        int ret;
>>
>> +     chained_irq_enter(chip, desc);
>> +
>>        status = readl_relaxed(bridge_base_addr + ISTATUS_LOCAL);
>>        if (status & PM_MSI_INT_MSI_MASK) {
>>                status = readl_relaxed(bridge_base_addr + ISTATUS_MSI);
>> @@ -424,6 +427,8 @@ static void mc_handle_msi(struct irq_desc *desc)
>>                                                    bit);
>>                }
>>        }
>> +
>> +     chained_irq_exit(chip, desc);
>>   }
>>
>>   static void mc_msi_bottom_irq_ack(struct irq_data *data)
>> @@ -563,6 +568,7 @@ static int mc_allocate_msi_domains(struct mc_pcie *port)
>>   static void mc_handle_intx(struct irq_desc *desc)
>>   {
>>        struct mc_pcie *port = irq_desc_get_handler_data(desc);
>> +     struct irq_chip *chip = irq_desc_get_chip(desc);
>>        struct device *dev = port->dev;
>>        void __iomem *bridge_base_addr =
>>                port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
>> @@ -570,6 +576,8 @@ static void mc_handle_intx(struct irq_desc *desc)
>>        u32 bit;
>>        int ret;
>>
>> +     chained_irq_enter(chip, desc);
>> +
>>        status = readl_relaxed(bridge_base_addr + ISTATUS_LOCAL);
>>        if (status & PM_MSI_INT_INTX_MASK) {
>>                status &= PM_MSI_INT_INTX_MASK;
>> @@ -581,6 +589,8 @@ static void mc_handle_intx(struct irq_desc *desc)
>>                                                    bit);
>>                }
>>        }
>> +
>> +     chained_irq_exit(chip, desc);
>>   }
>>
>>   static void mc_ack_intx_irq(struct irq_data *data)
>> --
>> 2.36.1
>>