TS-7100 updates #6
Conversation
Newer u-boot pass straps through the device-tree chosen node Removed cmdline processing
This fixes #1
| fprintf(stderr, "cpu-options missing from device-tree chosen.\n" | ||
| "This may be an outdated u-boot\n"); |
There was a problem hiding this comment.
| fprintf(stderr, "cpu-options missing from device-tree chosen.\n" | |
| "This may be an outdated u-boot\n"); | |
| fprintf(stderr, "cpu-options missing from loaded devicetree.\n" | |
| "This may be an outdated U-Boot build.\n"); |
| return; | ||
| } | ||
|
|
||
| printf("FPGA_REV=%d\n", fpeek32(0x0)); |
There was a problem hiding this comment.
I want to understand why this line changed from fpeek32(0x0) >> 16 to this. Was it always a bug or did this change happen at some point in the FPGA history and could break on older FPGA revisions?
| int chosen_read_u32(const char *name, uint32_t *value) | ||
| { | ||
| char path[256]; | ||
| FILE *fp; | ||
| uint32_t be_val; | ||
| size_t n; | ||
|
|
||
| snprintf(path, sizeof(path), "/sys/firmware/devicetree/base/chosen/%s", name); | ||
|
|
||
| fp = fopen(path, "rb"); | ||
| if (!fp) | ||
| return errno; | ||
| n = fread(&be_val, 1, sizeof(be_val), fp); | ||
| fclose(fp); | ||
|
|
||
| if (n != sizeof(be_val)) | ||
| return EIO; | ||
|
|
||
| *value = be32toh(be_val); | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
This all uses 4 space indenting, should be tabs
| if (ret) { | ||
| fprintf(stderr, "cpu-options missing from device-tree chosen.\n" | ||
| "This may be an outdated u-boot\n"); | ||
| return; | ||
| } | ||
|
|
||
| ret = chosen_read_u32("io-options", &io_options); | ||
| if (ret) { | ||
| return; | ||
| } | ||
|
|
||
| ret = chosen_read_u32("io-model", &io_model); | ||
| if (ret) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Rest of codebase looks like it does not add braces for single line following an if, suggest these change to
if (ret)
return;
for consistency.
EDIT: This would only apply to lines 41-49 above, I accidentally highlighted too far back.
| if (n != sizeof(be_val)) | ||
| return EIO; |
There was a problem hiding this comment.
Something to keep in mind in the future, when returning an error code we probably want to follow along with how errno handles it. e.g. return -1 and then set errno appropriately for consistency. Either that, or, as the kernel does it, return a negative code e.g. -EIO
| if (!fp) | ||
| return errno; |
There was a problem hiding this comment.
As above, if we just return -1 here, errno would get passed just the same.
|
Otherwise, re-tested with recent U-Boot and verified the changes all look good and give correct output. |
|
@markfeathers Would you like me to integrate the changes? The only real feedback I care about is: #6 (comment) as this seems like a breaking change. |
Fixes reading fpga rev/straps on the TS-7100
Fixes #1 and ships splash-convert on make install