| Message ID | 20240511235513.77301-1-marek.vasut+renesas@mailbox.org |
|---|---|
| State | New |
| Headers | show |
| Series | PCI: rcar: Demote WARN() to dev_warn_ratelimited() in rcar_pcie_wakeup() | expand |
Hello, > Avoid large backtrace, it is sufficient to warn the user that there has > been a link problem. Either the link has failed and the system is in need > of maintenance, or the link continues to work and user has been informed. > The message from the warning can be looked up in the sources. > > This makes an actual link issue less verbose. > > First of all, this controller has a limitation in that the controller > driver has to assist the hardware with transition to L1 link state by > writing L1IATN to PMCTRL register, the L1 and L0 link state switching > is not fully automatic on this controller. > > In case of an ASMedia ASM1062 PCIe SATA controller which does not support > ASPM, on entry to suspend or during platform pm_test, the SATA controller > enters D3hot state and the link enters L1 state. If the SATA controller > wakes up before rcar_pcie_wakeup() was called and returns to D0, the link > returns to L0 before the controller driver even started its transition to > L1 link state. At this point, the SATA controller did send an PM_ENTER_L1 > DLLP to the PCIe controller and the PCIe controller received it, and the > PCIe controller did set PMSR PMEL1RX bit. > > Once rcar_pcie_wakeup() is called, if the link is already back in L0 state > and PMEL1RX bit is set, the controller driver has no way to determine if > it should perform the link transition to L1 state, or treat the link as if > it is in L0 state. Currently the driver attempts to perform the transition > to L1 link state unconditionally, which in this specific case fails with a > PMSR L1FAEG poll timeout, however the link still works as it is already > back in L0 state. > > Reduce this warning verbosity. In case the link is really broken, the > rcar_pcie_config_access() would fail, otherwise it will succeed and any > system with this controller and ASM1062 can suspend without generating > a backtrace. Applied to controller/rcar, thank you! [1/1] PCI: rcar: Demote WARN() to dev_warn_ratelimited() in rcar_pcie_wakeup() https://git.kernel.org/pci/pci/c/a37647d5639b Krzysztof
diff --git a/drivers/pci/controller/pcie-rcar-host.c b/drivers/pci/controller/pcie-rcar-host.c index 996077ab7cfdb..c01efc6ea64f6 100644 --- a/drivers/pci/controller/pcie-rcar-host.c +++ b/drivers/pci/controller/pcie-rcar-host.c @@ -78,7 +78,11 @@ static int rcar_pcie_wakeup(struct device *pcie_dev, void __iomem *pcie_base) writel(L1IATN, pcie_base + PMCTLR); ret = readl_poll_timeout_atomic(pcie_base + PMSR, val, val & L1FAEG, 10, 1000); - WARN(ret, "Timeout waiting for L1 link state, ret=%d\n", ret); + if (ret) { + dev_warn_ratelimited(pcie_dev, + "Timeout waiting for L1 link state, ret=%d\n", + ret); + } writel(L1FAEG | PMEL1RX, pcie_base + PMSR); }
Avoid large backtrace, it is sufficient to warn the user that there has been a link problem. Either the link has failed and the system is in need of maintenance, or the link continues to work and user has been informed. The message from the warning can be looked up in the sources. This makes an actual link issue less verbose. First of all, this controller has a limitation in that the controller driver has to assist the hardware with transition to L1 link state by writing L1IATN to PMCTRL register, the L1 and L0 link state switching is not fully automatic on this controller. In case of an ASMedia ASM1062 PCIe SATA controller which does not support ASPM, on entry to suspend or during platform pm_test, the SATA controller enters D3hot state and the link enters L1 state. If the SATA controller wakes up before rcar_pcie_wakeup() was called and returns to D0, the link returns to L0 before the controller driver even started its transition to L1 link state. At this point, the SATA controller did send an PM_ENTER_L1 DLLP to the PCIe controller and the PCIe controller received it, and the PCIe controller did set PMSR PMEL1RX bit. Once rcar_pcie_wakeup() is called, if the link is already back in L0 state and PMEL1RX bit is set, the controller driver has no way to determine if it should perform the link transition to L1 state, or treat the link as if it is in L0 state. Currently the driver attempts to perform the transition to L1 link state unconditionally, which in this specific case fails with a PMSR L1FAEG poll timeout, however the link still works as it is already back in L0 state. Reduce this warning verbosity. In case the link is really broken, the rcar_pcie_config_access() would fail, otherwise it will succeed and any system with this controller and ASM1062 can suspend without generating a backtrace. Fixes: 84b576146294 ("PCI: rcar: Finish transition to L1 state in rcar_pcie_config_access()") Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org> --- Cc: "Krzysztof Wilczyński" <kw@linux.com> Cc: Bjorn Helgaas <bhelgaas@google.com> Cc: Lorenzo Pieralisi <lpieralisi@kernel.org> Cc: Rob Herring <robh@kernel.org> Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> Cc: linux-pci@vger.kernel.org Cc: linux-renesas-soc@vger.kernel.org --- drivers/pci/controller/pcie-rcar-host.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)