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.44k stars 6.4k forks source link

STM32G474: Write to flash Bank 2 address 0x08040000 does not work in 256K flash version #30148

Closed azeemshatp closed 3 years ago

azeemshatp commented 3 years ago

Describe the bug Write to flash Bank 2 address 0x08040000 from Bank 1 using the zephyr API, flash_write() does not seem to work as expected in STM32G474RC.

I am using 256K flash version of STM32G474 and BFB2 bit to switch between flash Banks 1 & 2. I am trying to write a new image to flash Bank 2 address 0x08040000, from the Bank 1. But I don't see anything being written to this address. However, when I use the address 0x08020000 instead, I can see the bytes written to the flash correctly. I don't know if I am messing up with the address offset here.

Is there anything special that needs to be done so that the proper flash bank is selected during the erase operation in the function erase_page()?

Below is my code.

dest_address = 0x08040000; uint32_t address = dest_address - CONFIG_FLASH_BASE_ADDRESS; // To get the offset flash_write_protection_set(flash_dev, false); flash_erase(flash_dev, address, FLASH_PAGE_SIZE); flash_write_protection_set(flash_dev, false); flash_write(flash_dev, address, (uint8_t *)source_buf, FLASH_PAGE_SIZE); flash_write_protection_set(flash_dev, true); (edited)

Flash partition &flash0 { partitions { compatible = "fixed-partitions";

address-cells = <1>;

    #size-cells = <1>;
    slot0_partition: partition@8000000 {
        label = "image-0";
        reg = <0x08000000 0x1F000>;
    };
    slot1_partition: partition@8040000 {
        label = "image-1";
        reg = <0x08040000 0x1F000>;
    };
    image_header_partition: partition@801F000 {
        label = "image-header";
        reg = <0x801F000 0x800>;
    };
    ota_status_partition: partition@801F800 {
        label = "ota-status";
        reg = <0x0801F800 0x800>;
    };
};

};

Environment: OS: Windows Toolchain: Zephyr Commit SHA or Version used: v2.4-branch

Additional context RM0440 of the MCU states that the Bank2 address starts at 0x08040000 even though a Bank size is 128K. I will try to create an example non-working code to demo this.

Reference https://github.com/barafael/g4-dual-bank-example

azeemshatp commented 3 years ago

An update on this: I have replaced the above partition with this now without any benefit yet:

&flash0 {
    /*
     * For more information, see:
     * http://docs.zephyrproject.org/latest/guides/dts/index.html#flash-partitions
     */
    partitions {
        compatible = "fixed-partitions";
        #address-cells = <1>;
        #size-cells = <1>;

        slot0_partition: partition@0 {
            label = "image-0";
            reg = <0x00000000 0x1F000>;
        };

        slot1_partition: partition@40000 {
            label = "image-1";
            reg = <0x00040000 0x1F000>;
        };

        /* To store the image header*/
        image_header_partition: partition@1F000 {
            label = "image-header";
            reg = <0x0001F000 0x00000800>;
        };

        /* The flash address to write the OTA status. This corresponds to the last page of Bank 1*/
        ota_status_partition: partition@1F800 {
            label = "ota-status";
            reg = <0x0001F800 0x00000800>;
        };
    };
};
azeemshatp commented 3 years ago

I placed the below code in the main.c of the zephyr flash_shell example.

uint8_t data[64];

printk("Testing: write to flash offset 0x40000 \n");

int ret;

off_t offset = 0x40000;
size_t size = 64;

// for(int i = 0; i < 2048; i++) {data[i]= i;}
for(int i = 0; i < 64; i++) {data[i]= i;}

ret = flash_write_protection_set(flash_device, false);
if (ret) {
    printk("Failed to disable flash protection (err: %d)."
            "\n", ret);
    return;
}
ret = flash_erase(flash_device, offset, size);
if (ret) {
    printk("flash_erase failed (err:%d).\n", ret);
    return;
}

// ret = flash_write(flash_device, offset, &data, FLASH_PAGE_SIZE);
ret = flash_write(flash_device, offset, &data, sizeof(data));
if (ret) {
    printk("flash_write failed (err:%d).\n", ret);
    return;
}

ret = flash_write_protection_set(flash_device, true);
if (ret) {
    printk("Failed to enable flash protection (err: %d)."
            "\n", ret);
    return;
}

uint8_t buf[64];
ret = flash_read(flash_device, offset, buf, sizeof(buf));
if (ret) {
    printf("Error reading from flash \r\n");
}
else {
    printf("Reading from flash. \n");
    for (int k = 0; k < sizeof(buf); k++) {
        printf("%.2x ", buf[k]);
    }
    printf("\n");
}

printk("Testing: write to flash offset 0x20000 \n");

offset = 0x20000;

for(int i = 0; i < 64; i++) {data[i]= 0xAA;}

ret = flash_write_protection_set(flash_device, false);
if (ret) {
    printk("Failed to disable flash protection (err: %d)."
            "\n", ret);
    return;
}
ret = flash_erase(flash_device, offset, size);
if (ret) {
    printk("flash_erase failed (err:%d).\n", ret);
    return;
}

ret = flash_write(flash_device, offset, &data, sizeof(data));
if (ret) {
    printk("flash_write failed (err:%d).\n", ret);
    return;
}

ret = flash_write_protection_set(flash_device, true);
if (ret) {
    printk("Failed to enable flash protection (err: %d)."
            "\n", ret);
    return;
}

ret = flash_read(flash_device, offset, buf, sizeof(buf));
if (ret) {
    printf("Error reading from flash. \n");
}
else {
    printf("Reading from flash.. \n");
    for (int k = 0; k < sizeof(buf); k++) {
        printf("%.2x ", buf[k]);
    }
    printf("\r\n");
}

printk("Success");

Both write operations worked fine in the nucleo board stm32g474re (512k version) and the output was as below.

Booting Zephyr OS build zephyr-v2.4.0 Found flash controller FLASH_CTRL. Flash I/O commands can be run.

Testing: write to flash offset 0x40000 Reading from flash . 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f 10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f 20 21 22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f 30 31 32 33 34 35 36 37 38 39 3a 3b 3c 3d 3e 3f

Testing: write to flash offset 0x20000 Reading from flash .. aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa Success

I now have 2 questions here:

  1. when I use the same code in my custom board (uses stm32g474rc that has 256K flash) it gives me the below error, while writing to the offset 0x40000. flash_erase failed (err:-22)

But surprisingly it works fine with the offset 0x20000.

What could be possibly wrong here?

  1. It doesnt make sense to me in G474 with only 128K size for each bank, how did it even allow me to successfully write to and read from the flash offset 0x20000. I thought this offset shouldn't even exist because the bank 2 starts from offset 0x40000 as per the user manual. Also the maximum bank size is 128k = 0x20000. So, where in flash did my write actually save the bytes?
ABOSTM commented 3 years ago

Hi @azeemshatp, It is working fine on my side (with default partition settings)

Because flash_shell sample activate this switch, it may explain it is working when you use flah_shell sample. Can you check the same?

  1. It doesnt make sense to me in G474 with only 128K size for each bank, how did it even allow me to successfully write to and read from the flash offset 0x20000.

G474 in dualbank configuration has 2 bank of 128 pages, with 2K pages. So 1st bank ends at 0x3FFFF. So 0x20000 is part of bank1

azeemshatp commented 3 years ago

MPU_ALLOW_FLASH_WRITE was already activated in my case.

Regarding the point 2: May be I am missing something here. I am using STM32G474RCT, which is a 256K flash version. So this can only have a single bank size of 128K max. So if we consider 2k sized pages in dual bank mode, there could only be 64 pages per bank, or not?

azeemshatp commented 3 years ago

Just noticed that the nucleo board where I found the code was working fine, has the MCU STM32G474RET, which is a 512K flash version. So the issue seems to be only with the 256K version.

ABOSTM commented 3 years ago

I am using STM32G474RCT,

Ok, as it was not specified, I was supposing you used nucleo_g474re which is supported in zephyr. So now, knowing that it seems you can write to address 0x08020000 and not at 0x08040000, maybe you are in single bank configuration. Can you check the value of option byte DBANK ?Aand make sure it is set to 1.

azeemshatp commented 3 years ago

The value of option byte DBANK was 1 already. However, I also had the options nSWBOOT0=0 and nBOOT0=1, for hardware specific reasons.

Just another note: In the 256k version, I was able to write manually to the Bank 2 (Address 0x08040000) correctly using STM32CubeProgrammer. So the Bank 2 address seems to be correct. But writing using the Zephyr API is what gave me the error.

ABOSTM commented 3 years ago

So with the dualbank configuration on STM32G474RCT, there is a discontinuity between bank1 and bank2 (0x08020000 -- 0x0803FFFF is physically not accesssible). Unfortunately, it seems STM32G4 flash driver is not able to tackle this discontinuity, specially when creating flash layout. In other word it assumes that bank2 is contigiuous to bank1, which explain why you cannot write at 0x08040000. I will check whether we can trick the driver to make it work.

martinjaeger commented 3 years ago

Also looking into this a bit. It seems like the issue is even beyond the STM32G4 implementation. The struct flash_pages_layout only contains the number of pages and their size, but no offset. So even if flash_stm32_page_layout in the flash_stm32g4x.c returned a separate layout for each bank, the function flash_get_page_info would not be able to know about the gap in the flash layout.

A very dirty trick to make it work is to just multiply the pages count by a factor of 2: https://github.com/zephyrproject-rtos/zephyr/blob/e83fab32d721034feabee8f1192bd2a9b8598e3b/drivers/flash/flash_stm32g4x.c#L192

I tried this out and it works. But it's interesting that I can still read out the written data at 0x08020000 from the previous tries using a debugger. So now I have the data in both locations. What would be the expected result if writing to a memory location that's physically not present (the discontinuity). I double-checked on my custom board, it's definitely an STM32G474RCT6. Same as @azeemshatp stated above.

martinjaeger commented 3 years ago

Played around a bit more and I can confirm that 0x08020000 and 0x08040000 are not mapped to the same physical memory, as I can store different data which persists after reset. Seems like we get additional 256k flash for free if we ignore the FLASH_SIZE register ;)

Btw, the value of FLASH_SIZE register correctly reads 0x0100FFFF, which means 256k.

ABOSTM commented 3 years ago

Hi, I don't want to modify flash_pages_layout as it is defined out of stm32 stuff, so I tried a much more complex trick. I push a branch in my fork (https://github.com/ABOSTM/zephyr.git): branch: FLASH_G4_BANK_DISCONTINUITY

It seems working but I don't have STM32G474RCT6 chip to test. @azeemshatp , @martinjaeger, could you test my branch on your board ? I am interested by status of both operations :

Seems like we get additional 256k flash for free if we ignore the FLASH_SIZE register ;)

You are lucky :smile: ... but you have no guaranty of any kind.

martinjaeger commented 3 years ago

Morning @ABOSTM. Tested your solution and it works like a charm.

At address 0x08040000 I can successfully read and write, at 0x08020000 I get E: Erase range invalid. Offset: 131072, len: 64.

I like the idea to create a large dummy flash page. Much cleaner than my quick hack. Are you creating a PR?

azeemshatp commented 3 years ago

Thanks a lot @ABOSTM and @martinjaeger for the fix. I am also gonna try this, and will update you how it went.

azeemshatp commented 3 years ago

With the workaround, just noticed that erasing the offset 0x1F800 as below throws error when I use FLASH_PAGE_SIZE. Works fine when I used a smaller size = 64. flash_erase(flash_device, offset, FLASH_PAGE_SIZE);

Could you please check this @martinjaeger

martinjaeger commented 3 years ago

@azeemshatp: Yes, same issue here. FLASH_PAGE_SIZE is correctly set to 2048 (verified with printf), so there seems to be a bug somewhere in the validation.

ABOSTM commented 3 years ago

I created the PR #30231, with a cleaner version of the patch. I fix the issue of offset 0x1F800 (condition at the border)

azeemshatp commented 3 years ago

Dear @ABOSTM Did you get a chance to look at the issue that I mentioned in the PR already?. A write to the offset 0x5F800 from Bank 2 fails every second time giving me the below log:

Booting Zephyr OS build zephyr-v2.4.0 Found flash controller FLASH_CTRL. Flash I/O commands can be run. Flash offset: 5F800 Reading from flash .. 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f 10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f 20 21 22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f 30 31 32 33 34 35 36 37 38 39 3a 3b 3c 3d 3e 3f Flash offset: 5F800 E: Word at offs 391168 not erased flash_write failed (err:-5).

I was using the below code to test,

uint8_t data[64];
uint8_t buf[64];
int ret;
size_t size = 64;

// off_t offset = 0x20000;
// off_t offset = 0x40000;
off_t offset = 0x5F800;

static const struct device *flash_device;
flash_device =
    device_get_binding(DT_CHOSEN_ZEPHYR_FLASH_CONTROLLER_LABEL);
if (flash_device) {
    printk("Found flash controller %s.\n",
        DT_CHOSEN_ZEPHYR_FLASH_CONTROLLER_LABEL);
    printk("Flash I/O commands can be run.\n");
} else {
    printk("**No flash controller found!**\n");
    printk("Run set_device <name> to specify one "
           "before using other commands.\n");
}

printf("Flash offset: %.X \r\n", offset);

for(int i = 0; i < 64; i++) {data[i]= i;}

ret = flash_write_protection_set(flash_device, false);
if (ret) {
    printk("Failed to disable flash protection (err: %d)."
            "\n", ret);
    return;
}
ret = flash_erase(flash_device, offset, size);
if (ret) {
    printk("flash_erase failed (err:%d).\n", ret);
    return;
}

ret = flash_write(flash_device, offset, &data, sizeof(data));
if (ret) {
    printk("flash_write failed (err:%d).\n", ret);
    return;
}

ret = flash_write_protection_set(flash_device, true);
if (ret) {
    printk("Failed to enable flash protection (err: %d)."
            "\n", ret);
    return;
}

ret = flash_read(flash_device, offset, buf, sizeof(buf));
if (ret) {
    printf("Error reading from flash \r\n");
}
else {
    printf("Reading from flash .. \r\n");
    for (int k = 0; k < sizeof(buf); k++) {
        printf("%.2x ", buf[k]);
    }
    printf("\r\n");
}

printf("Flash offset: %.X \r\n", offset);

for(int i = 0; i < 64; i++) {data[i]= 0xAA;}

ret = flash_write_protection_set(flash_device, false);
if (ret) {
    printk("Failed to disable flash protection (err: %d)."
            "\n", ret);
    return;
}
ret = flash_erase(flash_device, offset, size);
if (ret) {
    printk("flash_erase failed (err:%d).\n", ret);
    return;
}

ret = flash_write(flash_device, offset, &data, sizeof(data));
if (ret) {
    printk("flash_write failed (err:%d).\n", ret);
    return;
}

ret = flash_write_protection_set(flash_device, true);
if (ret) {
    printk("Failed to enable flash protection (err: %d)."
            "\n", ret);
    return;
}

ret = flash_read(flash_device, offset, buf, sizeof(buf));
if (ret) {
    printf("Error reading from flash \r\n");
}
else {
    printf("Reading from flash .. \r\n");
    for (int k = 0; k < sizeof(buf); k++) {
        printf("%.2x ", buf[k]);
    }
    printf("\r\n");
}

printk("Success");
ABOSTM commented 3 years ago

@azeemshatp can you please tell me the value at address 0x1FFF6FFE ? (You can read with STM32CubeProgrammer) Normally you should get something like 0xFFFF00D3.

azeemshatp commented 3 years ago

@ABOSTM It reads 0xFFFF00D4 and not 0xFFFF00D3

ABOSTM commented 3 years ago

Ok you are lucky: I have 0xFFFF00D3 (bootloader version) on my nucleo_g747re and there is an issue that bank are not swapped when booting from bank2. It is solved with 0xFFFF00D4. So I need to found a board with at least 0xFFFF00D4.

azeemshatp commented 3 years ago

Yea, you are right and that seemed to be fixed in the updated system bootloader.