* [PATCH 1/6] regmap: cache: Sort include headers alphabetically
2014-10-09 9:02 [PATCH 0/6] regmap: cache: cleanup and fix bugs Xiubo Li
@ 2014-10-09 9:02 ` Xiubo Li
2014-10-13 10:39 ` Mark Brown
2014-10-09 9:02 ` [PATCH 2/6] regmap: cache: cleanup regcache_hw_init() Xiubo Li
` (4 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Xiubo Li @ 2014-10-09 9:02 UTC (permalink / raw)
To: broonie; +Cc: linux-kernel, dp, Xiubo Li
If the inlcude headers aren't sorted alphabetically, then the
logical choice is to append new ones, however that creates a
lot of potential for conflicts or duplicates because every change
will then add new includes in the same location.
Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
---
drivers/base/regmap/regcache-flat.c | 2 +-
drivers/base/regmap/regcache-lzo.c | 2 +-
drivers/base/regmap/regcache-rbtree.c | 4 ++--
drivers/base/regmap/regcache.c | 8 ++++----
4 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/base/regmap/regcache-flat.c b/drivers/base/regmap/regcache-flat.c
index d9762e4..0246f44 100644
--- a/drivers/base/regmap/regcache-flat.c
+++ b/drivers/base/regmap/regcache-flat.c
@@ -10,9 +10,9 @@
* published by the Free Software Foundation.
*/
-#include <linux/slab.h>
#include <linux/device.h>
#include <linux/seq_file.h>
+#include <linux/slab.h>
#include "internal.h"
diff --git a/drivers/base/regmap/regcache-lzo.c b/drivers/base/regmap/regcache-lzo.c
index e210a6d..2d53f6f 100644
--- a/drivers/base/regmap/regcache-lzo.c
+++ b/drivers/base/regmap/regcache-lzo.c
@@ -10,9 +10,9 @@
* published by the Free Software Foundation.
*/
-#include <linux/slab.h>
#include <linux/device.h>
#include <linux/lzo.h>
+#include <linux/slab.h>
#include "internal.h"
diff --git a/drivers/base/regmap/regcache-rbtree.c b/drivers/base/regmap/regcache-rbtree.c
index f3e8fe0..d453a2c 100644
--- a/drivers/base/regmap/regcache-rbtree.c
+++ b/drivers/base/regmap/regcache-rbtree.c
@@ -10,11 +10,11 @@
* published by the Free Software Foundation.
*/
-#include <linux/slab.h>
-#include <linux/device.h>
#include <linux/debugfs.h>
+#include <linux/device.h>
#include <linux/rbtree.h>
#include <linux/seq_file.h>
+#include <linux/slab.h>
#include "internal.h"
diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
index f1280dc..4896804 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -10,12 +10,12 @@
* published by the Free Software Foundation.
*/
-#include <linux/slab.h>
-#include <linux/export.h>
-#include <linux/device.h>
-#include <trace/events/regmap.h>
#include <linux/bsearch.h>
+#include <linux/device.h>
+#include <linux/export.h>
+#include <linux/slab.h>
#include <linux/sort.h>
+#include <trace/events/regmap.h>
#include "internal.h"
--
2.1.0.27.g96db324
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 1/6] regmap: cache: Sort include headers alphabetically
2014-10-09 9:02 ` [PATCH 1/6] regmap: cache: Sort include headers alphabetically Xiubo Li
@ 2014-10-13 10:39 ` Mark Brown
0 siblings, 0 replies; 15+ messages in thread
From: Mark Brown @ 2014-10-13 10:39 UTC (permalink / raw)
To: Xiubo Li; +Cc: linux-kernel, dp
[-- Attachment #1: Type: text/plain, Size: 322 bytes --]
On Thu, Oct 09, 2014 at 05:02:52PM +0800, Xiubo Li wrote:
> If the inlcude headers aren't sorted alphabetically, then the
> logical choice is to append new ones, however that creates a
> lot of potential for conflicts or duplicates because every change
> will then add new includes in the same location.
Applied, thanks.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/6] regmap: cache: cleanup regcache_hw_init().
2014-10-09 9:02 [PATCH 0/6] regmap: cache: cleanup and fix bugs Xiubo Li
2014-10-09 9:02 ` [PATCH 1/6] regmap: cache: Sort include headers alphabetically Xiubo Li
@ 2014-10-09 9:02 ` Xiubo Li
2014-10-13 10:47 ` Mark Brown
2014-10-09 9:02 ` [PATCH 3/6] regmap: cache: fix errno in regcache_hw_init() Xiubo Li
` (3 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Xiubo Li @ 2014-10-09 9:02 UTC (permalink / raw)
To: broonie; +Cc: linux-kernel, dp, Xiubo Li
Remove the redundant code for regmap cache.
Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
---
drivers/base/regmap/regcache.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
index 4896804..c13b821 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -57,12 +57,9 @@ static int regcache_hw_init(struct regmap *map)
}
/* calculate the size of reg_defaults */
- for (count = 0, i = 0; i < map->num_reg_defaults_raw; i++) {
- val = regcache_get_val(map, map->reg_defaults_raw, i);
- if (regmap_volatile(map, i * map->reg_stride))
- continue;
- count++;
- }
+ for (count = 0, i = 0; i < map->num_reg_defaults_raw; i++)
+ if (!regmap_volatile(map, i * map->reg_stride))
+ count++;
map->reg_defaults = kmalloc(count * sizeof(struct reg_default),
GFP_KERNEL);
--
2.1.0.27.g96db324
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 2/6] regmap: cache: cleanup regcache_hw_init().
2014-10-09 9:02 ` [PATCH 2/6] regmap: cache: cleanup regcache_hw_init() Xiubo Li
@ 2014-10-13 10:47 ` Mark Brown
2014-10-14 1:52 ` Li.Xiubo
0 siblings, 1 reply; 15+ messages in thread
From: Mark Brown @ 2014-10-13 10:47 UTC (permalink / raw)
To: Xiubo Li; +Cc: linux-kernel, dp
[-- Attachment #1: Type: text/plain, Size: 317 bytes --]
On Thu, Oct 09, 2014 at 05:02:53PM +0800, Xiubo Li wrote:
> Remove the redundant code for regmap cache.
Applied, thanks. With changes like this it is much better if the
changelog explains why the code is redundant - say in words that the
value used is never referenced and looking up the value has no side
effects.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [PATCH 2/6] regmap: cache: cleanup regcache_hw_init().
2014-10-13 10:47 ` Mark Brown
@ 2014-10-14 1:52 ` Li.Xiubo
0 siblings, 0 replies; 15+ messages in thread
From: Li.Xiubo @ 2014-10-14 1:52 UTC (permalink / raw)
To: Mark Brown; +Cc: linux-kernel, dp
Hi Mark,
> Subject: Re: [PATCH 2/6] regmap: cache: cleanup regcache_hw_init().
>
> On Thu, Oct 09, 2014 at 05:02:53PM +0800, Xiubo Li wrote:
> > Remove the redundant code for regmap cache.
>
> Applied, thanks. With changes like this it is much better if the
> changelog explains why the code is redundant - say in words that the
> value used is never referenced and looking up the value has no side
> effects.
Thanks verrrry much for your comment and your always patient guidance.
BRs
Xiubo
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/6] regmap: cache: fix errno in regcache_hw_init()
2014-10-09 9:02 [PATCH 0/6] regmap: cache: cleanup and fix bugs Xiubo Li
2014-10-09 9:02 ` [PATCH 1/6] regmap: cache: Sort include headers alphabetically Xiubo Li
2014-10-09 9:02 ` [PATCH 2/6] regmap: cache: cleanup regcache_hw_init() Xiubo Li
@ 2014-10-09 9:02 ` Xiubo Li
2014-10-13 10:48 ` Mark Brown
2014-10-09 9:02 ` [PATCH 4/6] regmap: cache: speed regcache_hw_init() up Xiubo Li
` (2 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Xiubo Li @ 2014-10-09 9:02 UTC (permalink / raw)
To: broonie; +Cc: linux-kernel, dp, Xiubo Li
When kmalloc() fails, we should return -ENOMEM.
Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
---
drivers/base/regmap/regcache.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
index c13b821..8a7c270 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -44,7 +44,7 @@ static int regcache_hw_init(struct regmap *map)
map->cache_bypass = 1;
tmp_buf = kmalloc(map->cache_size_raw, GFP_KERNEL);
if (!tmp_buf)
- return -EINVAL;
+ return -ENOMEM;
ret = regmap_raw_read(map, 0, tmp_buf,
map->num_reg_defaults_raw);
map->cache_bypass = cache_bypass;
--
2.1.0.27.g96db324
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 4/6] regmap: cache: speed regcache_hw_init() up.
2014-10-09 9:02 [PATCH 0/6] regmap: cache: cleanup and fix bugs Xiubo Li
` (2 preceding siblings ...)
2014-10-09 9:02 ` [PATCH 3/6] regmap: cache: fix errno in regcache_hw_init() Xiubo Li
@ 2014-10-09 9:02 ` Xiubo Li
2014-10-13 10:48 ` Mark Brown
2014-10-09 9:02 ` [PATCH 5/6] regmap: cache: use kmalloc_array instead of kmalloc Xiubo Li
2014-10-09 9:02 ` [PATCH 6/6] regmap: cache: Fix possible ZERO_SIZE_PTR pointer dereferencing error Xiubo Li
5 siblings, 1 reply; 15+ messages in thread
From: Xiubo Li @ 2014-10-09 9:02 UTC (permalink / raw)
To: broonie; +Cc: linux-kernel, dp, Xiubo Li
This may speed regcache_hw_init() up for some cases that there
has volatile registers.
Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
---
drivers/base/regmap/regcache.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
index 8a7c270..45ab909 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -71,9 +71,9 @@ static int regcache_hw_init(struct regmap *map)
/* fill the reg_defaults */
map->num_reg_defaults = count;
for (i = 0, j = 0; i < map->num_reg_defaults_raw; i++) {
- val = regcache_get_val(map, map->reg_defaults_raw, i);
if (regmap_volatile(map, i * map->reg_stride))
continue;
+ val = regcache_get_val(map, map->reg_defaults_raw, i);
map->reg_defaults[j].reg = i * map->reg_stride;
map->reg_defaults[j].def = val;
j++;
--
2.1.0.27.g96db324
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 5/6] regmap: cache: use kmalloc_array instead of kmalloc
2014-10-09 9:02 [PATCH 0/6] regmap: cache: cleanup and fix bugs Xiubo Li
` (3 preceding siblings ...)
2014-10-09 9:02 ` [PATCH 4/6] regmap: cache: speed regcache_hw_init() up Xiubo Li
@ 2014-10-09 9:02 ` Xiubo Li
2014-10-13 10:49 ` Mark Brown
2014-10-09 9:02 ` [PATCH 6/6] regmap: cache: Fix possible ZERO_SIZE_PTR pointer dereferencing error Xiubo Li
5 siblings, 1 reply; 15+ messages in thread
From: Xiubo Li @ 2014-10-09 9:02 UTC (permalink / raw)
To: broonie; +Cc: linux-kernel, dp, Xiubo Li
This patch fixes checkpatch.pl warning for regmap cache.
WARNING : prefer kmalloc_array over kmalloc with multiply
Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
---
drivers/base/regmap/regcache.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
index 45ab909..a06bcd2 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -61,8 +61,8 @@ static int regcache_hw_init(struct regmap *map)
if (!regmap_volatile(map, i * map->reg_stride))
count++;
- map->reg_defaults = kmalloc(count * sizeof(struct reg_default),
- GFP_KERNEL);
+ map->reg_defaults = kmalloc_array(count, sizeof(struct reg_default),
+ GFP_KERNEL);
if (!map->reg_defaults) {
ret = -ENOMEM;
goto err_free;
--
2.1.0.27.g96db324
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 6/6] regmap: cache: Fix possible ZERO_SIZE_PTR pointer dereferencing error.
2014-10-09 9:02 [PATCH 0/6] regmap: cache: cleanup and fix bugs Xiubo Li
` (4 preceding siblings ...)
2014-10-09 9:02 ` [PATCH 5/6] regmap: cache: use kmalloc_array instead of kmalloc Xiubo Li
@ 2014-10-09 9:02 ` Xiubo Li
2014-10-13 10:54 ` Mark Brown
5 siblings, 1 reply; 15+ messages in thread
From: Xiubo Li @ 2014-10-09 9:02 UTC (permalink / raw)
To: broonie; +Cc: linux-kernel, dp, Xiubo Li
When all the registers are volatile(unlikely, but logically and mostly
will happen for some 'device' who has very few registers), then the
count will be euqal to 0, then kmalloc() will return ZERO_SIZE_PTR,
which equals to ((void *)16).
So this patch fix this with just doing the zero check before calling
kmalloc(). If the count == 0, so we can make sure that all the registers
are volatile, so no cache is need.
Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
---
drivers/base/regmap/regcache.c | 50 ++++++++++++++++++++++++------------------
1 file changed, 29 insertions(+), 21 deletions(-)
diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
index a06bcd2..f373c35 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -36,6 +36,23 @@ static int regcache_hw_init(struct regmap *map)
if (!map->num_reg_defaults_raw)
return -EINVAL;
+ /* calculate the size of reg_defaults */
+ for (count = 0, i = 0; i < map->num_reg_defaults_raw; i++)
+ if (!regmap_volatile(map, i * map->reg_stride))
+ count++;
+
+ /* all registers are volatile, so just bypass */
+ if (!count) {
+ map->cache_bypass = true;
+ return 0;
+ }
+
+ map->num_reg_defaults = count;
+ map->reg_defaults = kmalloc_array(count, sizeof(struct reg_default),
+ GFP_KERNEL);
+ if (!map->reg_defaults)
+ return -ENOMEM;
+
if (!map->reg_defaults_raw) {
u32 cache_bypass = map->cache_bypass;
dev_warn(map->dev, "No cache defaults, reading back from HW\n");
@@ -43,33 +60,21 @@ static int regcache_hw_init(struct regmap *map)
/* Bypass the cache access till data read from HW*/
map->cache_bypass = 1;
tmp_buf = kmalloc(map->cache_size_raw, GFP_KERNEL);
- if (!tmp_buf)
- return -ENOMEM;
+ if (!tmp_buf) {
+ ret = -ENOMEM;
+ goto err_free;
+ }
ret = regmap_raw_read(map, 0, tmp_buf,
map->num_reg_defaults_raw);
map->cache_bypass = cache_bypass;
- if (ret < 0) {
- kfree(tmp_buf);
- return ret;
- }
+ if (ret < 0)
+ goto err_cache_free;
+
map->reg_defaults_raw = tmp_buf;
map->cache_free = 1;
}
- /* calculate the size of reg_defaults */
- for (count = 0, i = 0; i < map->num_reg_defaults_raw; i++)
- if (!regmap_volatile(map, i * map->reg_stride))
- count++;
-
- map->reg_defaults = kmalloc_array(count, sizeof(struct reg_default),
- GFP_KERNEL);
- if (!map->reg_defaults) {
- ret = -ENOMEM;
- goto err_free;
- }
-
/* fill the reg_defaults */
- map->num_reg_defaults = count;
for (i = 0, j = 0; i < map->num_reg_defaults_raw; i++) {
if (regmap_volatile(map, i * map->reg_stride))
continue;
@@ -81,9 +86,10 @@ static int regcache_hw_init(struct regmap *map)
return 0;
+err_cache_free:
+ kfree(tmp_buf);
err_free:
- if (map->cache_free)
- kfree(map->reg_defaults_raw);
+ kfree(map->reg_defaults);
return ret;
}
@@ -147,6 +153,8 @@ int regcache_init(struct regmap *map, const struct regmap_config *config)
ret = regcache_hw_init(map);
if (ret < 0)
return ret;
+ if (map->cache_bypass)
+ return 0;
}
if (!map->max_register)
--
2.1.0.27.g96db324
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 6/6] regmap: cache: Fix possible ZERO_SIZE_PTR pointer dereferencing error.
2014-10-09 9:02 ` [PATCH 6/6] regmap: cache: Fix possible ZERO_SIZE_PTR pointer dereferencing error Xiubo Li
@ 2014-10-13 10:54 ` Mark Brown
2014-10-14 2:03 ` Li.Xiubo
0 siblings, 1 reply; 15+ messages in thread
From: Mark Brown @ 2014-10-13 10:54 UTC (permalink / raw)
To: Xiubo Li; +Cc: linux-kernel, dp
[-- Attachment #1: Type: text/plain, Size: 422 bytes --]
On Thu, Oct 09, 2014 at 05:02:57PM +0800, Xiubo Li wrote:
> When all the registers are volatile(unlikely, but logically and mostly
> will happen for some 'device' who has very few registers), then the
> count will be euqal to 0, then kmalloc() will return ZERO_SIZE_PTR,
> which equals to ((void *)16).
Applied, thanks. I'm assuming there's nothing affected by this in
Linus' tree which needs this as a fix immediately?
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [PATCH 6/6] regmap: cache: Fix possible ZERO_SIZE_PTR pointer dereferencing error.
2014-10-13 10:54 ` Mark Brown
@ 2014-10-14 2:03 ` Li.Xiubo
0 siblings, 0 replies; 15+ messages in thread
From: Li.Xiubo @ 2014-10-14 2:03 UTC (permalink / raw)
To: Mark Brown; +Cc: linux-kernel, dp
> Subject: Re: [PATCH 6/6] regmap: cache: Fix possible ZERO_SIZE_PTR pointer
> dereferencing error.
>
> On Thu, Oct 09, 2014 at 05:02:57PM +0800, Xiubo Li wrote:
> > When all the registers are volatile(unlikely, but logically and mostly
> > will happen for some 'device' who has very few registers), then the
> > count will be euqal to 0, then kmalloc() will return ZERO_SIZE_PTR,
> > which equals to ((void *)16).
>
> Applied, thanks. I'm assuming there's nothing affected by this in
> Linus' tree which needs this as a fix immediately?
Yes, this will happen on one case like my LS1 platform in late future.
Thanks,
BRs
Xiubo
^ permalink raw reply [flat|nested] 15+ messages in thread