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