2.0 add disable jtag for stm32

[AlexanderAmelkin]

Description

Add support for DISABLE_JTAG option for STM32Fx.

Benefits

Allow for using multiplexed JTAG pins on STM32Fx boards as IO.

Related Issues

#10898

[p3p]

This should probably be in the platforms initialisation, we want to avoid platform specific code outside the HAL unless absolutely necessary. (I realise the irony in saying this considering the code you modified 😉)

[AlexanderAmelkin]

Probably you're right, but until this is moved to where it truly belongs, I suggest merging it where it is now to allow for further works on some STM32-based boards, and maybe creating a separate issue to capture the intention to move this code to a better place.

[ejtagle]

@AlexanderAmelkin : This should be moved to HAL_init() . That function is per HAL

[p3p]

Something like this would probably be appropriate.

------------------------- Marlin/src/HAL/HAL_AVR/HAL.h -------------------------
index 9093910..822f29d 100644
@@ -353,4 +353,9 @@ inline void HAL_adc_init(void) {
 
 #define HAL_SENSITIVE_PINS 0, 1
 
+#ifdef __AVR_AT90USB1286__
+  #define JTAG_DISABLE() MCUCR = 0x80; MCUCR = 0x80
+#endif
+
+
 #endif // _HAL_AVR_H_

----------------------- Marlin/src/HAL/HAL_STM32F1/HAL.h -----------------------
index f538e27..69b0f97 100644
@@ -248,4 +248,6 @@ void HAL_enable_AdcFreerun(void);
 #define GET_PIN_MAP_INDEX(pin) pin
 #define PARSED_PIN_INDEX(code, dval) parser.intval(code, dval)
 
+#define JTAG_DISABLE() afio_cfg_debug_ports(AFIO_DEBUG_NONE)
+
 #endif // _HAL_STM32F1_H

----------------------- Marlin/src/HAL/HAL_STM32F4/HAL.h -----------------------
index 4eca976..5267788 100644
@@ -252,4 +252,6 @@ void HAL_enable_AdcFreerun(void);
 #define GET_PIN_MAP_INDEX(pin) pin
 #define PARSED_PIN_INDEX(code, dval) parser.intval(code, dval)
 
+#define JTAG_DISABLE() afio_cfg_debug_ports(AFIO_DEBUG_NONE)
+
 #endif // _HAL_STM32F4_H

---------------------------- Marlin/src/Marlin.cpp ----------------------------
index 694c9e0..20da216 100644
@@ -675,13 +675,10 @@ void setup() {
     Max7219_init();
   #endif
 
+  // Disable JTAG to free up pins for IO
   #if ENABLED(DISABLE_JTAG)
-    // Disable JTAG to free up pins for IO
-    #ifndef __AVR_AT90USB1286__
-      MCUCR = 0x80;
-      MCUCR = 0x80;
-    #elif defined(__STM32F1__) || defined(__STM32F4__)
-      afio_cfg_debug_ports(AFIO_DEBUG_NONE);
+    #ifdef JTAG_DISABLE
+      JTAG_DISABLE();
     #else
       #error "DISABLE_JTAG is not supported for the selected MCU/Board"
     #endif

[ejtagle]

@p3p : Thats a different and valid approximation. No particular preference

[xC0000005]

I would put this in a board's variant init function rather than the HAL for a given processor type. Are you repurposing a generic variant or creating one for the boards in question? If you have created one, that's the place to do board specific stuff like this.

[AlexanderAmelkin]

@p3p, I will refix as you suggest. So far I have pushed a fix of typo in my original commit.

[thinkyhead]

If this is ready, I don't mind merging it anytime.

[AlexanderAmelkin]

@thinkyhead, sorry, I didn't have time yet to work on this.

[p3p]

@thinkyhead My diff above can be used as is if you want to keep the JTAG config in Marlin Core and not move it into HAL_init as ejtagle suggested

[AlexanderAmelkin]

Thanks for merging, but I found that my patch (and @p3p's patch to it too) are too broad for STM32. Those MCUs have an ability to disable just JTAG or both JTAG and SWD. The merged version disables both, making it impossible to debug and re-flash via SWD. When I have time I'll probably submit a patch to disable JTAG only and leave SWD functional.

[thinkyhead]

Looking forward to it. Let’s keep this moving.

[AlexanderAmelkin]

Looking at commit 84926b1, it seems like something went wrong with the merge. I don't remember my commit message being this long a one-liner... :) I guess it's too late to fix it though, isn't it?

[thinkyhead]

Looks like the commit message has a short summary followed by a blank line and a longer description, which matches the description attached to the PR. That's all quite normal.

[AlexanderAmelkin]

@thinkyhead, I believe the original commit followed the 50/70 rule, while the version that got into bugfix-2.0.x has the description as one long line. At least this is how it looks in console:

But if that's fine with you, then it's fine with me too.

[AlexanderAmelkin]

I now see that it's common for this project. Well then. I was just used to Linux kernel rules...

[thinkyhead]

Some git clients will hard-wrap long descriptions, but when using the git command in the terminal and a configured EDITOR like joe or nano, long lines are not hard-wrapped.