Samraksh / eMote

eMote OS -- Multiple Ports (using .NET MF v4.3)
0 stars 0 forks source link

I2C driver hangs #311

Open WilliamAtSamraksh opened 9 years ago

WilliamAtSamraksh commented 9 years ago

eMote v. 1.0.12, powered by USB. When the Kiwi accelerometer is connected to the .NOW I2C lines the driver hangs. There seem to be side effects wrt the LCD.

Nathan-Stohs commented 9 years ago

LCD and accelerometer both use I2C1 module. Suspect a driver conflict. Will investigate.

AnanthAtSamraksh commented 9 years ago

The issue with I2C and accelerometer happens only in the optimized version (-O2) and not in debug version (-O0). It happens because in the optimized version, functions in I2C like I2C_Internal_Initialize are optimized away. This causes the LCD to fail.

AnanthAtSamraksh commented 9 years ago

It is actually the function I2C_Event_Handler in netmf_i2c.cpp that is being optimized away which is causing the crash. I will find out what inside this function is being optimized and will push the fix for it.

WilliamAtSamraksh commented 9 years ago

Is there a test for this that can be automated for the Test Rig?

AnanthAtSamraksh commented 9 years ago

We do have a test in the test rig for the LCD which uses I2C.

Nathan-Stohs commented 9 years ago

It is actually the function I2C_Event_Handler in netmf_i2c.cpp that is being optimized away which is causing the crash. I will find out what inside this function is being optimized and will push the fix for it.

I think that function is being optimized out because its not actually used. The LCD driver is actually in a different file for some reason (don't ask me why). You want LCD_PCF85162.cpp

I suspect the I2C driver you are looking at, netmf_i2c.cpp (which I think Nived wrote) is dead code.

AnanthAtSamraksh commented 9 years ago

I suspect the I2C driver you are looking at, netmf_i2c.cpp (which I think Nived wrote) is dead code.

Actually, no, this is not dead code. Here is the entry for netmf_i2c from TinyCLR.proj.

<ItemGroup>
    <DriverLibs Include="STM32F10x_i2c.$(LIB_EXT)" />
    <RequiredProjects Include="$(SPOCLIENT)\DeviceCode\Targets\Native\STM32F10x\DeviceCode\drivers\i2c\dotNetMF.proj" />
  </ItemGroup>

I2C_Event_Handler is initialized to handle the interrupts.

if(!CPU_INTC_ActivateInterrupt(STM32_AITC::c_IRQ_INDEX_I2C1_EV, I2C_Event_Handler, NULL ))
    return DS_Fail;

I did look into the LCD_PCF85162.cpp, but did not find anything that causes the crash.

Nathan-Stohs commented 9 years ago

Actually, no, this is not dead code. Here is the entry for netmf_i2c from TinyCLR.proj.

That just means the code is passed to the linker, it can still be quite dead (unreachable) otherwise.

Not to say you are necessarily wrong about it being not dead. The real question is, who calls STM32F10x_I2C_Driver::Initialize()

AnanthAtSamraksh commented 9 years ago

The real question is, who calls STM32F10x_I2C_Driver::Initialize()

Here is the call hierarchy (initialize function in each of the below files is called until STM32F10x_I2C_Driver::Initialize is invoked).

CLR\libraries\SPOT_Hardware\spot_hardware_native_Microsoft_SPOT_Hardware_I2CDevice.cpp => DeviceCode\pal\COM\i2c\i2c.cpp => DeviceCode/Targets/Native/STM32F10x/DeviceCode/drivers/i2c/netmf_i2c.cpp

i2c

Nathan-Stohs commented 9 years ago

So that looks dead to me, no?

AnanthAtSamraksh commented 9 years ago

I2C initialization is commented out in tinyhal.cpp --> //I2C_Initialize(); , but while debugging using a GPIO, I find that STM32F10x_I2C_Driver::Initialize is being invoked as well as I2C_Event_Handler. This happens due to the call from SPOT_Hardware library.

HRESULT Library_spot_hardware_native_Microsoft_SPOT_Hardware_I2CDevice::Initialize___VOID( CLR_RT_StackFrame& stack )
{
    NATIVE_PROFILE_CLR_HARDWARE();
    TINYCLR_HEADER();
    CLR_RT_HeapBlock* pThis = stack.This();  FAULT_ON_NULL(pThis);
TINYCLR_CHECK_HRESULT(CLR_RT_HeapBlock_I2CXAction::CreateInstance( *pThis, pThis[ FIELD__m_xAction ] ));

    I2C_Initialize();

    TINYCLR_NOCLEANUP();
}

The simple fix for this issue is to prevent I2C_Event_Handler from being optimized away (adding __attribute__((optimize("O0")))). However, the long term fix will be to combine the LCD LCD_PCF85162.cpp and I2C drivers netmf_i2c.cpp to avoid driver conflicts. Deferring this to another day after I am done with my current task.

MichaelAtSamraksh commented 9 years ago

It frustrates me that this thread seems to indicate that we do not have a firm grasp on what functions are used or not used.

This reminds me of how our drivers sometimes assume that they are the only drivers touching a resource so resources are initialized multiple times or break during the uninitialize sequence during a soft reboot.

Nathan-Stohs commented 9 years ago

It frustrates me that this thread seems to indicate that we do not have a firm grasp on what functions are used or not used.

This reminds me of how our drivers sometimes assume that they are the only drivers touching a resource so resources are initialized multiple times or break during the uninitialize sequence during a soft reboot.

Initialization in the beginning was very ad-hoc. I think we are slowly moving to fix that, there is just a lot of technical debt to pay. But there is a still a lot to be done.

WilliamAtSamraksh commented 8 years ago

I just tested this in eMote v. 14 and the problem still seems to be there.

ChrisAtSamraksh commented 8 years ago

201348d958ecccbe60c951d93c63bb4658f4f20c should fix the problem. Please retest.

WilliamAtSamraksh commented 8 years ago

Program in share:

GitHub Issues Attachments\2015.12.09 Accelerometer Freefall Detector [N] 1.1.zip

It no longer hangs. However, it now throws an exception:

#### Exception System.InvalidOperationException - CLR_E_INVALID_OPERATION (1) ####
#### Message: 
#### Microsoft.SPOT.Hardware.Port::ReservePin [IP: 0000] ####
#### Microsoft.SPOT.Hardware.I2CDevice::.ctor [IP: 0021] ####
#### Samraksh.eMote.SensorBoard.Accelerometer::.ctor [IP: 0021] ####
#### Samraksh.AppNote.Accelerometer.Program::Main [IP: 003a] ####
#### Exception System.Exception - 0x00000000 (1) ####
#### Message: I2C Bus initialization failed
#### Samraksh.eMote.SensorBoard.Accelerometer::.ctor [IP: 002d] ####
#### Samraksh.AppNote.Accelerometer.Program::Main [IP: 003a] ####

There seems to be a pin conflict. The only GPIO used in the program is J11/4, to provide Other Pwr to the Kiwi. Here's the connection map. Besides this, the Kiwi temperature sensor pin J2/4 is connected to J11/3 but that shouldn't matter.

|---------------------------------------------------|
| Kiwi                  ||  .NOW                    |
|-----------------------||--------------------------|
| Pin Desc  |   Number  ||  Pin Desc    |   Number  |
|-----------------------||--------------------------|
| I2C SDA   |   J2/6    ||  I2C SDA     |   J11/10  |
| I2C SCL   |   J2/7    ||  I2C SCL     |   J11/9   |
| Other Pwr |   J2/9    ||  GPIO        |   J11/4   |
| Gnd       |   J2/11   ||  Ground      |   J12/10  |
| Vin       |   J2/12   ||  VOut        |   J11/1   |
|---------------------------------------------------|
WilliamAtSamraksh commented 8 years ago

I tried the program again on a .NOW with nothing attached. Same exception.

ChrisAtSamraksh commented 8 years ago

When you instantiate a I2C device, MF will automatically reserve the GPIO pins for the I2C interface. If you try to instantiate the I2C device twice we get this "reservePin" error. Once we have a I2C device we should be able to use it for any I2C attached (including the LCD).

What is probably happening is that both the LCD and your program try and instantiate the same I2C device. They both should be able to use it, but we need a way to allow a user (or LCD driver) to use a currently instantiated I2C bus and not have it try and reserve the pins a second time.

ChrisAtSamraksh commented 8 years ago

Looking around the net there are a number of other people with the same problem. They end up just disposing after every use and reinstantiating a new instance (which is just messy). It is possible to have two devices on the same I2CDevice, but the LCD driver and the user need to use the same handle.

If we can just return an existing I2CDevice handle instead of trying to create a new one if one already exists then that would work but that is all MF code and it is not the most clear.