| Date | Sat, 9 Apr 2022 10:13:15 +0200 | From | Pavel Machek <> | Subject | Re: [PATCH 5.10 126/599] PCI: pciehp: Clear cmd_busy bit in polling mode |
| |
Hi!
> From: Liguang Zhang <zhangliguang@linux.alibaba.com> > > commit 92912b175178c7e895f5e5e9f1e30ac30319162b upstream. > > Writes to a Downstream Port's Slot Control register are PCIe hotplug > "commands." If the Port supports Command Completed events, software must > wait for a command to complete before writing to Slot Control again. > > pcie_do_write_cmd() sets ctrl->cmd_busy when it writes to Slot Control. If > software notification is enabled, i.e., PCI_EXP_SLTCTL_HPIE and > PCI_EXP_SLTCTL_CCIE are set, ctrl->cmd_busy is cleared by pciehp_isr(). > > But when software notification is disabled, as it is when pcie_init() > powers off an empty slot, pcie_wait_cmd() uses pcie_poll_cmd() to poll for > command completion, and it neglects to clear ctrl->cmd_busy, which leads to > spurious timeouts:
I'm pretty sure this fixes the problem, but... it is still not fully correct.
> +++ b/drivers/pci/hotplug/pciehp_hpc.c > @@ -98,6 +98,8 @@ static int pcie_poll_cmd(struct controll > if (slot_status & PCI_EXP_SLTSTA_CC) { > pcie_capability_write_word(pdev, PCI_EXP_SLTSTA, > PCI_EXP_SLTSTA_CC); > + ctrl->cmd_busy = 0; > + smp_mb(); > return 1; > }
Is the memory barrier neccessary? I don't see corresponding memory barrier for reading.
If it is neccessary, should we have WRITE_ONCE at the very least, or probably normal atomic operations?
Best regards, Pavel -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany [unhandled content-type:application/pgp-signature] |