Tomas,
Thank you! The patch looks great! Will test it today and report back.
However, one bug seems still to exist. In samd_protect() you iterate over
bank->prot_blocks[] however you start at a sector first and go to sector
last. The sector numbers are most probably larger than the num_prot_blocks
(16) so you will go off the end of the array. I suggest you find the
encompassing lock region (block) that contains each sector, s: [first,
last], and then protect that instead.
- Mark
On Thu, Dec 22, 2016 at 1:30 AM, Tomas Vanek <tom_...@users.sourceforge.net>
wrote:
> Please try
> http://openocd.zylin.com/3546
> and give a review.
>
> Tomas
>
>
> On 21.12.2016 23:56, Mark Odell wrote:
>
> I have been using a small SAMD21E16B (64k/8k) and thought things were
> fine. Then I switched to a bigger SAMD21J18A (256k/32k) part and bad things
> happened.
>
> Version
> -------
> Open On-Chip Debugger 0.10.0-dev-00419-gbcaf775-dirty (2016-12-21-14:48).
> I am a bit behind master but the file in question, at91samd.c, is the same.
>
> Background
> ----------
> I have an 8kB bootloader at address zero and an application that begins at
> 0x2000. On the 'E16B part I could SWD program the bootloader at 0 and then
> SWD program the application at 0x2000 using gdb and openocd. This does not
> work with the bigger flash size on the 'J18A though. I would SWD program
> the bootloader and my application would get erased and vice versa.
>
> Investigation
> -------------
> I ran openocd under control of my host gdb and started snooping around. I
> found that flash/nor/at91samd.c had hard coded the number of sectors to 16.
> This is not actually the case. There are exactly 16 *LOCK* regions but the
> number of erase rows (sectors?) varies depending upon the size of the flash
> array.
>
> When the chip->sector_size is calculated it just takes the size of the
> flash array and divides it by SAMD_NUM_SECTORS (16). For the 'E16B I got
> lucky because 64k/16 = 4k and my bootloader fit under 8k and my application
> started at 0x2000 (8k). Thus, gdb was able to erase and program the
> bootloader and the application without erasing the other.
>
> For the 'J18A chip->sector_size is 256k/16 = 16k. So now when gdb programs
> the bootloader he erases beyond the 8k and blows away part of my
> application. Likewise, to program the application gdb will now erase the
> bootloader as well. Not good.
>
> Design Issue
> ------------
> There appears to be a design decision to link the protection region size
> to the erase granularity (size). This provides correctness when it comes to
> locking flash regions but is not particularly friendly to programming
> smaller-than-lock-region sized programs when there are more than one copy
> of these programs to be flashed at different times as is the case for me
> and my bootloader + application.
>
> Work-Around
> -----------
> In my sandbox I have removed SAMD_NUM_SECTORS and added
> SAMD_PAGES_PER_SECTOR and SAMD_LOCK_REGION_SIZE. Next I set
> chip->sector_size = chip->page_size * SAMD_PAGES_PER_SECTOR (4) and
> bank->num_sectors = bank->size / chip->sector_size. Then I realized that
> samd_protect_check() would be wrong because my bank->num_sectors would be
> large, far larger than the 16 that were assumed so just set all the
> is_protected flags to zero and ignored the 16-bit LOCK register value. This
> is okay for me at this point since I don't lock any regions. Finally I had
> to update samd_erase() to loop over the now-correct number of actual
> sectors (rows) instead of a fixed 16 sectors with a sub-iteration of rows
> per sector.
>
> Now gdb and openocd can correctly erase and program all SAMD parts with
> the correct erase sector size (4 x page_size = 256 bytes) however I cannot
> lock flash regions. I'm not sure what it really means to lock a region at
> this point either. If a lock region on the 'J18A is 16kB and I want to lock
> a single 256 sector (row) then I simply cannot do it; the chip will lock
> the entire encompassing 16k region. What to do? Expose the lock region size
> to gdb? Does gdb know about locking something other than a sector? Should
> locking regions be moved to an atxxxx command like chip_erase?
>
> Not sure what to do. Any suggestions?
>
> --
> - Mark
> ( <mrfirmw...@gmail.com>mrfirmw...@gmail.com)
>
>
> ------------------------------------------------------------------------------
> Developer Access Program for Intel Xeon Phi Processors
> Access to Intel Xeon Phi processor-based developer platforms.
> With one year of Intel Parallel Studio XE.
> Training and support from Colfax.
> Order your platform today.http://sdm.link/intel
>
>
>
> _______________________________________________
> OpenOCD-devel mailing
> listOpenOCD-devel@lists.sourceforge.nethttps://lists.sourceforge.net/lists/listinfo/openocd-devel
>
>
>
------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/intel
_______________________________________________
OpenOCD-devel mailing list
OpenOCD-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openocd-devel
