oxidecomputer / phbl

Pico Host Boot Loader
Mozilla Public License 2.0
101 stars 7 forks source link

review: A PR for reviewing phbl #1

Closed dancrossnyc closed 2 years ago

dancrossnyc commented 2 years ago

Signed-off-by: Dan Cross cross@oxidecomputer.com

daym commented 2 years ago

start.S:

I suggest a comment on SEG32_LIMIT that that's 4 GiB on the def'n of SEG_WRITE.

Dan:

Is it work putting a comment on SEG_CODE saying that it's read-only?

Yes, worth it

Also, personally, I'd also have a SEG32_BASE = 0 << 16. I know it's technically not necessary--but if you have a limit and no base reads fishy.

dancrossnyc commented 2 years ago

start.S:

I suggest a comment on SEG32_LIMIT that that's 4 GiB on the def'n of SEG_WRITE.

Dan:

Is it work putting a comment on SEG_CODE saying that it's read-only?

Yes, worth it

Also, personally, I'd also have a SEG32_BASE = 0 << 16. I know it's technically not necessary--but if you have a limit and no base reads fishy.

All of the above done.