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

[Florian Eckert]

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@xxxxxxxxxx>
> > ---
> >  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