[Flash Driver] Implementation guidelines request

[bash]

Hi,
I'm trying to create an open source bootloader with USB-DFU HID capability using ChibiOS/RT.

Actually (on a STM32F103xB) I can read/write internal flash partition using ST "STM32F10x standard peripheral library".

I would like to contribute with ChibiOS by implementing a Flash Driver for HAL; I read the following threads but I don't know how to start.

• viewtopic.php?f=25&t=3237
• viewtopic.php?f=3&t=3320
• viewtopic.php?f=3&t=3524
• viewtopic.php?f=16&t=3656

Could you help me to choose files, interfaces, and methods that I should implent?

[Giovanni]

Hi,

A flash driver should implement the "interface" defined in \os\hal\lib\peripherals\flash\hal_flash.h.

Note that the interface is new and could change based on feedback. Post your questions in this thread.

Giovanni

[bash]

Hi,

sorry for delay, but I've been busy.

attached you can find a draft of FlashDriver that I will implement when you confirm the structure.

I added these files:

• os/hal/ports/STM32/STM32F1xx/hal_flash_lld.c
• os/hal/ports/STM32/STM32F1xx/hal_flash_lld.h
• os/hal/src/hal_iflash.c
• os/hal/include/hal_iflash.h

hal_flash_lld, only for STM32F1xx, implements all methods exposed by hal_flash class.
Then it exports init, start and stop functions in order to align with other LLD.
Anyway I think internal flash is always "started", so only init function is necessary in order to initialize internal structures; start and stop functions could be removed ??

hal_iflash (I don't know how to call, because hal_flash was already present) provides FlashDriver structure with filled VMT.

I also modified these files:

• os/hal/hal.mk
• os/hal/include/hal.h
• os/hal/ports/STM32/STM32F1xx/platform.mk
• os/hal/src/hal.c

Do you approve this structure? Should I modify something before proceed?

[Giovanni]

Hi,

Thanks for the code, I am in the middle of merging other things so this will go in queue.

I see a problem with hal_iflash being added as an HAL generic driver, that would force me to update all halconf.h files in all (internal and external) demos and we are late in this development cycle. Release is supposed to be 17.2.

Probably it would be better to make a device-specific mini driver placed inside the various STM32 directories and named stm32_iflash.c/.h. Alternatively we could create FLASHv1, FLASHv2 etc under STM32/LLD if some of those drivers could be shared between multiple STM32s.

The flash object could be constant and static unless you have variable data there, start/stop functions could be omitted. Inclusion would be made in the makefile.

In the next HAL we may discuss an high level flash module in the HAL.

Giovanni

[bash]

Hi,

attached you can find a working flash driver for STM32F1xx.

Based on ST's PM0075 programming manual, it does not currently support bank2 of STM32F1xx_XL devices.
A warning is generated at compile time.

I modified STM32F1xx/platform.mk only into USE_SMART_BUILD section and the driver could be enabled by adding
UDEFS += -DHAL_USE_FLASH=TRUE in project Makefile.
Is it ok?


Could you explain me this sentence?

The flash object could be constant and static unless you have variable data there [...]

Other LLD drivers are not static because they are global variables. How could I set static FlashDriver instance?
Then FlashDriver could not be const because the driver needs flash_state_t state variable.


At this time it is not thread safe. Is it mandatory to have thread safeness for all drivers?


I suggest to export lock/unlock interface to hal_flash.h because these functions are invoked at every erase/program calls.
Instead a possible workflow could be (1)unlock flash, (2) erase page, (3) program page, repeat (2) and (3) for every pages in flash and, at last, (4) lock flash.
What do you think?


Edited:
I changed the attachment in order to align with new ChibiOS commits.

[Giovanni]

Hi,

Looks good, just one thing I noticed is that the driver is not able to suspend erase but the capability bit is set, a minor thing.

Lets wait for feedback.

Giovanni