search

arduino-libraries / ArduinoLowPower

Powersave features for SAMD boards
GNU Lesser General Public License v2.1
80 stars 57 forks source link

Clock disabling code looks wrong #30

Open denisbaylor opened 4 years ago

denisbaylor commented 4 years ago

https://github.com/arduino-libraries/ArduinoLowPower/blob/6fd308af4aaf2759a13697f9e670e427eb517b3a/src/samd/ArduinoLowPower.cpp#L9

The comment at this line says "disable GCLK module". But my reading of the somewhat ambiguous documentation of the CLKCTRL register is different.

I believe each generic clock id has its own enable (although I don't see this spelled out clearly in the documentation). The documentation for CLKCTRL says:

This register allows the user to configure one of the generic clocks, as specified in the CLKCTRL.ID bit group. To write to the CLKCTRL register, do a 16-bit write with all configurations and the ID.

To read the CLKCTRL register, first do an 8-bit write to the CLKCTRL.ID bit group with the ID of the generic clock whose configuration is to be read, and then read the CLKCTRL register.

I believe what line 9 is actually doing (via a forbidden 8-bit write), is setting the enable to 0, setting the GEN field to 0 (which means select generic clock generator 0) and then applying these changes to the ID field that last happened to be set. So it's essentially disabling and corrupting the clock source for the most recently configured clock generator ID. Things will work ok if you don't intermingle this low power code with other libraries that configure clocks. In my case, trying to add an interrupt between calls to other library code (Adafruit_SSD1306) caused that other code to hang. I believe the reason that code hangs is that the clocks to SERCOM2 had been disabled by this bug.

As I understand things, this line should be:

  GCLK->CLKCTRL.reg =
      (uint16_t)(GCLK_CLKCTRL_ID(GCM_EIC));  // Disable EIC clock (also sets
                                             // GCLK_CTRL_CLKEN bit to 0)
PhilGoody commented 3 years ago

I am having a similar problem using this library with a FEATHER M0/RFM69 BOARD ADAFRUIT PID:3176 for RTC and pin wakeups. The radio tries to send one packet and fails then the command for sleep(2000) is executed and the the board never wakes up again. Have you tried your suggested change to the library?

denisbaylor commented 3 years ago

Yes. My change does fix the problems I was having.

PhilGoody commented 3 years ago

Thanks, it fixed my RFM69 problem too although Serial still stops working which, I guess, is a separate issue.

aalbinati commented 3 years ago

You saved me! Thanks!!! Tha arduino MKR 1310 can't work with an ADCInterrupt and a timed sleep at the sime time. But this fixes it. I'll make a pull request

aalbinati commented 3 years ago

@denisbaylor @PhilGoody I made a pull #request. Please be kind to test it and support it for it to be included in the master branch!

jremington commented 3 weeks ago

Fixed my problem with the Feather M0 LoRa radio, too. Now sleep modes can be implemented for the SAMD21 in the RadioHead library! For the details see https://forums.adafruit.com/viewtopic.php?t=211308

It is very sad that the library maintainer(s) can't be bothered commit this simple fix to a serious error in the library.