Re: [PATCH 1/5] PCI: intel-gw: Move interrupt enable to own function

Florian Eckert
2026-03-27T08:18:53Z

Hello Mani,

On 2026-03-26 15:00, Manivannan Sadhasivam wrote:
> On Tue, Mar 17, 2026 at 11:12:49AM +0100, Florian Eckert wrote:
>> To improve the readability of the code, move the interrupt enable
>> instructions to a separate function. That is already done for the
>> disable interrupt instruction.
>>
>> Signed-off-by: Florian Eckert <fe@dev.tdt.de>
>> ---
>> drivers/pci/controller/dwc/pcie-intel-gw.c | 11 ++++++++---
>> 1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c
>> b/drivers/pci/controller/dwc/pcie-intel-gw.c
>> index
>> c21906eced61896c8a8307dbd6b72d229f9a5c5f..3a85bd0ef1b7f9414ce19fe56d82a78e34e9b648
>> 100644
>> --- a/drivers/pci/controller/dwc/pcie-intel-gw.c
>> +++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
>> @@ -196,6 +196,13 @@ static void intel_pcie_device_rst_deassert(struct
>> intel_pcie *pcie)
>> gpiod_set_value_cansleep(pcie->reset_gpio, 0);
>> }
>>
>> +static void intel_pcie_core_irq_enable(struct intel_pcie *pcie)
>> +{
>> + pcie_app_wr(pcie, PCIE_APP_IRNEN, 0);
>> + pcie_app_wr(pcie, PCIE_APP_IRNCR, PCIE_APP_IRN_INT);
>> + pcie_app_wr(pcie, PCIE_APP_IRNEN, PCIE_APP_IRN_INT);
>
> I'm confused. Previous code changed PCIE_APP_IRNEN register by writing
> to
> PCIE_APP_IRN_INT field. But this function is now changing
> PCIE_APP_IRNEN and
> PCIE_APP_IRNCR registers.

First, all pending interrupts are cleared and disabled, just as this is
done
in the disable function 'intel_pcie_core_irq_disable()' [1].

After that, all relevant interrupts are enabled. The `PCIE_APP_IRNEN`
definition contains all the relevant interrupts that are of interest
[2].

As I unfortunately don’t have any documentation for this IP core, I
suspect
that the intention is to set the IP core for interrupt handling to a
specific
state.

Perhaps the problem was that the IP core did not reinitialize the
interrupt
register properly after a power cycle. In my view, it can’t do any harm
to
switch the interrupt register off and then on again to set the
Interrupts to
a specific state. They do the same in their SDK [4]. Maxlinear is the
only
company that uses this IP core.

- Florian

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/controller/dwc/pcie-intel-gw.c?h=master#n199
[2]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/controller/dwc/pcie-intel-gw.c?h=master#n52
[3]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/controller/dwc/pcie-intel-gw.c?h=master#n321
[4]