linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Krishna Chaitanya Chundru <quic_krichai@quicinc.com>
To: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Cc: helgaas@kernel.org, linux-pci@vger.kernel.org,
	linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org,
	quic_vbadigan@quicinc.com, quic_hemantk@quicinc.com,
	quic_nitegupt@quicinc.com, quic_skananth@quicinc.com,
	quic_ramkri@quicinc.com, swboyd@chromium.org,
	dmitry.baryshkov@linaro.org,
	"Stanimir Varbanov" <svarbanov@mm-sol.com>,
	"Andy Gross" <agross@kernel.org>,
	"Bjorn Andersson" <bjorn.andersson@linaro.org>,
	"Lorenzo Pieralisi" <lorenzo.pieralisi@arm.com>,
	"Rob Herring" <robh@kernel.org>,
	"Krzysztof Wilczyński" <kw@linux.com>,
	"Bjorn Helgaas" <bhelgaas@google.com>
Subject: Re: [PATCH v2 1/2] PCI: qcom: Add system PM support
Date: Thu, 30 Jun 2022 15:29:33 +0530	[thread overview]
Message-ID: <4b1631f9-220f-c378-164c-f7eea9db22ef@quicinc.com> (raw)
In-Reply-To: <20220630043415.GA5012@thinkpad>


On 6/30/2022 10:04 AM, Manivannan Sadhasivam wrote:
> On Wed, Jun 29, 2022 at 03:03:33PM +0530, Krishna chaitanya chundru wrote:
>> Add suspend and resume pm callbacks.
>>
>> When system suspends, and if the link is in L1ss, disable the clocks
>> so that system enters into low power state to save the maximum power.
>> And when the system resumes, enable the clocks back if they are
>> disabled in the suspend path.
>>
> Why only during L1ss and not L2/L3?

with aspm the link will automatically go to L1ss. for L2/L3 entry we 
need to explicitly send

PME turn off which we are not doing now. So we are checking only for L1ss.

>
>> Changes since v1:
>> 	- Fixed compilation errors.
>>
>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
>> ---
>>   drivers/pci/controller/dwc/pcie-qcom.c | 81 ++++++++++++++++++++++++++++++++++
>>   1 file changed, 81 insertions(+)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
>> index 6ab9089..8e9ef37 100644
>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>> @@ -41,6 +41,9 @@
>>   #define L23_CLK_RMV_DIS				BIT(2)
>>   #define L1_CLK_RMV_DIS				BIT(1)
>>   
>> +#define PCIE20_PARF_PM_STTS                     0x24
>> +#define PCIE20_PARF_PM_STTS_LINKST_IN_L1SUB    BIT(8)
>> +
>>   #define PCIE20_PARF_PHY_CTRL			0x40
>>   #define PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK	GENMASK(20, 16)
>>   #define PHY_CTRL_PHY_TX0_TERM_OFFSET(x)		((x) << 16)
>> @@ -190,6 +193,8 @@ struct qcom_pcie_ops {
>>   	void (*post_deinit)(struct qcom_pcie *pcie);
>>   	void (*ltssm_enable)(struct qcom_pcie *pcie);
>>   	int (*config_sid)(struct qcom_pcie *pcie);
>> +	int (*enable_clks)(struct qcom_pcie *pcie);
>> +	int (*disable_clks)(struct qcom_pcie *pcie);
> I think these could vary between platforms. Like some other platform may try to
> disable regulators etc... So use names such as suspend and resume.
Sure will change in the next patch.
>>   };
>>   
>>   struct qcom_pcie_cfg {
>> @@ -199,6 +204,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 support_pm_ops:1;
>>   };
>>   
>>   struct qcom_pcie {
>> @@ -209,6 +215,7 @@ struct qcom_pcie {
>>   	struct phy *phy;
>>   	struct gpio_desc *reset;
>>   	const struct qcom_pcie_cfg *cfg;
>> +	unsigned int is_suspended:1;
> Why do you need this flag? Is suspend going to happen multiple times in
> an out-of-order manner?

We are using this flag in the resume function to check whether we 
suspended and disabled

the clocks in the suspend path. And we want to use this flag to control 
the access to dbi etc

after suspend.

>>   };
>>   
>>   #define to_qcom_pcie(x)		dev_get_drvdata((x)->dev)
>> @@ -1308,6 +1315,23 @@ static void qcom_pcie_post_deinit_2_7_0(struct qcom_pcie *pcie)
>>   	clk_disable_unprepare(res->pipe_clk);
>>   }
>>   
> [...]
>
>> +static const struct dev_pm_ops qcom_pcie_pm_ops = {
>> +	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(qcom_pcie_pm_suspend, qcom_pcie_pm_resume)
> Use the new macro: NOIRQ_SYSTEM_SLEEP_PM_OPS
Will update in the next patch.
>> +};
>> +
>>   static const struct of_device_id qcom_pcie_match[] = {
>>   	{ .compatible = "qcom,pcie-apq8084", .data = &apq8084_cfg },
>>   	{ .compatible = "qcom,pcie-ipq8064", .data = &ipq8064_cfg },
>> @@ -1679,6 +1759,7 @@ static struct platform_driver qcom_pcie_driver = {
>>   	.probe = qcom_pcie_probe,
>>   	.driver = {
>>   		.name = "qcom-pcie",
>> +		.pm = &qcom_pcie_pm_ops,
> There will be warnings when CONFIG_PM_SLEEP is not set. So use below,
will update in the next patch.
>
> 		.pm = pm_sleep_ptr(&qcom_pcie_pm_ops),
>
> Thanks,
> Mani
>
>>   		.suppress_bind_attrs = true,
>>   		.of_match_table = qcom_pcie_match,
>>   	},
>> -- 
>> 2.7.4
>>

  reply	other threads:[~2022-06-30  9:59 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-24  7:28 [PATCH v1 0/2] PCI: Restrict pci transactions after pci suspend Krishna chaitanya chundru
2022-06-24  7:28 ` [PATCH v1 1/2] PCI: qcom: Add system PM support Krishna chaitanya chundru
2022-06-24 18:31   ` Matthias Kaehlcke
2022-06-24  7:28 ` [PATCH v1 2/2] PCI: qcom: Restrict pci transactions after pci suspend Krishna chaitanya chundru
2022-06-29  9:33 ` [PATCH v2 0/2] PCI: " Krishna chaitanya chundru
2022-06-29  9:33   ` [PATCH v2 1/2] PCI: qcom: Add system PM support Krishna chaitanya chundru
2022-06-29 19:33     ` Matthias Kaehlcke
2022-06-30  4:34     ` Manivannan Sadhasivam
2022-06-30  9:59       ` Krishna Chaitanya Chundru [this message]
2022-07-04  4:44         ` Manivannan Sadhasivam
2022-07-04  4:48           ` Manivannan Sadhasivam
2022-06-29  9:33   ` [PATCH v2 2/2] PCI: qcom: Restrict pci transactions after pci suspend Krishna chaitanya chundru
2022-07-04  4:54     ` Manivannan Sadhasivam
2022-07-01 14:13   ` [PATCH v3 0/2] PCI: " Krishna chaitanya chundru
2022-07-01 14:13     ` [PATCH v3 1/2] PCI: qcom: Add system PM support Krishna chaitanya chundru
2022-07-01 14:13     ` [PATCH v3 2/2] PCI: qcom: Restrict pci transactions after pci suspend Krishna chaitanya chundru
2022-07-06 14:40     ` [PATCH v4 0/2] PCI: " Krishna chaitanya chundru
2022-07-06 14:40       ` [PATCH v4 1/2] PCI: qcom: Add system PM support Krishna chaitanya chundru
2022-07-11 21:53         ` Matthias Kaehlcke
2022-07-14 10:41           ` Krishna Chaitanya Chundru
2022-07-15 13:43         ` Johan Hovold
2022-07-06 14:40       ` [PATCH v4 2/2] PCI: qcom: Restrict pci transactions after pci suspend Krishna chaitanya chundru
2022-07-12  0:34         ` Matthias Kaehlcke
2022-07-15 13:51         ` Johan Hovold
2022-07-06 15:13       ` [PATCH v4 0/2] PCI: " Dmitry Baryshkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4b1631f9-220f-c378-164c-f7eea9db22ef@quicinc.com \
    --to=quic_krichai@quicinc.com \
    --cc=agross@kernel.org \
    --cc=bhelgaas@google.com \
    --cc=bjorn.andersson@linaro.org \
    --cc=dmitry.baryshkov@linaro.org \
    --cc=helgaas@kernel.org \
    --cc=kw@linux.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=manivannan.sadhasivam@linaro.org \
    --cc=quic_hemantk@quicinc.com \
    --cc=quic_nitegupt@quicinc.com \
    --cc=quic_ramkri@quicinc.com \
    --cc=quic_skananth@quicinc.com \
    --cc=quic_vbadigan@quicinc.com \
    --cc=robh@kernel.org \
    --cc=svarbanov@mm-sol.com \
    --cc=swboyd@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).