Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

STM32F1xx (really just f103xe): support for SDIO flashing #6679

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bevanweiss
Copy link
Contributor

@bevanweiss bevanweiss commented Sep 6, 2024

Add support for GPIO_HIGH_SPEED mode for STM32F1xx mcus, allows for them to be set to 50MHz (with GPIO_HIGH_SPEED flag) instead of the 10MHz limit currently imposed.

Also change how Alternative Output / Open Drain mode is selected, now it needs both GPIO_FUNCTION(n) AND GPIO_OUTPUT / GPIO_INPUT. This is to allow for GPIO_FUNCTION(n) usage with default Pull Ups on other STM32 series (like for SDIO where STM32F4 configure these with pullups).
This has required the addition of GPIO_OUTPUT flags on several AF outputs. (FD)CAN, SERIAL, SDIO, PWM, SPI and USB.

Also fixed up the clock line selection for SDIO and FMSC lines. These don't appear to fit with the magic calculation previously in use.

The addition of SDIO for the stm32f103 allows for upgrades of the Tronxy X5SA Chitu-v6 mainboard without needing to touch the power switch at all :)

I've left (commented out) the swspi configuration for the flashsd, since it's still the easiest way of getting the SDIO compatible firmware onto the unit.

Also added some distinction between the SPI and SDIO modes of SD card updating.

Checking FatFS CFFI Build...
Connecting to MCU....Connected
Checking Current MCU Configuration...Done
MCU needs restart: is_config=1, is_shutdown=0
Attempting MCU Reset...Done
Waiting for device to reconnect...Done
Connecting to MCU....Connected
Initializing SD Card (over SDIO) and Mounting file system...

SD Card Information:
Version: 2.0
SDHC/SDXC: True
Write Protected: False
Sectors: 15728640
manufacturer_id: 254
oem_id: 42
product_name: SD16G
product_revision: 2.32
serial_number: 00009F68
manufacturing_date: 3/2021
capacity: 7.5 GiB
fs_type: FAT32
volume_label: b''
volume_serial: 3401818794
Uploading Klipper Firmware to SD Card...Done
Validating Upload...Done
Firmware Upload Complete: update.cbd, Size: 36876, Checksum (SHA1): 54ED77C47D87FAC6B04D94439E224999694FA7D0
Attempting MCU Reset...Done
Waiting for device to reconnect...Done
Connecting to MCU....Connected
Verifying Flash...Version matched...Done
Found and deleted update.cbd after reset
Firmware Flash Successful
Current Firmware: v0.12.0-291-gf5be9cb1e
Attempting MCU Reset...Done
SD Card Flash Complete

src/stm32/can.c Outdated Show resolved Hide resolved
src/stm32/stm32f1.c Outdated Show resolved Hide resolved
@Arksine
Copy link
Collaborator

Arksine commented Sep 8, 2024

I had a couple of initial comments. In addition to those, as you note, the SDIO peripheral is only available on the high capacity and XL capacity variants of the STM32F103. It will likely be necessary to add a new KConfig choice to make sure we dont attempt to enable the SDIO peripheral on standard capacity F103s.

@bevanweiss
Copy link
Contributor Author

bevanweiss commented Sep 8, 2024

I had a couple of initial comments. In addition to those, as you note, the SDIO peripheral is only available on the high capacity and XL capacity variants of the STM32F103. It will likely be necessary to add a new KConfig choice to make sure we dont attempt to enable the SDIO peripheral on standard capacity F103s.

It does seem 'ok' for the current F103's in the Klipper configuration set.
I have seen STMicro reference some other defines in some of their sample code. I'll see if I can trace this down to possibly add it in.
EDIT: Found it
/* #define STM32F10X_LD / /!< STM32F10X_LD: STM32 Low density devices /
/
#define STM32F10X_LD_VL / /!< STM32F10X_LD_VL: STM32 Low density Value Line devices /
/
#define STM32F10X_MD / /!< STM32F10X_MD: STM32 Medium density devices /
/
#define STM32F10X_MD_VL / /!< STM32F10X_MD_VL: STM32 Medium density Value Line devices /
/
#define STM32F10X_HD / /!< STM32F10X_HD: STM32 High density devices /
/
#define STM32F10X_HD_VL / /!< STM32F10X_HD_VL: STM32 High density value line devices /
/
#define STM32F10X_XL / /!< STM32F10X_XL: STM32 XL-density devices /
/
#define STM32F10X_CL / /!< STM32F10X_CL: STM32 Connectivity line devices */

Unfortunately the defines aren't in the direction I was hoping for (where a define for a specific MCU would bring in the appropriate density define also). I might need to create additional options in menuconfig for the sub-specialities of the STM32F103xx

@Sineos
Copy link
Collaborator

Sineos commented Sep 8, 2024

I might need to create additional options in menuconfig for the sub-specialities of the STM32F103xx

Not sure if this is worth it.
Today's options are already confusing for most users, so I see only very little value in adding a bunch of new options just to enable this feature.

@bevanweiss
Copy link
Contributor Author

I might need to create additional options in menuconfig for the sub-specialities of the STM32F103xx

Not sure if this is worth it. Today's options are already confusing for most users, so I see only very little value in adding a bunch of new options just to enable this feature.

What's the counter option?... are you suggesting no support for SDIO?

There's at least two significant groups of devices in the STM32F103 series of mcus (XL and LD),
which vary from sub 64kB flash/8kb RAM, no FSMC, no SDIO,
up to devices with 1MB flash/96kB RAm, FSMC and SDIO.
The current code assumes all STM32F103 devices have only 64kB Flash, and 20kB of RAM (STM32F103x8).
Having an STM32F103 (low/mid density) and STM32F103 (high/xl density) would allow for more distinction between the two groups, high/xl supports FSMC and SDIO.

@Sineos
Copy link
Collaborator

Sineos commented Sep 8, 2024

... are you suggesting no support for SDIO?

So far, I haven't seen anyone requesting this feature, and if there is no compelling real-world benefit, then yes, I suggest not adding more options, as the complexity is already too high, even if it means that it can't be used.

@bevanweiss
Copy link
Contributor Author

... are you suggesting no support for SDIO?

So far, I haven't seen anyone requesting this feature, and if there is no compelling real-world benefit, then yes, I suggest not adding more options, as the complexity is already too high, even if it means that it can't be used.

The compelling real world benefit is touchless firmware updates.
Current stock bootloaders largely use SDIO. And hence if SPI is used to do the SD flash, then going to the machine, and physically removing power is required to have the bootloader re-init for SDIO to do the update.
If Klipper uses SDIO itself, then the SD write, and bootloader flash are all done without having to touch the printer at all.

Perhaps the STM32F103x8 mcu support could be dropped. I don't see any printer that actually uses this mcu, they all use the high/XL density versions that I'm aware of. So the STM32F103 could equate to high/XL density.

@Arksine
Copy link
Collaborator

Arksine commented Sep 8, 2024

Perhaps the STM32F103x8 mcu support could be dropped. I don't see any printer that actually uses this mcu, they all use the high/XL density versions that I'm aware of. So the STM32F103 could equate to high/XL density.

There are likely numerous boards like the blue pill used as an auxiliary MCU to drive LEDs, displays, etc. In fact, there is a boolean option to support the x6 variant with 10KiB of RAM, so we know those are being used.

I don't see the harm in adding another STM32F103 variant to the Processor Model menu. An alternative would be to change that boolean to a menu, where the user can select between the x6, x8, and xe variants.

@bevanweiss
Copy link
Contributor Author

I don't see the harm in adding another STM32F103 variant to the Processor Model menu. An alternative would be to change that boolean to a menu, where the user can select between the x6, x8, and xe variants.

image
image

Any objections to something like this?
I'd have things like SDIO / FSMC(/FMC) (future) driven by the particular grouped subset (i.e. STM32F10X_XL) which would be selected by the specific subtype chosen (defaulting to STM32F103xE I think.. that matches the mcu name currently used).

Is there a KConfig guru that has guidance on how to 'select' the int value for RAM / FLASH from a choice selection without having to just select another specific boolean to then use in the config default match for the int?
image

@Arksine
Copy link
Collaborator

Arksine commented Sep 9, 2024

Any objections to something like this? I'd have things like SDIO / FSMC(/FMC) (future) driven by the particular grouped subset (i.e. STM32F10X_XL) which would be selected by the specific subtype chosen (defaulting to STM32F103xE I think.. that matches the mcu name currently used).

I'm ok with it, however Kevin may have some additional feedback since its a change to the build configuration. I do think the default needs to be equivalent to the current STM32F103 configuration, so I suspect its going to need to be the x8 variant as you show in the image. In addition, I don't think there is value in adding the x4 variant since Klipper cannot fit in 16kB of flash.

Is there a KConfig guru that has guidance on how to 'select' the int value for RAM / FLASH from a choice selection without having to just select another specific boolean to then use in the config default match for the int?

You can't do that, however you do not need to add another boolean. Instead you need to modify config RAM_SIZE and config FLASH_SIZE to use the specific variant selected in the subtype choice. For example, you could change config RAM_SIZE to something like the following:

@@ -236,18 +246,19 @@ config RAM_SIZE
     default 0x1800 if MACH_STM32F042
     default 0x4000 if MACH_STM32F070 || MACH_STM32F072
     default 0x2800 if MACH_STM32F103x6
-    default 0x5000 if MACH_STM32F103 && !MACH_STM32F103x6 # Ram size of stm32f103x8
+    default 0x5000 if MACH_STM32F103x8 || MACH_STM32F103xB
     default 0x8000 if MACH_STM32G431
     default 0x20000 if MACH_STM32G474
-    default 0xa000 if MACH_STM32L412
+    default 0xa000 if MACH_STM32L412 || MACH_STM32F103xC
     default 0x20000 if MACH_STM32F207
-    default 0x10000 if MACH_STM32F401
+    default 0x10000 if MACH_STM32F401 || MACH_STM32F103xD || MACH_STM32F103xE
     default 0x20000 if MACH_STM32F4x5 || MACH_STM32F446
     default 0x80000 if MACH_STM32F765
     default 0x9000 if MACH_STM32G07x
     default 0x24000 if MACH_STM32G0Bx
     default 0x20000 if MACH_STM32H7
     default 0x10000 if MACH_N32G45x
+    default 0x18000 if MACH_STM32F103xF || MACH_STM32F103xG

@KevinOConnor
Copy link
Collaborator

Thanks.

I haven't reviewed this PR, but I can provide a couple of high-level comments (take with a "grain of salt"):

  1. I'd prefer to avoid make menuconfig options if possible. In general, I'd look to add a new Kconfig option if the information is necessary to bootup that board. In general, I'd avoid adding Kconfig options to try to save users from a later misconfiguration. So, for example, we generally compile each mcu for the maximum number of pins available on that entire series of chips (and we don't ask users to select individual mcu types just to limit them to the subset of pins actually available).
  2. For what it is worth, I'm not sure pursuing sdio is worthwhile. It's never actually been enabled on any board. Now that Katapult has been ported and tested to so many boards, using it may be a better option. (That is, if the chitlu-v6 has a wonky bootloader that requires sdio support, both users and developers may be better served by replacing that bootloader instead of devising an elaborate mechanism to utilize it.)

Cheers,
-Kevin

@bevanweiss
Copy link
Contributor Author

  1. I'd prefer to avoid make menuconfig options if possible. In general, I'd look to add a new Kconfig option if the information is necessary to bootup that board. In general, I'd avoid adding Kconfig options to try to save users from a later misconfiguration. So, for example, we generally compile each mcu for the maximum number of pins available on that entire series of chips (and we don't ask users to select individual mcu types just to limit them to the subset of pins actually available).

I was hoping for the opposite here. My thoughts are that the Klipper printer.cfg file should ideally specify the low level MCU model codes for each mcu. So that less advanced end users don't even need to concern themselves with which board / mcu their stock printer has. They can just use the stock printer.cfg, which would give them 'the right' firmware for it.

  1. For what it is worth, I'm not sure pursuing sdio is worthwhile. It's never actually been enabled on any board. Now that Katapult has been ported and tested to so many boards, using it may be a better option. (That is, if the chitlu-v6 has a wonky bootloader that requires sdio support, both users and developers may be better served by replacing that bootloader instead of devising an elaborate mechanism to utilize it.)

The SDIO was actually just a step on my path to something else. I want to bring in FSMC/FMC support for the STM32 series, so that I can use the LCD on my Tronxy from within Klipper. I would think vendors (both OEM and 3rd party LCD) could then leverage this to stay closer to stock Klipper (without dodging up LCD add-ons / proprietary gcodes etc). I just got tired of having to turn the mainboard off all the time...

I'd think SDIO support in Klipper would be nice for Katapult also.

@bevanweiss bevanweiss force-pushed the support_hispeedperiph_stm32f1xx branch 3 times, most recently from 62dd403 to 5a1953f Compare September 10, 2024 10:40
this just sets them to 50MHz instead of the 10MHz fixed currently.

Adds SDIO mode for STM32F103 MCUs


Signed-off-by: Bevan Weiss <bevan.weiss@gmail.com>
@bevanweiss bevanweiss force-pushed the support_hispeedperiph_stm32f1xx branch from 5a1953f to 149be97 Compare September 10, 2024 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants