[PATCH] PCI: Do not enable async suspend for JMicron chips

[PATCH] PCI: Do not enable async suspend for JMicron chips

[Chuansheng Liu]

The JMicron chip 361/363/368 contains one SATA controller and
one PATA controller, they are brother-relation ship in PCI tree,
but for powering on these both controller, we must follow the
sequence one by one, otherwise one of them can not be powered on
successfully.

So here we disable the async suspend method for Jmicron chip.

Bug link:
https://bugzilla.kernel.org/show_bug.cgi?id=81551
https://bugzilla.kernel.org/show_bug.cgi?id=84861

And we can revert the below commit after this patch is applied:
e6b7e41(ata: Disabling the async PM for JMicron chip 363/361)

Cc: stable@vger.kernel.org # 3.15+
Acked-by: Aaron Lu <aaron.lu@intel.com>
Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
---
 drivers/pci/pci.c |   12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 625a4ac..53128f0 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2046,7 +2046,17 @@ void pci_pm_init(struct pci_dev *dev)
 	pm_runtime_forbid(&dev->dev);
 	pm_runtime_set_active(&dev->dev);
 	pm_runtime_enable(&dev->dev);
-	device_enable_async_suspend(&dev->dev);
+
+	/*
+	 * The JMicron chip 361/363/368 contains one SATA controller and
+	 * one PATA controller, they are brother-relation ship in PCI tree,
+	 * but for powering on these both controller, we must follow the
+	 * sequence one by one, otherwise one of them can not be powered on
+	 * successfully, so here we disable the async suspend method for
+	 * Jmicron chip.
+	 */
+	if (dev->vendor != PCI_VENDOR_ID_JMICRON)
+		device_enable_async_suspend(&dev->dev);
 	dev->wakeup_prepared = false;
 
 	dev->pm_cap = 0;
-- 
1.7.9.5

Re: [PATCH] PCI: Do not enable async suspend for JMicron chips

[Bjorn Helgaas]

On Tue, Nov 4, 2014 at 6:31 PM, Chuansheng Liu <chuansheng.liu@intel.com> wrote:
> The JMicron chip 361/363/368 contains one SATA controller and
> one PATA controller, they are brother-relation ship in PCI tree,
> but for powering on these both controller, we must follow the
> sequence one by one, otherwise one of them can not be powered on
> successfully.

This should mention what's broken and what problem a user would see.
This changelog sounds a lot like the one for e6b7e41cdd8c, so I don't
know if this is for a new, related problem, or what.

> So here we disable the async suspend method for Jmicron chip.
>
> Bug link:
> https://bugzilla.kernel.org/show_bug.cgi?id=81551
> https://bugzilla.kernel.org/show_bug.cgi?id=84861
>
> And we can revert the below commit after this patch is applied:
> e6b7e41(ata: Disabling the async PM for JMicron chip 363/361)
>
> Cc: stable@vger.kernel.org # 3.15+
> Acked-by: Aaron Lu <aaron.lu@intel.com>
> Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
> ---
>  drivers/pci/pci.c |   12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 625a4ac..53128f0 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -2046,7 +2046,17 @@ void pci_pm_init(struct pci_dev *dev)
>         pm_runtime_forbid(&dev->dev);
>         pm_runtime_set_active(&dev->dev);
>         pm_runtime_enable(&dev->dev);
> -       device_enable_async_suspend(&dev->dev);
> +
> +       /*
> +        * The JMicron chip 361/363/368 contains one SATA controller and
> +        * one PATA controller, they are brother-relation ship in PCI tree,
> +        * but for powering on these both controller, we must follow the
> +        * sequence one by one, otherwise one of them can not be powered on
> +        * successfully, so here we disable the async suspend method for
> +        * Jmicron chip.
> +        */
> +       if (dev->vendor != PCI_VENDOR_ID_JMICRON)
> +               device_enable_async_suspend(&dev->dev);

I don't like littering the core PCI code with vendor tests like this.
This would be the only one, except for an ancient DECchip 21050 bridge
erratum.

And why would we want a test for *all* JMicron devices here, when you
claim the problem only affects a few specific ones?

And what's the story with the e6b7e41cdd8c ("ata: Disabling the async
PM for JMicron chip 363/361") connection?  Is something broken even
with e6b7e41cdd8c, and this is a better fix?  Or is this simply a
different way of fixing the same problem?

>         dev->wakeup_prepared = false;
>
>         dev->pm_cap = 0;
> --
> 1.7.9.5
>

Re: [PATCH] PCI: Do not enable async suspend for JMicron chips

[Barto]

this patch solves these 2 bug reports :

https://bugzilla.kernel.org/show_bug.cgi?id=84861

https://bugzilla.kernel.org/show_bug.cgi?id=81551

in simple words : JMicron IDE/Sata controlers family ( JMBxxx ) are not
fully compatible with async_suspend feature, when a user tries to put
his PC on standby mode then at wake-up JMicron IDE/Sata controlers will
not work, because of a brother-relation between the SATA and IDE part on
this JMicron PCI card




Le 05/11/2014 19:01, Bjorn Helgaas a écrit :
> On Tue, Nov 4, 2014 at 6:31 PM, Chuansheng Liu <chuansheng.liu@intel.com> wrote:
>> The JMicron chip 361/363/368 contains one SATA controller and
>> one PATA controller, they are brother-relation ship in PCI tree,
>> but for powering on these both controller, we must follow the
>> sequence one by one, otherwise one of them can not be powered on
>> successfully.
> 
> This should mention what's broken and what problem a user would see.
> This changelog sounds a lot like the one for e6b7e41cdd8c, so I don't
> know if this is for a new, related problem, or what.
> 
>> So here we disable the async suspend method for Jmicron chip.
>>
>> Bug link:
>> https://bugzilla.kernel.org/show_bug.cgi?id=81551
>> https://bugzilla.kernel.org/show_bug.cgi?id=84861
>>
>> And we can revert the below commit after this patch is applied:
>> e6b7e41(ata: Disabling the async PM for JMicron chip 363/361)
>>
>> Cc: stable@vger.kernel.org # 3.15+
>> Acked-by: Aaron Lu <aaron.lu@intel.com>
>> Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
>> ---
>>  drivers/pci/pci.c |   12 +++++++++++-
>>  1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index 625a4ac..53128f0 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -2046,7 +2046,17 @@ void pci_pm_init(struct pci_dev *dev)
>>         pm_runtime_forbid(&dev->dev);
>>         pm_runtime_set_active(&dev->dev);
>>         pm_runtime_enable(&dev->dev);
>> -       device_enable_async_suspend(&dev->dev);
>> +
>> +       /*
>> +        * The JMicron chip 361/363/368 contains one SATA controller and
>> +        * one PATA controller, they are brother-relation ship in PCI tree,
>> +        * but for powering on these both controller, we must follow the
>> +        * sequence one by one, otherwise one of them can not be powered on
>> +        * successfully, so here we disable the async suspend method for
>> +        * Jmicron chip.
>> +        */
>> +       if (dev->vendor != PCI_VENDOR_ID_JMICRON)
>> +               device_enable_async_suspend(&dev->dev);
> 
> I don't like littering the core PCI code with vendor tests like this.
> This would be the only one, except for an ancient DECchip 21050 bridge
> erratum.
> 
> And why would we want a test for *all* JMicron devices here, when you
> claim the problem only affects a few specific ones?
> 
> And what's the story with the e6b7e41cdd8c ("ata: Disabling the async
> PM for JMicron chip 363/361") connection?  Is something broken even
> with e6b7e41cdd8c, and this is a better fix?  Or is this simply a
> different way of fixing the same problem?
> 
>>         dev->wakeup_prepared = false;
>>
>>         dev->pm_cap = 0;
>> --
>> 1.7.9.5
>>
> 

Re: [PATCH] PCI: Do not enable async suspend for JMicron chips

[Bjorn Helgaas]

On Wed, Nov 5, 2014 at 11:46 AM, Barto <mister.freeman@laposte.net> wrote:
> this patch solves these 2 bug reports :
>
> https://bugzilla.kernel.org/show_bug.cgi?id=84861
>
> https://bugzilla.kernel.org/show_bug.cgi?id=81551

Those bugs were already mentioned.  But e6b7e41cdd8c claims to solve
https://bugzilla.kernel.org/show_bug.cgi?id=81551, and 84861 is a
duplicate of 81551, so it should also be fixed by e6b7e41cdd8c.

So the question is, why was e6b7e41cdd8c insufficient?  Presumably it
was tested and somebody thought it did fix the problem.

> in simple words : JMicron IDE/Sata controlers family ( JMBxxx ) are not
> fully compatible with async_suspend feature, when a user tries to put
> his PC on standby mode then at wake-up JMicron IDE/Sata controlers will
> not work, because of a brother-relation between the SATA and IDE part on
> this JMicron PCI card
>
>
>
>
> Le 05/11/2014 19:01, Bjorn Helgaas a écrit :
>> On Tue, Nov 4, 2014 at 6:31 PM, Chuansheng Liu <chuansheng.liu@intel.com> wrote:
>>> The JMicron chip 361/363/368 contains one SATA controller and
>>> one PATA controller, they are brother-relation ship in PCI tree,
>>> but for powering on these both controller, we must follow the
>>> sequence one by one, otherwise one of them can not be powered on
>>> successfully.
>>
>> This should mention what's broken and what problem a user would see.
>> This changelog sounds a lot like the one for e6b7e41cdd8c, so I don't
>> know if this is for a new, related problem, or what.
>>
>>> So here we disable the async suspend method for Jmicron chip.
>>>
>>> Bug link:
>>> https://bugzilla.kernel.org/show_bug.cgi?id=81551
>>> https://bugzilla.kernel.org/show_bug.cgi?id=84861
>>>
>>> And we can revert the below commit after this patch is applied:
>>> e6b7e41(ata: Disabling the async PM for JMicron chip 363/361)
>>>
>>> Cc: stable@vger.kernel.org # 3.15+
>>> Acked-by: Aaron Lu <aaron.lu@intel.com>
>>> Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
>>> ---
>>>  drivers/pci/pci.c |   12 +++++++++++-
>>>  1 file changed, 11 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>>> index 625a4ac..53128f0 100644
>>> --- a/drivers/pci/pci.c
>>> +++ b/drivers/pci/pci.c
>>> @@ -2046,7 +2046,17 @@ void pci_pm_init(struct pci_dev *dev)
>>>         pm_runtime_forbid(&dev->dev);
>>>         pm_runtime_set_active(&dev->dev);
>>>         pm_runtime_enable(&dev->dev);
>>> -       device_enable_async_suspend(&dev->dev);
>>> +
>>> +       /*
>>> +        * The JMicron chip 361/363/368 contains one SATA controller and
>>> +        * one PATA controller, they are brother-relation ship in PCI tree,
>>> +        * but for powering on these both controller, we must follow the
>>> +        * sequence one by one, otherwise one of them can not be powered on
>>> +        * successfully, so here we disable the async suspend method for
>>> +        * Jmicron chip.
>>> +        */
>>> +       if (dev->vendor != PCI_VENDOR_ID_JMICRON)
>>> +               device_enable_async_suspend(&dev->dev);
>>
>> I don't like littering the core PCI code with vendor tests like this.
>> This would be the only one, except for an ancient DECchip 21050 bridge
>> erratum.
>>
>> And why would we want a test for *all* JMicron devices here, when you
>> claim the problem only affects a few specific ones?
>>
>> And what's the story with the e6b7e41cdd8c ("ata: Disabling the async
>> PM for JMicron chip 363/361") connection?  Is something broken even
>> with e6b7e41cdd8c, and this is a better fix?  Or is this simply a
>> different way of fixing the same problem?
>>
>>>         dev->wakeup_prepared = false;
>>>
>>>         dev->pm_cap = 0;
>>> --
>>> 1.7.9.5
>>>
>>

Re: [PATCH] PCI: Do not enable async suspend for JMicron chips

[Barto]

Bjorn : the patch initialy created for bug  81551 ( ATAPI-CD-ROM-drive
dead after resume from suspend/s2disk ) was not enough for the bug 84861
( JMicron Technology Corp. JMB368 IDE controller dead after resume when
async suspend is enabled ),

the reason : there are too much models inside the family of JMBxxx
JMicron SATA/IDE controlers PCI cards,  and the first patch targets ONLY
the JMB363/361 model, which is not enough :

if (pdev->vendor == PCI_VENDOR_ID_JMICRON &&
+		(pdev->device == PCI_DEVICE_ID_JMICRON_JMB363 ||
+		pdev->device == PCI_DEVICE_ID_JMICRON_JMB361))
+		device_disable_async_suspend(&pdev->dev);

for example I have a JMB363/368 JMicron SATA/IDE PCI card, and the first
patch created for the bug 81551 is not enough, that's why Chuansheng Liu
created a new patch who targets ALL models of JMicron JMBxxx SATA/IDE
cards, in order to be sure that these models of JMicron will have
"async_suspend feature disabled,

the good patch who works for all models of JMicron JMBxx SATA/IDE
controlers :

+     if (dev->vendor != PCI_VENDOR_ID_JMICRON)
+         device_enable_async_suspend(&dev->dev);
 	dev->wakeup_prepared = false;

	



Le 05/11/2014 20:03, Bjorn Helgaas a écrit :
> On Wed, Nov 5, 2014 at 11:46 AM, Barto <mister.freeman@laposte.net> wrote:
>> this patch solves these 2 bug reports :
>>
>> https://bugzilla.kernel.org/show_bug.cgi?id=
>>
>> https://bugzilla.kernel.org/show_bug.cgi?id=81551
> 
> Those bugs were already mentioned.  But e6b7e41cdd8c claims to solve
> https://bugzilla.kernel.org/show_bug.cgi?id=81551, and 84861 is a
> duplicate of 81551, so it should also be fixed by e6b7e41cdd8c.
> 
> So the question is, why was e6b7e41cdd8c insufficient?  Presumably it
> was tested and somebody thought it did fix the problem.
> 
>> in simple words : JMicron IDE/Sata controlers family ( JMBxxx ) are not
>> fully compatible with async_suspend feature, when a user tries to put
>> his PC on standby mode then at wake-up JMicron IDE/Sata controlers will
>> not work, because of a brother-relation between the SATA and IDE part on
>> this JMicron PCI card
>>
>>
>>
>>
>> Le 05/11/2014 19:01, Bjorn Helgaas a écrit :
>>> On Tue, Nov 4, 2014 at 6:31 PM, Chuansheng Liu <chuansheng.liu@intel.com> wrote:
>>>> The JMicron chip 361/363/368 contains one SATA controller and
>>>> one PATA controller, they are brother-relation ship in PCI tree,
>>>> but for powering on these both controller, we must follow the
>>>> sequence one by one, otherwise one of them can not be powered on
>>>> successfully.
>>>
>>> This should mention what's broken and what problem a user would see.
>>> This changelog sounds a lot like the one for e6b7e41cdd8c, so I don't
>>> know if this is for a new, related problem, or what.
>>>
>>>> So here we disable the async suspend method for Jmicron chip.
>>>>
>>>> Bug link:
>>>> https://bugzilla.kernel.org/show_bug.cgi?id=81551
>>>> https://bugzilla.kernel.org/show_bug.cgi?id=84861
>>>>
>>>> And we can revert the below commit after this patch is applied:
>>>> e6b7e41(ata: Disabling the async PM for JMicron chip 363/361)
>>>>
>>>> Cc: stable@vger.kernel.org # 3.15+
>>>> Acked-by: Aaron Lu <aaron.lu@intel.com>
>>>> Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
>>>> ---
>>>>  drivers/pci/pci.c |   12 +++++++++++-
>>>>  1 file changed, 11 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>>>> index 625a4ac..53128f0 100644
>>>> --- a/drivers/pci/pci.c
>>>> +++ b/drivers/pci/pci.c
>>>> @@ -2046,7 +2046,17 @@ void pci_pm_init(struct pci_dev *dev)
>>>>         pm_runtime_forbid(&dev->dev);
>>>>         pm_runtime_set_active(&dev->dev);
>>>>         pm_runtime_enable(&dev->dev);
>>>> -       device_enable_async_suspend(&dev->dev);
>>>> +
>>>> +       /*
>>>> +        * The JMicron chip 361/363/368 contains one SATA controller and
>>>> +        * one PATA controller, they are brother-relation ship in PCI tree,
>>>> +        * but for powering on these both controller, we must follow the
>>>> +        * sequence one by one, otherwise one of them can not be powered on
>>>> +        * successfully, so here we disable the async suspend method for
>>>> +        * Jmicron chip.
>>>> +        */
>>>> +       if (dev->vendor != PCI_VENDOR_ID_JMICRON)
>>>> +               device_enable_async_suspend(&dev->dev);
>>>
>>> I don't like littering the core PCI code with vendor tests like this.
>>> This would be the only one, except for an ancient DECchip 21050 bridge
>>> erratum.
>>>
>>> And why would we want a test for *all* JMicron devices here, when you
>>> claim the problem only affects a few specific ones?
>>>
>>> And what's the story with the e6b7e41cdd8c ("ata: Disabling the async
>>> PM for JMicron chip 363/361") connection?  Is something broken even
>>> with e6b7e41cdd8c, and this is a better fix?  Or is this simply a
>>> different way of fixing the same problem?
>>>
>>>>         dev->wakeup_prepared = false;
>>>>
>>>>         dev->pm_cap = 0;
>>>> --
>>>> 1.7.9.5
>>>>
>>>
> 

RE: [PATCH] PCI: Do not enable async suspend for JMicron chips

[Liu, Chuansheng]

SGVsbG8gQmpvcm4sDQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogQmpv
cm4gSGVsZ2FhcyBbbWFpbHRvOmJoZWxnYWFzQGdvb2dsZS5jb21dDQo+IFNlbnQ6IFRodXJzZGF5
LCBOb3ZlbWJlciAwNiwgMjAxNCAzOjA0IEFNDQo+IFRvOiBCYXJ0bw0KPiBDYzogTGl1LCBDaHVh
bnNoZW5nOyBMdSwgQWFyb247IFRlanVuIEhlbzsgUmFmYWVsIFd5c29ja2k7DQo+IGxpbnV4LXBj
aUB2Z2VyLmtlcm5lbC5vcmc7IGxpbnV4LWtlcm5lbEB2Z2VyLmtlcm5lbC5vcmcNCj4gU3ViamVj
dDogUmU6IFtQQVRDSF0gUENJOiBEbyBub3QgZW5hYmxlIGFzeW5jIHN1c3BlbmQgZm9yIEpNaWNy
b24gY2hpcHMNCj4gDQo+IE9uIFdlZCwgTm92IDUsIDIwMTQgYXQgMTE6NDYgQU0sIEJhcnRvIDxt
aXN0ZXIuZnJlZW1hbkBsYXBvc3RlLm5ldD4NCj4gd3JvdGU6DQo+ID4gdGhpcyBwYXRjaCBzb2x2
ZXMgdGhlc2UgMiBidWcgcmVwb3J0cyA6DQo+ID4NCj4gPiBodHRwczovL2J1Z3ppbGxhLmtlcm5l
bC5vcmcvc2hvd19idWcuY2dpP2lkPTg0ODYxDQo+ID4NCj4gPiBodHRwczovL2J1Z3ppbGxhLmtl
cm5lbC5vcmcvc2hvd19idWcuY2dpP2lkPTgxNTUxDQo+IA0KPiBUaG9zZSBidWdzIHdlcmUgYWxy
ZWFkeSBtZW50aW9uZWQuICBCdXQgZTZiN2U0MWNkZDhjIGNsYWltcyB0byBzb2x2ZQ0KPiBodHRw
czovL2J1Z3ppbGxhLmtlcm5lbC5vcmcvc2hvd19idWcuY2dpP2lkPTgxNTUxLCBhbmQgODQ4NjEg
aXMgYQ0KPiBkdXBsaWNhdGUgb2YgODE1NTEsIHNvIGl0IHNob3VsZCBhbHNvIGJlIGZpeGVkIGJ5
IGU2YjdlNDFjZGQ4Yy4NCj4gDQo+IFNvIHRoZSBxdWVzdGlvbiBpcywgd2h5IHdhcyBlNmI3ZTQx
Y2RkOGMgaW5zdWZmaWNpZW50PyAgUHJlc3VtYWJseSBpdA0KPiB3YXMgdGVzdGVkIGFuZCBzb21l
Ym9keSB0aG91Z2h0IGl0IGRpZCBmaXggdGhlIHByb2JsZW0uDQoNClRoZSBmaXJzdCBwYXRjaCBl
NmI3ZTQxY2RkOGMgd2hpY2ggaXMganVzdCBleGNsdWRlIHNvbWUgb2YgSk1pY3JvbiBjaGlwcygz
NjMvMzYxKSBvdXQgb2YgYXN5bmNfc3VzcGVuZCwNCnRoZW4gQmFydG8gZm91bmQgdGhlIHNhbWUg
aXNzdWUgb24gSk1pY3JvbiAzNjgsIHNvIHdlIG5lZWQgb25lIG1vcmUgZ2VuZXJhbCBwYXRjaCB0
byBsZXQgSk1pY3JvbiBjaGlwcw0Kb3V0IG9mIGFzeW5jX3N1c3BlbmQsIHNvIHdlIG1ha2UgdGhp
cyBwYXRjaC4NCg0KQmpvcm4sIHRqLA0KQ291bGQgeW91IGtpbmRseSB0YWtlIHRoaXMgcGF0Y2g/
IEFzIEJhcnRvIHNhaWQsIGl0IGVmZmVjdGVkIHRoZSB1c2VyIGV4cGVyaWVuY2UgaW5kZWVkLCB0
aGFua3MuDQoNCg==

Re: [PATCH] PCI: Do not enable async suspend for JMicron chips

[Bjorn Helgaas]

On Wed, Nov 5, 2014 at 6:48 PM, Liu, Chuansheng
<chuansheng.liu@intel.com> wrote:
> Hello Bjorn,
>
>> -----Original Message-----
>> From: Bjorn Helgaas [mailto:bhelgaas@google.com]
>> Sent: Thursday, November 06, 2014 3:04 AM
>> To: Barto
>> Cc: Liu, Chuansheng; Lu, Aaron; Tejun Heo; Rafael Wysocki;
>> linux-pci@vger.kernel.org; linux-kernel@vger.kernel.org
>> Subject: Re: [PATCH] PCI: Do not enable async suspend for JMicron chips
>>
>> On Wed, Nov 5, 2014 at 11:46 AM, Barto <mister.freeman@laposte.net>
>> wrote:
>> > this patch solves these 2 bug reports :
>> >
>> > https://bugzilla.kernel.org/show_bug.cgi?id=84861
>> >
>> > https://bugzilla.kernel.org/show_bug.cgi?id=81551
>>
>> Those bugs were already mentioned.  But e6b7e41cdd8c claims to solve
>> https://bugzilla.kernel.org/show_bug.cgi?id=81551, and 84861 is a
>> duplicate of 81551, so it should also be fixed by e6b7e41cdd8c.
>>
>> So the question is, why was e6b7e41cdd8c insufficient?  Presumably it
>> was tested and somebody thought it did fix the problem.
>
> The first patch e6b7e41cdd8c which is just exclude some of JMicron chips(363/361) out of async_suspend,
> then Barto found the same issue on JMicron 368, so we need one more general patch to let JMicron chips
> out of async_suspend, so we make this patch.
>
> Bjorn, tj,
> Could you kindly take this patch? As Barto said, it effected the user experience indeed, thanks.

Thanks for clarifying the changelog as far as the different chips and
the different bugzillas.

But you haven't addressed my concerns about (1) putting a PCI vendor
ID check in the generic PCI core code, and (2) applying this to *all*
JMicron devices.  You might want to explore a quirk-type solution or
maybe just add the JMicron 368 to the checks added by e6b7e41cdd8c.

Bjorn

Re: [PATCH] PCI: Do not enable async suspend for JMicron chips

[Barto]

Hello Bjorn,

in my bugreport I have already tried to add the JMicron 368 in the "if
statement" and it didn't work, check my message here :

https://bugzilla.kernel.org/show_bug.cgi?id=84861#c11

if Chuansheng has choosen a more generic way ( applying the patch to all
JMicron devices  ) it's because also because we don't know how many
JMBxxx models could be affected by this bug, tomorrow maybe one user
would create a bug report about a "JMB369" pci card who have again this
bug, and maybe on month later another user with a "JMB382", it could be
a nightmare for Chuanseng if he had to create every time a new patch for
each model of JMicron,

so for the moment the better approach for me is to disable async_suspend
for all JMBxxx JMicron, Chuanseng's patch seems reasonnable, as long as
we don't know the exact list of JMBxxx models we can assume that all
JMicron SATA/IDE are affected by this problem


Le 06/11/2014 05:08, Bjorn Helgaas a écrit :

> 
> But you haven't addressed my concerns about (1) putting a PCI vendor
> ID check in the generic PCI core code, and (2) applying this to *all*
> JMicron devices.  You might want to explore a quirk-type solution or
> maybe just add the JMicron 368 to the checks added by e6b7e41cdd8c.
> 
> Bjorn
> 

RE: [PATCH] PCI: Do not enable async suspend for JMicron chips

[Liu, Chuansheng]

SGVsbG8gQmpvcm4sDQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogQmpv
cm4gSGVsZ2FhcyBbbWFpbHRvOmJoZWxnYWFzQGdvb2dsZS5jb21dDQo+IFNlbnQ6IFRodXJzZGF5
LCBOb3ZlbWJlciAwNiwgMjAxNCAxMjowOSBQTQ0KPiBUbzogTGl1LCBDaHVhbnNoZW5nDQo+IENj
OiBCYXJ0bzsgVGVqdW4gSGVvICh0akBrZXJuZWwub3JnKTsgTHUsIEFhcm9uOyBSYWZhZWwgV3lz
b2NraTsNCj4gbGludXgtcGNpQHZnZXIua2VybmVsLm9yZzsgbGludXgta2VybmVsQHZnZXIua2Vy
bmVsLm9yZw0KPiBTdWJqZWN0OiBSZTogW1BBVENIXSBQQ0k6IERvIG5vdCBlbmFibGUgYXN5bmMg
c3VzcGVuZCBmb3IgSk1pY3JvbiBjaGlwcw0KPiANCj4gT24gV2VkLCBOb3YgNSwgMjAxNCBhdCA2
OjQ4IFBNLCBMaXUsIENodWFuc2hlbmcNCj4gPGNodWFuc2hlbmcubGl1QGludGVsLmNvbT4gd3Jv
dGU6DQo+ID4gSGVsbG8gQmpvcm4sDQo+ID4NCj4gPj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0t
LS0NCj4gPj4gRnJvbTogQmpvcm4gSGVsZ2FhcyBbbWFpbHRvOmJoZWxnYWFzQGdvb2dsZS5jb21d
DQo+ID4+IFNlbnQ6IFRodXJzZGF5LCBOb3ZlbWJlciAwNiwgMjAxNCAzOjA0IEFNDQo+ID4+IFRv
OiBCYXJ0bw0KPiA+PiBDYzogTGl1LCBDaHVhbnNoZW5nOyBMdSwgQWFyb247IFRlanVuIEhlbzsg
UmFmYWVsIFd5c29ja2k7DQo+ID4+IGxpbnV4LXBjaUB2Z2VyLmtlcm5lbC5vcmc7IGxpbnV4LWtl
cm5lbEB2Z2VyLmtlcm5lbC5vcmcNCj4gPj4gU3ViamVjdDogUmU6IFtQQVRDSF0gUENJOiBEbyBu
b3QgZW5hYmxlIGFzeW5jIHN1c3BlbmQgZm9yIEpNaWNyb24gY2hpcHMNCj4gPj4NCj4gPj4gT24g
V2VkLCBOb3YgNSwgMjAxNCBhdCAxMTo0NiBBTSwgQmFydG8gPG1pc3Rlci5mcmVlbWFuQGxhcG9z
dGUubmV0Pg0KPiA+PiB3cm90ZToNCj4gPj4gPiB0aGlzIHBhdGNoIHNvbHZlcyB0aGVzZSAyIGJ1
ZyByZXBvcnRzIDoNCj4gPj4gPg0KPiA+PiA+IGh0dHBzOi8vYnVnemlsbGEua2VybmVsLm9yZy9z
aG93X2J1Zy5jZ2k/aWQ9ODQ4NjENCj4gPj4gPg0KPiA+PiA+IGh0dHBzOi8vYnVnemlsbGEua2Vy
bmVsLm9yZy9zaG93X2J1Zy5jZ2k/aWQ9ODE1NTENCj4gPj4NCj4gPj4gVGhvc2UgYnVncyB3ZXJl
IGFscmVhZHkgbWVudGlvbmVkLiAgQnV0IGU2YjdlNDFjZGQ4YyBjbGFpbXMgdG8gc29sdmUNCj4g
Pj4gaHR0cHM6Ly9idWd6aWxsYS5rZXJuZWwub3JnL3Nob3dfYnVnLmNnaT9pZD04MTU1MSwgYW5k
IDg0ODYxIGlzIGENCj4gPj4gZHVwbGljYXRlIG9mIDgxNTUxLCBzbyBpdCBzaG91bGQgYWxzbyBi
ZSBmaXhlZCBieSBlNmI3ZTQxY2RkOGMuDQo+ID4+DQo+ID4+IFNvIHRoZSBxdWVzdGlvbiBpcywg
d2h5IHdhcyBlNmI3ZTQxY2RkOGMgaW5zdWZmaWNpZW50PyAgUHJlc3VtYWJseSBpdA0KPiA+PiB3
YXMgdGVzdGVkIGFuZCBzb21lYm9keSB0aG91Z2h0IGl0IGRpZCBmaXggdGhlIHByb2JsZW0uDQo+
ID4NCj4gPiBUaGUgZmlyc3QgcGF0Y2ggZTZiN2U0MWNkZDhjIHdoaWNoIGlzIGp1c3QgZXhjbHVk
ZSBzb21lIG9mIEpNaWNyb24NCj4gY2hpcHMoMzYzLzM2MSkgb3V0IG9mIGFzeW5jX3N1c3BlbmQs
DQo+ID4gdGhlbiBCYXJ0byBmb3VuZCB0aGUgc2FtZSBpc3N1ZSBvbiBKTWljcm9uIDM2OCwgc28g
d2UgbmVlZCBvbmUgbW9yZQ0KPiBnZW5lcmFsIHBhdGNoIHRvIGxldCBKTWljcm9uIGNoaXBzDQo+
ID4gb3V0IG9mIGFzeW5jX3N1c3BlbmQsIHNvIHdlIG1ha2UgdGhpcyBwYXRjaC4NCj4gPg0KPiA+
IEJqb3JuLCB0aiwNCj4gPiBDb3VsZCB5b3Uga2luZGx5IHRha2UgdGhpcyBwYXRjaD8gQXMgQmFy
dG8gc2FpZCwgaXQgZWZmZWN0ZWQgdGhlIHVzZXINCj4gZXhwZXJpZW5jZSBpbmRlZWQsIHRoYW5r
cy4NCj4gDQo+IFRoYW5rcyBmb3IgY2xhcmlmeWluZyB0aGUgY2hhbmdlbG9nIGFzIGZhciBhcyB0
aGUgZGlmZmVyZW50IGNoaXBzIGFuZA0KPiB0aGUgZGlmZmVyZW50IGJ1Z3ppbGxhcy4NCj4gDQo+
IEJ1dCB5b3UgaGF2ZW4ndCBhZGRyZXNzZWQgbXkgY29uY2VybnMgYWJvdXQgKDEpIHB1dHRpbmcg
YSBQQ0kgdmVuZG9yDQo+IElEIGNoZWNrIGluIHRoZSBnZW5lcmljIFBDSSBjb3JlIGNvZGUsIGFu
ZCAoMikgYXBwbHlpbmcgdGhpcyB0byAqYWxsKg0KPiBKTWljcm9uIGRldmljZXMuICBZb3UgbWln
aHQgd2FudCB0byBleHBsb3JlIGEgcXVpcmstdHlwZSBzb2x1dGlvbiBvcg0KPiBtYXliZSBqdXN0
IGFkZCB0aGUgSk1pY3JvbiAzNjggdG8gdGhlIGNoZWNrcyBhZGRlZCBieSBlNmI3ZTQxY2RkOGMu
DQpVbmRlcnN0YW5kIHlvdXIgcG9pbnQsIGluIGZhY3QsIGJlZm9yZSB0aGlzIHBhdGNoIHN1Ym1p
dHRlZCwgSSBoYWQgd3JpdHRlbiBhbm90aGVyIHBhdGNoIGh0dHBzOi8vbGttbC5vcmcvbGttbC8y
MDE0LzkvMjQvNjgNCndoaWNoIGFkZHJlc3NlZCB0byBhZGQgdGhlIHF1aXJrLXR5cGUgc29sdXRp
b24gaW4gQVRBIGNvZGUsIGFuZCBBYXJvbiBnaXZlbiBiZXR0ZXIgc3VnZ2VzdGlvbiB0aGF0IGlt
cGxlbWVudGVkIGF0IHBjaV9wbV9pbml0KCkuDQpIb3cgZG8geW91IHRoaW5rIG9mIGl0PyBUaGFu
a3MuDQoNCg0K

Re: [PATCH] PCI: Do not enable async suspend for JMicron chips

[Aaron Lu]

On 11/06/2014 01:29 PM, Liu, Chuansheng wrote:
> Hello Bjorn,
> 
>> -----Original Message-----
>> From: Bjorn Helgaas [mailto:bhelgaas@google.com]
>> Sent: Thursday, November 06, 2014 12:09 PM
>> To: Liu, Chuansheng
>> Cc: Barto; Tejun Heo (tj@kernel.org); Lu, Aaron; Rafael Wysocki;
>> linux-pci@vger.kernel.org; linux-kernel@vger.kernel.org
>> Subject: Re: [PATCH] PCI: Do not enable async suspend for JMicron chips
>>
>> On Wed, Nov 5, 2014 at 6:48 PM, Liu, Chuansheng
>> <chuansheng.liu@intel.com> wrote:
>>> Hello Bjorn,
>>>
>>>> -----Original Message-----
>>>> From: Bjorn Helgaas [mailto:bhelgaas@google.com]
>>>> Sent: Thursday, November 06, 2014 3:04 AM
>>>> To: Barto
>>>> Cc: Liu, Chuansheng; Lu, Aaron; Tejun Heo; Rafael Wysocki;
>>>> linux-pci@vger.kernel.org; linux-kernel@vger.kernel.org
>>>> Subject: Re: [PATCH] PCI: Do not enable async suspend for JMicron chips
>>>>
>>>> On Wed, Nov 5, 2014 at 11:46 AM, Barto <mister.freeman@laposte.net>
>>>> wrote:
>>>>> this patch solves these 2 bug reports :
>>>>>
>>>>> https://bugzilla.kernel.org/show_bug.cgi?id=84861
>>>>>
>>>>> https://bugzilla.kernel.org/show_bug.cgi?id=81551
>>>>
>>>> Those bugs were already mentioned.  But e6b7e41cdd8c claims to solve
>>>> https://bugzilla.kernel.org/show_bug.cgi?id=81551, and 84861 is a
>>>> duplicate of 81551, so it should also be fixed by e6b7e41cdd8c.
>>>>
>>>> So the question is, why was e6b7e41cdd8c insufficient?  Presumably it
>>>> was tested and somebody thought it did fix the problem.
>>>
>>> The first patch e6b7e41cdd8c which is just exclude some of JMicron
>> chips(363/361) out of async_suspend,
>>> then Barto found the same issue on JMicron 368, so we need one more
>> general patch to let JMicron chips
>>> out of async_suspend, so we make this patch.
>>>
>>> Bjorn, tj,
>>> Could you kindly take this patch? As Barto said, it effected the user
>> experience indeed, thanks.
>>
>> Thanks for clarifying the changelog as far as the different chips and
>> the different bugzillas.
>>
>> But you haven't addressed my concerns about (1) putting a PCI vendor
>> ID check in the generic PCI core code, and (2) applying this to *all*
>> JMicron devices.  You might want to explore a quirk-type solution or
>> maybe just add the JMicron 368 to the checks added by e6b7e41cdd8c.
> Understand your point, in fact, before this patch submitted, I had written another patch https://lkml.org/lkml/2014/9/24/68
> which addressed to add the quirk-type solution in ATA code, and Aaron given better suggestion that implemented at pci_pm_init().
> How do you think of it? Thanks.

I think Bjorn means that we should place the code as a fixup somewhere
in the quirks.c. I didn't take a closer look but DECLARE_PCI_FIXUP_FINAL
for those JMicron PCI devices seems to be a proper phase.

Thanks,
Aaron

RE: [PATCH] PCI: Do not enable async suspend for JMicron chips

[Liu, Chuansheng]

DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogTHUsIEFhcm9uDQo+IFNl
bnQ6IFRodXJzZGF5LCBOb3ZlbWJlciAwNiwgMjAxNCAxOjM3IFBNDQo+IFRvOiBMaXUsIENodWFu
c2hlbmc7IEJqb3JuIEhlbGdhYXMNCj4gQ2M6IEJhcnRvOyBUZWp1biBIZW8gKHRqQGtlcm5lbC5v
cmcpOyBSYWZhZWwgV3lzb2NraTsNCj4gbGludXgtcGNpQHZnZXIua2VybmVsLm9yZzsgbGludXgt
a2VybmVsQHZnZXIua2VybmVsLm9yZw0KPiBTdWJqZWN0OiBSZTogW1BBVENIXSBQQ0k6IERvIG5v
dCBlbmFibGUgYXN5bmMgc3VzcGVuZCBmb3IgSk1pY3JvbiBjaGlwcw0KPiANCj4gT24gMTEvMDYv
MjAxNCAwMToyOSBQTSwgTGl1LCBDaHVhbnNoZW5nIHdyb3RlOg0KPiA+IEhlbGxvIEJqb3JuLA0K
PiA+DQo+ID4+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+ID4+IEZyb206IEJqb3JuIEhl
bGdhYXMgW21haWx0bzpiaGVsZ2Fhc0Bnb29nbGUuY29tXQ0KPiA+PiBTZW50OiBUaHVyc2RheSwg
Tm92ZW1iZXIgMDYsIDIwMTQgMTI6MDkgUE0NCj4gPj4gVG86IExpdSwgQ2h1YW5zaGVuZw0KPiA+
PiBDYzogQmFydG87IFRlanVuIEhlbyAodGpAa2VybmVsLm9yZyk7IEx1LCBBYXJvbjsgUmFmYWVs
IFd5c29ja2k7DQo+ID4+IGxpbnV4LXBjaUB2Z2VyLmtlcm5lbC5vcmc7IGxpbnV4LWtlcm5lbEB2
Z2VyLmtlcm5lbC5vcmcNCj4gPj4gU3ViamVjdDogUmU6IFtQQVRDSF0gUENJOiBEbyBub3QgZW5h
YmxlIGFzeW5jIHN1c3BlbmQgZm9yIEpNaWNyb24gY2hpcHMNCj4gPj4NCj4gPj4gT24gV2VkLCBO
b3YgNSwgMjAxNCBhdCA2OjQ4IFBNLCBMaXUsIENodWFuc2hlbmcNCj4gPj4gPGNodWFuc2hlbmcu
bGl1QGludGVsLmNvbT4gd3JvdGU6DQo+ID4+PiBIZWxsbyBCam9ybiwNCj4gPj4+DQo+ID4+Pj4g
LS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gPj4+PiBGcm9tOiBCam9ybiBIZWxnYWFzIFtt
YWlsdG86YmhlbGdhYXNAZ29vZ2xlLmNvbV0NCj4gPj4+PiBTZW50OiBUaHVyc2RheSwgTm92ZW1i
ZXIgMDYsIDIwMTQgMzowNCBBTQ0KPiA+Pj4+IFRvOiBCYXJ0bw0KPiA+Pj4+IENjOiBMaXUsIENo
dWFuc2hlbmc7IEx1LCBBYXJvbjsgVGVqdW4gSGVvOyBSYWZhZWwgV3lzb2NraTsNCj4gPj4+PiBs
aW51eC1wY2lAdmdlci5rZXJuZWwub3JnOyBsaW51eC1rZXJuZWxAdmdlci5rZXJuZWwub3JnDQo+
ID4+Pj4gU3ViamVjdDogUmU6IFtQQVRDSF0gUENJOiBEbyBub3QgZW5hYmxlIGFzeW5jIHN1c3Bl
bmQgZm9yIEpNaWNyb24gY2hpcHMNCj4gPj4+Pg0KPiA+Pj4+IE9uIFdlZCwgTm92IDUsIDIwMTQg
YXQgMTE6NDYgQU0sIEJhcnRvIDxtaXN0ZXIuZnJlZW1hbkBsYXBvc3RlLm5ldD4NCj4gPj4+PiB3
cm90ZToNCj4gPj4+Pj4gdGhpcyBwYXRjaCBzb2x2ZXMgdGhlc2UgMiBidWcgcmVwb3J0cyA6DQo+
ID4+Pj4+DQo+ID4+Pj4+IGh0dHBzOi8vYnVnemlsbGEua2VybmVsLm9yZy9zaG93X2J1Zy5jZ2k/
aWQ9ODQ4NjENCj4gPj4+Pj4NCj4gPj4+Pj4gaHR0cHM6Ly9idWd6aWxsYS5rZXJuZWwub3JnL3No
b3dfYnVnLmNnaT9pZD04MTU1MQ0KPiA+Pj4+DQo+ID4+Pj4gVGhvc2UgYnVncyB3ZXJlIGFscmVh
ZHkgbWVudGlvbmVkLiAgQnV0IGU2YjdlNDFjZGQ4YyBjbGFpbXMgdG8gc29sdmUNCj4gPj4+PiBo
dHRwczovL2J1Z3ppbGxhLmtlcm5lbC5vcmcvc2hvd19idWcuY2dpP2lkPTgxNTUxLCBhbmQgODQ4
NjEgaXMgYQ0KPiA+Pj4+IGR1cGxpY2F0ZSBvZiA4MTU1MSwgc28gaXQgc2hvdWxkIGFsc28gYmUg
Zml4ZWQgYnkgZTZiN2U0MWNkZDhjLg0KPiA+Pj4+DQo+ID4+Pj4gU28gdGhlIHF1ZXN0aW9uIGlz
LCB3aHkgd2FzIGU2YjdlNDFjZGQ4YyBpbnN1ZmZpY2llbnQ/ICBQcmVzdW1hYmx5IGl0DQo+ID4+
Pj4gd2FzIHRlc3RlZCBhbmQgc29tZWJvZHkgdGhvdWdodCBpdCBkaWQgZml4IHRoZSBwcm9ibGVt
Lg0KPiA+Pj4NCj4gPj4+IFRoZSBmaXJzdCBwYXRjaCBlNmI3ZTQxY2RkOGMgd2hpY2ggaXMganVz
dCBleGNsdWRlIHNvbWUgb2YgSk1pY3Jvbg0KPiA+PiBjaGlwcygzNjMvMzYxKSBvdXQgb2YgYXN5
bmNfc3VzcGVuZCwNCj4gPj4+IHRoZW4gQmFydG8gZm91bmQgdGhlIHNhbWUgaXNzdWUgb24gSk1p
Y3JvbiAzNjgsIHNvIHdlIG5lZWQgb25lIG1vcmUNCj4gPj4gZ2VuZXJhbCBwYXRjaCB0byBsZXQg
Sk1pY3JvbiBjaGlwcw0KPiA+Pj4gb3V0IG9mIGFzeW5jX3N1c3BlbmQsIHNvIHdlIG1ha2UgdGhp
cyBwYXRjaC4NCj4gPj4+DQo+ID4+PiBCam9ybiwgdGosDQo+ID4+PiBDb3VsZCB5b3Uga2luZGx5
IHRha2UgdGhpcyBwYXRjaD8gQXMgQmFydG8gc2FpZCwgaXQgZWZmZWN0ZWQgdGhlIHVzZXINCj4g
Pj4gZXhwZXJpZW5jZSBpbmRlZWQsIHRoYW5rcy4NCj4gPj4NCj4gPj4gVGhhbmtzIGZvciBjbGFy
aWZ5aW5nIHRoZSBjaGFuZ2Vsb2cgYXMgZmFyIGFzIHRoZSBkaWZmZXJlbnQgY2hpcHMgYW5kDQo+
ID4+IHRoZSBkaWZmZXJlbnQgYnVnemlsbGFzLg0KPiA+Pg0KPiA+PiBCdXQgeW91IGhhdmVuJ3Qg
YWRkcmVzc2VkIG15IGNvbmNlcm5zIGFib3V0ICgxKSBwdXR0aW5nIGEgUENJIHZlbmRvcg0KPiA+
PiBJRCBjaGVjayBpbiB0aGUgZ2VuZXJpYyBQQ0kgY29yZSBjb2RlLCBhbmQgKDIpIGFwcGx5aW5n
IHRoaXMgdG8gKmFsbCoNCj4gPj4gSk1pY3JvbiBkZXZpY2VzLiAgWW91IG1pZ2h0IHdhbnQgdG8g
ZXhwbG9yZSBhIHF1aXJrLXR5cGUgc29sdXRpb24gb3INCj4gPj4gbWF5YmUganVzdCBhZGQgdGhl
IEpNaWNyb24gMzY4IHRvIHRoZSBjaGVja3MgYWRkZWQgYnkgZTZiN2U0MWNkZDhjLg0KPiA+IFVu
ZGVyc3RhbmQgeW91ciBwb2ludCwgaW4gZmFjdCwgYmVmb3JlIHRoaXMgcGF0Y2ggc3VibWl0dGVk
LCBJIGhhZCB3cml0dGVuDQo+IGFub3RoZXIgcGF0Y2ggaHR0cHM6Ly9sa21sLm9yZy9sa21sLzIw
MTQvOS8yNC82OA0KPiA+IHdoaWNoIGFkZHJlc3NlZCB0byBhZGQgdGhlIHF1aXJrLXR5cGUgc29s
dXRpb24gaW4gQVRBIGNvZGUsIGFuZCBBYXJvbiBnaXZlbg0KPiBiZXR0ZXIgc3VnZ2VzdGlvbiB0
aGF0IGltcGxlbWVudGVkIGF0IHBjaV9wbV9pbml0KCkuDQo+ID4gSG93IGRvIHlvdSB0aGluayBv
ZiBpdD8gVGhhbmtzLg0KPiANCj4gSSB0aGluayBCam9ybiBtZWFucyB0aGF0IHdlIHNob3VsZCBw
bGFjZSB0aGUgY29kZSBhcyBhIGZpeHVwIHNvbWV3aGVyZQ0KPiBpbiB0aGUgcXVpcmtzLmMuIEkg
ZGlkbid0IHRha2UgYSBjbG9zZXIgbG9vayBidXQgREVDTEFSRV9QQ0lfRklYVVBfRklOQUwNCj4g
Zm9yIHRob3NlIEpNaWNyb24gUENJIGRldmljZXMgc2VlbXMgdG8gYmUgYSBwcm9wZXIgcGhhc2Uu
DQoNClRoYW5rcyBBYXJvbiwgdGhlbiBob3cgYWJvdXQgYmVsb3cgcGF0Y2g/DQoNCmRpZmYgLS1n
aXQgYS9kcml2ZXJzL2F0YS9wYXRhX2ptaWNyb24uYyBiL2RyaXZlcnMvYXRhL3BhdGFfam1pY3Jv
bi5jDQppbmRleCA0N2U0MThiLi45ZTg1Zjg2IDEwMDY0NA0KLS0tIGEvZHJpdmVycy9hdGEvcGF0
YV9qbWljcm9uLmMNCisrKyBiL2RyaXZlcnMvYXRhL3BhdGFfam1pY3Jvbi5jDQpAQCAtMTU4LDYg
KzE1OCwyMSBAQCBzdGF0aWMgaW50IGptaWNyb25faW5pdF9vbmUgKHN0cnVjdCBwY2lfZGV2ICpw
ZGV2LCBjb25zdCBzdHJ1Y3QgcGNpX2RldmljZV9pZCAqaQ0KICAgICAgICByZXR1cm4gYXRhX3Bj
aV9ibWRtYV9pbml0X29uZShwZGV2LCBwcGksICZqbWljcm9uX3NodCwgTlVMTCwgMCk7DQogfQ0K
IA0KKy8qDQorICogRm9yIEpNaWNyb24gY2hpcHMsIHdlIG5lZWQgdG8gZGlzYWJsZSB0aGUgYXN5
bmNfc3VzcGVuZCBtZXRob2QsIG90aGVyd2lzZQ0KKyAqIHRoZXkgd2lsbCBoaXQgdGhlIHBvd2Vy
LW9uIGlzc3VlIHdoZW4gZG9pbmcgZGV2aWNlIHJlc3VtZSwgYWRkIG9uZSBxdWljaw0KKyAqIHNv
bHV0aW9uIHRvIGRpc2FibGUgdGhlIGFzeW5jX3N1c3BlbmQgbWV0aG9kLg0KKyAqLw0KK3N0YXRp
YyB2b2lkIHBjaV9hc3luY19zdXNwZW5kX2ZpeHVwKHN0cnVjdCBwY2lfZGV2ICpwZGV2KQ0KK3sN
CisgICAgICAgLyoNCisgICAgICAgICogZGlzYWJsaW5nIHRoZSBhc3luY19zdXNwZW5kIG1ldGhv
ZCBmb3IgSk1pY3JvbiBjaGlwcyB0bw0KKyAgICAgICAgKiBhdm9pZCBkZXZpY2UgcmVzdW1pbmcg
aXNzdWUuDQorICAgICAgICAqLw0KKyAgICAgICBkZXZpY2VfZGlzYWJsZV9hc3luY19zdXNwZW5k
KCZwZGV2LT5kZXYpOw0KK30NCitERUNMQVJFX1BDSV9GSVhVUF9GSU5BTChQQ0lfVkVORE9SX0lE
X0pNSUNST04sIFBDSV9BTllfSUQsIHBjaV9hc3luY19zdXNwZW5kX2ZpeHVwKTsNCisNCiBzdGF0
aWMgY29uc3Qgc3RydWN0IHBjaV9kZXZpY2VfaWQgam1pY3Jvbl9wY2lfdGJsW10gPSB7DQogICAg
ICAgIHsgUENJX1ZFTkRPUl9JRF9KTUlDUk9OLCBQQ0lfQU5ZX0lELCBQQ0lfQU5ZX0lELCBQQ0lf
QU5ZX0lELA0KICAgICAgICAgIFBDSV9DTEFTU19TVE9SQUdFX0lERSA8PCA4LCAweGZmZmYwMCwg
MCB9LA0KDQpCYXJ0bywNCkNvdWxkIHlvdSBoYXZlIGEgdHJ5IG9uIHlvdXIgc2lkZT8gVGhhbmtz
Lg0KDQoNCg==

Re: [PATCH] PCI: Do not enable async suspend for JMicron chips

[Barto]

I tried your patch and it doesn't work,

I think you have forgotten something, maybe you need also to modify the
file /drivers/ata/ahci.c and not only /drivers/ata/pata_jmicron.c

don't forget that I have a JMB363/368 SATA/IDE controller PCIe, which is
both a SATA and IDE controller in one PCie card, and a IDE hardisk is
connected on this JMB363/368 SATA/IDE controller PCIe,

for now the patch who works is this :

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2046,7 +2046,17 @@ void pci_pm_init(struct pci_dev *dev)
 	pm_runtime_forbid(&dev->dev);
 	pm_runtime_set_active(&dev->dev);
 	pm_runtime_enable(&dev->dev);
-	device_enable_async_suspend(&dev->dev);
+	
+     /*
+      * The JMicron chip 361/363/368 contains one SATA controller and
+      * one PATA controller, they are brother-relation ship in PCI tree,
+      * but for powering on these both controller, we must follow the
+      * sequence one by one, otherwise one of them can not be powered on
+      * successfully, so here we disable the async suspend method for
+      * Jmicron chip.
+      */
+     if (dev->vendor != PCI_VENDOR_ID_JMICRON)
+         device_enable_async_suspend(&dev->dev);
 	dev->wakeup_prepared = false;

 	dev->pm_cap = 0;


Le 06/11/2014 07:39, Liu, Chuansheng a écrit :
> 

> diff --git a/drivers/ata/pata_jmicron.c b/drivers/ata/pata_jmicron.c
> index 47e418b..9e85f86 100644
> --- a/drivers/ata/pata_jmicron.c
> +++ b/drivers/ata/pata_jmicron.c
> @@ -158,6 +158,21 @@ static int jmicron_init_one (struct pci_dev *pdev, const struct pci_device_id *i
>         return ata_pci_bmdma_init_one(pdev, ppi, &jmicron_sht, NULL, 0);
>  }
>  
> +/*
> + * 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 quick
> + * 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_FINAL(PCI_VENDOR_ID_JMICRON, PCI_ANY_ID, pci_async_suspend_fixup);
> +
>  static const struct pci_device_id jmicron_pci_tbl[] = {
>         { PCI_VENDOR_ID_JMICRON, PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
>           PCI_CLASS_STORAGE_IDE << 8, 0xffff00, 0 },
> 
> Barto,
> Could you have a try on your side? Thanks.
> 
> 

Re: [PATCH] PCI: Do not enable async suspend for JMicron chips

[Bjorn Helgaas]

On Wed, Nov 5, 2014 at 11:39 PM, Liu, Chuansheng
<chuansheng.liu@intel.com> wrote:
>
>
>> -----Original Message-----
>> From: Lu, Aaron
>> Sent: Thursday, November 06, 2014 1:37 PM
>> To: Liu, Chuansheng; Bjorn Helgaas
>> Cc: Barto; Tejun Heo (tj@kernel.org); Rafael Wysocki;
>> linux-pci@vger.kernel.org; linux-kernel@vger.kernel.org
>> Subject: Re: [PATCH] PCI: Do not enable async suspend for JMicron chips
>>
>> On 11/06/2014 01:29 PM, Liu, Chuansheng wrote:
>> > Hello Bjorn,
>> >
>> >> -----Original Message-----
>> >> From: Bjorn Helgaas [mailto:bhelgaas@google.com]
>> >> Sent: Thursday, November 06, 2014 12:09 PM
>> >> To: Liu, Chuansheng
>> >> Cc: Barto; Tejun Heo (tj@kernel.org); Lu, Aaron; Rafael Wysocki;
>> >> linux-pci@vger.kernel.org; linux-kernel@vger.kernel.org
>> >> Subject: Re: [PATCH] PCI: Do not enable async suspend for JMicron chips
>> >>
>> >> On Wed, Nov 5, 2014 at 6:48 PM, Liu, Chuansheng
>> >> <chuansheng.liu@intel.com> wrote:
>> >>> Hello Bjorn,
>> >>>
>> >>>> -----Original Message-----
>> >>>> From: Bjorn Helgaas [mailto:bhelgaas@google.com]
>> >>>> Sent: Thursday, November 06, 2014 3:04 AM
>> >>>> To: Barto
>> >>>> Cc: Liu, Chuansheng; Lu, Aaron; Tejun Heo; Rafael Wysocki;
>> >>>> linux-pci@vger.kernel.org; linux-kernel@vger.kernel.org
>> >>>> Subject: Re: [PATCH] PCI: Do not enable async suspend for JMicron chips
>> >>>>
>> >>>> On Wed, Nov 5, 2014 at 11:46 AM, Barto <mister.freeman@laposte.net>
>> >>>> wrote:
>> >>>>> this patch solves these 2 bug reports :
>> >>>>>
>> >>>>> https://bugzilla.kernel.org/show_bug.cgi?id=84861
>> >>>>>
>> >>>>> https://bugzilla.kernel.org/show_bug.cgi?id=81551
>> >>>>
>> >>>> Those bugs were already mentioned.  But e6b7e41cdd8c claims to solve
>> >>>> https://bugzilla.kernel.org/show_bug.cgi?id=81551, and 84861 is a
>> >>>> duplicate of 81551, so it should also be fixed by e6b7e41cdd8c.
>> >>>>
>> >>>> So the question is, why was e6b7e41cdd8c insufficient?  Presumably it
>> >>>> was tested and somebody thought it did fix the problem.
>> >>>
>> >>> The first patch e6b7e41cdd8c which is just exclude some of JMicron
>> >> chips(363/361) out of async_suspend,
>> >>> then Barto found the same issue on JMicron 368, so we need one more
>> >> general patch to let JMicron chips
>> >>> out of async_suspend, so we make this patch.
>> >>>
>> >>> Bjorn, tj,
>> >>> Could you kindly take this patch? As Barto said, it effected the user
>> >> experience indeed, thanks.
>> >>
>> >> Thanks for clarifying the changelog as far as the different chips and
>> >> the different bugzillas.
>> >>
>> >> But you haven't addressed my concerns about (1) putting a PCI vendor
>> >> ID check in the generic PCI core code, and (2) applying this to *all*
>> >> JMicron devices.  You might want to explore a quirk-type solution or
>> >> maybe just add the JMicron 368 to the checks added by e6b7e41cdd8c.
>> > Understand your point, in fact, before this patch submitted, I had written
>> another patch https://lkml.org/lkml/2014/9/24/68
>> > which addressed to add the quirk-type solution in ATA code, and Aaron given
>> better suggestion that implemented at pci_pm_init().
>> > How do you think of it? Thanks.
>>
>> I think Bjorn means that we should place the code as a fixup somewhere
>> in the quirks.c. I didn't take a closer look but DECLARE_PCI_FIXUP_FINAL
>> for those JMicron PCI devices seems to be a proper phase.
>
> Thanks Aaron, then how about below patch?
>
> diff --git a/drivers/ata/pata_jmicron.c b/drivers/ata/pata_jmicron.c
> index 47e418b..9e85f86 100644
> --- a/drivers/ata/pata_jmicron.c
> +++ b/drivers/ata/pata_jmicron.c
> @@ -158,6 +158,21 @@ static int jmicron_init_one (struct pci_dev *pdev, const struct pci_device_id *i
>         return ata_pci_bmdma_init_one(pdev, ppi, &jmicron_sht, NULL, 0);
>  }
>
> +/*
> + * 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 quick
> + * solution to disable the async_suspend method.

A "quick solution" is a red flag for me, because it's a hint that you
just want the problem to go away without fully understanding it.
That's probably not the case; it's probably just that *I* don't
understand it all yet.

> +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_FINAL(PCI_VENDOR_ID_JMICRON, PCI_ANY_ID, pci_async_suspend_fixup);

I know Barto tested this and it didn't work, but this *strategy* is
far better than adding the JMicron test in pci_pm_init().  It's ideal
if a quirk like this could be in the pata_jmicron.c file, so it's only
included when that driver is loaded.  But it would still be OK if it
had to be in drivers/pci/quirks.c, e.g., if something has to be done
even before the driver is loaded.

The idea of a quirk is to work around a defect in a device.  What is
the defect in this case?  It seems there are two devices involved,
e.g. (from https://bugzilla.kernel.org/show_bug.cgi?id=81551):

  02:00.0 JMicron Technology Corp. JMB363 SATA/IDE Controller
  02:00.1 JMicron Technology Corp. JMB363 SATA/IDE Controller

The PCI power management code is designed to work correctly with all
devices that conform to the spec.  So either one of these devices
doesn't conform to the spec, or the PM code is assuming some behavior
that the spec doesn't actually require.  If the latter, we need to fix
the PM code, because it won't work with other non-JMicron devices
either.

I haven't seen reports of other devices, so my guess is that it really
*is* a defect in these JMicron devices, but I still need a better
understanding of exactly what's broken.  Maybe it's something like:

  - 02:00.0 and 02:00.1 are both in D3
  - if we try to put 02:00.1 in D0 first, it fails ("Refused to change
power state, currently in D3")

and the resolution is to change 02:00.0 from D3 to D0 first, then
change 02:00.1 from D3 to D0.  Or maybe it's just that there needs to
be a delay between changing 02:00.0 and changing 02:00.1 (since I
suspect we *start* those changes in that order anyway)?

In either case it sounds like a device defect, because I don't see
anything in the PCI Power Management spec about ordering requirements
for multifunction devices.

Speaking of ordering, what is it that guarantees 02:00.0 will be
powered up before 02:00.1 when async suspend is disabled?  Is it the
dpm_noirq_list order in dpm_resume_noirq()?

I don't know how much we gain by allowing async resume of
multifunction devices.  Is it possible that Windows always powers up
multifunction devices in order?  I doubt that Windows would have a
quirk similar to what you're proposing here, so I wonder how they deal
with this issue.

It sounds like this issue only affects multifunction JMicron devices,
so possibly the quirk could be made smarter by only disabling async
suspend when it finds those.

Per https://bugzilla.kernel.org/show_bug.cgi?id=81551#c8, this is a
regression introduced by 76569faa62c4 ("PM / sleep: Asynchronous
threads for resume_noirq"), which appeared in v3.15.  That information
needs to be in the changelog.

Rafael, do you want to jump in here?  I know almost nothing about the
PCI power management code, so if you want to take over this whole
thing, that would be fine by me.

Bjorn

Re: [PATCH] PCI: Do not enable async suspend for JMicron chips

[Barto]

> The idea of a quirk is to work around a defect in a device.  What is
> the defect in this case?  It seems there are two devices involved,
> e.g. (from https://bugzilla.kernel.org/show_bug.cgi?id=81551):
> 
>   02:00.0 JMicron Technology Corp. JMB363 SATA/IDE Controller
>   02:00.1 JMicron Technology Corp. JMB363 SATA/IDE Controller
> 

in my case I don't have exactly the same lines in dmesg,

my JMicron JMB363/368 seems to have a different design, it's not exactly
identical to JMB363 SATA/IDE Controller, in dmesg I can read this :

dmesg | grep micron

[    0.860659] pata_jmicron 0000:03:00.1: enabling device (0000 -> 0001)
[    0.866760] scsi0 : pata_jmicron
[    0.870045] scsi1 : pata_jmicron

lspci :

lspci | grep JMicron

03:00.0 SATA controller: JMicron Technology Corp. JMB363 SATA/IDE
Controller (rev 10)
03:00.1 IDE interface: JMicron Technology Corp. JMB368 IDE controller
(rev 10)

RE: [PATCH] PCI: Do not enable async suspend for JMicron chips

[Liu, Chuansheng]

SGVsbG8gQmpvcm4sDQoNCldpbGwgc2VuZCBvdXQgb25lIG5ldyBxdWlyay1zb2x1dGlvbiwgYW5k
IHNvbWUgcmVhY3Rpb24gYmVsb3c6KQ0KDQo+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+
IEZyb206IEJqb3JuIEhlbGdhYXMgW21haWx0bzpiaGVsZ2Fhc0Bnb29nbGUuY29tXQ0KPiBTZW50
OiBGcmlkYXksIE5vdmVtYmVyIDA3LCAyMDE0IDE6MzkgQU0NCj4gVG86IExpdSwgQ2h1YW5zaGVu
Zw0KPiBDYzogTHUsIEFhcm9uOyBCYXJ0bzsgVGVqdW4gSGVvICh0akBrZXJuZWwub3JnKTsgUmFm
YWVsIFd5c29ja2k7DQo+IGxpbnV4LXBjaUB2Z2VyLmtlcm5lbC5vcmc7IGxpbnV4LWtlcm5lbEB2
Z2VyLmtlcm5lbC5vcmcNCj4gU3ViamVjdDogUmU6IFtQQVRDSF0gUENJOiBEbyBub3QgZW5hYmxl
IGFzeW5jIHN1c3BlbmQgZm9yIEpNaWNyb24gY2hpcHMNCj4gDQo+IE9uIFdlZCwgTm92IDUsIDIw
MTQgYXQgMTE6MzkgUE0sIExpdSwgQ2h1YW5zaGVuZw0KPiA8Y2h1YW5zaGVuZy5saXVAaW50ZWwu
Y29tPiB3cm90ZToNCj4gPg0KPiA+DQo+ID4+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+
ID4+IEZyb206IEx1LCBBYXJvbg0KPiA+PiBTZW50OiBUaHVyc2RheSwgTm92ZW1iZXIgMDYsIDIw
MTQgMTozNyBQTQ0KPiA+PiBUbzogTGl1LCBDaHVhbnNoZW5nOyBCam9ybiBIZWxnYWFzDQo+ID4+
IENjOiBCYXJ0bzsgVGVqdW4gSGVvICh0akBrZXJuZWwub3JnKTsgUmFmYWVsIFd5c29ja2k7DQo+
ID4+IGxpbnV4LXBjaUB2Z2VyLmtlcm5lbC5vcmc7IGxpbnV4LWtlcm5lbEB2Z2VyLmtlcm5lbC5v
cmcNCj4gPj4gU3ViamVjdDogUmU6IFtQQVRDSF0gUENJOiBEbyBub3QgZW5hYmxlIGFzeW5jIHN1
c3BlbmQgZm9yIEpNaWNyb24gY2hpcHMNCj4gPj4NCj4gPj4gT24gMTEvMDYvMjAxNCAwMToyOSBQ
TSwgTGl1LCBDaHVhbnNoZW5nIHdyb3RlOg0KPiA+PiA+IEhlbGxvIEJqb3JuLA0KPiA+PiA+DQo+
ID4+ID4+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+ID4+ID4+IEZyb206IEJqb3JuIEhl
bGdhYXMgW21haWx0bzpiaGVsZ2Fhc0Bnb29nbGUuY29tXQ0KPiA+PiA+PiBTZW50OiBUaHVyc2Rh
eSwgTm92ZW1iZXIgMDYsIDIwMTQgMTI6MDkgUE0NCj4gPj4gPj4gVG86IExpdSwgQ2h1YW5zaGVu
Zw0KPiA+PiA+PiBDYzogQmFydG87IFRlanVuIEhlbyAodGpAa2VybmVsLm9yZyk7IEx1LCBBYXJv
bjsgUmFmYWVsIFd5c29ja2k7DQo+ID4+ID4+IGxpbnV4LXBjaUB2Z2VyLmtlcm5lbC5vcmc7IGxp
bnV4LWtlcm5lbEB2Z2VyLmtlcm5lbC5vcmcNCj4gPj4gPj4gU3ViamVjdDogUmU6IFtQQVRDSF0g
UENJOiBEbyBub3QgZW5hYmxlIGFzeW5jIHN1c3BlbmQgZm9yIEpNaWNyb24gY2hpcHMNCj4gPj4g
Pj4NCj4gPj4gPj4gT24gV2VkLCBOb3YgNSwgMjAxNCBhdCA2OjQ4IFBNLCBMaXUsIENodWFuc2hl
bmcNCj4gPj4gPj4gPGNodWFuc2hlbmcubGl1QGludGVsLmNvbT4gd3JvdGU6DQo+ID4+ID4+PiBI
ZWxsbyBCam9ybiwNCj4gPj4gPj4+DQo+ID4+ID4+Pj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0t
LS0NCj4gPj4gPj4+PiBGcm9tOiBCam9ybiBIZWxnYWFzIFttYWlsdG86YmhlbGdhYXNAZ29vZ2xl
LmNvbV0NCj4gPj4gPj4+PiBTZW50OiBUaHVyc2RheSwgTm92ZW1iZXIgMDYsIDIwMTQgMzowNCBB
TQ0KPiA+PiA+Pj4+IFRvOiBCYXJ0bw0KPiA+PiA+Pj4+IENjOiBMaXUsIENodWFuc2hlbmc7IEx1
LCBBYXJvbjsgVGVqdW4gSGVvOyBSYWZhZWwgV3lzb2NraTsNCj4gPj4gPj4+PiBsaW51eC1wY2lA
dmdlci5rZXJuZWwub3JnOyBsaW51eC1rZXJuZWxAdmdlci5rZXJuZWwub3JnDQo+ID4+ID4+Pj4g
U3ViamVjdDogUmU6IFtQQVRDSF0gUENJOiBEbyBub3QgZW5hYmxlIGFzeW5jIHN1c3BlbmQgZm9y
IEpNaWNyb24NCj4gY2hpcHMNCj4gPj4gPj4+Pg0KPiA+PiA+Pj4+IE9uIFdlZCwgTm92IDUsIDIw
MTQgYXQgMTE6NDYgQU0sIEJhcnRvDQo+IDxtaXN0ZXIuZnJlZW1hbkBsYXBvc3RlLm5ldD4NCj4g
Pj4gPj4+PiB3cm90ZToNCj4gPj4gPj4+Pj4gdGhpcyBwYXRjaCBzb2x2ZXMgdGhlc2UgMiBidWcg
cmVwb3J0cyA6DQo+ID4+ID4+Pj4+DQo+ID4+ID4+Pj4+IGh0dHBzOi8vYnVnemlsbGEua2VybmVs
Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9ODQ4NjENCj4gPj4gPj4+Pj4NCj4gPj4gPj4+Pj4gaHR0cHM6
Ly9idWd6aWxsYS5rZXJuZWwub3JnL3Nob3dfYnVnLmNnaT9pZD04MTU1MQ0KPiA+PiA+Pj4+DQo+
ID4+ID4+Pj4gVGhvc2UgYnVncyB3ZXJlIGFscmVhZHkgbWVudGlvbmVkLiAgQnV0IGU2YjdlNDFj
ZGQ4YyBjbGFpbXMgdG8NCj4gc29sdmUNCj4gPj4gPj4+PiBodHRwczovL2J1Z3ppbGxhLmtlcm5l
bC5vcmcvc2hvd19idWcuY2dpP2lkPTgxNTUxLCBhbmQgODQ4NjEgaXMgYQ0KPiA+PiA+Pj4+IGR1
cGxpY2F0ZSBvZiA4MTU1MSwgc28gaXQgc2hvdWxkIGFsc28gYmUgZml4ZWQgYnkgZTZiN2U0MWNk
ZDhjLg0KPiA+PiA+Pj4+DQo+ID4+ID4+Pj4gU28gdGhlIHF1ZXN0aW9uIGlzLCB3aHkgd2FzIGU2
YjdlNDFjZGQ4YyBpbnN1ZmZpY2llbnQ/ICBQcmVzdW1hYmx5DQo+IGl0DQo+ID4+ID4+Pj4gd2Fz
IHRlc3RlZCBhbmQgc29tZWJvZHkgdGhvdWdodCBpdCBkaWQgZml4IHRoZSBwcm9ibGVtLg0KPiA+
PiA+Pj4NCj4gPj4gPj4+IFRoZSBmaXJzdCBwYXRjaCBlNmI3ZTQxY2RkOGMgd2hpY2ggaXMganVz
dCBleGNsdWRlIHNvbWUgb2YgSk1pY3Jvbg0KPiA+PiA+PiBjaGlwcygzNjMvMzYxKSBvdXQgb2Yg
YXN5bmNfc3VzcGVuZCwNCj4gPj4gPj4+IHRoZW4gQmFydG8gZm91bmQgdGhlIHNhbWUgaXNzdWUg
b24gSk1pY3JvbiAzNjgsIHNvIHdlIG5lZWQgb25lIG1vcmUNCj4gPj4gPj4gZ2VuZXJhbCBwYXRj
aCB0byBsZXQgSk1pY3JvbiBjaGlwcw0KPiA+PiA+Pj4gb3V0IG9mIGFzeW5jX3N1c3BlbmQsIHNv
IHdlIG1ha2UgdGhpcyBwYXRjaC4NCj4gPj4gPj4+DQo+ID4+ID4+PiBCam9ybiwgdGosDQo+ID4+
ID4+PiBDb3VsZCB5b3Uga2luZGx5IHRha2UgdGhpcyBwYXRjaD8gQXMgQmFydG8gc2FpZCwgaXQg
ZWZmZWN0ZWQgdGhlIHVzZXINCj4gPj4gPj4gZXhwZXJpZW5jZSBpbmRlZWQsIHRoYW5rcy4NCj4g
Pj4gPj4NCj4gPj4gPj4gVGhhbmtzIGZvciBjbGFyaWZ5aW5nIHRoZSBjaGFuZ2Vsb2cgYXMgZmFy
IGFzIHRoZSBkaWZmZXJlbnQgY2hpcHMgYW5kDQo+ID4+ID4+IHRoZSBkaWZmZXJlbnQgYnVnemls
bGFzLg0KPiA+PiA+Pg0KPiA+PiA+PiBCdXQgeW91IGhhdmVuJ3QgYWRkcmVzc2VkIG15IGNvbmNl
cm5zIGFib3V0ICgxKSBwdXR0aW5nIGEgUENJIHZlbmRvcg0KPiA+PiA+PiBJRCBjaGVjayBpbiB0
aGUgZ2VuZXJpYyBQQ0kgY29yZSBjb2RlLCBhbmQgKDIpIGFwcGx5aW5nIHRoaXMgdG8gKmFsbCoN
Cj4gPj4gPj4gSk1pY3JvbiBkZXZpY2VzLiAgWW91IG1pZ2h0IHdhbnQgdG8gZXhwbG9yZSBhIHF1
aXJrLXR5cGUgc29sdXRpb24gb3INCj4gPj4gPj4gbWF5YmUganVzdCBhZGQgdGhlIEpNaWNyb24g
MzY4IHRvIHRoZSBjaGVja3MgYWRkZWQgYnkgZTZiN2U0MWNkZDhjLg0KPiA+PiA+IFVuZGVyc3Rh
bmQgeW91ciBwb2ludCwgaW4gZmFjdCwgYmVmb3JlIHRoaXMgcGF0Y2ggc3VibWl0dGVkLCBJIGhh
ZCB3cml0dGVuDQo+ID4+IGFub3RoZXIgcGF0Y2ggaHR0cHM6Ly9sa21sLm9yZy9sa21sLzIwMTQv
OS8yNC82OA0KPiA+PiA+IHdoaWNoIGFkZHJlc3NlZCB0byBhZGQgdGhlIHF1aXJrLXR5cGUgc29s
dXRpb24gaW4gQVRBIGNvZGUsIGFuZCBBYXJvbg0KPiBnaXZlbg0KPiA+PiBiZXR0ZXIgc3VnZ2Vz
dGlvbiB0aGF0IGltcGxlbWVudGVkIGF0IHBjaV9wbV9pbml0KCkuDQo+ID4+ID4gSG93IGRvIHlv
dSB0aGluayBvZiBpdD8gVGhhbmtzLg0KPiA+Pg0KPiA+PiBJIHRoaW5rIEJqb3JuIG1lYW5zIHRo
YXQgd2Ugc2hvdWxkIHBsYWNlIHRoZSBjb2RlIGFzIGEgZml4dXAgc29tZXdoZXJlDQo+ID4+IGlu
IHRoZSBxdWlya3MuYy4gSSBkaWRuJ3QgdGFrZSBhIGNsb3NlciBsb29rIGJ1dCBERUNMQVJFX1BD
SV9GSVhVUF9GSU5BTA0KPiA+PiBmb3IgdGhvc2UgSk1pY3JvbiBQQ0kgZGV2aWNlcyBzZWVtcyB0
byBiZSBhIHByb3BlciBwaGFzZS4NCj4gPg0KPiA+IFRoYW5rcyBBYXJvbiwgdGhlbiBob3cgYWJv
dXQgYmVsb3cgcGF0Y2g/DQo+ID4NCj4gPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9hdGEvcGF0YV9q
bWljcm9uLmMgYi9kcml2ZXJzL2F0YS9wYXRhX2ptaWNyb24uYw0KPiA+IGluZGV4IDQ3ZTQxOGIu
LjllODVmODYgMTAwNjQ0DQo+ID4gLS0tIGEvZHJpdmVycy9hdGEvcGF0YV9qbWljcm9uLmMNCj4g
PiArKysgYi9kcml2ZXJzL2F0YS9wYXRhX2ptaWNyb24uYw0KPiA+IEBAIC0xNTgsNiArMTU4LDIx
IEBAIHN0YXRpYyBpbnQgam1pY3Jvbl9pbml0X29uZSAoc3RydWN0IHBjaV9kZXYgKnBkZXYsDQo+
IGNvbnN0IHN0cnVjdCBwY2lfZGV2aWNlX2lkICppDQo+ID4gICAgICAgICByZXR1cm4gYXRhX3Bj
aV9ibWRtYV9pbml0X29uZShwZGV2LCBwcGksICZqbWljcm9uX3NodCwgTlVMTCwNCj4gMCk7DQo+
ID4gIH0NCj4gPg0KPiA+ICsvKg0KPiA+ICsgKiBGb3IgSk1pY3JvbiBjaGlwcywgd2UgbmVlZCB0
byBkaXNhYmxlIHRoZSBhc3luY19zdXNwZW5kIG1ldGhvZCwNCj4gb3RoZXJ3aXNlDQo+ID4gKyAq
IHRoZXkgd2lsbCBoaXQgdGhlIHBvd2VyLW9uIGlzc3VlIHdoZW4gZG9pbmcgZGV2aWNlIHJlc3Vt
ZSwgYWRkIG9uZSBxdWljaw0KPiA+ICsgKiBzb2x1dGlvbiB0byBkaXNhYmxlIHRoZSBhc3luY19z
dXNwZW5kIG1ldGhvZC4NCj4gDQo+IEEgInF1aWNrIHNvbHV0aW9uIiBpcyBhIHJlZCBmbGFnIGZv
ciBtZSwgYmVjYXVzZSBpdCdzIGEgaGludCB0aGF0IHlvdQ0KPiBqdXN0IHdhbnQgdGhlIHByb2Js
ZW0gdG8gZ28gYXdheSB3aXRob3V0IGZ1bGx5IHVuZGVyc3RhbmRpbmcgaXQuDQo+IFRoYXQncyBw
cm9iYWJseSBub3QgdGhlIGNhc2U7IGl0J3MgcHJvYmFibHkganVzdCB0aGF0ICpJKiBkb24ndA0K
PiB1bmRlcnN0YW5kIGl0IGFsbCB5ZXQuDQo+IA0KPiA+ICtzdGF0aWMgdm9pZCBwY2lfYXN5bmNf
c3VzcGVuZF9maXh1cChzdHJ1Y3QgcGNpX2RldiAqcGRldikNCj4gPiArew0KPiA+ICsgICAgICAg
LyoNCj4gPiArICAgICAgICAqIGRpc2FibGluZyB0aGUgYXN5bmNfc3VzcGVuZCBtZXRob2QgZm9y
IEpNaWNyb24gY2hpcHMgdG8NCj4gPiArICAgICAgICAqIGF2b2lkIGRldmljZSByZXN1bWluZyBp
c3N1ZS4NCj4gPiArICAgICAgICAqLw0KPiA+ICsgICAgICAgZGV2aWNlX2Rpc2FibGVfYXN5bmNf
c3VzcGVuZCgmcGRldi0+ZGV2KTsNCj4gPiArfQ0KPiA+ICtERUNMQVJFX1BDSV9GSVhVUF9GSU5B
TChQQ0lfVkVORE9SX0lEX0pNSUNST04sIFBDSV9BTllfSUQsDQo+IHBjaV9hc3luY19zdXNwZW5k
X2ZpeHVwKTsNCj4gDQo+IEkga25vdyBCYXJ0byB0ZXN0ZWQgdGhpcyBhbmQgaXQgZGlkbid0IHdv
cmssIGJ1dCB0aGlzICpzdHJhdGVneSogaXMNCj4gZmFyIGJldHRlciB0aGFuIGFkZGluZyB0aGUg
Sk1pY3JvbiB0ZXN0IGluIHBjaV9wbV9pbml0KCkuICBJdCdzIGlkZWFsDQo+IGlmIGEgcXVpcmsg
bGlrZSB0aGlzIGNvdWxkIGJlIGluIHRoZSBwYXRhX2ptaWNyb24uYyBmaWxlLCBzbyBpdCdzIG9u
bHkNCj4gaW5jbHVkZWQgd2hlbiB0aGF0IGRyaXZlciBpcyBsb2FkZWQuICBCdXQgaXQgd291bGQg
c3RpbGwgYmUgT0sgaWYgaXQNCj4gaGFkIHRvIGJlIGluIGRyaXZlcnMvcGNpL3F1aXJrcy5jLCBl
LmcuLCBpZiBzb21ldGhpbmcgaGFzIHRvIGJlIGRvbmUNCj4gZXZlbiBiZWZvcmUgdGhlIGRyaXZl
ciBpcyBsb2FkZWQuDQo+IA0KPiBUaGUgaWRlYSBvZiBhIHF1aXJrIGlzIHRvIHdvcmsgYXJvdW5k
IGEgZGVmZWN0IGluIGEgZGV2aWNlLiAgV2hhdCBpcw0KPiB0aGUgZGVmZWN0IGluIHRoaXMgY2Fz
ZT8gIEl0IHNlZW1zIHRoZXJlIGFyZSB0d28gZGV2aWNlcyBpbnZvbHZlZCwNCj4gZS5nLiAoZnJv
bSBodHRwczovL2J1Z3ppbGxhLmtlcm5lbC5vcmcvc2hvd19idWcuY2dpP2lkPTgxNTUxKToNCj4g
DQo+ICAgMDI6MDAuMCBKTWljcm9uIFRlY2hub2xvZ3kgQ29ycC4gSk1CMzYzIFNBVEEvSURFIENv
bnRyb2xsZXINCj4gICAwMjowMC4xIEpNaWNyb24gVGVjaG5vbG9neSBDb3JwLiBKTUIzNjMgU0FU
QS9JREUgQ29udHJvbGxlcg0KPiANCj4gVGhlIFBDSSBwb3dlciBtYW5hZ2VtZW50IGNvZGUgaXMg
ZGVzaWduZWQgdG8gd29yayBjb3JyZWN0bHkgd2l0aCBhbGwNCj4gZGV2aWNlcyB0aGF0IGNvbmZv
cm0gdG8gdGhlIHNwZWMuICBTbyBlaXRoZXIgb25lIG9mIHRoZXNlIGRldmljZXMNCj4gZG9lc24n
dCBjb25mb3JtIHRvIHRoZSBzcGVjLCBvciB0aGUgUE0gY29kZSBpcyBhc3N1bWluZyBzb21lIGJl
aGF2aW9yDQo+IHRoYXQgdGhlIHNwZWMgZG9lc24ndCBhY3R1YWxseSByZXF1aXJlLiAgSWYgdGhl
IGxhdHRlciwgd2UgbmVlZCB0byBmaXgNCj4gdGhlIFBNIGNvZGUsIGJlY2F1c2UgaXQgd29uJ3Qg
d29yayB3aXRoIG90aGVyIG5vbi1KTWljcm9uIGRldmljZXMNCj4gZWl0aGVyLg0KPiANCj4gSSBo
YXZlbid0IHNlZW4gcmVwb3J0cyBvZiBvdGhlciBkZXZpY2VzLCBzbyBteSBndWVzcyBpcyB0aGF0
IGl0IHJlYWxseQ0KPiAqaXMqIGEgZGVmZWN0IGluIHRoZXNlIEpNaWNyb24gZGV2aWNlcywgYnV0
IEkgc3RpbGwgbmVlZCBhIGJldHRlcg0KPiB1bmRlcnN0YW5kaW5nIG9mIGV4YWN0bHkgd2hhdCdz
IGJyb2tlbi4gIE1heWJlIGl0J3Mgc29tZXRoaW5nIGxpa2U6DQo+IA0KPiAgIC0gMDI6MDAuMCBh
bmQgMDI6MDAuMSBhcmUgYm90aCBpbiBEMw0KPiAgIC0gaWYgd2UgdHJ5IHRvIHB1dCAwMjowMC4x
IGluIEQwIGZpcnN0LCBpdCBmYWlscyAoIlJlZnVzZWQgdG8gY2hhbmdlDQo+IHBvd2VyIHN0YXRl
LCBjdXJyZW50bHkgaW4gRDMiKQ0KWWVzLCBpdCBpcyB0aGUgY2FzZSwgMDI6MDAuMCBhbmQgMDI6
MDAuMSBhcmUgYnJvdGhlciByZWxhdGlvbnNoaXAsIGJ1dCBoYXMgcG93ZXItb24NCnNlcXVlbmNl
IGRlcGVuZGVuY3kuDQoNCj4gDQo+IGFuZCB0aGUgcmVzb2x1dGlvbiBpcyB0byBjaGFuZ2UgMDI6
MDAuMCBmcm9tIEQzIHRvIEQwIGZpcnN0LCB0aGVuDQo+IGNoYW5nZSAwMjowMC4xIGZyb20gRDMg
dG8gRDAuICBPciBtYXliZSBpdCdzIGp1c3QgdGhhdCB0aGVyZSBuZWVkcyB0bw0KPiBiZSBhIGRl
bGF5IGJldHdlZW4gY2hhbmdpbmcgMDI6MDAuMCBhbmQgY2hhbmdpbmcgMDI6MDAuMSAoc2luY2Ug
SQ0KPiBzdXNwZWN0IHdlICpzdGFydCogdGhvc2UgY2hhbmdlcyBpbiB0aGF0IG9yZGVyIGFueXdh
eSk/DQpEZWxheSBoYXMgbm8gaGVscCBmb3IgdGhpcyBpc3N1ZSwgd2UgbXVzdCBmb2xsb3cgdGhl
IG9yZGVyIHRvIHBvd2VyIG9uIG9uZSBieSBvbmUuDQoNCj4gDQo+IEluIGVpdGhlciBjYXNlIGl0
IHNvdW5kcyBsaWtlIGEgZGV2aWNlIGRlZmVjdCwgYmVjYXVzZSBJIGRvbid0IHNlZQ0KPiBhbnl0
aGluZyBpbiB0aGUgUENJIFBvd2VyIE1hbmFnZW1lbnQgc3BlYyBhYm91dCBvcmRlcmluZyByZXF1
aXJlbWVudHMNCj4gZm9yIG11bHRpZnVuY3Rpb24gZGV2aWNlcy4NCj4gDQo+IFNwZWFraW5nIG9m
IG9yZGVyaW5nLCB3aGF0IGlzIGl0IHRoYXQgZ3VhcmFudGVlcyAwMjowMC4wIHdpbGwgYmUNCj4g
cG93ZXJlZCB1cCBiZWZvcmUgMDI6MDAuMSB3aGVuIGFzeW5jIHN1c3BlbmQgaXMgZGlzYWJsZWQ/
ICBJcyBpdCB0aGUNCj4gZHBtX25vaXJxX2xpc3Qgb3JkZXIgaW4gZHBtX3Jlc3VtZV9ub2lycSgp
Pw0KWWVzLCBpdCBkZXBlbmRzIG9uIHRoYXQgbGlzdCB0byBwb3dlciBvbiBpbiBvcmRlci4NCg0K
PiANCj4gSSBkb24ndCBrbm93IGhvdyBtdWNoIHdlIGdhaW4gYnkgYWxsb3dpbmcgYXN5bmMgcmVz
dW1lIG9mDQo+IG11bHRpZnVuY3Rpb24gZGV2aWNlcy4gIElzIGl0IHBvc3NpYmxlIHRoYXQgV2lu
ZG93cyBhbHdheXMgcG93ZXJzIHVwDQo+IG11bHRpZnVuY3Rpb24gZGV2aWNlcyBpbiBvcmRlcj8g
IEkgZG91YnQgdGhhdCBXaW5kb3dzIHdvdWxkIGhhdmUgYQ0KPiBxdWlyayBzaW1pbGFyIHRvIHdo
YXQgeW91J3JlIHByb3Bvc2luZyBoZXJlLCBzbyBJIHdvbmRlciBob3cgdGhleSBkZWFsDQo+IHdp
dGggdGhpcyBpc3N1ZS4NCj4gDQo+IEl0IHNvdW5kcyBsaWtlIHRoaXMgaXNzdWUgb25seSBhZmZl
Y3RzIG11bHRpZnVuY3Rpb24gSk1pY3JvbiBkZXZpY2VzLA0KPiBzbyBwb3NzaWJseSB0aGUgcXVp
cmsgY291bGQgYmUgbWFkZSBzbWFydGVyIGJ5IG9ubHkgZGlzYWJsaW5nIGFzeW5jDQo+IHN1c3Bl
bmQgd2hlbiBpdCBmaW5kcyB0aG9zZS4NClllcywgdGhlIG5ldyBxdWlyay1zb2x1dGlvbiB3aWxs
IGJlIHNlbnQgb3V0IHNvb24uDQoNCj4gDQo+IFBlciBodHRwczovL2J1Z3ppbGxhLmtlcm5lbC5v
cmcvc2hvd19idWcuY2dpP2lkPTgxNTUxI2M4LCB0aGlzIGlzIGENCj4gcmVncmVzc2lvbiBpbnRy
b2R1Y2VkIGJ5IDc2NTY5ZmFhNjJjNCAoIlBNIC8gc2xlZXA6IEFzeW5jaHJvbm91cw0KPiB0aHJl
YWRzIGZvciByZXN1bWVfbm9pcnEiKSwgd2hpY2ggYXBwZWFyZWQgaW4gdjMuMTUuICBUaGF0IGlu
Zm9ybWF0aW9uDQo+IG5lZWRzIHRvIGJlIGluIHRoZSBjaGFuZ2Vsb2cuDQo+IA0KRmFpcmx5IGVu
b3VnaC4gVGhlIG5ldyBxdWlyay1zb2x1dGlvbiBwYXRjaCBoYXMgYmVlbiB0ZXN0ZWQgYnkgQmFy
dG8gc3VjY2Vzc2Z1bGx5LA0Kd2lsbCB1cGRhdGUgdGhlIGNoYW5nZWxvZyB3aXRoIG1vcmUgZGV0
YWlsLg0KSXQgaXMgcmVncmVzc2lvbiwgYnV0IG5vdCBmdWxseSByZWFsIHNpbmNlIHdlIGFjdHVh
bGx5IGZvdW5kIHRoZSBjaGFyYWN0ZXIgZm9yIEpNaWNyb24gZGV2aWNlcy4NCg0KDQo=

[PATCH] PCI: Do not enable async suspend for JMicron chips

[Chuansheng Liu]

The JMicron chip 361/363/368 contains one SATA controller and
one PATA controller, they are brother-relation ship in PCI tree,
but for powering on these both controller, we must follow the
sequence one by one, otherwise one of them can not be powered on
successfully.

So here we disable the async suspend method for Jmicron chip.

Cc: stable@vger.kernel.org # 3.15+
Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
---
 drivers/pci/pci.c |   12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 625a4ac..53128f0 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2046,7 +2046,17 @@ void pci_pm_init(struct pci_dev *dev)
 	pm_runtime_forbid(&dev->dev);
 	pm_runtime_set_active(&dev->dev);
 	pm_runtime_enable(&dev->dev);
-	device_enable_async_suspend(&dev->dev);
+
+	/*
+	 * The JMicron chip 361/363/368 contains one SATA controller and
+	 * one PATA controller, they are brother-relation ship in PCI tree,
+	 * but for powering on these both controller, we must follow the
+	 * sequence one by one, otherwise one of them can not be powered on
+	 * successfully, so here we disable the async suspend method for
+	 * Jmicron chip.
+	 */
+	if (dev->vendor != PCI_VENDOR_ID_JMICRON)
+		device_enable_async_suspend(&dev->dev);
 	dev->wakeup_prepared = false;
 
 	dev->pm_cap = 0;
-- 
1.7.9.5

Re: [PATCH] PCI: Do not enable async suspend for JMicron chips

[Aaron Lu]

On 11/05/2014 09:07 AM, Chuansheng Liu wrote:
> The JMicron chip 361/363/368 contains one SATA controller and
> one PATA controller, they are brother-relation ship in PCI tree,
> but for powering on these both controller, we must follow the
> sequence one by one, otherwise one of them can not be powered on
> successfully.
> 
> So here we disable the async suspend method for Jmicron chip.
>

Place the buglink here?

  Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=84861
  Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=81551
> Cc: stable@vger.kernel.org # 3.15+
> Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>

Patch looks good to me, thanks for fixing this!

-Aaron

> ---
>  drivers/pci/pci.c |   12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 625a4ac..53128f0 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -2046,7 +2046,17 @@ void pci_pm_init(struct pci_dev *dev)
>  	pm_runtime_forbid(&dev->dev);
>  	pm_runtime_set_active(&dev->dev);
>  	pm_runtime_enable(&dev->dev);
> -	device_enable_async_suspend(&dev->dev);
> +
> +	/*
> +	 * The JMicron chip 361/363/368 contains one SATA controller and
> +	 * one PATA controller, they are brother-relation ship in PCI tree,
> +	 * but for powering on these both controller, we must follow the
> +	 * sequence one by one, otherwise one of them can not be powered on
> +	 * successfully, so here we disable the async suspend method for
> +	 * Jmicron chip.
> +	 */
> +	if (dev->vendor != PCI_VENDOR_ID_JMICRON)
> +		device_enable_async_suspend(&dev->dev);
>  	dev->wakeup_prepared = false;
>  
>  	dev->pm_cap = 0;
> 

Re: [PATCH] PCI: Do not enable async suspend for JMicron chips

[Aaron Lu]

On 11/05/2014 09:33 AM, Aaron Lu wrote:
> On 11/05/2014 09:07 AM, Chuansheng Liu wrote:
>> The JMicron chip 361/363/368 contains one SATA controller and
>> one PATA controller, they are brother-relation ship in PCI tree,
>> but for powering on these both controller, we must follow the
>> sequence one by one, otherwise one of them can not be powered on
>> successfully.
>>
>> So here we disable the async suspend method for Jmicron chip.
>>
> 
> Place the buglink here?
> 
>   Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=84861
>   Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=81551
>> Cc: stable@vger.kernel.org # 3.15+
>> Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
> 
> Patch looks good to me, thanks for fixing this!

BTW, the previous change to the ATA code can be deleted now.
So maybe also put them into this patch.

Regards,
Aaron
 
>> ---
>>  drivers/pci/pci.c |   12 +++++++++++-
>>  1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index 625a4ac..53128f0 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -2046,7 +2046,17 @@ void pci_pm_init(struct pci_dev *dev)
>>  	pm_runtime_forbid(&dev->dev);
>>  	pm_runtime_set_active(&dev->dev);
>>  	pm_runtime_enable(&dev->dev);
>> -	device_enable_async_suspend(&dev->dev);
>> +
>> +	/*
>> +	 * The JMicron chip 361/363/368 contains one SATA controller and
>> +	 * one PATA controller, they are brother-relation ship in PCI tree,
>> +	 * but for powering on these both controller, we must follow the
>> +	 * sequence one by one, otherwise one of them can not be powered on
>> +	 * successfully, so here we disable the async suspend method for
>> +	 * Jmicron chip.
>> +	 */
>> +	if (dev->vendor != PCI_VENDOR_ID_JMICRON)
>> +		device_enable_async_suspend(&dev->dev);
>>  	dev->wakeup_prepared = false;
>>  
>>  	dev->pm_cap = 0;
>>
> 

Re: [PATCH] PCI: Do not enable async suspend for JMicron chips

[Tejun Heo]

On Wed, Nov 05, 2014 at 09:07:45AM +0800, Chuansheng Liu wrote:
> The JMicron chip 361/363/368 contains one SATA controller and
> one PATA controller, they are brother-relation ship in PCI tree,
> but for powering on these both controller, we must follow the
> sequence one by one, otherwise one of them can not be powered on
> successfully.
> 
> So here we disable the async suspend method for Jmicron chip.
> 
> Cc: stable@vger.kernel.org # 3.15+
> Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>

Applied to libata/for-3.18-fixes.

Thanks.

-- 
tejun

Re: [PATCH] PCI: Do not enable async suspend for JMicron chips

[Barto]

I tried the patch, it solves the problem,

but I had to change the patch in order to be compatible with 3.18rc3
source code :

patching file drivers/pci/pci.c
Hunk #1 FAILED at 2046.
1 out of 1 hunk FAILED -- saving rejects to file drivers/pci/pci.c.rej

here is the correct patch for kernel 3.18rc3 source code :

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2046,7 +2046,17 @@ void pci_pm_init(struct pci_dev *dev)
 	pm_runtime_forbid(&dev->dev);
 	pm_runtime_set_active(&dev->dev);
 	pm_runtime_enable(&dev->dev);
-	device_enable_async_suspend(&dev->dev);
+	
+     /*
+      * The JMicron chip 361/363/368 contains one SATA controller and
+      * one PATA controller, they are brother-relation ship in PCI tree,
+      * but for powering on these both controller, we must follow the
+      * sequence one by one, otherwise one of them can not be powered on
+      * successfully, so here we disable the async suspend method for
+      * Jmicron chip.
+      */
+     if (dev->vendor != PCI_VENDOR_ID_JMICRON)
+         device_enable_async_suspend(&dev->dev);
 	dev->wakeup_prepared = false;

 	dev->pm_cap = 0;


Le 05/11/2014 17:31, Tejun Heo a écrit :
> On Wed, Nov 05, 2014 at 09:07:45AM +0800, Chuansheng Liu wrote:
>> The JMicron chip 361/363/368 contains one SATA controller and
>> one PATA controller, they are brother-relation ship in PCI tree,
>> but for powering on these both controller, we must follow the
>> sequence one by one, otherwise one of them can not be powered on
>> successfully.
>>
>> So here we disable the async suspend method for Jmicron chip.
>>
>> Cc: stable@vger.kernel.org # 3.15+
>> Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
> 
> Applied to libata/for-3.18-fixes.
> 
> Thanks.
> 

Re: [PATCH] PCI: Do not enable async suspend for JMicron chips

[Tejun Heo]

On Wed, Nov 05, 2014 at 11:31:59AM -0500, Tejun Heo wrote:
> On Wed, Nov 05, 2014 at 09:07:45AM +0800, Chuansheng Liu wrote:
> > The JMicron chip 361/363/368 contains one SATA controller and
> > one PATA controller, they are brother-relation ship in PCI tree,
> > but for powering on these both controller, we must follow the
> > sequence one by one, otherwise one of them can not be powered on
> > successfully.
> > 
> > So here we disable the async suspend method for Jmicron chip.
> > 
> > Cc: stable@vger.kernel.org # 3.15+
> > Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
> 
> Applied to libata/for-3.18-fixes.

Ah, this is a PCI patch.  Popping from libata tree.

Thanks.

-- 
tejun