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

Add Nucleo_F746ZG support #25971

Open
wants to merge 25 commits into
base: bugfix-2.1.x
Choose a base branch
from

Conversation

Bob-the-Kuhn
Copy link
Contributor

This PR adds support for the Nucleo_F746ZG development board.

The main value of this PR is it adds SDIO support for the STM32F7xx series.

No changes were needed for SPI access to the SD card.

Just for fun I added pin assignments for the REPRAP_DISCOUNT_FULL_GRAPHIC_SMART_CONTROLLER and similar devices. A custom cable is needed to provide the needed +5V.

Testing hardware:

Environments were added which support bootloader and non-bootloader systems.

The bootloader environment needs some additional work. Right now two variables need to be updated in order to change the build/load address. VECT_TAB_OFFSET is used by a low level init routine to set the VTOR. I couldn't figure out how to use that variable to modify the load/build address so I created a LD file that has the offset hard coded into it. The Marlin defined boards use board_build.offset or board_build.address to accomplish this. Maybe the solution is to create a Marlin STMF32F7xx variant.

The new pins file has a BOARD_PREINIT macro in it. It is used to init the ITCM ram or else The SDIO callbacks don't register correctly.

@thisiskeithb
Copy link
Member

Just for fun I added pin assignments for the REPRAP_DISCOUNT_FULL_GRAPHIC_SMART_CONTROLLER and similar devices. A custom cable is needed to provide the needed +5V.

While I don't expect this board to be popular / actually used for a 3D printer, it'd be good to add an #ifndef NO_CONTROLLER_CUSTOM_WIRING_WARNING check & error that we've added to other pins files that require a custom LCD cable.

@Bob-the-Kuhn
Copy link
Contributor Author

Created a local/Marlin variant. That solved the "unable to change load point" problem.

Added M43 support. M42 and M43 need some more work.

@Bob-the-Kuhn
Copy link
Contributor Author

Looks like M43 is working correctly but M42 will assign the same PWM channel to multiple pins. The result is multiple pins are locked together. Changing one pin's PWM setting results in the other pin(s) behaving the same.

… the new variant more generic

1) added a routine that tracks the PWM channel usage and won't allow two pins to be assigned to the same PWM.

2) tried to make the STM32F746 variant more generic
@Bob-the-Kuhn
Copy link
Contributor Author

Added a routine that keeps track of the PWM assignments and doesn't allow one PWM channel to be assigned to two different physical pins.

I'm not sure that's the best long term approach.

Probably should add a debug print for the case when an attempt to assign one PWM channel to multiple pins is prevented.

Right now it's only enabled for the STM32F7xx CPU. Need to consider if it should be dropped, more CPUs enabled, ...


Still need to back out my config file changes. So far I'm 0 for 2 on fixing it.

@Bob-the-Kuhn
Copy link
Contributor Author

Finally was able to back out the inadvertent config file changes.

Added a wrapper to PWM_PINS to keep it from getting compiled for non-target boards. This cleared up a Klemtest issue.

1) consolidate #if logic in fastio.h

2) allow use of PWM_PIN with G0 and H7 parts.

3) add #if statements to PWM_PIN so that it could be used with smaller CPUs in the future.
@Bob-the-Kuhn
Copy link
Contributor Author

Just did a little cleanup and minor changes:

  1. consolidate #if logic in fastio.h
  2. allow use of PWM_PIN with G0 and H7 parts.
  3. add #if statements to PWM_PIN so that it could be used with smaller CPUs in the future.

I believe this PR is ready for review.

Still not sure if the PWM_PIN change should remain. It's nice to have when setting up a new board but of no use to the average user.

@thinkyhead
Copy link
Member

Still not sure if the PWM_PIN change should remain. It's nice to have when setting up a new board but of no use to the average user.

We can retain it as a switchable option, as it seems like it could be useful for future development.

abandon looking at variant specific implementations & switch to scanning analogInputPin array
@Bob-the-Kuhn
Copy link
Contributor Author

Changed the approach used in the digital_pin_to_analog_ch function.

Looks like there are two methods used to map analog inputs to digital pin names used in the STM32 variants.

  • defines NUM_ANALOG_FIRST & NUM_ANALOG_INPUTS along with the array digitalPin[] map a contiguous range of pins to analog inputs.
  • define NUM_ANALOG_INPUTS along with the arrays analogInputPin[] and digitalPin[] map multiple ranges of pins toanalog inputs.

Rather than trying to mimic variant specific implementations, switched to scanning the analogInputPin array. The scanning method should be applicable to most of the variants.

@Bob-the-Kuhn
Copy link
Contributor Author

Still trouble shooting a bug on the STM32F746 code.

The STM32F746 code is only allowing a subset of the available analog inputs to be mapped/used. I was expecting to be able to assign any of the analog inputs as the TEMP_0_PIN. Right now I can assign any input from the subset as the TEMP_0_PIN.

Analog inputs should have been specified as 0-23 rather than pin names.

I also backed out the analogInputToDigitalPin function changes in HAL.h.  This function is provided by the platform so the old style will do just fine.
@Bob-the-Kuhn
Copy link
Contributor Author

I think this PR is ready to go.

The "analog input mapping" problem was cause by me using pin names during testing rather than analog channel numbers. The pins_NUCLEO_F746ZG.h file was updated to show explicitly what are the acceptable analog input designations.

I also found out that the analogInputToDigitalPin changes in HAL.h were not needed. The platform supplies this function for the STM32 based boards.

@thinkyhead
Copy link
Member

thinkyhead commented Jul 7, 2023

Is there something that you wanted to add in relation to the new environment for STM32H753ZI? It might be better to hold the changes adding that environment until there's a board to go with it.

@Bob-the-Kuhn
Copy link
Contributor Author

I'm OK putting this PR on hold until a STM32H753ZI 3D printer controller is available,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants