Re: [PATCH] pci: Disable slot presence detection around bus reset

[Martin Mokrejs]

Hi Alex,
  I was just going to ask you whether your patch would "explain" why pciehp has
in my experience broken presence detection while acpiphp has not (on 3.7 kernel)
and whether the patch will fix it.
  Some testing I have done in the past on 3.2 kernel and on 3.7.1, with no fixes.
Maybe you are interested in these threads? Actually, another user confirmed that
pciehp is broken on 3.7 while luckily, he also could have shifted to acpiphp.
Still, it is weird the behavior is different for different express cards
(USB3 vs. SATA vs. RS232 vs. firewire).

Four thread subjects on card presence detection:

Re: 3.2.11: PCI Express card cannot be re-detected withing cca 60sec timeframe
Re: linux-3.4-rc5: eSATA Sil3132 ExpressCard removal results in warn_slowpath_common
Re: Dell Vostro 3550: pci_hotplug+acpiphp require 'pcie_aspm=force' on kernel command-line for hotplug to work
Re: [PATCH v3 3/6] ACPI/pci_slot: update PCI slot information when PCI hotplug event happens
Re: 3.8.0-rc4+ - Oops on removing WinTV-HVR-1400 expresscard TV Tuner

Maybe you will crack it? ;-)
Thanks,
Martin

Alex Williamson wrote:
> On Thu, 2013-02-14 at 11:37 -0700, Alex Williamson wrote:
>> A bus reset can trigger a presence detection change and result in a
>> suprise hotplug.  This is generally not what we want to happen when
>> trying to reset a device.  Disable the presence detection control on
>> on bridges around bus reset.
>>
>> Signed-off-by: Alex Williamson <alex.williamson@xxxxxxxxxx>
>> ---
>>  drivers/pci/pci.c |   29 ++++++++++++++++++++++++-----
>>  1 file changed, 24 insertions(+), 5 deletions(-)
> 
> 
> Hmm, this doesn't seem to be sufficient, still seeing it
> occasionally :-\
> 
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index 5cb5820..c1f7d77 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -3229,8 +3229,8 @@ static int pci_pm_reset(struct pci_dev *dev, int probe)
>>  
>>  static int pci_parent_bus_reset(struct pci_dev *dev, int probe)
>>  {
>> -	u16 ctrl;
>> -	struct pci_dev *pdev;
>> +	u16 ctrl, flags, sltctl = 0;
>> +	struct pci_dev *pdev, *bridge;
>>  
>>  	if (pci_is_root_bus(dev->bus) || dev->subordinate || !dev->bus->self)
>>  		return -ENOTTY;
>> @@ -3242,15 +3242,34 @@ static int pci_parent_bus_reset(struct pci_dev *dev, int probe)
>>  	if (probe)
>>  		return 0;
>>  
>> -	pci_read_config_word(dev->bus->self, PCI_BRIDGE_CONTROL, &ctrl);
>> +	bridge = dev->bus->self;
>> +
>> +	/*
>> +	 * If the parent device supports a slot with presence detection
>> +	 * change enabled, holding the bus in reset can trigger that and
>> +	 * cause an unwanted surprise removal.  Disable presence detection
>> +	 * around the bus reset.
>> +	 */
>> +	pcie_capability_read_word(bridge, PCI_EXP_FLAGS, &flags);
>> +	if (flags & PCI_EXP_FLAGS_SLOT) {
>> +		pcie_capability_read_word(bridge, PCI_EXP_SLTCTL, &sltctl);
>> +		if (sltctl & PCI_EXP_SLTCTL_PDCE)
>> +			pcie_capability_write_word(bridge, PCI_EXP_SLTCTL,
>> +						sltctl & ~PCI_EXP_SLTCTL_PDCE);
>> +	}
>> +
>> +	pci_read_config_word(bridge, PCI_BRIDGE_CONTROL, &ctrl);
>>  	ctrl |= PCI_BRIDGE_CTL_BUS_RESET;
>> -	pci_write_config_word(dev->bus->self, PCI_BRIDGE_CONTROL, ctrl);
>> +	pci_write_config_word(bridge, PCI_BRIDGE_CONTROL, ctrl);
>>  	msleep(100);
>>  
>>  	ctrl &= ~PCI_BRIDGE_CTL_BUS_RESET;
>> -	pci_write_config_word(dev->bus->self, PCI_BRIDGE_CONTROL, ctrl);
>> +	pci_write_config_word(bridge, PCI_BRIDGE_CONTROL, ctrl);
>>  	msleep(100);
>>  
>> +	if (sltctl & PCI_EXP_SLTCTL_PDCE)
>> +		pcie_capability_write_word(bridge, PCI_EXP_SLTCTL, sltctl);
>> +
>>  	return 0;
>>  }
>>  
>>
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/