[PATCH 0/5] PCI: intel-gw: Fixes to make the driver working again

[PATCH 0/5] PCI: intel-gw: Fixes to make the driver working again

[Florian Eckert]

This patch series fixes the 'intel-gw' driver to work again with the
current pcie framework. The following changes are:

* Move interrupt 'enable' to its own function to improve readability.
* Enable clock for the PHY before PHY init call.
* Add missing 'start_link' callback that was added to the PCIe dwc
  framework.
* Read ATU base assignment from the DTS rather than specifying it in the
  source.
* Remove unused preprocessor define.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
---
Florian Eckert (5):
      PCI: intel-gw: Move interrupt enable to own function
      PCI: intel-gw: Enable clock before phy init
      PCI: intel-gw: Add start_link callback function
      PCI: intel-gw: Remove atu base assignment
      PCI: intel-gw: Remove unused define

 drivers/pci/controller/dwc/pcie-intel-gw.c | 47 +++++++++++++++---------------
 1 file changed, 24 insertions(+), 23 deletions(-)
---
base-commit: f338e77383789c0cae23ca3d48adcc5e9e137e3c
change-id: 20260317-pcie-intel-gw-50902113f9e1

Best regards,
-- 
Florian Eckert <fe@dev.tdt.de>

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

[Florian Eckert]

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);
+}
+
 static void intel_pcie_core_irq_disable(struct intel_pcie *pcie)
 {
 	pcie_app_wr(pcie, PCIE_APP_IRNEN, 0);
@@ -317,9 +324,7 @@ static int intel_pcie_host_setup(struct intel_pcie *pcie)
 	if (ret)
 		goto app_init_err;
 
-	/* Enable integrated interrupts */
-	pcie_app_wr_mask(pcie, PCIE_APP_IRNEN, PCIE_APP_IRN_INT,
-			 PCIE_APP_IRN_INT);
+	intel_pcie_core_irq_enable(pcie);
 
 	return 0;
 

-- 
2.47.3

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

[Manivannan Sadhasivam]

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.

- Mani

> +}
> +
>  static void intel_pcie_core_irq_disable(struct intel_pcie *pcie)
>  {
>  	pcie_app_wr(pcie, PCIE_APP_IRNEN, 0);
> @@ -317,9 +324,7 @@ static int intel_pcie_host_setup(struct intel_pcie *pcie)
>  	if (ret)
>  		goto app_init_err;
>  
> -	/* Enable integrated interrupts */
> -	pcie_app_wr_mask(pcie, PCIE_APP_IRNEN, PCIE_APP_IRN_INT,
> -			 PCIE_APP_IRN_INT);
> +	intel_pcie_core_irq_enable(pcie);
>  
>  	return 0;
>  
> 
> -- 
> 2.47.3
> 

-- 
மணிவண்ணன் சதாசிவம்

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

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

[Manivannan Sadhasivam]

On Fri, Mar 27, 2026 at 09:18:49AM +0100, Florian Eckert wrote:
> 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.
> 

Ok, fair enough. But you need to add the above info to the commit message
including the reference to driver snippet in Maxlinear repo.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

[PATCH 2/5] PCI: intel-gw: Enable clock before phy init

[Florian Eckert]

To ensure that the boot sequence is correct, the dwc pcie core clock must
be switched on before phy init call.

This changes are based on patched kernel sources of the MaxLinear SDK,
which can be found at https://github.com/maxlinear/linux

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
---
 drivers/pci/controller/dwc/pcie-intel-gw.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c b/drivers/pci/controller/dwc/pcie-intel-gw.c
index 3a85bd0ef1b7f9414ce19fe56d82a78e34e9b648..6110a8adb8732dbbd5e9e2db68a0606ccf032ae1 100644
--- a/drivers/pci/controller/dwc/pcie-intel-gw.c
+++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
@@ -292,13 +292,9 @@ static int intel_pcie_host_setup(struct intel_pcie *pcie)
 
 	intel_pcie_core_rst_assert(pcie);
 	intel_pcie_device_rst_assert(pcie);
-
-	ret = phy_init(pcie->phy);
-	if (ret)
-		return ret;
-
 	intel_pcie_core_rst_deassert(pcie);
 
+	/* Controller clock must be provided earlier than PHY */
 	ret = clk_prepare_enable(pcie->core_clk);
 	if (ret) {
 		dev_err(pcie->pci.dev, "Core clock enable failed: %d\n", ret);
@@ -307,13 +303,17 @@ static int intel_pcie_host_setup(struct intel_pcie *pcie)
 
 	pci->atu_base = pci->dbi_base + 0xC0000;
 
+	ret = phy_init(pcie->phy);
+	if (ret)
+		goto phy_err;
+
 	intel_pcie_ltssm_disable(pcie);
 	intel_pcie_link_setup(pcie);
 	intel_pcie_init_n_fts(pci);
 
 	ret = dw_pcie_setup_rc(&pci->pp);
 	if (ret)
-		goto app_init_err;
+		goto phy_err;
 
 	dw_pcie_upconfig_setup(pci);
 
@@ -322,13 +322,13 @@ static int intel_pcie_host_setup(struct intel_pcie *pcie)
 
 	ret = dw_pcie_wait_for_link(pci);
 	if (ret)
-		goto app_init_err;
+		goto phy_err;
 
 	intel_pcie_core_irq_enable(pcie);
 
 	return 0;
 
-app_init_err:
+phy_err:
 	clk_disable_unprepare(pcie->core_clk);
 clk_err:
 	intel_pcie_core_rst_assert(pcie);

-- 
2.47.3

Re: [PATCH 2/5] PCI: intel-gw: Enable clock before phy init

[Manivannan Sadhasivam]

On Tue, Mar 17, 2026 at 11:12:50AM +0100, Florian Eckert wrote:
> To ensure that the boot sequence is correct, the dwc pcie core clock must
> be switched on before phy init call.
> 
> This changes are based on patched kernel sources of the MaxLinear SDK,
> which can be found at https://github.com/maxlinear/linux
> 

How can we treat Maxlinear kernel source as a reference for Intel driver?
Atleast, you need to provide some info onto why this should be trusted as a
reference like used a product etc...

> Signed-off-by: Florian Eckert <fe@dev.tdt.de>
> ---
>  drivers/pci/controller/dwc/pcie-intel-gw.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c b/drivers/pci/controller/dwc/pcie-intel-gw.c
> index 3a85bd0ef1b7f9414ce19fe56d82a78e34e9b648..6110a8adb8732dbbd5e9e2db68a0606ccf032ae1 100644
> --- a/drivers/pci/controller/dwc/pcie-intel-gw.c
> +++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
> @@ -292,13 +292,9 @@ static int intel_pcie_host_setup(struct intel_pcie *pcie)
>  
>  	intel_pcie_core_rst_assert(pcie);
>  	intel_pcie_device_rst_assert(pcie);
> -
> -	ret = phy_init(pcie->phy);
> -	if (ret)
> -		return ret;
> -
>  	intel_pcie_core_rst_deassert(pcie);
>  
> +	/* Controller clock must be provided earlier than PHY */
>  	ret = clk_prepare_enable(pcie->core_clk);
>  	if (ret) {
>  		dev_err(pcie->pci.dev, "Core clock enable failed: %d\n", ret);
> @@ -307,13 +303,17 @@ static int intel_pcie_host_setup(struct intel_pcie *pcie)
>  
>  	pci->atu_base = pci->dbi_base + 0xC0000;
>  
> +	ret = phy_init(pcie->phy);
> +	if (ret)
> +		goto phy_err;

You just messed up the whole error path. This one should branch to
err_disable_core_clk that disables core_clk and deasserts reset.

> +
>  	intel_pcie_ltssm_disable(pcie);
>  	intel_pcie_link_setup(pcie);
>  	intel_pcie_init_n_fts(pci);
>  
>  	ret = dw_pcie_setup_rc(&pci->pp);
>  	if (ret)
> -		goto app_init_err;
> +		goto phy_err;

And this to err_phy_exit that does phy_exit().

>  
>  	dw_pcie_upconfig_setup(pci);
>  
> @@ -322,13 +322,13 @@ static int intel_pcie_host_setup(struct intel_pcie *pcie)
>  
>  	ret = dw_pcie_wait_for_link(pci);
>  	if (ret)
> -		goto app_init_err;
> +		goto phy_err;
>  
>  	intel_pcie_core_irq_enable(pcie);
>  
>  	return 0;
>  

Here, you should add shuffle phy_exit() accordingly.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH 2/5] PCI: intel-gw: Enable clock before phy init

[Florian Eckert]

Hello Mani,

Thanks for your review.

On 2026-03-26 17:08, Manivannan Sadhasivam wrote:
> On Tue, Mar 17, 2026 at 11:12:50AM +0100, Florian Eckert wrote:
>> To ensure that the boot sequence is correct, the dwc pcie core clock 
>> must
>> be switched on before phy init call.
>> 
>> This changes are based on patched kernel sources of the MaxLinear SDK,
>> which can be found at https://github.com/maxlinear/linux
>> 
> 
> How can we treat Maxlinear kernel source as a reference for Intel 
> driver?
> Atleast, you need to provide some info onto why this should be trusted 
> as a
> reference like used a product etc...

As far as I know, this driver is only used by Maxlinear’s URX851 and 
URX850
SoCs. However, the chip was originally developed by Intel when they
acquired Lantiq’s home networking division in 2015 [1] for this SoCs.
In 2020 the home network division was sold to Maxlinear [2].

Since then, Maxlinear has been responsible for the driver. However, 
their
SDK is outdated and based on kernel 5.15. Other than that, not much is
happening! Even the developers listed as maintainers can no longer be
reached. When it came to the patch set, the email couldn't be delivered 
to
the responsible developer 'Chuanhua Lei <lchuanhua@maxlinear.com>' 
either.
The email bounced back.

The company I work for is using the chip and is currently in the process 
of
extracting the key components from the SDK so that the SoC can work 
again with
a mainline kernel again.

[1] 
https://www.intc.com/news-events/press-releases/detail/364/intel-to-acquire-lantiq-advancing-the-connected-home
[2] 
https://investors.maxlinear.com/press-releases/detail/395/maxlinear-to-acquire-intels-home-gateway-platform

>> Signed-off-by: Florian Eckert <fe@dev.tdt.de>
>> ---
>>  drivers/pci/controller/dwc/pcie-intel-gw.c | 16 ++++++++--------
>>  1 file changed, 8 insertions(+), 8 deletions(-)
>> 
>> diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c 
>> b/drivers/pci/controller/dwc/pcie-intel-gw.c
>> index 
>> 3a85bd0ef1b7f9414ce19fe56d82a78e34e9b648..6110a8adb8732dbbd5e9e2db68a0606ccf032ae1 
>> 100644
>> --- a/drivers/pci/controller/dwc/pcie-intel-gw.c
>> +++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
>> @@ -292,13 +292,9 @@ static int intel_pcie_host_setup(struct 
>> intel_pcie *pcie)
>> 
>>  	intel_pcie_core_rst_assert(pcie);
>>  	intel_pcie_device_rst_assert(pcie);
>> -
>> -	ret = phy_init(pcie->phy);
>> -	if (ret)
>> -		return ret;
>> -
>>  	intel_pcie_core_rst_deassert(pcie);
>> 
>> +	/* Controller clock must be provided earlier than PHY */
>>  	ret = clk_prepare_enable(pcie->core_clk);
>>  	if (ret) {
>>  		dev_err(pcie->pci.dev, "Core clock enable failed: %d\n", ret);
>> @@ -307,13 +303,17 @@ static int intel_pcie_host_setup(struct 
>> intel_pcie *pcie)
>> 
>>  	pci->atu_base = pci->dbi_base + 0xC0000;
>> 
>> +	ret = phy_init(pcie->phy);
>> +	if (ret)
>> +		goto phy_err;
> 
> You just messed up the whole error path. This one should branch to
> err_disable_core_clk that disables core_clk and deasserts reset.
> 
>> +
>>  	intel_pcie_ltssm_disable(pcie);
>>  	intel_pcie_link_setup(pcie);
>>  	intel_pcie_init_n_fts(pci);
>> 
>>  	ret = dw_pcie_setup_rc(&pci->pp);
>>  	if (ret)
>> -		goto app_init_err;
>> +		goto phy_err;
> 
> And this to err_phy_exit that does phy_exit().
> 
>> 
>>  	dw_pcie_upconfig_setup(pci);
>> 
>> @@ -322,13 +322,13 @@ static int intel_pcie_host_setup(struct 
>> intel_pcie *pcie)
>> 
>>  	ret = dw_pcie_wait_for_link(pci);
>>  	if (ret)
>> -		goto app_init_err;
>> +		goto phy_err;
>> 
>>  	intel_pcie_core_irq_enable(pcie);
>> 
>>  	return 0;
>> 
> 
> Here, you should add shuffle phy_exit() accordingly.

I'll take another look at that.

Thanks

- Florian

Re: [PATCH 2/5] PCI: intel-gw: Enable clock before phy init

[Manivannan Sadhasivam]

On Fri, Mar 27, 2026 at 08:42:21AM +0100, Florian Eckert wrote:
> Hello Mani,
> 
> Thanks for your review.
> 
> On 2026-03-26 17:08, Manivannan Sadhasivam wrote:
> > On Tue, Mar 17, 2026 at 11:12:50AM +0100, Florian Eckert wrote:
> > > To ensure that the boot sequence is correct, the dwc pcie core clock
> > > must
> > > be switched on before phy init call.
> > > 
> > > This changes are based on patched kernel sources of the MaxLinear SDK,
> > > which can be found at https://github.com/maxlinear/linux
> > > 
> > 
> > How can we treat Maxlinear kernel source as a reference for Intel
> > driver?
> > Atleast, you need to provide some info onto why this should be trusted
> > as a
> > reference like used a product etc...
> 
> As far as I know, this driver is only used by Maxlinear’s URX851 and URX850
> SoCs. However, the chip was originally developed by Intel when they
> acquired Lantiq’s home networking division in 2015 [1] for this SoCs.
> In 2020 the home network division was sold to Maxlinear [2].
> 
> Since then, Maxlinear has been responsible for the driver. However, their
> SDK is outdated and based on kernel 5.15. Other than that, not much is
> happening! Even the developers listed as maintainers can no longer be
> reached. When it came to the patch set, the email couldn't be delivered to
> the responsible developer 'Chuanhua Lei <lchuanhua@maxlinear.com>' either.
> The email bounced back.
> 
> The company I work for is using the chip and is currently in the process of
> extracting the key components from the SDK so that the SoC can work again
> with
> a mainline kernel again.
> 

Ok, thanks for the background.

- Mani

> [1] https://www.intc.com/news-events/press-releases/detail/364/intel-to-acquire-lantiq-advancing-the-connected-home
> [2] https://investors.maxlinear.com/press-releases/detail/395/maxlinear-to-acquire-intels-home-gateway-platform
> 
> > > Signed-off-by: Florian Eckert <fe@dev.tdt.de>
> > > ---
> > >  drivers/pci/controller/dwc/pcie-intel-gw.c | 16 ++++++++--------
> > >  1 file changed, 8 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c
> > > b/drivers/pci/controller/dwc/pcie-intel-gw.c
> > > index 3a85bd0ef1b7f9414ce19fe56d82a78e34e9b648..6110a8adb8732dbbd5e9e2db68a0606ccf032ae1
> > > 100644
> > > --- a/drivers/pci/controller/dwc/pcie-intel-gw.c
> > > +++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
> > > @@ -292,13 +292,9 @@ static int intel_pcie_host_setup(struct
> > > intel_pcie *pcie)
> > > 
> > >  	intel_pcie_core_rst_assert(pcie);
> > >  	intel_pcie_device_rst_assert(pcie);
> > > -
> > > -	ret = phy_init(pcie->phy);
> > > -	if (ret)
> > > -		return ret;
> > > -
> > >  	intel_pcie_core_rst_deassert(pcie);
> > > 
> > > +	/* Controller clock must be provided earlier than PHY */
> > >  	ret = clk_prepare_enable(pcie->core_clk);
> > >  	if (ret) {
> > >  		dev_err(pcie->pci.dev, "Core clock enable failed: %d\n", ret);
> > > @@ -307,13 +303,17 @@ static int intel_pcie_host_setup(struct
> > > intel_pcie *pcie)
> > > 
> > >  	pci->atu_base = pci->dbi_base + 0xC0000;
> > > 
> > > +	ret = phy_init(pcie->phy);
> > > +	if (ret)
> > > +		goto phy_err;
> > 
> > You just messed up the whole error path. This one should branch to
> > err_disable_core_clk that disables core_clk and deasserts reset.
> > 
> > > +
> > >  	intel_pcie_ltssm_disable(pcie);
> > >  	intel_pcie_link_setup(pcie);
> > >  	intel_pcie_init_n_fts(pci);
> > > 
> > >  	ret = dw_pcie_setup_rc(&pci->pp);
> > >  	if (ret)
> > > -		goto app_init_err;
> > > +		goto phy_err;
> > 
> > And this to err_phy_exit that does phy_exit().
> > 
> > > 
> > >  	dw_pcie_upconfig_setup(pci);
> > > 
> > > @@ -322,13 +322,13 @@ static int intel_pcie_host_setup(struct
> > > intel_pcie *pcie)
> > > 
> > >  	ret = dw_pcie_wait_for_link(pci);
> > >  	if (ret)
> > > -		goto app_init_err;
> > > +		goto phy_err;
> > > 
> > >  	intel_pcie_core_irq_enable(pcie);
> > > 
> > >  	return 0;
> > > 
> > 
> > Here, you should add shuffle phy_exit() accordingly.
> 
> I'll take another look at that.
> 
> Thanks
> 
> - Florian

-- 
மணிவண்ணன் சதாசிவம்

[PATCH 3/5] PCI: intel-gw: Add start_link callback function

[Florian Eckert]

The pcie-intel-gw driver has no start_link callback function. This commit
adds the missing callback function so that the driver works again and does
not abort with an error during probing.

Fixes: c5097b9869a1 ("Revert "PCI: dwc: Wait for link up only if link is started"")
Fixes: da56a1bfbab5 ("PCI: dwc: Wait for link up only if link is started")
Signed-off-by: Florian Eckert <fe@dev.tdt.de>
---
 drivers/pci/controller/dwc/pcie-intel-gw.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c b/drivers/pci/controller/dwc/pcie-intel-gw.c
index 6110a8adb8732dbbd5e9e2db68a0606ccf032ae1..6bd25f8da605032bfdb97596fb3a1f6a03e88bfc 100644
--- a/drivers/pci/controller/dwc/pcie-intel-gw.c
+++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
@@ -285,6 +285,16 @@ static void intel_pcie_turn_off(struct intel_pcie *pcie)
 	pcie_rc_cfg_wr_mask(pcie, PCI_COMMAND, PCI_COMMAND_MEMORY, 0);
 }
 
+static int intel_pcie_start_link(struct dw_pcie *pci)
+{
+	struct intel_pcie *pcie = dev_get_drvdata(pci->dev);
+
+	intel_pcie_device_rst_deassert(pcie);
+	intel_pcie_ltssm_enable(pcie);
+
+	return 0;
+}
+
 static int intel_pcie_host_setup(struct intel_pcie *pcie)
 {
 	int ret;
@@ -310,20 +320,8 @@ static int intel_pcie_host_setup(struct intel_pcie *pcie)
 	intel_pcie_ltssm_disable(pcie);
 	intel_pcie_link_setup(pcie);
 	intel_pcie_init_n_fts(pci);
-
-	ret = dw_pcie_setup_rc(&pci->pp);
-	if (ret)
-		goto phy_err;
-
 	dw_pcie_upconfig_setup(pci);
 
-	intel_pcie_device_rst_deassert(pcie);
-	intel_pcie_ltssm_enable(pcie);
-
-	ret = dw_pcie_wait_for_link(pci);
-	if (ret)
-		goto phy_err;
-
 	intel_pcie_core_irq_enable(pcie);
 
 	return 0;
@@ -386,6 +384,7 @@ static int intel_pcie_rc_init(struct dw_pcie_rp *pp)
 }
 
 static const struct dw_pcie_ops intel_pcie_ops = {
+	.start_link = intel_pcie_start_link,
 };
 
 static const struct dw_pcie_host_ops intel_pcie_dw_ops = {

-- 
2.47.3

Re: [PATCH 3/5] PCI: intel-gw: Add start_link callback function

[Manivannan Sadhasivam]

On Tue, Mar 17, 2026 at 11:12:51AM +0100, Florian Eckert wrote:
> The pcie-intel-gw driver has no start_link callback function. This commit
> adds the missing callback function so that the driver works again and does
> not abort with an error during probing.
> 

So the driver was broken due to the below commits? If so, please share some
failure logs as well.

- Mani

> Fixes: c5097b9869a1 ("Revert "PCI: dwc: Wait for link up only if link is started"")
> Fixes: da56a1bfbab5 ("PCI: dwc: Wait for link up only if link is started")
> Signed-off-by: Florian Eckert <fe@dev.tdt.de>
> ---
>  drivers/pci/controller/dwc/pcie-intel-gw.c | 23 +++++++++++------------
>  1 file changed, 11 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c b/drivers/pci/controller/dwc/pcie-intel-gw.c
> index 6110a8adb8732dbbd5e9e2db68a0606ccf032ae1..6bd25f8da605032bfdb97596fb3a1f6a03e88bfc 100644
> --- a/drivers/pci/controller/dwc/pcie-intel-gw.c
> +++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
> @@ -285,6 +285,16 @@ static void intel_pcie_turn_off(struct intel_pcie *pcie)
>  	pcie_rc_cfg_wr_mask(pcie, PCI_COMMAND, PCI_COMMAND_MEMORY, 0);
>  }
>  
> +static int intel_pcie_start_link(struct dw_pcie *pci)
> +{
> +	struct intel_pcie *pcie = dev_get_drvdata(pci->dev);
> +
> +	intel_pcie_device_rst_deassert(pcie);
> +	intel_pcie_ltssm_enable(pcie);
> +
> +	return 0;
> +}
> +
>  static int intel_pcie_host_setup(struct intel_pcie *pcie)
>  {
>  	int ret;
> @@ -310,20 +320,8 @@ static int intel_pcie_host_setup(struct intel_pcie *pcie)
>  	intel_pcie_ltssm_disable(pcie);
>  	intel_pcie_link_setup(pcie);
>  	intel_pcie_init_n_fts(pci);
> -
> -	ret = dw_pcie_setup_rc(&pci->pp);
> -	if (ret)
> -		goto phy_err;
> -
>  	dw_pcie_upconfig_setup(pci);
>  
> -	intel_pcie_device_rst_deassert(pcie);
> -	intel_pcie_ltssm_enable(pcie);
> -
> -	ret = dw_pcie_wait_for_link(pci);
> -	if (ret)
> -		goto phy_err;
> -
>  	intel_pcie_core_irq_enable(pcie);
>  
>  	return 0;
> @@ -386,6 +384,7 @@ static int intel_pcie_rc_init(struct dw_pcie_rp *pp)
>  }
>  
>  static const struct dw_pcie_ops intel_pcie_ops = {
> +	.start_link = intel_pcie_start_link,
>  };
>  
>  static const struct dw_pcie_host_ops intel_pcie_dw_ops = {
> 
> -- 
> 2.47.3
> 

-- 
மணிவண்ணன் சதாசிவம்

[PATCH 4/5] PCI: intel-gw: Remove atu base assignment

[Florian Eckert]

In the current implementation, only one PCIe bridge is recognised. This
change removes the assignment of the ATU base address during host setup.
Instead, the ATU base address is read from the device tree. To do this,
the 'atu' range of the DTS entry must be changed for PCIe.

Old DTS entry for PCIe:
reg = <0xd1000000 0x1000>,
	<0xd3000000 0x20000>,
	<0xd0c41000.0x1000>;
reg-names = "dbi", "config", "app";

New DTS entry for PCIe
reg = <0xd1000000 0x1000>,
	<0xd10c0000 0x1000>,
	<0xd3000000 0x20000>,
	<0xd0c41000.0x1000>;
reg-names = "dbi", "atu", "config", "app";

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
---
 drivers/pci/controller/dwc/pcie-intel-gw.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c b/drivers/pci/controller/dwc/pcie-intel-gw.c
index 6bd25f8da605032bfdb97596fb3a1f6a03e88bfc..cec972d5aa9107d4708338bd7349415a31f0e688 100644
--- a/drivers/pci/controller/dwc/pcie-intel-gw.c
+++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
@@ -311,8 +311,6 @@ static int intel_pcie_host_setup(struct intel_pcie *pcie)
 		goto clk_err;
 	}
 
-	pci->atu_base = pci->dbi_base + 0xC0000;
-
 	ret = phy_init(pcie->phy);
 	if (ret)
 		goto phy_err;

-- 
2.47.3

Re: [PATCH 4/5] PCI: intel-gw: Remove atu base assignment

[Manivannan Sadhasivam]

On Tue, Mar 17, 2026 at 11:12:52AM +0100, Florian Eckert wrote:
> In the current implementation, only one PCIe bridge is recognised. This
> change removes the assignment of the ATU base address during host setup.
> Instead, the ATU base address is read from the device tree. To do this,
> the 'atu' range of the DTS entry must be changed for PCIe.
> 
> Old DTS entry for PCIe:
> reg = <0xd1000000 0x1000>,
> 	<0xd3000000 0x20000>,
> 	<0xd0c41000.0x1000>;
> reg-names = "dbi", "config", "app";
> 
> New DTS entry for PCIe
> reg = <0xd1000000 0x1000>,
> 	<0xd10c0000 0x1000>,
> 	<0xd3000000 0x20000>,
> 	<0xd0c41000.0x1000>;
> reg-names = "dbi", "atu", "config", "app";
> 

You just broke the DT backwards compatibility here. What if the driver is used
with an old DT that doesn't provide this 'atu' entry? The driver will break.
Moreover, this entry should be added to the DT binding first before any driver
change.

- Mani

> Signed-off-by: Florian Eckert <fe@dev.tdt.de>
> ---
>  drivers/pci/controller/dwc/pcie-intel-gw.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c b/drivers/pci/controller/dwc/pcie-intel-gw.c
> index 6bd25f8da605032bfdb97596fb3a1f6a03e88bfc..cec972d5aa9107d4708338bd7349415a31f0e688 100644
> --- a/drivers/pci/controller/dwc/pcie-intel-gw.c
> +++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
> @@ -311,8 +311,6 @@ static int intel_pcie_host_setup(struct intel_pcie *pcie)
>  		goto clk_err;
>  	}
>  
> -	pci->atu_base = pci->dbi_base + 0xC0000;
> -
>  	ret = phy_init(pcie->phy);
>  	if (ret)
>  		goto phy_err;
> 
> -- 
> 2.47.3
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH 4/5] PCI: intel-gw: Remove atu base assignment

[Florian Eckert]

On 2026-03-26 17:15, Manivannan Sadhasivam wrote:
> On Tue, Mar 17, 2026 at 11:12:52AM +0100, Florian Eckert wrote:
>> In the current implementation, only one PCIe bridge is recognised. 
>> This
>> change removes the assignment of the ATU base address during host 
>> setup.
>> Instead, the ATU base address is read from the device tree. To do 
>> this,
>> the 'atu' range of the DTS entry must be changed for PCIe.
>> 
>> Old DTS entry for PCIe:
>> reg = <0xd1000000 0x1000>,
>> 	<0xd3000000 0x20000>,
>> 	<0xd0c41000.0x1000>;
>> reg-names = "dbi", "config", "app";
>> 
>> New DTS entry for PCIe
>> reg = <0xd1000000 0x1000>,
>> 	<0xd10c0000 0x1000>,
>> 	<0xd3000000 0x20000>,
>> 	<0xd0c41000.0x1000>;
>> reg-names = "dbi", "atu", "config", "app";
>> 
> 
> You just broke the DT backwards compatibility here. What if the driver 
> is used
> with an old DT that doesn't provide this 'atu' entry? The driver will 
> break.
> Moreover, this entry should be added to the DT binding first before any 
> driver
> change.

When I looked at the driver’s history, I found a note in a commit that 
the driver
is only used internally by Maxlinear [1]. The maintainer
'Lei Chuan Hua <lchuanhua@maxlinear.com>' from Maxlinear was still 
active at
that time. He mentioned this on the LKML [2].

Therefore, backward compatibility shouldn’t be necessary.

Unfortunately, it’s already been three months since I last looked into 
this issue.
But as far as I can remember I have to read the 'atu' resource from DTS 
and so have
to remove this line so the driver does work again.

Because the resource is already initialized by the dwc core during the 
function
call 'dw_pcie_get_resources()'on line 141 [3]. This function is called 
in the DWC
host core [4]. This raises the question of why the driver is doing this 
itself at
all, when the core already handles it for us.

Additional, I have noticed that it seems to be bad practice to include 
this
resource information in the source code. This information should instead 
be
loaded via the DTS see commit [5].

If backwards compatibility isn't an issue, then the DWC core should 
handle
loading the 'atu' resource from the DTS.

Is that how you see it too?


Thanks for your time and review

- Florian

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/pci/controller/dwc?h=master&id=07ae413e169da3697e633dd4489db0d681a04460
[2] 
https://lore.kernel.org/all/BY3PR19MB507667CE7531D863E1E5F8AEBDD82@BY3PR19MB5076.namprd19.prod.outlook.com/
[3] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/controller/dwc/pcie-designware.c#n141
[4] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/controller/dwc/pcie-designware-host.c#n544
[5] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/Documentation/devicetree/bindings/pci?id=f500a2f1282750fb344ce535d78071cf1493efd1

>> Signed-off-by: Florian Eckert <fe@dev.tdt.de>
>> ---
>>  drivers/pci/controller/dwc/pcie-intel-gw.c | 2 --
>>  1 file changed, 2 deletions(-)
>> 
>> diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c 
>> b/drivers/pci/controller/dwc/pcie-intel-gw.c
>> index 
>> 6bd25f8da605032bfdb97596fb3a1f6a03e88bfc..cec972d5aa9107d4708338bd7349415a31f0e688 
>> 100644
>> --- a/drivers/pci/controller/dwc/pcie-intel-gw.c
>> +++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
>> @@ -311,8 +311,6 @@ static int intel_pcie_host_setup(struct intel_pcie 
>> *pcie)
>>  		goto clk_err;
>>  	}
>> 
>> -	pci->atu_base = pci->dbi_base + 0xC0000;
>> -
>>  	ret = phy_init(pcie->phy);
>>  	if (ret)
>>  		goto phy_err;
>> 
>> --
>> 2.47.3
>> 

Re: [PATCH 4/5] PCI: intel-gw: Remove atu base assignment

[Manivannan Sadhasivam]

On Fri, Mar 27, 2026 at 12:44:34PM +0100, Florian Eckert wrote:
> On 2026-03-26 17:15, Manivannan Sadhasivam wrote:
> > On Tue, Mar 17, 2026 at 11:12:52AM +0100, Florian Eckert wrote:
> > > In the current implementation, only one PCIe bridge is recognised.
> > > This
> > > change removes the assignment of the ATU base address during host
> > > setup.
> > > Instead, the ATU base address is read from the device tree. To do
> > > this,
> > > the 'atu' range of the DTS entry must be changed for PCIe.
> > > 
> > > Old DTS entry for PCIe:
> > > reg = <0xd1000000 0x1000>,
> > > 	<0xd3000000 0x20000>,
> > > 	<0xd0c41000.0x1000>;
> > > reg-names = "dbi", "config", "app";
> > > 
> > > New DTS entry for PCIe
> > > reg = <0xd1000000 0x1000>,
> > > 	<0xd10c0000 0x1000>,
> > > 	<0xd3000000 0x20000>,
> > > 	<0xd0c41000.0x1000>;
> > > reg-names = "dbi", "atu", "config", "app";
> > > 
> > 
> > You just broke the DT backwards compatibility here. What if the driver
> > is used
> > with an old DT that doesn't provide this 'atu' entry? The driver will
> > break.
> > Moreover, this entry should be added to the DT binding first before any
> > driver
> > change.
> 
> When I looked at the driver’s history, I found a note in a commit that the
> driver
> is only used internally by Maxlinear [1]. The maintainer
> 'Lei Chuan Hua <lchuanhua@maxlinear.com>' from Maxlinear was still active at
> that time. He mentioned this on the LKML [2].
> 
> Therefore, backward compatibility shouldn’t be necessary.
> 

Ok, fair enough. But you do need the DT binding change and you should justify
in the commit log of both binding and driver on why you are breaking ABI and why
it is OK.

> Unfortunately, it’s already been three months since I last looked into this
> issue.
> But as far as I can remember I have to read the 'atu' resource from DTS and
> so have
> to remove this line so the driver does work again.
> 
> Because the resource is already initialized by the dwc core during the
> function
> call 'dw_pcie_get_resources()'on line 141 [3]. This function is called in
> the DWC
> host core [4]. This raises the question of why the driver is doing this
> itself at

> all, when the core already handles it for us.
> 

dw_pcie_get_resources() got added recently. So this driver has been passing the
iATU address from its inception.

> Additional, I have noticed that it seems to be bad practice to include this
> resource information in the source code. This information should instead be
> loaded via the DTS see commit [5].
> 
> If backwards compatibility isn't an issue, then the DWC core should handle
> loading the 'atu' resource from the DTS.
> 

Yes, we should always avoid hardcoding the MMIO address in the driver and obtain
it from the hardware description like DT. But some DWC drivers do hardcode the
MMIO address as they often have resources like 'atu' at a fixed offset from the
DBI base address. And those drivers often ended up updating the offset based on
the IP version or compatible. So it is best to get this info from DT.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

[PATCH 5/5] PCI: intel-gw: Remove unused define

[Florian Eckert]

The C preprocessor define 'PCIE_APP_INTX_OFST' is not used in the sources
and can therefore be deleted.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
---
 drivers/pci/controller/dwc/pcie-intel-gw.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c b/drivers/pci/controller/dwc/pcie-intel-gw.c
index cec972d5aa9107d4708338bd7349415a31f0e688..1a62d4aed9b495d6cfa31f0e1c92f2b4ac6b09f4 100644
--- a/drivers/pci/controller/dwc/pcie-intel-gw.c
+++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
@@ -47,7 +47,6 @@
 #define PCIE_APP_IRN_INTD		BIT(16)
 #define PCIE_APP_IRN_MSG_LTR		BIT(18)
 #define PCIE_APP_IRN_SYS_ERR_RC		BIT(29)
-#define PCIE_APP_INTX_OFST		12
 
 #define PCIE_APP_IRN_INT \
 	(PCIE_APP_IRN_AER_REPORT | PCIE_APP_IRN_PME | \

-- 
2.47.3