| Message ID | 1455196399.18445.1.camel@ingics.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Feb 11, 2016 at 2:13 PM, Axel Lin <axel.lin@ingics.com> wrote: > For devtype with specific gpio_dir_out implementation, current code looks > wrong because below code sets both gc->direction_output and > mpc8xxx_gc->direction_output to the same function. > > gc->direction_output = devtype->gpio_dir_out ?: gc->direction_output; > mpc8xxx_gc->direction_output = gc->direction_output; > > Set mpc8xxx_gc->direction_output = gc->direction_output first to fix it. > This way mpc8xxx_gc->direction_output actually calls the standard > bgpio_dir_out() to update register. > > Setting gc->direction_output to gc->direction_output looks strange. > This patch also slightly changes the code for better readability. > > Fixes: commit 42178e2a1e42 ("drivers/gpio: Switch gpio-mpc8xxx to use gpio-generic") > Signed-off-by: Axel Lin <axel.lin@ingics.com> Liu please comment on this patch. Yours, Linus Walleij -- 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
SGksIEF4ZWwsDQoNClllcyB5b3UgYXJlIHJpZ2h0Lg0KDQpNYXliZSB3ZSBjYW4gbWFrZSB0aGUg Y2hhbmdlIGp1c3QgbGlrZSB0aGlzOg0KDQorCW1wYzh4eHhfZ2MtPmRpcmVjdGlvbl9vdXRwdXQg PSBnYy0+ZGlyZWN0aW9uX291dHB1dDsNCglnYy0+ZGlyZWN0aW9uX291dHB1dCA9IGRldnR5cGUt PmdwaW9fZGlyX291dCA/OiBnYy0+ZGlyZWN0aW9uX291dHB1dDsNCglnYy0+Z2V0ID0gZGV2dHlw ZS0+Z3Bpb19nZXQgPzogZ2MtPmdldDsNCglnYy0+dG9faXJxID0gbXBjOHh4eF9ncGlvX3RvX2ly cTsNCg0KLQltcGM4eHh4X2djLT5kaXJlY3Rpb25fb3V0cHV0ID0gZ2MtPmRpcmVjdGlvbl9vdXRw dXQ7DQoNCkJlc3QgUmVnYXJkcywNCkxpdSBHYW5nDQotLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0t LQ0KLS0tDQpIaSwNCkkgZG9uJ3QgaGF2ZSB0aGlzIGgvdywgcGxlYXNlIHJldmlldyBhbmQgdGVz dCBpZiB0aGlzIHBhdGNoIHdvcmtzLg0KVGhhbmssDQpBeGVsDQogZHJpdmVycy9ncGlvL2dwaW8t bXBjOHh4eC5jIHwgMTEgKysrKysrKy0tLS0NCiAxIGZpbGUgY2hhbmdlZCwgNyBpbnNlcnRpb25z KCspLCA0IGRlbGV0aW9ucygtKQ0KDQpkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncGlvL2dwaW8tbXBj OHh4eC5jIGIvZHJpdmVycy9ncGlvL2dwaW8tbXBjOHh4eC5jIGluZGV4IGVjZGIyN2EuLjBlZDI3 OWEgMTAwNjQ0DQotLS0gYS9kcml2ZXJzL2dwaW8vZ3Bpby1tcGM4eHh4LmMNCisrKyBiL2RyaXZl cnMvZ3Bpby9ncGlvLW1wYzh4eHguYw0KQEAgLTM0NCwxMSArMzQ0LDE0IEBAIHN0YXRpYyBpbnQg bXBjOHh4eF9wcm9iZShzdHJ1Y3QgcGxhdGZvcm1fZGV2aWNlICpwZGV2KQ0KIAkgKi8NCiAJbXBj OHh4eF9pcnFfY2hpcC5pcnFfc2V0X3R5cGUgPSBkZXZ0eXBlLT5pcnFfc2V0X3R5cGU7DQogDQot CWdjLT5kaXJlY3Rpb25fb3V0cHV0ID0gZGV2dHlwZS0+Z3Bpb19kaXJfb3V0ID86IGdjLT5kaXJl Y3Rpb25fb3V0cHV0Ow0KLQlnYy0+Z2V0ID0gZGV2dHlwZS0+Z3Bpb19nZXQgPzogZ2MtPmdldDsN Ci0JZ2MtPnRvX2lycSA9IG1wYzh4eHhfZ3Bpb190b19pcnE7DQotDQogCW1wYzh4eHhfZ2MtPmRp cmVjdGlvbl9vdXRwdXQgPSBnYy0+ZGlyZWN0aW9uX291dHB1dDsNCisJaWYgKGRldnR5cGUtPmdw aW9fZGlyX291dCkNCisJCWdjLT5kaXJlY3Rpb25fb3V0cHV0ID0gZGV2dHlwZS0+Z3Bpb19kaXJf b3V0Ow0KKw0KKwlpZiAoZGV2dHlwZS0+Z3Bpb19nZXQpDQorCQlnYy0+Z2V0ID0gZGV2dHlwZS0+ Z3Bpb19nZXQ7DQorDQorCWdjLT50b19pcnEgPSBtcGM4eHh4X2dwaW9fdG9faXJxOw0KIA0KIAly ZXQgPSBncGlvY2hpcF9hZGRfZGF0YShnYywgbXBjOHh4eF9nYyk7DQogCWlmIChyZXQpIHsNCi0t DQoyLjEuNA0KDQoNCg0K -- 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
diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c index ecdb27a..0ed279a 100644 --- a/drivers/gpio/gpio-mpc8xxx.c +++ b/drivers/gpio/gpio-mpc8xxx.c @@ -344,11 +344,14 @@ static int mpc8xxx_probe(struct platform_device *pdev) */ mpc8xxx_irq_chip.irq_set_type = devtype->irq_set_type; - gc->direction_output = devtype->gpio_dir_out ?: gc->direction_output; - gc->get = devtype->gpio_get ?: gc->get; - gc->to_irq = mpc8xxx_gpio_to_irq; - mpc8xxx_gc->direction_output = gc->direction_output; + if (devtype->gpio_dir_out) + gc->direction_output = devtype->gpio_dir_out; + + if (devtype->gpio_get) + gc->get = devtype->gpio_get; + + gc->to_irq = mpc8xxx_gpio_to_irq; ret = gpiochip_add_data(gc, mpc8xxx_gc); if (ret) {
For devtype with specific gpio_dir_out implementation, current code looks wrong because below code sets both gc->direction_output and mpc8xxx_gc->direction_output to the same function. gc->direction_output = devtype->gpio_dir_out ?: gc->direction_output; mpc8xxx_gc->direction_output = gc->direction_output; Set mpc8xxx_gc->direction_output = gc->direction_output first to fix it. This way mpc8xxx_gc->direction_output actually calls the standard bgpio_dir_out() to update register. Setting gc->direction_output to gc->direction_output looks strange. This patch also slightly changes the code for better readability. Fixes: commit 42178e2a1e42 ("drivers/gpio: Switch gpio-mpc8xxx to use gpio-generic") Signed-off-by: Axel Lin <axel.lin@ingics.com> --- Hi, I don't have this h/w, please review and test if this patch works. Thank, Axel drivers/gpio/gpio-mpc8xxx.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)