lkml.org 
[lkml]   [2022]   [Jul]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2 1/2] PCI: qcom: Add system PM support
    On Mon, Jul 04, 2022 at 10:14:06AM +0530, Manivannan Sadhasivam wrote:
    > On Thu, Jun 30, 2022 at 03:29:33PM +0530, Krishna Chaitanya Chundru wrote:
    > >
    > > 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.
    > >
    >
    > Okay, please mention this in the commit message.
    >
    > > >
    > > > > 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.
    > >
    >
    > Okay. The DBI access patch is not included in this series, so you should
    > mention it in the commit message.
    >

    Ah, it is the patch 2/2. Still you can mention the reasoning.

    Thanks,
    Mani

    > Thanks,
    > Mani
    >
    > > > > };
    > > > > #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
    > > > >
    >
    > --
    > மணிவண்ணன் சதாசிவம்

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

    \
     
     \ /
      Last update: 2022-07-04 06:49    [W:3.195 / U:0.120 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site