Allow loading an OSCCAL value from EEPROM

[matthijskooijman]

When an EEPROM address is given through OSCCAL_EEPROM_ADDR at
compiletime, a byte is read from that address on startup and written to
OSCCAL (unless it is 0xff, which should never be a meaningful
calibration value). This allows improving the accuracy of the UART
baudrate when running off the internal oscillator.

Enabling this option adds 30 bytes on the atmega328p build. This can
probably be reduced by inlining eeprom_read_byte.

[matthijskooijman]

Note that I think inlining eeprom_read_byte should probably happen before
merging this PR, but I wanted to get some feedback on the general idea
first, before diving into this (also, I couldn't find the avr-libc
implementation of eeprom_read_byte() just now).

Looking at the assembly output, removing the overhead of the function
call and removing the polling of EEPE (since we're sure no EEPROM
write is in progress) should reduce the cost of this feature to 18 bytes
on an atmega328p.

[matthijskooijman]

(Oh, one more reason to not merge yet is that I didn't actually test this code yet, ran out of time for today when the code was about finished and wanted to at least get some feedback before shelving this for a few days)

[WestfW]

I was hoping to adjust OSCCAL without using EEPROM, perhaps by merging the calibration with the burning algorithm, and modifying the code on the fly, or something. Figuring out what to use as a clock reference is ... annoying, however :-(

[matthijskooijman]

@WestfW, that's actually a good idea, to store the calibration byte in flash instead of EEPROM. Doing the calibration on the fly (i.e. in the bootloader) will almost certainly be too big to fit in 512 bytes along with the bootloader, but perhaps that's not what you meant.

I've been working on an automated calibration and bootloader flashing tool (running standalone on an Arduino, based on Nick Gammon's Atmega_Board_Programmer sketch), which does calibration by uploading a sketch to the target MCU that calibrates against an external 32kiHz crystal, and writes the result to EEPROM. I can probably modify that sketch to instead write the result through SPI back to the programmer, which can then insert the value into the bootloader image during flash. One common alternative is to let the programmer generate a clock on the MOSI line instead of using a 32kiHz crystal.

As for getting the value into the bootloader, I can see a few approaches:

1. Create a PROGMEM const that contains a dummy value, and add a bit of code that loads that byte from PROGMEM and stores it to OSCCAL (optionally ignoring a 0xff value so that if no calibration byte was added, no value is written at startup). Setting the actual calibration value is then done by overwriting the PROGMEM variable, whose location can either be automatically be taken from the .elf file, or manually figured out. This approach probably takes 15 bytes of flash.
2. Add some code that loads a hardcoded value to OSCCAL (unconditionally), and then change the resulting machine code. The compiler would generate a ldi and sts instruction, and the ldi would be modified to change the value to be written. ldi divides the value to load over its two instruction bytes, so this is a bit more complicated than just changing a single byte in the bootloader image. Also, if no calibration byte is set while flashing the bootloader, some hardcoded default byte is written to OSCCAL, which will be worse than the factory calibration in some cases, making this approach less desirable. This approach would be the smallest possible, namely 6 bytes of flash.
3. Add some NOPs to the bootloader, and while flashing the bootloader replace those nops with the required ldi and sts instructions (with the right calibration value encoded in them). This approach also takes 6 bytes, but has the advantage that if the bootloader is written without any changes, the OSCCAL value is untouched. The downside is that full instructions must be generated by or hardcoded in the bootloader flashing tool, including the (device-dependent) location of the OSCCAL register.
4. Let the bootloader use pgm_load_word or something similar to load the calibration bytes from the last 2 bytes of flash. This requires that the bootloader compiled size is 2 bytes smaller than the available space. This is essentially the same as option 1, but with a hardcoded location in flash instead of a PROGMEM variable that is placed by the linker.

I'm inclined to go with either option 1 or 3, of which option 1 is probably the easiest and cleanest to implement. Any thoughts?

Apart from the above, one upside of storing the result in EEPROM is that you can reflash the bootloader without having to re-calibrate, since the previous value is just saved. In any case, it seems acceptable to also add code to read OSCCAL from EEPROM (as in this PR now), since there might be cases where EEPROM is preferred over inserting into flash directly. Users can then just select either option based on what they need.

[matthijskooijman]

@WestfW, any chance you could have a look at my previous comment? My need for loading calibration bytes from the bootloader has become a bit more urgent again, so I'll be investing some time to implement this soon (but I'd rather implement something that is eligible for merging).

[matthijskooijman]

Oh, just thought of another option, I edited it into the above list.

[matthijskooijman]

I ended up implementation a combination of option 1 and 4. The OSCCAL value is stored in progmem variable defined in the source, but by putting it in a special section (just like the version number), its position is fixed at the end of the flash space (just before the version number, which has not moved). I've added this as a new commit, keeping the EEPROM implementation as well (since I expect that might also be useful for some people, and both can happily coexist).