SpenceKonde / DxCore

Arduino core for AVR DA, DB, DD, EA and future DU-series parts - Microchip's latest and greatest AVRs. Library maintainers: Porting help and adviccee is available.
Other
181 stars 47 forks source link

Feature request AVR*DB*: Disable PORTD.PIN0CTRL input in DxCore #76

Closed Miraculix200 closed 3 years ago

Miraculix200 commented 3 years ago

Problem: Due to a silicon bug PORTD pin 0 (VDDIO2) consumes energy (like >15uA), when the input is not disabled.

I've looked at the dxcore code, and I'm not sure if it's possible to read the silicon version. But if it is, and there is enough space for this addition, you may want to disable the input on certain AVRDB silicon versions. E.g. like this

PORTD.PIN0CTRL= PORT_ISC_INPUT_DISABLE_gc;

While it can easily be disabled in each application code, it will probably save some time for new users who didn't RTFM trying to figure out why their sleeping MCU consumes so much energy.

SpenceKonde commented 3 years ago

This should already be that way? Are you sure it's not already done? from init() in wiring,c:

  #if ((defined(DB_28_PINS) || defined(DB_32_pins)) && !defined(NO_PIN_PD0_BUG))
    // PD0 does not exist on these parts - VDDIO2 took it's (physical) spot.
    // but due to a silicon bug, the input buffer is on, but it's input is floating. Per errata, we are supposed to turn it off.
    PORTD.PIN0CTRL=PORT_ISC_INPUT_DISABLE_gc;
  #endif

You can read the silicon rev, it's SYSCFG.REVID (I think there's even a DxCore "example" that prints that as well as a the chip serial number) - but since it's not compiletime known, I don't expect to actually test it to determine whether to use the workaround unless using the workaround causes problems on parts that don't have the silicon bug.

Bootloader doesn't have that - I'd rather not put things like that into the bootloader:

  1. Not sure if we even have the spave on the 128k parts... with the Optiboot style nvm hook for writing to flash from app, we had literally 0 bytes left on the 128k parts, and making it fit, I was getting desperate, rewriting routines in assembly just to save 1 or 2 instruction words and shit like that. We have a bit more breathing room with the new self-programming hook which is only 2 words long (the optiboot method was a port of what you do for parts with V1 of NVMCTRL, like the 0/1-series tinies/megas; on those there is a temporary page buffer, which must be written to from the boot section (via st aimed at the address in the mapped flash), and to actually write the page, you need to write the appropriate command to NVMCTRL.CTRLA from within the boot section - so it has to do two things. But with the NVMCTRL version in Dx-series, writing to NVMCTRL.CTRLA is not restricted by where you're executing from (I'm surprised this is checked on the 0/1-series, but the esteemed Mr. Westfield says it is and I'm not about to get into a debate over this); The new NVMCTRKL only does any checking of whether you're executing from an appropriately privileged part of the flash when you actually write to the flash using the spm instruction (to write a word) or the st instruction aimed at mapped flash (note that the latter is not an option on 128k DA-series because of silicon errata), So my write-flash-from-app entry point is just spm z+ ret just before the version number at end of bootloader.
  2. Even if we can fit it (we probably can, now - I think it would be only 6 bytes (ldi, sts) if we don't check whether it needs to be done), that would mean we would need a different bootloader binary for 28 and 32 pin DB-series parts than is used for 48+ pin parts or 28/32-pin DA parts. Meaning twice as many bootloader binaries. (I don't know if you noticed, but we currently get away with the same binaries for all the DA/DB parts of a given amount of flash ;-) We can't re-use them on the DD's though - based on the product brief (no datasheets or headers released yet), they must have changed PORTMUX.USARTROUTEA layout, since there are too many pin mappings for USART0 to fit in the DA/mega0 layout.
Miraculix200 commented 3 years ago

Ah, nice. I encountered the problem when using DxCore 1.2.something. Didn't know it was fixed.

And yes, doesn't make sense to put this into the bootloader, when you can also put it into wiring.c. I didn't think it through when I created the feature request.