qmk_firmware icon indicating copy to clipboard operation
qmk_firmware copied to clipboard

Support specific STM32 (flash) memory variants

Open elagil opened this issue 2 years ago • 6 comments

The problem

STM32 processors come in different memory and feature variants for each model family.

For example, the STM32F042 exists in two categories of versions that differ in memory sizes:

  • STM32F042x4 with 16 kB of flash
  • STM32F042x6 with 32 kB of flash

Currently, QMK simply assumes a flash size and thus chooses a linker file, like here.

This does not necessarily match the memory layout of the chip that was used on a board, so the keyboard's MCU setting should instead specify a certain chip.

Solving it

I propose to use this place for general discussions.

Previous work was done in:

  • https://github.com/qmk/qmk_firmware/pull/8179 → Adds specific MCU models to all boards.
  • https://github.com/qmk/qmk_firmware/pull/16529 → Proposes a structure for implementing granular selection of a certain chip.
  • https://github.com/qmk/qmk_firmware/pull/16536 → This is my attempt at fixing this issue.

Note that memory suffixes are always the same:

  • 4 → 16 kB of flash
  • 6 → 32 kB of flash
  • 8 → 64 kB of flash
  • B → 128 kB of flash
  • C → 256 kB of flash
  • etc.

elagil avatar Mar 05 '22 13:03 elagil

At this point, perhaps we should discuss whether or not we should be generating the ldscripts. @fauxpark what do you think?

As far as I can tell the majority of the ones used for ChibiOS are identical, with the only things changing being the flash size, the ram size, and if we've got a different offset for placing the resulting binary (custom bootloader etc.) -- the rest should be basically common between all of them.

I can't see it being terribly fun migrating all the existing boards to add suffixes, especially with the debate that occurred on #8179 -- perhaps we just set up default flash sizes in the mcu_selection (STM32F042 => automatically STM32F042x406 or whatever), allow for override in keyboard (like changing B/C/Z/D/E suffix etc.) as per the STM32F042x4 you've got in the examples.

Not sure I want to explicitly define each variant (F042, F042x6, F042x8) in the schema/constants, though.

tzarc avatar Mar 06 '22 00:03 tzarc

Some remarks from my side.

whether or not we should be generating the ldscripts

That is certainly possible, but since ChibiOS provides linker scripts for all supported processors, I don't see the benefit. We would have to convert the complexity of script selection to a (higher?) complexity of script generation. For example, when looking at the linker scripts for STM32F072xB and STM32F746xG processors, the latter has additional memory regions, not just one flash and one RAM area. Regions are also optionally assigned alias names for use in firmware (e.g. "ETH_RAM" for ethernet buffers on the F7 chip). I know that we don't need all this functionality, but generating a linker file is not always trivial - and we already have all linker files that we need.

I can't see it being terribly fun migrating all the existing boards to add suffixes

This is quick to do with search/replace, because we can assign the same processor variant to all of them. I am aware of the discussion in https://github.com/qmk/qmk_firmware/pull/8179. However, in the current state of the firmware, not being able to select the memory configuration forces you to use the default configuration that the Makefile dictates. So why not just replace the current MCU values with the default values that were used until now? That makes sure, all boards are compiled/linked the same way as before.

Not sure I want to explicitly define each variant (F042, F042x6, F042x8) in the schema/constants, though.

I would rather remove the check in the schema file, because MCU compatibility is checked again twice during compilation:

  • In the info.py script
  • In the Makefile

I don't like the idea of having two identical lists of supported processors in info.py and keyboard.jsonschema, which you have to maintain in parallel. One should be enough.

As an advantage to the Python script, the check in info.py does not require adding all variants explicitly, as I only match against the beginning of the processor name in my PR.

I also suggest something a bit more flexible for defining supported processors. Consider this small example:

@dataclass
class Processor:
    family_name: str            # e.g. "STM32F042"
    bootloader: str             # e.g. "stm32-dfu"
    valid_affixes: List[str]    # e.g. ['4', '6']

    def matches(self, processor: str) -> bool:
        """Checks, whether the provided processor name matches this Processor definition.

        Args:
            processor (str): The processor to check.

        Returns:
            bool: True, if the name matches, False otherwise.
        """
        if processor.startswith(self.family_name) and processor[-1] in self.valid_affixes:
            return True

        else:
            return False

CHIBIOS_PROCESSORS = [Processor('STM32F042', 'stm32-dfu', ['4', '6']), Processor('STM32F072', 'stm32-dfu', ['4', '6'])]

# Exemplary check:
def check_if_processor_is_in_list(processor: str):
    return any([p.matches(processor) for p in CHIBIOS_PROCESSORS])
    
check_if_processor_is_in_list('STM32F042x6') # Will return True
check_if_processor_is_in_list('STM32F043x5') # Will return False
check_if_processor_is_in_list('STM32F043x6') # Will return False

elagil avatar Mar 06 '22 09:03 elagil

I made some changes to my PR that follow the above suggestion. Please consider https://github.com/qmk/qmk_firmware/pull/16536/commits/f9dab28ace2f74aa94663a66561d94f2d71d6690.

It includes a list of all currently supported processors, and also lists the available memory configurations. This matches the list of available linker files from ChibiOS.

I also updated all affected keyboards, by specifying the processor type in such a way that it builds with the same configuration as in the current state of QMK: https://github.com/qmk/qmk_firmware/pull/16536/commits/6713a0d30cf2d932c5ffd035665c06c7b2ddf093.

elagil avatar Mar 06 '22 11:03 elagil

This issue has been automatically marked as stale because it has not had activity in the last 90 days. It will be closed in the next 30 days unless it is tagged properly or other activity occurs. For maintainers: Please label with bug, in progress, on hold, discussion or to do to prevent the issue from being re-flagged.

stale[bot] avatar Jun 12 '22 18:06 stale[bot]

This issue has been automatically closed because it has not had activity in the last 30 days. If this issue is still valid, re-open the issue and let us know. // [stale-action-closed]

github-actions[bot] avatar Jul 20 '22 02:07 github-actions[bot]

Reopening this, because the problem is still there.

That [generating the ldscripts] is certainly possible, but since ChibiOS provides linker scripts for all supported processors, I don't see the benefit.

Actually one of the problems is that ChibiOS does not provide linker script for all supported MCUs — in many cases it does not provide scripts for some intermediate flash sizes, but getting the flash size correct is important if we want to have a working firmware size check for these MCUs. One particular case that does not seem to be handled anywhere is STM32F303xB vs STM32F303xC (e.g., https://github.com/qmk/qmk_firmware/pull/8179#discussion_r379912229 and https://github.com/qmk/qmk_firmware/issues/10836 — apparently the firmware built for STM32F303xC runs on STM32F303xB only because of the undocumented presence of extra SRAM (!) and flash on F303xB chips).

Also generating actual files might not be needed, because the required information can be passed on the linker command line, using something like

LDFLAGS = -T STM32_generic.ld -Wl,--defsym=__mcu_flash_size=131072 -Wl,--defsym=__mcu_ram_size=20480

and STM32_generic.ld would contain something like

__bootloader_start_size = DEFINED(__bootloader_start_size) ? __bootloader_start_size : 0;
__bootloader_end_size = DEFINED(__bootloader_end_size) ? __bootloader_end_size : 0;
__storage_flash_size = DEFINED(__storage_flash_size) ? __storage_flash_size : 0;
__storage_flash_offset = DEFINED(__storage_flash_offset) ? __storage_flash_offset : 0;

MEMORY
{
    flash0  : org = 0x08000000 + __bootloader_start_size,
              len = __mcu_flash_size - __bootloader_start_size - __bootloader_end_size - ((__storage_flash_offset == 0) ? __storage_flash_size : 0)
    flash1  : org = (__storage_flash_size == 0) ? 0x00000000 : 0x08000000 + ((__storage_flash_offset == 0) ? __mcu_flash_size - __bootloader_end_size - __storage_flash_size : __storage_flash_offset),
              len = __storage_flash_size
    ram0   (wx) : org = 0x20000000, len = __mcu_ram_size
}

But this would require maintaining a registry of supported STM32 chip variants somewhere anyway (just not in the form of *.ld files). Although we could also have a generic LD file as above and small LD files defining just the flash and RAM sizes; this might even be better, because then we could put any non-generic LD files there too and treat them in the same way (in case someone ever wants to use those F7xx chips with lots of RAM areas).

Also F303 chips have an extra CCM RAM region and therefore might need a separate LD file (but the current LD files for those chips don't actually use that memory region for anything; we can also define it using some symbols too).

sigprof avatar Jul 24 '22 14:07 sigprof

This issue has been automatically marked as stale because it has not had activity in the last 90 days. It will be closed in the next 30 days unless it is tagged properly or other activity occurs. For maintainers: Please label with bug, in progress, on hold, discussion or to do to prevent the issue from being re-flagged.

github-actions[bot] avatar Oct 24 '22 02:10 github-actions[bot]

This issue has been automatically closed because it has not had activity in the last 30 days. If this issue is still valid, re-open the issue and let us know. // [stale-action-closed]

github-actions[bot] avatar Nov 24 '22 02:11 github-actions[bot]