diff mbox

[2/7] Input: pmic8xxx-pwrkey - Migrate to regmap APIs

Message ID 1386718996-3733-3-git-send-email-sboyd@codeaurora.org (mailing list archive)
State New, archived
Headers show

Commit Message

Stephen Boyd Dec. 10, 2013, 11:43 p.m. UTC 
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(-)

Comments

Dmitry Torokhov Dec. 15, 2013, 11:33 a.m. UTC | #1 
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
>
Mark Brown Jan. 2, 2014, 6:49 p.m. UTC | #2 
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).
Dmitry Torokhov Jan. 2, 2014, 7:17 p.m. UTC | #3 
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.
Mark Brown Jan. 3, 2014, 12:50 a.m. UTC | #4 
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.
Wang, Yalin Jan. 21, 2014, 9:34 a.m. UTC | #5 
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
diff mbox

Patch

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;