discostuboogaloo / optiboot

Automatically exported from code.google.com/p/optiboot
0 stars 0 forks source link

Drop support for memory reads/writes with length longer than 256 bytes #110

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
AVR061: STK500 Communication Protocol specifically states that the length 
argument within the Cmnd_STK_PROG_PAGE and Cmnd_STK_READ_PAGE commands should 
not be larger than 256 bytes. Therefore the uint8_t for the length argument of 
writebuffer() and read_mem() would be sufficient, saving a couple of bytes.

The high byte could be dropped upon reception.

Original issue reported on code.google.com by johnpatc...@googlemail.com on 10 Aug 2014 at 1:06

GoogleCodeExporter commented 8 years ago
I've attached a small patch outlining the idea. At least for me (avr-gcc 4.9.0) 
it saves 24 bytes and works just fine with avrdude 6.1.

Original comment by johnpatc...@googlemail.com on 10 Aug 2014 at 1:16

GoogleCodeExporter commented 8 years ago
I've uploaded the wrong file beforehand, it should be the right one this time 
;).

Original comment by johnpatc...@googlemail.com on 10 Aug 2014 at 1:18

Attachments:

GoogleCodeExporter commented 8 years ago
Just to be clear: A length of 256 bytes would require a high byte to represent 
it, but avrdude does not exhaust this limit and writes only data as big as a 
page (128 byte).

Original comment by johnpatc...@googlemail.com on 10 Aug 2014 at 1:42

GoogleCodeExporter commented 8 years ago
ATmega1284 and other "large" chips have a 256byte pagesize, which was the 
reason for JUST putting in the support for length larger than a byte. 
(https://code.google.com/p/optiboot/issues/detail?id=104 )

I suppose the length could be made conditional on chip pagesize; the bigger 
chips have a bigger minimum bootloader size, anyway...

Original comment by wes...@gmail.com on 14 Aug 2014 at 6:22

GoogleCodeExporter commented 8 years ago
I've totally forgotten about the ATmega1284 and other chips with bigger 
pagesizes. However I would vote for the conditional approach then, because it 
saves about 25 bytes.

BTW: I forgot to change the type of the length variable within main(), which 
saves another couple of bytes. Overall this modification is worth the effort - 
at least in my book, as we are talking about a reduction in code size of about 
5 - 10 percent. Maybe it would even be possible to fit the EEPROM support in 
with some more tweaks.

Original comment by johnpatc...@googlemail.com on 14 Aug 2014 at 5:53

GoogleCodeExporter commented 8 years ago
How does this look?

Original comment by wes...@gmail.com on 17 Aug 2014 at 4:06

Attachments:

GoogleCodeExporter commented 8 years ago
Haven't tested it, but looks good and straightforward to me. Personally I would 
define GETLENGTH(l) for uint8_t different, so that there would be no assignment 
to "l" for the high byte, but I guess the compiler is smart enough to figure it 
out. Could be advisable to increase readability, though.

Therefore I suggest the following (once again untested ;)):

#if SPM_PAGESIZE > 255
typedef pagelen_t uint16_t;
#define GETLENGTH(l) l = getch()<<8; l |= getch()
#else
typedef uint8_t pagelen_t;
#define GETLENGTH(l) (void)getch() /* skip high byte */; l = getch()
#endif

Original comment by johnpatc...@googlemail.com on 17 Aug 2014 at 10:49

GoogleCodeExporter commented 8 years ago
Committed (slightly different) code in version 6.2

Original comment by wes...@gmail.com on 22 Aug 2014 at 3:41