Change in flashrom[master]: s25f.c: Fix mismatched function definitions

[Angel Pons (Code Review)]

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

Change subject: s25f.c: Fix mismatched function definitions
......................................................................

Patch Set 3:

(1 comment)

Commit Message:

https://review.coreboot.org/c/flashrom/+/52473/comment/4f27b447_fbd10277 
PS1, Line 8: 
           : This was missed because `uint32_t` is `unsigned int` in most cases.
           : However, it is not the case for DJGPP 6.1.0 for some reason.
> Last couple of weeks have been pretty painful :| I've just been on and off review unofficially to help Anastasia which probably made it even more unclear. Since I rarely -2 I erroneously assumed you figured something might be strange. Any ways, cheers Angel and sorry for any confusion as well. Things like it would have been easier if I was full time Flashrom and hanging on IRC but sadly I am not.

Ouch. I'm sorry to hear that. Hope things get better soon!
> I figured addr is of a specific length because of how it is decoded into precisely four unsigned bytes:
>    (addr >> 24) & 0xff,
>    (addr >> 16) & 0xff,
>    (addr >> 8) & 0xff,
>    (addr & 0xff)

Good point! Yes, addr is decoded into 4 bytes for 4BA (4-Byte Address) SPI commands. This would be 3 bytes for regular SPI commands, and I haven't checked what non-SPI flash chips do. So, I agree that addr needs to be at least 4 bytes in size.

The point of this patch is to fix build issues on unusual targets like DJGPP. These build issues are regressions: flashrom 1.2 builds successfully on DJGPP. In this case, the easiest solution is to fix the mismatched function definitions. I used `unsigned int` because it's what the rest of flashrom uses.

Still, if we agree that `uint32_t` would be a better choice (at least you and I do), we can simply retype everything in another patch. Aaand since `uint32_t` is `unsigned int` on x86, replacing the type should be a reproducible change! 😊
> Is `blocklen` just unused or am I not seeing it?

About the blocklen parameter: yes, is unused in many of the block erasers for SPI flash chips. There are some SPI block erasers that use blocklen, and I imagine some non-SPI block erasers do so as well. I suppose blocklen is unused because these block erasers always erase blocks of a certain size, and the block erasers don't need to use the value of blocklen.

We could add a sanity check and complain when the value of blocklen doesn't match the expected value. However, I'm not sure how useful this would be, and doing this involves reading *lots* of datasheets:

for each block eraser function that does not use blocklen (func) {
    for each chip that uses $func (chip) {
    	read datasheet of $chip to know the erase block size
    }
 }