Re: [PATCH v2] PCI: pciehp: Clear LBMS on hot-remove to prevent link speed reduction

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

 



On 6/18/2024 11:51 AM, Smita Koralahalli wrote:

[snip]

But IIUC LBMS is set by hardware but never cleared by hardware, so if
we remove a device and power off the slot, it doesn't seem like LBMS
could be telling us anything useful (what could we do in response to
LBMS when the slot is empty?), so it makes sense to me to clear it.

It seems like pciehp_unconfigure_device() does sort of PCI core and
driver-related things and possibly could be something shared by all
hotplug drivers, while remove_board() does things more specific to the
hotplug model (pciehp, shpchp, etc).

  From that perspective, clearing LBMS might fit better in
remove_board().  In that case, I wonder whether it should be done
after turning off slot power?  This patch clears is *before* turning
off the power, so I wonder if hardware could possibly set it again
before the poweroff?

While clearing LBMS in remove_board() here:

if (POWER_CTRL(ctrl)) {
	pciehp_power_off_slot(ctrl);
+	pcie_capability_write_word(ctrl->pcie->port, PCI_EXP_LNKSTA,
				   PCI_EXP_LNKSTA_LBMS);

	/*
	 * After turning power off, we must wait for at least 1 second
	 * before taking any action that relies on power having been
	 * removed from the slot/adapter.
	 */
	msleep(1000);

	/* Ignore link or presence changes caused by power off */
	atomic_and(~(PCI_EXP_SLTSTA_DLLSC | PCI_EXP_SLTSTA_PDC),
		   &ctrl->pending_events);
}

This can happen too right? I.e Just after the slot poweroff and before LBMS clearing the PDC/PDSC could be fired. Then pciehp_handle_presence_or_link_change() would hit case "OFF_STATE" and proceed with pciehp_enable_slot() ....pcie_failed_link_retrain() and ultimately link speed drops..

So, I added clearing just before turning off the slot.. Let me know if I'm thinking it right.

Thanks
Smita

Yeah by talking to HW people I realized that HW could interfere possibly
anytime to set LBMS when the slot power is on. Will change it to include in
remove_board().


[snip]




[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux