sparkfun / Arduino_Apollo3

Arduino core to support the Apollo3 microcontroller from Ambiq Micro
83 stars 38 forks source link

[Observation] Disabling an IOM causes much badness on v2.1 #412

Open PaulZC opened 3 years ago

PaulZC commented 3 years ago

OpenLog Artemis puts the Aretmis into deep sleep between measurements to save battery power. With v1.2 of the core, we could disable the I2C IOM to save a little extra power. With v2.1 of the core, this causes much badness...

Steps to reproduce: Apollo3 core v1.2.1 RedBoard Artemis ATP Connect an oscilloscope to the standard SCL and SDA pins (39 and 40) Run the following example:

#include <Wire.h>

void setup() {
  Serial.begin(115200);
  Serial.println(F("Apollo3: IOM disable test"));

  Wire.begin();

  Serial.print(F("First Wire.endTransmission returned "));
  Wire.beginTransmission(0x55);
  Serial.println(Wire.endTransmission());
  //Serial.flush();

  delayMicroseconds(50);

  Wire.end();

  //Force IOM4 off to save power during deep sleep
  //Wire uses IOM4 on Artemis ATP (Pin 39 = SCL, Pin 40 = SDA)
  am_hal_pwrctrl_periph_disable(AM_HAL_PWRCTRL_PERIPH_IOM4);

  //Deep Sleep would be here
  delayMicroseconds(50);

  Wire.begin();

  Serial.print(F("Second Wire.endTransmission returned "));
  Wire.beginTransmission(0x55);
  Serial.println(Wire.endTransmission());
  Serial.flush();
}

void loop() {
}

The serial monitor should show:

image

The 'scope should show:

image

Switch to v2.1 of the core and we get:

image

It looks like the second Wire.begin is not re-enabling the IOM for us. If we enable it manually by changing the code to:

#include <Wire.h>

void setup() {
  Serial.begin(115200);
  Serial.println(F("Apollo3: IOM disable test"));

  Wire.begin();

  Serial.print(F("First Wire.endTransmission returned "));
  Wire.beginTransmission(0x55);
  Serial.println(Wire.endTransmission());
  //Serial.flush();

  delayMicroseconds(50);

  Wire.end();

  //Force IOM4 off to save power during deep sleep
  //Wire uses IOM4 on Artemis ATP (Pin 39 = SCL, Pin 40 = SDA)
  am_hal_pwrctrl_periph_disable(AM_HAL_PWRCTRL_PERIPH_IOM4);

  //Deep Sleep would be here
  delayMicroseconds(50);

  //Re=enable IOM4 manually. Wire.begin does not do it for us
  am_hal_pwrctrl_periph_enable(AM_HAL_PWRCTRL_PERIPH_IOM4);

  Wire.begin();

  Serial.print(F("Second Wire.endTransmission returned "));
  Wire.beginTransmission(0x55);
  Serial.println(Wire.endTransmission());
  Serial.flush();
}

void loop() {
}

The HardFault goes away:

image

But we only see the first transaction on the Wire bus. The second transaction is MIA...

image

Cheers, Paul

paulvha commented 3 years ago

The problem is with Wire.end(). Actually there are 2 issues and a catch-22:

Current situation In the current Wire.end() it will just delete the MBED-OS I2C instance, without de-init or releasing the IOM module. The delete will release the allocated memory for the instance. BUT in Wire.end() it does NOT reset the pointer (“master”) to zero.

Next time around when you call Wire.Begin(), it believes the MBED-OS I2C instance is still valid and NO init is performed. As the module is powered-down in the sketch, it will stay powered down and will cause a crash during write / am_hal_iom_blocking_transfer.

When you enable the power to the module again in the sketch before calling Wire.begin(), the configuration settings in IOM are still not re-initialized with Wire.begin() and will stay on it’s default values. Hence no output.

P.s. Although the allocated memory for the instance was released, the content is still there. Hence it can still be used, BUT that is only until the memory is re-used for other purposes.

Second situation Now if in Wire.end() we include master= NULL after delete, the next time Wire.begin() is called it will try to create a new I2C instance. This will fail when it tries to am_hal_iom_initialize() because it was never deinit / released. It is part of am_hal_iom.c, where it will check whether the IOM was already initialized / in-use :

   if (g_IOMhandles[ui32Module].prefix.s.bInit)

    {
        return AM_HAL_STATUS_INVALID_OPERATION;
    }

There is a call i2c_free() in i2c_api.c (part of Apollo3 target) which can release and powerdown the IOM, but MBED-OS I2C.cpp does not have any call that can be used to reference that.

Now what?

  1. Ask MBED to include a call in I2C.cpp (in drivers/source) to be able to call i2c_free().

  2. Until that time do changes in Wire.cpp (part of Sparkfun library).

Wire.end() Do NOT “delete master”, actually don’t do anything. Make sure to keep the instance so the memory will not be overwritten.

Wire.begin() change

    if(!master){
        master = new mbed::I2C((PinName)_sda, (PinName)_scl);
        setClock(100000); //Default to 100kHz
    }

to

    if(!master){
        master = new mbed::I2C((PinName)_sda, (PinName)_scl);
    }
    setClock(100000); //Default to 100kHz   

When setting the frequency it will do a de-init and init which will reset the configuration settings in the IOM.

Now when in the sketch power is disabled with am_hal_pwrctrl_periph_disable() manually, one should restore the power with am_hal_pwrctrl_periph_enable() manually as well before calling Wire.begin(). Given the change with setClock() it will reset the configuration parameters for the IOM. If you do not enable the power back before calling begin, the am_hal_iom_disable() will fail as part of de-init (for frequency change) as it tries to write to the IOM that does not have power.

Paul

PaulZC commented 3 years ago

Thank you Paul - much appreciated! Paul