| Message ID | 1417763857-11993-1-git-send-email-chuansheng.liu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Fri, Dec 05, 2014 at 03:17:37PM +0800, Chuansheng Liu wrote: > DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_JMICRON, PCI_DEVICE_ID_JMICRON_JMB360, quirk_jmicron_ata); > DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_JMICRON, PCI_DEVICE_ID_JMICRON_JMB361, quirk_jmicron_ata); > DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_JMICRON, PCI_DEVICE_ID_JMICRON_JMB362, quirk_jmicron_ata); > @@ -1519,6 +1534,8 @@ DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_JMICRON, PCI_DEVICE_ID_JMICRON_JMB3 > DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_JMICRON, PCI_DEVICE_ID_JMICRON_JMB366, quirk_jmicron_ata); > DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_JMICRON, PCI_DEVICE_ID_JMICRON_JMB368, quirk_jmicron_ata); > DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_JMICRON, PCI_DEVICE_ID_JMICRON_JMB369, quirk_jmicron_ata); > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_JMICRON, PCI_ANY_ID, > + pci_async_suspend_fixup); Why is this being done through pci quirks? e6b7e41cdd8 implements the same quirk in the respective drivers. What's the difference here? Thanks.
> Why is this being done through pci quirks? e6b7e41cdd8 implements the > same quirk in the respective drivers. What's the difference here? > > Thanks. > the difference is that the commit e6b7e41cdd8 "ata: Disabling the async PM for JMicron chip 363/361" doesn't work with my JMicron 363/368, because in this commit "the if statement conditions" are not suitable to my JMicron 363/368 card ( mismatch PCI_ID ), I tried this patch and it doesn't work, check the if statement you will understand why : https://github.com/rjarzmik/linux/commit/e6b7e41cdd8cae0591e04d9519b65470110e2d44 my JMicron 363/368 is both an IDE/SATA controler pcie card, Chuansheng has found the solution by adding a line in drivers/pci/quirks.c file in order to be sure that ALL variants of JMicron 3xx/3xx chips will be targeted : +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_JMICRON, PCI_ANY_ID, + pci_async_suspend_fixup); Le 05/12/2014 15:45, Tejun Heo a écrit : > On Fri, Dec 05, 2014 at 03:17:37PM +0800, Chuansheng Liu wrote: >> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_JMICRON, PCI_DEVICE_ID_JMICRON_JMB360, quirk_jmicron_ata); >> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_JMICRON, PCI_DEVICE_ID_JMICRON_JMB361, quirk_jmicron_ata); >> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_JMICRON, PCI_DEVICE_ID_JMICRON_JMB362, quirk_jmicron_ata); >> @@ -1519,6 +1534,8 @@ DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_JMICRON, PCI_DEVICE_ID_JMICRON_JMB3 >> DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_JMICRON, PCI_DEVICE_ID_JMICRON_JMB366, quirk_jmicron_ata); >> DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_JMICRON, PCI_DEVICE_ID_JMICRON_JMB368, quirk_jmicron_ata); >> DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_JMICRON, PCI_DEVICE_ID_JMICRON_JMB369, quirk_jmicron_ata); >> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_JMICRON, PCI_ANY_ID, >> + pci_async_suspend_fixup); > > Why is this being done through pci quirks? e6b7e41cdd8 implements the > same quirk in the respective drivers. What's the difference here? > > Thanks. > -- 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
On Fri, 5 Dec 2014, Chuansheng Liu wrote: > Some history from > commit e6b7e41cdd8c ("ata: Disabling the async PM for JMicron chip 363/361") > == > Since v3.15, the PM feature of async noirq > commit 76569faa62c4 ("PM / sleep: Asynchronous threads for resume_noirq") is introduced. > > Then Jay hit one system resuming issue that one of the JMicron controller > can not be powered up successfully. > > His device tree is like below: > +-1c.4-[02]--+-00.0 JMicron Technology Corp. JMB363 SATA/IDE Controller > | \-00.1 JMicron Technology Corp. JMB363 SATA/IDE Controller > > After investigation, we found the the Micron chip 363 included > one SATA controller(0000:02:00.0) and one PATA controller(0000:02:00.1), > these two controllers do not have parent-children relationship, > but the PATA controller only can be powered on after the SATA controller > has finished the powering on. > > If we enabled the async noirq(), then the below error is hit during noirq > phase: > pata_jmicron 0000:02:00.1: Refused to change power state, currently in D3 > Here for JMicron chip 363/361, we need forcedly to disable the async method. You know, this is exactly why device_pm_wait_for_dev() exists -- so that asynchronous power-management operations can wait for other devices even when there's no parent-child relation. You should try to use device_pm_wait_for_dev() instead of disabling async suspend/resume. Alan Stern -- 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
On Fri, Dec 05, 2014 at 02:18:40PM -0500, Alan Stern wrote: > On Fri, 5 Dec 2014, Chuansheng Liu wrote: > > > Some history from > > commit e6b7e41cdd8c ("ata: Disabling the async PM for JMicron chip 363/361") > > == > > Since v3.15, the PM feature of async noirq > > commit 76569faa62c4 ("PM / sleep: Asynchronous threads for resume_noirq") is introduced. > > > > Then Jay hit one system resuming issue that one of the JMicron controller > > can not be powered up successfully. > > > > His device tree is like below: > > +-1c.4-[02]--+-00.0 JMicron Technology Corp. JMB363 SATA/IDE Controller > > | \-00.1 JMicron Technology Corp. JMB363 SATA/IDE Controller > > > > After investigation, we found the the Micron chip 363 included > > one SATA controller(0000:02:00.0) and one PATA controller(0000:02:00.1), > > these two controllers do not have parent-children relationship, > > but the PATA controller only can be powered on after the SATA controller > > has finished the powering on. > > > > If we enabled the async noirq(), then the below error is hit during noirq > > phase: > > pata_jmicron 0000:02:00.1: Refused to change power state, currently in D3 > > Here for JMicron chip 363/361, we need forcedly to disable the async method. > > You know, this is exactly why device_pm_wait_for_dev() exists -- so > that asynchronous power-management operations can wait for other > devices even when there's no parent-child relation. > > You should try to use device_pm_wait_for_dev() instead of disabling > async suspend/resume. Dropping this patch for now, waiting for a response to Alan's suggestion. Bjorn -- 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
On Sat, Jan 10, 2015 at 1:46 AM, Bjorn Helgaas <bhelgaas@google.com> wrote: > On Fri, Dec 05, 2014 at 02:18:40PM -0500, Alan Stern wrote: >> On Fri, 5 Dec 2014, Chuansheng Liu wrote: >> >> You know, this is exactly why device_pm_wait_for_dev() exists -- so >> that asynchronous power-management operations can wait for other >> devices even when there's no parent-child relation. >> >> You should try to use device_pm_wait_for_dev() instead of disabling >> async suspend/resume. > > Dropping this patch for now, waiting for a response to Alan's suggestion. From Alan's comments #106: https://bugzilla.kernel.org/show_bug.cgi?id=81551#c106 It seems Chuansheng's patch is the best solution for now, so perhaps it can be considered to be merged? Regards, Aaron -- 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
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index 90acb32..1963080 100644 --- a/drivers/pci/quirks.c +++ b/drivers/pci/quirks.c @@ -1501,6 +1501,21 @@ static void quirk_jmicron_ata(struct pci_dev *pdev) pci_read_config_dword(pdev, PCI_CLASS_REVISION, &class); pdev->class = class >> 8; } + +/* + * For JMicron chips, we need to disable the async_suspend method, otherwise + * they will hit the power-on issue when doing device resume, add one quirk + * solution to disable the async_suspend method. + */ +static void pci_async_suspend_fixup(struct pci_dev *pdev) +{ + /* + * disabling the async_suspend method for JMicron chips to + * avoid device resuming issue. + */ + device_disable_async_suspend(&pdev->dev); +} + DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_JMICRON, PCI_DEVICE_ID_JMICRON_JMB360, quirk_jmicron_ata); DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_JMICRON, PCI_DEVICE_ID_JMICRON_JMB361, quirk_jmicron_ata); DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_JMICRON, PCI_DEVICE_ID_JMICRON_JMB362, quirk_jmicron_ata); @@ -1519,6 +1534,8 @@ DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_JMICRON, PCI_DEVICE_ID_JMICRON_JMB3 DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_JMICRON, PCI_DEVICE_ID_JMICRON_JMB366, quirk_jmicron_ata); DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_JMICRON, PCI_DEVICE_ID_JMICRON_JMB368, quirk_jmicron_ata); DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_JMICRON, PCI_DEVICE_ID_JMICRON_JMB369, quirk_jmicron_ata); +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_JMICRON, PCI_ANY_ID, + pci_async_suspend_fixup); #endif
Some history from commit e6b7e41cdd8c ("ata: Disabling the async PM for JMicron chip 363/361") == Since v3.15, the PM feature of async noirq commit 76569faa62c4 ("PM / sleep: Asynchronous threads for resume_noirq") is introduced. Then Jay hit one system resuming issue that one of the JMicron controller can not be powered up successfully. His device tree is like below: +-1c.4-[02]--+-00.0 JMicron Technology Corp. JMB363 SATA/IDE Controller | \-00.1 JMicron Technology Corp. JMB363 SATA/IDE Controller After investigation, we found the the Micron chip 363 included one SATA controller(0000:02:00.0) and one PATA controller(0000:02:00.1), these two controllers do not have parent-children relationship, but the PATA controller only can be powered on after the SATA controller has finished the powering on. If we enabled the async noirq(), then the below error is hit during noirq phase: pata_jmicron 0000:02:00.1: Refused to change power state, currently in D3 Here for JMicron chip 363/361, we need forcedly to disable the async method. Bug detail: https://bugzilla.kernel.org/show_bug.cgi?id=81551 == After that, Barto reported the same issue, but his Jmicron chip is JMB368, so it can not be covered by commit e6b7e41cdd8c ("ata: Disabling the async PM for JMicron chip 363/361"). Bug link: https://bugzilla.kernel.org/show_bug.cgi?id=84861 Here we think Jmicron chips have the same issue as describled in commit e6b7e41cdd8c ("ata: Disabling the async PM for JMicron chip 363/361"), so here add one quirk to disable the JMicron chips' PM async feature. Cc: stable@vger.kernel.org # 3.15+ Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com> --- drivers/pci/quirks.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)