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

[Florian Eckert <fe@dev.tdt.de>]

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] 
https://github.com/maxlinear/linux/blob/updk_9.1.90/drivers/pci/controller/dwc/pcie-intel-gw.c#L431