[PATCH 0/5] lib: fixes for fdt functions

[PATCH 0/5] lib: fixes for fdt functions

[Heinrich Schuchardt]

* Use correct type for variables used in error checks
* Avoid duplicates checks
* Don't assign values that are never used.

Heinrich Schuchardt (5):
  lib: simplify fdt_parse_plmt_node()
  lib: simplify fdt_parse_plicsw_node()
  lib: fix fdt_parse_plmt_node()
  lib: fix fdt_parse_plicsw_node()
  lib: simplify fdt_translate_address()

 lib/utils/fdt/fdt_helper.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

-- 
2.37.2

[PATCH 1/5] lib: simplify fdt_parse_plmt_node()

[Heinrich Schuchardt]

We should not check !plmt_base || !plmt_size twice.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
 lib/utils/fdt/fdt_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/utils/fdt/fdt_helper.c b/lib/utils/fdt/fdt_helper.c
index 89b8bba..dbd7484 100644
--- a/lib/utils/fdt/fdt_helper.c
+++ b/lib/utils/fdt/fdt_helper.c
@@ -814,7 +814,7 @@ int fdt_parse_plmt_node(void *fdt, int nodeoffset, unsigned long *plmt_base,
 
 	rc = fdt_get_node_addr_size(fdt, nodeoffset, 0,
 				    &reg_addr, &reg_size);
-	if (rc < 0 || !plmt_base || !plmt_size)
+	if (rc < 0)
 		return SBI_ENODEV;
 	*plmt_base = reg_addr;
 	*plmt_size = reg_size;
-- 
2.37.2

[PATCH 1/5] lib: simplify fdt_parse_plmt_node()

[Xiang W]

? 2022-11-25???? 15:08 +0100?Heinrich Schuchardt???
> We should not check !plmt_base || !plmt_size twice.
> 
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Look good to me

Reviewed-by: Xiang W <wxjstz@126.com>
> ---
> ?lib/utils/fdt/fdt_helper.c | 2 +-
> ?1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/utils/fdt/fdt_helper.c b/lib/utils/fdt/fdt_helper.c
> index 89b8bba..dbd7484 100644
> --- a/lib/utils/fdt/fdt_helper.c
> +++ b/lib/utils/fdt/fdt_helper.c
> @@ -814,7 +814,7 @@ int fdt_parse_plmt_node(void *fdt, int nodeoffset, unsigned long *plmt_base,
> ?
> ????????rc = fdt_get_node_addr_size(fdt, nodeoffset, 0,
> ??????????????????????????????????? &reg_addr, &reg_size);
> -???????if (rc < 0 || !plmt_base || !plmt_size)
> +???????if (rc < 0)
> ????????????????return SBI_ENODEV;
> ????????*plmt_base = reg_addr;
> ????????*plmt_size = reg_size;
> -- 
> 2.37.2
> 
> 

[PATCH 1/5] lib: simplify fdt_parse_plmt_node()

[Yu-Chien Peter Lin]

On Fri, Nov 25, 2022 at 03:08:23PM +0100, Heinrich Schuchardt wrote:
> We should not check !plmt_base || !plmt_size twice.
> 
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>

Looks good to me.
Reviewed-by: Yu Chien Peter Lin <peterlin@andestech.com>

> ---
>  lib/utils/fdt/fdt_helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/utils/fdt/fdt_helper.c b/lib/utils/fdt/fdt_helper.c
> index 89b8bba..dbd7484 100644
> --- a/lib/utils/fdt/fdt_helper.c
> +++ b/lib/utils/fdt/fdt_helper.c
> @@ -814,7 +814,7 @@ int fdt_parse_plmt_node(void *fdt, int nodeoffset, unsigned long *plmt_base,
>  
>  	rc = fdt_get_node_addr_size(fdt, nodeoffset, 0,
>  				    &reg_addr, &reg_size);
> -	if (rc < 0 || !plmt_base || !plmt_size)
> +	if (rc < 0)
>  		return SBI_ENODEV;
>  	*plmt_base = reg_addr;
>  	*plmt_size = reg_size;
> -- 
> 2.37.2

[PATCH 2/5] lib: simplify fdt_parse_plicsw_node()

[Heinrich Schuchardt]

We should not check !plicsw_base || !size twice.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
 lib/utils/fdt/fdt_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/utils/fdt/fdt_helper.c b/lib/utils/fdt/fdt_helper.c
index dbd7484..57c0248 100644
--- a/lib/utils/fdt/fdt_helper.c
+++ b/lib/utils/fdt/fdt_helper.c
@@ -868,7 +868,7 @@ int fdt_parse_plicsw_node(void *fdt, int nodeoffset, unsigned long *plicsw_base,
 
 	rc = fdt_get_node_addr_size(fdt, nodeoffset, 0,
 				    &reg_addr, &reg_size);
-	if (rc < 0 || !plicsw_base || !size)
+	if (rc < 0)
 		return SBI_ENODEV;
 	*plicsw_base = reg_addr;
 	*size = reg_size;
-- 
2.37.2

[PATCH 2/5] lib: simplify fdt_parse_plicsw_node()

[Xiang W]

? 2022-11-25???? 15:08 +0100?Heinrich Schuchardt???
> We should not check !plicsw_base || !size twice.
> 
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Look good to me

Reviewed-by: Xiang W <wxjstz@126.com>
> ---
> ?lib/utils/fdt/fdt_helper.c | 2 +-
> ?1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/utils/fdt/fdt_helper.c b/lib/utils/fdt/fdt_helper.c
> index dbd7484..57c0248 100644
> --- a/lib/utils/fdt/fdt_helper.c
> +++ b/lib/utils/fdt/fdt_helper.c
> @@ -868,7 +868,7 @@ int fdt_parse_plicsw_node(void *fdt, int nodeoffset, unsigned long *plicsw_base,
> ?
> ????????rc = fdt_get_node_addr_size(fdt, nodeoffset, 0,
> ??????????????????????????????????? &reg_addr, &reg_size);
> -???????if (rc < 0 || !plicsw_base || !size)
> +???????if (rc < 0)
> ????????????????return SBI_ENODEV;
> ????????*plicsw_base = reg_addr;
> ????????*size = reg_size;
> -- 
> 2.37.2
> 
> 

[PATCH 2/5] lib: simplify fdt_parse_plicsw_node()

[Yu-Chien Peter Lin]

On Fri, Nov 25, 2022 at 03:08:24PM +0100, Heinrich Schuchardt wrote:
> We should not check !plicsw_base || !size twice.
> 
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>

Looks good to me
Reviewed-by: Yu Chien Peter Lin <peterlin@andestech.com>

> ---
>  lib/utils/fdt/fdt_helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/utils/fdt/fdt_helper.c b/lib/utils/fdt/fdt_helper.c
> index dbd7484..57c0248 100644
> --- a/lib/utils/fdt/fdt_helper.c
> +++ b/lib/utils/fdt/fdt_helper.c
> @@ -868,7 +868,7 @@ int fdt_parse_plicsw_node(void *fdt, int nodeoffset, unsigned long *plicsw_base,
>  
>  	rc = fdt_get_node_addr_size(fdt, nodeoffset, 0,
>  				    &reg_addr, &reg_size);
> -	if (rc < 0 || !plicsw_base || !size)
> +	if (rc < 0)
>  		return SBI_ENODEV;
>  	*plicsw_base = reg_addr;
>  	*size = reg_size;
> -- 
> 2.37.2
> 
> 
> -- 
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi

[PATCH 3/5] lib: fix fdt_parse_plmt_node()

[Heinrich Schuchardt]

cpu_offset, cpu_intc_offset must int to discover failed invocations of
fdt_node_offset_by_phandle() or fdt_parent_offset().

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
 lib/utils/fdt/fdt_helper.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/utils/fdt/fdt_helper.c b/lib/utils/fdt/fdt_helper.c
index 57c0248..ea969e5 100644
--- a/lib/utils/fdt/fdt_helper.c
+++ b/lib/utils/fdt/fdt_helper.c
@@ -805,7 +805,7 @@ int fdt_parse_plmt_node(void *fdt, int nodeoffset, unsigned long *plmt_base,
 {
 	const fdt32_t *val;
 	int rc, i, count;
-	uint64_t reg_addr, reg_size, cpu_offset, cpu_intc_offset;
+	uint64_t reg_addr, reg_size;
 	u32 phandle, hwirq, hartid, hcount;
 
 	if (nodeoffset < 0 || !fdt || !plmt_base ||
@@ -826,6 +826,8 @@ int fdt_parse_plmt_node(void *fdt, int nodeoffset, unsigned long *plmt_base,
 
 	hcount = 0;
 	for (i = 0; i < (count / 2); i++) {
+		int cpu_offset, cpu_intc_offset;
+
 		phandle = fdt32_to_cpu(val[2 * i]);
 		hwirq = fdt32_to_cpu(val[2 * i + 1]);
 
-- 
2.37.2

[PATCH 3/5] lib: fix fdt_parse_plmt_node()

[Xiang W]

? 2022-11-25???? 15:08 +0100?Heinrich Schuchardt???
> cpu_offset, cpu_intc_offset must int to discover failed invocations of
> fdt_node_offset_by_phandle() or fdt_parent_offset().
> 
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
https://github.com/riscv-software-src/opensbi/blob/master/lib/utils/fdt/fdt_helper.c#L837
if (cpu_intc_offset < 0) should be if (cpu_offset < 0)

Regards,
Xiang W
> ---
> ?lib/utils/fdt/fdt_helper.c | 4 +++-
> ?1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/utils/fdt/fdt_helper.c b/lib/utils/fdt/fdt_helper.c
> index 57c0248..ea969e5 100644
> --- a/lib/utils/fdt/fdt_helper.c
> +++ b/lib/utils/fdt/fdt_helper.c
> @@ -805,7 +805,7 @@ int fdt_parse_plmt_node(void *fdt, int nodeoffset, unsigned long *plmt_base,
> ?{
> ????????const fdt32_t *val;
> ????????int rc, i, count;
> -???????uint64_t reg_addr, reg_size, cpu_offset, cpu_intc_offset;
> +???????uint64_t reg_addr, reg_size;
> ????????u32 phandle, hwirq, hartid, hcount;
> ?
> ????????if (nodeoffset < 0 || !fdt || !plmt_base ||
> @@ -826,6 +826,8 @@ int fdt_parse_plmt_node(void *fdt, int nodeoffset, unsigned long *plmt_base,
> ?
> ????????hcount = 0;
> ????????for (i = 0; i < (count / 2); i++) {
> +???????????????int cpu_offset, cpu_intc_offset;
> +
> ????????????????phandle = fdt32_to_cpu(val[2 * i]);
> ????????????????hwirq = fdt32_to_cpu(val[2 * i + 1]);
> ?
> -- 
> 2.37.2
> 
> 

[PATCH 4/5] lib: fix fdt_parse_plicsw_node()

[Heinrich Schuchardt]

cpu_offset and cpu_intc_offset must be int to detect failed invocations of
fdt_node_offset_by_phandle() or fdt_parent_offset().

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
 lib/utils/fdt/fdt_helper.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/utils/fdt/fdt_helper.c b/lib/utils/fdt/fdt_helper.c
index ea969e5..da96282 100644
--- a/lib/utils/fdt/fdt_helper.c
+++ b/lib/utils/fdt/fdt_helper.c
@@ -861,7 +861,7 @@ int fdt_parse_plicsw_node(void *fdt, int nodeoffset, unsigned long *plicsw_base,
 {
 	const fdt32_t *val;
 	int rc, i, count;
-	uint64_t reg_addr, reg_size, cpu_offset, cpu_intc_offset;
+	uint64_t reg_addr, reg_size;
 	u32 phandle, hwirq, hartid, hcount;
 
 	if (nodeoffset < 0 || !fdt || !plicsw_base ||
@@ -882,6 +882,8 @@ int fdt_parse_plicsw_node(void *fdt, int nodeoffset, unsigned long *plicsw_base,
 
 	hcount = 0;
 	for (i = 0; i < (count / 2); i++) {
+		int cpu_offset, cpu_intc_offset;
+
 		phandle = fdt32_to_cpu(val[2 * i]);
 		hwirq = fdt32_to_cpu(val[2 * i + 1]);
 
-- 
2.37.2

[PATCH 4/5] lib: fix fdt_parse_plicsw_node()

[Xiang W]

? 2022-11-25???? 15:08 +0100?Heinrich Schuchardt???
> cpu_offset and cpu_intc_offset must be int to detect failed invocations of
> fdt_node_offset_by_phandle() or fdt_parent_offset().
> 
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
https://github.com/riscv-software-src/opensbi/blob/master/lib/utils/fdt/fdt_helper.c#L891
if (cpu_intc_offset < 0) should be if (cpu_offset < 0)

Regards,
Xiang W
> ---
> ?lib/utils/fdt/fdt_helper.c | 4 +++-
> ?1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/utils/fdt/fdt_helper.c b/lib/utils/fdt/fdt_helper.c
> index ea969e5..da96282 100644
> --- a/lib/utils/fdt/fdt_helper.c
> +++ b/lib/utils/fdt/fdt_helper.c
> @@ -861,7 +861,7 @@ int fdt_parse_plicsw_node(void *fdt, int nodeoffset, unsigned long *plicsw_base,
> ?{
> ????????const fdt32_t *val;
> ????????int rc, i, count;
> -???????uint64_t reg_addr, reg_size, cpu_offset, cpu_intc_offset;
> +???????uint64_t reg_addr, reg_size;
> ????????u32 phandle, hwirq, hartid, hcount;
> ?
> ????????if (nodeoffset < 0 || !fdt || !plicsw_base ||
> @@ -882,6 +882,8 @@ int fdt_parse_plicsw_node(void *fdt, int nodeoffset, unsigned long *plicsw_base,
> ?
> ????????hcount = 0;
> ????????for (i = 0; i < (count / 2); i++) {
> +???????????????int cpu_offset, cpu_intc_offset;
> +
> ????????????????phandle = fdt32_to_cpu(val[2 * i]);
> ????????????????hwirq = fdt32_to_cpu(val[2 * i + 1]);
> ?
> -- 
> 2.37.2
> 
> 

[PATCH 5/5] lib: simplify fdt_translate_address()

[Heinrich Schuchardt]

Don't assign a value to offset which is never used.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
 lib/utils/fdt/fdt_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/utils/fdt/fdt_helper.c b/lib/utils/fdt/fdt_helper.c
index da96282..b8d0eea 100644
--- a/lib/utils/fdt/fdt_helper.c
+++ b/lib/utils/fdt/fdt_helper.c
@@ -124,7 +124,7 @@ static int fdt_translate_address(void *fdt, uint64_t reg, int parent,
 	int i, rlen;
 	int cell_addr, cell_size;
 	const fdt32_t *ranges;
-	uint64_t offset = 0, caddr = 0, paddr = 0, rsize = 0;
+	uint64_t offset, caddr = 0, paddr = 0, rsize = 0;
 
 	cell_addr = fdt_address_cells(fdt, parent);
 	if (cell_addr < 1)
-- 
2.37.2

[PATCH 5/5] lib: simplify fdt_translate_address()

[Xiang W]

? 2022-11-25???? 15:08 +0100?Heinrich Schuchardt???
> Don't assign a value to offset which is never used.
> 
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Look good to me

Reviewed-by: Xiang W <wxjstz@126.com>
> ---
> ?lib/utils/fdt/fdt_helper.c | 2 +-
> ?1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/utils/fdt/fdt_helper.c b/lib/utils/fdt/fdt_helper.c
> index da96282..b8d0eea 100644
> --- a/lib/utils/fdt/fdt_helper.c
> +++ b/lib/utils/fdt/fdt_helper.c
> @@ -124,7 +124,7 @@ static int fdt_translate_address(void *fdt, uint64_t reg, int parent,
> ????????int i, rlen;
> ????????int cell_addr, cell_size;
> ????????const fdt32_t *ranges;
> -???????uint64_t offset = 0, caddr = 0, paddr = 0, rsize = 0;
> +???????uint64_t offset, caddr = 0, paddr = 0, rsize = 0;
> ?
> ????????cell_addr = fdt_address_cells(fdt, parent);
> ????????if (cell_addr < 1)
> -- 
> 2.37.2
> 
>