linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] Use regmap+devm in pm8xxx input drivers
@ 2013-12-10 23:43 Stephen Boyd
  2013-12-10 23:43 ` [PATCH 1/7] Input: pmic8xxx-pwrkey - Pass input device directly to interrupt Stephen Boyd
                   ` (6 more replies)
  0 siblings, 7 replies; 18+ messages in thread
From: Stephen Boyd @ 2013-12-10 23:43 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, linux-input,
	Thomas Gleixner

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.

There was no devm_request_any_context_irq() available, so I've added
it here.

Stephen Boyd (7):
  Input: pmic8xxx-pwrkey - Pass input device directly to interrupt
  Input: pmic8xxx-pwrkey - Migrate to regmap APIs
  Input: pm8xxx-vibrator - Migrate to devm_* APIs
  Input: pm8xxx-vibrator - Migrate to regmap APIs
  genirq: Add devm_request_any_context_irq()
  Input: pmic8xxx-keypad - Migrate to devm_* APIs
  Input: pmic8xxx-keypad - Migrate to regmap APIs

 drivers/input/keyboard/pmic8xxx-keypad.c | 143 ++++++++++---------------------
 drivers/input/misc/pm8xxx-vibrator.c     | 107 ++++++-----------------
 drivers/input/misc/pmic8xxx-pwrkey.c     |  35 ++++----
 include/linux/interrupt.h                |   5 ++
 kernel/irq/devres.c                      |  45 ++++++++++
 5 files changed, 141 insertions(+), 194 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/7] Input: pmic8xxx-pwrkey - Pass input device directly to interrupt
  2013-12-10 23:43 [PATCH 0/7] Use regmap+devm in pm8xxx input drivers Stephen Boyd
@ 2013-12-10 23:43 ` Stephen Boyd
  2013-12-15 11:21   ` Dmitry Torokhov
  2013-12-10 23:43 ` [PATCH 2/7] Input: pmic8xxx-pwrkey - Migrate to regmap APIs Stephen Boyd
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Stephen Boyd @ 2013-12-10 23:43 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, linux-input

Instead of passing the pointer to the container structure just
pass the input device here. This saves a dereference in the fast
path.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/input/misc/pmic8xxx-pwrkey.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/input/misc/pmic8xxx-pwrkey.c b/drivers/input/misc/pmic8xxx-pwrkey.c
index ce3c426..233b216 100644
--- a/drivers/input/misc/pmic8xxx-pwrkey.c
+++ b/drivers/input/misc/pmic8xxx-pwrkey.c
@@ -32,26 +32,21 @@
  * @key_press_irq: key press irq number
  */
 struct pmic8xxx_pwrkey {
-	struct input_dev *pwr;
 	int key_press_irq;
 };
 
-static irqreturn_t pwrkey_press_irq(int irq, void *_pwrkey)
+static irqreturn_t pwrkey_press_irq(int irq, void *pwr)
 {
-	struct pmic8xxx_pwrkey *pwrkey = _pwrkey;
-
-	input_report_key(pwrkey->pwr, KEY_POWER, 1);
-	input_sync(pwrkey->pwr);
+	input_report_key(pwr, KEY_POWER, 1);
+	input_sync(pwr);
 
 	return IRQ_HANDLED;
 }
 
-static irqreturn_t pwrkey_release_irq(int irq, void *_pwrkey)
+static irqreturn_t pwrkey_release_irq(int irq, void *pwr)
 {
-	struct pmic8xxx_pwrkey *pwrkey = _pwrkey;
-
-	input_report_key(pwrkey->pwr, KEY_POWER, 0);
-	input_sync(pwrkey->pwr);
+	input_report_key(pwr, KEY_POWER, 0);
+	input_sync(pwr);
 
 	return IRQ_HANDLED;
 }
@@ -147,12 +142,11 @@ static int pmic8xxx_pwrkey_probe(struct platform_device *pdev)
 	}
 
 	pwrkey->key_press_irq = key_press_irq;
-	pwrkey->pwr = pwr;
 
 	platform_set_drvdata(pdev, pwrkey);
 
 	err = devm_request_irq(&pdev->dev, key_press_irq, pwrkey_press_irq,
-		IRQF_TRIGGER_RISING, "pmic8xxx_pwrkey_press", pwrkey);
+		IRQF_TRIGGER_RISING, "pmic8xxx_pwrkey_press", pwr);
 	if (err < 0) {
 		dev_dbg(&pdev->dev, "Can't get %d IRQ for pwrkey: %d\n",
 				 key_press_irq, err);
@@ -160,7 +154,7 @@ static int pmic8xxx_pwrkey_probe(struct platform_device *pdev)
 	}
 
 	err = devm_request_irq(&pdev->dev, key_release_irq, pwrkey_release_irq,
-		 IRQF_TRIGGER_RISING, "pmic8xxx_pwrkey_release", pwrkey);
+		 IRQF_TRIGGER_RISING, "pmic8xxx_pwrkey_release", pwr);
 	if (err < 0) {
 		dev_dbg(&pdev->dev, "Can't get %d IRQ for pwrkey: %d\n",
 				 key_release_irq, err);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/7] Input: pmic8xxx-pwrkey - Migrate to regmap APIs
  2013-12-10 23:43 [PATCH 0/7] Use regmap+devm in pm8xxx input drivers Stephen Boyd
  2013-12-10 23:43 ` [PATCH 1/7] Input: pmic8xxx-pwrkey - Pass input device directly to interrupt Stephen Boyd
@ 2013-12-10 23:43 ` Stephen Boyd
  2013-12-15 11:33   ` Dmitry Torokhov
  2013-12-10 23:43 ` [PATCH 3/7] Input: pm8xxx-vibrator - Migrate to devm_* APIs Stephen Boyd
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Stephen Boyd @ 2013-12-10 23:43 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, linux-input

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.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/input/misc/pmic8xxx-pwrkey.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/input/misc/pmic8xxx-pwrkey.c b/drivers/input/misc/pmic8xxx-pwrkey.c
index 233b216..a4de105 100644
--- a/drivers/input/misc/pmic8xxx-pwrkey.c
+++ b/drivers/input/misc/pmic8xxx-pwrkey.c
@@ -18,9 +18,9 @@
 #include <linux/input.h>
 #include <linux/interrupt.h>
 #include <linux/platform_device.h>
+#include <linux/regmap.h>
 #include <linux/log2.h>
 
-#include <linux/mfd/pm8xxx/core.h>
 #include <linux/input/pmic8xxx-pwrkey.h>
 
 #define PON_CNTL_1 0x1C
@@ -83,7 +83,8 @@ static int pmic8xxx_pwrkey_probe(struct platform_device *pdev)
 	int key_press_irq = platform_get_irq(pdev, 1);
 	int err;
 	unsigned int delay;
-	u8 pon_cntl;
+	unsigned int pon_cntl;
+	struct regmap *regmap;
 	struct pmic8xxx_pwrkey *pwrkey;
 	const struct pm8xxx_pwrkey_platform_data *pdata =
 					dev_get_platdata(&pdev->dev);
@@ -108,6 +109,10 @@ static int pmic8xxx_pwrkey_probe(struct platform_device *pdev)
 		err = -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";
@@ -116,7 +121,7 @@ static int pmic8xxx_pwrkey_probe(struct platform_device *pdev)
 	delay = (pdata->kpd_trigger_delay_us << 10) / USEC_PER_SEC;
 	delay = 1 + ilog2(delay);
 
-	err = pm8xxx_readb(pdev->dev.parent, PON_CNTL_1, &pon_cntl);
+	err = regmap_read(regmap, PON_CNTL_1, &pon_cntl);
 	if (err < 0) {
 		dev_err(&pdev->dev, "failed reading PON_CNTL_1 err=%d\n", err);
 		return err;
@@ -129,7 +134,7 @@ static int pmic8xxx_pwrkey_probe(struct platform_device *pdev)
 	else
 		pon_cntl &= ~PON_CNTL_PULL_UP;
 
-	err = pm8xxx_writeb(pdev->dev.parent, PON_CNTL_1, pon_cntl);
+	err = regmap_write(regmap, PON_CNTL_1, pon_cntl);
 	if (err < 0) {
 		dev_err(&pdev->dev, "failed writing PON_CNTL_1 err=%d\n", err);
 		return err;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/7] Input: pm8xxx-vibrator - Migrate to devm_* APIs
  2013-12-10 23:43 [PATCH 0/7] Use regmap+devm in pm8xxx input drivers Stephen Boyd
  2013-12-10 23:43 ` [PATCH 1/7] Input: pmic8xxx-pwrkey - Pass input device directly to interrupt Stephen Boyd
  2013-12-10 23:43 ` [PATCH 2/7] Input: pmic8xxx-pwrkey - Migrate to regmap APIs Stephen Boyd
@ 2013-12-10 23:43 ` Stephen Boyd
  2013-12-10 23:43 ` [PATCH 4/7] Input: pm8xxx-vibrator - Migrate to regmap APIs Stephen Boyd
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 18+ messages in thread
From: Stephen Boyd @ 2013-12-10 23:43 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, linux-input

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

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/input/misc/pm8xxx-vibrator.c | 44 +++++++++++-------------------------
 1 file changed, 13 insertions(+), 31 deletions(-)

diff --git a/drivers/input/misc/pm8xxx-vibrator.c b/drivers/input/misc/pm8xxx-vibrator.c
index ec086f6..2a92ab0 100644
--- a/drivers/input/misc/pm8xxx-vibrator.c
+++ b/drivers/input/misc/pm8xxx-vibrator.c
@@ -186,13 +186,13 @@ static int pm8xxx_vib_probe(struct platform_device *pdev)
 	int error;
 	u8 val;
 
-	vib = kzalloc(sizeof(*vib), GFP_KERNEL);
-	input_dev = input_allocate_device();
-	if (!vib || !input_dev) {
-		dev_err(&pdev->dev, "couldn't allocate memory\n");
-		error = -ENOMEM;
-		goto err_free_mem;
-	}
+	vib = devm_kzalloc(&pdev->dev, sizeof(*vib), GFP_KERNEL);
+	if (!vib)
+		return -ENOMEM;
+
+	input_dev = devm_input_allocate_device(&pdev->dev);
+	if (!input_dev)
+		return -ENOMEM;
 
 	INIT_WORK(&vib->work, pm8xxx_work_handler);
 	vib->dev = &pdev->dev;
@@ -201,17 +201,17 @@ static int pm8xxx_vib_probe(struct platform_device *pdev)
 	/* operate in manual mode */
 	error = pm8xxx_vib_read_u8(vib, &val, VIB_DRV);
 	if (error < 0)
-		goto err_free_mem;
+		return error;
+
 	val &= ~VIB_DRV_EN_MANUAL_MASK;
 	error = pm8xxx_vib_write_u8(vib, val, VIB_DRV);
 	if (error < 0)
-		goto err_free_mem;
+		return error;
 
 	vib->reg_vib_drv = val;
 
 	input_dev->name = "pm8xxx_vib_ffmemless";
 	input_dev->id.version = 1;
-	input_dev->dev.parent = &pdev->dev;
 	input_dev->close = pm8xxx_vib_close;
 	input_set_drvdata(input_dev, vib);
 	input_set_capability(vib->vib_input_dev, EV_FF, FF_RUMBLE);
@@ -221,35 +221,18 @@ static int pm8xxx_vib_probe(struct platform_device *pdev)
 	if (error) {
 		dev_err(&pdev->dev,
 			"couldn't register vibrator as FF device\n");
-		goto err_free_mem;
+		return error;
 	}
 
 	error = input_register_device(input_dev);
 	if (error) {
 		dev_err(&pdev->dev, "couldn't register input device\n");
-		goto err_destroy_memless;
+		input_ff_destroy(input_dev);
+		return error;
 	}
 
 	platform_set_drvdata(pdev, vib);
 	return 0;
-
-err_destroy_memless:
-	input_ff_destroy(input_dev);
-err_free_mem:
-	input_free_device(input_dev);
-	kfree(vib);
-
-	return error;
-}
-
-static int pm8xxx_vib_remove(struct platform_device *pdev)
-{
-	struct pm8xxx_vib *vib = platform_get_drvdata(pdev);
-
-	input_unregister_device(vib->vib_input_dev);
-	kfree(vib);
-
-	return 0;
 }
 
 #ifdef CONFIG_PM_SLEEP
@@ -268,7 +251,6 @@ static SIMPLE_DEV_PM_OPS(pm8xxx_vib_pm_ops, pm8xxx_vib_suspend, NULL);
 
 static struct platform_driver pm8xxx_vib_driver = {
 	.probe		= pm8xxx_vib_probe,
-	.remove		= pm8xxx_vib_remove,
 	.driver		= {
 		.name	= "pm8xxx-vib",
 		.owner	= THIS_MODULE,
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 4/7] Input: pm8xxx-vibrator - Migrate to regmap APIs
  2013-12-10 23:43 [PATCH 0/7] Use regmap+devm in pm8xxx input drivers Stephen Boyd
                   ` (2 preceding siblings ...)
  2013-12-10 23:43 ` [PATCH 3/7] Input: pm8xxx-vibrator - Migrate to devm_* APIs Stephen Boyd
@ 2013-12-10 23:43 ` Stephen Boyd
  2013-12-10 23:43 ` [PATCH 5/7] genirq: Add devm_request_any_context_irq() Stephen Boyd
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 18+ messages in thread
From: Stephen Boyd @ 2013-12-10 23:43 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, linux-input

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.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/input/misc/pm8xxx-vibrator.c | 61 +++++++++---------------------------
 1 file changed, 14 insertions(+), 47 deletions(-)

diff --git a/drivers/input/misc/pm8xxx-vibrator.c b/drivers/input/misc/pm8xxx-vibrator.c
index 2a92ab0..cb5a8e8f 100644
--- a/drivers/input/misc/pm8xxx-vibrator.c
+++ b/drivers/input/misc/pm8xxx-vibrator.c
@@ -17,7 +17,7 @@
 #include <linux/platform_device.h>
 #include <linux/input.h>
 #include <linux/slab.h>
-#include <linux/mfd/pm8xxx/core.h>
+#include <linux/regmap.h>
 
 #define VIB_DRV			0x4A
 
@@ -35,7 +35,7 @@
  * struct pm8xxx_vib - structure to hold vibrator data
  * @vib_input_dev: input device supporting force feedback
  * @work: work structure to set the vibration parameters
- * @dev: device supporting force feedback
+ * @regmap: regmap for register read/write
  * @speed: speed of vibration set from userland
  * @active: state of vibrator
  * @level: level of vibration to set in the chip
@@ -44,7 +44,7 @@
 struct pm8xxx_vib {
 	struct input_dev *vib_input_dev;
 	struct work_struct work;
-	struct device *dev;
+	struct regmap *regmap;
 	int speed;
 	int level;
 	bool active;
@@ -52,42 +52,6 @@ struct pm8xxx_vib {
 };
 
 /**
- * pm8xxx_vib_read_u8 - helper to read a byte from pmic chip
- * @vib: pointer to vibrator structure
- * @data: placeholder for data to be read
- * @reg: register address
- */
-static int pm8xxx_vib_read_u8(struct pm8xxx_vib *vib,
-				 u8 *data, u16 reg)
-{
-	int rc;
-
-	rc = pm8xxx_readb(vib->dev->parent, reg, data);
-	if (rc < 0)
-		dev_warn(vib->dev, "Error reading pm8xxx reg 0x%x(0x%x)\n",
-				reg, rc);
-	return rc;
-}
-
-/**
- * pm8xxx_vib_write_u8 - helper to write a byte to pmic chip
- * @vib: pointer to vibrator structure
- * @data: data to write
- * @reg: register address
- */
-static int pm8xxx_vib_write_u8(struct pm8xxx_vib *vib,
-				 u8 data, u16 reg)
-{
-	int rc;
-
-	rc = pm8xxx_writeb(vib->dev->parent, reg, data);
-	if (rc < 0)
-		dev_warn(vib->dev, "Error writing pm8xxx reg 0x%x(0x%x)\n",
-				reg, rc);
-	return rc;
-}
-
-/**
  * pm8xxx_vib_set - handler to start/stop vibration
  * @vib: pointer to vibrator structure
  * @on: state to set
@@ -95,14 +59,14 @@ static int pm8xxx_vib_write_u8(struct pm8xxx_vib *vib,
 static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on)
 {
 	int rc;
-	u8 val = vib->reg_vib_drv;
+	unsigned int val = vib->reg_vib_drv;
 
 	if (on)
 		val |= ((vib->level << VIB_DRV_SEL_SHIFT) & VIB_DRV_SEL_MASK);
 	else
 		val &= ~VIB_DRV_SEL_MASK;
 
-	rc = pm8xxx_vib_write_u8(vib, val, VIB_DRV);
+	rc = regmap_write(vib->regmap, VIB_DRV, val);
 	if (rc < 0)
 		return rc;
 
@@ -118,9 +82,9 @@ static void pm8xxx_work_handler(struct work_struct *work)
 {
 	struct pm8xxx_vib *vib = container_of(work, struct pm8xxx_vib, work);
 	int rc;
-	u8 val;
+	unsigned int val;
 
-	rc = pm8xxx_vib_read_u8(vib, &val, VIB_DRV);
+	rc = regmap_read(vib->regmap, VIB_DRV, &val);
 	if (rc < 0)
 		return;
 
@@ -184,27 +148,30 @@ static int pm8xxx_vib_probe(struct platform_device *pdev)
 	struct pm8xxx_vib *vib;
 	struct input_dev *input_dev;
 	int error;
-	u8 val;
+	unsigned int val;
 
 	vib = devm_kzalloc(&pdev->dev, sizeof(*vib), GFP_KERNEL);
 	if (!vib)
 		return -ENOMEM;
 
+	vib->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+	if (!vib->regmap)
+		return -ENODEV;
+
 	input_dev = devm_input_allocate_device(&pdev->dev);
 	if (!input_dev)
 		return -ENOMEM;
 
 	INIT_WORK(&vib->work, pm8xxx_work_handler);
-	vib->dev = &pdev->dev;
 	vib->vib_input_dev = input_dev;
 
 	/* operate in manual mode */
-	error = pm8xxx_vib_read_u8(vib, &val, VIB_DRV);
+	error = regmap_read(vib->regmap, VIB_DRV, &val);
 	if (error < 0)
 		return error;
 
 	val &= ~VIB_DRV_EN_MANUAL_MASK;
-	error = pm8xxx_vib_write_u8(vib, val, VIB_DRV);
+	error = regmap_write(vib->regmap, VIB_DRV, val);
 	if (error < 0)
 		return error;
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 5/7] genirq: Add devm_request_any_context_irq()
  2013-12-10 23:43 [PATCH 0/7] Use regmap+devm in pm8xxx input drivers Stephen Boyd
                   ` (3 preceding siblings ...)
  2013-12-10 23:43 ` [PATCH 4/7] Input: pm8xxx-vibrator - Migrate to regmap APIs Stephen Boyd
@ 2013-12-10 23:43 ` Stephen Boyd
  2013-12-15 10:42   ` Dmitry Torokhov
  2013-12-10 23:43 ` [PATCH 6/7] Input: pmic8xxx-keypad - Migrate to devm_* APIs Stephen Boyd
  2013-12-10 23:43 ` [PATCH 7/7] Input: pmic8xxx-keypad - Migrate to regmap APIs Stephen Boyd
  6 siblings, 1 reply; 18+ messages in thread
From: Stephen Boyd @ 2013-12-10 23:43 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, linux-input,
	Thomas Gleixner

Some drivers use request_any_context_irq() but there isn't a
devm_* function for it. Add one so that these drivers don't need
to explicitly free the irq on driver detach.

Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 include/linux/interrupt.h |  5 +++++
 kernel/irq/devres.c       | 45 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+)

diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index c9e831d..8334f9b 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -160,6 +160,11 @@ devm_request_irq(struct device *dev, unsigned int irq, irq_handler_t handler,
 					 devname, dev_id);
 }
 
+extern int __must_check
+devm_request_any_context_irq(struct device *dev, unsigned int irq,
+		 irq_handler_t handler, unsigned long irqflags,
+		 const char *devname, void *dev_id);
+
 extern void devm_free_irq(struct device *dev, unsigned int irq, void *dev_id);
 
 /*
diff --git a/kernel/irq/devres.c b/kernel/irq/devres.c
index bd8e788..f71e9ff 100644
--- a/kernel/irq/devres.c
+++ b/kernel/irq/devres.c
@@ -73,6 +73,51 @@ int devm_request_threaded_irq(struct device *dev, unsigned int irq,
 EXPORT_SYMBOL(devm_request_threaded_irq);
 
 /**
+ *	devm_request_any_context_irq - allocate an interrupt line for a managed device
+ *	@dev: device to request interrupt for
+ *	@irq: Interrupt line to allocate
+ *	@handler: Function to be called when the IRQ occurs
+ *	@thread_fn: function to be called in a threaded interrupt context. NULL
+ *		    for devices which handle everything in @handler
+ *	@irqflags: Interrupt type flags
+ *	@devname: An ascii name for the claiming device
+ *	@dev_id: A cookie passed back to the handler function
+ *
+ *	Except for the extra @dev argument, this function takes the
+ *	same arguments and performs the same function as
+ *	request_any_context_irq().  IRQs requested with this function will be
+ *	automatically freed on driver detach.
+ *
+ *	If an IRQ allocated with this function needs to be freed
+ *	separately, devm_free_irq() must be used.
+ */
+int devm_request_any_context_irq(struct device *dev, unsigned int irq,
+			      irq_handler_t handler, unsigned long irqflags,
+			      const char *devname, void *dev_id)
+{
+	struct irq_devres *dr;
+	int rc;
+
+	dr = devres_alloc(devm_irq_release, sizeof(struct irq_devres),
+			  GFP_KERNEL);
+	if (!dr)
+		return -ENOMEM;
+
+	rc = request_any_context_irq(irq, handler, irqflags, devname, dev_id);
+	if (rc) {
+		devres_free(dr);
+		return rc;
+	}
+
+	dr->irq = irq;
+	dr->dev_id = dev_id;
+	devres_add(dev, dr);
+
+	return 0;
+}
+EXPORT_SYMBOL(devm_request_any_context_irq);
+
+/**
  *	devm_free_irq - free an interrupt
  *	@dev: device to free interrupt for
  *	@irq: Interrupt line to free
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 6/7] Input: pmic8xxx-keypad - Migrate to devm_* APIs
  2013-12-10 23:43 [PATCH 0/7] Use regmap+devm in pm8xxx input drivers Stephen Boyd
                   ` (4 preceding siblings ...)
  2013-12-10 23:43 ` [PATCH 5/7] genirq: Add devm_request_any_context_irq() Stephen Boyd
@ 2013-12-10 23:43 ` Stephen Boyd
  2013-12-16 15:37   ` Dmitry Torokhov
  2013-12-10 23:43 ` [PATCH 7/7] Input: pmic8xxx-keypad - Migrate to regmap APIs Stephen Boyd
  6 siblings, 1 reply; 18+ messages in thread
From: Stephen Boyd @ 2013-12-10 23:43 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, linux-input

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

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/input/keyboard/pmic8xxx-keypad.c | 62 +++++++++-----------------------
 1 file changed, 17 insertions(+), 45 deletions(-)

diff --git a/drivers/input/keyboard/pmic8xxx-keypad.c b/drivers/input/keyboard/pmic8xxx-keypad.c
index 2c9f19a..4e6bfbf 100644
--- a/drivers/input/keyboard/pmic8xxx-keypad.c
+++ b/drivers/input/keyboard/pmic8xxx-keypad.c
@@ -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,
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 7/7] Input: pmic8xxx-keypad - Migrate to regmap APIs
  2013-12-10 23:43 [PATCH 0/7] Use regmap+devm in pm8xxx input drivers Stephen Boyd
                   ` (5 preceding siblings ...)
  2013-12-10 23:43 ` [PATCH 6/7] Input: pmic8xxx-keypad - Migrate to devm_* APIs Stephen Boyd
@ 2013-12-10 23:43 ` Stephen Boyd
  6 siblings, 0 replies; 18+ messages in thread
From: Stephen Boyd @ 2013-12-10 23:43 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, linux-input

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.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/input/keyboard/pmic8xxx-keypad.c | 81 ++++++++++++--------------------
 1 file changed, 29 insertions(+), 52 deletions(-)

diff --git a/drivers/input/keyboard/pmic8xxx-keypad.c b/drivers/input/keyboard/pmic8xxx-keypad.c
index 4e6bfbf..c6d3d21 100644
--- a/drivers/input/keyboard/pmic8xxx-keypad.c
+++ b/drivers/input/keyboard/pmic8xxx-keypad.c
@@ -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;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 5/7] genirq: Add devm_request_any_context_irq()
  2013-12-10 23:43 ` [PATCH 5/7] genirq: Add devm_request_any_context_irq() Stephen Boyd
@ 2013-12-15 10:42   ` Dmitry Torokhov
  0 siblings, 0 replies; 18+ messages in thread
From: Dmitry Torokhov @ 2013-12-15 10:42 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, linux-input, Stephen Boyd

On Tue, Dec 10, 2013 at 03:43:14PM -0800, Stephen Boyd wrote:
> Some drivers use request_any_context_irq() but there isn't a
> devm_* function for it. Add one so that these drivers don't need
> to explicitly free the irq on driver detach.
> 
> Cc: Thomas Gleixner <tglx@linutronix.de>

Thomas, would it be OK with me picking this patch through my tree if you
do not have issues with it?

Thanks.

> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  include/linux/interrupt.h |  5 +++++
>  kernel/irq/devres.c       | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 50 insertions(+)
> 
> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
> index c9e831d..8334f9b 100644
> --- a/include/linux/interrupt.h
> +++ b/include/linux/interrupt.h
> @@ -160,6 +160,11 @@ devm_request_irq(struct device *dev, unsigned int irq, irq_handler_t handler,
>  					 devname, dev_id);
>  }
>  
> +extern int __must_check
> +devm_request_any_context_irq(struct device *dev, unsigned int irq,
> +		 irq_handler_t handler, unsigned long irqflags,
> +		 const char *devname, void *dev_id);
> +
>  extern void devm_free_irq(struct device *dev, unsigned int irq, void *dev_id);
>  
>  /*
> diff --git a/kernel/irq/devres.c b/kernel/irq/devres.c
> index bd8e788..f71e9ff 100644
> --- a/kernel/irq/devres.c
> +++ b/kernel/irq/devres.c
> @@ -73,6 +73,51 @@ int devm_request_threaded_irq(struct device *dev, unsigned int irq,
>  EXPORT_SYMBOL(devm_request_threaded_irq);
>  
>  /**
> + *	devm_request_any_context_irq - allocate an interrupt line for a managed device
> + *	@dev: device to request interrupt for
> + *	@irq: Interrupt line to allocate
> + *	@handler: Function to be called when the IRQ occurs
> + *	@thread_fn: function to be called in a threaded interrupt context. NULL
> + *		    for devices which handle everything in @handler
> + *	@irqflags: Interrupt type flags
> + *	@devname: An ascii name for the claiming device
> + *	@dev_id: A cookie passed back to the handler function
> + *
> + *	Except for the extra @dev argument, this function takes the
> + *	same arguments and performs the same function as
> + *	request_any_context_irq().  IRQs requested with this function will be
> + *	automatically freed on driver detach.
> + *
> + *	If an IRQ allocated with this function needs to be freed
> + *	separately, devm_free_irq() must be used.
> + */
> +int devm_request_any_context_irq(struct device *dev, unsigned int irq,
> +			      irq_handler_t handler, unsigned long irqflags,
> +			      const char *devname, void *dev_id)
> +{
> +	struct irq_devres *dr;
> +	int rc;
> +
> +	dr = devres_alloc(devm_irq_release, sizeof(struct irq_devres),
> +			  GFP_KERNEL);
> +	if (!dr)
> +		return -ENOMEM;
> +
> +	rc = request_any_context_irq(irq, handler, irqflags, devname, dev_id);
> +	if (rc) {
> +		devres_free(dr);
> +		return rc;
> +	}
> +
> +	dr->irq = irq;
> +	dr->dev_id = dev_id;
> +	devres_add(dev, dr);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(devm_request_any_context_irq);
> +
> +/**
>   *	devm_free_irq - free an interrupt
>   *	@dev: device to free interrupt for
>   *	@irq: Interrupt line to free
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation
> 

-- 
Dmitry

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/7] Input: pmic8xxx-pwrkey - Pass input device directly to interrupt
  2013-12-10 23:43 ` [PATCH 1/7] Input: pmic8xxx-pwrkey - Pass input device directly to interrupt Stephen Boyd
@ 2013-12-15 11:21   ` Dmitry Torokhov
  2013-12-16 19:59     ` Stephen Boyd
  0 siblings, 1 reply; 18+ messages in thread
From: Dmitry Torokhov @ 2013-12-15 11:21 UTC (permalink / raw)
  To: Stephen Boyd; +Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, linux-input

Hi Stephen,

On Tue, Dec 10, 2013 at 03:43:10PM -0800, Stephen Boyd wrote:
> Instead of passing the pointer to the container structure just
> pass the input device here. This saves a dereference in the fast
> path.
> 
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  drivers/input/misc/pmic8xxx-pwrkey.c | 22 ++++++++--------------
>  1 file changed, 8 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/input/misc/pmic8xxx-pwrkey.c b/drivers/input/misc/pmic8xxx-pwrkey.c
> index ce3c426..233b216 100644
> --- a/drivers/input/misc/pmic8xxx-pwrkey.c
> +++ b/drivers/input/misc/pmic8xxx-pwrkey.c
> @@ -32,26 +32,21 @@
>   * @key_press_irq: key press irq number
>   */
>  struct pmic8xxx_pwrkey {
> -	struct input_dev *pwr;

This causes compile errors as you need pwrkey->pwr in remove() to
unregister input device. I'll fix it up.

>  	int key_press_irq;
>  };
>  
> -static irqreturn_t pwrkey_press_irq(int irq, void *_pwrkey)
> +static irqreturn_t pwrkey_press_irq(int irq, void *pwr)
>  {
> -	struct pmic8xxx_pwrkey *pwrkey = _pwrkey;
> -
> -	input_report_key(pwrkey->pwr, KEY_POWER, 1);
> -	input_sync(pwrkey->pwr);
> +	input_report_key(pwr, KEY_POWER, 1);
> +	input_sync(pwr);
>  
>  	return IRQ_HANDLED;
>  }
>  
> -static irqreturn_t pwrkey_release_irq(int irq, void *_pwrkey)
> +static irqreturn_t pwrkey_release_irq(int irq, void *pwr)
>  {
> -	struct pmic8xxx_pwrkey *pwrkey = _pwrkey;
> -
> -	input_report_key(pwrkey->pwr, KEY_POWER, 0);
> -	input_sync(pwrkey->pwr);
> +	input_report_key(pwr, KEY_POWER, 0);
> +	input_sync(pwr);
>  
>  	return IRQ_HANDLED;
>  }
> @@ -147,12 +142,11 @@ static int pmic8xxx_pwrkey_probe(struct platform_device *pdev)
>  	}
>  
>  	pwrkey->key_press_irq = key_press_irq;
> -	pwrkey->pwr = pwr;
>  
>  	platform_set_drvdata(pdev, pwrkey);
>  
>  	err = devm_request_irq(&pdev->dev, key_press_irq, pwrkey_press_irq,
> -		IRQF_TRIGGER_RISING, "pmic8xxx_pwrkey_press", pwrkey);
> +		IRQF_TRIGGER_RISING, "pmic8xxx_pwrkey_press", pwr);
>  	if (err < 0) {
>  		dev_dbg(&pdev->dev, "Can't get %d IRQ for pwrkey: %d\n",
>  				 key_press_irq, err);
> @@ -160,7 +154,7 @@ static int pmic8xxx_pwrkey_probe(struct platform_device *pdev)
>  	}
>  
>  	err = devm_request_irq(&pdev->dev, key_release_irq, pwrkey_release_irq,
> -		 IRQF_TRIGGER_RISING, "pmic8xxx_pwrkey_release", pwrkey);
> +		 IRQF_TRIGGER_RISING, "pmic8xxx_pwrkey_release", pwr);
>  	if (err < 0) {
>  		dev_dbg(&pdev->dev, "Can't get %d IRQ for pwrkey: %d\n",
>  				 key_release_irq, err);
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation
> 

-- 
Dmitry

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/7] Input: pmic8xxx-pwrkey - Migrate to regmap APIs
  2013-12-10 23:43 ` [PATCH 2/7] Input: pmic8xxx-pwrkey - Migrate to regmap APIs Stephen Boyd
@ 2013-12-15 11:33   ` Dmitry Torokhov
  2014-01-02 18:49     ` Mark Brown
  2014-01-21  9:34     ` bug fix for mmc queue.c Wang, Yalin
  0 siblings, 2 replies; 18+ messages in thread
From: Dmitry Torokhov @ 2013-12-15 11:33 UTC (permalink / raw)
  To: Stephen Boyd; +Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, linux-input

On Tue, Dec 10, 2013 at 03:43:11PM -0800, Stephen Boyd wrote:
> 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.
> 
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  drivers/input/misc/pmic8xxx-pwrkey.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/input/misc/pmic8xxx-pwrkey.c b/drivers/input/misc/pmic8xxx-pwrkey.c
> index 233b216..a4de105 100644
> --- a/drivers/input/misc/pmic8xxx-pwrkey.c
> +++ b/drivers/input/misc/pmic8xxx-pwrkey.c
> @@ -18,9 +18,9 @@
>  #include <linux/input.h>
>  #include <linux/interrupt.h>
>  #include <linux/platform_device.h>
> +#include <linux/regmap.h>
>  #include <linux/log2.h>
>  
> -#include <linux/mfd/pm8xxx/core.h>
>  #include <linux/input/pmic8xxx-pwrkey.h>
>  
>  #define PON_CNTL_1 0x1C
> @@ -83,7 +83,8 @@ static int pmic8xxx_pwrkey_probe(struct platform_device *pdev)
>  	int key_press_irq = platform_get_irq(pdev, 1);
>  	int err;
>  	unsigned int delay;
> -	u8 pon_cntl;
> +	unsigned int pon_cntl;
> +	struct regmap *regmap;
>  	struct pmic8xxx_pwrkey *pwrkey;
>  	const struct pm8xxx_pwrkey_platform_data *pdata =
>  					dev_get_platdata(&pdev->dev);
> @@ -108,6 +109,10 @@ static int pmic8xxx_pwrkey_probe(struct platform_device *pdev)
>  		err = -ENOMEM;
>  	}
>  
> +	regmap = dev_get_regmap(pdev->dev.parent, NULL);
> +	if (!regmap)
> +		return -ENODEV;

And you are leaking memory here...

> +
>  	input_set_capability(pwr, EV_KEY, KEY_POWER);
>  
>  	pwr->name = "pmic8xxx_pwrkey";
> @@ -116,7 +121,7 @@ static int pmic8xxx_pwrkey_probe(struct platform_device *pdev)
>  	delay = (pdata->kpd_trigger_delay_us << 10) / USEC_PER_SEC;
>  	delay = 1 + ilog2(delay);
>  
> -	err = pm8xxx_readb(pdev->dev.parent, PON_CNTL_1, &pon_cntl);
> +	err = regmap_read(regmap, PON_CNTL_1, &pon_cntl);
>  	if (err < 0) {
>  		dev_err(&pdev->dev, "failed reading PON_CNTL_1 err=%d\n", err);
>  		return err;
> @@ -129,7 +134,7 @@ static int pmic8xxx_pwrkey_probe(struct platform_device *pdev)
>  	else
>  		pon_cntl &= ~PON_CNTL_PULL_UP;
>  
> -	err = pm8xxx_writeb(pdev->dev.parent, PON_CNTL_1, pon_cntl);
> +	err = regmap_write(regmap, PON_CNTL_1, pon_cntl);
>  	if (err < 0) {
>  		dev_err(&pdev->dev, "failed writing PON_CNTL_1 err=%d\n", err);
>  		return err;
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation
> 

-- 
Dmitry

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 6/7] Input: pmic8xxx-keypad - Migrate to devm_* APIs
  2013-12-10 23:43 ` [PATCH 6/7] Input: pmic8xxx-keypad - Migrate to devm_* APIs Stephen Boyd
@ 2013-12-16 15:37   ` Dmitry Torokhov
  2013-12-17  2:01     ` spamassassin system account
  0 siblings, 1 reply; 18+ messages in thread
From: Dmitry Torokhov @ 2013-12-16 15:37 UTC (permalink / raw)
  To: Stephen Boyd; +Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, linux-input

On Tue, Dec 10, 2013 at 03:43:15PM -0800, Stephen Boyd wrote:
> Simplify the error paths and reduce the lines of code in this
> driver by using the devm_* APIs.
> 
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  drivers/input/keyboard/pmic8xxx-keypad.c | 62 +++++++++-----------------------
>  1 file changed, 17 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/input/keyboard/pmic8xxx-keypad.c b/drivers/input/keyboard/pmic8xxx-keypad.c
> index 2c9f19a..4e6bfbf 100644
> --- a/drivers/input/keyboard/pmic8xxx-keypad.c
> +++ b/drivers/input/keyboard/pmic8xxx-keypad.c
> @@ -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);

Why are we removing restoring wakeup capable state?

> -	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,
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation
> 

-- 
Dmitry

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/7] Input: pmic8xxx-pwrkey - Pass input device directly to interrupt
  2013-12-15 11:21   ` Dmitry Torokhov
@ 2013-12-16 19:59     ` Stephen Boyd
  0 siblings, 0 replies; 18+ messages in thread
From: Stephen Boyd @ 2013-12-16 19:59 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, linux-input

On 12/15, Dmitry Torokhov wrote:
> Hi Stephen,
> 
> On Tue, Dec 10, 2013 at 03:43:10PM -0800, Stephen Boyd wrote:
> > Instead of passing the pointer to the container structure just
> > pass the input device here. This saves a dereference in the fast
> > path.
> > 
> > Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> > ---
> >  drivers/input/misc/pmic8xxx-pwrkey.c | 22 ++++++++--------------
> >  1 file changed, 8 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/input/misc/pmic8xxx-pwrkey.c b/drivers/input/misc/pmic8xxx-pwrkey.c
> > index ce3c426..233b216 100644
> > --- a/drivers/input/misc/pmic8xxx-pwrkey.c
> > +++ b/drivers/input/misc/pmic8xxx-pwrkey.c
> > @@ -32,26 +32,21 @@
> >   * @key_press_irq: key press irq number
> >   */
> >  struct pmic8xxx_pwrkey {
> > -	struct input_dev *pwr;
> 
> This causes compile errors as you need pwrkey->pwr in remove() to
> unregister input device. I'll fix it up.
> 

Ah sorry. I forgot to send the patch before this that converts it
to devm.

> >  	err = devm_request_irq(&pdev->dev, key_press_irq, pwrkey_press_irq,
> > -		IRQF_TRIGGER_RISING, "pmic8xxx_pwrkey_press", pwrkey);
> > +		IRQF_TRIGGER_RISING, "pmic8xxx_pwrkey_press", pwr);

As you can see here because this driver isn't currently using
devm_request_irq().

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 6/7] Input: pmic8xxx-keypad - Migrate to devm_* APIs
  2013-12-16 15:37   ` Dmitry Torokhov
@ 2013-12-17  2:01     ` spamassassin system account
  0 siblings, 0 replies; 18+ messages in thread
From: spamassassin system account @ 2013-12-17  2:01 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, linux-input

On 12/16, Dmitry Torokhov wrote:
> On Tue, Dec 10, 2013 at 03:43:15PM -0800, Stephen Boyd wrote:
> > -
> > -static int pmic8xxx_kp_remove(struct platform_device *pdev)
> > -{
> > -	struct pmic8xxx_kp *kp = platform_get_drvdata(pdev);
> > -
> > -	device_init_wakeup(&pdev->dev, 0);
> 
> Why are we removing restoring wakeup capable state?
> 

It's the only thing blocking removal of the remove callback and
as the driver is being unbound it didn't seem like we cared about
the state of the wakeup of the device. I greped the kernel tree
and I couldn't see a consistent pattern where driver probe was
setting the flag and driver remove was clearing it. Do we need to
keep it?

> > -	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

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/7] Input: pmic8xxx-pwrkey - Migrate to regmap APIs
  2013-12-15 11:33   ` Dmitry Torokhov
@ 2014-01-02 18:49     ` Mark Brown
  2014-01-02 19:17       ` Dmitry Torokhov
  2014-01-21  9:34     ` bug fix for mmc queue.c Wang, Yalin
  1 sibling, 1 reply; 18+ messages in thread
From: Mark Brown @ 2014-01-02 18:49 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Stephen Boyd, linux-arm-msm, linux-kernel, linux-arm-kernel, linux-input

[-- Attachment #1: Type: text/plain, Size: 654 bytes --]

On Sun, Dec 15, 2013 at 03:33:37AM -0800, Dmitry Torokhov wrote:
> On Tue, Dec 10, 2013 at 03:43:11PM -0800, Stephen Boyd wrote:

> > +	regmap = dev_get_regmap(pdev->dev.parent, NULL);
> > +	if (!regmap)
> > +		return -ENODEV;

> And you are leaking memory here...

He's not, dev_get_regmap() just gets a pointer to an existing regmap -
no reference is taken and nothing is allocated.  It's a helper that's
mainly there so that generic code can be written without needing the
regmap to be passed around.  The caller is responsible for ensuring that
it will stick around for as long as it's used (generally by having it
lifetime managed with the device).

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/7] Input: pmic8xxx-pwrkey - Migrate to regmap APIs
  2014-01-02 18:49     ` Mark Brown
@ 2014-01-02 19:17       ` Dmitry Torokhov
  2014-01-03  0:50         ` Mark Brown
  0 siblings, 1 reply; 18+ messages in thread
From: Dmitry Torokhov @ 2014-01-02 19:17 UTC (permalink / raw)
  To: Mark Brown
  Cc: linux-arm-msm, Stephen Boyd, linux-kernel, linux-arm-kernel, linux-input

On Thu, Jan 02, 2014 at 06:49:59PM +0000, Mark Brown wrote:
> On Sun, Dec 15, 2013 at 03:33:37AM -0800, Dmitry Torokhov wrote:
> > On Tue, Dec 10, 2013 at 03:43:11PM -0800, Stephen Boyd wrote:
> 
> > > +	regmap = dev_get_regmap(pdev->dev.parent, NULL);
> > > +	if (!regmap)
> > > +		return -ENODEV;
> 
> > And you are leaking memory here...
> 
> He's not, dev_get_regmap() just gets a pointer to an existing regmap -
> no reference is taken and nothing is allocated.  It's a helper that's
> mainly there so that generic code can be written without needing the
> regmap to be passed around.  The caller is responsible for ensuring that
> it will stick around for as long as it's used (generally by having it
> lifetime managed with the device).

I was not talking about data returned by dev_get_regmap() but all other
memory that was allocated before as this was pre devm conversion of the
driver.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/7] Input: pmic8xxx-pwrkey - Migrate to regmap APIs
  2014-01-02 19:17       ` Dmitry Torokhov
@ 2014-01-03  0:50         ` Mark Brown
  0 siblings, 0 replies; 18+ messages in thread
From: Mark Brown @ 2014-01-03  0:50 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Stephen Boyd, linux-arm-msm, linux-kernel, linux-arm-kernel, linux-input

[-- Attachment #1: Type: text/plain, Size: 772 bytes --]

On Thu, Jan 02, 2014 at 11:17:57AM -0800, Dmitry Torokhov wrote:
> On Thu, Jan 02, 2014 at 06:49:59PM +0000, Mark Brown wrote:
> > On Sun, Dec 15, 2013 at 03:33:37AM -0800, Dmitry Torokhov wrote:

> > > > +	regmap = dev_get_regmap(pdev->dev.parent, NULL);
> > > > +	if (!regmap)
> > > > +		return -ENODEV;

> > > And you are leaking memory here...

> > He's not, dev_get_regmap() just gets a pointer to an existing regmap -
> > no reference is taken and nothing is allocated.  It's a helper that's

> I was not talking about data returned by dev_get_regmap() but all other
> memory that was allocated before as this was pre devm conversion of the
> driver.

Ah, OK - I thought you meant that as that was the code immediately prior
to your reply.  Sorry for the confusion.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug fix for mmc  queue.c
  2013-12-15 11:33   ` Dmitry Torokhov
  2014-01-02 18:49     ` Mark Brown
@ 2014-01-21  9:34     ` Wang, Yalin
  1 sibling, 0 replies; 18+ messages in thread
From: Wang, Yalin @ 2014-01-21  9:34 UTC (permalink / raw)
  To: linux-kernel, linux-arm-msm, linux-arm-kernel, linux-input
  Cc: Duan, Yongjian, Hang, Xuliang (EXT), Zhang, Bojie

[-- Attachment #1: Type: text/plain, Size: 5851 bytes --]

Hi  

We encounter a problem when use sdcard ,
The driver probe will failed like this :

<6>[  121.644102] mmc0: mmc_start_bkops: Starting bkops
<6>[  133.039845] mmc0: mmc_start_bkops: raw_bkops_status=0x2, from_exception=0
<6>[  133.039888] mmc0: mmc_start_bkops: Starting bkops
<6>[  147.931642] mmc0: mmc_start_bkops: raw_bkops_status=0x2, from_exception=1
<6>[  148.634009] mmc0: mmc_start_bkops: Starting bkops
<6>[  164.279748] mmc1: slot status change detected (0 -> 1), GPIO_ACTIVE_LOW
<6>[  164.612904] mmc1: new high speed SD card at address 1234
<4>[  164.620819] kworker/u:29: page allocation failure: order:5, mode:0x40d0
<6>[  164.629745] [<c010c514>] (unwind_backtrace+0x0/0x11c) from [<c0217e78>] (warn_alloc_failed+0x104/0x130)
<6>[  164.629789] [<c0217e78>] (warn_alloc_failed+0x104/0x130) from [<c021b39c>] (__alloc_pages_nodemask+0x7d4/0x8f4)
<6>[  164.629828] [<c021b39c>] (__alloc_pages_nodemask+0x7d4/0x8f4) from [<c021b510>] (__get_free_pages+0x10/0x24)
<6>[  164.629867] [<c021b510>] (__get_free_pages+0x10/0x24) from [<c0246630>] (kmalloc_order_trace+0x20/0xe0)
<6>[  164.629904] [<c0246630>] (kmalloc_order_trace+0x20/0xe0) from [<c0249550>] (__kmalloc+0x30/0x270)
<6>[  164.629939] [<c0249550>] (__kmalloc+0x30/0x270) from [<c061cdbc>] (mmc_alloc_sg+0x18/0x40)
<6>[  164.629974] [<c061cdbc>] (mmc_alloc_sg+0x18/0x40) from [<c061d524>] (mmc_init_queue+0x418/0x4fc)
<6>[  164.630008] [<c061d524>] (mmc_init_queue+0x418/0x4fc) from [<c06195a0>] (mmc_blk_alloc_req+0x18c/0x3d0)
<6>[  164.630043] [<c06195a0>] (mmc_blk_alloc_req+0x18c/0x3d0) from [<c061b2a8>] (mmc_blk_probe+0x74/0x2c0)
<6>[  164.630078] [<c061b2a8>] (mmc_blk_probe+0x74/0x2c0) from [<c060d68c>] (mmc_bus_probe+0x14/0x18)
<6>[  164.630115] [<c060d68c>] (mmc_bus_probe+0x14/0x18) from [<c045a928>] (driver_probe_device+0x134/0x334)
<6>[  164.630154] [<c045a928>] (driver_probe_device+0x134/0x334) from [<c0458e0c>] (bus_for_each_drv+0x48/0x8c)
<6>[  164.630190] [<c0458e0c>] (bus_for_each_drv+0x48/0x8c) from [<c045a77c>] (device_attach+0x7c/0xa0)
<6>[  164.630222] [<c045a77c>] (device_attach+0x7c/0xa0) from [<c0459ca0>] (bus_probe_device+0x28/0x98)
<6>[  164.630257] [<c0459ca0>] (bus_probe_device+0x28/0x98) from [<c04583f0>] (device_add+0x3f4/0x5a8)
<6>[  164.630292] [<c04583f0>] (device_add+0x3f4/0x5a8) from [<c060dd7c>] (mmc_add_card+0x1f0/0x2e8)
<6>[  164.630329] [<c060dd7c>] (mmc_add_card+0x1f0/0x2e8) from [<c0613990>] (mmc_attach_sd+0x234/0x278)
<6>[  164.630364] [<c0613990>] (mmc_attach_sd+0x234/0x278) from [<c060cab8>] (mmc_rescan+0x24c/0x2cc)
<6>[  164.630400] [<c060cab8>] (mmc_rescan+0x24c/0x2cc) from [<c01a2a40>] (process_one_work+0x200/0x400)
<6>[  164.630438] [<c01a2a40>] (process_one_work+0x200/0x400) from [<c01a2df0>] (worker_thread+0x184/0x2a4)
<6>[  164.630474] [<c01a2df0>] (worker_thread+0x184/0x2a4) from [<c01a74ac>] (kthread+0x80/0x90)
<6>[  164.630510] [<c01a74ac>] (kthread+0x80/0x90) from [<c0106aec>] (kernel_thread_exit+0x0/0x8)
<6>[  164.630529] Mem-info:
<6>[  164.630542] Normal per-cpu:
<6>[  164.630559] CPU    0: hi:  186, btch:  31 usd:   0
<6>[  164.630573] HighMem per-cpu:
<6>[  164.630588] CPU    0: hi:   90, btch:  15 usd:   0
<6>[  164.630624] active_anon:96301 inactive_anon:586 isolated_anon:4
<6>[  164.630633]  active_file:26021 inactive_file:25748 isolated_file:0
<6>[  164.630641]  unevictable:694 dirty:4 writeback:0 unstable:0
<6>[  164.630649]  free:5131 slab_reclaimable:3282 slab_unreclaimable:5632
<6>[  164.630658]  mapped:36273 shmem:649 pagetables:3794 bounce:0
<6>[  164.630666]  free_cma:139
<6>[  164.630716] Normal free:18660kB min:3136kB low:3920kB high:4704kB active_anon:183052kB inactive_anon:1980kB active_file:98188kB inactive_file:98364kB unevictable:0kB isolated(anon):0kB isolated(file):0kB present:616020kB mlocked:0kB dirty:8kB writeback:0kB mapped:134632kB shmem:1992kB slab_reclaimable:13128kB slab_unreclaimable:22528kB kernel_stack:10528kB pagetables:15176kB unstable:0kB bounce:0kB free_cma:112kB writeback_tmp:0kB pages_scanned:0 all_unreclaimable? no
<6>[  164.630769] lowmem_reserve[]: 0 1691 1691
<6>[  164.630831] HighMem free:1864kB min:208kB low:480kB high:756kB active_anon:202152kB inactive_anon:364kB active_file:5896kB inactive_file:4628kB unevictable:2776kB isolated(anon):16kB isolated(file):0kB present:216488kB mlocked:0kB dirty:8kB writeback:0kB mapped:10460kB shmem:604kB slab_reclaimable:0kB slab_unreclaimable:0kB kernel_stack:0kB pagetables:0kB unstable:0kB bounce:0kB free_cma:444kB writeback_tmp:0kB pages_scanned:0 all_unreclaimable? no
<6>[  164.630883] lowmem_reserve[]: 0 0 0
<6>[  164.630913] Normal: 1215*4kB (UEMC) 421*8kB (UEMC) 396*16kB (UM) 114*32kB (UM) 7*64kB (UM) 0*128kB 0*256kB 0*512kB 0*1024kB 0*2048kB 0*4096kB = 18660kB
<6>[  164.631034] HighMem: 190*4kB (UMRC) 26*8kB (UMRC) 9*16kB (UMC) 1*32kB (C) 0*64kB 1*128kB (C) 0*256kB 0*512kB 0*1024kB 0*2048kB 0*4096kB = 1272kB
<6>[  164.631154] 53112 total pagecache pages
<6>[  164.631167] 0 pages in swap cache
<6>[  164.631183] Swap cache stats: add 0, delete 0, find 0/0
<6>[  164.631197] Free swap  = 0kB
<6>[  164.631209] Total swap = 0kB
<6>[  164.648069] 230912 pages of RAM
<6>[  164.648087] 12304 free pages
<6>[  164.648099] 30954 reserved pages
<6>[  164.648111] 5947 slab pages
<6>[  164.648123] 358720 pages shared
<6>[  164.648135] 0 pages swap cached
<4>[  164.648468] mmcblk: probe of mmc1:1234 failed with error -12



The reason is that mmc_alloc_sg() function will use kmalloc to init 
Scatterlist , in this issue, it is 32KB memory , the kmalloc will fail if there is
Not enough low memory ,  instead, we can use vmalloc if the request memory is larger than
A PAGE_SIZE , we make a patch for this problems  (see attachment patch.diff) ,
Hope can  be merged into mainline .


Thanks 





[-- Attachment #2: patch.diff --]
[-- Type: application/octet-stream, Size: 2508 bytes --]

diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
index cef1a41..839b506 100644
--- a/drivers/mmc/card/queue.c
+++ b/drivers/mmc/card/queue.c
@@ -15,6 +15,7 @@
 #include <linux/freezer.h>
 #include <linux/kthread.h>
 #include <linux/scatterlist.h>
+#include <linux/vmalloc.h>
 
 #include <linux/mmc/card.h>
 #include <linux/mmc/host.h>
@@ -198,8 +199,13 @@ static void mmc_urgent_request(struct request_queue *q)
 static struct scatterlist *mmc_alloc_sg(int sg_len, int *err)
 {
 	struct scatterlist *sg;
+	size_t size = sizeof(struct scatterlist)*sg_len;
+
+	if (size > PAGE_SIZE)
+		sg = vmalloc(size);
+	else
+		sg = kmalloc(size, GFP_KERNEL);
 
-	sg = kmalloc(sizeof(struct scatterlist)*sg_len, GFP_KERNEL);
 	if (!sg)
 		*err = -ENOMEM;
 	else {
@@ -210,6 +216,14 @@ static struct scatterlist *mmc_alloc_sg(int sg_len, int *err)
 	return sg;
 }
 
+static void mmc_alloc_free(const void *addr)
+{
+	if (is_vmalloc_addr(addr))
+		vfree(addr);
+	else
+		kfree(addr);
+}
+
 static void mmc_queue_setup_discard(struct request_queue *q,
 				    struct mmc_card *card)
 {
@@ -397,20 +411,20 @@ success:
 
 	return 0;
  free_bounce_sg:
-	kfree(mqrq_cur->bounce_sg);
+	mmc_alloc_free(mqrq_cur->bounce_sg);
 	mqrq_cur->bounce_sg = NULL;
-	kfree(mqrq_prev->bounce_sg);
+	mmc_alloc_free(mqrq_prev->bounce_sg);
 	mqrq_prev->bounce_sg = NULL;
 
  cleanup_queue:
-	kfree(mqrq_cur->sg);
+	mmc_alloc_free(mqrq_cur->sg);
 	mqrq_cur->sg = NULL;
-	kfree(mqrq_cur->bounce_buf);
+	mmc_alloc_free(mqrq_cur->bounce_buf);
 	mqrq_cur->bounce_buf = NULL;
 
-	kfree(mqrq_prev->sg);
+	mmc_alloc_free(mqrq_prev->sg);
 	mqrq_prev->sg = NULL;
-	kfree(mqrq_prev->bounce_buf);
+	mmc_alloc_free(mqrq_prev->bounce_buf);
 	mqrq_prev->bounce_buf = NULL;
 
 	blk_cleanup_queue(mq->queue);
@@ -436,22 +450,22 @@ void mmc_cleanup_queue(struct mmc_queue *mq)
 	blk_start_queue(q);
 	spin_unlock_irqrestore(q->queue_lock, flags);
 
-	kfree(mqrq_cur->bounce_sg);
+	mmc_alloc_free(mqrq_cur->bounce_sg);
 	mqrq_cur->bounce_sg = NULL;
 
-	kfree(mqrq_cur->sg);
+	mmc_alloc_free(mqrq_cur->sg);
 	mqrq_cur->sg = NULL;
 
-	kfree(mqrq_cur->bounce_buf);
+	mmc_alloc_free(mqrq_cur->bounce_buf);
 	mqrq_cur->bounce_buf = NULL;
 
-	kfree(mqrq_prev->bounce_sg);
+	mmc_alloc_free(mqrq_prev->bounce_sg);
 	mqrq_prev->bounce_sg = NULL;
 
-	kfree(mqrq_prev->sg);
+	mmc_alloc_free(mqrq_prev->sg);
 	mqrq_prev->sg = NULL;
 
-	kfree(mqrq_prev->bounce_buf);
+	mmc_alloc_free(mqrq_prev->bounce_buf);
 	mqrq_prev->bounce_buf = NULL;
 
 	mq->card = NULL;

^ permalink raw reply related	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2014-01-21  9:34 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-12-10 23:43 [PATCH 0/7] Use regmap+devm in pm8xxx input drivers Stephen Boyd
2013-12-10 23:43 ` [PATCH 1/7] Input: pmic8xxx-pwrkey - Pass input device directly to interrupt Stephen Boyd
2013-12-15 11:21   ` Dmitry Torokhov
2013-12-16 19:59     ` Stephen Boyd
2013-12-10 23:43 ` [PATCH 2/7] Input: pmic8xxx-pwrkey - Migrate to regmap APIs Stephen Boyd
2013-12-15 11:33   ` Dmitry Torokhov
2014-01-02 18:49     ` Mark Brown
2014-01-02 19:17       ` Dmitry Torokhov
2014-01-03  0:50         ` Mark Brown
2014-01-21  9:34     ` bug fix for mmc queue.c Wang, Yalin
2013-12-10 23:43 ` [PATCH 3/7] Input: pm8xxx-vibrator - Migrate to devm_* APIs Stephen Boyd
2013-12-10 23:43 ` [PATCH 4/7] Input: pm8xxx-vibrator - Migrate to regmap APIs Stephen Boyd
2013-12-10 23:43 ` [PATCH 5/7] genirq: Add devm_request_any_context_irq() Stephen Boyd
2013-12-15 10:42   ` Dmitry Torokhov
2013-12-10 23:43 ` [PATCH 6/7] Input: pmic8xxx-keypad - Migrate to devm_* APIs Stephen Boyd
2013-12-16 15:37   ` Dmitry Torokhov
2013-12-17  2:01     ` spamassassin system account
2013-12-10 23:43 ` [PATCH 7/7] Input: pmic8xxx-keypad - Migrate to regmap APIs Stephen Boyd

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).