diff mbox

dma: dw: Add suspend and resume handling for PCI mode DW_DMAC.

Message ID 1386684373-24753-1-git-send-email-chiau.ee.chew@intel.com (mailing list archive)
State Rejected
Headers show

Commit Message

Chew Chiau Ee Dec. 10, 2013, 2:06 p.m. UTC
From: Chew, Chiau Ee <chiau.ee.chew@intel.com>

This is to disable/enable DW_DMAC hw during suspend/resume.

Signed-off-by: Chew, Chiau Ee <chiau.ee.chew@intel.com>
Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/dma/dw/pci.c |   33 +++++++++++++++++++++++++++++++++
 1 files changed, 33 insertions(+), 0 deletions(-)

Comments

Vinod Koul Dec. 10, 2013, 10:10 a.m. UTC | #1
On Tue, Dec 10, 2013 at 10:06:13PM +0800, Chew Chiau Ee wrote:
> From: Chew, Chiau Ee <chiau.ee.chew@intel.com>
> 
> This is to disable/enable DW_DMAC hw during suspend/resume.
> 
> Signed-off-by: Chew, Chiau Ee <chiau.ee.chew@intel.com>
> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/dma/dw/pci.c |   33 +++++++++++++++++++++++++++++++++
>  1 files changed, 33 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/dma/dw/pci.c b/drivers/dma/dw/pci.c
> index e89fc24..97bc3a2 100644
> --- a/drivers/dma/dw/pci.c
> +++ b/drivers/dma/dw/pci.c
> @@ -75,6 +75,36 @@ static void dw_pci_remove(struct pci_dev *pdev)
>  		dev_warn(&pdev->dev, "can't remove device properly: %d\n", ret);
>  }
>  
> +#ifdef CONFIG_PM_SLEEP
> +
> +static int dw_pci_suspend_noirq(struct device *dev)
> +{
> +	struct pci_dev *pci = to_pci_dev(dev);
> +	struct dw_dma_chip *chip = pci_get_drvdata(pci);
> +
> +	return dw_dma_suspend(chip);
> +};
> +
> +static int dw_pci_resume_noirq(struct device *dev)
> +{
> +	struct pci_dev *pci = to_pci_dev(dev);
> +	struct dw_dma_chip *chip = pci_get_drvdata(pci);
> +
> +	return dw_dma_resume(chip);
> +};
> +
> +#else /* !CONFIG_PM_SLEEP */
> +
> +#define dw_pci_suspend_noirq	NULL
> +#define dw_pci_resume_noirq	NULL
> +
> +#endif /* !CONFIG_PM_SLEEP */
How about SET_SYSTEM_SLEEP_PM_OPS instead?

--
~Vinod
> +
> +static const struct dev_pm_ops dw_pci_dev_pm_ops = {
> +	.suspend_noirq = dw_pci_suspend_noirq,
> +	.resume_noirq = dw_pci_resume_noirq,
> +};
> +
>  static DEFINE_PCI_DEVICE_TABLE(dw_pci_id_table) = {
>  	/* Medfield */
>  	{ PCI_VDEVICE(INTEL, 0x0827), (kernel_ulong_t)&dw_pci_pdata },
> @@ -92,6 +122,9 @@ static struct pci_driver dw_pci_driver = {
>  	.id_table	= dw_pci_id_table,
>  	.probe		= dw_pci_probe,
>  	.remove		= dw_pci_remove,
> +	.driver	= {
> +		.pm	= &dw_pci_dev_pm_ops,
> +	},
>  };
>  
>  module_pci_driver(dw_pci_driver);
> -- 
> 1.7.4.4
>
Andy Shevchenko Dec. 10, 2013, 11:56 a.m. UTC | #2
T24gVHVlLCAyMDEzLTEyLTEwIGF0IDE1OjQwICswNTMwLCBWaW5vZCBLb3VsIHdyb3RlOg0KPiBP
biBUdWUsIERlYyAxMCwgMjAxMyBhdCAxMDowNjoxM1BNICswODAwLCBDaGV3IENoaWF1IEVlIHdy
b3RlOg0KPiA+IEZyb206IENoZXcsIENoaWF1IEVlIDxjaGlhdS5lZS5jaGV3QGludGVsLmNvbT4N
Cj4gPiANCj4gPiBUaGlzIGlzIHRvIGRpc2FibGUvZW5hYmxlIERXX0RNQUMgaHcgZHVyaW5nIHN1
c3BlbmQvcmVzdW1lLg0KPiA+IA0KPiA+IFNpZ25lZC1vZmYtYnk6IENoZXcsIENoaWF1IEVlIDxj
aGlhdS5lZS5jaGV3QGludGVsLmNvbT4NCj4gPiBBY2tlZC1ieTogQW5keSBTaGV2Y2hlbmtvIDxh
bmRyaXkuc2hldmNoZW5rb0BsaW51eC5pbnRlbC5jb20+DQo+ID4gLS0tDQo+ID4gIGRyaXZlcnMv
ZG1hL2R3L3BjaS5jIHwgICAzMyArKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysNCj4g
PiAgMSBmaWxlcyBjaGFuZ2VkLCAzMyBpbnNlcnRpb25zKCspLCAwIGRlbGV0aW9ucygtKQ0KPiA+
IA0KPiA+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2RtYS9kdy9wY2kuYyBiL2RyaXZlcnMvZG1hL2R3
L3BjaS5jDQo+ID4gaW5kZXggZTg5ZmMyNC4uOTdiYzNhMiAxMDA2NDQNCj4gPiAtLS0gYS9kcml2
ZXJzL2RtYS9kdy9wY2kuYw0KPiA+ICsrKyBiL2RyaXZlcnMvZG1hL2R3L3BjaS5jDQo+ID4gQEAg
LTc1LDYgKzc1LDM2IEBAIHN0YXRpYyB2b2lkIGR3X3BjaV9yZW1vdmUoc3RydWN0IHBjaV9kZXYg
KnBkZXYpDQo+ID4gIAkJZGV2X3dhcm4oJnBkZXYtPmRldiwgImNhbid0IHJlbW92ZSBkZXZpY2Ug
cHJvcGVybHk6ICVkXG4iLCByZXQpOw0KPiA+ICB9DQo+ID4gIA0KPiA+ICsjaWZkZWYgQ09ORklH
X1BNX1NMRUVQDQo+ID4gKw0KPiA+ICtzdGF0aWMgaW50IGR3X3BjaV9zdXNwZW5kX25vaXJxKHN0
cnVjdCBkZXZpY2UgKmRldikNCj4gPiArew0KPiA+ICsJc3RydWN0IHBjaV9kZXYgKnBjaSA9IHRv
X3BjaV9kZXYoZGV2KTsNCj4gPiArCXN0cnVjdCBkd19kbWFfY2hpcCAqY2hpcCA9IHBjaV9nZXRf
ZHJ2ZGF0YShwY2kpOw0KPiA+ICsNCj4gPiArCXJldHVybiBkd19kbWFfc3VzcGVuZChjaGlwKTsN
Cj4gPiArfTsNCj4gPiArDQo+ID4gK3N0YXRpYyBpbnQgZHdfcGNpX3Jlc3VtZV9ub2lycShzdHJ1
Y3QgZGV2aWNlICpkZXYpDQo+ID4gK3sNCj4gPiArCXN0cnVjdCBwY2lfZGV2ICpwY2kgPSB0b19w
Y2lfZGV2KGRldik7DQo+ID4gKwlzdHJ1Y3QgZHdfZG1hX2NoaXAgKmNoaXAgPSBwY2lfZ2V0X2Ry
dmRhdGEocGNpKTsNCj4gPiArDQo+ID4gKwlyZXR1cm4gZHdfZG1hX3Jlc3VtZShjaGlwKTsNCj4g
PiArfTsNCj4gPiArDQo+ID4gKyNlbHNlIC8qICFDT05GSUdfUE1fU0xFRVAgKi8NCj4gPiArDQo+
ID4gKyNkZWZpbmUgZHdfcGNpX3N1c3BlbmRfbm9pcnEJTlVMTA0KPiA+ICsjZGVmaW5lIGR3X3Bj
aV9yZXN1bWVfbm9pcnEJTlVMTA0KPiA+ICsNCj4gPiArI2VuZGlmIC8qICFDT05GSUdfUE1fU0xF
RVAgKi8NCj4gSG93IGFib3V0IFNFVF9TWVNURU1fU0xFRVBfUE1fT1BTIGluc3RlYWQ/DQoNClNv
LCB3ZSBhcmUgdXNpbmcgKl9ub2lycSB2ZXJzaW9ucyBvZiB0aGUgZnVuY3Rpb25zIGhlcmUuIFdo
YXQgaGFwcGVuZWQNCndoZW4gd2Ugc3dpdGNoIHRvIG5vcm1hbCBvbmVzPyBBbnkgc2lkZSBlZmZl
Y3RzPw0KDQo+IA0KPiAtLQ0KPiB+Vmlub2QNCj4gPiArDQo+ID4gK3N0YXRpYyBjb25zdCBzdHJ1
Y3QgZGV2X3BtX29wcyBkd19wY2lfZGV2X3BtX29wcyA9IHsNCj4gPiArCS5zdXNwZW5kX25vaXJx
ID0gZHdfcGNpX3N1c3BlbmRfbm9pcnEsDQo+ID4gKwkucmVzdW1lX25vaXJxID0gZHdfcGNpX3Jl
c3VtZV9ub2lycSwNCj4gPiArfTsNCj4gPiArDQo+ID4gIHN0YXRpYyBERUZJTkVfUENJX0RFVklD
RV9UQUJMRShkd19wY2lfaWRfdGFibGUpID0gew0KPiA+ICAJLyogTWVkZmllbGQgKi8NCj4gPiAg
CXsgUENJX1ZERVZJQ0UoSU5URUwsIDB4MDgyNyksIChrZXJuZWxfdWxvbmdfdCkmZHdfcGNpX3Bk
YXRhIH0sDQo+ID4gQEAgLTkyLDYgKzEyMiw5IEBAIHN0YXRpYyBzdHJ1Y3QgcGNpX2RyaXZlciBk
d19wY2lfZHJpdmVyID0gew0KPiA+ICAJLmlkX3RhYmxlCT0gZHdfcGNpX2lkX3RhYmxlLA0KPiA+
ICAJLnByb2JlCQk9IGR3X3BjaV9wcm9iZSwNCj4gPiAgCS5yZW1vdmUJCT0gZHdfcGNpX3JlbW92
ZSwNCj4gPiArCS5kcml2ZXIJPSB7DQo+ID4gKwkJLnBtCT0gJmR3X3BjaV9kZXZfcG1fb3BzLA0K
PiA+ICsJfSwNCj4gPiAgfTsNCj4gPiAgDQo+ID4gIG1vZHVsZV9wY2lfZHJpdmVyKGR3X3BjaV9k
cml2ZXIpOw0KPiA+IC0tIA0KPiA+IDEuNy40LjQNCj4gPiANCj4gDQoNCi0tIA0KQW5keSBTaGV2
Y2hlbmtvIDxhbmRyaXkuc2hldmNoZW5rb0BpbnRlbC5jb20+DQpJbnRlbCBGaW5sYW5kIE95DQot
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0KSW50ZWwgRmlubGFuZCBPeQpSZWdpc3RlcmVkIEFkZHJlc3M6IFBMIDI4MSwg
MDAxODEgSGVsc2lua2kgCkJ1c2luZXNzIElkZW50aXR5IENvZGU6IDAzNTc2MDYgLSA0IApEb21p
Y2lsZWQgaW4gSGVsc2lua2kgCgpUaGlzIGUtbWFpbCBhbmQgYW55IGF0dGFjaG1lbnRzIG1heSBj
b250YWluIGNvbmZpZGVudGlhbCBtYXRlcmlhbCBmb3IKdGhlIHNvbGUgdXNlIG9mIHRoZSBpbnRl
bmRlZCByZWNpcGllbnQocykuIEFueSByZXZpZXcgb3IgZGlzdHJpYnV0aW9uCmJ5IG90aGVycyBp
cyBzdHJpY3RseSBwcm9oaWJpdGVkLiBJZiB5b3UgYXJlIG5vdCB0aGUgaW50ZW5kZWQKcmVjaXBp
ZW50LCBwbGVhc2UgY29udGFjdCB0aGUgc2VuZGVyIGFuZCBkZWxldGUgYWxsIGNvcGllcy4K

--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chew Chiau Ee Dec. 16, 2013, 8:21 a.m. UTC | #3
Vmlub2QsDQoNCkFzIG1lbnRpb25lZCBieSBBbmR5LCB3ZSBhcmUgdXNpbmcgKl9ub2lycSAgdmVy
aW9uIG9mIHN1c3BlbmQvcmVzdW1lIFBNIGNhbGxiYWNrIHdoZXJlYnkgdGhlIGNhbGxiYWNrcyB3
b3VsZCBiZSBleGVjdXRlZCBhZnRlciBJUlEgaGFuZGxlcnMgaGF2ZSBiZWVuIGRpc2FibGVkLiBJ
ZiB1c2luZyBTRVRfU1lTVEVNX1NMRUVQX1BNX09QUywgaXQgd291bGQgYmUgdGhlIG5vcm1hbCBz
dXNwZW5kL3Jlc3VtZSBQTSBjYWxsYmFjay4gTG9va2luZyBhdCB0aGUgRGVzZ2lud2FyZSBETUFD
IHBsYXRmb3JtIGNvZGUgKGRyaXZlcnMvZG1hL2R3L3BsYXRmb3JtLmMpLCBpdCBpcyB1c2luZyB0
aGUgKl9ub2lycSBzdXNwZW5kL3Jlc3VtZSBQTSBjYWxsYmFjay4gSXMgaXQgYWR2aXNhYmxlIHRv
IHVzZSB0aGUgbm9ybWFsIHN1c3BlbmQvcmVzdW1lIFBNIGNhbGxiYWNrIGluc3RlYWQgb2YgKl9u
b2lycSBzdXNwZW5kL1BNIGNhbGxiYWNrPyANCg0KVGhhbmtzLA0KQ2hpYXUgRWUNCg0KLS0tLS1P
cmlnaW5hbCBNZXNzYWdlLS0tLS0NCkZyb206IFNoZXZjaGVua28sIEFuZHJpeSANClNlbnQ6IFR1
ZXNkYXksIERlY2VtYmVyIDEwLCAyMDEzIDc6NTcgUE0NClRvOiBLb3VsLCBWaW5vZA0KQ2M6IENo
ZXcsIENoaWF1IEVlOyBWaXJlc2ggS3VtYXI7IEFuZHkgU2hldmNoZW5rbzsgV2lsbGlhbXMsIERh
biBKOyBkbWFlbmdpbmVAdmdlci5rZXJuZWwub3JnOyBsaW51eC1rZXJuZWxAdmdlci5rZXJuZWwu
b3JnDQpTdWJqZWN0OiBSZTogW1BBVENIXSBkbWE6IGR3OiBBZGQgc3VzcGVuZCBhbmQgcmVzdW1l
IGhhbmRsaW5nIGZvciBQQ0kgbW9kZSBEV19ETUFDLg0KDQpPbiBUdWUsIDIwMTMtMTItMTAgYXQg
MTU6NDAgKzA1MzAsIFZpbm9kIEtvdWwgd3JvdGU6DQo+IE9uIFR1ZSwgRGVjIDEwLCAyMDEzIGF0
IDEwOjA2OjEzUE0gKzA4MDAsIENoZXcgQ2hpYXUgRWUgd3JvdGU6DQo+ID4gRnJvbTogQ2hldywg
Q2hpYXUgRWUgPGNoaWF1LmVlLmNoZXdAaW50ZWwuY29tPg0KPiA+IA0KPiA+IFRoaXMgaXMgdG8g
ZGlzYWJsZS9lbmFibGUgRFdfRE1BQyBodyBkdXJpbmcgc3VzcGVuZC9yZXN1bWUuDQo+ID4gDQo+
ID4gU2lnbmVkLW9mZi1ieTogQ2hldywgQ2hpYXUgRWUgPGNoaWF1LmVlLmNoZXdAaW50ZWwuY29t
Pg0KPiA+IEFja2VkLWJ5OiBBbmR5IFNoZXZjaGVua28gPGFuZHJpeS5zaGV2Y2hlbmtvQGxpbnV4
LmludGVsLmNvbT4NCj4gPiAtLS0NCj4gPiAgZHJpdmVycy9kbWEvZHcvcGNpLmMgfCAgIDMzICsr
KysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKw0KPiA+ICAxIGZpbGVzIGNoYW5nZWQsIDMz
IGluc2VydGlvbnMoKyksIDAgZGVsZXRpb25zKC0pDQo+ID4gDQo+ID4gZGlmZiAtLWdpdCBhL2Ry
aXZlcnMvZG1hL2R3L3BjaS5jIGIvZHJpdmVycy9kbWEvZHcvcGNpLmMgaW5kZXggDQo+ID4gZTg5
ZmMyNC4uOTdiYzNhMiAxMDA2NDQNCj4gPiAtLS0gYS9kcml2ZXJzL2RtYS9kdy9wY2kuYw0KPiA+
ICsrKyBiL2RyaXZlcnMvZG1hL2R3L3BjaS5jDQo+ID4gQEAgLTc1LDYgKzc1LDM2IEBAIHN0YXRp
YyB2b2lkIGR3X3BjaV9yZW1vdmUoc3RydWN0IHBjaV9kZXYgKnBkZXYpDQo+ID4gIAkJZGV2X3dh
cm4oJnBkZXYtPmRldiwgImNhbid0IHJlbW92ZSBkZXZpY2UgcHJvcGVybHk6ICVkXG4iLCByZXQp
OyAgDQo+ID4gfQ0KPiA+ICANCj4gPiArI2lmZGVmIENPTkZJR19QTV9TTEVFUA0KPiA+ICsNCj4g
PiArc3RhdGljIGludCBkd19wY2lfc3VzcGVuZF9ub2lycShzdHJ1Y3QgZGV2aWNlICpkZXYpIHsN
Cj4gPiArCXN0cnVjdCBwY2lfZGV2ICpwY2kgPSB0b19wY2lfZGV2KGRldik7DQo+ID4gKwlzdHJ1
Y3QgZHdfZG1hX2NoaXAgKmNoaXAgPSBwY2lfZ2V0X2RydmRhdGEocGNpKTsNCj4gPiArDQo+ID4g
KwlyZXR1cm4gZHdfZG1hX3N1c3BlbmQoY2hpcCk7DQo+ID4gK307DQo+ID4gKw0KPiA+ICtzdGF0
aWMgaW50IGR3X3BjaV9yZXN1bWVfbm9pcnEoc3RydWN0IGRldmljZSAqZGV2KSB7DQo+ID4gKwlz
dHJ1Y3QgcGNpX2RldiAqcGNpID0gdG9fcGNpX2RldihkZXYpOw0KPiA+ICsJc3RydWN0IGR3X2Rt
YV9jaGlwICpjaGlwID0gcGNpX2dldF9kcnZkYXRhKHBjaSk7DQo+ID4gKw0KPiA+ICsJcmV0dXJu
IGR3X2RtYV9yZXN1bWUoY2hpcCk7DQo+ID4gK307DQo+ID4gKw0KPiA+ICsjZWxzZSAvKiAhQ09O
RklHX1BNX1NMRUVQICovDQo+ID4gKw0KPiA+ICsjZGVmaW5lIGR3X3BjaV9zdXNwZW5kX25vaXJx
CU5VTEwNCj4gPiArI2RlZmluZSBkd19wY2lfcmVzdW1lX25vaXJxCU5VTEwNCj4gPiArDQo+ID4g
KyNlbmRpZiAvKiAhQ09ORklHX1BNX1NMRUVQICovDQo+IEhvdyBhYm91dCBTRVRfU1lTVEVNX1NM
RUVQX1BNX09QUyBpbnN0ZWFkPw0KDQpTbywgd2UgYXJlIHVzaW5nICpfbm9pcnEgdmVyc2lvbnMg
b2YgdGhlIGZ1bmN0aW9ucyBoZXJlLiBXaGF0IGhhcHBlbmVkIHdoZW4gd2Ugc3dpdGNoIHRvIG5v
cm1hbCBvbmVzPyBBbnkgc2lkZSBlZmZlY3RzPw0KDQo+IA0KPiAtLQ0KPiB+Vmlub2QNCj4gPiAr
DQo+ID4gK3N0YXRpYyBjb25zdCBzdHJ1Y3QgZGV2X3BtX29wcyBkd19wY2lfZGV2X3BtX29wcyA9
IHsNCj4gPiArCS5zdXNwZW5kX25vaXJxID0gZHdfcGNpX3N1c3BlbmRfbm9pcnEsDQo+ID4gKwku
cmVzdW1lX25vaXJxID0gZHdfcGNpX3Jlc3VtZV9ub2lycSwgfTsNCj4gPiArDQo+ID4gIHN0YXRp
YyBERUZJTkVfUENJX0RFVklDRV9UQUJMRShkd19wY2lfaWRfdGFibGUpID0gew0KPiA+ICAJLyog
TWVkZmllbGQgKi8NCj4gPiAgCXsgUENJX1ZERVZJQ0UoSU5URUwsIDB4MDgyNyksIChrZXJuZWxf
dWxvbmdfdCkmZHdfcGNpX3BkYXRhIH0sIEBAIA0KPiA+IC05Miw2ICsxMjIsOSBAQCBzdGF0aWMg
c3RydWN0IHBjaV9kcml2ZXIgZHdfcGNpX2RyaXZlciA9IHsNCj4gPiAgCS5pZF90YWJsZQk9IGR3
X3BjaV9pZF90YWJsZSwNCj4gPiAgCS5wcm9iZQkJPSBkd19wY2lfcHJvYmUsDQo+ID4gIAkucmVt
b3ZlCQk9IGR3X3BjaV9yZW1vdmUsDQo+ID4gKwkuZHJpdmVyCT0gew0KPiA+ICsJCS5wbQk9ICZk
d19wY2lfZGV2X3BtX29wcywNCj4gPiArCX0sDQo+ID4gIH07DQo+ID4gIA0KPiA+ICBtb2R1bGVf
cGNpX2RyaXZlcihkd19wY2lfZHJpdmVyKTsNCj4gPiAtLQ0KPiA+IDEuNy40LjQNCj4gPiANCj4g
DQoNCi0tDQpBbmR5IFNoZXZjaGVua28gPGFuZHJpeS5zaGV2Y2hlbmtvQGludGVsLmNvbT4gSW50
ZWwgRmlubGFuZCBPeQ0K
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vinod Koul Dec. 18, 2013, 3:49 p.m. UTC | #4
On Mon, Dec 16, 2013 at 01:51:47PM +0530, Chew, Chiau Ee wrote:
> Vinod,
Please do *NOT* top post

Please fix your MUA to wrap lines with 80chars. I have fix below to make it
readble...

> As mentioned by Andy, we are using *_noirq  verion of suspend/resume PM
> callback whereby the callbacks would be executed after IRQ handlers have been
> disabled. If using SET_SYSTEM_SLEEP_PM_OPS, it would be the normal
> suspend/resume PM callback. Looking at the Desginware DMAC platform code
> (drivers/dma/dw/platform.c), it is using the *_noirq suspend/resume PM
> callback. Is it advisable to use the normal suspend/resume PM callback instead
> of *_noirq suspend/PM callback? 

i dont see a reason why we need the noirq versions

--
~Vinod

> 
> Thanks,
> Chiau Ee
> 
> -----Original Message-----
> From: Shevchenko, Andriy 
> Sent: Tuesday, December 10, 2013 7:57 PM
> To: Koul, Vinod
> Cc: Chew, Chiau Ee; Viresh Kumar; Andy Shevchenko; Williams, Dan J; dmaengine@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] dma: dw: Add suspend and resume handling for PCI mode DW_DMAC.
> 
> On Tue, 2013-12-10 at 15:40 +0530, Vinod Koul wrote:
> > On Tue, Dec 10, 2013 at 10:06:13PM +0800, Chew Chiau Ee wrote:
> > > From: Chew, Chiau Ee <chiau.ee.chew@intel.com>
> > > 
> > > This is to disable/enable DW_DMAC hw during suspend/resume.
> > > 
> > > Signed-off-by: Chew, Chiau Ee <chiau.ee.chew@intel.com>
> > > Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > ---
> > >  drivers/dma/dw/pci.c |   33 +++++++++++++++++++++++++++++++++
> > >  1 files changed, 33 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/drivers/dma/dw/pci.c b/drivers/dma/dw/pci.c index 
> > > e89fc24..97bc3a2 100644
> > > --- a/drivers/dma/dw/pci.c
> > > +++ b/drivers/dma/dw/pci.c
> > > @@ -75,6 +75,36 @@ static void dw_pci_remove(struct pci_dev *pdev)
> > >  		dev_warn(&pdev->dev, "can't remove device properly: %d\n", ret);  
> > > }
> > >  
> > > +#ifdef CONFIG_PM_SLEEP
> > > +
> > > +static int dw_pci_suspend_noirq(struct device *dev) {
> > > +	struct pci_dev *pci = to_pci_dev(dev);
> > > +	struct dw_dma_chip *chip = pci_get_drvdata(pci);
> > > +
> > > +	return dw_dma_suspend(chip);
> > > +};
> > > +
> > > +static int dw_pci_resume_noirq(struct device *dev) {
> > > +	struct pci_dev *pci = to_pci_dev(dev);
> > > +	struct dw_dma_chip *chip = pci_get_drvdata(pci);
> > > +
> > > +	return dw_dma_resume(chip);
> > > +};
> > > +
> > > +#else /* !CONFIG_PM_SLEEP */
> > > +
> > > +#define dw_pci_suspend_noirq	NULL
> > > +#define dw_pci_resume_noirq	NULL
> > > +
> > > +#endif /* !CONFIG_PM_SLEEP */
> > How about SET_SYSTEM_SLEEP_PM_OPS instead?
> 
> So, we are using *_noirq versions of the functions here. What happened when we switch to normal ones? Any side effects?
> 
> > 
> > --
> > ~Vinod
> > > +
> > > +static const struct dev_pm_ops dw_pci_dev_pm_ops = {
> > > +	.suspend_noirq = dw_pci_suspend_noirq,
> > > +	.resume_noirq = dw_pci_resume_noirq, };
> > > +
> > >  static DEFINE_PCI_DEVICE_TABLE(dw_pci_id_table) = {
> > >  	/* Medfield */
> > >  	{ PCI_VDEVICE(INTEL, 0x0827), (kernel_ulong_t)&dw_pci_pdata }, @@ 
> > > -92,6 +122,9 @@ static struct pci_driver dw_pci_driver = {
> > >  	.id_table	= dw_pci_id_table,
> > >  	.probe		= dw_pci_probe,
> > >  	.remove		= dw_pci_remove,
> > > +	.driver	= {
> > > +		.pm	= &dw_pci_dev_pm_ops,
> > > +	},
> > >  };
> > >  
> > >  module_pci_driver(dw_pci_driver);
> > > --
> > > 1.7.4.4
> > > 
> > 
> 
> --
> Andy Shevchenko <andriy.shevchenko@intel.com> Intel Finland Oy
Andy Shevchenko Dec. 19, 2013, 10:51 a.m. UTC | #5
On Wed, 2013-12-18 at 21:19 +0530, Vinod Koul wrote:
> On Mon, Dec 16, 2013 at 01:51:47PM +0530, Chew, Chiau Ee wrote:

> > As mentioned by Andy, we are using *_noirq  verion of suspend/resume PM
> > callback whereby the callbacks would be executed after IRQ handlers have been
> > disabled. If using SET_SYSTEM_SLEEP_PM_OPS, it would be the normal
> > suspend/resume PM callback. Looking at the Desginware DMAC platform code
> > (drivers/dma/dw/platform.c), it is using the *_noirq suspend/resume PM
> > callback. Is it advisable to use the normal suspend/resume PM callback instead
> > of *_noirq suspend/PM callback? 
> 
> i dont see a reason why we need the noirq versions

Okay. I imagine the following use case.

For example we have compiled in DMA driver (dw_dmac) along with, for
example, SPI driver.

System was scheduled to go sleep.

An order of calling IIUC might be DMA first, then SPI (since they are
not in parent / child relations).

What was happened when SPI would like to do a DMA transfer and DMA is
going to sleep? I'm trying to understand if this is a case.

> > > How about SET_SYSTEM_SLEEP_PM_OPS instead?
> > 
> > So, we are using *_noirq versions of the functions here. What happened when we switch to normal ones? Any side effects?
Vinod Koul Jan. 20, 2014, 9:25 a.m. UTC | #6
On Thu, Dec 19, 2013 at 12:51:29PM +0200, Andy Shevchenko wrote:
> On Wed, 2013-12-18 at 21:19 +0530, Vinod Koul wrote:
> > On Mon, Dec 16, 2013 at 01:51:47PM +0530, Chew, Chiau Ee wrote:
> 
> > > As mentioned by Andy, we are using *_noirq  verion of suspend/resume PM
> > > callback whereby the callbacks would be executed after IRQ handlers have been
> > > disabled. If using SET_SYSTEM_SLEEP_PM_OPS, it would be the normal
> > > suspend/resume PM callback. Looking at the Desginware DMAC platform code
> > > (drivers/dma/dw/platform.c), it is using the *_noirq suspend/resume PM
> > > callback. Is it advisable to use the normal suspend/resume PM callback instead
> > > of *_noirq suspend/PM callback? 
> > 
> > i dont see a reason why we need the noirq versions
> 
> Okay. I imagine the following use case.
> 
> For example we have compiled in DMA driver (dw_dmac) along with, for
> example, SPI driver.
> 
> System was scheduled to go sleep.
> 
> An order of calling IIUC might be DMA first, then SPI (since they are
> not in parent / child relations).
> 
> What was happened when SPI would like to do a DMA transfer and DMA is
> going to sleep? I'm trying to understand if this is a case.
In that case how does no irq version help us?

For these cases, I have been using suspend_late. Since the dmaengine driver is
providing service to other clients (SPI), it needs to esnure that it suspends
after SPI using suspend_late and resume using resume_early. That way dma is
availble whenever the client is active

--
~Vinod

> 
> > > > How about SET_SYSTEM_SLEEP_PM_OPS instead?
> > > 
> > > So, we are using *_noirq versions of the functions here. What happened when we switch to normal ones? Any side effects?
> 
> 
> 
> -- 
> Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Intel Finland Oy
>
Vinod Koul Jan. 20, 2014, 12:07 p.m. UTC | #7
On Mon, Jan 20, 2014 at 06:17:56PM +0530, Shevchenko, Andriy wrote:
> On Mon, 2014-01-20 at 14:55 +0530, Vinod Koul wrote:
> > On Thu, Dec 19, 2013 at 12:51:29PM +0200, Andy Shevchenko wrote:
> > > On Wed, 2013-12-18 at 21:19 +0530, Vinod Koul wrote:
> > > > On Mon, Dec 16, 2013 at 01:51:47PM +0530, Chew, Chiau Ee wrote:
> > > 
> > > > > As mentioned by Andy, we are using *_noirq  verion of suspend/resume PM
> > > > > callback whereby the callbacks would be executed after IRQ handlers have been
> > > > > disabled. If using SET_SYSTEM_SLEEP_PM_OPS, it would be the normal
> > > > > suspend/resume PM callback. Looking at the Desginware DMAC platform code
> > > > > (drivers/dma/dw/platform.c), it is using the *_noirq suspend/resume PM
> > > > > callback. Is it advisable to use the normal suspend/resume PM callback instead
> > > > > of *_noirq suspend/PM callback? 
> > > > 
> > > > i dont see a reason why we need the noirq versions
> > > 
> > > Okay. I imagine the following use case.
> > > 
> > > For example we have compiled in DMA driver (dw_dmac) along with, for
> > > example, SPI driver.
> > > 
> > > System was scheduled to go sleep.
> > > 
> > > An order of calling IIUC might be DMA first, then SPI (since they are
> > > not in parent / child relations).
> > > 
> > > What was happened when SPI would like to do a DMA transfer and DMA is
> > > going to sleep? I'm trying to understand if this is a case.
> > In that case how does no irq version help us?
> 
> It guarantees that we have no user of DMA anymore, since there is no
> interrupt going on.
well how is that. It will gaurantee that there wont be interrupt. User can still
submit a transaction or another transaction will be in progress...

> > For these cases, I have been using suspend_late. Since the dmaengine driver is
> > providing service to other clients (SPI), it needs to esnure that it suspends
> > after SPI using suspend_late and resume using resume_early. That way dma is
> > availble whenever the client is active
> 
> suspend_late is working in context that interrupt handler may be
> invoked. Thus, to have DMA driver be properly shut down we have to
> wait / terminate possible ongoing transfer.
Well client is already suspended via .suspend. So where is the transaction :)
> 
> It seems for me all DMA drivers that are using
> system .suspend()/.resume() are potentially buggy.
Yup!
Andy Shevchenko Jan. 20, 2014, 12:47 p.m. UTC | #8
On Mon, 2014-01-20 at 14:55 +0530, Vinod Koul wrote:
> On Thu, Dec 19, 2013 at 12:51:29PM +0200, Andy Shevchenko wrote:

> > On Wed, 2013-12-18 at 21:19 +0530, Vinod Koul wrote:

> > > On Mon, Dec 16, 2013 at 01:51:47PM +0530, Chew, Chiau Ee wrote:

> > 

> > > > As mentioned by Andy, we are using *_noirq  verion of suspend/resume PM

> > > > callback whereby the callbacks would be executed after IRQ handlers have been

> > > > disabled. If using SET_SYSTEM_SLEEP_PM_OPS, it would be the normal

> > > > suspend/resume PM callback. Looking at the Desginware DMAC platform code

> > > > (drivers/dma/dw/platform.c), it is using the *_noirq suspend/resume PM

> > > > callback. Is it advisable to use the normal suspend/resume PM callback instead

> > > > of *_noirq suspend/PM callback? 

> > > 

> > > i dont see a reason why we need the noirq versions

> > 

> > Okay. I imagine the following use case.

> > 

> > For example we have compiled in DMA driver (dw_dmac) along with, for

> > example, SPI driver.

> > 

> > System was scheduled to go sleep.

> > 

> > An order of calling IIUC might be DMA first, then SPI (since they are

> > not in parent / child relations).

> > 

> > What was happened when SPI would like to do a DMA transfer and DMA is

> > going to sleep? I'm trying to understand if this is a case.

> In that case how does no irq version help us?


It guarantees that we have no user of DMA anymore, since there is no
interrupt going on.

> For these cases, I have been using suspend_late. Since the dmaengine driver is

> providing service to other clients (SPI), it needs to esnure that it suspends

> after SPI using suspend_late and resume using resume_early. That way dma is

> availble whenever the client is active


suspend_late is working in context that interrupt handler may be
invoked. Thus, to have DMA driver be properly shut down we have to
wait / terminate possible ongoing transfer.

It seems for me all DMA drivers that are using
system .suspend()/.resume() are potentially buggy.

> 

> --

> ~Vinod

> 

> > 

> > > > > How about SET_SYSTEM_SLEEP_PM_OPS instead?

> > > > 

> > > > So, we are using *_noirq versions of the functions here. What happened when we switch to normal ones? Any side effects?

> > 

> > 

> > 

> > -- 

> > Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> > Intel Finland Oy

> > 

> 


-- 
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
Andy Shevchenko Jan. 20, 2014, 2:08 p.m. UTC | #9
On Mon, 2014-01-20 at 17:37 +0530, Vinod Koul wrote:
> On Mon, Jan 20, 2014 at 06:17:56PM +0530, Shevchenko, Andriy wrote:

> > On Mon, 2014-01-20 at 14:55 +0530, Vinod Koul wrote:

> > > On Thu, Dec 19, 2013 at 12:51:29PM +0200, Andy Shevchenko wrote:

> > > > On Wed, 2013-12-18 at 21:19 +0530, Vinod Koul wrote:

> > > > > On Mon, Dec 16, 2013 at 01:51:47PM +0530, Chew, Chiau Ee wrote:

> > > > 

> > > > > > As mentioned by Andy, we are using *_noirq  verion of suspend/resume PM

> > > > > > callback whereby the callbacks would be executed after IRQ handlers have been

> > > > > > disabled. If using SET_SYSTEM_SLEEP_PM_OPS, it would be the normal

> > > > > > suspend/resume PM callback. Looking at the Desginware DMAC platform code

> > > > > > (drivers/dma/dw/platform.c), it is using the *_noirq suspend/resume PM

> > > > > > callback. Is it advisable to use the normal suspend/resume PM callback instead

> > > > > > of *_noirq suspend/PM callback? 

> > > > > 

> > > > > i dont see a reason why we need the noirq versions

> > > > 

> > > > Okay. I imagine the following use case.

> > > > 

> > > > For example we have compiled in DMA driver (dw_dmac) along with, for

> > > > example, SPI driver.

> > > > 

> > > > System was scheduled to go sleep.

> > > > 

> > > > An order of calling IIUC might be DMA first, then SPI (since they are

> > > > not in parent / child relations).

> > > > 

> > > > What was happened when SPI would like to do a DMA transfer and DMA is

> > > > going to sleep? I'm trying to understand if this is a case.

> > > In that case how does no irq version help us?

> > 

> > It guarantees that we have no user of DMA anymore, since there is no

> > interrupt going on.

> well how is that. It will gaurantee that there wont be interrupt. User can still

> submit a transaction or another transaction will be in progress...


This is how system suspend callback tree is called.

First it calls .suspend() for all devices, then .suspend_late(),
then .suspend_noirq().

There is set of assumptions per each callback round. After .suspend()
the device must be quiescent.

But...

> > > For these cases, I have been using suspend_late. Since the dmaengine driver is

> > > providing service to other clients (SPI), it needs to esnure that it suspends

> > > after SPI using suspend_late and resume using resume_early. That way dma is

> > > availble whenever the client is active

> > 

> > suspend_late is working in context that interrupt handler may be

> > invoked. Thus, to have DMA driver be properly shut down we have to

> > wait / terminate possible ongoing transfer.

> Well client is already suspended via .suspend. So where is the transaction :)


...as I already wrote before we have no parent-child relationship
between DMA and, for example, SPI. That means we may possible have the
case when SPI's .suspend() will be called later than DMA's one.

> > It seems for me all DMA drivers that are using

> > system .suspend()/.resume() are potentially buggy.

> Yup!


So, we have to decide what to do with them. .suspend_late() still seems
for me not the best approach. *Or* we have to check for ongoing
transaction and do something with it. *Or* just shut down the device and
rely on DMA transaction initiator that it handles the terminated
transaction properly.

What is you opinion?


-- 
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
Vinod Koul Jan. 26, 2014, 11:17 a.m. UTC | #10
On Mon, Jan 20, 2014 at 07:38:32PM +0530, Shevchenko, Andriy wrote:
> > > > > > > As mentioned by Andy, we are using *_noirq  verion of suspend/resume PM
> > > > > > > callback whereby the callbacks would be executed after IRQ handlers have been
> > > > > > > disabled. If using SET_SYSTEM_SLEEP_PM_OPS, it would be the normal
> > > > > > > suspend/resume PM callback. Looking at the Desginware DMAC platform code
> > > > > > > (drivers/dma/dw/platform.c), it is using the *_noirq suspend/resume PM
> > > > > > > callback. Is it advisable to use the normal suspend/resume PM callback instead
> > > > > > > of *_noirq suspend/PM callback? 
> > > > > > 
> > > > > > i dont see a reason why we need the noirq versions
> > > > > 
> > > > > Okay. I imagine the following use case.
> > > > > 
> > > > > For example we have compiled in DMA driver (dw_dmac) along with, for
> > > > > example, SPI driver.
> > > > > 
> > > > > System was scheduled to go sleep.
> > > > > 
> > > > > An order of calling IIUC might be DMA first, then SPI (since they are
> > > > > not in parent / child relations).
> > > > > 
> > > > > What was happened when SPI would like to do a DMA transfer and DMA is
> > > > > going to sleep? I'm trying to understand if this is a case.
> > > > In that case how does no irq version help us?
> > > 
> > > It guarantees that we have no user of DMA anymore, since there is no
> > > interrupt going on.
> > well how is that. It will gaurantee that there wont be interrupt. User can still
> > submit a transaction or another transaction will be in progress...
> 
> This is how system suspend callback tree is called.
> 
> First it calls .suspend() for all devices, then .suspend_late(),
> then .suspend_noirq().
> 
> There is set of assumptions per each callback round. After .suspend()
> the device must be quiescent.
> 
> But...
> 
> > > > For these cases, I have been using suspend_late. Since the dmaengine driver is
> > > > providing service to other clients (SPI), it needs to esnure that it suspends
> > > > after SPI using suspend_late and resume using resume_early. That way dma is
> > > > availble whenever the client is active
> > > 
> > > suspend_late is working in context that interrupt handler may be
> > > invoked. Thus, to have DMA driver be properly shut down we have to
> > > wait / terminate possible ongoing transfer.
> > Well client is already suspended via .suspend. So where is the transaction :)
> 
> ...as I already wrote before we have no parent-child relationship
> between DMA and, for example, SPI. That means we may possible have the
> case when SPI's .suspend() will be called later than DMA's one.
> 
> > > It seems for me all DMA drivers that are using
> > > system .suspend()/.resume() are potentially buggy.
> > Yup!
> 
> So, we have to decide what to do with them. .suspend_late() still seems
> for me not the best approach. *Or* we have to check for ongoing
> transaction and do something with it. *Or* just shut down the device and
> rely on DMA transaction initiator that it handles the terminated
> transaction properly.

As you clearly said, we dont have a parent-child relatation though we have big
dependency. I think this is true for DMA clients, i ran into similar situation
with i2c few days back!

So only think which can give us a good system behaviour would be clients getting
suspended first and then then service providing subsystems. (same reason why we
do dma driver loading and init much before others drivers)

So yes in the .suspend callback of the client, it needs to
1) abort any transactions it has
2) make the client quiscent

then the .late_suspend kicks in and suspend the core drivers like dma.

This is how it has worked reliably for me in production systems. I am all ears
if we have a better and cleaner apprach to this problem :)
Andy Shevchenko Jan. 27, 2014, 10:17 a.m. UTC | #11
On Sun, 2014-01-26 at 16:47 +0530, Vinod Koul wrote:

> > > > > For these cases, I have been using suspend_late. Since the dmaengine driver is
> > > > > providing service to other clients (SPI), it needs to esnure that it suspends
> > > > > after SPI using suspend_late and resume using resume_early. That way dma is
> > > > > availble whenever the client is active
> > > > 
> > > > suspend_late is working in context that interrupt handler may be
> > > > invoked. Thus, to have DMA driver be properly shut down we have to
> > > > wait / terminate possible ongoing transfer.
> > > Well client is already suspended via .suspend. So where is the transaction :)
> > 
> > ...as I already wrote before we have no parent-child relationship
> > between DMA and, for example, SPI. That means we may possible have the
> > case when SPI's .suspend() will be called later than DMA's one.
> > 
> > > > It seems for me all DMA drivers that are using
> > > > system .suspend()/.resume() are potentially buggy.
> > > Yup!
> > 
> > So, we have to decide what to do with them. .suspend_late() still seems
> > for me not the best approach. *Or* we have to check for ongoing
> > transaction and do something with it. *Or* just shut down the device and
> > rely on DMA transaction initiator that it handles the terminated
> > transaction properly.
> 
> As you clearly said, we dont have a parent-child relatation though we have big
> dependency. I think this is true for DMA clients, i ran into similar situation
> with i2c few days back!
> 
> So only think which can give us a good system behaviour would be clients getting
> suspended first and then then service providing subsystems.

Agree.

>  (same reason why we
> do dma driver loading and init much before others drivers)

Yes, it would be done via deferred probe.

> So yes in the .suspend callback of the client, it needs to
> 1) abort any transactions it has
> 2) make the client quiscent
> 
> then the .late_suspend kicks in and suspend the core drivers like dma.
> 
> This is how it has worked reliably for me in production systems. I am all ears
> if we have a better and cleaner apprach to this problem :)

Yes, summarize everything we discussed we have to:
 - provide suspend_late, resume_early callbacks in the DMA driver
instead of *_noirq versions
 - ensure that all clients on our platforms follow the described
scenario

Chiau Ee, I think you may to change your patch accordingly.
Chew Chiau Ee Jan. 28, 2014, 7:21 a.m. UTC | #12
> -----Original Message-----

> From: Andy Shevchenko [mailto:andriy.shevchenko@linux.intel.com]

> Sent: Monday, January 27, 2014 6:17 PM

> To: Koul, Vinod

> Cc: Andy Shevchenko; Chew, Chiau Ee; Viresh Kumar; Williams, Dan J;

> dmaengine@vger.kernel.org; linux-kernel@vger.kernel.org

> Subject: Re: [PATCH] dma: dw: Add suspend and resume handling for PCI mode

> DW_DMAC.

> 

> On Sun, 2014-01-26 at 16:47 +0530, Vinod Koul wrote:

> 

> > > > > > For these cases, I have been using suspend_late. Since the

> > > > > > dmaengine driver is providing service to other clients (SPI),

> > > > > > it needs to esnure that it suspends after SPI using

> > > > > > suspend_late and resume using resume_early. That way dma is

> > > > > > availble whenever the client is active

> > > > >

> > > > > suspend_late is working in context that interrupt handler may be

> > > > > invoked. Thus, to have DMA driver be properly shut down we have

> > > > > to wait / terminate possible ongoing transfer.

> > > > Well client is already suspended via .suspend. So where is the

> > > > transaction :)

> > >

> > > ...as I already wrote before we have no parent-child relationship

> > > between DMA and, for example, SPI. That means we may possible have

> > > the case when SPI's .suspend() will be called later than DMA's one.

> > >

> > > > > It seems for me all DMA drivers that are using system

> > > > > .suspend()/.resume() are potentially buggy.

> > > > Yup!

> > >

> > > So, we have to decide what to do with them. .suspend_late() still

> > > seems for me not the best approach. *Or* we have to check for

> > > ongoing transaction and do something with it. *Or* just shut down

> > > the device and rely on DMA transaction initiator that it handles the

> > > terminated transaction properly.

> >

> > As you clearly said, we dont have a parent-child relatation though we

> > have big dependency. I think this is true for DMA clients, i ran into

> > similar situation with i2c few days back!

> >

> > So only think which can give us a good system behaviour would be

> > clients getting suspended first and then then service providing subsystems.

> 

> Agree.

> 

> >  (same reason why we

> > do dma driver loading and init much before others drivers)

> 

> Yes, it would be done via deferred probe.

> 

> > So yes in the .suspend callback of the client, it needs to

> > 1) abort any transactions it has

> > 2) make the client quiscent

> >

> > then the .late_suspend kicks in and suspend the core drivers like dma.

> >

> > This is how it has worked reliably for me in production systems. I am

> > all ears if we have a better and cleaner apprach to this problem :)

> 

> Yes, summarize everything we discussed we have to:

>  - provide suspend_late, resume_early callbacks in the DMA driver instead of

> *_noirq versions

>  - ensure that all clients on our platforms follow the described scenario

> 

> Chiau Ee, I think you may to change your patch accordingly.


Ok. Noted

> 

> 

> --

> Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> Intel Finland Oy
diff mbox

Patch

diff --git a/drivers/dma/dw/pci.c b/drivers/dma/dw/pci.c
index e89fc24..97bc3a2 100644
--- a/drivers/dma/dw/pci.c
+++ b/drivers/dma/dw/pci.c
@@ -75,6 +75,36 @@  static void dw_pci_remove(struct pci_dev *pdev)
 		dev_warn(&pdev->dev, "can't remove device properly: %d\n", ret);
 }
 
+#ifdef CONFIG_PM_SLEEP
+
+static int dw_pci_suspend_noirq(struct device *dev)
+{
+	struct pci_dev *pci = to_pci_dev(dev);
+	struct dw_dma_chip *chip = pci_get_drvdata(pci);
+
+	return dw_dma_suspend(chip);
+};
+
+static int dw_pci_resume_noirq(struct device *dev)
+{
+	struct pci_dev *pci = to_pci_dev(dev);
+	struct dw_dma_chip *chip = pci_get_drvdata(pci);
+
+	return dw_dma_resume(chip);
+};
+
+#else /* !CONFIG_PM_SLEEP */
+
+#define dw_pci_suspend_noirq	NULL
+#define dw_pci_resume_noirq	NULL
+
+#endif /* !CONFIG_PM_SLEEP */
+
+static const struct dev_pm_ops dw_pci_dev_pm_ops = {
+	.suspend_noirq = dw_pci_suspend_noirq,
+	.resume_noirq = dw_pci_resume_noirq,
+};
+
 static DEFINE_PCI_DEVICE_TABLE(dw_pci_id_table) = {
 	/* Medfield */
 	{ PCI_VDEVICE(INTEL, 0x0827), (kernel_ulong_t)&dw_pci_pdata },
@@ -92,6 +122,9 @@  static struct pci_driver dw_pci_driver = {
 	.id_table	= dw_pci_id_table,
 	.probe		= dw_pci_probe,
 	.remove		= dw_pci_remove,
+	.driver	= {
+		.pm	= &dw_pci_dev_pm_ops,
+	},
 };
 
 module_pci_driver(dw_pci_driver);