Change in ...flashrom[master]: ichspi.c: Remove unneeded 'esle'

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/33347

Change subject: ichspi.c: Remove unneeded 'esle'
......................................................................

ichspi.c: Remove unneeded 'esle'

'else' is not needed after a 'break' or 'return'.

Change-Id: Ie000732158f27632ee92404c66a9aab43f3b374c
Signed-off-by: Elyes HAOUAS 
---
M ichspi.c
1 file changed, 11 insertions(+), 12 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/47/33347/1

diff --git a/ichspi.c b/ichspi.c
index a30ca02..e50a2e3 100644
--- a/ichspi.c
+++ b/ichspi.c
@@ -1246,19 +1246,18 @@
 {
    if (hwseq_data.only_4k) {
    	return 4 * 1024;
-	} else {
-		uint8_t enc_berase;
-		static const uint32_t dec_berase[4] = {
-			256,
-			4 * 1024,
-			8 * 1024,
-			64 * 1024
-		};
-
-		ich_hwseq_set_addr(addr);
-		enc_berase = (REGREAD16(ICH9_REG_HSFS) & HSFS_BERASE) >> HSFS_BERASE_OFF;
-		return dec_berase[enc_berase];
    }
+	uint8_t enc_berase;
+	static const uint32_t dec_berase[4] = {
+		256,
+		4 * 1024,
+		8 * 1024,
+		64 * 1024
+	};
+
+	ich_hwseq_set_addr(addr);
+	enc_berase = (REGREAD16(ICH9_REG_HSFS) & HSFS_BERASE) >> HSFS_BERASE_OFF;
+	return dec_berase[enc_berase];
 }

/* Polls for Cycle Done Status, Flash Cycle Error or timeout in 8 us intervals.

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/33347 )

Change subject: ichspi.c: Remove unneeded 'esle'
......................................................................

Patch Set 1: Code-Review+1

(1 comment)

https://review.coreboot.org/#/c/33347/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/33347/1//COMMIT_MSG@7 
PS1, Line 7: esle
else

Change in ...flashrom[master]: ichspi.c: Remove unneeded 'else'

[HAOUAS Elyes (Code Review)]

Hello Angel Pons, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/flashrom/+/33347

to look at the new patch set (#2).

Change subject: ichspi.c: Remove unneeded 'else'
......................................................................

ichspi.c: Remove unneeded 'else'

'else' is not needed after a 'break' or 'return'.

Change-Id: Ie000732158f27632ee92404c66a9aab43f3b374c
Signed-off-by: Elyes HAOUAS 
---
M ichspi.c
1 file changed, 11 insertions(+), 12 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/47/33347/2

Change in ...flashrom[master]: ichspi.c: Remove unneeded 'else'

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/33347 )

Change subject: ichspi.c: Remove unneeded 'else'
......................................................................

Patch Set 2: Code-Review+2

Change in ...flashrom[master]: ichspi.c: Remove unneeded 'else'

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/33347 )

Change subject: ichspi.c: Remove unneeded 'else'
......................................................................

Patch Set 2:

(1 comment)

We still need to have that discussion how we can
argue that this is a fix...

https://review.coreboot.org/#/c/33347/2/ichspi.c 
File ichspi.c:

https://review.coreboot.org/#/c/33347/2/ichspi.c@1250 
PS2, Line 1250: 	uint8_t enc_berase;
              : 	static const uint32_t dec_berase[4] = {
              : 		256,
              : 		4 * 1024,
              : 		8 * 1024,
              : 		64 * 1024
              : 	};
              : 
Please keep declarations on top.

Change in ...flashrom[master]: ichspi.c: Remove unneeded 'else'

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/33347 )

Change subject: ichspi.c: Remove unneeded 'else'
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/#/c/33347/2/ichspi.c 
File ichspi.c:

https://review.coreboot.org/#/c/33347/2/ichspi.c@1250 
PS2, Line 1250: 	uint8_t enc_berase;
              : 	static const uint32_t dec_berase[4] = {
              : 		256,
              : 		4 * 1024,
              : 		8 * 1024,
              : 		64 * 1024
              : 	};
              :
> Please keep declarations on top.

Done.
Thx

Change in ...flashrom[master]: ichspi.c: Remove unneeded 'else'

[HAOUAS Elyes (Code Review)]

Hello Angel Pons, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/flashrom/+/33347

to look at the new patch set (#3).

Change subject: ichspi.c: Remove unneeded 'else'
......................................................................

ichspi.c: Remove unneeded 'else'

'else' is not needed after a 'break' or 'return'.

Change-Id: Ie000732158f27632ee92404c66a9aab43f3b374c
Signed-off-by: Elyes HAOUAS 
---
M ichspi.c
1 file changed, 12 insertions(+), 12 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/47/33347/3

Change in ...flashrom[master]: ichspi.c: Remove unneeded 'else'

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/33347 )

Change subject: ichspi.c: Remove unneeded 'else'
......................................................................

Patch Set 3:

Elyes, did you check if the upstream (Linux kernel) checkpatch
also complains about these `else`s?

As I told you on IRC, that would be enough reason for me to
merge these patches.

Change in ...flashrom[master]: ichspi.c: Remove unneeded 'else'

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/33347 )

Change subject: ichspi.c: Remove unneeded 'else'
......................................................................

Patch Set 3:
> Patch Set 3:
>
> Elyes, did you check if the upstream (Linux kernel) checkpatch
> also complains about these `else`s?
>
> As I told you on IRC, that would be enough reason for me to
> merge these patches.

Thank you for the review.
Yes I did.
I've used https://raw.githubusercontent.com/torvalds/linux/master/scripts/checkpatch.p... 
and here is for example the log of "./checkpatch.pl --no-tree --file ichspi.c" : https://pastebin.com/H9W73U6h
as you can see, we have :
"WARNING: else is not generally useful after a break or return
#1249: FILE: ichspi.c:1249:
+		return 4 * 1024;
+	} else {
"

Change in ...flashrom[master]: ichspi.c: Remove unneeded 'else'

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/33347 )

Change subject: ichspi.c: Remove unneeded 'else'
......................................................................

Patch Set 3:
> > Elyes, did you check if the upstream (Linux kernel) checkpatch
> > also complains about these `else`s?
> >
> > As I told you on IRC, that would be enough reason for me to
> > merge these patches.
>
> Thank you for the review.
> Yes I did.
> I've used https://raw.githubusercontent.com/torvalds/linux/master/scripts/checkpatch.p... 
> and here is for example the log of "./checkpatch.pl --no-tree --file ichspi.c" : https://pastebin.com/H9W73U6h
> as you can see, we have :
> "WARNING: else is not generally useful after a break or return
> #1249: FILE: ichspi.c:1249:
> return 4 * 1024;
> } else {
>
> "

Thanks for the confirmation.

Change in ...flashrom[master]: ichspi.c: Remove unneeded 'else'

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/33347 )

Change subject: ichspi.c: Remove unneeded 'else'
......................................................................

Patch Set 3: Code-Review+2

Change in ...flashrom[master]: ichspi.c: Remove unneeded 'else'

[Nico Huber (Code Review)]

Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/c/flashrom/+/33347 )

Change subject: ichspi.c: Remove unneeded 'else'
......................................................................

ichspi.c: Remove unneeded 'else'

'else' is not needed after a 'break' or 'return'.

Change-Id: Ie000732158f27632ee92404c66a9aab43f3b374c
Signed-off-by: Elyes HAOUAS 
Reviewed-on: https://review.coreboot.org/c/flashrom/+/33347
Tested-by: build bot (Jenkins) 
Reviewed-by: Nico Huber 
---
M ichspi.c
1 file changed, 12 insertions(+), 12 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Nico Huber: Looks good to me, approved

diff --git a/ichspi.c b/ichspi.c
index a30ca02..6932678 100644
--- a/ichspi.c
+++ b/ichspi.c
@@ -1244,21 +1244,21 @@
  */
 static uint32_t ich_hwseq_get_erase_block_size(unsigned int addr)
 {
+	uint8_t enc_berase;
+	static const uint32_t dec_berase[4] = {
+		256,
+		4 * 1024,
+		8 * 1024,
+		64 * 1024
+	};
+
    if (hwseq_data.only_4k) {
    	return 4 * 1024;
-	} else {
-		uint8_t enc_berase;
-		static const uint32_t dec_berase[4] = {
-			256,
-			4 * 1024,
-			8 * 1024,
-			64 * 1024
-		};
-
-		ich_hwseq_set_addr(addr);
-		enc_berase = (REGREAD16(ICH9_REG_HSFS) & HSFS_BERASE) >> HSFS_BERASE_OFF;
-		return dec_berase[enc_berase];
    }
+
+	ich_hwseq_set_addr(addr);
+	enc_berase = (REGREAD16(ICH9_REG_HSFS) & HSFS_BERASE) >> HSFS_BERASE_OFF;
+	return dec_berase[enc_berase];
 }

/* Polls for Cycle Done Status, Flash Cycle Error or timeout in 8 us intervals.