Re: [net-next v2] net: wwan: t7xx: reset device if suspend fails

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Sergey Ryazanov <ryazanov.s.a@xxxxxxxxx>

Hi Jinjian,

On 29.10.2024 05:46, Jinjian Song wrote:
From: Sergey Ryazanov <ryazanov.s.a@xxxxxxxxx>
On 22.10.2024 11:43, Jinjian Song wrote:
If driver fails to set the device to suspend, it means that the
device is abnormal. In this case, reset the device to recover
when PCIe device is offline.

Is it a reproducible or a speculative issue? Does the fix recover modem from a problematic state?

Anyway we need someone more familiar with this hardware (Intel or MediaTek engineer) to Ack the change to make sure we are not going to put a system in a more complicated state.

Hi Sergey,

This is a very difficult issue to replicate onece occured and fixed.

The issue occured when driver and device lost the connection. I have
encountered this problem twice so far:
1. During suspend/resume stress test, there was a probabilistic D3L2
time sequence issue with the BIOS, result in PCIe link down, driver
read and write the register of device invalid, so suspend failed.
This issue was eventually fixed in the BIOS and I was able to restore
it through the reset module after reproducing the problem.

2. During idle test, the modem probabilistic hang up, result in PCIe
link down, driver read and write the register of device invalid, so
suspend failed. This issue was eventually fiex in device modem firmware
by adjust a certain power supply voltage, and reset modem as a workround
to restore when the MBIM port command timeout in userspace applycations.

Hardware reset modem to recover was discussed with MTK, and they said
that if we don't want to keep the on-site problem location in case of
suspend failure, we can use the recover solution.
Both the ocurred issues result in the PCIe link issue, driver can't read and writer the register of WWAN device, so I want to add this path to restore, hardware reset modem can recover modem, but using the pci_channle_offline() as the judgment is my inference.

Thank you for the clarification. Let me summarize what I've understood from the explanation:
a) there were hardware (firmware) issues,
b) issues already were solved,
c) issues were not directly related to the device suspension procedure,
d) you want to implement a backup plan to make the modem support robust.

If got it right, then I would like to recommend to implement a generic error handling solution for the PCIe interface. You can check this document: Documentation/PCI/pci-error-recovery.rst
Hi Sergey,

Yes, got it right.
I want to identify the scenario and then recover by reset device,
otherwise suspend failure will aways prevent the system from suspending
if it occurs.

Suddenly, I am not an expert in the PCIe link recovery procedure, so I've CCed this message to PCI subsystem maintainers. I hope they can suggest a conceptually correct way to handle these cases.

Thanks.

Signed-off-by: Jinjian Song <jinjian.song@xxxxxxxxxxx>
---
V2:
  * Add judgment, reset when device is offline
---
  drivers/net/wwan/t7xx/t7xx_pci.c | 4 ++++
  1 file changed, 4 insertions(+)

diff --git a/drivers/net/wwan/t7xx/t7xx_pci.c b/drivers/net/wwan/ t7xx/t7xx_pci.c
index e556e5bd49ab..4f89a353588b 100644
--- a/drivers/net/wwan/t7xx/t7xx_pci.c
+++ b/drivers/net/wwan/t7xx/t7xx_pci.c
@@ -427,6 +427,10 @@ static int __t7xx_pci_pm_suspend(struct pci_dev *pdev) iowrite32(T7XX_L1_BIT(0), IREG_BASE(t7xx_dev) + ENABLE_ASPM_LOWPWR);
      atomic_set(&t7xx_dev->md_pm_state, MTK_PM_RESUMED);
      t7xx_pcie_mac_set_int(t7xx_dev, SAP_RGU_INT);
+    if (pci_channel_offline(pdev)) {
+        dev_err(&pdev->dev, "Device offline, reset to recover\n");
+        t7xx_reset_device(t7xx_dev, PLDR);
+    }
      return ret;
  }

--
Sergey


Best Regards,
Jinjian




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux