adafruit / Adafruit_nRF52_Bootloader

USB-enabled bootloaders for the nRF52 BLE SoC chips
MIT License
450 stars 409 forks source link

Use separate UF2_FAMILY for nRF52833 #256

Closed xudongzheng closed 2 years ago

xudongzheng commented 2 years ago

Per my understanding, the logic in src/usb/uf2/ghostfat.c will prevent nRF52840 UF2 firmware from being flashed to nRF52833 and vice versa (so long as uf2conv.py is invoked with the correct UF2_FAMILY token).

jpconstantineau commented 2 years ago

Will this have an impact on circuitpython UF2 builds? I suspect they use the same family. There are no Bluemicro833 in the wild but I am not sure about the pca10100. Is this related to ZMK or other firmwares?

jpconstantineau commented 2 years ago

If this is merged in, changes to the CircuitPython builds will be needed. See makefile here @tannewt comments on how to proceed?

xudongzheng commented 2 years ago

I think there would be a case for keeping 0xADA52840 if there are lots of nRF52833 boards with the existing bootloader out there.

I have some plans to run ZMK on nRF52833 but other stuff as well.

Wouldn't CircuitPython already be building different firmware for nRF52833 compared to nRF52840? I'm not too familiar with it. Is the Python script also deployed via UF2?

hathach commented 2 years ago

yeah, it should use different uf2 family to prevent incompatible updatring, before merging this PR. you need to submit an PR to uf2 specs to update https://github.com/microsoft/uf2/blob/master/utils/uf2families.json .

Note: I am not sure if uf2 maintainer would agree to use ada52833, if not you could just generate a random number.

jpconstantineau commented 2 years ago

I agree we should get a new number. Now is the time to make it right and get the changes done here and at other places (CP). There aren't too many of these nrf52833 out there yet. And those people who have them would more likely have a dev kit where they can update the bootloader easily.

xudongzheng commented 2 years ago

I've changed the UF2_FAMILY to 0x621e937a.