From 774ec26e3bf2f49ad2c505711386e2abe334a346 Mon Sep 17 00:00:00 2001 From: Andreas Fritiofson Date: Thu, 13 Jun 2024 11:32:51 +0200 Subject: [PATCH] Verify flash contents after programming Add an option, on by default, to read out flash contents after programming and compare it to the programmed data. Without it, bugs in flash loader algos and other problems will silently leave flash corrupted. --- pyocd/core/options.py | 2 ++ pyocd/flash/builder.py | 40 +++++++++++++++++++++++----------- pyocd/flash/file_programmer.py | 9 ++++++-- pyocd/flash/loader.py | 11 ++++++++-- pyocd/subcommands/load_cmd.py | 3 ++- 5 files changed, 47 insertions(+), 18 deletions(-) diff --git a/pyocd/core/options.py b/pyocd/core/options.py index 8e3d9b3b4..173fc31b0 100644 --- a/pyocd/core/options.py +++ b/pyocd/core/options.py @@ -146,6 +146,8 @@ class OptionInfo(NamedTuple): "Name of test firmware binary."), OptionInfo('user_script', str, None, "Path of the user script file."), + OptionInfo('verify', bool, True, + "Controls whether a verify pass should be performed after flash programming has finished. Default is True"), OptionInfo('warning.cortex_m_default', bool, True, "Whether to show the warning about use of the cortex_m target type. Default is True."), diff --git a/pyocd/flash/builder.py b/pyocd/flash/builder.py index 3b6ff45d5..b56efca92 100644 --- a/pyocd/flash/builder.py +++ b/pyocd/flash/builder.py @@ -132,6 +132,11 @@ def mark_all_pages_not_same(self): for page in self.page_list: page.same = False + def mark_all_pages_unknown(self): + """@brief Sets the same flag to False for all pages in this sector.""" + for page in self.page_list: + page.same = None + def __repr__(self): return "<_FlashSector@%x addr=%x size=%x wgt=%g pages=%s, subsectors=%d>" % ( id(self), self.addr, self.size, self.erase_weight, self.page_list, self.n_subsectors) @@ -417,7 +422,7 @@ def add_page_with_existing_data(): page = add_page_with_existing_data() sector_page_addr += page.size - def program(self, chip_erase=None, progress_cb=None, smart_flash=True, fast_verify=False, keep_unwritten=True, no_reset=False): + def program(self, chip_erase=None, progress_cb=None, smart_flash=True, fast_verify=False, keep_unwritten=True, no_reset=False, verify=True): """@brief Determine fastest method of flashing and then run flash programming. Data must have already been added with add_data(). @@ -444,6 +449,8 @@ def program(self, chip_erase=None, progress_cb=None, smart_flash=True, fast_veri be read from memory and restored while programming. @param no_reset Boolean indicating whether if the device should not be reset after the programming process has finished. + @param verify Boolean indicating whether a verify pass should be performed after the + programming process has finished. """ # Send notification that we're about to program flash. @@ -535,12 +542,6 @@ def program(self, chip_erase=None, progress_cb=None, smart_flash=True, fast_veri else: flash_operation = self._sector_erase_program(progress_cb) - # Cleanup flash algo and reset target after programming. - self.flash.cleanup() - - if no_reset is not True: - self.flash.target.reset_and_halt() - program_finish = time() self.perf.program_time = program_finish - program_start self.perf.program_type = flash_operation @@ -571,6 +572,23 @@ def program(self, chip_erase=None, progress_cb=None, smart_flash=True, fast_veri self.perf.skipped_byte_count = skipped_byte_count self.perf.skipped_page_count = skipped_page_count + if verify: + LOG.info("Verifying") + # Reset same flag for all pages to enable rescanning + for sector in self.sector_list: + sector.mark_all_pages_unknown() + self._scan_pages_for_same(progress_cb) + failed_pages = [page for page in self.page_list if page.same is False] + if failed_pages: + LOG.debug("Verify failed for pages {}".format(failed_pages)) + raise FlashProgramFailure('flash verify failure', address=failed_pages[0].addr) + + # Cleanup flash algo and reset target after programming. + self.flash.cleanup() + + if no_reset is not True: + self.flash.target.reset_and_halt() + if self.log_performance: if chip_erase: LOG.info("Erased chip, programmed %d bytes (%s), skipped %d bytes (%s) at %.02f kB/s", @@ -899,12 +917,6 @@ def _scan_pages_for_same(self, progress_cb=_stub_progress): if self.sector_erase_weight > 0: progress_cb(float(progress) / float(self.sector_erase_weight)) - # If we have to program any pages of a sector, then mark all pages of that sector - # as needing to be programmed, since the sector will be erased. - for sector in self.sector_list: - if sector.are_any_pages_not_same(): - sector.mark_all_pages_not_same() - return progress def _next_nonsame_page(self, i): @@ -937,6 +949,8 @@ def _sector_erase_program_double_buffer(self, progress_cb=_stub_progress): self.flash.init(self.flash.Operation.ERASE) for sector in self.sector_list: if sector.are_any_pages_not_same(): + # If the sector is erased, all its pages must be programmed + sector.mark_all_pages_not_same() # Erase the sector for addr in sector.addrs: self.flash.erase_sector(addr) diff --git a/pyocd/flash/file_programmer.py b/pyocd/flash/file_programmer.py index 0c1e1b8e9..fd315bd13 100755 --- a/pyocd/flash/file_programmer.py +++ b/pyocd/flash/file_programmer.py @@ -63,7 +63,8 @@ def __init__(self, smart_flash: Optional[bool] = None, trust_crc: Optional[bool] = None, keep_unwritten: Optional[bool] = None, - no_reset: Optional[bool] = None + no_reset: Optional[bool] = None, + verify: Optional[bool] = None ): """@brief Constructor. @@ -86,6 +87,8 @@ def __init__(self, be read from memory and restored while programming. @param no_reset Boolean indicating whether if the device should not be reset after the programming process has finished. + @param verify Boolean indicating whether a verify pass should be performed after the + programming process has finished. """ self._session = session self._chip_erase = chip_erase @@ -93,6 +96,7 @@ def __init__(self, self._trust_crc = trust_crc self._keep_unwritten = keep_unwritten self._no_reset = no_reset + self._verify = verify self._progress = progress self._loader = None @@ -152,7 +156,8 @@ def program(self, file_or_path: Union[str, IO[bytes]], file_format: Optional[str smart_flash=self._smart_flash, trust_crc=self._trust_crc, keep_unwritten=self._keep_unwritten, - no_reset=self._no_reset) + no_reset=self._no_reset, + verify=self._verify) # file_obj = None # Open the file if a path was provided. diff --git a/pyocd/flash/loader.py b/pyocd/flash/loader.py index d766fa610..69267c5ca 100755 --- a/pyocd/flash/loader.py +++ b/pyocd/flash/loader.py @@ -143,6 +143,7 @@ class MemoryLoader: _trust_crc: Optional[bool] _keep_unwritten: Optional[bool] _no_reset: Optional[bool] + _verify: Optional[bool] def __init__(self, session: "Session", @@ -151,7 +152,8 @@ def __init__(self, smart_flash: Optional[bool] = None, trust_crc: Optional[bool] = None, keep_unwritten: Optional[bool] = None, - no_reset: Optional[bool] = None + no_reset: Optional[bool] = None, + verify: Optional[bool] = None ): """@brief Constructor. @@ -174,6 +176,8 @@ def __init__(self, be read from memory and restored while programming. @param no_reset Boolean indicating whether if the device should not be reset after the programming process has finished. + @param verify Boolean indicating whether a verify pass should be performed after the + programming process has finished. """ self._session = session assert session.board @@ -198,6 +202,8 @@ def __init__(self, else self._session.options.get('keep_unwritten') self._no_reset = no_reset if (no_reset is not None) \ else self._session.options.get('no_reset') + self._verify = verify if (verify is not None) \ + else self._session.options.get('verify') self._reset_state() @@ -296,7 +302,8 @@ def commit(self): smart_flash=self._smart_flash, fast_verify=self._trust_crc, keep_unwritten=self._keep_unwritten, - no_reset=self._no_reset) + no_reset=self._no_reset, + verify=self._verify) perfList.append(perf) didChipErase = True diff --git a/pyocd/subcommands/load_cmd.py b/pyocd/subcommands/load_cmd.py index 29f7b2ef2..00704912e 100644 --- a/pyocd/subcommands/load_cmd.py +++ b/pyocd/subcommands/load_cmd.py @@ -101,7 +101,8 @@ def invoke(self) -> int: programmer = FileProgrammer(session, chip_erase=self._args.erase, trust_crc=self._args.trust_crc, - no_reset=self._args.no_reset) + no_reset=self._args.no_reset, + verify=self._args.verify) for filename in self._args.file: # Get an initial path with the argument as-is. file_path = Path(filename).expanduser()