Hi Ryan,
On 21.10.2015 15:48, Ryan Harkin wrote:
<snip>
Perhaps also the "c", "w", "l" and "ll" in "cword" should be renamed to "u8", "u16", "u32" and "u64"? We aren't actually looking for a "char" in this file, but an 8 bit value, for example.
Correct. But I suspect that u8 etc will not be possible as variable names. As they are already taken. The current names are not perfect but still not that bad. Feel free to come up with some better names that match the meaning of the variables more closely in v2...
Yeah, I just noticed that problem too. I'll think of an appropriate new name. For now, I've just used w8, w16, w32 and w64, meaning "width 8", etc., but that might not be very clear either.
I'm actually liking the "w" names now. Let me know if it doesn't suit.
I'm fine with the "w" names.
Now for a separate but related issue, so I thought I'd ask about it here. Looking through the code to make this rename, I noticed a few u32 pointers, eg:
1056: u32 *flash; 1063: flash = (u32 *)info->start[sect]; 1145: u32 *flash; 1151: flash = (u32 *)info->start[i];
Perhaps I'm lucky that my flash part lives in the 32-bit address range, but I think this may need fixing too - with a separate patch. That took me to looking at the definition of flash_info_t in include/flash.h and I see there are quite a few "uchar", "ushort" and "ulong"s in there. Do you think these will need changing too?
The cfi flash driver unfortunately has not seem much "love" in the last few years. As parallel NOR flash devices are used rarely nowadays. So you will most likely find many places that could use some cleanup / fixes. And yes, some "ulong"s might cause problems on the new 64bit systems. I really appreciate it that you take a look at this.
Thanks, Stefan