[U-Boot] [PATCH] board/BuR/common: incorrect check of dtb

[U-Boot] [PATCH] board/BuR/common: incorrect check of dtb

[Heinrich Schuchardt]

The logical expression to check the dtb is incorrect in
load_devicetree.

The problem was indicated by cppcheck.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
I do not have a board for testing.
Please, review carefully.
---
 board/BuR/common/common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/board/BuR/common/common.c b/board/BuR/common/common.c
index 876150402c..c0316b9ebd 100644
--- a/board/BuR/common/common.c
+++ b/board/BuR/common/common.c
@@ -265,7 +265,7 @@ static int load_devicetree(void)
 	char *dtbname = getenv("dtb");
 	char *dtbdev = getenv("dtbdev");
 	char *dtppart = getenv("dtbpart");
-	if (!dtbdev || !dtbdev || !dtbname) {
+	if (!dtbdev || !dtbpart || !dtbname) {
 		printf("%s: <dtbdev>/<dtbpart>/<dtb> missing.\n", __func__);
 		return -1;
 	}
-- 
2.11.0

[U-Boot] [PATCH] board/BuR/common: incorrect check of dtb

[Hannes Schmelzer]

On 05/03/2017 11:44 PM, Heinrich Schuchardt wrote:
> The logical expression to check the dtb is incorrect in
> load_devicetree.
>
> The problem was indicated by cppcheck.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> I do not have a board for testing.
> Please, review carefully.
> ---
>   board/BuR/common/common.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/board/BuR/common/common.c b/board/BuR/common/common.c
> index 876150402c..c0316b9ebd 100644
> --- a/board/BuR/common/common.c
> +++ b/board/BuR/common/common.c
> @@ -265,7 +265,7 @@ static int load_devicetree(void)
>   	char *dtbname = getenv("dtb");
>   	char *dtbdev = getenv("dtbdev");
>   	char *dtppart = getenv("dtbpart");
> -	if (!dtbdev || !dtbdev || !dtbname) {
> +	if (!dtbdev || !dtbpart || !dtbname) {
>   		printf("%s: <dtbdev>/<dtbpart>/<dtb> missing.\n", __func__);
>   		return -1;
>   	}
Reviewed-by: Hannes Schmelzer <oe5hpm@oevsv.at>
Acked-by: Hannes Schmelzer <oe5hpm@oevsv.at>

Thanks!

[U-Boot] board/BuR/common: incorrect check of dtb

[Tom Rini]

On Wed, May 03, 2017 at 11:44:11PM +0200, xypron.glpk at gmx.de wrote:

> The logical expression to check the dtb is incorrect in
> load_devicetree.
> 
> The problem was indicated by cppcheck.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Reviewed-by: Hannes Schmelzer <oe5hpm@oevsv.at>
> Acked-by: Hannes Schmelzer <oe5hpm@oevsv.at>
> ---
> I do not have a board for testing.
> Please, review carefully.
> ---
>  board/BuR/common/common.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/board/BuR/common/common.c b/board/BuR/common/common.c
> index 876150402c..c0316b9ebd 100644
> --- a/board/BuR/common/common.c
> +++ b/board/BuR/common/common.c
> @@ -265,7 +265,7 @@ static int load_devicetree(void)
>  	char *dtbname = getenv("dtb");
>  	char *dtbdev = getenv("dtbdev");
>  	char *dtppart = getenv("dtbpart");
> -	if (!dtbdev || !dtbdev || !dtbname) {
> +	if (!dtbdev || !dtbpart || !dtbname) {
>  		printf("%s: <dtbdev>/<dtbpart>/<dtb> missing.\n", __func__);
>  		return -1;
>  	}

This breaks some boards such as brppt1_mmc, which I agree doesn't make a
lot of sense with just the above context, can you please test building
'BuR' via buildman?  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170505/fc22c8e6/attachment.sig>

[U-Boot] board/BuR/common: incorrect check of dtb

[Anatolij Gustschin]

On Fri, 5 May 2017 12:38:06 -0400
Tom Rini trini at konsulko.com wrote:
...
> >  	char *dtbname = getenv("dtb");
> >  	char *dtbdev = getenv("dtbdev");
> >  	char *dtppart = getenv("dtbpart");
> > -	if (!dtbdev || !dtbdev || !dtbname) {
> > +	if (!dtbdev || !dtbpart || !dtbname) {
> >  		printf("%s: <dtbdev>/<dtbpart>/<dtb> missing.\n", __func__);
> >  		return -1;
> >  	}  
> 
> This breaks some boards such as brppt1_mmc, which I agree doesn't make a
> lot of sense with just the above context, can you please test building
> 'BuR' via buildman?  Thanks!

dtbpart is wrong here, the defined variable is dtppart.

--
Anatolij

[U-Boot] board/BuR/common: incorrect check of dtb

[Tom Rini]

On Fri, May 05, 2017 at 08:11:54PM +0200, Anatolij Gustschin wrote:
> On Fri, 5 May 2017 12:38:06 -0400
> Tom Rini trini at konsulko.com wrote:
> ...
> > >  	char *dtbname = getenv("dtb");
> > >  	char *dtbdev = getenv("dtbdev");
> > >  	char *dtppart = getenv("dtbpart");
> > > -	if (!dtbdev || !dtbdev || !dtbname) {
> > > +	if (!dtbdev || !dtbpart || !dtbname) {
> > >  		printf("%s: <dtbdev>/<dtbpart>/<dtb> missing.\n", __func__);
> > >  		return -1;
> > >  	}  
> > 
> > This breaks some boards such as brppt1_mmc, which I agree doesn't make a
> > lot of sense with just the above context, can you please test building
> > 'BuR' via buildman?  Thanks!
> 
> dtbpart is wrong here, the defined variable is dtppart.

Ah-ha!  Time to clean off the laptop screen...

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170505/d9525728/attachment.sig>

[U-Boot] board/BuR/common: incorrect check of dtb

[Hannes Schmelzer]

On 05/05/2017 08:14 PM, Tom Rini wrote:
> On Fri, May 05, 2017 at 08:11:54PM +0200, Anatolij Gustschin wrote:
>> On Fri, 5 May 2017 12:38:06 -0400
>> Tom Rini trini at konsulko.com wrote:
>> ...
>>>>   	char *dtbname = getenv("dtb");
>>>>   	char *dtbdev = getenv("dtbdev");
>>>>   	char *dtppart = getenv("dtbpart");
>>>> -	if (!dtbdev || !dtbdev || !dtbname) {
>>>> +	if (!dtbdev || !dtbpart || !dtbname) {
>>>>   		printf("%s: <dtbdev>/<dtbpart>/<dtb> missing.\n", __func__);
>>>>   		return -1;
>>>>   	}
>>> This breaks some boards such as brppt1_mmc, which I agree doesn't make a
>>> lot of sense with just the above context, can you please test building
>>> 'BuR' via buildman?  Thanks!
>> dtbpart is wrong here, the defined variable is dtppart.
> Ah-ha!  Time to clean off the laptop screen...
Yeah, maybe we should also do some cosmetic cleanup on this with an 
extra patch.
This typo is here since the early days ;-)

Heinrich, could you do so?

cheers,
Hannes

[U-Boot] [PATCH v2 1/1] board/BuR/common: incorrect check of dtb

[Heinrich Schuchardt]

The logical expression to check the dtb is incorrect in
load_devicetree.

The problem was indicated by cppcheck.

The inconsistent variable name dtppart is changed to dtbpart.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v2:
  fix syntax error due to incorrect spelling of variable

  Building was checked with buildman

  $ buildman -k brxre1
  boards.cfg is up to date. Nothing to do.
  Building current source for 1 boards (1 thread, 4 jobs per thread)
      1    0    0 /1      brxre1     
  $ buildman -k brppt1
  boards.cfg is up to date. Nothing to do.
  Building current source for 3 boards (3 threads, 2 jobs per thread)
      3    0    0 /3      0:00:45  : brppt1_nand

  I do not have a BuR board available for actual testing.

v1:
  Original patch
  https://patchwork.ozlabs.org/patch/758237/
---
 board/BuR/common/common.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/board/BuR/common/common.c b/board/BuR/common/common.c
index 876150402c..5cc82c9473 100644
--- a/board/BuR/common/common.c
+++ b/board/BuR/common/common.c
@@ -264,13 +264,13 @@ static int load_devicetree(void)
 #else
 	char *dtbname = getenv("dtb");
 	char *dtbdev = getenv("dtbdev");
-	char *dtppart = getenv("dtbpart");
-	if (!dtbdev || !dtbdev || !dtbname) {
+	char *dtbpart = getenv("dtbpart");
+	if (!dtbdev || !dtbpart || !dtbname) {
 		printf("%s: <dtbdev>/<dtbpart>/<dtb> missing.\n", __func__);
 		return -1;
 	}
 
-	if (fs_set_blk_dev(dtbdev, dtppart, FS_TYPE_EXT)) {
+	if (fs_set_blk_dev(dtbdev, dtbpart, FS_TYPE_EXT)) {
 		puts("load_devicetree: set_blk_dev failed.\n");
 		return -1;
 	}
-- 
2.11.0

[U-Boot] [PATCH v2 1/1] board/BuR/common: incorrect check of dtb

[Hannes Schmelzer]

> The logical expression to check the dtb is incorrect in
> load_devicetree.
>
> The problem was indicated by cppcheck.
>
> The inconsistent variable name dtppart is changed to dtbpart.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2:
>    fix syntax error due to incorrect spelling of variable
>
>    Building was checked with buildman
>
>    $ buildman -k brxre1
>    boards.cfg is up to date. Nothing to do.
>    Building current source for 1 boards (1 thread, 4 jobs per thread)
>        1    0    0 /1      brxre1
>    $ buildman -k brppt1
>    boards.cfg is up to date. Nothing to do.
>    Building current source for 3 boards (3 threads, 2 jobs per thread)
>        3    0    0 /3      0:00:45  : brppt1_nand
>
>    I do not have a BuR board available for actual testing.
>
> v1:
>    Original patch
>    https://patchwork.ozlabs.org/patch/758237/
> ---
>   board/BuR/common/common.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/board/BuR/common/common.c b/board/BuR/common/common.c
> index 876150402c..5cc82c9473 100644
> --- a/board/BuR/common/common.c
> +++ b/board/BuR/common/common.c
> @@ -264,13 +264,13 @@ static int load_devicetree(void)
>   #else
>   	char *dtbname = getenv("dtb");
>   	char *dtbdev = getenv("dtbdev");
> -	char *dtppart = getenv("dtbpart");
> -	if (!dtbdev || !dtbdev || !dtbname) {
> +	char *dtbpart = getenv("dtbpart");
> +	if (!dtbdev || !dtbpart || !dtbname) {
>   		printf("%s: <dtbdev>/<dtbpart>/<dtb> missing.\n", __func__);
>   		return -1;
>   	}
>   
> -	if (fs_set_blk_dev(dtbdev, dtppart, FS_TYPE_EXT)) {
> +	if (fs_set_blk_dev(dtbdev, dtbpart, FS_TYPE_EXT)) {
>   		puts("load_devicetree: set_blk_dev failed.\n");
>   		return -1;
>   	}
Reviewed-by: Hannes Schmelzer <oe5hpm@oevsv.at>
Acked-by: Hannes Schmelzer <oe5hpm@oevsv.at>

[U-Boot] [U-Boot, v2, 1/1] board/BuR/common: incorrect check of dtb

[Tom Rini]

On Fri, May 05, 2017 at 08:57:32PM +0200, xypron.glpk at gmx.de wrote:

> The logical expression to check the dtb is incorrect in
> load_devicetree.
> 
> The problem was indicated by cppcheck.
> 
> The inconsistent variable name dtppart is changed to dtbpart.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Reviewed-by: Hannes Schmelzer <oe5hpm@oevsv.at>
> Acked-by: Hannes Schmelzer <oe5hpm@oevsv.at>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170506/1d74fe2f/attachment.sig>