Re: [U-Boot] [PATCH 1/4] regmap: clean up regmap allocation

[Simon Glass]

Hi Masahiro,

On 17 April 2018 at 20:38, Masahiro Yamada 
wrote:
> Putting zero length array at the end of struct is a common technique
> to embed arbitrary length of members.  There is no good reason to let
> regmap_alloc_count() branch by "if (count <= 1)".
>
> As far as I understood the code, regmap->base is an alias of
> regmap->ranges[0].start, but it is not helpful but make the code
> just ugly.
>
> Rename regmap_alloc_count() to regmap_alloc() because the _count
> suffix seems pointless.
>
> Signed-off-by: Masahiro Yamada 
> ---
>
>  drivers/core/regmap.c | 31 +--
>  include/regmap.h  |  7 ++-
>  2 files changed, 11 insertions(+), 27 deletions(-)

This seems fine to me and does not increase the number of allocations.

Reviewed-by: Simon Glass 

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH 1/4] regmap: clean up regmap allocation

[Masahiro Yamada]

Putting zero length array at the end of struct is a common technique
to embed arbitrary length of members.  There is no good reason to let
regmap_alloc_count() branch by "if (count <= 1)".

As far as I understood the code, regmap->base is an alias of
regmap->ranges[0].start, but it is not helpful but make the code
just ugly.

Rename regmap_alloc_count() to regmap_alloc() because the _count
suffix seems pointless.

Signed-off-by: Masahiro Yamada 
---

 drivers/core/regmap.c | 31 +--
 include/regmap.h  |  7 ++-
 2 files changed, 11 insertions(+), 27 deletions(-)

diff --git a/drivers/core/regmap.c b/drivers/core/regmap.c
index 8a0e00f..6c3dbe9 100644
--- a/drivers/core/regmap.c
+++ b/drivers/core/regmap.c
@@ -18,22 +18,13 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-static struct regmap *regmap_alloc_count(int count)
+static struct regmap *regmap_alloc(int count)
 {
struct regmap *map;
 
-   map = malloc(sizeof(struct regmap));
+   map = malloc(sizeof(*map) + sizeof(map->ranges[0]) * count);
if (!map)
return NULL;
-   if (count <= 1) {
-   map->range = >base_range;
-   } else {
-   map->range = malloc(count * sizeof(struct regmap_range));
-   if (!map->range) {
-   free(map);
-   return NULL;
-   }
-   }
map->range_count = count;
 
return map;
@@ -46,12 +37,11 @@ int regmap_init_mem_platdata(struct udevice *dev, fdt_val_t 
*reg, int count,
struct regmap_range *range;
struct regmap *map;
 
-   map = regmap_alloc_count(count);
+   map = regmap_alloc(count);
if (!map)
return -ENOMEM;
 
-   map->base = *reg;
-   for (range = map->range; count > 0; reg += 2, range++, count--) {
+   for (range = map->ranges; count > 0; reg += 2, range++, count--) {
range->start = *reg;
range->size = reg[1];
}
@@ -84,11 +74,11 @@ int regmap_init_mem(struct udevice *dev, struct regmap 
**mapp)
if (!count)
return -EINVAL;
 
-   map = regmap_alloc_count(count);
+   map = regmap_alloc(count);
if (!map)
return -ENOMEM;
 
-   for (range = map->range, index = 0; count > 0;
+   for (range = map->ranges, index = 0; count > 0;
 count--, range++, index++) {
fdt_size_t sz;
if (of_live_active()) {
@@ -102,7 +92,6 @@ int regmap_init_mem(struct udevice *dev, struct regmap 
**mapp)
range->size = sz;
}
}
-   map->base = map->range[0].start;
 
*mapp = map;
 
@@ -116,15 +105,13 @@ void *regmap_get_range(struct regmap *map, unsigned int 
range_num)
 
if (range_num >= map->range_count)
return NULL;
-   range = >range[range_num];
+   range = >ranges[range_num];
 
return map_sysmem(range->start, range->size);
 }
 
 int regmap_uninit(struct regmap *map)
 {
-   if (map->range_count > 1)
-   free(map->range);
free(map);
 
return 0;
@@ -132,7 +119,7 @@ int regmap_uninit(struct regmap *map)
 
 int regmap_read(struct regmap *map, uint offset, uint *valp)
 {
-   uint32_t *ptr = map_physmem(map->base + offset, 4, MAP_NOCACHE);
+   u32 *ptr = map_physmem(map->ranges[0].start + offset, 4, MAP_NOCACHE);
 
*valp = le32_to_cpu(readl(ptr));
 
@@ -141,7 +128,7 @@ int regmap_read(struct regmap *map, uint offset, uint *valp)
 
 int regmap_write(struct regmap *map, uint offset, uint val)
 {
-   uint32_t *ptr = map_physmem(map->base + offset, 4, MAP_NOCACHE);
+   u32 *ptr = map_physmem(map->ranges[0].start + offset, 4, MAP_NOCACHE);
 
writel(cpu_to_le32(val), ptr);
 
diff --git a/include/regmap.h b/include/regmap.h
index 493a5d8..858aa7e 100644
--- a/include/regmap.h
+++ b/include/regmap.h
@@ -22,15 +22,12 @@ struct regmap_range {
 /**
  * struct regmap - a way of accessing hardware/bus registers
  *
- * @base:  Base address of register map
  * @range_count: Number of ranges available within the map
- * @range: Pointer to the list of ranges, allocated if @range_count > 1
- * @base_range:If @range_count is <= 1, @range points here
+ * @ranges:Array of ranges
  */
 struct regmap {
-   phys_addr_t base;
int range_count;
-   struct regmap_range *range, base_range;
+   struct regmap_range ranges[];
 };
 
 /*
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot