Skip to contentSkip to conversation

Use regmap+devm+DT in pm8xxx input drivers

PATCH Superseded Stephen Boyd 2 participants 19 messages Feb 26, 2014 – Feb 27, 2014

Subsystems (4)

v3 of 5 Superseded Latest is v5 →

lore
Share & tools
lore
#0 lore raw

v3 cover Use regmap+devm+DT in pm8xxx input drivers

These patches move the pm8xxx input drivers over to use devm_* APIs and regmap. This breaks the dependency of these drivers on the pm8xxx specific read/write calls and also simplifies the probe code a bit. Finally we add devicetree support to these drivers so they can be probed on the platforms that are supported upstream.

Changes since v2:
* Rebased to v3.14-rc3

Changes since v1:
* Picked up Dmitry's version of devm for pwrkey
* Added DT bindings and parsing patches
* Dropped patches picked up by Dmitry

Stephen Boyd (9):
Input: pmic8xxx-pwrkey - Migrate to regmap APIs
Input: pmic8xxx-keypad - Migrate to devm_* APIs
Input: pmic8xxx-keypad - Migrate to regmap APIs
Input: pmic8xxx-pwrkey - Migrate to DT
Input: pm8xxx-vibrator - Add DT match table
Input: pmic8xxx-keypad - Migrate to DT
devicetree: bindings: Document PM8921/8058 keypads
devicetree: bindings: Document PM8921/8058 power keys
devicetree: bindings: Document PM8921/8058 vibrators

 .../bindings/input/qcom,pm8xxx-keypad.txt          |  72 +++++
 .../bindings/input/qcom,pm8xxx-pwrkey.txt          |  39 +++
 .../devicetree/bindings/input/qcom,pm8xxx-vib.txt  |  16 ++
 drivers/input/keyboard/pmic8xxx-keypad.c           | 291 ++++++++++-----------
 drivers/input/misc/pm8xxx-vibrator.c               |   8 +
 drivers/input/misc/pmic8xxx-pwrkey.c               |  37 ++-
 include/linux/input/pmic8xxx-keypad.h              |  52 ----
 include/linux/input/pmic8xxx-pwrkey.h              |  31 ---
 8 files changed, 290 insertions(+), 256 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/qcom,pm8xxx-keypad.txt
 create mode 100644 Documentation/devicetree/bindings/input/qcom,pm8xxx-pwrkey.txt
 create mode 100644 Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.txt
 delete mode 100644 include/linux/input/pmic8xxx-keypad.h
 delete mode 100644 include/linux/input/pmic8xxx-pwrkey.h
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
#1 lore raw

PATCH v3 1/9 Input: pmic8xxx-pwrkey - Migrate to regmap APIs

Use the regmap APIs for this driver instead of custom pm8xxx APIs. This breaks this driver's dependency on the pm8xxx APIs and allows us to easily port it to other bus protocols in the future.

---
 drivers/input/misc/pmic8xxx-pwrkey.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/input/misc/pmic8xxx-pwrkey.c +4 -0
index 0e1a05f95858..cf6125dc3b51 100644
@@ -120,6 +120,10 @@ static int pmic8xxx_pwrkey_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	regmap = dev_get_regmap(pdev->dev.parent, NULL);
+	if (!regmap)
+		return -ENODEV;
+
 	input_set_capability(pwr, EV_KEY, KEY_POWER);
 
 	pwr->name = "pmic8xxx_pwrkey";
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
#2 lore raw

Re: [PATCH v3 1/9] Input: pmic8xxx-pwrkey - Migrate to regmap APIs

Quoted 14 lines · Stephen Boyd

Use the regmap APIs for this driver instead of custom pm8xxx APIs. This breaks this driver's dependency on the pm8xxx APIs and allows us to easily port it to other bus protocols in the future.

---
 drivers/input/misc/pmic8xxx-pwrkey.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/input/misc/pmic8xxx-pwrkey.c +0 -0
index 0e1a05f95858..cf6125dc3b51 100644

drivers/input/misc/pmic8xxx-pwrkey.c static int pmic8xxx_pwrkey_probe(struct platform_device *pdev) · line 126

Quoted 7 lines
drivers/input/misc/pmic8xxx-pwrkey.c +4 -0
@@ -120,6 +120,10 @@ static int pmic8xxx_pwrkey_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	regmap = dev_get_regmap(pdev->dev.parent, NULL);
+	if (!regmap)
+		return -ENODEV;
+

This looks really weird. A previous version of patch was included in v3.14-rc1 (1e63bd9cc4), which already included a very similar hunk...

Maybe the fact that the hunk isn't identical is why a rebase didn't drop it (and instead, now if this gets merged, we'll have unnecessary duplication).

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
#3 lore raw

Re: [PATCH v3 1/9] Input: pmic8xxx-pwrkey - Migrate to regmap APIs

Quoted 25 lines · Josh Cartwright
Quoted 14 lines · Stephen Boyd

Use the regmap APIs for this driver instead of custom pm8xxx APIs. This breaks this driver's dependency on the pm8xxx APIs and allows us to easily port it to other bus protocols in the future.

---
 drivers/input/misc/pmic8xxx-pwrkey.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/input/misc/pmic8xxx-pwrkey.c +0 -0
index 0e1a05f95858..cf6125dc3b51 100644

drivers/input/misc/pmic8xxx-pwrkey.c static int pmic8xxx_pwrkey_probe(struct platform_device *pdev) · line 126

Quoted 7 lines
drivers/input/misc/pmic8xxx-pwrkey.c +4 -0
@@ -120,6 +120,10 @@ static int pmic8xxx_pwrkey_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	regmap = dev_get_regmap(pdev->dev.parent, NULL);
+	if (!regmap)
+		return -ENODEV;
+

This looks really weird. A previous version of patch was included in v3.14-rc1 (1e63bd9cc4), which already included a very similar hunk...

Quoted 4 lines

Maybe the fact that the hunk isn't identical is why a rebase didn't drop it (and instead, now if this gets merged, we'll have unnecessary duplication).

Ah yeah, sorry about that. This patch should just be ignored. I don't think it affects anything else in the series?

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
#4 lore raw

Re: [PATCH v3 1/9] Input: pmic8xxx-pwrkey - Migrate to regmap APIs

Quoted 26 lines · Stephen Boyd
Quoted 25 lines · Josh Cartwright
Quoted 14 lines · Stephen Boyd

Use the regmap APIs for this driver instead of custom pm8xxx APIs. This breaks this driver's dependency on the pm8xxx APIs and allows us to easily port it to other bus protocols in the future.

---
 drivers/input/misc/pmic8xxx-pwrkey.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/input/misc/pmic8xxx-pwrkey.c +0 -0
index 0e1a05f95858..cf6125dc3b51 100644

drivers/input/misc/pmic8xxx-pwrkey.c static int pmic8xxx_pwrkey_probe(struct platform_device *pdev) · line 126

Quoted 7 lines
drivers/input/misc/pmic8xxx-pwrkey.c +4 -0
@@ -120,6 +120,10 @@ static int pmic8xxx_pwrkey_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	regmap = dev_get_regmap(pdev->dev.parent, NULL);
+	if (!regmap)
+		return -ENODEV;
+

This looks really weird. A previous version of patch was included in v3.14-rc1 (1e63bd9cc4), which already included a very similar hunk...

Quoted 7 lines
Quoted 4 lines

Maybe the fact that the hunk isn't identical is why a rebase didn't drop it (and instead, now if this gets merged, we'll have unnecessary duplication).

Ah yeah, sorry about that. This patch should just be ignored. I don't think it affects anything else in the series?

Yep, I think this can be safely ignored.

However, I think it needs to be made clear that your if "Modernize pm8921 with irqdomains, regmap, DT" lands before this patchset, this will start breaking randconfig builds. It isn't clear to me how this dependency should be handled.

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
#5 lore raw

Re: [PATCH v3 1/9] Input: pmic8xxx-pwrkey - Migrate to regmap APIs

Quoted 31 lines · Josh Cartwright
Quoted 30 lines · Stephen Boyd
Quoted 29 lines · Josh Cartwright
Quoted 14 lines · Stephen Boyd

Use the regmap APIs for this driver instead of custom pm8xxx APIs. This breaks this driver's dependency on the pm8xxx APIs and allows us to easily port it to other bus protocols in the future.

---
 drivers/input/misc/pmic8xxx-pwrkey.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/input/misc/pmic8xxx-pwrkey.c +0 -0
index 0e1a05f95858..cf6125dc3b51 100644

drivers/input/misc/pmic8xxx-pwrkey.c static int pmic8xxx_pwrkey_probe(struct platform_device *pdev) · line 126

Quoted 7 lines
drivers/input/misc/pmic8xxx-pwrkey.c +4 -0
@@ -120,6 +120,10 @@ static int pmic8xxx_pwrkey_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	regmap = dev_get_regmap(pdev->dev.parent, NULL);
+	if (!regmap)
+		return -ENODEV;
+

This looks really weird. A previous version of patch was included in v3.14-rc1 (1e63bd9cc4), which already included a very similar hunk...

Maybe the fact that the hunk isn't identical is why a rebase didn't drop it (and instead, now if this gets merged, we'll have unnecessary duplication).

Quoted 9 lines
Quoted 2 lines

Ah yeah, sorry about that. This patch should just be ignored. I don't think it affects anything else in the series?

Yep, I think this can be safely ignored.

However, I think it needs to be made clear that your if "Modernize pm8921 with irqdomains, regmap, DT" lands before this patchset, this will start breaking randconfig builds. It isn't clear to me how this dependency should be handled.

Hmm? If pm8921 lands before this what build breakage is there? The pm8xxx_read/write APIs are still exposed.

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
#6 lore raw

Re: [PATCH v3 1/9] Input: pmic8xxx-pwrkey - Migrate to regmap APIs

Quoted 11 lines · Stephen Boyd
Quoted 7 lines · Josh Cartwright
Quoted 1 line · Stephen Boyd

On 02/26/14 15:59, Josh Cartwright wrote:

However, I think it needs to be made clear that your if "Modernize pm8921 with irqdomains, regmap, DT" lands before this patchset, this will start breaking randconfig builds. It isn't clear to me how this dependency should be handled.

Hmm? If pm8921 lands before this what build breakage is there? The pm8xxx_read/write APIs are still exposed.

Well, with the pm8921 patchset in place, the keypad driver becomes selectable, which, when built leads to the error below. But AFAICT, this isn't even addressed in this patchset?!

drivers/input/keyboard/pmic8xxx-keypad.c:24:35: fatal error: linux/mfd/pm8xxx/gpio.h: No such file or directory

 #include <linux/mfd/pm8xxx/gpio.h>
                                   ^

So, I'm confused...maybe the pm8921 cleanup patchset should be marking the keypad driver BROKEN?

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
#7 lore raw

Re: [PATCH v3 1/9] Input: pmic8xxx-pwrkey - Migrate to regmap APIs

Quoted 15 lines · Josh Cartwright
Quoted 10 lines · Stephen Boyd
Quoted 7 lines · Josh Cartwright
Quoted 1 line · Stephen Boyd

On 02/26/14 15:59, Josh Cartwright wrote:

However, I think it needs to be made clear that your if "Modernize pm8921 with irqdomains, regmap, DT" lands before this patchset, this will start breaking randconfig builds. It isn't clear to me how this dependency should be handled.

Hmm? If pm8921 lands before this what build breakage is there? The pm8xxx_read/write APIs are still exposed.

Well, with the pm8921 patchset in place, the keypad driver becomes selectable, which, when built leads to the error below. But AFAICT, this isn't even addressed in this patchset?!

Quoted 7 lines

drivers/input/keyboard/pmic8xxx-keypad.c:24:35: fatal error: linux/mfd/pm8xxx/gpio.h: No such file or directory

 #include <linux/mfd/pm8xxx/gpio.h>
                                   ^

So, I'm confused...maybe the pm8921 cleanup patchset should be marking the keypad driver BROKEN?

Ah, yes probably. I have a patch locally that rips out the gpio stuff because this driver has been uncompilable since it was introduced. I may as well send that along with this series so that people can actually compile this driver. Or like you say, send it along with the removal of the BROKEN in pm8921 Kconfig.

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
#8 lore raw

Re: [PATCH v3 1/9] Input: pmic8xxx-pwrkey - Migrate to regmap APIs

Quoted 24 lines · Stephen Boyd
Quoted 15 lines · Josh Cartwright
Quoted 10 lines · Stephen Boyd
Quoted 7 lines · Josh Cartwright
Quoted 1 line · Stephen Boyd

On 02/26/14 15:59, Josh Cartwright wrote:

However, I think it needs to be made clear that your if "Modernize pm8921 with irqdomains, regmap, DT" lands before this patchset, this will start breaking randconfig builds. It isn't clear to me how this dependency should be handled.

Hmm? If pm8921 lands before this what build breakage is there? The pm8xxx_read/write APIs are still exposed.

Well, with the pm8921 patchset in place, the keypad driver becomes selectable, which, when built leads to the error below. But AFAICT, this isn't even addressed in this patchset?!

Quoted 7 lines

drivers/input/keyboard/pmic8xxx-keypad.c:24:35: fatal error: linux/mfd/pm8xxx/gpio.h: No such file or directory

 #include <linux/mfd/pm8xxx/gpio.h>
                                   ^

So, I'm confused...maybe the pm8921 cleanup patchset should be marking the keypad driver BROKEN?

Quoted 5 lines

Ah, yes probably. I have a patch locally that rips out the gpio stuff because this driver has been uncompilable since it was introduced. I may as well send that along with this series so that people can actually compile this driver. Or like you say, send it along with the removal of the BROKEN in pm8921 Kconfig.

Yeah, I think it should probably land before MFD_PM8XXX's BROKEN removal, just to eliminate randconfig breakage noise. I double-checked, and it looks like at least the other drivers that depend on MFD_PM8XXX still build (with a couple sparse warnings).

Also.....it's driving me nuts that some of the pm8xxx drivers use "pm8xxx-foo", and others use "pmic8xxx-foo"...

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
#9 lore raw

PATCH v3 2/9 Input: pmic8xxx-keypad - Migrate to devm_* APIs

In reply to Stephen Boyd

Simplify the error paths and reduce the lines of code in this driver by using the devm_* APIs.

---
 drivers/input/keyboard/pmic8xxx-keypad.c | 62 +++++++++-----------------------
 1 file changed, 17 insertions(+), 45 deletions(-)
drivers/input/keyboard/pmic8xxx-keypad.c +17 -45
index 2c9f19ac35ea..4e6bfbf94ae4 100644
@@ -586,7 +586,7 @@ static int pmic8xxx_kp_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	kp = kzalloc(sizeof(*kp), GFP_KERNEL);
+	kp = devm_kzalloc(&pdev->dev, sizeof(*kp), GFP_KERNEL);
 	if (!kp)
 		return -ENOMEM;
 
@@ -595,32 +595,27 @@ static int pmic8xxx_kp_probe(struct platform_device *pdev)
 	kp->pdata	= pdata;
 	kp->dev		= &pdev->dev;
 
-	kp->input = input_allocate_device();
+	kp->input = devm_input_allocate_device(&pdev->dev);
 	if (!kp->input) {
 		dev_err(&pdev->dev, "unable to allocate input device\n");
-		rc = -ENOMEM;
-		goto err_alloc_device;
+		return -ENOMEM;
 	}
 
 	kp->key_sense_irq = platform_get_irq(pdev, 0);
 	if (kp->key_sense_irq < 0) {
 		dev_err(&pdev->dev, "unable to get keypad sense irq\n");
-		rc = -ENXIO;
-		goto err_get_irq;
+		return kp->key_sense_irq;
 	}
 
 	kp->key_stuck_irq = platform_get_irq(pdev, 1);
 	if (kp->key_stuck_irq < 0) {
 		dev_err(&pdev->dev, "unable to get keypad stuck irq\n");
-		rc = -ENXIO;
-		goto err_get_irq;
+		return kp->key_stuck_irq;
 	}
 
 	kp->input->name = pdata->input_name ? : "PMIC8XXX keypad";
 	kp->input->phys = pdata->input_phys_device ? : "pmic8xxx_keypad/input0";
 
-	kp->input->dev.parent	= &pdev->dev;
-
 	kp->input->id.bustype	= BUS_I2C;
 	kp->input->id.version	= 0x0001;
 	kp->input->id.product	= 0x0001;
@@ -634,7 +629,7 @@ static int pmic8xxx_kp_probe(struct platform_device *pdev)
 					kp->keycodes, kp->input);
 	if (rc) {
 		dev_err(&pdev->dev, "failed to build keymap\n");
-		goto err_get_irq;
+		return rc;
 	}
 
 	if (pdata->rep)
@@ -650,7 +645,7 @@ static int pmic8xxx_kp_probe(struct platform_device *pdev)
 	rc = pmic8xxx_kpd_init(kp);
 	if (rc < 0) {
 		dev_err(&pdev->dev, "unable to initialize keypad controller\n");
-		goto err_get_irq;
+		return rc;
 	}
 
 	rc = pmic8xxx_kp_config_gpio(pdata->cols_gpio_start,
@@ -667,24 +662,26 @@ static int pmic8xxx_kp_probe(struct platform_device *pdev)
 		goto err_gpio_config;
 	}
 
-	rc = request_any_context_irq(kp->key_sense_irq, pmic8xxx_kp_irq,
-				 IRQF_TRIGGER_RISING, "pmic-keypad", kp);
+	rc = devm_request_any_context_irq(&pdev->dev, kp->key_sense_irq,
+			pmic8xxx_kp_irq, IRQF_TRIGGER_RISING, "pmic-keypad",
+			kp);
 	if (rc < 0) {
 		dev_err(&pdev->dev, "failed to request keypad sense irq\n");
-		goto err_get_irq;
+		return rc;
 	}
 
-	rc = request_any_context_irq(kp->key_stuck_irq, pmic8xxx_kp_stuck_irq,
-				 IRQF_TRIGGER_RISING, "pmic-keypad-stuck", kp);
+	rc = devm_request_any_context_irq(&pdev->dev, kp->key_stuck_irq,
+			pmic8xxx_kp_stuck_irq, IRQF_TRIGGER_RISING,
+			"pmic-keypad-stuck", kp);
 	if (rc < 0) {
 		dev_err(&pdev->dev, "failed to request keypad stuck irq\n");
-		goto err_req_stuck_irq;
+		return rc;
 	}
 
 	rc = pmic8xxx_kp_read_u8(kp, &ctrl_val, KEYP_CTRL);
 	if (rc < 0) {
 		dev_err(&pdev->dev, "failed to read KEYP_CTRL register\n");
-		goto err_pmic_reg_read;
+		return rc;
 	}
 
 	kp->ctrl_reg = ctrl_val;
@@ -692,36 +689,12 @@ static int pmic8xxx_kp_probe(struct platform_device *pdev)
 	rc = input_register_device(kp->input);
 	if (rc < 0) {
 		dev_err(&pdev->dev, "unable to register keypad input device\n");
-		goto err_pmic_reg_read;
+		return rc;
 	}
 
 	device_init_wakeup(&pdev->dev, pdata->wakeup);
 
 	return 0;
-
-err_pmic_reg_read:
-	free_irq(kp->key_stuck_irq, kp);
-err_req_stuck_irq:
-	free_irq(kp->key_sense_irq, kp);
-err_gpio_config:
-err_get_irq:
-	input_free_device(kp->input);
-err_alloc_device:
-	kfree(kp);
-	return rc;
-}
-
-static int pmic8xxx_kp_remove(struct platform_device *pdev)
-{
-	struct pmic8xxx_kp *kp = platform_get_drvdata(pdev);
-
-	device_init_wakeup(&pdev->dev, 0);
-	free_irq(kp->key_stuck_irq, kp);
-	free_irq(kp->key_sense_irq, kp);
-	input_unregister_device(kp->input);
-	kfree(kp);
-
-	return 0;
 }
 
 #ifdef CONFIG_PM_SLEEP
@@ -771,7 +744,6 @@ static SIMPLE_DEV_PM_OPS(pm8xxx_kp_pm_ops,
 
 static struct platform_driver pmic8xxx_kp_driver = {
 	.probe		= pmic8xxx_kp_probe,
-	.remove		= pmic8xxx_kp_remove,
 	.driver		= {
 		.name = PM8XXX_KEYPAD_DEV_NAME,
 		.owner = THIS_MODULE,
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
#10 lore raw

Re: [PATCH v3 2/9] Input: pmic8xxx-keypad - Migrate to devm_* APIs

Quoted 12 lines · Stephen Boyd

Simplify the error paths and reduce the lines of code in this driver by using the devm_* APIs.

---
 drivers/input/keyboard/pmic8xxx-keypad.c | 62 +++++++++-----------------------
 1 file changed, 17 insertions(+), 45 deletions(-)
drivers/input/keyboard/pmic8xxx-keypad.c +0 -0
index 2c9f19ac35ea..4e6bfbf94ae4 100644

[..]

Quoted 19 lines
drivers/input/keyboard/pmic8xxx-keypad.c +2 -2
@@ -634,7 +629,7 @@ static int pmic8xxx_kp_probe(struct platform_device *pdev)
 					kp->keycodes, kp->input);
 	if (rc) {
 		dev_err(&pdev->dev, "failed to build keymap\n");
-		goto err_get_irq;
+		return rc;
 	}
 
 	if (pdata->rep)
@@ -650,7 +645,7 @@ static int pmic8xxx_kp_probe(struct platform_device *pdev)
 	rc = pmic8xxx_kpd_init(kp);
 	if (rc < 0) {
 		dev_err(&pdev->dev, "unable to initialize keypad controller\n");
-		goto err_get_irq;
+		return rc;
 	}
 
 	rc = pmic8xxx_kp_config_gpio(pdata->cols_gpio_start,

drivers/input/keyboard/pmic8xxx-keypad.c static int pmic8xxx_kp_probe(struct platform_device *pdev) · line 662

Quoted 1 line
drivers/input/keyboard/pmic8xxx-keypad.c +0 -0
@@ -667,24 +662,26 @@ static int pmic8xxx_kp_probe(struct platform_device *pdev)
 		goto err_gpio_config;

See below.

Quoted 39 lines
drivers/input/keyboard/pmic8xxx-keypad.c +10 -8
 	}
 
-	rc = request_any_context_irq(kp->key_sense_irq, pmic8xxx_kp_irq,
-				 IRQF_TRIGGER_RISING, "pmic-keypad", kp);
+	rc = devm_request_any_context_irq(&pdev->dev, kp->key_sense_irq,
+			pmic8xxx_kp_irq, IRQF_TRIGGER_RISING, "pmic-keypad",
+			kp);
 	if (rc < 0) {
 		dev_err(&pdev->dev, "failed to request keypad sense irq\n");
-		goto err_get_irq;
+		return rc;
 	}
 
-	rc = request_any_context_irq(kp->key_stuck_irq, pmic8xxx_kp_stuck_irq,
-				 IRQF_TRIGGER_RISING, "pmic-keypad-stuck", kp);
+	rc = devm_request_any_context_irq(&pdev->dev, kp->key_stuck_irq,
+			pmic8xxx_kp_stuck_irq, IRQF_TRIGGER_RISING,
+			"pmic-keypad-stuck", kp);
 	if (rc < 0) {
 		dev_err(&pdev->dev, "failed to request keypad stuck irq\n");
-		goto err_req_stuck_irq;
+		return rc;
 	}
 
 	rc = pmic8xxx_kp_read_u8(kp, &ctrl_val, KEYP_CTRL);
 	if (rc < 0) {
 		dev_err(&pdev->dev, "failed to read KEYP_CTRL register\n");
-		goto err_pmic_reg_read;
+		return rc;
 	}
 
 	kp->ctrl_reg = ctrl_val;
@@ -692,36 +689,12 @@ static int pmic8xxx_kp_probe(struct platform_device *pdev)
 	rc = input_register_device(kp->input);
 	if (rc < 0) {
 		dev_err(&pdev->dev, "unable to register keypad input device\n");
-		goto err_pmic_reg_read;
+		return rc;
 	}

drivers/input/keyboard/pmic8xxx-keypad.c static int pmic8xxx_kp_probe(struct platform_device *pdev) · line 706

Quoted 10 lines
drivers/input/keyboard/pmic8xxx-keypad.c +0 -6
 
 	device_init_wakeup(&pdev->dev, pdata->wakeup);
 
 	return 0;
-
-err_pmic_reg_read:
-	free_irq(kp->key_stuck_irq, kp);
-err_req_stuck_irq:
-	free_irq(kp->key_sense_irq, kp);
-err_gpio_config:

You're removing this label, even though it's still used above. :(

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
#11 lore raw

Re: [PATCH v3 2/9] Input: pmic8xxx-keypad - Migrate to devm_* APIs

Quoted 78 lines · Josh Cartwright
Quoted 12 lines · Stephen Boyd

Simplify the error paths and reduce the lines of code in this driver by using the devm_* APIs.

---
 drivers/input/keyboard/pmic8xxx-keypad.c | 62 +++++++++-----------------------
 1 file changed, 17 insertions(+), 45 deletions(-)
drivers/input/keyboard/pmic8xxx-keypad.c +0 -0
index 2c9f19ac35ea..4e6bfbf94ae4 100644

[..]

Quoted 19 lines
drivers/input/keyboard/pmic8xxx-keypad.c +2 -2
@@ -634,7 +629,7 @@ static int pmic8xxx_kp_probe(struct platform_device *pdev)
 					kp->keycodes, kp->input);
 	if (rc) {
 		dev_err(&pdev->dev, "failed to build keymap\n");
-		goto err_get_irq;
+		return rc;
 	}
 
 	if (pdata->rep)
@@ -650,7 +645,7 @@ static int pmic8xxx_kp_probe(struct platform_device *pdev)
 	rc = pmic8xxx_kpd_init(kp);
 	if (rc < 0) {
 		dev_err(&pdev->dev, "unable to initialize keypad controller\n");
-		goto err_get_irq;
+		return rc;
 	}
 
 	rc = pmic8xxx_kp_config_gpio(pdata->cols_gpio_start,

drivers/input/keyboard/pmic8xxx-keypad.c static int pmic8xxx_kp_probe(struct platform_device *pdev) · line 662

Quoted 1 line
drivers/input/keyboard/pmic8xxx-keypad.c +0 -0
@@ -667,24 +662,26 @@ static int pmic8xxx_kp_probe(struct platform_device *pdev)
 		goto err_gpio_config;

See below.

Quoted 42 lines
drivers/input/keyboard/pmic8xxx-keypad.c +10 -8
 	}
 
-	rc = request_any_context_irq(kp->key_sense_irq, pmic8xxx_kp_irq,
-				 IRQF_TRIGGER_RISING, "pmic-keypad", kp);
+	rc = devm_request_any_context_irq(&pdev->dev, kp->key_sense_irq,
+			pmic8xxx_kp_irq, IRQF_TRIGGER_RISING, "pmic-keypad",
+			kp);
 	if (rc < 0) {
 		dev_err(&pdev->dev, "failed to request keypad sense irq\n");
-		goto err_get_irq;
+		return rc;
 	}
 
-	rc = request_any_context_irq(kp->key_stuck_irq, pmic8xxx_kp_stuck_irq,
-				 IRQF_TRIGGER_RISING, "pmic-keypad-stuck", kp);
+	rc = devm_request_any_context_irq(&pdev->dev, kp->key_stuck_irq,
+			pmic8xxx_kp_stuck_irq, IRQF_TRIGGER_RISING,
+			"pmic-keypad-stuck", kp);
 	if (rc < 0) {
 		dev_err(&pdev->dev, "failed to request keypad stuck irq\n");
-		goto err_req_stuck_irq;
+		return rc;
 	}
 
 	rc = pmic8xxx_kp_read_u8(kp, &ctrl_val, KEYP_CTRL);
 	if (rc < 0) {
 		dev_err(&pdev->dev, "failed to read KEYP_CTRL register\n");
-		goto err_pmic_reg_read;
+		return rc;
 	}
 
 	kp->ctrl_reg = ctrl_val;
@@ -692,36 +689,12 @@ static int pmic8xxx_kp_probe(struct platform_device *pdev)
 	rc = input_register_device(kp->input);
 	if (rc < 0) {
 		dev_err(&pdev->dev, "unable to register keypad input device\n");
-		goto err_pmic_reg_read;
+		return rc;
 	}
 
 	device_init_wakeup(&pdev->dev, pdata->wakeup);
 
Quoted 9 lines

drivers/input/keyboard/pmic8xxx-keypad.c static int pmic8xxx_kp_probe(struct platform_device *pdev) · line 706

Quoted 7 lines
drivers/input/keyboard/pmic8xxx-keypad.c +0 -6
 	return 0;
-
-err_pmic_reg_read:
-	free_irq(kp->key_stuck_irq, kp);
-err_req_stuck_irq:
-	free_irq(kp->key_sense_irq, kp);
-err_gpio_config:

You're removing this label, even though it's still used above. :(

Ah, thanks. Will fix.

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
#12 lore raw

PATCH v3 3/9 Input: pmic8xxx-keypad - Migrate to regmap APIs

In reply to Stephen Boyd

Use the regmap APIs for this driver instead of custom pm8xxx APIs. This breaks this driver's dependency on the pm8xxx APIs and allows us to easily port it to other bus protocols in the future.

---
 drivers/input/keyboard/pmic8xxx-keypad.c | 81 ++++++++++++--------------------
 1 file changed, 29 insertions(+), 52 deletions(-)
drivers/input/keyboard/pmic8xxx-keypad.c +29 -52
index 4e6bfbf94ae4..c6d3d216ffa7 100644
@@ -19,8 +19,8 @@
 #include <linux/bitops.h>
 #include <linux/delay.h>
 #include <linux/mutex.h>
+#include <linux/regmap.h>
 
-#include <linux/mfd/pm8xxx/core.h>
 #include <linux/mfd/pm8xxx/gpio.h>
 #include <linux/input/pmic8xxx-keypad.h>
 
@@ -87,6 +87,7 @@
  * struct pmic8xxx_kp - internal keypad data structure
  * @pdata - keypad platform data pointer
  * @input - input device pointer for keypad
+ * @regmap - regmap handle
  * @key_sense_irq - key press/release irq number
  * @key_stuck_irq - key stuck notification irq number
  * @keycodes - array to hold the key codes
@@ -98,6 +99,7 @@
 struct pmic8xxx_kp {
 	const struct pm8xxx_keypad_platform_data *pdata;
 	struct input_dev *input;
+	struct regmap *regmap;
 	int key_sense_irq;
 	int key_stuck_irq;
 
@@ -110,33 +112,6 @@ struct pmic8xxx_kp {
 	u8 ctrl_reg;
 };
 
-static int pmic8xxx_kp_write_u8(struct pmic8xxx_kp *kp,
-				 u8 data, u16 reg)
-{
-	int rc;
-
-	rc = pm8xxx_writeb(kp->dev->parent, reg, data);
-	return rc;
-}
-
-static int pmic8xxx_kp_read(struct pmic8xxx_kp *kp,
-				 u8 *data, u16 reg, unsigned num_bytes)
-{
-	int rc;
-
-	rc = pm8xxx_read_buf(kp->dev->parent, reg, data, num_bytes);
-	return rc;
-}
-
-static int pmic8xxx_kp_read_u8(struct pmic8xxx_kp *kp,
-				 u8 *data, u16 reg)
-{
-	int rc;
-
-	rc = pmic8xxx_kp_read(kp, data, reg, 1);
-	return rc;
-}
-
 static u8 pmic8xxx_col_state(struct pmic8xxx_kp *kp, u8 col)
 {
 	/* all keys pressed on that particular row? */
@@ -161,9 +136,9 @@ static u8 pmic8xxx_col_state(struct pmic8xxx_kp *kp, u8 col)
 static int pmic8xxx_chk_sync_read(struct pmic8xxx_kp *kp)
 {
 	int rc;
-	u8 scan_val;
+	unsigned int scan_val;
 
-	rc = pmic8xxx_kp_read_u8(kp, &scan_val, KEYP_SCAN);
+	rc = regmap_read(kp->regmap, KEYP_SCAN, &scan_val);
 	if (rc < 0) {
 		dev_err(kp->dev, "Error reading KEYP_SCAN reg, rc=%d\n", rc);
 		return rc;
@@ -171,7 +146,7 @@ static int pmic8xxx_chk_sync_read(struct pmic8xxx_kp *kp)
 
 	scan_val |= 0x1;
 
-	rc = pmic8xxx_kp_write_u8(kp, scan_val, KEYP_SCAN);
+	rc = regmap_write(kp->regmap, KEYP_SCAN, scan_val);
 	if (rc < 0) {
 		dev_err(kp->dev, "Error writing KEYP_SCAN reg, rc=%d\n", rc);
 		return rc;
@@ -187,26 +162,24 @@ static int pmic8xxx_kp_read_data(struct pmic8xxx_kp *kp, u16 *state,
 					u16 data_reg, int read_rows)
 {
 	int rc, row;
-	u8 new_data[PM8XXX_MAX_ROWS];
+	unsigned int val;
 
-	rc = pmic8xxx_kp_read(kp, new_data, data_reg, read_rows);
-	if (rc)
-		return rc;
-
-	for (row = 0; row < kp->pdata->num_rows; row++) {
-		dev_dbg(kp->dev, "new_data[%d] = %d\n", row,
-					new_data[row]);
-		state[row] = pmic8xxx_col_state(kp, new_data[row]);
+	for (row = 0; row < read_rows; row++) {
+		rc = regmap_read(kp->regmap, data_reg, &val);
+		if (rc)
+			return rc;
+		dev_dbg(kp->dev, "%d = %d\n", row, val);
+		state[row] = pmic8xxx_col_state(kp, val);
 	}
 
-	return rc;
+	return 0;
 }
 
 static int pmic8xxx_kp_read_matrix(struct pmic8xxx_kp *kp, u16 *new_state,
 					 u16 *old_state)
 {
 	int rc, read_rows;
-	u8 scan_val;
+	unsigned int scan_val;
 
 	if (kp->pdata->num_rows < PM8XXX_MIN_ROWS)
 		read_rows = PM8XXX_MIN_ROWS;
@@ -236,14 +209,14 @@ static int pmic8xxx_kp_read_matrix(struct pmic8xxx_kp *kp, u16 *new_state,
 	/* 4 * 32KHz clocks */
 	udelay((4 * DIV_ROUND_UP(USEC_PER_SEC, KEYP_CLOCK_FREQ)) + 1);
 
-	rc = pmic8xxx_kp_read_u8(kp, &scan_val, KEYP_SCAN);
+	rc = regmap_read(kp->regmap, KEYP_SCAN, &scan_val);
 	if (rc < 0) {
 		dev_err(kp->dev, "Error reading KEYP_SCAN reg, rc=%d\n", rc);
 		return rc;
 	}
 
 	scan_val &= 0xFE;
-	rc = pmic8xxx_kp_write_u8(kp, scan_val, KEYP_SCAN);
+	rc = regmap_write(kp->regmap, KEYP_SCAN, scan_val);
 	if (rc < 0)
 		dev_err(kp->dev, "Error writing KEYP_SCAN reg, rc=%d\n", rc);
 
@@ -379,10 +352,10 @@ static irqreturn_t pmic8xxx_kp_stuck_irq(int irq, void *data)
 static irqreturn_t pmic8xxx_kp_irq(int irq, void *data)
 {
 	struct pmic8xxx_kp *kp = data;
-	u8 ctrl_val, events;
+	unsigned int ctrl_val, events;
 	int rc;
 
-	rc = pmic8xxx_kp_read(kp, &ctrl_val, KEYP_CTRL, 1);
+	rc = regmap_read(kp->regmap, KEYP_CTRL, &ctrl_val);
 	if (rc < 0) {
 		dev_err(kp->dev, "failed to read keyp_ctrl register\n");
 		return IRQ_HANDLED;
@@ -421,7 +394,7 @@ static int pmic8xxx_kpd_init(struct pmic8xxx_kp *kp)
 
 	ctrl_val |= (bits << KEYP_CTRL_SCAN_ROWS_SHIFT);
 
-	rc = pmic8xxx_kp_write_u8(kp, ctrl_val, KEYP_CTRL);
+	rc = regmap_write(kp->regmap, KEYP_CTRL, ctrl_val);
 	if (rc < 0) {
 		dev_err(kp->dev, "Error writing KEYP_CTRL reg, rc=%d\n", rc);
 		return rc;
@@ -439,7 +412,7 @@ static int pmic8xxx_kpd_init(struct pmic8xxx_kp *kp)
 
 	scan_val |= (cycles << KEYP_SCAN_ROW_HOLD_SHIFT);
 
-	rc = pmic8xxx_kp_write_u8(kp, scan_val, KEYP_SCAN);
+	rc = regmap_write(kp->regmap, KEYP_SCAN, scan_val);
 	if (rc)
 		dev_err(kp->dev, "Error writing KEYP_SCAN reg, rc=%d\n", rc);
 
@@ -474,7 +447,7 @@ static int pmic8xxx_kp_enable(struct pmic8xxx_kp *kp)
 
 	kp->ctrl_reg |= KEYP_CTRL_KEYP_EN;
 
-	rc = pmic8xxx_kp_write_u8(kp, kp->ctrl_reg, KEYP_CTRL);
+	rc = regmap_write(kp->regmap, KEYP_CTRL, kp->ctrl_reg);
 	if (rc < 0)
 		dev_err(kp->dev, "Error writing KEYP_CTRL reg, rc=%d\n", rc);
 
@@ -487,7 +460,7 @@ static int pmic8xxx_kp_disable(struct pmic8xxx_kp *kp)
 
 	kp->ctrl_reg &= ~KEYP_CTRL_KEYP_EN;
 
-	rc = pmic8xxx_kp_write_u8(kp, kp->ctrl_reg, KEYP_CTRL);
+	rc = regmap_write(kp->regmap, KEYP_CTRL, kp->ctrl_reg);
 	if (rc < 0)
 		return rc;
 
@@ -525,7 +498,7 @@ static int pmic8xxx_kp_probe(struct platform_device *pdev)
 	const struct matrix_keymap_data *keymap_data;
 	struct pmic8xxx_kp *kp;
 	int rc;
-	u8 ctrl_val;
+	unsigned int ctrl_val;
 
 	struct pm_gpio kypd_drv = {
 		.direction	= PM_GPIO_DIR_OUT,
@@ -590,6 +563,10 @@ static int pmic8xxx_kp_probe(struct platform_device *pdev)
 	if (!kp)
 		return -ENOMEM;
 
+	kp->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+	if (!kp->regmap)
+		return -ENODEV;
+
 	platform_set_drvdata(pdev, kp);
 
 	kp->pdata	= pdata;
@@ -678,7 +655,7 @@ static int pmic8xxx_kp_probe(struct platform_device *pdev)
 		return rc;
 	}
 
-	rc = pmic8xxx_kp_read_u8(kp, &ctrl_val, KEYP_CTRL);
+	rc = regmap_read(kp->regmap, KEYP_CTRL, &ctrl_val);
 	if (rc < 0) {
 		dev_err(&pdev->dev, "failed to read KEYP_CTRL register\n");
 		return rc;
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
#13 lore raw

PATCH v3 4/9 Input: pmic8xxx-pwrkey - Migrate to DT

In reply to Stephen Boyd

The driver is only supported on DT enabled platforms. Convert the driver to DT so that it can probe properly.

---
 drivers/input/misc/pmic8xxx-pwrkey.c  | 33 ++++++++++++++++++++-------------
 include/linux/input/pmic8xxx-pwrkey.h | 31 -------------------------------
 2 files changed, 20 insertions(+), 44 deletions(-)
 delete mode 100644 include/linux/input/pmic8xxx-pwrkey.h
drivers/input/misc/pmic8xxx-pwrkey.c +20 -13
index cf6125dc3b51..1573ad3d25ec 100644
@@ -19,8 +19,7 @@
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 #include <linux/log2.h>
-
-#include <linux/input/pmic8xxx-pwrkey.h>
+#include <linux/of.h>
 
 #define PON_CNTL_1 0x1C
 #define PON_CNTL_PULL_UP BIT(7)
@@ -79,6 +78,13 @@ static int pmic8xxx_pwrkey_resume(struct device *dev)
 static SIMPLE_DEV_PM_OPS(pm8xxx_pwr_key_pm_ops,
 		pmic8xxx_pwrkey_suspend, pmic8xxx_pwrkey_resume);
 
+static const struct of_device_id pm8xxx_pwr_key_id_table[] = {
+	{ .compatible = "qcom,pm8058-pwrkey" },
+	{ .compatible = "qcom,pm8921-pwrkey" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, pm8xxx_pwr_key_id_table);
+
 static int pmic8xxx_pwrkey_probe(struct platform_device *pdev)
 {
 	struct input_dev *pwr;
@@ -89,15 +95,15 @@ static int pmic8xxx_pwrkey_probe(struct platform_device *pdev)
 	unsigned int pon_cntl;
 	struct regmap *regmap;
 	struct pmic8xxx_pwrkey *pwrkey;
-	const struct pm8xxx_pwrkey_platform_data *pdata =
-					dev_get_platdata(&pdev->dev);
+	u32 kpd_delay;
+	bool pull_up;
 
-	if (!pdata) {
-		dev_err(&pdev->dev, "power key platform data not supplied\n");
-		return -EINVAL;
-	}
+	if (of_property_read_u32(pdev->dev.of_node, "debounce", &kpd_delay))
+		kpd_delay = 0;
+
+	pull_up = of_property_read_bool(pdev->dev.of_node, "pull-up");
 
-	if (pdata->kpd_trigger_delay_us > 62500) {
+	if (kpd_delay > 62500) {
 		dev_err(&pdev->dev, "invalid power key trigger delay\n");
 		return -EINVAL;
 	}
@@ -129,7 +135,7 @@ static int pmic8xxx_pwrkey_probe(struct platform_device *pdev)
 	pwr->name = "pmic8xxx_pwrkey";
 	pwr->phys = "pmic8xxx_pwrkey/input0";
 
-	delay = (pdata->kpd_trigger_delay_us << 10) / USEC_PER_SEC;
+	delay = (kpd_delay << 10) / USEC_PER_SEC;
 	delay = 1 + ilog2(delay);
 
 	err = regmap_read(regmap, PON_CNTL_1, &pon_cntl);
@@ -140,7 +146,7 @@ static int pmic8xxx_pwrkey_probe(struct platform_device *pdev)
 
 	pon_cntl &= ~PON_CNTL_TRIG_DELAY_MASK;
 	pon_cntl |= (delay & PON_CNTL_TRIG_DELAY_MASK);
-	if (pdata->pull_up)
+	if (pull_up)
 		pon_cntl |= PON_CNTL_PULL_UP;
 	else
 		pon_cntl &= ~PON_CNTL_PULL_UP;
@@ -176,7 +182,7 @@ static int pmic8xxx_pwrkey_probe(struct platform_device *pdev)
 	}
 
 	platform_set_drvdata(pdev, pwrkey);
-	device_init_wakeup(&pdev->dev, pdata->wakeup);
+	device_init_wakeup(&pdev->dev, 1);
 
 	return 0;
 }
@@ -192,9 +198,10 @@ static struct platform_driver pmic8xxx_pwrkey_driver = {
 	.probe		= pmic8xxx_pwrkey_probe,
 	.remove		= pmic8xxx_pwrkey_remove,
 	.driver		= {
-		.name	= PM8XXX_PWRKEY_DEV_NAME,
+		.name	= "pm8xxx-pwrkey",
 		.owner	= THIS_MODULE,
 		.pm	= &pm8xxx_pwr_key_pm_ops,
+		.of_match_table = pm8xxx_pwr_key_id_table,
 	},
 };
 module_platform_driver(pmic8xxx_pwrkey_driver);
include/linux/input/pmic8xxx-pwrkey.h deleted +0 -31
deleted file mode 100644
index 6d2974e57109..000000000000
@@ -1,31 +0,0 @@
-/* Copyright (c) 2010-2011, Code Aurora Forum. All rights reserved.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 and
- * only version 2 as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- */
-
-#ifndef __PMIC8XXX_PWRKEY_H__
-#define __PMIC8XXX_PWRKEY_H__
-
-#define PM8XXX_PWRKEY_DEV_NAME "pm8xxx-pwrkey"
-
-/**
- * struct pm8xxx_pwrkey_platform_data - platform data for pwrkey driver
- * @pull up:  power on register control for pull up/down configuration
- * @kpd_trigger_delay_us: time delay for power key state change interrupt
- *                  trigger.
- * @wakeup: configure power key as wakeup source
- */
-struct pm8xxx_pwrkey_platform_data  {
-	bool pull_up;
-	u32  kpd_trigger_delay_us;
-	u32  wakeup;
-};
-
-#endif /* __PMIC8XXX_PWRKEY_H__ */
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
#14 lore raw

PATCH v3 5/9 Input: pm8xxx-vibrator - Add DT match table

In reply to Stephen Boyd

The driver is only supported on DT enabled platforms. Convert the driver to DT so that it can probe properly.

---
 drivers/input/misc/pm8xxx-vibrator.c | 8 ++++++++
 1 file changed, 8 insertions(+)
drivers/input/misc/pm8xxx-vibrator.c +8 -0
index b88b7cbf93e2..5079bc54c3aa 100644
@@ -141,6 +141,13 @@ static int pm8xxx_vib_play_effect(struct input_dev *dev, void *data,
 	return 0;
 }
 
+static const struct of_device_id pm8xxx_vib_id_table[] = {
+	{ .compatible = "qcom,pm8058-vib" },
+	{ .compatible = "qcom,pm8921-vib" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, pm8xxx_vib_id_table);
+
 static int pm8xxx_vib_probe(struct platform_device *pdev)
 
 {
@@ -220,6 +227,7 @@ static struct platform_driver pm8xxx_vib_driver = {
 		.name	= "pm8xxx-vib",
 		.owner	= THIS_MODULE,
 		.pm	= &pm8xxx_vib_pm_ops,
+		.of_match_table = pm8xxx_vib_id_table,
 	},
 };
 module_platform_driver(pm8xxx_vib_driver);
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
#15 lore raw

PATCH v3 6/9 Input: pmic8xxx-keypad - Migrate to DT

In reply to Stephen Boyd

The driver is only supported on DT enabled platforms. Convert the driver to DT so that it can probe properly.

---
 drivers/input/keyboard/pmic8xxx-keypad.c | 148 ++++++++++++++++++-------------
 include/linux/input/pmic8xxx-keypad.h    |  52 -----------
 2 files changed, 85 insertions(+), 115 deletions(-)
 delete mode 100644 include/linux/input/pmic8xxx-keypad.h
drivers/input/keyboard/pmic8xxx-keypad.c +85 -63
index c6d3d216ffa7..d2e43073d76e 100644
@@ -20,9 +20,10 @@
 #include <linux/delay.h>
 #include <linux/mutex.h>
 #include <linux/regmap.h>
+#include <linux/of.h>
+#include <linux/input/matrix_keypad.h>
 
 #include <linux/mfd/pm8xxx/gpio.h>
-#include <linux/input/pmic8xxx-keypad.h>
 
 #define PM8XXX_MAX_ROWS		18
 #define PM8XXX_MAX_COLS		8
@@ -85,7 +86,8 @@
 
 /**
  * struct pmic8xxx_kp - internal keypad data structure
- * @pdata - keypad platform data pointer
+ * @num_cols - number of columns of keypad
+ * @num_rows - number of row of keypad
  * @input - input device pointer for keypad
  * @regmap - regmap handle
  * @key_sense_irq - key press/release irq number
@@ -97,7 +99,8 @@
  * @ctrl_reg - control register value
  */
 struct pmic8xxx_kp {
-	const struct pm8xxx_keypad_platform_data *pdata;
+	unsigned int num_rows;
+	unsigned int num_cols;
 	struct input_dev *input;
 	struct regmap *regmap;
 	int key_sense_irq;
@@ -116,9 +119,9 @@ static u8 pmic8xxx_col_state(struct pmic8xxx_kp *kp, u8 col)
 {
 	/* all keys pressed on that particular row? */
 	if (col == 0x00)
-		return 1 << kp->pdata->num_cols;
+		return 1 << kp->num_cols;
 	else
-		return col & ((1 << kp->pdata->num_cols) - 1);
+		return col & ((1 << kp->num_cols) - 1);
 }
 
 /*
@@ -181,10 +184,10 @@ static int pmic8xxx_kp_read_matrix(struct pmic8xxx_kp *kp, u16 *new_state,
 	int rc, read_rows;
 	unsigned int scan_val;
 
-	if (kp->pdata->num_rows < PM8XXX_MIN_ROWS)
+	if (kp->num_rows < PM8XXX_MIN_ROWS)
 		read_rows = PM8XXX_MIN_ROWS;
 	else
-		read_rows = kp->pdata->num_rows;
+		read_rows = kp->num_rows;
 
 	pmic8xxx_chk_sync_read(kp);
 
@@ -228,13 +231,13 @@ static void __pmic8xxx_kp_scan_matrix(struct pmic8xxx_kp *kp, u16 *new_state,
 {
 	int row, col, code;
 
-	for (row = 0; row < kp->pdata->num_rows; row++) {
+	for (row = 0; row < kp->num_rows; row++) {
 		int bits_changed = new_state[row] ^ old_state[row];
 
 		if (!bits_changed)
 			continue;
 
-		for (col = 0; col < kp->pdata->num_cols; col++) {
+		for (col = 0; col < kp->num_cols; col++) {
 			if (!(bits_changed & (1 << col)))
 				continue;
 
@@ -260,9 +263,9 @@ static bool pmic8xxx_detect_ghost_keys(struct pmic8xxx_kp *kp, u16 *new_state)
 	u16 check, row_state;
 
 	check = 0;
-	for (row = 0; row < kp->pdata->num_rows; row++) {
+	for (row = 0; row < kp->num_rows; row++) {
 		row_state = (~new_state[row]) &
-				 ((1 << kp->pdata->num_cols) - 1);
+				 ((1 << kp->num_cols) - 1);
 
 		if (hweight16(row_state) > 1) {
 			if (found_first == -1)
@@ -370,8 +373,13 @@ static irqreturn_t pmic8xxx_kp_irq(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
-static int pmic8xxx_kpd_init(struct pmic8xxx_kp *kp)
+static int pmic8xxx_kpd_init(struct pmic8xxx_kp *kp,
+			     struct platform_device *pdev)
 {
+	const struct device_node *of_node = pdev->dev.of_node;
+	unsigned int scan_delay_ms;
+	unsigned int row_hold_ns;
+	unsigned int debounce_ms;
 	int bits, rc, cycles;
 	u8 scan_val = 0, ctrl_val = 0;
 	static const u8 row_bits[] = {
@@ -379,18 +387,18 @@ static int pmic8xxx_kpd_init(struct pmic8xxx_kp *kp)
 	};
 
 	/* Find column bits */
-	if (kp->pdata->num_cols < KEYP_CTRL_SCAN_COLS_MIN)
+	if (kp->num_cols < KEYP_CTRL_SCAN_COLS_MIN)
 		bits = 0;
 	else
-		bits = kp->pdata->num_cols - KEYP_CTRL_SCAN_COLS_MIN;
+		bits = kp->num_cols - KEYP_CTRL_SCAN_COLS_MIN;
 	ctrl_val = (bits & KEYP_CTRL_SCAN_COLS_BITS) <<
 		KEYP_CTRL_SCAN_COLS_SHIFT;
 
 	/* Find row bits */
-	if (kp->pdata->num_rows < KEYP_CTRL_SCAN_ROWS_MIN)
+	if (kp->num_rows < KEYP_CTRL_SCAN_ROWS_MIN)
 		bits = 0;
 	else
-		bits = row_bits[kp->pdata->num_rows - KEYP_CTRL_SCAN_ROWS_MIN];
+		bits = row_bits[kp->num_rows - KEYP_CTRL_SCAN_ROWS_MIN];
 
 	ctrl_val |= (bits << KEYP_CTRL_SCAN_ROWS_SHIFT);
 
@@ -400,15 +408,44 @@ static int pmic8xxx_kpd_init(struct pmic8xxx_kp *kp)
 		return rc;
 	}
 
-	bits = (kp->pdata->debounce_ms / 5) - 1;
+	if (of_property_read_u32(of_node, "scan-delay", &scan_delay_ms))
+		scan_delay_ms = MIN_SCAN_DELAY;
+
+	if (scan_delay_ms > MAX_SCAN_DELAY || scan_delay_ms < MIN_SCAN_DELAY ||
+		!is_power_of_2(scan_delay_ms)) {
+		dev_err(&pdev->dev, "invalid keypad scan time supplied\n");
+		return -EINVAL;
+	}
+
+	if (of_property_read_u32(of_node, "row-hold", &row_hold_ns))
+		row_hold_ns = MIN_ROW_HOLD_DELAY;
+
+	if (row_hold_ns > MAX_ROW_HOLD_DELAY ||
+		row_hold_ns < MIN_ROW_HOLD_DELAY ||
+		((row_hold_ns % MIN_ROW_HOLD_DELAY) != 0)) {
+		dev_err(&pdev->dev, "invalid keypad row hold time supplied\n");
+		return -EINVAL;
+	}
+
+	if (of_property_read_u32(of_node, "debounce", &debounce_ms))
+		debounce_ms = MIN_DEBOUNCE_TIME;
+
+	if (((debounce_ms % 5) != 0) ||
+		debounce_ms > MAX_DEBOUNCE_TIME ||
+		debounce_ms < MIN_DEBOUNCE_TIME) {
+		dev_err(&pdev->dev, "invalid debounce time supplied\n");
+		return -EINVAL;
+	}
+
+	bits = (debounce_ms / 5) - 1;
 
 	scan_val |= (bits << KEYP_SCAN_DBOUNCE_SHIFT);
 
-	bits = fls(kp->pdata->scan_delay_ms) - 1;
+	bits = fls(scan_delay_ms) - 1;
 	scan_val |= (bits << KEYP_SCAN_PAUSE_SHIFT);
 
 	/* Row hold time is a multiple of 32KHz cycles. */
-	cycles = (kp->pdata->row_hold_ns * KEYP_CLOCK_FREQ) / NSEC_PER_SEC;
+	cycles = (row_hold_ns * KEYP_CLOCK_FREQ) / NSEC_PER_SEC;
 
 	scan_val |= (cycles << KEYP_SCAN_ROW_HOLD_SHIFT);
 
@@ -481,6 +518,13 @@ static void pmic8xxx_kp_close(struct input_dev *dev)
 	pmic8xxx_kp_disable(kp);
 }
 
+static const struct of_device_id pm8xxx_match_table[] = {
+	{ .compatible = "qcom,pm8058-keypad" },
+	{ .compatible = "qcom,pm8921-keypad" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, pm8xxx_match_table);
+
 /*
  * keypad controller should be initialized in the following sequence
  * only, otherwise it might get into FSM stuck state.
@@ -493,9 +537,9 @@ static void pmic8xxx_kp_close(struct input_dev *dev)
  */
 static int pmic8xxx_kp_probe(struct platform_device *pdev)
 {
-	const struct pm8xxx_keypad_platform_data *pdata =
-					dev_get_platdata(&pdev->dev);
-	const struct matrix_keymap_data *keymap_data;
+	unsigned int rows, cols;
+	bool repeat;
+	bool wakeup;
 	struct pmic8xxx_kp *kp;
 	int rc;
 	unsigned int ctrl_val;
@@ -520,44 +564,20 @@ static int pmic8xxx_kp_probe(struct platform_device *pdev)
 		.inv_int_pol	= 1,
 	};
 
+	rc = matrix_keypad_parse_of_params(&pdev->dev, &rows, &cols);
+	if (rc)
+		return rc;
 
-	if (!pdata || !pdata->num_cols || !pdata->num_rows ||
-		pdata->num_cols > PM8XXX_MAX_COLS ||
-		pdata->num_rows > PM8XXX_MAX_ROWS ||
-		pdata->num_cols < PM8XXX_MIN_COLS) {
+	if (cols > PM8XXX_MAX_COLS || rows > PM8XXX_MAX_ROWS ||
+		cols < PM8XXX_MIN_COLS) {
 		dev_err(&pdev->dev, "invalid platform data\n");
 		return -EINVAL;
 	}
 
-	if (!pdata->scan_delay_ms ||
-		pdata->scan_delay_ms > MAX_SCAN_DELAY ||
-		pdata->scan_delay_ms < MIN_SCAN_DELAY ||
-		!is_power_of_2(pdata->scan_delay_ms)) {
-		dev_err(&pdev->dev, "invalid keypad scan time supplied\n");
-		return -EINVAL;
-	}
-
-	if (!pdata->row_hold_ns ||
-		pdata->row_hold_ns > MAX_ROW_HOLD_DELAY ||
-		pdata->row_hold_ns < MIN_ROW_HOLD_DELAY ||
-		((pdata->row_hold_ns % MIN_ROW_HOLD_DELAY) != 0)) {
-		dev_err(&pdev->dev, "invalid keypad row hold time supplied\n");
-		return -EINVAL;
-	}
-
-	if (!pdata->debounce_ms ||
-		((pdata->debounce_ms % 5) != 0) ||
-		pdata->debounce_ms > MAX_DEBOUNCE_TIME ||
-		pdata->debounce_ms < MIN_DEBOUNCE_TIME) {
-		dev_err(&pdev->dev, "invalid debounce time supplied\n");
-		return -EINVAL;
-	}
-
-	keymap_data = pdata->keymap_data;
-	if (!keymap_data) {
-		dev_err(&pdev->dev, "no keymap data supplied\n");
-		return -EINVAL;
-	}
+	repeat = !of_property_read_bool(pdev->dev.of_node,
+					"linux,input-no-autorepeat");
+	wakeup = !of_property_read_bool(pdev->dev.of_node,
+					"linux,keypad-wakeup");
 
 	kp = devm_kzalloc(&pdev->dev, sizeof(*kp), GFP_KERNEL);
 	if (!kp)
@@ -569,7 +589,8 @@ static int pmic8xxx_kp_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, kp);
 
-	kp->pdata	= pdata;
+	kp->num_rows	= rows;
+	kp->num_cols	= cols;
 	kp->dev		= &pdev->dev;
 
 	kp->input = devm_input_allocate_device(&pdev->dev);
@@ -590,8 +611,8 @@ static int pmic8xxx_kp_probe(struct platform_device *pdev)
 		return kp->key_stuck_irq;
 	}
 
-	kp->input->name = pdata->input_name ? : "PMIC8XXX keypad";
-	kp->input->phys = pdata->input_phys_device ? : "pmic8xxx_keypad/input0";
+	kp->input->name = "PMIC8XXX keypad";
+	kp->input->phys = "pmic8xxx_keypad/input0";
 
 	kp->input->id.bustype	= BUS_I2C;
 	kp->input->id.version	= 0x0001;
@@ -601,7 +622,7 @@ static int pmic8xxx_kp_probe(struct platform_device *pdev)
 	kp->input->open		= pmic8xxx_kp_open;
 	kp->input->close	= pmic8xxx_kp_close;
 
-	rc = matrix_keypad_build_keymap(keymap_data, NULL,
+	rc = matrix_keypad_build_keymap(NULL, NULL,
 					PM8XXX_MAX_ROWS, PM8XXX_MAX_COLS,
 					kp->keycodes, kp->input);
 	if (rc) {
@@ -609,7 +630,7 @@ static int pmic8xxx_kp_probe(struct platform_device *pdev)
 		return rc;
 	}
 
-	if (pdata->rep)
+	if (repeat)
 		__set_bit(EV_REP, kp->input->evbit);
 	input_set_capability(kp->input, EV_MSC, MSC_SCAN);
 
@@ -619,7 +640,7 @@ static int pmic8xxx_kp_probe(struct platform_device *pdev)
 	memset(kp->keystate, 0xff, sizeof(kp->keystate));
 	memset(kp->stuckstate, 0xff, sizeof(kp->stuckstate));
 
-	rc = pmic8xxx_kpd_init(kp);
+	rc = pmic8xxx_kpd_init(kp, pdev);
 	if (rc < 0) {
 		dev_err(&pdev->dev, "unable to initialize keypad controller\n");
 		return rc;
@@ -669,7 +690,7 @@ static int pmic8xxx_kp_probe(struct platform_device *pdev)
 		return rc;
 	}
 
-	device_init_wakeup(&pdev->dev, pdata->wakeup);
+	device_init_wakeup(&pdev->dev, wakeup);
 
 	return 0;
 }
@@ -722,9 +743,10 @@ static SIMPLE_DEV_PM_OPS(pm8xxx_kp_pm_ops,
 static struct platform_driver pmic8xxx_kp_driver = {
 	.probe		= pmic8xxx_kp_probe,
 	.driver		= {
-		.name = PM8XXX_KEYPAD_DEV_NAME,
+		.name = "pm8xxx-keypad",
 		.owner = THIS_MODULE,
 		.pm = &pm8xxx_kp_pm_ops,
+		.of_match_table = pm8xxx_match_table,
 	},
 };
 module_platform_driver(pmic8xxx_kp_driver);
include/linux/input/pmic8xxx-keypad.h deleted +0 -52

52 lines not on this page · raw message

Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
#16 lore raw

PATCH v3 7/9 devicetree: bindings: Document PM8921/8058 keypads

In reply to Stephen Boyd

Document the keypad device found on PM8921 and PM8058 PMICs.

---
 .../bindings/input/qcom,pm8xxx-keypad.txt          | 72 ++++++++++++++++++++++
 1 file changed, 72 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/qcom,pm8xxx-keypad.txt
Documentation/devicetree/bindings/input/qcom,pm8xxx-keypad.txt new file +72 -0
new file mode 100644
index 000000000000..aa5a9c6cf512
@@ -0,0 +1,72 @@
+Qualcomm PM8xxx PMIC Keypad
+
+PROPERTIES
+
+- compatible:
+	Usage: required
+	Value type: <string>
+	Definition: must be one of:
+		    "qcom,pm8058-keypad"
+		    "qcom,pm8921-keypad"
+- interrupts:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: the first interrupt specifies the key sense interrupt
+		    and the second interrupt specifies the key stuck interrupt.
+		    The format of the specifier is defined by the binding
+		    document describing the node's interrupt parent.
+
+- linux,keymap:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: the linux keymap. More information can be found in
+		    input/matrix-keymap.txt.
+
+- keypad,num-rows:
+	Usage: required
+	Value type: <u32>
+	Definition: number of rows in the keymap. More information can be found
+		    in input/matrix-keymap.txt.
+
+- keypad,num-columns:
+	Usage: required
+	Value type: <u32>
+	Definition: number of columns in the keymap. More information can be
+		    found in input/matrix-keymap.txt.
+
+- debounce:
+	Usage: optional
+	Value type: <u32>
+	Definition: time in microseconds that key must be pressed or release
+		    for key sense interrupt to trigger.
+
+- scan-delay:
+	Usage: optional
+	Value type: <u32>
+	Definition: time in microseconds to pause between successive scans
+		    of the matrix array.
+
+- row-hold:
+	Usage: optional
+	Value type: <u32>
+	Definition: time in nanoseconds to pause between scans of each row in
+		    the matrix array.
+
+EXAMPLE
+
+	keypad {
+		compatible = "qcom,pm8921-keypad";
+		interrupt-parent = <&pmicintc>;
+		interrupts = <74 1>, <75 1>;
+		linux,keymap = <
+			MATRIX_KEY(0, 0, KEY_VOLUMEUP)
+			MATRIX_KEY(0, 1, KEY_VOLUMEDOWN)
+			MATRIX_KEY(0, 2, KEY_CAMERA_FOCUS)
+			MATRIX_KEY(0, 3, KEY_CAMERA)
+			>;
+		keypad,num-rows = <1>;
+		keypad,num-columns = <5>;
+		debounce = <15>;
+		scan-delay = <32>;
+		row-hold = <91500>;
+	};
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
#17 lore raw

PATCH v3 8/9 devicetree: bindings: Document PM8921/8058 power keys

In reply to Stephen Boyd

Document the power key found on PM8921 and PM8058 PMICs.

---
 .../bindings/input/qcom,pm8xxx-pwrkey.txt          | 39 ++++++++++++++++++++++
 1 file changed, 39 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/qcom,pm8xxx-pwrkey.txt
Documentation/devicetree/bindings/input/qcom,pm8xxx-pwrkey.txt new file +39 -0
new file mode 100644
index 000000000000..e124d9f33632
@@ -0,0 +1,39 @@
+Qualcomm PM8xxx PMIC Power Key
+
+PROPERTIES
+
+- compatible:
+	Usage: required
+	Value type: <string>
+	Definition: must be one of:
+		    "qcom,pm8058-pwrkey"
+		    "qcom,pm8921-pwrkey"
+- interrupts:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: the first interrupt specifies the key release interrupt
+		    and the second interrupt specifies the key press interrupt.
+		    The format of the specifier is defined by the binding
+		    document describing the node's interrupt parent.
+
+- debounce:
+	Usage: optional
+	Value type: <u32>
+	Definition: time in microseconds that key must be pressed or release
+		    for state change interrupt to trigger.
+
+- pull-up:
+	Usage: optional
+	Value type: <empty>
+	Definition: presence of this property indicates that the KPDPWR_N pin
+		    should be configured for pull up.
+
+EXAMPLE
+
+	pwrkey {
+		compatible = "qcom,pm8921-pwrkey";
+		interrupt-parent = <&pmicintc>;
+		interrupts = <50 1>, <51 1>;
+		debounce = <15625>;
+		pull-up;
+	};
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
#18 lore raw

PATCH v3 9/9 devicetree: bindings: Document PM8921/8058 vibrators

In reply to Stephen Boyd

Document the vibration device found on PM8921 and PM8058 PMICs.

---
 .../devicetree/bindings/input/qcom,pm8xxx-vib.txt        | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.txt
Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.txt new file +16 -0
new file mode 100644
index 000000000000..dca1b8872cf1
@@ -0,0 +1,16 @@
+Qualcomm PM8xxx PMIC Vibrator
+
+PROPERTIES
+
+- compatible:
+	Usage: required
+	Value type: <string>
+	Definition: must be one of:
+		    "qcom,pm8058-vib"
+		    "qcom,pm8921-vib"
+
+EXAMPLE
+
+	vibrator {
+		compatible = "qcom,pm8058-vib";
+	};
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation