1 comment:
Commit Message:
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
}
}To view, visit change 52473. To unsubscribe, or for help writing mail filters, visit settings.