AdaCore / Ada_Drivers_Library

Ada source code and complete sample GNAT projects for selected bare-board platforms supported by GNAT.
BSD 3-Clause "New" or "Revised" License
236 stars 141 forks source link

mma8653 initialization fails on MicroBit #393

Closed rveenker closed 1 year ago

rveenker commented 2 years ago

The accelerometer example for the Micro:Bit fails to initialize. (Using Micro:Bit v1.3B, firmware 0249) Running a Python example however works fine and once initialized, the Ada accelerometer example will also work. But after a reboot (power off/on), the Ada accelerometer fails again, and I keep getting the same result values.

By attaching a logic analyzer (Smartscope) to the i2c bus, I noticed a small difference in the way the register values are transmitted to the mma8653. I have attached two screenshots of the initialization procedure. It appears that the Ada version emits extra address fields for the register values, whereas the Python version uses a single address field for both register id and its value.

Ada version: MicroBit_Accelerator_Ada

Python version: MicroBit_Accelerator_Python

Fabien-Chouteau commented 2 years ago

Hi Rob, The I2C protocol can be used in different ways. The Python version doesn't send stop/start tokens between address and data access, but that doesn't necessarily explain the problem.

What error do you see?

rveenker commented 2 years ago

Hello Fabien, What I see is that the values from the mma8653 do not change at all and sometimes there are no values at all. Only after running another program that also uses the mma8653, I get results that make sense. So my guess was that the problem is caused by incorrect or incomplete initialization. For testing, I modified the accelerometer so that it sends the exact same register values as the python program, but this did not solve the issue. Do you have suggestions on how to proceed? Thanks!

Fabien-Chouteau commented 2 years ago

For testing, I modified the accelerometer so that it sends the exact same register values as the python program, but this did not solve the issue.

That's what I would have recommended. So if it doesn't work I don't know what to say...

simonjwright commented 2 years ago

I couldn't get the accelerometer example to work (it was constantly returning the same values). I tried the mu MicroPython development environment as recommended here, and that didn't work either. So I tried restoring the standard firmware instead of the Segger JLink version - success!

Need to check out pyOCD.

rveenker commented 2 years ago

Unfortunately still not able to get the accelerometer example running on my MicroBit V1.3b. The firmware versions I used are 0241, 0249 and 0253. In all cases I get the arrow, but the values remain the same.

Now that I also have a MicroBit v1.5 (firmware 0253), I was curious if the problem persisted. I rewrote the accelerometer example to use the LSM303AGR instead of the MMA8653. The example works, but with exactly the same precondition. In order to work, the accelerometer must be used first (initialized) by another (python) program.

@Simon, Did the example also work after power down/up? If so, Where can I find the firmware (.hex) files you used? Thanks in advance!

simonjwright commented 2 years ago

I'm at this moment using 0243.

I'm sorry to have misled you before: what I think I meant (it was a bit of a drive-by) was that mu wouldn't load with the Segger firmware, but did with the standard version; and after that the Ada code worked. So now I'm in exactly the same position as you.

I see that on my other Mac I made some changes to the MMA8653 setup; I'll need some time (and opportunity) to work out where I'd got to.

simonjwright commented 2 years ago

I don’t think the problem is with the accelerometer; it’s with TWI (Nordic’s I2C implementation). I looked at the TWI peripheral data while the accelerometer was working (after Python initialisation) and while it wasn’t, and couldn’t see any obviously-significant difference. I tried running under Cortex GNAT RTS, which involved some changes because it’s a Ravenscar RTS; with this, both with and without Python initialisation, reading the chip’s whoami register returns 0 (there’s no visible display, because this is happening during elaboration, so tasking isn’t running yet). I can only think there’s some undocumented board initialisation which my microbit (1.3B - at any rate, the FCC ID is 2AKFPMB013B) requires, which is done somewhere in mu. I tried looking at microbit-dal, not very helpful (it seems to be incomplete, for a start). I hate to abandon something like this!

rveenker commented 2 years ago

Since the problem occurs with both V1.3B and V1.5 boards and with the MMA8653 an LSM303AGR, I don't think it is hardware related. It must be some sort of initialization issue indeed. @Simon, Thanks for investigating!

Fabien-Chouteau commented 2 years ago

Did you both tried with this change applied? https://github.com/AdaCore/Ada_Drivers_Library/pull/402

simonjwright commented 2 years ago

@Fabien-Chouteau - yes. I did wonder whether there was an issue with HFCLOCK, but so far as I can see there’s no difference between working & non-working setups (I’m a little unsure about this, because to run mu I need a standard firmware, which means I have to use pyocd, which can’t access peripheral memory ...)

simonjwright commented 1 year ago

I found that the board could be put into the working state by running BBC-MicroBit-First-Experience-1460979530935.hex (which means I can carry on with the JLink firmware, and get a better debug experience than I’ve got with pyocd - which also doesn’t run on my M1 mac).

It seemed to me that the retained settings in the nRF51 would most likely be in TWI, GPIO, or GPIOTE. Unfortunately, there was no difference between these when running from power-on or after the First Experience firmware.

I also tried running a VL53L1X - again, didn’t work. I should try running after the First Experience ...

JeremyGrosser commented 1 year ago

I tried this on a v1.3b board with 0253 firmware. The accelerometer works for me.

Here's the code I used: https://github.com/JeremyGrosser/microbit_accel

JeremyGrosser commented 1 year ago

Hmm, I commented too soon... After a power cycle, I see the same behavior @simonjwright reported, which is that it always returns the same obviously incorrect values.

JeremyGrosser commented 1 year ago

I've been looking into this for a few hours and I'm fairly convinced that @Fabien-Chouteau was right in his first comment... The Python driver correctly sends a Repeated Start between the address and data sections of a transaction, whereas nRF.TWI is sending Stop then Start.

The MMA8653 datasheet pretty clearly shows that repeated starts are required on this device. We'll need to modify the TWI driver to support that.

I'm not sure why it works when the Ada code is loaded after the Python code. The state machine is a bit complicated, but my best guess is that the Python code is leaving the TWI peripheral in the middle of a transaction, after sending STARTTX but before calling STOP. When the Ada driver then calls STARTTX a second time, the hardware issues a repeated start.

Likely unrelated, but worth noting: The I2C clock is running a bit fast... Measuring 410-421 KHz with my logic analyzer. The MMA8653 is only specified up to 400 KHz, though it appears to be responding at this slightly out of spec speed. Still, we should fix the clock.

I've started working on a rewrite of the TWI driver, similar to what I did with RP.I2C. It's going to take some time.

simonjwright commented 1 year ago

I think both of the changes here are necessary, the first to supply the missing initialization, the second to avoid the stop/start issue.

diff --git a/arch/ARM/Nordic/drivers/nrf_common/nrf-twi.adb b/arch/ARM/Nordic/drivers/nrf_common/nrf-twi.adb
index d3423ec6..a09a65f6 100644
--- a/arch/ARM/Nordic/drivers/nrf_common/nrf-twi.adb
+++ b/arch/ARM/Nordic/drivers/nrf_common/nrf-twi.adb
@@ -134,6 +134,9 @@ package body nRF.TWI is
       --  Set Address
       This.Periph.ADDRESS.ADDRESS := UInt7 (Addr / 2);

+      This.Periph.SHORTS.BB_SUSPEND := Disabled;
+      This.Periph.SHORTS.BB_STOP := Disabled;
+
       --  Prepare first byte
       This.Periph.TXD.TXD := Data (Data'First);

@@ -266,34 +269,29 @@ package body nRF.TWI is
    is
    begin

-      This.Do_Stop_Sequence := False;
+      This.Do_Stop_Sequence := True;

       case Mem_Addr_Size is
          when Memory_Size_8b =>
             This.Master_Transmit (Addr    => Addr,
-                                  Data    => (0 => UInt8 (Mem_Addr)),
+                                  Data    =>
+                                    (0 => UInt8 (Mem_Addr)) & Data,
                                   Status  => Status,
                                   Timeout => Timeout);
          when Memory_Size_16b =>
             This.Master_Transmit (Addr    => Addr,
-                                  Data    => (UInt8 (Shift_Right (Mem_Addr, 8)),
-                                              UInt8 (Mem_Addr and 16#FF#)),
+                                  Data    =>
+                                    (UInt8 (Shift_Right (Mem_Addr, 8)),
+                                     UInt8 (Mem_Addr and 16#FF#)) & Data,
                                   Status  => Status,
                                   Timeout => Timeout);
       end case;

-      This.Do_Stop_Sequence := True;
-
       if Status /= Ok then
          This.Stop_Sequence;
          return;
       end if;
-
-      This.Master_Transmit (Addr    => Addr,
-                            Data    => Data,
-                            Status  => Status,
-                            Timeout => Timeout);
    end Mem_Write;

    --------------
JeremyGrosser commented 1 year ago

With that patch, the accelerometer appears to be initialized correctly after power cycle and is using repeated starts in I2C transactions. Seems like a good fix to me!

simonjwright commented 1 year ago

I also tried against an external VL53L1X; didn’t work before the change, OK afterward.

Interestingly, the MMA8653 datasheet says there shouldn’t be a stop/start between the transmit & receive parts of a read request, yet inserting them seems to be fine. Also, the VL53L1X datasheet has subtly different expectations!

JeremyGrosser commented 1 year ago

There are some older I2C devices that won't work with repeated starts but generally anything with silicon designed in the last 15-20 years should be okay. It would be nice to have a switch to turn off repeated start, but it's probably unnecessary these days.

rveenker commented 1 year ago

Just tried the proposed fix on my two Micro:bit boards (V1.3B using the mma8653 and V1.5 which uses the LSM303AGR), and can say that the fix works on both devices. Thanks for fixing this issue!

rveenker commented 1 year ago

I noticed that on the V1.3B board, the accelerometer demo also works after being disconnected, however on the V1.5 board the demo starts after loading, but not from a cold start. There may still be an issue with the LSM303AGR.

rveenker commented 1 year ago

Connecting power to the V1.5 board apparently behaves differently, because on this board you also need to press the reset button :-) So I guess there is no issue with the LSM303AGR.