lkml.org 
[lkml]   [2013]   [Nov]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
SubjectRe: [PATCH] PCI: add a quirk for keeping Bus Master bit on shutdown
From
If this turns out to be a problem specific to Acer V573G, we can simply
wrap the if (pdev->...) line in ata/ahci.c with if
(dmi_check_system(acer_v573g))
so that only acer v573g laptops will be affected by this patch. The remaining
part of the patch won't need to be changed.

I can do an updated one as soon as it is confirmed that this is an acer specific
issue.

2013/11/26 Bjorn Helgaas <bhelgaas@google.com>:
> [+cc Lan, Khalid, Konstantin, Alan, Takao, Jility, Florian, linux-kernel]
>
> On Tue, Nov 12, 2013 at 07:40:03PM +0000, Chang Liu wrote:
>> This fixes https://bugzilla.kernel.org/show_bug.cgi?id=63861
>>
>> Commit b566a22c2 and 7897e60227 made pci_device_shutdown()
>> unconditionally clear Bus Master bit for every pci devices.
>> While this works for most hardware, certain devices are not
>> compatible with this. Intel Lynx Point-LP SATA Controller,
>> for example, will hang the system if its Bus Master bit is
>> cleared during device shutdown. This patch adds a pci quirk
>> so that device drivers can instruct pci_device_shutdown()
>> to keep Bus Master from being cleared, and then implements
>> this mechanism for the Intel Lynx Point-LP AHCI driver.
>>
>> Signed-off-by: Chang Liu <cl91tp@gmail.com>
>
> I'm not 100% comfortable with disabling bus mastering in general;
> see [1] for more details.
>
> But given that we have been doing it for quite a while (b566a22c2 appeared
> in v3.5 on Jul 21, 2012), and Lynx Point should be in lots of machines, I'm
> surprised that we don't see more reports of this problem if it really
> affects all Lynx Point parts.
>
> Jility reported the same problem [2], and I did find one other similar
> report [3] from Florian. All three reports (Chang, Jility, Florian) are on
> the same exact machine (Acer V5-573G), which seems sort of suspicious.
>
> Lan, do you have access to any Lynx Point boxes? Can you test and see
> whether they hang on power-off also? I suspect this might be something
> specific to the Acer box, not a generic Lynx Point issue.
>
> Bjorn
>
> [1] http://lkml.kernel.org/r/20120427190033.GA17588@ldl.usa.hp.com
> [2] https://bugzilla.kernel.org/show_bug.cgi?id=63861#c21
> [3] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1249329
>
>> ---
>> As per Takao's suggestion, add a new member into struct pci_dev
>> and add a quirk in the ahci driver. I tested this on my
>> machine (Acer V5-573G) and it works fine.
>>
>> drivers/ata/ahci.c | 8 ++++++++
>> drivers/pci/pci-driver.c | 11 ++++++++---
>> include/linux/pci.h | 1 +
>> 3 files changed, 17 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
>> index 8e28f92..de6efcb 100644
>> --- a/drivers/ata/ahci.c
>> +++ b/drivers/ata/ahci.c
>> @@ -1385,6 +1385,14 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>>
>> pci_set_master(pdev);
>>
>> + /* We normally clear Bus Master on pci device shutdown. However,
>> + * doing so for Intel Lynx Point-LP SATA Controller [AHCI mode]
>> + * hangs the system. Therefore keep it.
>> + * See bug report: https://bugzilla.kernel.org/show_bug.cgi?id=63861
>> + */
>> + if (pdev->vendor == PCI_VENDOR_ID_INTEL && pdev->device == 0x9c03)
>> + pdev->keep_busmaster_on_shutdown = 1;
>> +
>> if (hpriv->flags & AHCI_HFLAG_MULTI_MSI)
>> return ahci_host_activate(host, pdev->irq, n_msis);
>>
>> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
>> index 38f3c01..ff15b0c 100644
>> --- a/drivers/pci/pci-driver.c
>> +++ b/drivers/pci/pci-driver.c
>> @@ -392,10 +392,15 @@ static void pci_device_shutdown(struct device *dev)
>> pci_msix_shutdown(pci_dev);
>>
>> /*
>> - * Turn off Bus Master bit on the device to tell it to not
>> - * continue to do DMA. Don't touch devices in D3cold or unknown states.
>> + * If the hardware is okay with it, turn off Bus Master bit
>> + * on the device to tell it not to continue doing DMA.
>> + * Don't touch devices in D3cold or unknown states.
>> + * On certain hardware clearing Bus Master bit on shutdown
>> + * may hang the entire system. In these cases the driver of
>> + * these devices should set keep_busmaster_on_shutdown to 1.
>> */
>> - if (pci_dev->current_state <= PCI_D3hot)
>> + if (!pci_dev->keep_busmaster_on_shutdown
>> + && pci_dev->current_state <= PCI_D3hot)
>> pci_clear_master(pci_dev);
>> }
>>
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index da172f9..63db735 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -322,6 +322,7 @@ struct pci_dev {
>> /* keep track of device state */
>> unsigned int is_added:1;
>> unsigned int is_busmaster:1; /* device is busmaster */
>> + unsigned int keep_busmaster_on_shutdown:1; /* do not clear busmaster on shutdown */
>> unsigned int no_msi:1; /* device may not use msi */
>> unsigned int block_cfg_access:1; /* config space access is blocked */
>> unsigned int broken_parity_status:1; /* Device generates false positive parity */
>> --
>> 1.8.4.2
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html


\
 
 \ /
  Last update: 2013-11-26 05:21    [W:0.088 / U:0.060 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site