Add mcu HC32f460 common to Voxelab Aquila, X2 and 1.03 mainboard

[SteveGotthardt]

Adding support for the Huada HC32F460. This processor is shipping on Voxelab mainboards.
The chip has a common footprint with STM32 series, but the IP for ADC, USART are very different and needed new src files.

I tested it on mainsail and Octoprint with Octoklipper.

Notes: https://gist.github.com/SteveGotthardt/2ed249f197f5928eaf5c3d3877d55503

Signed-off-by: Steven Gotthardt gotthardt@gmail.com

[KevinOConnor]

Interesting, thanks. Sorry for the delay in responding.

As high-level feedback, I'm a bit leery of importing in a full vendor HAL/BSP implementation to Klipper - in particular for a "clone" chip. My experience has been that the STM and Atmel HAL implementations have notable quality issues. Klipper typically writes directly to the hardware registers (ie, bare metal), and I think that approach has reduced overall maintenance costs.

Are these chips entirely incompatible with the stm32 chips at the register level - or is it just some peripherals that are not compatible? Is it possible to override just a handful of the files in src/stm32/ (eg, hc32f460_serial.c)?

Separately, a big change like this would need to be rebased and broken up into commits by function - for example, a commit adding base library files, a commit adding micro-controller support, a commit adding example config files. See https://www.klipper3d.org/CONTRIBUTING.html .

Thanks again,
-Kevin

[SteveGotthardt]

The hc32f460 has very different peripherals, clock setup, GPIO, ADC, etc. Also the interrupts and vectors are different. The CM4 is same (systick, etc) and the pinout is close. So it does not seem to be a close variation of the STM32 family (It may be able to be shoehorned in, but I think it will be very messy)

For the library - I was just following the contribution guidelines to import most of the library without changing - I could just pull the applicable routines in (like I did for the bloated irq stuff). Would that be better if I could leave off the library completely?

Bigger picture question (not that this will happen) What distinguishes a STM32? is that just generic for a CM flavor? Would CM3, CM4, etc be a better categorization?

[KevinOConnor]

Okay. I'm not familiar with the hc32f460, so if I get something wrong, just let me know.

The hc32f460 has very different peripherals, clock setup, GPIO, ADC, etc.

If there isn't common hardware devices that can share code between the stm32 line and hc32f460 then I agree they should just be kept separate. Creating a separate src/hc32f460 directory makes sense in that case.

For the library - I was just following the contribution guidelines to import most of the library without changing

On most architectures we generally only pull in the register definitions (the equivalent of lib/hc32f460/mcu/common/hc32f460.h) and then write directly to those memory mapped registers. So, instead of something like (from src/hc32f460/gpio.c):

void
gpio_peripheral(uint32_t gpio, int func, int pull_up)
{
    stc_port_init_t stcPortInit;

    irqstatus_t flag = irq_save();

    stcPortInit.enPinMode       = func;
    stcPortInit.enLatch         = Disable;
    stcPortInit.enExInt         = Disable;
    stcPortInit.enInvert        = Disable;
    stcPortInit.enPullUp        = pull_up ? Enable : Disable;
    stcPortInit.enPinDrv        = Pin_Drv_L;
    stcPortInit.enPinOType      = Pin_OType_Cmos;
    stcPortInit.enPinSubFunc    = Disable;

    PORT_Init(GPIO2PORT(gpio), GPIO2BIT(gpio), &stcPortInit);

    irq_restore(flag);
}

we'd typically have something like (from src/stm32/gpioperiph.c):

void
gpio_peripheral(uint32_t gpio, uint32_t mode, int pullup)
{
    GPIO_TypeDef *regs = digital_regs[GPIO2PORT(gpio)];

    // Enable GPIO clock
    gpio_clock_enable(regs);

    // Configure GPIO
    uint32_t mode_bits = mode & 0xf, func = (mode >> 4) & 0xf, od = mode >> 8;
    uint32_t pup = pullup ? (pullup > 0 ? 1 : 2) : 0;
    uint32_t pos = gpio % 16, af_reg = pos / 8;
    uint32_t af_shift = (pos % 8) * 4, af_msk = 0x0f << af_shift;
    uint32_t m_shift = pos * 2, m_msk = 0x03 << m_shift;

    regs->AFR[af_reg] = (regs->AFR[af_reg] & ~af_msk) | (func << af_shift);
    regs->MODER = (regs->MODER & ~m_msk) | (mode_bits << m_shift);
    regs->PUPDR = (regs->PUPDR & ~m_msk) | (pup << m_shift);
    regs->OTYPER = (regs->OTYPER & ~(1 << pos)) | (od << pos);

    // Setup OSPEEDR:
    // stm32f0 is ~10Mhz at 50pF
    // stm32f2 is ~25Mhz at 40pF
    // stm32f4 is ~50Mhz at 40pF
    // stm32g0 is ~30Mhz at 50pF
    // stm32h7 is ~85Mhz at 50pF
    uint32_t ospeed = CONFIG_MACH_STM32F0 ? 0x01 : 0x02;
    regs->OSPEEDR = (regs->OSPEEDR & ~m_msk) | (ospeed << m_shift);
}

It's a slightly different way of doing things that doesn't require pulling in big HAL libraries.

To be clear, I don't see an issue with pulling in a handful of c files from a library/hal if you want to use them. For example, there is lib/stm32f4/system_stm32f4xx.c . Sometimes we even pull in a handful of library c files and modify them (for example, lib/lpc176x/device/system_LPC17xx.c and the documented changes in lib/lpc176x/lpc176x.patch).

We try to keep the lib/ code pristine so that it's clear to future developers what is "upstream code", what is "Klipper code", and what is been "modified upstream code". If there are c files in the vendor HAL that aren't used at all then they don't need to be pulled into the Klipper repo. Unfortunately, many of the vendor HALs have lots of layered includes that make it very hard to pull in just the library code that is desired. I don't know if this is the case for the hc32f460 hal. Ultimately, it becomes a judgement call on balancing various code maintenance goals. On most architectures, just writing to the memory mapped registers directly seems the best way to avoid tangled HAL includes (and other quality issues they have). Looking at your PR today, I see 200K+ lines of added library code and I'm not sure that's a good balance.

Bigger picture question (not that this will happen) What distinguishes a STM32? is that just generic for a CM flavor? Would CM3, CM4, etc be a better categorization?

We keep all the stm32 chip support in the src/stm32/ directory because these chips often use the same "low-level hardware blocks". For example, src/stm32/spi.c is used on every stm32 chip we support. The watchdog.c is similar. The usbfs.c is used on several chips, while all the others use usbotg.c. So, it's a practical distinction around code reuse.

It's not based on cortex-m3/m4 cpus. The src/atsam/, atsamd, rp2040, and lpc176x directories also support cortex-m0/m0+/m3/m4 chips.

As a further example, src/atsamd/ is for Atmel samd21 and samd51 chips, while src/atsam/ directory is for Atmel sam3x8e/sam3x8c/sam4s8c/sam4e8e chips. So, choice of directory is not by manufacturer either - it's dependent on code reuse - the hardware blocks on the samd21 are very similar to the samd51 hardware blocks - but nothing similar to the sam3/sam4 hardware blocks.

like I did for the bloated irq stuff

Can you expand on this? It looks like the code doesn't use the src/armcm_irq.c and src/armcm_boot.c code - which all the other Klipper arm cortex-m chips use. Is this due to something unusual in the hc32f460 chip? EDIT: Actually it does use that Klipper code, so if you could explain further it would help.

Thanks,
-Kevin

[SteveGotthardt]

For the irq comment i made about bloat - i mean that I didn't use the HAL irq support from the library since they forced their own systick function which collided with klipper and also it added a lot of code/memory space usage. I could do the same (not use HAL) for the other peripherals. A few of he HAL functions are useful (clock support, BAUD calculations) and I could just add those to appropriate .c files.

I misunderstood the library inclusion - thanks for clearing that up. I can reduce the needed library files significantly.

I have been using my Aquila printer for a couple of months without issue. Klipper works well!

[github-actions]

It looks like this GitHub Pull Request has become inactive. If there are any further updates, you can add a comment here or open a new ticket.

Best regards,
~ Your friendly GitIssueBot

PS: I'm just an automated script, not a human being.

[Sineos]

@SteveGotthardt There seems to be some interest in your work. See https://klipper.discourse.group/t/support-for-hdsc-chips-hc32f460

[SteveGotthardt]

@Sineos - thanks for the info! I joined discourse.