libdriver / w25qxx

W25QXX(W25Q80, W25Q16, W25Q32, W25Q64, W25Q128, W25Q256) full function driver library for general MCU and Linux.
https://www.libdriver.com
MIT License
682 stars 74 forks source link

w25qxx_set_status_x() wrong check after write #24

Closed KestrelLuca closed 1 month ago

KestrelLuca commented 1 month ago

Version

No response

Describe the bug

w25qxx_set_status2() and w25qxx_set_status3() functions perform the check (after the write) always on Status Register 1 (W25QXX_COMMAND_READ_STATUS_REG1)

Reproduce

No response

Expected behavior

No response

Additional context

No response

libdriver commented 1 month ago

Thank you for using LibDriver W25QXX. 微信图片_20240906155710 微信图片_20240906155714 微信图片_20240906155717 微信图片_20240906155720 According to the description in the datasheet, status register 1, status register 2, and status register 1 are different types of registers. The above is their description, and the driver processing seems to be correct.

KestrelLuca commented 1 month ago

Hi, thanks for your reply.

`uint8_t w25qxx_set_status3(w25qxx_handle_t *handle, uint8_t status) { uint8_t res; uint8_t buf[2]; uint32_t timeout; uint8_t status_check;

if (handle == NULL)                                                                                  /* check handle */
{
    return 2;                                                                                        /* return error */
}   
if (handle->inited != 1)                                                                             /* check handle initialization */
{
    return 3;                                                                                        /* return error */
}

if (handle->spi_qspi == W25QXX_INTERFACE_SPI)                                                        /* spi interface */
{
    if (handle->dual_quad_spi_enable != 0)                                                           /* enable dual quad spi */
    {
        res = a_w25qxx_qspi_write_read(handle, W25QXX_COMMAND_VOLATILE_SR_WRITE_ENABLE, 1,
                                       0x00000000, 0x00, 0x00,
                                       0x00000000, 0x00, 0x00,
                                       0x00, NULL, 0x00,
                                       NULL, 0x00, 0x00);                                            /* qspi write read */
        if (res != 0)                                                                                /* check result */
        {
            handle->debug_print("w25qxx: write enable failed.\n");                                   /* write enable failed */

            return 1;                                                                                /* return error */
        }
        buf[0] = status;                                                                             /* set status */
        res = a_w25qxx_qspi_write_read(handle, W25QXX_COMMAND_WRITE_STATUS_REG3, 1,
                                       0x00000000, 0x00, 0x00,
                                       0x00000000, 0x00, 0x00,
                                       0x00, (uint8_t *)buf, 1,
                                       NULL, 0x00, 1);                                               /* qspi write read */
        if (res != 0)                                                                                /* check result */
        {
            handle->debug_print("w25qxx: set status3 failed.\n");                                    /* set status3 failed */

            return 1;                                                                                /* return error */
        }

        timeout = 1000;                                                                              /* max 1000 ms */
        while (timeout != 0)                                                                         /* check timeout */
        {
            res = a_w25qxx_qspi_write_read(handle, W25QXX_COMMAND_READ_STATUS_REG1, 1,
                                           0x00000000, 0x00, 0x00,
                                           0x00000000, 0x00, 0x00,
                                           0x00, NULL, 0x00,
                                          (uint8_t *)&status_check, 1, 1);                           /* qspi write read */
            if (res != 0)                                                                            /* check result */
            {
                handle->debug_print("w25qxx: get status1 failed.\n");                                /* get status1 failed */

                return 1;                                                                            /* return error */
            }`

this is the w25qxx_set_status3, as you can see, after the write, a read of this register is performed to confirm the correct write. the command being used is wrong, W25QXX_COMMAND_READ_STATUS_REG1 it has been used rather than W25QXX_COMMAND_READ_STATUS_REG3. the mistake is also present in the register 2

libdriver commented 1 month ago

image image image I seem to understand your question. According to the datasheet(./datasheet/), there are three status registers, each with a different defined function. The lowest bit of status register 1 should be the status bit written by all status registers, while the definitions of status registers 2 and 3 are shown in the above figure, where their lowest bits represent status register protect and reserved.So when performing any state register changing, only the lowest bit of state register 1 will indicate the write status, and all should check this bit to confirm successful changing.

KestrelLuca commented 1 month ago

I see, my apologies I thought a read-back for each status register was missing where this has to be implemented in the application instead. Thanks

libdriver commented 1 month ago

I am very happy that your issue has been successfully resolved. This driver use busy bit to check the changing status and you can also read the status at the application layer to check the changing status.