* [PATCH 0/5] PCI: intel-gw: Fixes to make the driver working again
@ 2026-03-17 10:12 Florian Eckert
2026-03-17 10:12 ` [PATCH 1/5] PCI: intel-gw: Move interrupt enable to own function Florian Eckert
` (4 more replies)
0 siblings, 5 replies; 15+ messages in thread
From: Florian Eckert @ 2026-03-17 10:12 UTC (permalink / raw)
To: Chuanhua Lei, Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Johan Hovold,
Sajid Dalvi, Ajay Agarwal
Cc: linux-pci, linux-kernel, 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>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/5] PCI: intel-gw: Move interrupt enable to own function
2026-03-17 10:12 [PATCH 0/5] PCI: intel-gw: Fixes to make the driver working again Florian Eckert
@ 2026-03-17 10:12 ` Florian Eckert
2026-03-26 14:00 ` Manivannan Sadhasivam
2026-03-17 10:12 ` [PATCH 2/5] PCI: intel-gw: Enable clock before phy init Florian Eckert
` (3 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Florian Eckert @ 2026-03-17 10:12 UTC (permalink / raw)
To: Chuanhua Lei, Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Johan Hovold,
Sajid Dalvi, Ajay Agarwal
Cc: linux-pci, linux-kernel, 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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/5] PCI: intel-gw: Enable clock before phy init
2026-03-17 10:12 [PATCH 0/5] PCI: intel-gw: Fixes to make the driver working again Florian Eckert
2026-03-17 10:12 ` [PATCH 1/5] PCI: intel-gw: Move interrupt enable to own function Florian Eckert
@ 2026-03-17 10:12 ` Florian Eckert
2026-03-26 16:08 ` Manivannan Sadhasivam
2026-03-17 10:12 ` [PATCH 3/5] PCI: intel-gw: Add start_link callback function Florian Eckert
` (2 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Florian Eckert @ 2026-03-17 10:12 UTC (permalink / raw)
To: Chuanhua Lei, Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Johan Hovold,
Sajid Dalvi, Ajay Agarwal
Cc: linux-pci, linux-kernel, 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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/5] PCI: intel-gw: Add start_link callback function
2026-03-17 10:12 [PATCH 0/5] PCI: intel-gw: Fixes to make the driver working again Florian Eckert
2026-03-17 10:12 ` [PATCH 1/5] PCI: intel-gw: Move interrupt enable to own function Florian Eckert
2026-03-17 10:12 ` [PATCH 2/5] PCI: intel-gw: Enable clock before phy init Florian Eckert
@ 2026-03-17 10:12 ` Florian Eckert
2026-03-26 16:13 ` Manivannan Sadhasivam
2026-03-17 10:12 ` [PATCH 4/5] PCI: intel-gw: Remove atu base assignment Florian Eckert
2026-03-17 10:12 ` [PATCH 5/5] PCI: intel-gw: Remove unused define Florian Eckert
4 siblings, 1 reply; 15+ messages in thread
From: Florian Eckert @ 2026-03-17 10:12 UTC (permalink / raw)
To: Chuanhua Lei, Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Johan Hovold,
Sajid Dalvi, Ajay Agarwal
Cc: linux-pci, linux-kernel, 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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 4/5] PCI: intel-gw: Remove atu base assignment
2026-03-17 10:12 [PATCH 0/5] PCI: intel-gw: Fixes to make the driver working again Florian Eckert
` (2 preceding siblings ...)
2026-03-17 10:12 ` [PATCH 3/5] PCI: intel-gw: Add start_link callback function Florian Eckert
@ 2026-03-17 10:12 ` Florian Eckert
2026-03-26 16:15 ` Manivannan Sadhasivam
2026-03-17 10:12 ` [PATCH 5/5] PCI: intel-gw: Remove unused define Florian Eckert
4 siblings, 1 reply; 15+ messages in thread
From: Florian Eckert @ 2026-03-17 10:12 UTC (permalink / raw)
To: Chuanhua Lei, Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Johan Hovold,
Sajid Dalvi, Ajay Agarwal
Cc: linux-pci, linux-kernel, 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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 5/5] PCI: intel-gw: Remove unused define
2026-03-17 10:12 [PATCH 0/5] PCI: intel-gw: Fixes to make the driver working again Florian Eckert
` (3 preceding siblings ...)
2026-03-17 10:12 ` [PATCH 4/5] PCI: intel-gw: Remove atu base assignment Florian Eckert
@ 2026-03-17 10:12 ` Florian Eckert
4 siblings, 0 replies; 15+ messages in thread
From: Florian Eckert @ 2026-03-17 10:12 UTC (permalink / raw)
To: Chuanhua Lei, Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Johan Hovold,
Sajid Dalvi, Ajay Agarwal
Cc: linux-pci, linux-kernel, 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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/5] PCI: intel-gw: Move interrupt enable to own function
2026-03-17 10:12 ` [PATCH 1/5] PCI: intel-gw: Move interrupt enable to own function Florian Eckert
@ 2026-03-26 14:00 ` Manivannan Sadhasivam
2026-03-27 8:18 ` Florian Eckert
0 siblings, 1 reply; 15+ messages in thread
From: Manivannan Sadhasivam @ 2026-03-26 14:00 UTC (permalink / raw)
To: Florian Eckert
Cc: Chuanhua Lei, Lorenzo Pieralisi, Krzysztof Wilczyński,
Rob Herring, Bjorn Helgaas, Johan Hovold, Sajid Dalvi,
Ajay Agarwal, linux-pci, linux-kernel
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
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/5] PCI: intel-gw: Enable clock before phy init
2026-03-17 10:12 ` [PATCH 2/5] PCI: intel-gw: Enable clock before phy init Florian Eckert
@ 2026-03-26 16:08 ` Manivannan Sadhasivam
2026-03-27 7:42 ` Florian Eckert
0 siblings, 1 reply; 15+ messages in thread
From: Manivannan Sadhasivam @ 2026-03-26 16:08 UTC (permalink / raw)
To: Florian Eckert
Cc: Chuanhua Lei, Lorenzo Pieralisi, Krzysztof Wilczyński,
Rob Herring, Bjorn Helgaas, Johan Hovold, Sajid Dalvi,
Ajay Agarwal, linux-pci, linux-kernel
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
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/5] PCI: intel-gw: Add start_link callback function
2026-03-17 10:12 ` [PATCH 3/5] PCI: intel-gw: Add start_link callback function Florian Eckert
@ 2026-03-26 16:13 ` Manivannan Sadhasivam
0 siblings, 0 replies; 15+ messages in thread
From: Manivannan Sadhasivam @ 2026-03-26 16:13 UTC (permalink / raw)
To: Florian Eckert
Cc: Chuanhua Lei, Lorenzo Pieralisi, Krzysztof Wilczyński,
Rob Herring, Bjorn Helgaas, Johan Hovold, Sajid Dalvi,
Ajay Agarwal, linux-pci, linux-kernel
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
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/5] PCI: intel-gw: Remove atu base assignment
2026-03-17 10:12 ` [PATCH 4/5] PCI: intel-gw: Remove atu base assignment Florian Eckert
@ 2026-03-26 16:15 ` Manivannan Sadhasivam
2026-03-27 11:44 ` Florian Eckert
0 siblings, 1 reply; 15+ messages in thread
From: Manivannan Sadhasivam @ 2026-03-26 16:15 UTC (permalink / raw)
To: Florian Eckert
Cc: Chuanhua Lei, Lorenzo Pieralisi, Krzysztof Wilczyński,
Rob Herring, Bjorn Helgaas, Johan Hovold, Sajid Dalvi,
Ajay Agarwal, linux-pci, linux-kernel
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
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/5] PCI: intel-gw: Enable clock before phy init
2026-03-26 16:08 ` Manivannan Sadhasivam
@ 2026-03-27 7:42 ` Florian Eckert
2026-03-27 10:02 ` Manivannan Sadhasivam
0 siblings, 1 reply; 15+ messages in thread
From: Florian Eckert @ 2026-03-27 7:42 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Chuanhua Lei, Lorenzo Pieralisi, Krzysztof Wilczyński,
Rob Herring, Bjorn Helgaas, Johan Hovold, Sajid Dalvi,
Ajay Agarwal, linux-pci, linux-kernel
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/5] PCI: intel-gw: Move interrupt enable to own function
2026-03-26 14:00 ` Manivannan Sadhasivam
@ 2026-03-27 8:18 ` Florian Eckert
2026-03-27 10:00 ` Manivannan Sadhasivam
0 siblings, 1 reply; 15+ messages in thread
From: Florian Eckert @ 2026-03-27 8:18 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Chuanhua Lei, Lorenzo Pieralisi, Krzysztof Wilczyński,
Rob Herring, Bjorn Helgaas, Johan Hovold, Sajid Dalvi,
Ajay Agarwal, linux-pci, linux-kernel
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/5] PCI: intel-gw: Move interrupt enable to own function
2026-03-27 8:18 ` Florian Eckert
@ 2026-03-27 10:00 ` Manivannan Sadhasivam
0 siblings, 0 replies; 15+ messages in thread
From: Manivannan Sadhasivam @ 2026-03-27 10:00 UTC (permalink / raw)
To: Florian Eckert
Cc: Chuanhua Lei, Lorenzo Pieralisi, Krzysztof Wilczyński,
Rob Herring, Bjorn Helgaas, Johan Hovold, Sajid Dalvi,
Ajay Agarwal, linux-pci, linux-kernel
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
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/5] PCI: intel-gw: Enable clock before phy init
2026-03-27 7:42 ` Florian Eckert
@ 2026-03-27 10:02 ` Manivannan Sadhasivam
0 siblings, 0 replies; 15+ messages in thread
From: Manivannan Sadhasivam @ 2026-03-27 10:02 UTC (permalink / raw)
To: Florian Eckert
Cc: Chuanhua Lei, Lorenzo Pieralisi, Krzysztof Wilczyński,
Rob Herring, Bjorn Helgaas, Johan Hovold, Sajid Dalvi,
Ajay Agarwal, linux-pci, linux-kernel
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
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/5] PCI: intel-gw: Remove atu base assignment
2026-03-26 16:15 ` Manivannan Sadhasivam
@ 2026-03-27 11:44 ` Florian Eckert
0 siblings, 0 replies; 15+ messages in thread
From: Florian Eckert @ 2026-03-27 11:44 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Chuanhua Lei, Lorenzo Pieralisi, Krzysztof Wilczyński,
Rob Herring, Bjorn Helgaas, Johan Hovold, Sajid Dalvi,
Ajay Agarwal, linux-pci, linux-kernel
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
>>
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2026-03-27 11:44 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-17 10:12 [PATCH 0/5] PCI: intel-gw: Fixes to make the driver working again Florian Eckert
2026-03-17 10:12 ` [PATCH 1/5] PCI: intel-gw: Move interrupt enable to own function Florian Eckert
2026-03-26 14:00 ` Manivannan Sadhasivam
2026-03-27 8:18 ` Florian Eckert
2026-03-27 10:00 ` Manivannan Sadhasivam
2026-03-17 10:12 ` [PATCH 2/5] PCI: intel-gw: Enable clock before phy init Florian Eckert
2026-03-26 16:08 ` Manivannan Sadhasivam
2026-03-27 7:42 ` Florian Eckert
2026-03-27 10:02 ` Manivannan Sadhasivam
2026-03-17 10:12 ` [PATCH 3/5] PCI: intel-gw: Add start_link callback function Florian Eckert
2026-03-26 16:13 ` Manivannan Sadhasivam
2026-03-17 10:12 ` [PATCH 4/5] PCI: intel-gw: Remove atu base assignment Florian Eckert
2026-03-26 16:15 ` Manivannan Sadhasivam
2026-03-27 11:44 ` Florian Eckert
2026-03-17 10:12 ` [PATCH 5/5] PCI: intel-gw: Remove unused define Florian Eckert
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox