linux-kernel - Re: [PATCH v1 1/2] PCI: dwc: Fix resume failure if no EP is connected on some platforms

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20241016181829.lfgjtw76sm6pzgcz@thinkpad>
Date: Wed, 16 Oct 2024 23:48:29 +0530
From: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
To: Hongxing Zhu <hongxing.zhu@....com>
Cc: "jingoohan1@...il.com" <jingoohan1@...il.com>,
	"kwilczynski@...nel.org" <kwilczynski@...nel.org>,
	"bhelgaas@...gle.com" <bhelgaas@...gle.com>,
	"lpieralisi@...nel.org" <lpieralisi@...nel.org>,
	Frank Li <frank.li@....com>, "robh@...nel.org" <robh@...nel.org>,
	"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"imx@...ts.linux.dev" <imx@...ts.linux.dev>
Subject: Re: [PATCH v1 1/2] PCI: dwc: Fix resume failure if no EP is
 connected on some platforms

On Tue, Oct 08, 2024 at 08:25:32AM +0000, Hongxing Zhu wrote:
> > -----Original Message-----
> > From: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
> > Sent: 2024年10月3日 14:04
> > To: Hongxing Zhu <hongxing.zhu@....com>
> > Cc: jingoohan1@...il.com; kwilczynski@...nel.org; bhelgaas@...gle.com;
> > lpieralisi@...nel.org; Frank Li <frank.li@....com>; robh@...nel.org;
> > linux-pci@...r.kernel.org; linux-kernel@...r.kernel.org; imx@...ts.linux.dev
> > Subject: Re: [PATCH v1 1/2] PCI: dwc: Fix resume failure if no EP is connected on
> > some platforms
> > 
> > On Wed, Sep 25, 2024 at 01:48:36PM +0800, Richard Zhu wrote:
> > > The dw_pcie_suspend_noirq() function currently returns success
> > > directly if no endpoint (EP) device is connected. However, on some
> > > platforms, power loss occurs during suspend, causing dw_resume() to do
> > nothing in this case.
> > > This results in a system halt because the DWC controller is not
> > > initialized after power-on during resume.
> > >
> > > Change call to deinit() in suspend and init() at resume regardless of
> > 
> > s/Change call to/Call
> > 
> > > whether there are EP device connections or not. It is not harmful to
> > > perform deinit() and init() again for the no power-off case, and it
> > > keeps the code simple and consistent in logic.
> > >
> > > Fixes: 4774faf854f5 ("PCI: dwc: Implement generic suspend/resume
> > > functionality")
> > > Signed-off-by: Richard Zhu <hongxing.zhu@....com>
> > > Reviewed-by: Frank Li <Frank.Li@....com>
> > > ---
> > >  .../pci/controller/dwc/pcie-designware-host.c | 30
> > > +++++++++----------
> > >  1 file changed, 15 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c
> > > b/drivers/pci/controller/dwc/pcie-designware-host.c
> > > index a0822d5371bc..cb8c3c2bcc79 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > > @@ -933,23 +933,23 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci)
> > >  	if (dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKCTL) &
> > PCI_EXP_LNKCTL_ASPM_L1)
> > >  		return 0;
> > >
> > 
> > There is one more condition above. It checks whether the link is in L1ss state or
> > not and if it is, the just returns 0. Going by your case, if the power goes off during
> > suspend, then it will be an issue, right?
> > 
> Hi Manivannan:
> Thanks for your comments.
> Yes, you're right. It's a problem that power is off in suspend when link
>  is in L1ss.
> How about to issue another patch to fix this problem?
> Since this commit is verified to fix the resume failure when no EP is
>  connected. I'm not sure I can combine them together or not.
> 

Fine with me.

- Mani

> Best Regards
> Richard Zhu
> > > -	if (dw_pcie_get_ltssm(pci) <= DW_PCIE_LTSSM_DETECT_ACT)
> > > -		return 0;
> > > -
> > > -	if (pci->pp.ops->pme_turn_off)
> > > -		pci->pp.ops->pme_turn_off(&pci->pp);
> > > -	else
> > > -		ret = dw_pcie_pme_turn_off(pci);
> > > +	if (dw_pcie_get_ltssm(pci) > DW_PCIE_LTSSM_DETECT_ACT) {
> > > +		/* Only send out PME_TURN_OFF when PCIE link is up */
> > 
> > Move this comment above the 'if' condition.
> > 
> > - Mani
> > 
> > > +		if (pci->pp.ops->pme_turn_off)
> > > +			pci->pp.ops->pme_turn_off(&pci->pp);
> > > +		else
> > > +			ret = dw_pcie_pme_turn_off(pci);
> > >
> > > -	if (ret)
> > > -		return ret;
> > > +		if (ret)
> > > +			return ret;
> > >
> > > -	ret = read_poll_timeout(dw_pcie_get_ltssm, val, val ==
> > DW_PCIE_LTSSM_L2_IDLE,
> > > -				PCIE_PME_TO_L2_TIMEOUT_US/10,
> > > -				PCIE_PME_TO_L2_TIMEOUT_US, false, pci);
> > > -	if (ret) {
> > > -		dev_err(pci->dev, "Timeout waiting for L2 entry! LTSSM: 0x%x\n",
> > val);
> > > -		return ret;
> > > +		ret = read_poll_timeout(dw_pcie_get_ltssm, val, val ==
> > DW_PCIE_LTSSM_L2_IDLE,
> > > +					PCIE_PME_TO_L2_TIMEOUT_US/10,
> > > +					PCIE_PME_TO_L2_TIMEOUT_US, false, pci);
> > > +		if (ret) {
> > > +			dev_err(pci->dev, "Timeout waiting for L2 entry! LTSSM: 0x%x\n",
> > val);
> > > +			return ret;
> > > +		}
> > >  	}
> > >
> > >  	if (pci->pp.ops->deinit)
> > > --
> > > 2.37.1
> > >
> > 
> > --
> > மணிவண்ணன் சதாசிவம்

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