Hi Simon,
On Thu, Aug 2, 2018 at 2:21 PM, Simon Glass sjg@chromium.org wrote:
On 31 July 2018 at 04:01, Mario Six mario.six@gdsys.cc wrote:
Both fdtdec_get_addr_size_fixed and of_address_to_resource can fail with an error, which is not currently checked during regmap initialization.
Since the indentation depth is already quite deep, extract a new 'init_range' method to do the initialization.
Signed-off-by: Mario Six mario.six@gdsys.cc
v2 -> v3: New in v3
drivers/core/regmap.c | 68 ++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 56 insertions(+), 12 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
nit below
diff --git a/drivers/core/regmap.c b/drivers/core/regmap.c index 4ebab233490..51d9cadc510 100644 --- a/drivers/core/regmap.c +++ b/drivers/core/regmap.c @@ -56,6 +56,58 @@ int regmap_init_mem_platdata(struct udevice *dev, fdt_val_t *reg, int count, return 0; } #else +/**
- init_range() - Initialize a single range of a regmap
- @node: Device node that will use the map in question
- @range: Pointer to a regmap_range structure that will be initialized
- @addr_len: The length of the addr parts of the reg property
- @size_len: The length of the size parts of the reg property
- @index: The index of the range to initialize
- This function will read the necessary 'reg' information from the device tree
- (the 'addr' part, and the 'length' part), and initialize the range in
- quesion.
- Return: 0 if OK, -ve on error
- */
+static int init_range(ofnode node, struct regmap_range *range, int addr_len,
int size_len, int index)+{
fdt_size_t sz;struct resource r;if (of_live_active()) {int ret;ret = of_address_to_resource(ofnode_to_np(node),index, &r);if (ret) {debug("%s: Could not read resource of range %d (ret = %d)\n",ofnode_get_name(node), index, ret);return ret;}range->start = r.start;range->size = r.end - r.start + 1;return 0;}I wonder about having an else here? That makes it clear that this function has two implementations.
OK, shouldn't be a problem. I originally didn't put the else in because of the length of the fdtdec_get_add_size_fixed call, but I can extract the ofnode_to_offset into a "offset" variable, which should make the line more easily indentable.
range->start = fdtdec_get_addr_size_fixed(gd->fdt_blob,ofnode_to_offset(node),"reg", index, addr_len,size_len, &sz, true);if (range->start == FDT_ADDR_T_NONE) {debug("%s: Could not read start of range %d\n",ofnode_get_name(node), index);return -EINVAL;}range->size = sz;return 0;+}
int regmap_init_mem(ofnode node, struct regmap **mapp) { struct regmap_range *range; @@ -64,7 +116,6 @@ int regmap_init_mem(ofnode node, struct regmap **mapp) int addr_len, size_len, both_len; int len; int index;
struct resource r; addr_len = ofnode_read_simple_addr_cells(ofnode_get_parent(node)); if (addr_len < 0) {@@ -101,17 +152,10 @@ int regmap_init_mem(ofnode node, struct regmap **mapp)
for (range = map->ranges, index = 0; count > 0; count--, range++, index++) {
fdt_size_t sz;if (of_live_active()) {of_address_to_resource(ofnode_to_np(node), index, &r);range->start = r.start;range->size = r.end - r.start + 1;} else {range->start = fdtdec_get_addr_size_fixed(gd->fdt_blob,ofnode_to_offset(node), "reg", index,addr_len, size_len, &sz, true);range->size = sz;}
int ret = init_range(node, range, addr_len, size_len, index);if (ret)return ret; } *mapp = map;-- 2.11.0
Best regards, Mario