Xiubo Li (6):
regmap: cache: Sort include headers alphabetically
regmap: cache: cleanup regcache_hw_init().
regmap: cache: fix errno in regcache_hw_init()
regmap: cache: speed regcache_hw_init() up.
regmap: cache: use kmalloc_array instead of kmalloc
regmap: cache: Fix possible ZERO_SIZE_PTR pointer dereferencing error.
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 | 63 +++++++++++++++++++----------------
4 files changed, 38 insertions(+), 33 deletions(-)
--
2.1.0.27.g96db324
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 <[email protected]>
---
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
Remove the redundant code for regmap cache.
Signed-off-by: Xiubo Li <[email protected]>
---
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
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 <[email protected]>
---
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
This patch fixes checkpatch.pl warning for regmap cache.
WARNING : prefer kmalloc_array over kmalloc with multiply
Signed-off-by: Xiubo Li <[email protected]>
---
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
This may speed regcache_hw_init() up for some cases that there
has volatile registers.
Signed-off-by: Xiubo Li <[email protected]>
---
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
When kmalloc() fails, we should return -ENOMEM.
Signed-off-by: Xiubo Li <[email protected]>
---
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
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.
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.
On Thu, Oct 09, 2014 at 05:02:54PM +0800, Xiubo Li wrote:
> When kmalloc() fails, we should return -ENOMEM.
Applied, thanks.
On Thu, Oct 09, 2014 at 05:02:55PM +0800, Xiubo Li wrote:
> This may speed regcache_hw_init() up for some cases that there
> has volatile registers.
Applied, thanks.
On Thu, Oct 09, 2014 at 05:02:56PM +0800, Xiubo Li wrote:
> This patch fixes checkpatch.pl warning for regmap cache.
> WARNING : prefer kmalloc_array over kmalloc with multiply
Applied, thanks.
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?
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
> 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