flash: add note about program size

[0xc0170]

This unit is the minimum writable size that flash controller supports.

Raised here #4966

@LMESTM @LiyouZhou @c1728p9

[LMESTM]

LGTM
One question was raised though: is there a risk to actually degrade performance by defining the page size to 1 if the application tries to program page by page ?

[0xc0170]

One question was raised though: is there a risk to actually degrade performance by defining the page size to 1 if the application tries to program page by page ?

I would say yes. @LMESTM there is a PR from you that is setting it to a small number, you can test it by flashing big chunk of data (or do you have datasheets that would have timings for different page sizes) ?

[LMESTM]

@0xc0170 my point is about how people do interpret and use get_page_size as of now in mbed programs.
AS you state in this PR , page_size is the minimum size that can be programmed, but drivers using the API should definitely program several / many "pages" at a time, and I hope they do so.

[0xc0170]

I rebased, the minimum was not correct, as it invokes a question - what is then maximum. The page size defines a writeable page size - this is the number of bytes that flash controller supports. It could state the optimum writable chunk (in your case it would be set to optimum size).

drivers using the API should definitely program several / many "pages" at a time, and I hope they do so.

FlashIAP is protected with mutex, thus only one program page at the time.

[0xc0170]

Any review for this small change? @LMESTM Does it add a value as requested?

[LMESTM]

Looking a bit further, maybe it is possible to add a note like:
"The size to program with flash_program_page shall be a multiple of this page size."

And I think the flash_program_page needs some update as well.
For now it says

/** Program one page starting at defined address
...
int32_t flash_program_page(flash_t *obj, uint32_t address, const uint8_t *data, uint32_t size);

But looking at the code FlashIAP.cpp that calls to flash_program_page, it says

// addr and size should be aligned to page size, and multiple of page size

So I guess that flash_program_page (should be flash_program_pages ?) must actually program a entire number of pages (within a sector). not One

[LiyouZhou]

should we say minimum programmable size? There is no API that queries the capabilities of the underlying implementation. So i think this api should tell the user what they can do, not what they should do.

[0xc0170]

I fixed program_page HAL as suggested above by @LMESTM

[LMESTM]

Looks ok with this comment. And now that I better understand the API, maybe "minimum" could even be used, as the "maximum" size is known: this is the number of bytes until next sector boundary:

The pages should not cross multiple sectors.

[theotherjimmy]

/morph test

[mbed-bot]

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 1361

All builds and test passed!