MediaTek-Labs / mt3620_m4_software

mt3620_m4_driver
Other
32 stars 29 forks source link

I2C DMA Problems #13

Closed brandonhanner closed 4 years ago

brandonhanner commented 4 years ago

Hey guys, it's me again.

I'm now trying to use I2C to communicate with a peripheral that has transfers in excess of 35 bytes.

The problem I am seeing is when I use the mtk_os_hal_write function, and when my input is larger than the FIFO_MAX_LEN of 8, the driver tries to use dma but results in all '0's being sent out onto the bus. When I use fifo mode (ie under 8 bytes) everything works great. What's weird is it seems like the function returns just fine, its just that the wrong bytes are being sent out of the interface. I have already tried splitting my data into multiples of 8 but the slave device wont not accept data that way, although i am successfully able to do the reads in multiples of 8 with no issues.

I have checked that I have OSAI_ENABLE_DMA in the compile definitions and im using release_200115 which has no changes to the i2c files from the release that just came out. I saw that mhal osai changed so I copied in those changes but no dice.

LawranceLiu commented 4 years ago

Hi Brandon, Thanks for your feedback. Since the current latest version is release_200320, and the I2C sample code shows the I2C loopback test for length 1B ~ 64B works fine on Seeed board. I'm wondering if this issue still happen with release_200320 ?

By the way, do you use Seeed board or AVNET board ? Which ISU did you use ? (ISU0 or ISU1 or ...) One potential issue I noticed is : On Seeed board, the I2C loopback test result of 1B ~ 64B is OK. On AVENT board, the I2C loopback test result of 1B ~ 9B is OK. but if the length is larger than 9B, the test result becomes unstable. There are two approach to make the test result on AVNET board more stable:

  1. Add pull-high circuit to the I2C bus.
  2. Swich the ISU role, Configure ISU2 be I2C master, and ISU1 as I2C Slave. If you are using AVNET board and ISU2 as I2C master, maybe you could try to use ISU1.
LawranceLiu commented 4 years ago

Hi Brandon, One more reminding for you: Please don't declare the I2C Tx/Rx buffer as global array. Please allocate the I2C Tx/Rx buffer using pvPortMalloc( ) or declare it as local array. The background reason is the DMA is only supported for M4 SYSRAM, not M4 TCM. If you declare the buffer as global array, it would be placed in M4 TCM.

brandonhanner commented 4 years ago

Hey Lawrance,

I am using I2C Master on ISU2 on the AVNET board. I know it has the lsm6dso on the same bus but I believe it to not have any effect on the other transfers.

I was able to dig into the drivers yesterday and modify them to handle fifo transfers in excess of 8 bytes (by constantly reading the FIFO_FULL flag and filling the FIFO with respect to that until the transfer is complete) and I have no trouble sending 35bytes of data back to back on the bus.

Looking at the logic analyzer with a transfer under 8 bytes everything goes according to plan, but as soon as dma is used, the i2c master successfully sets up a transfer to the correct address (including ack from slave) but sends 35 bytes of all 0's. It feels like the operation of DMA transferring the bytes to the i2c peripheral is failing or perhaps the placement of the data into dma region itself is failing. I'm not exactly sure how to verify what's going on in the DMA region.

I tried using the latest release just to double check before making this reply but i am seeing the same behavior.

I included the sample I used with the latest release that I modified to make a larger transfer. *NOTE: you have to pause the debugger and change "f" of main to 1 in order for the program to proceed, or remove that while loop alltogether. This is in place to make sure debugger is in sync with sphere while debugging.

/*
 * (C) 2005-2019 MediaTek Inc. All rights reserved.
 *
 * Copyright Statement:
 *
 * This MT3620 driver software/firmware and related documentation
 * ("MediaTek Software") are protected under relevant copyright laws.
 * The information contained herein is confidential and proprietary to
 * MediaTek Inc. ("MediaTek"). You may only use, reproduce, modify, or
 * distribute (as applicable) MediaTek Software if you have agreed to and been
 * bound by this Statement and the applicable license agreement with MediaTek
 * ("License Agreement") and been granted explicit permission to do so within
 * the License Agreement ("Permitted User"). If you are not a Permitted User,
 * please cease any access or use of MediaTek Software immediately.
 *
 * BY OPENING THIS FILE, RECEIVER HEREBY UNEQUIVOCALLY ACKNOWLEDGES AND AGREES
 * THAT MEDIATEK SOFTWARE RECEIVED FROM MEDIATEK AND/OR ITS REPRESENTATIVES ARE
 * PROVIDED TO RECEIVER ON AN "AS-IS" BASIS ONLY. MEDIATEK EXPRESSLY DISCLAIMS
 * ANY AND ALL WARRANTIES, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE
 * IMPLIED WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE OR
 * NONINFRINGEMENT. NEITHER DOES MEDIATEK PROVIDE ANY WARRANTY WHATSOEVER WITH
 * RESPECT TO THE SOFTWARE OF ANY THIRD PARTY WHICH MAY BE USED BY,
 * INCORPORATED IN, OR SUPPLIED WITH MEDIATEK SOFTWARE, AND RECEIVER AGREES TO
 * LOOK ONLY TO SUCH THIRD PARTY FOR ANY WARRANTY CLAIM RELATING THERETO.
 * RECEIVER EXPRESSLY ACKNOWLEDGES THAT IT IS RECEIVER'S SOLE RESPONSIBILITY TO
 * OBTAIN FROM ANY THIRD PARTY ALL PROPER LICENSES CONTAINED IN MEDIATEK
 * SOFTWARE. MEDIATEK SHALL ALSO NOT BE RESPONSIBLE FOR ANY MEDIATEK SOFTWARE
 * RELEASES MADE TO RECEIVER'S SPECIFICATION OR TO CONFORM TO A PARTICULAR
 * STANDARD OR OPEN FORUM. RECEIVER'S SOLE AND EXCLUSIVE REMEDY AND MEDIATEK'S
 * ENTIRE AND CUMULATIVE LIABILITY WITH RESPECT TO MEDIATEK SOFTWARE RELEASED
 * HEREUNDER WILL BE ANY SOFTWARE LICENSE FEES OR SERVICE CHARGE PAID BY
 * RECEIVER TO MEDIATEK DURING THE PRECEDING TWELVE (12) MONTHS FOR SUCH
 * MEDIATEK SOFTWARE AT ISSUE.
 */

#include "FreeRTOS.h"
#include "task.h"
#include "printf.h"
#include "mt3620.h"

#include "os_hal_uart.h"
#include "os_hal_i2c.h"

/*
 *Additional Note:
 *This sample code uses ISU1 as I2C Master and ISU2 as I2C Slave for the I2C
 *loopback test. ISU1(SDA/SDL) should be connected to ISU2(SDA/SDL).
 *Note, This sample code works only on Seeed development board, NOT on AVNET
 *development board. Since ISU2 of AVNET development board has an additional
 *sensor connected, if run this sample code on AVNET development board, some I2C
 *transport error might be happening. One possible work around for AVNET
 * development board is to change I2C_MAX_LEN from 64 to 8.
*/

/******************************************************************************/
/* Configurations */
/******************************************************************************/
static const uint8_t uart_port_num = OS_HAL_UART_ISU0;
static const uint8_t i2c_master_speed = I2C_SCL_400kHz;
static const uint8_t i2c_master_port_num = OS_HAL_I2C_ISU2;

#define I2C_MIN_LEN 1
#define I2C_MAX_LEN 64 /* For AVNET development board, please change to 8. */
#define I2C_SLAVE_TIMEOUT 10000 /* 10000ms */

#define APP_STACK_SIZE_BYTES (1024 / 4)

/******************************************************************************/
/* Applicaiton Hooks */
/******************************************************************************/
/* Hook for "stack over flow". */
void vApplicationStackOverflowHook(TaskHandle_t xTask, char *pcTaskName)
{
    printf("%s: %s\n", __func__, pcTaskName);
}

/* Hook for "memory allocation failed". */
void vApplicationMallocFailedHook(void)
{
    printf("%s\n", __func__);
}

/* Hook for "printf". */
void _putchar(char character)
{
    mtk_os_hal_uart_put_char(uart_port_num, character);
    if (character == '\n')
        mtk_os_hal_uart_put_char(uart_port_num, '\r');
}

/******************************************************************************/
/* Functions */
/******************************************************************************/

uint8_t randombytes[290] = { 0x96, 0x7d, 0xa7, 0x87, 0x94, 0xa1, 0x0a, 0xdd, 0x67, 0x7d, 0x1d, 0x9a, 0xd8, 0x42, 0x2a, 0x28,
            0x6f, 0xde, 0xa6, 0x95, 0xaa, 0x69, 0x8a, 0xf1, 0x91, 0x5d, 0xa2, 0xaa, 0xd4, 0xae, 0x54, 0x00,
            0x9c, 0x14, 0xd9, 0x17, 0xb5, 0x52, 0x19, 0x9d, 0x51, 0xed, 0x05, 0x4e, 0xf9, 0x99, 0xc1, 0xb2,
            0x9a, 0x08, 0x6d, 0x51, 0x88, 0x4d, 0x3a, 0x1e, 0xc2, 0x1d, 0x1f, 0x2c, 0x5c, 0x75, 0xad, 0x1f,
            0xc4, 0xe8, 0xff, 0xa6, 0x4c, 0x9b, 0x4f, 0x68, 0xc5, 0xe8, 0xd1, 0xdb, 0x72, 0x4c, 0x14, 0xdd,
            0x45, 0x3e, 0x7f, 0x35, 0x65, 0xc8, 0x25, 0xd7, 0x85, 0x32, 0x0c, 0x39, 0x8b, 0x88, 0x58, 0x4a,
            0x68, 0xf2, 0x6d, 0xfb, 0x0e, 0x31, 0x93, 0x3e, 0xd3, 0x60, 0xaf, 0x5c, 0xd6, 0x62, 0x5e, 0x12,
            0x7c, 0x18, 0x98, 0x3a, 0x99, 0x4c, 0x2f, 0x7c, 0xb3, 0xfb, 0xfa, 0x89, 0xe2, 0x8f, 0x7b, 0x98,
            0xdd, 0xcf, 0x89, 0x14, 0x58, 0x61, 0xf8, 0x57, 0xe6, 0xfe, 0x96, 0x15, 0xd7, 0x57, 0xc8, 0xa2,
            0x17, 0xa5, 0x76, 0xe1, 0x34, 0x48, 0xa9, 0xbc, 0x87, 0x40, 0x32, 0x72, 0x9d, 0x68, 0xf5, 0x61,
            0xc0, 0x93, 0xa4, 0x60, 0xe0, 0xbf, 0x19, 0x03, 0x50, 0x48, 0x96, 0x8c, 0x70, 0x1f, 0xe9, 0x0f,
            0x70, 0x81, 0x47, 0x29, 0x9c, 0x72, 0x9d, 0x64, 0x1d, 0xbd, 0x6e, 0x90, 0x4d, 0xd0, 0x3b, 0x4a,
            0x0a, 0xd0, 0x16, 0x17, 0x87, 0x7a, 0xe8, 0xdd, 0xab, 0xb0, 0xfa, 0xc5, 0xf0, 0x86, 0x6f, 0x81,
            0x91, 0xb1, 0x28, 0xf5, 0xb2, 0x92, 0x00, 0x1d, 0xfb, 0xad, 0x9e, 0x34, 0x38, 0xec, 0x90, 0xe1,
            0xea, 0x1f, 0x1b, 0x28, 0x52, 0x92, 0x42, 0xd8, 0x78, 0xe6, 0x4e, 0x10, 0xa4, 0x94, 0x67, 0xad,
            0x7f, 0x5a, 0x95, 0x3d, 0xe8, 0xd9, 0xff, 0xb6, 0xe8, 0x12, 0xc7, 0x5b, 0xda, 0xda, 0x94, 0x27,
            0x91, 0xb1, 0x28, 0xf5, 0xb2, 0x92, 0x00, 0x1d, 0xfb, 0xad, 0x9e, 0x34, 0x38, 0xec, 0x90, 0xe1,
            0x91, 0xb1, 0x28, 0xf5, 0xb2, 0x92, 0x00, 0x1d, 0xfb, 0xad, 0x9e, 0x34, 0x38, 0xec, 0x90, 0xe1, 0xe1, 0xe1
};

void i2c_master_task(void *pParameters)
{
    uint8_t write_buf[I2C_MAX_LEN] = {0};
    uint8_t read_buf[I2C_MAX_LEN] = {0};
    int i, length, ret = 0;

    printf("[I2C Demo]I2C Master Task Started. (ISU%d)\n",
        i2c_master_port_num);
    mtk_os_hal_i2c_speed_init(i2c_master_port_num, i2c_master_speed);
    while (1) {
        vTaskDelay(pdMS_TO_TICKS(1000));
        uint8_t test[1] = { 0 };
        mtk_os_hal_i2c_write(i2c_master_port_num,
            0x00, test, 1);
        vTaskDelay(pdMS_TO_TICKS(2));
        mtk_os_hal_i2c_write(i2c_master_port_num,
            0x60, randombytes, 35);

    }
}

_Noreturn void RTCoreMain(void)
{
    volatile bool f = false;
    while (!f) {
        // empty
    }
    /* Setup Vector Table */
    NVIC_SetupVectorTable();

    /* Init UART */
    mtk_os_hal_uart_ctlr_init(uart_port_num);
    printf("\nFreeRTOS I2C Demo\n");

    /* Init I2C Master/Slave */
    mtk_os_hal_i2c_ctrl_init(i2c_master_port_num);

    /* Create I2C Master/Slave Task */
    xTaskCreate(i2c_master_task, "I2C Master Task", APP_STACK_SIZE_BYTES,
        NULL, 4, NULL);

    vTaskStartScheduler();
    for (;;)
        __asm__("wfi");
}
LawranceLiu commented 4 years ago

Hi Brandon, Thanks for your sharing. I believe this issue could be fixed as following:

Change from : mtk_os_hal_i2c_write(i2c_master_port_num, 0x60, randombytes, 35); To : memcpy(write_buf, randombytes, 35); mtk_os_hal_i2c_write(i2c_master_port_num, 0x60, write_buf, 35);

The key point is described in my previous post: The DMA is only supported for M4 SYSRAM, not M4 TCM. When you try to do DMA, you have to make sure the buffer address is get from pvPortMalloc( ) or local array. We will update our DMA driver later to print some error log when the DMA buffer address is in TCM.

brandonhanner commented 4 years ago

Ahh ok. Sorry I did not see your post just before my response.

Thanks for the clarity!

brandonhanner commented 4 years ago

Just wanted to confirm the memory situation was the issue. After declaring the variable locally everything works.

Thanks again!