abcminiuser / lufa

LUFA - the Lightweight USB Framework for AVRs.
http://www.lufa-lib.org
1.04k stars 325 forks source link

HID Bootloader does not work #76

Closed NicoHood closed 8 years ago

NicoHood commented 8 years ago

I tried the c program which seems to work now. However I got serious bootloader problems now:

The sketch will not load. It seems to be a magic key problem but I noticed several weird errors. It must be something simple i guess? I even tried a fixed bootkey address which the arduino team uses:

//uint16_t MagicBootKey ATTR_NO_INIT;

volatile uint8_t *const MagicBootKeyPtr = (volatile uint8_t *)0x8000;
#define MagicBootKey *MagicBootKeyPtr

/** Special startup routine to check if the bootloader was started via a watchdog reset, and if the magic application
 *  start key has been loaded into \ref MagicBootKey. If the bootloader started via the watchdog and the key is valid,
 *  this will force the user application to start via a software jump.
 */
void Application_Jump_Check(void)
{
    LEDs_Init();

    LEDs_SetAllLEDs(0);

    if(MCUSR & (1 << WDRF))
    LEDs_TurnOnLEDs(LEDS_LED1); //rx

    if(MagicBootKey == MAGIC_BOOT_KEY)
    LEDs_TurnOnLEDs(LEDS_LED2); //tx

    /* If the reset source was the bootloader and the key is correct, clear it and jump to the application */
    if ((MCUSR & (1 << WDRF)) && (MagicBootKey == MAGIC_BOOT_KEY))
    {
        MagicBootKey = 0;

        // cppcheck-suppress constStatement
        ((void (*)(void))0x0000)();
    }
}

But only the first led goes on.

MCU          = atmega32u4
ARCH         = AVR8
BOARD        = LEONARDO
F_CPU        = 16000000
F_USB        = $(F_CPU)
OPTIMIZATION = s
TARGET       = BootloaderHID
SRC          = $(TARGET).c Descriptors.c $(LUFA_SRC_USB)
LUFA_PATH    = ../../LUFA
CC_FLAGS     = -DUSE_LUFA_CONFIG_HEADER -IConfig/
LD_FLAGS     = -Wl,--section-start=.text=$(BOOT_START_OFFSET)

# Flash size and bootloader section sizes of the target, in KB. These must
# match the target's total FLASH size and the bootloader size set in the
# device's fuses.
FLASH_SIZE_KB        := 32
BOOT_SECTION_SIZE_KB := 4

[...]

avrdude:
    avrdude -patmega32u4 -cstk500v1 -P/dev/ttyACM0 -b19200 -e -Ulock:w:0x3F:m -Uefuse:w:0xCB:m -Uhfuse:w:0xD8:m -Ulfuse:w:0xFF:m
    avrdude -patmega32u4 -cstk500v1 -P/dev/ttyACM0 -b19200 -Uflash:w:./$(TARGET).hex:i -Ulock:w:0x2F:m

Edit: The leds were off because of my wrong bootkey address "fix". The problem was something different, see PR #77

NicoHood commented 8 years ago

Also One question: Why does the HID Bootloader has no manufacturer string etc? Is this a matter of code size? I'd rather add it to make it more complete in my version.

abcminiuser commented 8 years ago

Great @NicoHood - I'll close this issue to keep things tidy (but feel free to continue the discussion, I still read comments on all issues) since the PR supersedes it.

Yes, the manufacturer string was removed to save space. I need to re-check with the latest code across all devices, but at least a while ago I was only barely fitting it into the 2KB allotted on the smaller 2 series (ATMEGA*U2, AT90USB*2) devices.

NicoHood commented 8 years ago

With avr-gcc 5.3 and lto it fits FYI: https://github.com/NicoHood/AVR-Development-Environment-Script

Before you merge the PR, just a little note: It would make sense to also apply this patch to the code, as it reduces the code even more: https://github.com/NicoHood/SecureLoader/commit/afda49c36788595e0c3f2e61a35b3e0dfdecf209

Also a do while loop in the main function would improve the loop a tiny bit.

Event though I copied clock_prescale_set(clock_div_1); from the CBC bootloader, is it really required? Why was this used?

NicoHood commented 8 years ago

This is also wrong: https://github.com/abcminiuser/lufa/blob/master/Bootloaders/HID/Descriptors.c#L178

Fix: https://github.com/abcminiuser/lufa/blob/master/Demos/Device/LowLevel/KeyboardMouse/Descriptors.c#L327

Edit: Fix merged!

BTW: I would not close this issue until the PR is merged. But I need some more time for the PR to compare what I've changed in my own Project.

NicoHood commented 8 years ago

This patch should also be applied to allow the start bootloader command to be successful recognized by the host: https://github.com/abcminiuser/lufa/commit/e3988f19dda2345032f575f31f275693571d9267

Edit: moved to https://github.com/abcminiuser/lufa/issues/116