Adding support for 100pin version of H32F460 and Tenlog D3 32bit printer

[ggiraudon]

src/hc32f460 : Added GPIO configurations for 100pin version of H32F460

Added the necessary setting and Kconfig to allow klipper to build for the
100 pin version of the H32F460 microcontroller.
MCU package can be selected in menuconfig.

config : Added base configuration for Tenlog D3 printer with 32bit mainboard

Added an initial configuration for the Tenlog D3 printer.
This configuration is for the 32bit version of their mainboard.
It is the default mainboard shipped with the V2 version of said printer.

Initial work by audiobrian at https://github.com/audiobrian/tld3pro_klipper

Signed-off-by: Guillaume Giraudon [email protected]

[KevinOConnor]

Thanks. @SteveGotthardt - can you review this change?

In addition to Steve's review, I have a couple of comments:

1. The new config file added in this PR should follow the guidelines at https://www.klipper3d.org/Example_Configs.html . Probably best to split the new config into a separate PR so that @JamesH1978 can more easily review it.
2. Please do not add revision information to the copyright blocks in the code - add that information to the commit messages instead (see https://www.klipper3d.org/CONTRIBUTING.html#what-to-expect-in-a-review ).

-Kevin

[SteveGotthardt]

I have reviewed the changes. I approve.

[ggiraudon]

Based on Keving's comments, do you guys want me to remove the printer config file from this PR and create a separate one ?
As to revision information in the copyright block, I'm not sure what you mean but if you could point out the segments you'd like me to fix, I can do it right away.

G

[JamesH1978]

yes please, split it out, and follow the guidelines for the regression tests as outlined in the doc linked.

And you should not update the copyright info in changed code. Declaring a copyright on an existing file when making minor changes to that file is discouraged your additions are covered in the commit

Thanks
James

[KevinOConnor]

Thanks. I have an additional question - is there a reason why a Kconfig option has been added? Could we just always define all 100 pins even if the user is using the 64 pin package? The change just seems to be mainly to enumerations, and I don't see a harm in always defining them.

-Kevin

[ggiraudon]

I was airing on the side of caution on that one. While the changes I added do allow me to run Klipper on boards with the 100pin package, I do not have any boards with 64pin version of the IC to verify the impact those changes could have.
I'd probably want to check the MCUs datasheet to double check.
As such, the kconfig was there to isolate these changes to builds specifically targeted for the 100pin package, leave all previous code and feature for 64pin package untouched.
If someone can test a 100pin binary on a 64pin package to confirm nothing is broken, I see no objection to removing the kconfig option.

-G

[KevinOConnor]

Thanks. I don't think there is an issue with adding enumerations (nor a problem with adding hard_pwm definitions). This doesn't change the hardware accesses at all - it just adds definitions in the klipper host/mcu communication. It is common practice for us to compile for the largest package on stm32 and atsam mcus. We really want to run identical code on as many chips as feasible.

-Kevin

[ggiraudon]

Done. Both 64 and 100 pin definitions are merged and Kconfig option has been removed.

[KevinOConnor]

Thanks, it seems fine, but you should remove the existing DECL_ENUMERATION_RANGE("pin", "PD2", GPIO('D', 2), 1); definition (the new PD0 definition replaces the old one). Also, please don't make the unrelated whitespace change to Kconfig.

-Kevin

[ggiraudon]

I've removed the extra line from Kconfig.
That being said, I disagree with the removal of the PD2 declaration.
PD0 and PD2 are 2 different IO pins on the MCU and different boards may use them independently.

[KevinOConnor]

PD0 and PD2 are 2 different IO pins on the MCU and different boards may use them independently.

Maybe I'm missing something. There is an existing line: DECL_ENUMERATION_RANGE("pin", "PD2", GPIO('D', 2), 1); and this new patch adds: DECL_ENUMERATION_RANGE("pin", "PD0", GPIO('D', 0), 16);. The original line adds a definition for PD2, and the new line adds 16 definitions for PD0, PD1, PD2, PD3, ... PD15. This seems to be a regression as we should not define a mapping for PD2 twice.

-Kevin

[ggiraudon]

Nope, you aren't missing anything. The issue has to do with me not having slept enough :)
Line removed.

[KevinOConnor]

Thanks.

-Kevin