linux-kernel - Re: [PATCH 2/3] PCI: dwc: qcom: Set suspend

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Tue, 17 May 2022 20:41:34 +0530
From:   Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
To:     Bjorn Helgaas <helgaas@...nel.org>
Cc:     bhelgaas@...gle.com, lorenzo.pieralisi@....com, kbusch@...nel.org,
        hch@....de, linux-nvme@...ts.infradead.org,
        linux-pci@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        linux-kernel@...r.kernel.org, svarbanov@...sol.com,
        bjorn.andersson@...aro.org, axboe@...com,
        quic_vbadigan@...cinc.com, quic_krichai@...cinc.com,
        quic_nitirawa@...cinc.com, vidyas@...dia.com, sagi@...mberg.me
Subject: Re: [PATCH 2/3] PCI: dwc: qcom: Set suspend_poweroff flag for SC7280

On Mon, May 16, 2022 at 03:19:50PM -0500, Bjorn Helgaas wrote:
> On Fri, May 13, 2022 at 04:30:26PM +0530, Manivannan Sadhasivam wrote:
> > For aggressive power saving on SC7280 SoCs, the power for the PCI devices
> > will be taken off during system suspend. Hence, notify the same to the
> > PCI device drivers using "suspend_poweroff" flag so that the drivers can
> > prepare the PCI devices to handle the poweroff and recover them during
> > resume.
> 
> No doubt "power ... will be taken off during system suspend" is true,
> but this isn't very informative.  Is this a property of SC7280?  A
> choice made by the SC7280 driver?  Why is this not applicable to other
> systems?
> 

The SC7280's RPMh firmware is cutting off the PCIe power domain during system
suspend. And as I explained in previous patch, the RC driver itself may put the
devices in D3cold conditionally on this platform. The reason is to save power
as this chipset is being used in Chromebooks.

Thanks,
Mani

> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
> > ---
> >  drivers/pci/controller/dwc/pcie-qcom.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > index 6ab90891801d..4b0ad2827f8f 100644
> > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > @@ -199,6 +199,7 @@ struct qcom_pcie_cfg {
> >  	unsigned int has_ddrss_sf_tbu_clk:1;
> >  	unsigned int has_aggre0_clk:1;
> >  	unsigned int has_aggre1_clk:1;
> > +	unsigned int suspend_poweroff:1;
> >  };
> >  
> >  struct qcom_pcie {
> > @@ -1220,6 +1221,10 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
> >  	if (pcie->cfg->pipe_clk_need_muxing)
> >  		clk_set_parent(res->pipe_clk_src, res->ref_clk_src);
> >  
> > +	/* Indicate PCI device drivers that the power will be taken off during system suspend */
> > +	if (pcie->cfg->suspend_poweroff)
> > +		pci->pp.bridge->suspend_poweroff = true;
> > +
> >  	ret = clk_bulk_prepare_enable(res->num_clks, res->clks);
> >  	if (ret < 0)
> >  		goto err_disable_regulators;
> > @@ -1548,6 +1553,7 @@ static const struct qcom_pcie_cfg sc7280_cfg = {
> >  	.ops = &ops_1_9_0,
> >  	.has_tbu_clk = true,
> >  	.pipe_clk_need_muxing = true,
> > +	.suspend_poweroff = true,
> >  };
> >  
> >  static const struct dw_pcie_ops dw_pcie_ops = {
> > -- 
> > 2.25.1
> > 

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