zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.01k stars 6.16k forks source link

drivers: spi: Apollo3 Blue MCU is not able to communicate with SX1262 #73959

Open srcnert opened 3 weeks ago

srcnert commented 3 weeks ago

Apollo3 Blue MCU is not able to communicate with SX1262 transceiver over spi. For test purposes, I used sample/subsys/lorawan/class_a example project.

The code is running on RAK11720 module from RAKwireless. It is based on Apollo3 Blue SoC with a Semtech SX1262 transceiver. Currently, I am trying to add rak11720 board support to zephyr project.

This overlay file can be used to compile class_a example project: apollo3_evb-overlay.txt

There are two problems. Firstly, I need a custom CS support for Apollo3. Current board uses pin 11 as CS pin for SPI 1 peripheral. Secondly, current spi_ambiq_xfer function is not able to set up SX1262 chip. I prepared a sample function to set up chip over spi correctly. Via this function, I am able to join a LoRaWAN network.

static int spi_ambiq_xfer(const struct device *dev, const struct spi_config *config)
{
    struct spi_ambiq_data *data = dev->data;
    struct spi_context *ctx = &data->ctx;
    am_hal_iom_transfer_t trans = {0};
    int ret = 0;
    uint8_t tx_buf[256] = {0};
    uint8_t rx_buf[256] = {0};

    while(1)
    {
        uint8_t offset = 0;
        memset(&trans, 0x00, sizeof(am_hal_iom_transfer_t));
        memset(&tx_buf, 0x00, sizeof(am_hal_iom_transfer_t));
        memset(&rx_buf, 0x00, sizeof(am_hal_iom_transfer_t));

        if ((ctx->tx_len != 0) && (ctx->rx_len != 0))
        {
            if(ctx->rx_buf == NULL)
            {
                offset = ctx->rx_len;
                spi_context_update_rx(ctx, 1, ctx->rx_len);
            }

            /* Check Length */
            if(ctx->tx_len > sizeof(tx_buf))
            {
                return -1;
            }

            if(ctx->rx_len > sizeof(rx_buf))
            {
                return -1;
            }

            uint32_t len = offset + ctx->rx_len;
            memcpy(tx_buf, ctx->tx_buf, ctx->tx_len);

            trans.eDirection      = AM_HAL_IOM_FULLDUPLEX;
            trans.ui32NumBytes    = len;
            trans.pui32TxBuffer   = (uint32_t *) tx_buf;
            trans.pui32RxBuffer   = (uint32_t *) rx_buf;

            ret = am_hal_iom_spi_blocking_fullduplex(data->iom_handler, &trans);
            if(ret != 0)
            {
                LOG_ERR("Spi full duplex comm error: %d", ret);
                return ret;
            }

            /* Copy response to rx_buf */
            memcpy(ctx->rx_buf, &rx_buf[offset], ctx->rx_len);

            /* Move buffers */
            spi_context_update_tx(ctx, 1, ctx->tx_len);
            spi_context_update_rx(ctx, 1, ctx->rx_len);
        }
        else if (ctx->tx_len != 0)
        {
            if(ctx->tx_buf == NULL)
            {
                spi_context_update_tx(ctx, 1, ctx->tx_len);
            }

            trans.eDirection      = AM_HAL_IOM_TX;
            trans.ui32NumBytes    = ctx->tx_len;
            trans.pui32TxBuffer   = (uint32_t *) ctx->tx_buf;

            ret = am_hal_iom_blocking_transfer(data->iom_handler, &trans);
            if(ret != 0)
            {
                LOG_ERR("Spi tx comm error: %d", ret);
                return ret;
            }

            spi_context_update_tx(ctx, 1, ctx->tx_len);
        }
        else if (ctx->rx_len != 0)
        {
            if(ctx->rx_buf == NULL)
            {
                spi_context_update_rx(ctx, 1, ctx->rx_len);
            }

            trans.eDirection      = AM_HAL_IOM_RX;
            trans.ui32NumBytes    = ctx->rx_len;
            trans.pui32RxBuffer   = (uint32_t *) ctx->rx_buf;

            ret = am_hal_iom_blocking_transfer(data->iom_handler, &trans);
            if(ret != 0)
            {
                LOG_ERR("Spi rx comm error: %d", ret);
                return ret;
            }

            spi_context_update_rx(ctx, 1, ctx->rx_len);
        }
        else
        {
            break;
        }
    }

    spi_context_complete(ctx, dev, 0);

    return ret;
}

Please check following file to see how I add custom CS support and spi_ambiq_xfer function. spi_ambiq-c.txt

AlessandroLuo commented 3 weeks ago

Hi @srcnert, For CS control, you can define spi1 as below in your own board's pinctrl.dtsi, and don't need to control it manually.
spi1_default: spi1_default { group1 { pinmux = , , ; }; group2 { pinmux = ; drive-push-pull; ambiq,iom-mspi = <1>; ambiq,iom-nce-module = <1>; ambiq,iom-num = <1>; }; };

For the spi_ambiq_xfer function issue you met, may I know how you declare your spi_buf in application layer? and what are the spi packet look like? it will be very helpful if you can share some segments of the waveform.

srcnert commented 3 weeks ago

Hi @AlessandroLuo,

Firstly, current ambiq spi code is not able to control cs pin for spi-1. Even while spi cs pin is "pinmux = ", I am not seeing any change on my logic analyzer screen. Actually, this is also stated inside spi_ambiq.c file with following line "TODO Need to get iom_nce from different nodes of spi". Because of that, I continue to use custom cs implementation.

To see spi_buf, please check drivers/lora/sx126x.c file. I do not make any custom change.

I tested same class-a example code on Apollo3-Evb board and screen spi output with a logic analyzer.

With ambiq spi_ambiq_xfer function, logic analyzer output is like below: ambiq_spi_file.csv

With my spi_ambiq_xfer function, logic analyzer output is like below: my_spi_file.csv

If you want to make same test, please do not forget to set pin16 to gnd.

srcnert commented 1 week ago

Hello @AlessandroLuo,

I have developed an improved spi_ambiq_xfer function compared to my previous implementation. I know that spi_ambiq_xfer must work with all Ambiq's chipset and all sensors. However, I do not understand why current spi_ambiq_xfer function is too complex. My simple one works with Sx1262 chip. I hope this can help you. (DMA is also not working on my version.)

static int spi_ambiq_xfer(const struct device *dev, const struct spi_config *config)
{
    struct spi_ambiq_data *data = dev->data;
    struct spi_context *ctx = &data->ctx;
    int ret = 0;

    do
    {
        am_hal_iom_transfer_t trans = {0};
        uint32_t cur_num = 0, rem_num = 0;

        if (spi_context_tx_buf_on(ctx) && spi_context_rx_buf_on(ctx))
        {
            rem_num = MIN(ctx->rx_len, ctx->tx_len);
            cur_num = (rem_num > AM_HAL_IOM_MAX_TXNSIZE_SPI)
                          ? AM_HAL_IOM_MAX_TXNSIZE_SPI
                          : rem_num;

            trans.eDirection      = AM_HAL_IOM_FULLDUPLEX;
            trans.ui32NumBytes    = cur_num;
            trans.pui32TxBuffer   = (uint32_t *) ctx->tx_buf;
            trans.pui32RxBuffer   = (uint32_t *) ctx->rx_buf;

#ifdef CONFIG_SPI_AMBIQ_DMA
            if (AM_HAL_STATUS_SUCCESS !=
                am_hal_iom_nonblocking_transfer(data->iom_handler, &trans, spi_ambiq_callback,
                                (void *)dev)) {
                spi_ambiq_reset(dev);
                LOG_ERR("Spi nonblocking transfer error: %d", ret);
                return -EIO;
            }
            ret = spi_context_wait_for_completion(ctx);
            if (ret != 0) {
                spi_ambiq_reset(dev);
                spi_context_complete(ctx, dev, ret);
                LOG_ERR("Spi context wait error: %d", ret);
                return -EIO;
            }

#else
            if (AM_HAL_STATUS_SUCCESS != 
                am_hal_iom_spi_blocking_fullduplex(data->iom_handler, &trans)) {
                LOG_ERR("Spi blocking fullduplex transfer error: %d", ret);
                return -EIO;
            }
#endif

            /* Move buffers */
            spi_context_update_tx(ctx, 1, cur_num);
            spi_context_update_rx(ctx, 1, cur_num);
        }
        else if (spi_context_tx_buf_on(ctx))
        {
            rem_num = ctx->tx_len;
            cur_num = (rem_num > AM_HAL_IOM_MAX_TXNSIZE_SPI)
                          ? AM_HAL_IOM_MAX_TXNSIZE_SPI
                          : rem_num;

            trans.eDirection      = AM_HAL_IOM_TX;
            trans.ui32NumBytes    = cur_num;
            trans.pui32TxBuffer   = (uint32_t *) ctx->tx_buf;

#ifdef CONFIG_SPI_AMBIQ_DMA
            if (AM_HAL_STATUS_SUCCESS !=
                am_hal_iom_nonblocking_transfer(data->iom_handler, &trans, spi_ambiq_callback,
                                (void *)dev)) {
                spi_ambiq_reset(dev);
                LOG_ERR("Spi nonblocking transfer error: %d", ret);
                return -EIO;
            }
            ret = spi_context_wait_for_completion(ctx);
            if (ret != 0) {
                spi_ambiq_reset(dev);
                spi_context_complete(ctx, dev, ret);
                LOG_ERR("Spi context wait error: %d", ret);
                return -EIO;
            }
#else
            if (AM_HAL_STATUS_SUCCESS != 
                am_hal_iom_blocking_transfer(data->iom_handler, &trans)) {
                LOG_ERR("Spi blocking transfer error: %d", ret);
                return -EIO;
            }
#endif

            /* Move buffers */
            spi_context_update_tx(ctx, 1, cur_num);
        }
        else if (spi_context_rx_buf_on(ctx))
        {
            rem_num = ctx->rx_len;
            cur_num = (rem_num > AM_HAL_IOM_MAX_TXNSIZE_SPI)
                          ? AM_HAL_IOM_MAX_TXNSIZE_SPI
                          : rem_num;

            trans.eDirection      = AM_HAL_IOM_RX;
            trans.ui32NumBytes    = cur_num;
            trans.pui32RxBuffer   = (uint32_t *) ctx->rx_buf;

#ifdef CONFIG_SPI_AMBIQ_DMA
            if (AM_HAL_STATUS_SUCCESS !=
                am_hal_iom_nonblocking_transfer(data->iom_handler, &trans, spi_ambiq_callback,
                                (void *)dev)) {
                spi_ambiq_reset(dev);
                LOG_ERR("Spi nonblocking transfer error: %d", ret);
                return -EIO;
            }
            ret = spi_context_wait_for_completion(ctx);
            if (ret != 0) {
                spi_ambiq_reset(dev);
                spi_context_complete(ctx, dev, ret);
                LOG_ERR("Spi context wait error: %d", ret);
                return -EIO;
            }
#else
            if (AM_HAL_STATUS_SUCCESS != 
                am_hal_iom_blocking_transfer(data->iom_handler, &trans)) {
                LOG_ERR("Spi blocking transfer error: %d", ret);
                return -EIO;
            }
#endif

            /* Move buffers */
            spi_context_update_rx(ctx, 1, cur_num);     
        }
    } while (spi_context_tx_buf_on(ctx) || spi_context_rx_buf_on(ctx));

    spi_context_complete(ctx, dev, 0);

    return ret;
}

Secondly, for cs control, how can user define more than one cs pin for spi controller using current Ambiq implementation? In my opinion, custom cs implementation is a must to define more than one cs pin. Or, Am I wrong?

spi_ambiq.txt

Thanks.

AlessandroLuo commented 1 week ago

Hi @srcnert,

spi_ambiq was verified with all ambiq chips, including apollo3, apollo4 and apollo5 (not released to zephyr yet). There are three kinds of devices we tested: 1) Sensors, the baseline sample we use is the accel_polling which uses adxl362, I replaced the spi driver with the one you wrote, it's not working, device cannot been detected. 2) Memories, we have written a PSRAM device driver and corresponding sample (not released to zephyr yet) and tested with spi_ambiq, btw, the spi_nor driver should also work with current spi_ambiq. 3) BLE controller, apollo4 family MCUs use SPI as HCI to communicate with BLE controller, check hci_ambiq.c for more information. there are plenty of samples user can run, the simplest one is the peripheral_hr sample.

For the cs control implementation you mentioned, you are correct, that's why we left a TODO label in spi_ambiq_xfer, currently it only support one device, we will add multi-drop support soon.

Thank you

srcnert commented 1 week ago

Hello @AlessandroLuo

Thanks for your support. Just to be sure, you used the my spi_ambiq.txt file for testing, right? You didn't just change the spi_ambiq_xfer function. Also, you set cs-gpios inside your overlay file like below, right?

&spi1 {
    compatible = "ambiq,spi";
    status = "okay";
    pinctrl-0 = <&spi1_default>;
    pinctrl-names = "default";
    clock-frequency = <DT_FREQ_M(1)>;
    cs-gpios = <&gpio0_31 11 GPIO_ACTIVE_LOW>;

Otherwise, it will not work. Because, I need custom cs support that is defined inside txt file. You can change txt file to c file and please do not use DMA.

Secondly, are you sure that spi_ambiq driver at the main repo is latest one?

I tested spi with different accelerometer that is not defined inside zephyr. My sensor is not working with current spi_ambiq driver. Maybe, I am doing smt wrong. I am able to buy an adxl345 on my side. I will buy it and test it via samples/sensor/accel_polling application. I will inform you when I test it.

I know, it must also work with other MCUs and sensors. However, communication with sx1262 is a must for rak11720 board. I will try to continue to support this topic. If you want me to test any specific case, please inform me. Thanks.

AlessandroLuo commented 1 week ago

Hi @srcnert,

1) Yes, I used spi_ambiq.txt file. and here is my device tree: &spi7 { compatible = "ambiq,spi"; pinctrl-0 = <&spi7_default>; pinctrl-names = "default"; cs-gpios = <&gpio64_95 90 GPIO_ACTIVE_LOW>; clock-frequency = <DT_FREQ_M(1)>; status = "okay";

adxl362: adxl362@0 {
    compatible = "adi,adxl362";
    reg = <0>;
    spi-max-frequency = <DT_FREQ_M(1)>;
};

};

2) spi_ambiq driver at the main repo is latest one, one thing you need to check is that the way you create spi buffers, please compare with adxl362_reg_access, or can you share the sx1262 driver you use?

Thank you

srcnert commented 1 week ago

Thanks @AlessandroLuo

  1. Thanks for test. Probably, it is only working for my specific case, apollo3_blue + sx1262. I will buy an adxl accelerometer and inform you.
  2. I do not write any specific sx1262 driver. I am using zephyr drivers. You can find it on drivers/lora/sx126x.c file.
AlessandroLuo commented 1 week ago

Hi @srcnert,

I see, I'll take a look this week. BTW, where can we buy your rak11720 board? or is it possible you can ship one to our office? then we can also build a local environment to test.

AlessandroLuo commented 1 week ago

Hi @srcnert, I debugged the sx1262 driver today and found one bug in spi_ambiq, please try the updated file to see if it can solve the issue you met before. spi_ambiq.txt

srcnert commented 1 week ago

Hi @AlessandroLuo,

I tested it but it is not solving the problem. I am waiting my accelerometer to make more tests, I will inform you. You can check it (https://github.com/zephyrproject-rtos/zephyr/pull/74661) to see how I define cs pin.

Secondly, if you can provide your address, we will post you a rak11720 board. Thanks for your support.

AlessandroLuo commented 1 week ago

Hi @srcnert, Our office address is: 深圳市南山区深南大道9966号威盛科技大厦19楼1906单元 Room 1906, VIA Technologies Building, 9966, Shennan Avenue, Nanshan District, Shenzhen, China, 518057 Thank you!

AlessandroLuo commented 1 week ago

@srcnert I compared the waveforms captured from the spi_ambiq you wrote and the one I shared yesterday, and found one weird phenomenon: For read operations, for example here we read 1 byte data from address 0x8D8: ef26a6aa60028091829d9fdb489409c This is the waveform when running your driver, it only sent request but no read at all: 2c1fddafaa042afec74670153b7ef74 This is the waveform when running my driver, it read 1 byte back, I did not connect spi to any device so the value was 0: 1e77cc1e4861aa9b5881b4dc0602403 The reason why your driver did not read is because at that time the first ctx->rx_buf is NULL and data read should be placed in the second rx_buf, however it did not pass the first check in your spi_ambiq_xfer, and regarded as a pure write. if (spi_context_tx_buf_on(ctx) && spi_context_rx_buf_on(ctx)) If so, your driver may not work for any read operations, I'm confused how it work with your device on board.

srcnert commented 6 hours ago

Hello @AlessandroLuo,

Sorry for late response. At the end, I found a bme280 sensor to test spi on apollo3_evb board. I did not change anything on your spi_ambiq.c file and it is working if I do not change CS pin. If I set CS pin as pin-1, it is not working. (On rak11720, spi0's CS pin is 1.) Maybe, it is the reason why sx1262 communication failed with your spi_ambiq.c file. I was trying to add custom cs support to your spi_ambiq driver and maybe I am breaking smt. However, for rak11720, custom CS support is necessary. Please use following sample project to check spi communication.

bme280.zip

Secondly, you are right about your observation, I changed my spi_ambiq.c file. Current spi_ambiq.c file is also working successfully with bme280 and sx1262. I am able to change CS pin.

spi_ambiq.txt

Thirdly, we ship a rak11720 board to you. Thanks for your support.

AlessandroLuo commented 5 hours ago

Hi @srcnert, custom CS support will be added in https://github.com/zephyrproject-rtos/zephyr/pull/75064