Hello Lukas,

On 3/18/19 6:13 PM, Lukas Wunner wrote:
> On Mon, Mar 18, 2019 at 05:31:29PM +0300, Sergey Miroshnichenko wrote:
>> pcie_init() invokes pcie_disable_notification() for empty slots, which in
>> turn disables the Command Complete interrupt (unset PCI_EXP_SLTCTL_CCIE)
>> and right after that waits for Command Complete event (PCI_EXP_SLTSTA_CC).
>>
>> This results in longer boot time - 4 seconds of delay for each empty slot:
>>
>> [    4.200325] pciehp 0042:04:08.0:pcie204: Slot #40 AttnBtn+ PwrCtrl+ MRL- AttnInd+ PwrInd+ HotPlug+ Surprise- Interlock- NoCompl- LLActRep+
>> [    4.200607] pciehp 0042:04:08.0:pcie204: pciehp_get_power_status: SLOTCTRL 80 value read 1c0
>> [    6.217938] pciehp 0042:04:08.0:pcie204: Timeout on hotplug command 0x01c0 (issued 2020 msec ago)
>> [    6.217966] pciehp 0042:04:08.0:pcie204: pcie_disable_notification: SLOTCTRL 80 write cmd 0
>> [    6.237938] pciehp 0042:04:08.0:pcie204: Timeout on hotplug command 0x01c0 (issued 2040 msec ago)
>> [    8.257939] pciehp 0042:04:08.0:pcie204: Timeout on hotplug command 0x05c0 (issued 2020 msec ago)
>> [    8.257974] pciehp 0042:04:08.0:pcie204: pciehp_power_off_slot: SLOTCTRL 80 write cmd 400
>> [    8.258034] pci_bus 0042:07: dev 00, created physical slot 40
>> [    8.277941] pciehp 0042:04:08.0:pcie204: Timeout on hotplug command 0x05c0 (issued 2040 msec ago)
>> [    8.277967] pciehp 0042:04:08.0:pcie204: pcie_enable_notification: SLOTCTRL 80 write cmd 1031
> 
> Those "Timeout on hotplug command" messages look suspicious.  The port
> claims to support Command Completed Events, but doesn't seem to set the
> Command Completed bit in the Slot Status register.  Ultimately that's
> the reason for the delays you're seeing.
> 
> We already have quirks for various Intel and Qualcomm ports with broken
> Command Completed support (see at the bottom of pciehp_hpc.c) and we
> know that older Thunderbolt chips erroneously claim to support Command
> Completed Events but in reality don't support it (see quirk in pcie_init()).
> Maybe the hardware you're dealing with needs such a quirk as well?
Thanks, I'll rework this as a quirk for PEX 8796. The "Command
Completed" seems to be half-broken there: the SLTSTA_CC bit is always
zero without SLTCTL_CCIE, but works just fine with SLTCTL_CCIE, so the
pciehp_isr() wakes up the pcie_wait_cmd().

Best regards,
Serge

> 
> Thanks,
> 
> Lukas
> 
>> Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
>> Cc: Lukas Wunner <lukas@wunner.de>
>> ---
>>  drivers/pci/hotplug/pciehp_hpc.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
>> index 6a2365cd794e..eec5d6107360 100644
>> --- a/drivers/pci/hotplug/pciehp_hpc.c
>> +++ b/drivers/pci/hotplug/pciehp_hpc.c
>> @@ -118,8 +118,10 @@ static void pcie_wait_cmd(struct controller *ctrl)
>>  	if (ctrl->slot_ctrl & PCI_EXP_SLTCTL_HPIE &&
>>  	    ctrl->slot_ctrl & PCI_EXP_SLTCTL_CCIE)
>>  		rc = wait_event_timeout(ctrl->queue, !ctrl->cmd_busy, timeout);
>> -	else
>> +	else if (ctrl->slot_ctrl & PCI_EXP_SLTCTL_CCIE)
>>  		rc = pcie_poll_cmd(ctrl, jiffies_to_msecs(timeout));
>> +	else
>> +		return;
>>  
>>  	if (!rc)
>>  		ctrl_info(ctrl, "Timeout on hotplug command %#06x (issued %u msec ago)\n",
>> -- 
>> 2.20.1
>>