OTP whitelabel options for RP2350 boards

Development
61

I’m glad I was able to do justice to your work on the OTP, thank you so much for the feedback.

I’m taking a slight break from this. I want to sync with you when available. Also, the “you burn it, it’s ruined” work flow has given me anxiety nightmares so I’d like that to settle down as well :slight_smile:

1 Like
62 
    OTP_DIRTYPE_DEVICE_NAME = 0x0005,
    OTP_DIRTYPE_DEVICE_VERSION = 0x0006,
    OTP_DIRTYPE_DEVICE_REVISION = 0x0007,
    OTP_DIRTYPE_PRODUCTION_LOCATION = 0x0008,
    OTP_DIRTYPE_MANUFACTURER = 0x0009,
    OTP_DIRTYPE_PRODUCTION_DATE = 0x000A,

I understand why you propose this change, and it does seem much more logical than compounding various different data layouts.

It does seem like a waste to have a lot of blank records though, so let’s try to fill the last (first) directory page as much as possible.

63

It’s a non-goal to fill the directory page. It’s better to think of it as a fixed cost. If one tries to fill it now, and then three revisions later we wasted some entries that cause it to overflow to the next page, then … no good.

At the same time, I agree that there is no need to attempt to “save” entries. As soon as there is a use for them, let’s add them. :slight_smile:

I’ve been knocked out by flu, so have done nothing related to buspirate. Give me a week before I am likely productive again…

1 Like
64

Wish you a speedy recovery.

The flu did me in a few weeks ago, and now have the worst head cold maybe ever. I wasn’t particularly productive today and will also be out another day or two.

Hope you recover quickly!

1 Like
65

And you as well!
/ \ more chars / \

1 Like
66

Hi @ian,

Likely will be able to look at this more soon. Want to verify I don’t waste time, so I have four questions for your approval / answer:

  1. Can you approve the “big change” for the OTP directory:

Modify any existing concept of the “otp directory” structure to store records lengths in units of bytes / uint8_t (not units of OTP Rows aka uint16_t).

  1. Can you tell me which branch I should base the OTP directory entry updates / helper APIs on?

I have my own branch where I’ve kept my musings for the directory entry stuff, and want to clean it up for consideration and integration into your recommended branch.

  1. Can we have a binding, irrevocable pact requiring that, for strings having an OTP DIRECTORY ENTRY type, they will ALWAYS be stored with the trailing NULL byte, and that the trailing NULL byte will be included in the size field of the directory entry? (e.g., directory entry’s size == strlen(string)+1, and that last byte will always be NULL; to do otherwise is inviting a world of hurt)

  2. Can you confirm I should move forward with the API I previously posted (somewhere), modeled after the FindFirst() / FindNext() file-system style enumeration API is what I’d been targeting, with each core having its own context (so no synchronization between cores is required)?

Thank you!

67
  1. Yes , certainly. I agree.
  2. I’ve pushed some stuff to otp_whitelabel in the otp (not dump) and cert commands. Nothing polished or refined, and may duplicate work you’ve already done. Specifically, I was working on a “big function” to verify OTP range is free and not protected, write to OTP, verify OTP, optionally lock pages, verify locked pages, optionally write a directory entry. it includes the CRC16 write/check. It is kind of a mess, but I got it figured out and to the point where I wanted to wait on further discussion with you.
  3. Yes of course.
  4. A proper API sounds good. I wrote a simple function to search for a specific entry type, but it is very very basic and pretty specific to my code.
1 Like
68

OK, I will start from your otp_whitelabel project.

I will unapologeticaly rip and replace existing functions based on personal preferences.

I will start with small functions (I have most already written). In part, this is because of the unknown edge case behavior for the bootrom when reading ECC encoded rows … I recommend only reading RAW so better debugging information will be available.

  • bool is_otp_range_ecc_writable(otp_row_idx start, size_t count)
  • bool is_otp_range_hardlocked(otp_row_idx start, size_t count)
  • bool is_otp_range_softlocked(otp_row_idx start, size_t count)
  • bool otp_softlock_page(otp_row_idx start, size_t count) – caller must ensure start is first item of a page, and count is multiple of page size … else will report an error
  • bool otp_hardlock_page(otp_page_idx page) – caller must ensure start is first item of a page, and count is multiple of page size … else will report an error

I have functions to read and write to the OTP, including verification before the function reports success. Those are fully tested and ready to migrate…

I will start from your otp_whitelabel branch.

1 Like
69

Feel free to tear the copper wire out of the walls and sell it for scrap :slight_smile:

I made a sloppy swerve into OTP after getting the cert working. The code is exploratory and not worth much.

Then I got the thing that’s going around, and pushing forward on OTP in that state was unpleasant. I reverted to hardware, my happy place.

2 Likes
70

OK, work is now in progress … starting with having this branch build for all platforms again.

Update for the last few days:

  • Reviewed all the new commits, took notes of areas to dig into
  • Have a decent-sized list of existing functions to expose
  • Have list of new functions to be written
  • Have list of cleanup todo
  • Have prioritized list of files to … tear down to studs and rewire :slight_smile:
  • Strong desire to enable C23 … so many quality-of-life improvements!

Software is my happy place…
It’s good for us to return to our happy(ier) place, eh?

1 Like
71

As long as it doesn’t break anything, and I can keep coding as I currently do, then it is fine with me to use C23 in otp stuff. Please avoid source wide updates though, until we see what happens.

72

@henrygab - I have two items on the whitelabel/otp:

  • BP_OTP_PRODUCT_VERSION_STRING is limited to single character, or there is a hard assert somewhere. For example, try to compile for 5XL with the extra two letters, which “6” works fine.
  • We are using the RPI unique ID in ASCII as the Board-ID, so that could be written as part of the whitelabel process instead of two separate functions.
73

I see you fixed the first point in the push, thank you so much.

74
The static assertion was valid.

For the whitelabel, I had only reserved four OTP rows, or eight total characters. Of those, the first character was a space, leaving seven characters for the actual per-board string.

// BP5XL was configured to use:
#define BP_OTP_PRODUCT_VERSION_STRING "5XL REV0"
// which gets a space inserted at front:
static const char  _product_string[] = " " BP_OTP_PRODUCT_VERSION_STRING;
// which really means:
_product_string[] = " BP5XL REV0";
//                   ....-....1.
// which as you can see is eleven characters

To make visibility and feedback easier, I’ve created draft PR 217 on github … Note that it’s to your otp_whitelabel branch.

Here's my current gameplan

  • Cleanup enough to build all platforms again
  • Byte-count instead of row count in BP_OTPDIR entries!
  • Reserve top 8 bits of BP_OTPDIR to identify classes of entries.
    • E.g., 0x8000'0000 → entry can be present multiple times
    • E.g., 0x4000'0000 → entry type is critical to further parsing (e.g., versioning increase for later records)
    • E.g., 0x2000'0000 → Embedded entry (no data, or trivial data <=16 bits stored directly within the directory entry).
  • Get helper functions, includingBP_OTPDIR related, working
    • Iterator functions based on FindFirst() / FindNext() mechanic, as agreed.
    • Helper function: Find single/first BP_OTPDIR entry of given type
    • helper function: Write string + BP_OTPDIR entry.
      • For a defined string type identifiers
      • Writes the string to requested start OTP row, ensuring null-terminated string as agreed
      • If successfully written, also creates the BP_OTPDIR entry.
    • Helper function: Find writable area, including variable criteria:
      • OTP page aligned start
      • OTP page aligned end
      • Allow soft-locked pages (for debugging purposes)
      • Disallow pages with single-bits already flipped … those are normally OK for ECC writes, because BRBP will hide that single-bit error.
    • Define entry types for:
      • each USB whitelabel string
      • BP_OTPDIRENTRY version == 1
      • … etc. …
  • Modify the USB WhiteLabel code to do the following:
    • Take as input, a structure with ALL possible input (strings, etc.)
    • If any of those entries already exist in BP_OTPDIR, validate they are identical (or fail with error)
    • Validate ALL strings as meeting unique USB whitelabel requirement BEFORE doing any OTP operations … string length offsets, strings will be in range, etc. … essentially a map of where all the data will end up.
    • Use BP_OTPDIR helper function to find page-start-aligned zone of sufficient size for whitelabel data (note: currently identical to the hardcoded 0xC0, but could be useful if an OTP area is unusable on a board).
      • Test by corrupting OTP rows 0x0C0…0x0FF first
    • Use BP_OTPDIR helper functions to write each of string entries + corresponding BP_OTPDIR entries
    • Continue as previously done…
      • Write the 16-row whitelabel structure
      • Write the whitelabel address
      • Write the whitelabel
  • Migrate the certificate stuff to use the above helper functions to find an area

One thing I currently struggle with is the process of adding new commands, and ensuring there’s at least default help text. As the project grows, this appears to be a likely source of growing pains, so I think it’ll be valuable to spend time to get a maintainable, hierarchical command set defined. Likely will be at least partly custom, due to “mode” entry / exit causing the addition and/or removal of commands as “valid” for the current state.

I’ll reply with a separate thread so discussions on improving / reorganization of the command structure can have the wisdom of all our contributors and watchers…

2 Likes
75

I suggest merging main into otp_whitelabel. I did a major cleanup of the hardware version and revision switches.

#if BP_VER !=5

For example this around the otp includes is replaced with a platform indicator.

#if RPI_PLATFORM == RP2350

I’m going to do another round updates soon, and will try to make the switches feature based instead of chip or hardware based:

BP_HW_IOEXP_595 //board that use 595 for IO
BP_HW_IOEXP_NONE // boards with no IO expander (6)
BP_HW_IOEXP_I2C //future boards with I2C IO expander

And similar for features that are shared across multiple boards (SD card vs NAND, PWM PSU vs DAC PSU, etc).

76

Ugh… That sounds painful. Ok. Let me merge what I’ve got into your branch first (as there’s no conflicts). That way it won’t have to be done twice…

77

OK, I merged the changes into your otp_whitelabel branch.

Can you please let me know when you’ve merged the stuff from the main branch? I’m off to sleep for a while…

1 Like
78

I did a merge via pull request and cleaned up the conflicts.

79

I take that back, I did something horrible :slight_smile:

Edit: I think it is merged. Somehow I merged both ways? I don’t understand what happened because I did it though a pull request and it ran checks and everything.

80

Sometimes I wish I were less predictive. :slight_smile: Yes, git is sometimes painful. If you want, I can cleanup the history (e.g., rebase) into a new branch?

1 Like