diff mbox

[4/4,v4] GPIO: gpio-dwapb: Suspend & Resume PM enabling

Message ID 1410859335-11080-5-git-send-email-alvin.chen@intel.com
State Not Applicable, archived
Headers show

Commit Message

Chen, Alvin Sept. 16, 2014, 9:22 a.m. UTC 
This patch enables suspend and resume mode for the power management, and
it is based on Josef Ahmad's previous work.

Reviewed-by: Hock Leong Kweh <hock.leong.kweh@intel.com>
Signed-off-by: Weike Chen <alvin.chen@intel.com>
---
 drivers/gpio/gpio-dwapb.c |  109 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 109 insertions(+)

Comments

Andy Shevchenko Sept. 16, 2014, 7:24 a.m. UTC | #1 
T24gVHVlLCAyMDE0LTA5LTE2IGF0IDAyOjIyIC0wNzAwLCBXZWlrZSBDaGVuIHdyb3RlOg0KPiBU
aGlzIHBhdGNoIGVuYWJsZXMgc3VzcGVuZCBhbmQgcmVzdW1lIG1vZGUgZm9yIHRoZSBwb3dlciBt
YW5hZ2VtZW50LCBhbmQNCj4gaXQgaXMgYmFzZWQgb24gSm9zZWYgQWhtYWQncyBwcmV2aW91cyB3
b3JrLg0KPiANCg0KRmV3IGNvbW1lbnRzIGJlbG93Lg0KDQo+IFJldmlld2VkLWJ5OiBIb2NrIExl
b25nIEt3ZWggPGhvY2subGVvbmcua3dlaEBpbnRlbC5jb20+DQoNCllvdSBzdGlsbCBrZWVwIHRo
YXQgZ3V5IGFzIHJldmlld2VyIGluIGEgd2hvbGUgc2VyaWVzLCBob3dldmVyIEkgZGlkbid0DQpz
ZWUgYSB3b3JkIGZyb20gaGltIGhlcmUuIEhvdyBpcyBpdCBwb3NzaWJsZT8NCg0KPiBTaWduZWQt
b2ZmLWJ5OiBXZWlrZSBDaGVuIDxhbHZpbi5jaGVuQGludGVsLmNvbT4NCj4gLS0tDQo+ICBkcml2
ZXJzL2dwaW8vZ3Bpby1kd2FwYi5jIHwgIDEwOSArKysrKysrKysrKysrKysrKysrKysrKysrKysr
KysrKysrKysrKysrKysrKysNCj4gIDEgZmlsZSBjaGFuZ2VkLCAxMDkgaW5zZXJ0aW9ucygrKQ0K
PiANCj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvZ3Bpby9ncGlvLWR3YXBiLmMgYi9kcml2ZXJzL2dw
aW8vZ3Bpby1kd2FwYi5jDQo+IGluZGV4IDlhNzZlM2MuLjE0ZDgzYWYgMTAwNjQ0DQo+IC0tLSBh
L2RyaXZlcnMvZ3Bpby9ncGlvLWR3YXBiLmMNCj4gKysrIGIvZHJpdmVycy9ncGlvL2dwaW8tZHdh
cGIuYw0KPiBAQCAtNTAsMTEgKzUwLDE0IEBADQo+ICAjZGVmaW5lIEdQSU9fU1dQT1JUX0REUl9T
SVpFCShHUElPX1NXUE9SVEJfRERSIC0gR1BJT19TV1BPUlRBX0REUikNCj4gIA0KPiAgc3RydWN0
IGR3YXBiX2dwaW87DQo+ICtzdHJ1Y3QgZHdhcGJfY29udGV4dDsNCj4gIA0KPiAgc3RydWN0IGR3
YXBiX2dwaW9fcG9ydCB7DQo+ICAJc3RydWN0IGJncGlvX2NoaXAJYmdjOw0KPiAgCWJvb2wJCQlp
c19yZWdpc3RlcmVkOw0KPiAgCXN0cnVjdCBkd2FwYl9ncGlvCSpncGlvOw0KPiArCXN0cnVjdCBk
d2FwYl9jb250ZXh0CSpjdHg7DQo+ICsJdW5zaWduZWQgaW50CQlpZHg7DQo+ICB9Ow0KPiAgDQo+
ICBzdHJ1Y3QgZHdhcGJfZ3BpbyB7DQo+IEBAIC0zNzcsNiArMzgwLDcgQEAgc3RhdGljIGludCBk
d2FwYl9ncGlvX2FkZF9wb3J0KHN0cnVjdCBkd2FwYl9ncGlvICpncGlvLA0KPiAgDQo+ICAJcG9y
dCA9ICZncGlvLT5wb3J0c1tvZmZzXTsNCj4gIAlwb3J0LT5ncGlvID0gZ3BpbzsNCj4gKwlwb3J0
LT5pZHggPSBwcC0+aWR4Ow0KPiAgDQo+ICAJZGF0ID0gZ3Bpby0+cmVncyArIEdQSU9fRVhUX1BP
UlRBICsgKHBwLT5pZHggKiBHUElPX0VYVF9QT1JUX1NJWkUpOw0KPiAgCXNldCA9IGdwaW8tPnJl
Z3MgKyBHUElPX1NXUE9SVEFfRFIgKyAocHAtPmlkeCAqIEdQSU9fU1dQT1JUX0RSX1NJWkUpOw0K
PiBAQCAtNTg0LDEwICs1ODgsMTE1IEBAIHN0YXRpYyBjb25zdCBzdHJ1Y3Qgb2ZfZGV2aWNlX2lk
IGR3YXBiX29mX21hdGNoW10gPSB7DQo+ICB9Ow0KPiAgTU9EVUxFX0RFVklDRV9UQUJMRShvZiwg
ZHdhcGJfb2ZfbWF0Y2gpOw0KPiAgDQo+ICsjaWZkZWYgQ09ORklHX1BNX1NMRUVQDQo+ICsvKiBT
dG9yZSBHUElPIGNvbnRleHQgYWNyb3NzIHN5c3RlbS13aWRlIHN1c3BlbmQvcmVzdW1lIHRyYW5z
aXRpb25zICovDQo+ICtzdHJ1Y3QgZHdhcGJfY29udGV4dCB7DQo+ICsJdTMyIGRhdGE7DQo+ICsJ
dTMyIGRpcjsNCj4gKwl1MzIgZXh0Ow0KPiArCXUzMiBpbnRfZW47DQo+ICsJdTMyIGludF9tYXNr
Ow0KPiArCXUzMiBpbnRfdHlwZTsNCj4gKwl1MzIgaW50X3BvbDsNCj4gKwl1MzIgaW50X2RlYjsN
Cj4gK307DQoNCj4gKw0KPiArc3RhdGljIGludCBkd2FwYl9ncGlvX3N1c3BlbmQoc3RydWN0IGRl
dmljZSAqZGV2KQ0KPiArew0KPiArCXN0cnVjdCBwbGF0Zm9ybV9kZXZpY2UgKnBkZXYgPSB0b19w
bGF0Zm9ybV9kZXZpY2UoZGV2KTsNCj4gKwlzdHJ1Y3QgZHdhcGJfZ3BpbyAqZ3BpbyA9IHBsYXRm
b3JtX2dldF9kcnZkYXRhKHBkZXYpOw0KPiArCXN0cnVjdCBiZ3Bpb19jaGlwICpiZ2MJPSAmZ3Bp
by0+cG9ydHNbMF0uYmdjOw0KPiArCXVuc2lnbmVkIGxvbmcgZmxhZ3M7DQo+ICsJaW50IGk7DQo+
ICsNCj4gKwlzcGluX2xvY2tfaXJxc2F2ZSgmYmdjLT5sb2NrLCBmbGFncyk7DQo+ICsJZm9yIChp
ID0gMDsgaSA8IGdwaW8tPm5yX3BvcnRzOyBpKyspIHsNCj4gKwkJdW5zaWduZWQgaW50IG9mZnNl
dDsNCj4gKwkJdW5zaWduZWQgaW50IGlkeCA9IGdwaW8tPnBvcnRzW2ldLmlkeDsNCj4gKwkJc3Ry
dWN0IGR3YXBiX2NvbnRleHQgKmN0eCA9IGdwaW8tPnBvcnRzW2ldLmN0eDsNCj4gKw0KPiArCQlp
ZiAoIWN0eCkgew0KPiArCQkJY3R4ID0gZGV2bV9remFsbG9jKGRldiwgc2l6ZW9mKCpjdHgpLCBH
RlBfS0VSTkVMKTsNCj4gKwkJCWdwaW8tPnBvcnRzW2ldLmN0eCA9IGN0eDsNCj4gKwkJfQ0KDQpJ
IGRvbid0IHRoaW5rIGl0J3MgYSByaWdodCBwbGFjZSB0byBhbGxvY2F0ZSB0aGlzIHJlc291cmNl
LCBlc3BlY2lhbGx5DQp3aXRoIGRldm1fIGhlbHBlci4gQ2FuIHlvdSBtb3ZlIHRoaXMgdG8gcHJv
YmUoKSBzdGFnZT8NCg0KT3IgeW91IGV2ZW4gY2FuIGVtYmVkIGNvbnRleCBzdHJ1Y3R1cmUgaW5z
aWRlIGNoaXAgb25lIHdpdGggI2lmZGVmDQpDT05GSUdfUE1fU0xFRVAuDQoNCk1heWJlIG90aGVy
cyBjb3VsZCBjb21tZW50IG9uIHRoaXMuDQoNCj4gKw0KPiArCQlvZmZzZXQgPSBHUElPX1NXUE9S
VEFfRERSICsgKGlkeCAqIEdQSU9fU1dQT1JUX0REUl9TSVpFKTsNCg0KTm8gbmVlZCB0byBoYXZl
IHBhcmVudGhlc2VzIGhlcmUuIENoZWNrIHRoZSBjb2RlIGJlbG93IGFzIHdlbGwuDQoNCj4gKwkJ
Y3R4LT5kaXIgPSBkd2FwYl9yZWFkKGdwaW8sIG9mZnNldCk7DQo+ICsNCj4gKwkJb2Zmc2V0ID0g
R1BJT19TV1BPUlRBX0RSICsgKGlkeCAqIEdQSU9fU1dQT1JUX0RSX1NJWkUpOw0KPiArCQljdHgt
PmRhdGEgPSBkd2FwYl9yZWFkKGdwaW8sIG9mZnNldCk7DQo+ICsNCj4gKwkJb2Zmc2V0ID0gR1BJ
T19FWFRfUE9SVEEgKyAoaWR4ICogR1BJT19FWFRfUE9SVF9TSVpFKTsNCj4gKwkJY3R4LT5leHQg
PSBkd2FwYl9yZWFkKGdwaW8sIG9mZnNldCk7DQo+ICsNCj4gKwkJLyogT25seSBwb3J0IEEgY2Fu
IHByb3ZpZGUgaW50ZXJydXB0cyAqLw0KPiArCQlpZiAoaWR4ID09IDApIHsNCj4gKwkJCWN0eC0+
aW50X21hc2sJPSBkd2FwYl9yZWFkKGdwaW8sIEdQSU9fSU5UTUFTSyk7DQo+ICsJCQljdHgtPmlu
dF9lbgk9IGR3YXBiX3JlYWQoZ3BpbywgR1BJT19JTlRFTik7DQo+ICsJCQljdHgtPmludF9wb2wJ
PSBkd2FwYl9yZWFkKGdwaW8sIEdQSU9fSU5UX1BPTEFSSVRZKTsNCj4gKwkJCWN0eC0+aW50X3R5
cGUJPSBkd2FwYl9yZWFkKGdwaW8sIEdQSU9fSU5UVFlQRV9MRVZFTCk7DQo+ICsJCQljdHgtPmlu
dF9kZWIJPSBkd2FwYl9yZWFkKGdwaW8sIEdQSU9fUE9SVEFfREVCT1VOQ0UpOw0KPiArDQo+ICsJ
CQkvKiBNYXNrIG91dCBpbnRlcnJ1cHRzICovDQo+ICsJCQlkd2FwYl93cml0ZShncGlvLCBHUElP
X0lOVE1BU0ssIDB4ZmZmZmZmZmYpOw0KPiArCQl9DQo+ICsJfQ0KPiArCXNwaW5fdW5sb2NrX2ly
cXJlc3RvcmUoJmJnYy0+bG9jaywgZmxhZ3MpOw0KPiArDQo+ICsJcmV0dXJuIDA7DQo+ICt9DQo+
ICsNCj4gK3N0YXRpYyBpbnQgZHdhcGJfZ3Bpb19yZXN1bWUoc3RydWN0IGRldmljZSAqZGV2KQ0K
PiArew0KPiArCXN0cnVjdCBwbGF0Zm9ybV9kZXZpY2UgKnBkZXYgPSB0b19wbGF0Zm9ybV9kZXZp
Y2UoZGV2KTsNCj4gKwlzdHJ1Y3QgZHdhcGJfZ3BpbyAqZ3BpbyA9IHBsYXRmb3JtX2dldF9kcnZk
YXRhKHBkZXYpOw0KPiArCXN0cnVjdCBiZ3Bpb19jaGlwICpiZ2MJPSAmZ3Bpby0+cG9ydHNbMF0u
YmdjOw0KPiArCXVuc2lnbmVkIGxvbmcgZmxhZ3M7DQo+ICsJaW50IGk7DQo+ICsNCj4gKwlzcGlu
X2xvY2tfaXJxc2F2ZSgmYmdjLT5sb2NrLCBmbGFncyk7DQo+ICsJZm9yIChpID0gMDsgaSA8IGdw
aW8tPm5yX3BvcnRzOyBpKyspIHsNCj4gKwkJdW5zaWduZWQgaW50IG9mZnNldDsNCj4gKwkJdW5z
aWduZWQgaW50IGlkeCA9IGdwaW8tPnBvcnRzW2ldLmlkeDsNCj4gKwkJc3RydWN0IGR3YXBiX2Nv
bnRleHQgKmN0eCA9IGdwaW8tPnBvcnRzW2ldLmN0eDsNCj4gKw0KPiArCQlCVUdfT04oY3R4ID09
IDApOw0KPiArDQo+ICsJCW9mZnNldCA9IEdQSU9fU1dQT1JUQV9EUiArIChpZHggKiBHUElPX1NX
UE9SVF9EUl9TSVpFKTsNCj4gKwkJZHdhcGJfd3JpdGUoZ3Bpbywgb2Zmc2V0LCBjdHgtPmRhdGEp
Ow0KPiArDQo+ICsJCW9mZnNldCA9IEdQSU9fU1dQT1JUQV9ERFIgKyAoaWR4ICogR1BJT19TV1BP
UlRfRERSX1NJWkUpOw0KPiArCQlkd2FwYl93cml0ZShncGlvLCBvZmZzZXQsIGN0eC0+ZGlyKTsN
Cj4gKw0KPiArCQlvZmZzZXQgPSBHUElPX0VYVF9QT1JUQSArIChpZHggKiBHUElPX0VYVF9QT1JU
X1NJWkUpOw0KPiArCQlkd2FwYl93cml0ZShncGlvLCBvZmZzZXQsIGN0eC0+ZXh0KTsNCj4gKw0K
PiArCQkvKiBPbmx5IHBvcnQgQSBjYW4gcHJvdmlkZSBpbnRlcnJ1cHRzICovDQo+ICsJCWlmIChp
ZHggPT0gMCkgew0KPiArCQkJZHdhcGJfd3JpdGUoZ3BpbywgR1BJT19JTlRUWVBFX0xFVkVMLCBj
dHgtPmludF90eXBlKTsNCj4gKwkJCWR3YXBiX3dyaXRlKGdwaW8sIEdQSU9fSU5UX1BPTEFSSVRZ
LCBjdHgtPmludF9wb2wpOw0KPiArCQkJZHdhcGJfd3JpdGUoZ3BpbywgR1BJT19QT1JUQV9ERUJP
VU5DRSwgY3R4LT5pbnRfZGViKTsNCj4gKwkJCWR3YXBiX3dyaXRlKGdwaW8sIEdQSU9fSU5URU4s
IGN0eC0+aW50X2VuKTsNCj4gKwkJCWR3YXBiX3dyaXRlKGdwaW8sIEdQSU9fSU5UTUFTSywgY3R4
LT5pbnRfbWFzayk7DQo+ICsNCj4gKwkJCS8qIENsZWFyIG91dCBzcHVyaW91cyBpbnRlcnJ1cHRz
ICovDQo+ICsJCQlkd2FwYl93cml0ZShncGlvLCBHUElPX1BPUlRBX0VPSSwgMHhmZmZmZmZmZik7
DQo+ICsJCX0NCj4gKwl9DQo+ICsJc3Bpbl91bmxvY2tfaXJxcmVzdG9yZSgmYmdjLT5sb2NrLCBm
bGFncyk7DQo+ICsNCj4gKwlyZXR1cm4gMDsNCj4gK30NCj4gKyNlbmRpZg0KPiArDQo+ICtzdGF0
aWMgU0lNUExFX0RFVl9QTV9PUFMoZHdhcGJfZ3Bpb19wbV9vcHMsIGR3YXBiX2dwaW9fc3VzcGVu
ZCwNCj4gKwkJCSBkd2FwYl9ncGlvX3Jlc3VtZSk7DQo+ICsNCj4gIHN0YXRpYyBzdHJ1Y3QgcGxh
dGZvcm1fZHJpdmVyIGR3YXBiX2dwaW9fZHJpdmVyID0gew0KPiAgCS5kcml2ZXIJCT0gew0KPiAg
CQkubmFtZQk9ICJncGlvLWR3YXBiIiwNCj4gIAkJLm93bmVyCT0gVEhJU19NT0RVTEUsDQo+ICsJ
CS5wbQk9ICZkd2FwYl9ncGlvX3BtX29wcywNCj4gIAkJLm9mX21hdGNoX3RhYmxlID0gb2ZfbWF0
Y2hfcHRyKGR3YXBiX29mX21hdGNoKSwNCj4gIAl9LA0KPiAgCS5wcm9iZQkJPSBkd2FwYl9ncGlv
X3Byb2JlLA0KDQoNCi0tIA0KQW5keSBTaGV2Y2hlbmtvIDxhbmRyaXkuc2hldmNoZW5rb0BpbnRl
bC5jb20+DQpJbnRlbCBGaW5sYW5kIE95DQotLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0KSW50ZWwgRmlubGFuZCBPeQpS
ZWdpc3RlcmVkIEFkZHJlc3M6IFBMIDI4MSwgMDAxODEgSGVsc2lua2kgCkJ1c2luZXNzIElkZW50
aXR5IENvZGU6IDAzNTc2MDYgLSA0IApEb21pY2lsZWQgaW4gSGVsc2lua2kgCgpUaGlzIGUtbWFp
bCBhbmQgYW55IGF0dGFjaG1lbnRzIG1heSBjb250YWluIGNvbmZpZGVudGlhbCBtYXRlcmlhbCBm
b3IKdGhlIHNvbGUgdXNlIG9mIHRoZSBpbnRlbmRlZCByZWNpcGllbnQocykuIEFueSByZXZpZXcg
b3IgZGlzdHJpYnV0aW9uCmJ5IG90aGVycyBpcyBzdHJpY3RseSBwcm9oaWJpdGVkLiBJZiB5b3Ug
YXJlIG5vdCB0aGUgaW50ZW5kZWQKcmVjaXBpZW50LCBwbGVhc2UgY29udGFjdCB0aGUgc2VuZGVy
IGFuZCBkZWxldGUgYWxsIGNvcGllcy4K

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andy Shevchenko Sept. 16, 2014, 7:26 a.m. UTC | #2 
On Tue, 2014-09-16 at 02:22 -0700, Weike Chen wrote:
> This patch enables suspend and resume mode for the power management, and
> it is based on Josef Ahmad's previous work.
> 

Few comments below.

> Reviewed-by: Hock Leong Kweh <hock.leong.kweh@intel.com>

You still keep that guy as reviewer in a whole series, however I didn't
see a word from him here. How is it possible?

> Signed-off-by: Weike Chen <alvin.chen@intel.com>
> ---
>  drivers/gpio/gpio-dwapb.c |  109 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 109 insertions(+)
> 
> diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
> index 9a76e3c..14d83af 100644
> --- a/drivers/gpio/gpio-dwapb.c
> +++ b/drivers/gpio/gpio-dwapb.c
> @@ -50,11 +50,14 @@
>  #define GPIO_SWPORT_DDR_SIZE	(GPIO_SWPORTB_DDR - GPIO_SWPORTA_DDR)
>  
>  struct dwapb_gpio;
> +struct dwapb_context;
>  
>  struct dwapb_gpio_port {
>  	struct bgpio_chip	bgc;
>  	bool			is_registered;
>  	struct dwapb_gpio	*gpio;
> +	struct dwapb_context	*ctx;
> +	unsigned int		idx;
>  };
>  
>  struct dwapb_gpio {
> @@ -377,6 +380,7 @@ static int dwapb_gpio_add_port(struct dwapb_gpio *gpio,
>  
>  	port = &gpio->ports[offs];
>  	port->gpio = gpio;
> +	port->idx = pp->idx;
>  
>  	dat = gpio->regs + GPIO_EXT_PORTA + (pp->idx * GPIO_EXT_PORT_SIZE);
>  	set = gpio->regs + GPIO_SWPORTA_DR + (pp->idx * GPIO_SWPORT_DR_SIZE);
> @@ -584,10 +588,115 @@ static const struct of_device_id dwapb_of_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, dwapb_of_match);
>  
> +#ifdef CONFIG_PM_SLEEP
> +/* Store GPIO context across system-wide suspend/resume transitions */
> +struct dwapb_context {
> +	u32 data;
> +	u32 dir;
> +	u32 ext;
> +	u32 int_en;
> +	u32 int_mask;
> +	u32 int_type;
> +	u32 int_pol;
> +	u32 int_deb;
> +};

> +
> +static int dwapb_gpio_suspend(struct device *dev)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct dwapb_gpio *gpio = platform_get_drvdata(pdev);
> +	struct bgpio_chip *bgc	= &gpio->ports[0].bgc;
> +	unsigned long flags;
> +	int i;
> +
> +	spin_lock_irqsave(&bgc->lock, flags);
> +	for (i = 0; i < gpio->nr_ports; i++) {
> +		unsigned int offset;
> +		unsigned int idx = gpio->ports[i].idx;
> +		struct dwapb_context *ctx = gpio->ports[i].ctx;
> +
> +		if (!ctx) {
> +			ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> +			gpio->ports[i].ctx = ctx;
> +		}

I don't think it's a right place to allocate this resource, especially
with devm_ helper. Can you move this to probe() stage?

Or you even can embed contex structure inside chip one with #ifdef
CONFIG_PM_SLEEP.

Maybe others could comment on this.

> +
> +		offset = GPIO_SWPORTA_DDR + (idx * GPIO_SWPORT_DDR_SIZE);

No need to have parentheses here. Check the code below as well.

> +		ctx->dir = dwapb_read(gpio, offset);
> +
> +		offset = GPIO_SWPORTA_DR + (idx * GPIO_SWPORT_DR_SIZE);
> +		ctx->data = dwapb_read(gpio, offset);
> +
> +		offset = GPIO_EXT_PORTA + (idx * GPIO_EXT_PORT_SIZE);
> +		ctx->ext = dwapb_read(gpio, offset);
> +
> +		/* Only port A can provide interrupts */
> +		if (idx == 0) {
> +			ctx->int_mask	= dwapb_read(gpio, GPIO_INTMASK);
> +			ctx->int_en	= dwapb_read(gpio, GPIO_INTEN);
> +			ctx->int_pol	= dwapb_read(gpio, GPIO_INT_POLARITY);
> +			ctx->int_type	= dwapb_read(gpio, GPIO_INTTYPE_LEVEL);
> +			ctx->int_deb	= dwapb_read(gpio, GPIO_PORTA_DEBOUNCE);
> +
> +			/* Mask out interrupts */
> +			dwapb_write(gpio, GPIO_INTMASK, 0xffffffff);
> +		}
> +	}
> +	spin_unlock_irqrestore(&bgc->lock, flags);
> +
> +	return 0;
> +}
> +
> +static int dwapb_gpio_resume(struct device *dev)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct dwapb_gpio *gpio = platform_get_drvdata(pdev);
> +	struct bgpio_chip *bgc	= &gpio->ports[0].bgc;
> +	unsigned long flags;
> +	int i;
> +
> +	spin_lock_irqsave(&bgc->lock, flags);
> +	for (i = 0; i < gpio->nr_ports; i++) {
> +		unsigned int offset;
> +		unsigned int idx = gpio->ports[i].idx;
> +		struct dwapb_context *ctx = gpio->ports[i].ctx;
> +
> +		BUG_ON(ctx == 0);
> +
> +		offset = GPIO_SWPORTA_DR + (idx * GPIO_SWPORT_DR_SIZE);
> +		dwapb_write(gpio, offset, ctx->data);
> +
> +		offset = GPIO_SWPORTA_DDR + (idx * GPIO_SWPORT_DDR_SIZE);
> +		dwapb_write(gpio, offset, ctx->dir);
> +
> +		offset = GPIO_EXT_PORTA + (idx * GPIO_EXT_PORT_SIZE);
> +		dwapb_write(gpio, offset, ctx->ext);
> +
> +		/* Only port A can provide interrupts */
> +		if (idx == 0) {
> +			dwapb_write(gpio, GPIO_INTTYPE_LEVEL, ctx->int_type);
> +			dwapb_write(gpio, GPIO_INT_POLARITY, ctx->int_pol);
> +			dwapb_write(gpio, GPIO_PORTA_DEBOUNCE, ctx->int_deb);
> +			dwapb_write(gpio, GPIO_INTEN, ctx->int_en);
> +			dwapb_write(gpio, GPIO_INTMASK, ctx->int_mask);
> +
> +			/* Clear out spurious interrupts */
> +			dwapb_write(gpio, GPIO_PORTA_EOI, 0xffffffff);
> +		}
> +	}
> +	spin_unlock_irqrestore(&bgc->lock, flags);
> +
> +	return 0;
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(dwapb_gpio_pm_ops, dwapb_gpio_suspend,
> +			 dwapb_gpio_resume);
> +
>  static struct platform_driver dwapb_gpio_driver = {
>  	.driver		= {
>  		.name	= "gpio-dwapb",
>  		.owner	= THIS_MODULE,
> +		.pm	= &dwapb_gpio_pm_ops,
>  		.of_match_table = of_match_ptr(dwapb_of_match),
>  	},
>  	.probe		= dwapb_gpio_probe,
Chen, Alvin Sept. 17, 2014, 6:46 a.m. UTC | #3 
> 

> > Reviewed-by: Hock Leong Kweh <hock.leong.kweh@intel.com>

> 

> You still keep that guy as reviewer in a whole series, however I didn't see a

> word from him here. How is it possible?

In our internal review, he gave me a lot of suggestions. 

> > +	for (i = 0; i < gpio->nr_ports; i++) {

> > +		unsigned int offset;

> > +		unsigned int idx = gpio->ports[i].idx;

> > +		struct dwapb_context *ctx = gpio->ports[i].ctx;

> > +

> > +		if (!ctx) {

> > +			ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);

> > +			gpio->ports[i].ctx = ctx;

> > +		}

> 

> I don't think it's a right place to allocate this resource, especially with devm_

> helper. Can you move this to probe() stage?

> 

> Or you even can embed contex structure inside chip one with #ifdef

> CONFIG_PM_SLEEP.

> 


OK, I will improve it.

> Maybe others could comment on this.

> 

> > +

> > +		offset = GPIO_SWPORTA_DDR + (idx * GPIO_SWPORT_DDR_SIZE);

> 

> No need to have parentheses here. Check the code below as well.

OK. I will remove them.
diff mbox

Patch

diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
index 9a76e3c..14d83af 100644
--- a/drivers/gpio/gpio-dwapb.c
+++ b/drivers/gpio/gpio-dwapb.c
@@ -50,11 +50,14 @@ 
 #define GPIO_SWPORT_DDR_SIZE	(GPIO_SWPORTB_DDR - GPIO_SWPORTA_DDR)
 
 struct dwapb_gpio;
+struct dwapb_context;
 
 struct dwapb_gpio_port {
 	struct bgpio_chip	bgc;
 	bool			is_registered;
 	struct dwapb_gpio	*gpio;
+	struct dwapb_context	*ctx;
+	unsigned int		idx;
 };
 
 struct dwapb_gpio {
@@ -377,6 +380,7 @@  static int dwapb_gpio_add_port(struct dwapb_gpio *gpio,
 
 	port = &gpio->ports[offs];
 	port->gpio = gpio;
+	port->idx = pp->idx;
 
 	dat = gpio->regs + GPIO_EXT_PORTA + (pp->idx * GPIO_EXT_PORT_SIZE);
 	set = gpio->regs + GPIO_SWPORTA_DR + (pp->idx * GPIO_SWPORT_DR_SIZE);
@@ -584,10 +588,115 @@  static const struct of_device_id dwapb_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, dwapb_of_match);
 
+#ifdef CONFIG_PM_SLEEP
+/* Store GPIO context across system-wide suspend/resume transitions */
+struct dwapb_context {
+	u32 data;
+	u32 dir;
+	u32 ext;
+	u32 int_en;
+	u32 int_mask;
+	u32 int_type;
+	u32 int_pol;
+	u32 int_deb;
+};
+
+static int dwapb_gpio_suspend(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct dwapb_gpio *gpio = platform_get_drvdata(pdev);
+	struct bgpio_chip *bgc	= &gpio->ports[0].bgc;
+	unsigned long flags;
+	int i;
+
+	spin_lock_irqsave(&bgc->lock, flags);
+	for (i = 0; i < gpio->nr_ports; i++) {
+		unsigned int offset;
+		unsigned int idx = gpio->ports[i].idx;
+		struct dwapb_context *ctx = gpio->ports[i].ctx;
+
+		if (!ctx) {
+			ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+			gpio->ports[i].ctx = ctx;
+		}
+
+		offset = GPIO_SWPORTA_DDR + (idx * GPIO_SWPORT_DDR_SIZE);
+		ctx->dir = dwapb_read(gpio, offset);
+
+		offset = GPIO_SWPORTA_DR + (idx * GPIO_SWPORT_DR_SIZE);
+		ctx->data = dwapb_read(gpio, offset);
+
+		offset = GPIO_EXT_PORTA + (idx * GPIO_EXT_PORT_SIZE);
+		ctx->ext = dwapb_read(gpio, offset);
+
+		/* Only port A can provide interrupts */
+		if (idx == 0) {
+			ctx->int_mask	= dwapb_read(gpio, GPIO_INTMASK);
+			ctx->int_en	= dwapb_read(gpio, GPIO_INTEN);
+			ctx->int_pol	= dwapb_read(gpio, GPIO_INT_POLARITY);
+			ctx->int_type	= dwapb_read(gpio, GPIO_INTTYPE_LEVEL);
+			ctx->int_deb	= dwapb_read(gpio, GPIO_PORTA_DEBOUNCE);
+
+			/* Mask out interrupts */
+			dwapb_write(gpio, GPIO_INTMASK, 0xffffffff);
+		}
+	}
+	spin_unlock_irqrestore(&bgc->lock, flags);
+
+	return 0;
+}
+
+static int dwapb_gpio_resume(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct dwapb_gpio *gpio = platform_get_drvdata(pdev);
+	struct bgpio_chip *bgc	= &gpio->ports[0].bgc;
+	unsigned long flags;
+	int i;
+
+	spin_lock_irqsave(&bgc->lock, flags);
+	for (i = 0; i < gpio->nr_ports; i++) {
+		unsigned int offset;
+		unsigned int idx = gpio->ports[i].idx;
+		struct dwapb_context *ctx = gpio->ports[i].ctx;
+
+		BUG_ON(ctx == 0);
+
+		offset = GPIO_SWPORTA_DR + (idx * GPIO_SWPORT_DR_SIZE);
+		dwapb_write(gpio, offset, ctx->data);
+
+		offset = GPIO_SWPORTA_DDR + (idx * GPIO_SWPORT_DDR_SIZE);
+		dwapb_write(gpio, offset, ctx->dir);
+
+		offset = GPIO_EXT_PORTA + (idx * GPIO_EXT_PORT_SIZE);
+		dwapb_write(gpio, offset, ctx->ext);
+
+		/* Only port A can provide interrupts */
+		if (idx == 0) {
+			dwapb_write(gpio, GPIO_INTTYPE_LEVEL, ctx->int_type);
+			dwapb_write(gpio, GPIO_INT_POLARITY, ctx->int_pol);
+			dwapb_write(gpio, GPIO_PORTA_DEBOUNCE, ctx->int_deb);
+			dwapb_write(gpio, GPIO_INTEN, ctx->int_en);
+			dwapb_write(gpio, GPIO_INTMASK, ctx->int_mask);
+
+			/* Clear out spurious interrupts */
+			dwapb_write(gpio, GPIO_PORTA_EOI, 0xffffffff);
+		}
+	}
+	spin_unlock_irqrestore(&bgc->lock, flags);
+
+	return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(dwapb_gpio_pm_ops, dwapb_gpio_suspend,
+			 dwapb_gpio_resume);
+
 static struct platform_driver dwapb_gpio_driver = {
 	.driver		= {
 		.name	= "gpio-dwapb",
 		.owner	= THIS_MODULE,
+		.pm	= &dwapb_gpio_pm_ops,
 		.of_match_table = of_match_ptr(dwapb_of_match),
 	},
 	.probe		= dwapb_gpio_probe,