From: Bjorn Helgaas <helgaas@kernel.org> To: Krishna Chaitanya Chundru <quic_krichai@quicinc.com> Cc: linux-pci@vger.kernel.org, linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, mka@chromium.org, quic_vbadigan@quicinc.com, quic_hemantk@quicinc.com, quic_nitegupt@quicinc.com, quic_skananth@quicinc.com, quic_ramkri@quicinc.com, manivannan.sadhasivam@linaro.org, swboyd@chromium.org, dmitry.baryshkov@linaro.org, svarbanov@mm-sol.com, agross@kernel.org, andersson@kernel.org, konrad.dybcio@somainline.org, lpieralisi@kernel.org, robh@kernel.org, kw@linux.com, bhelgaas@google.com, linux-phy@lists.infradead.org, vkoul@kernel.org, kishon@ti.com, mturquette@baylibre.com, linux-clk@vger.kernel.org, Bjorn Andersson <bjorn.andersson@linaro.org>, "Rafael J. Wysocki" <rafael@kernel.org>, linux-pm@vger.kernel.org, Marc Zyngier <maz@kernel.org>, Kevin Rowland <kevin.p.rowland@gmail.com> Subject: Re: [PATCH v7 1/5] PCI: qcom: Add system suspend and resume support Date: Wed, 12 Oct 2022 19:44:44 -0500 [thread overview] Message-ID: <20221013004444.GA3135316@bhelgaas> (raw) In-Reply-To: <04ace1ae-26d0-4157-b7eb-8dff29895180@quicinc.com> [+cc Marc, Kevin] On Wed, Oct 12, 2022 at 07:36:52PM +0530, Krishna Chaitanya Chundru wrote: > On 10/6/2022 2:43 AM, Bjorn Helgaas wrote: [I'm declaring quote text bankruptcy and dropping the huge wall of text. The IRQ affinity change you mention seems to be the critical issue :)] > > The PCIe spec clearly envisions Refclk being turned off > > (sec 5.5.3.3.1) and PHYs being powered off (sec 5.5.3.2) while in > > L1.2. > > > > I've been assuming L1.2 exit (which includes Refclk being turned on > > and PHYs being powered up) is completely handled by hardware, but it > > sounds like the Qcom controller needs software assistance which fields > > an interrupt when CLKREQ# is asserted and turns on Refclk and the > > PHYs? > > > > 5.5.3 does say "All Link and PHY state must be maintained during L1.2, > > or must be restored upon exit using implementation specific means", > > and maybe Qcom counts as using implementation specific means. > > > > I *am* concerned about whether software can do the L1.2 exit fast > > enough, but the biggest reason I'm struggling with this is because > > using the syscore framework to work around IRQ affinity changes that > > happen late in suspend just seems kind of kludgy and it doesn't seem > > like it fits cleanly in the power management model. > > Can you please suggest any another way to work around IRQ affinity > changes. One of your earlier patches [1] made dw_msi_mask_irq() look like this: static void dw_msi_mask_irq(struct irq_data *d) { struct pcie_port *pp = irq_data_get_irq_chip_data(d->parent_data); struct dw_pcie *pci = to_dw_pcie_from_pp(pp); if (dw_pcie_link_up(pci)) pci_msi_mask_irq(d); irq_chip_mask_parent(d); } That was an awful lot like Marc's suggestion [2] that the pci_msi_mask_irq() should be redundant. If it's truly redundant, maybe pci_msi_mask_irq() can be removed from dw_msi_mask_irq() (and other similar *_mask_irq() implementations) completely? Bjorn [1] https://lore.kernel.org/r/1659526134-22978-3-git-send-email-quic_krichai@quicinc.com [2] https://lore.kernel.org/linux-pci/86k05m7dkr.wl-maz@kernel.org/ -- linux-phy mailing list linux-phy@lists.infradead.org https://lists.infradead.org/mailman/listinfo/linux-phy
WARNING: multiple messages have this Message-ID (diff)
From: Bjorn Helgaas <helgaas@kernel.org> To: Krishna Chaitanya Chundru <quic_krichai@quicinc.com> Cc: linux-pci@vger.kernel.org, linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, mka@chromium.org, quic_vbadigan@quicinc.com, quic_hemantk@quicinc.com, quic_nitegupt@quicinc.com, quic_skananth@quicinc.com, quic_ramkri@quicinc.com, manivannan.sadhasivam@linaro.org, swboyd@chromium.org, dmitry.baryshkov@linaro.org, svarbanov@mm-sol.com, agross@kernel.org, andersson@kernel.org, konrad.dybcio@somainline.org, lpieralisi@kernel.org, robh@kernel.org, kw@linux.com, bhelgaas@google.com, linux-phy@lists.infradead.org, vkoul@kernel.org, kishon@ti.com, mturquette@baylibre.com, linux-clk@vger.kernel.org, Bjorn Andersson <bjorn.andersson@linaro.org>, "Rafael J. Wysocki" <rafael@kernel.org>, linux-pm@vger.kernel.org, Marc Zyngier <maz@kernel.org>, Kevin Rowland <kevin.p.rowland@gmail.com> Subject: Re: [PATCH v7 1/5] PCI: qcom: Add system suspend and resume support Date: Wed, 12 Oct 2022 19:44:44 -0500 [thread overview] Message-ID: <20221013004444.GA3135316@bhelgaas> (raw) In-Reply-To: <04ace1ae-26d0-4157-b7eb-8dff29895180@quicinc.com> [+cc Marc, Kevin] On Wed, Oct 12, 2022 at 07:36:52PM +0530, Krishna Chaitanya Chundru wrote: > On 10/6/2022 2:43 AM, Bjorn Helgaas wrote: [I'm declaring quote text bankruptcy and dropping the huge wall of text. The IRQ affinity change you mention seems to be the critical issue :)] > > The PCIe spec clearly envisions Refclk being turned off > > (sec 5.5.3.3.1) and PHYs being powered off (sec 5.5.3.2) while in > > L1.2. > > > > I've been assuming L1.2 exit (which includes Refclk being turned on > > and PHYs being powered up) is completely handled by hardware, but it > > sounds like the Qcom controller needs software assistance which fields > > an interrupt when CLKREQ# is asserted and turns on Refclk and the > > PHYs? > > > > 5.5.3 does say "All Link and PHY state must be maintained during L1.2, > > or must be restored upon exit using implementation specific means", > > and maybe Qcom counts as using implementation specific means. > > > > I *am* concerned about whether software can do the L1.2 exit fast > > enough, but the biggest reason I'm struggling with this is because > > using the syscore framework to work around IRQ affinity changes that > > happen late in suspend just seems kind of kludgy and it doesn't seem > > like it fits cleanly in the power management model. > > Can you please suggest any another way to work around IRQ affinity > changes. One of your earlier patches [1] made dw_msi_mask_irq() look like this: static void dw_msi_mask_irq(struct irq_data *d) { struct pcie_port *pp = irq_data_get_irq_chip_data(d->parent_data); struct dw_pcie *pci = to_dw_pcie_from_pp(pp); if (dw_pcie_link_up(pci)) pci_msi_mask_irq(d); irq_chip_mask_parent(d); } That was an awful lot like Marc's suggestion [2] that the pci_msi_mask_irq() should be redundant. If it's truly redundant, maybe pci_msi_mask_irq() can be removed from dw_msi_mask_irq() (and other similar *_mask_irq() implementations) completely? Bjorn [1] https://lore.kernel.org/r/1659526134-22978-3-git-send-email-quic_krichai@quicinc.com [2] https://lore.kernel.org/linux-pci/86k05m7dkr.wl-maz@kernel.org/
next prev parent reply other threads:[~2022-10-13 0:48 UTC|newest] Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-09-20 10:22 [PATCH v7 0/5] PCI: qcom: Add system suspend & resume support Krishna chaitanya chundru 2022-09-20 10:22 ` Krishna chaitanya chundru 2022-09-20 10:22 ` [PATCH v7 1/5] PCI: qcom: Add system suspend and " Krishna chaitanya chundru 2022-09-20 10:22 ` Krishna chaitanya chundru 2022-09-20 18:16 ` Bjorn Helgaas 2022-09-20 18:16 ` Bjorn Helgaas 2022-09-21 9:53 ` Krishna Chaitanya Chundru 2022-09-21 9:53 ` Krishna Chaitanya Chundru 2022-09-21 16:56 ` Bjorn Helgaas 2022-09-21 16:56 ` Bjorn Helgaas 2022-09-22 15:39 ` Krishna Chaitanya Chundru 2022-09-22 15:39 ` Krishna Chaitanya Chundru 2022-09-22 18:42 ` Bjorn Helgaas 2022-09-22 18:42 ` Bjorn Helgaas 2022-09-23 1:59 ` Krishna Chaitanya Chundru 2022-09-23 1:59 ` Krishna Chaitanya Chundru 2022-09-23 14:26 ` Bjorn Helgaas 2022-09-23 14:26 ` Bjorn Helgaas 2022-09-25 1:53 ` Krishna Chaitanya Chundru 2022-09-25 1:53 ` Krishna Chaitanya Chundru 2022-09-28 14:32 ` Krishna Chaitanya Chundru 2022-09-28 14:32 ` Krishna Chaitanya Chundru 2022-09-26 15:30 ` Krishna Chaitanya Chundru 2022-09-26 15:30 ` Krishna Chaitanya Chundru 2022-09-29 18:53 ` Bjorn Helgaas 2022-09-29 18:53 ` Bjorn Helgaas 2022-10-03 12:10 ` Krishna Chaitanya Chundru 2022-10-03 12:10 ` Krishna Chaitanya Chundru 2022-10-05 21:13 ` Bjorn Helgaas 2022-10-05 21:13 ` Bjorn Helgaas 2022-10-12 14:06 ` Krishna Chaitanya Chundru 2022-10-12 14:06 ` Krishna Chaitanya Chundru 2022-10-13 0:44 ` Bjorn Helgaas [this message] 2022-10-13 0:44 ` Bjorn Helgaas 2022-09-20 21:58 ` Jeff Johnson 2022-09-20 21:58 ` Jeff Johnson 2022-09-20 10:22 ` [PATCH v7 2/5] PCI: qcom: Add retry logic for link to be stable in either L1.1 or L1.2 Krishna chaitanya chundru 2022-09-20 10:22 ` Krishna chaitanya chundru 2022-09-20 22:00 ` Jeff Johnson 2022-09-20 22:00 ` Jeff Johnson 2022-09-24 6:05 ` Vinod Koul 2022-09-24 6:05 ` Vinod Koul 2022-09-25 1:51 ` Krishna Chaitanya Chundru 2022-09-25 1:51 ` Krishna Chaitanya Chundru 2022-09-20 10:22 ` [PATCH v7 3/5] phy: core: Add support for phy suspend & resume Krishna chaitanya chundru 2022-09-20 10:22 ` Krishna chaitanya chundru 2022-09-20 10:22 ` [PATCH v7 4/5] phy: qcom: Add power suspend & resume callbacks to PCIe phy Krishna chaitanya chundru 2022-09-20 10:22 ` Krishna chaitanya chundru 2022-09-20 10:22 ` [PATCH v7 5/5] clk: qcom: gcc-sc7280: Update the .pwrsts for PCIe GDSC Krishna chaitanya chundru 2022-09-20 10:22 ` Krishna chaitanya chundru 2022-09-27 3:23 ` (subset) [PATCH v7 0/5] PCI: qcom: Add system suspend & resume support Bjorn Andersson 2022-09-27 3:23 ` Bjorn Andersson
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=20221013004444.GA3135316@bhelgaas \ --to=helgaas@kernel.org \ --cc=agross@kernel.org \ --cc=andersson@kernel.org \ --cc=bhelgaas@google.com \ --cc=bjorn.andersson@linaro.org \ --cc=dmitry.baryshkov@linaro.org \ --cc=kevin.p.rowland@gmail.com \ --cc=kishon@ti.com \ --cc=konrad.dybcio@somainline.org \ --cc=kw@linux.com \ --cc=linux-arm-msm@vger.kernel.org \ --cc=linux-clk@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pci@vger.kernel.org \ --cc=linux-phy@lists.infradead.org \ --cc=linux-pm@vger.kernel.org \ --cc=lpieralisi@kernel.org \ --cc=manivannan.sadhasivam@linaro.org \ --cc=maz@kernel.org \ --cc=mka@chromium.org \ --cc=mturquette@baylibre.com \ --cc=quic_hemantk@quicinc.com \ --cc=quic_krichai@quicinc.com \ --cc=quic_nitegupt@quicinc.com \ --cc=quic_ramkri@quicinc.com \ --cc=quic_skananth@quicinc.com \ --cc=quic_vbadigan@quicinc.com \ --cc=rafael@kernel.org \ --cc=robh@kernel.org \ --cc=svarbanov@mm-sol.com \ --cc=swboyd@chromium.org \ --cc=vkoul@kernel.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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.