stlink-org / stlink

Open source STM32 MCU programming toolset
BSD 3-Clause "New" or "Revised" License
4.38k stars 1.24k forks source link

[feature] --mass-erase for st-flash write commands #1397

Closed hannesweisbach closed 4 months ago

hannesweisbach commented 4 months ago

Hello,

I have an STM32H7 where flash verification fails (probably somehow related to #1249 and/or #1294). #1238 indicates that a --mass-erase-flag for st-flash write would be welcome.

Open points:

Best regards, Hannes

(Closes #1238) (Closes #1366)

Ant-ON commented 4 months ago

@hannesweisbach

My thoughts on open points:

In general, I think it’s not worth adding mass_erase field to struct _stlink. It would be nice to add a erase type parameter to the stlink_write_flash and stlink_mwrite_flash functions.

hannesweisbach commented 4 months ago

I changed the the type of the mass_erase member in flash_opts to int32_t. I also introduced an enum erase_type_t in flash_common.h, which now gets passed to stlink_fwrite_flash, stlink_mwrite_flash, and stlink_write_flash.

I think it's a bit unclean in the regard that calling stlink_fwrite_flash for OTP-programming ignores this argument alltogether, but oh well, you can't win them all ;)

With the introduction of the erase_type-flag, I also removed themass_erasefield fromstruct stlink`.

Regarding the masserase command: I found stlink erase quite intuitive.

Do you imagine a command like: stlink mass_erase (i.e. completely new command), or stlink --mass-erase erase [...] (i.e. if the flag is given, the erase command ignores additional arguments and performs a mass erase)?

Ant-ON commented 4 months ago

@hannesweisbach Hmm, you're right. With OTP calls it really looks a little illogical. We can replace MASS_ERASE to something like NO_ERASE, then everything will look more logical. I imagined stlink --mass-erase erase without address and size.

hannesweisbach commented 4 months ago

I changed the erase command such that the --mass-erase is observed. I added it as separate commit only for review reasons. I'll squash the commit before merging.

I have NO_ERASE yet to implement.

Nightwalker-87 commented 4 months ago

@hannesweisbach Thank you very much for the update. There is no need to squash the commits. An upcoming merge will add all separate changes at once and basically has the same effect seen from the perspective of the destination branch.

hannesweisbach commented 4 months ago

Yes, I know. I'm always unhappy with the resulting commit message though. Atleast on GitLab. I haven't done merge squashes with the UI on Github.

hannesweisbach commented 4 months ago

Sorry for the delay. I squashed the commits and cleaned the commit message up (no code changes).

Nightwalker-87 commented 4 months ago

Yes, I know. I'm always unhappy with the resulting commit message though. Atleast on GitLab. I haven't done merge squashes with the UI on Github.

@hannesweisbach: I've noticed that you force pushed in order to squash. It shall be ok here, but please take into account for future contributions that this is not considered a good practise for technical reasons: https://docs.github.com/de/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-pull-requests.