Feature stm32g4 can support #13565

m-ecry · 2020-09-07T15:31:22Z

Summary of changes

Added CAN support for the STM32G4.
Added support of FDCAN3 interface for the STM32 family.
Added fdcan_core_clk calculation instead of basing the calculation on a fixed 10MHz.

Impact of changes

Migration actions required

Documentation

None.

Pull request type

[x] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

I've tested it with the NUCLEO-G474RE board & FDCAN-Adapter, don't know exactly how to do this section.

[] No Tests required for this change (E.g docs only update)
[] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers

@jeromecoutant

- The FDCAN_clk is calculated on runtime from the according RCC-registers

ciarmcom · 2020-09-07T16:00:30Z

@m-ecry, thank you for your changes.
@jeromecoutant @ARMmbed/mbed-os-maintainers please review.

0xc0170 · 2020-09-09T10:49:25Z

This shall be a patch update, I updated the type and the label.

Thanks for the contribution.

mbed-ci · 2020-09-09T11:29:40Z

Jenkins CI Test : ❌ FAILED

Build Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs	Status
jenkins-ci/mbed-os-ci_unittests	✔️
jenkins-ci/mbed-os-ci_build-ARM	❌
jenkins-ci/mbed-os-ci_build-GCC_ARM	❌

jeromecoutant · 2020-09-09T12:30:58Z

targets/TARGET_STM/TARGET_STM32G4/TARGET_STM32G474xx/TARGET_NUCLEO_G474RE/system_clock.c

    RCC_OscInitStruct.PLL.PLLP = RCC_PLLP_DIV2;
-    RCC_OscInitStruct.PLL.PLLQ = RCC_PLLQ_DIV2;
+    RCC_OscInitStruct.PLL.PLLQ = RCC_PLLQ_DIV4;


So you decrease SystemCoreClock from 170 MHz to 160,
and you get PLLQ frequency at 80 MHz

Could you explain me again the reason ?
Thx

With a core frequency of 170MHz, many CAN-frequencies in my tests could not be setup correctly.

For example I worked at a frequency of 250k. With a core frequency of 170MHz this gave me:

Field Calculated Value Register Value

ntq1 510 (0x1FE) 253 (0xfd)

ntq2 169 (0xA9) 40 (0x28)

sjw 169 (0xA9) 40 (0x28)

So the register could not store the value, the interface could not be setup properly.

The current limitation is the result of the api not using the prescaler (always at 1).

I propose to use the prescaler, calculated this way:

int ntq = HAL_RCCEx_GetPeriphCLKFreq(RCC_PERIPHCLK_FDCAN) / hz; int nominalPrescaler = 1; // !When the sample point should be lower than 50%, this must be changed to // !IS_FDCAN_NOMINAL_TSEG2(ntq/nominalPrescaler), since // NTSEG2 and SJW max values are lower. For now the sample point is fix @75% while (!IS_FDCAN_NOMINAL_TSEG1(ntq/nominalPrescaler)){ nominalPrescaler ++; if (!IS_FDCAN_NOMINAL_PRESCALER(nominalPrescaler)){ error("Could not determine good nominalPrescaler. Bad clock value\n"); } } ntq = ntq/nominalPrescaler;

See commit.

Still there can't be guaranteed, that the resulting timing is within tolerance, so this will still require manual adaptation when using a strange value.

Even with that a frequency of 170MHz caused many setups to fail, so I reduced it to 80MHz, which worked just fine for most (not all) of my test-configurations. Unfortunately I not record my tests properly, I can repeat and record them to show it better, if needed.

Thx for details.
Maybe we could code like this:

#if DEVICE_CAN PLLN = 80 #else PLLN = 85

If you disable CAN, you keep the maximum capability ?

That fine by me, I will add this with a comment.

Added in 2a13fa1, added comment in 73493b9.

jeromecoutant · 2020-09-09T12:52:33Z

targets/TARGET_STM/can_api.c

@@ -118,7 +179,8 @@ static void _can_init_freq_direct(can_t *obj, const can_pinmap_t *pinmap, int hz
    Phase_segment_2            | 30 tq          | <nts2> = <n_tq> - 1 - <nts1>
    Synchronization_Jump_width | 30 tq          | <nsjw> = <nts2>
    */
-    int ntq = 10000000 / hz;
+
+    int ntq = _can_get_core_frequency() / hz;


Maybe you could try HAL_RCCEx_GetPeriphCLKFreq with RCC_PERIPHCLK_FDCAN ?

Mmm it seems that STM32H7 doesn't provide the same function... :-(

Note you can try code like this:

#ifdef TARGET_STM32G4 int ntq = HAL_RCCEx_GetPeriphCLKFreq(RCC_PERIPHCLK_FDCAN)/ hz; #else // STM32H7 doesn't support yet HAL_RCCEx_GetPeriphCLKFreq for FDCAN // Internal ST ticket 92465 int ntq = 10000000 / hz; #endif

This is a really nice catch, thank you! I added your proposal in. (35c9e7a)

jeromecoutant · 2020-09-10T13:28:01Z

targets/TARGET_STM/can_api.c

@@ -491,6 +573,17 @@ void FDCAN2_IT1_IRQHandler(void)
    can_irq(CAN_2, 1);
 }

+void FDCAN3_IT0_IRQHandler(void)


#if defined(FDCAN3_BASE) is missing

I didn't add this line, since it was also missing for the FDCAN2 Handlers, which otherwise are also guarded with the #if defined(GDCAN2_BASE).

But I agree, it makes sense to add the guard for both the FDCAN2 and FDCAN3 Handlers.

Added guards in 35c9e7a

m-ecry · 2020-09-11T10:02:54Z

Thanks for your comments, I will look into them and improve the sections until monday!

AGlass0fMilk · 2020-09-11T13:44:04Z

Thanks for this! I will be testing this out in the near future :-)

@jeromecoutant

- Thanks to @jeromecoutant for showing the HAL funtion - Added #ifdef guard to FDCAN2/3 handler functions

- This enables more frequencies, but without regard to the accuracy. May still require manual clock setup, to remain in tolerance window

- Previously a received msg was fixed of data_type

- with 170MHz as can-core-frequency, the accuracy for many baudrates is too low. 160MHz is better for a broad range of frequencies

m-ecry · 2020-09-14T16:18:16Z

Added a small fix to support remote-messages (d0c8ad7)

jeromecoutant

Very good! I could perform some CAN tests:

between NUCLEO_H743ZI2 (FDCAN) and NUCLEO_L476RG (CAN) in master branch
between NUCLEO_H743ZI2 (FDCAN) and NUCLEO_L476RG (CAN) with this pull request (and a small patch, see my comment)
between NUCLEO_H743ZI2 (FDCAN) and NUCLEO_G474RE (FDCAN) with this pull request

jeromecoutant · 2020-09-16T11:16:11Z

targets/TARGET_STM/can_api.c

+#else
+    // STM32H7 doesn't support yet HAL_RCCEx_GetPeriphCLKFreq for FDCAN
+    // Internal ST ticket 92465
+    int ntq = 10000000 / hz;


Not hz in this function but f...

Sorry for that slip-up, I have no H7 to test, but should have noticed it anyway. Thanks for testing!

Changed in 73493b9

Pull request has been modified.

- can_frequency uses f instead of hz for can frequency - Also added comment to system_clock

mbed-ci · 2020-09-16T16:23:57Z

Jenkins CI Test : ✔️ SUCCESS

Build Number: 2 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs	Status
jenkins-ci/mbed-os-ci_unittests	✔️
jenkins-ci/mbed-os-ci_build-ARM	✔️
jenkins-ci/mbed-os-ci_build-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_greentea-test	✔️
jenkins-ci/mbed-os-ci_dynamic-memory-usage	✔️
jenkins-ci/mbed-os-ci_cloud-client-pytest	✔️

0xc0170 · 2020-09-17T07:33:43Z

CI restarted (to get the status again)

mbed-ci · 2020-09-17T08:41:40Z

Jenkins CI Test : ✔️ SUCCESS

Build Number: 3 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs	Status
jenkins-ci/mbed-os-ci_unittests	✔️
jenkins-ci/mbed-os-ci_build-ARM	✔️
jenkins-ci/mbed-os-ci_build-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_greentea-test	✔️
jenkins-ci/mbed-os-ci_dynamic-memory-usage	✔️
jenkins-ci/mbed-os-ci_cloud-client-pytest	✔️

m-ecry added 4 commits September 7, 2020 02:04

make G4 target compileable with CAN support

9bc2deb

Added support for FDCAN3

9886532

Changed PLL to 160MHz, PLLQ to 80MHz

f32efe4

Calculate FDCAN_clk instead of assuming fix 10MHz

08ce2f2

- The FDCAN_clk is calculated on runtime from the according RCC-registers

ciarmcom added the release-type: feature label Sep 7, 2020

ciarmcom requested review from jeromecoutant and a team September 7, 2020 16:00

ciarmcom added the needs: review label Sep 7, 2020

0xc0170 added release-type: patch Indentifies a PR as containing just a patch and removed release-type: feature labels Sep 9, 2020

mergify bot added needs: work and removed needs: review labels Sep 9, 2020

jeromecoutant reviewed Sep 9, 2020

View reviewed changes

jeromecoutant reviewed Sep 10, 2020

View reviewed changes

deveritec-marec and others added 4 commits September 14, 2020 15:24

Use HAL function for FDCAN_CLK-calculation

35c9e7a

- Thanks to @jeromecoutant for showing the HAL funtion - Added #ifdef guard to FDCAN2/3 handler functions

STM-can-api: Added usage of prescaler

13b6633

- This enables more frequencies, but without regard to the accuracy. May still require manual clock setup, to remain in tolerance window

STM-can-api: Support reading of remote_msg

d0c8ad7

- Previously a received msg was fixed of data_type

STMG4-sys-clk: If can PLLQ=160MHz, else 170MHz

2a13fa1

- with 170MHz as can-core-frequency, the accuracy for many baudrates is too low. 160MHz is better for a broad range of frequencies

0xc0170 previously approved these changes Sep 15, 2020

View reviewed changes

0xc0170 added needs: review and removed needs: work labels Sep 15, 2020

mergify bot added needs: CI and removed needs: review labels Sep 15, 2020

jeromecoutant suggested changes Sep 16, 2020

View reviewed changes

m-ecry force-pushed the feature-stm32g4-can-support branch from b280255 to d4e22e2 Compare September 16, 2020 15:28

STM-can-api: Fixed variable name for H7

73493b9

- can_frequency uses f instead of hz for can frequency - Also added comment to system_clock

m-ecry force-pushed the feature-stm32g4-can-support branch from d4e22e2 to 73493b9 Compare September 16, 2020 15:35

jeromecoutant approved these changes Sep 16, 2020

View reviewed changes

0xc0170 approved these changes Sep 17, 2020

View reviewed changes

0xc0170 added ready for merge and removed needs: CI labels Sep 17, 2020

0xc0170 merged commit 75544a7 into ARMmbed:master Sep 17, 2020

mergify bot removed the ready for merge label Sep 17, 2020

mbedmain added release-version: 6.4.0 Release-pending and removed release-type: patch Indentifies a PR as containing just a patch Release-pending labels Oct 20, 2020

jeromecoutant mentioned this pull request Jun 28, 2021

CAN: Remote frame is not working for H743ZI2 #12788

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature stm32g4 can support #13565

Feature stm32g4 can support #13565

m-ecry commented Sep 7, 2020 •

edited by 0xc0170

Loading

ciarmcom commented Sep 7, 2020

0xc0170 commented Sep 9, 2020

mbed-ci commented Sep 9, 2020

jeromecoutant Sep 9, 2020

m-ecry Sep 14, 2020

jeromecoutant Sep 14, 2020

m-ecry Sep 14, 2020

m-ecry Sep 16, 2020

jeromecoutant Sep 9, 2020 •

edited

Loading

jeromecoutant Sep 9, 2020

jeromecoutant Sep 11, 2020

m-ecry Sep 14, 2020 •

edited

Loading

jeromecoutant Sep 10, 2020

m-ecry Sep 14, 2020

m-ecry Sep 16, 2020

m-ecry commented Sep 11, 2020

AGlass0fMilk commented Sep 11, 2020

m-ecry commented Sep 14, 2020

jeromecoutant left a comment

jeromecoutant Sep 16, 2020

m-ecry Sep 16, 2020

m-ecry Sep 16, 2020

mbed-ci commented Sep 16, 2020

0xc0170 commented Sep 17, 2020

mbed-ci commented Sep 17, 2020

Field	Calculated Value	Register Value
ntq1	510 (0x1FE)	253 (0xfd)
ntq2	169 (0xA9)	40 (0x28)
sjw	169 (0xA9)	40 (0x28)

Feature stm32g4 can support #13565

Feature stm32g4 can support #13565

Conversation

m-ecry commented Sep 7, 2020 • edited by 0xc0170 Loading

Summary of changes

Impact of changes

Migration actions required

Documentation

Pull request type

Test results

Reviewers

ciarmcom commented Sep 7, 2020

0xc0170 commented Sep 9, 2020

mbed-ci commented Sep 9, 2020

Jenkins CI Test : ❌ FAILED

Build Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeromecoutant Sep 9, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

m-ecry Sep 14, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

m-ecry commented Sep 11, 2020

AGlass0fMilk commented Sep 11, 2020

m-ecry commented Sep 14, 2020

jeromecoutant left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbed-ci commented Sep 16, 2020

Jenkins CI Test : ✔️ SUCCESS

Build Number: 2 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

0xc0170 commented Sep 17, 2020

mbed-ci commented Sep 17, 2020

Jenkins CI Test : ✔️ SUCCESS

Build Number: 3 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

m-ecry commented Sep 7, 2020 •

edited by 0xc0170

Loading

jeromecoutant Sep 9, 2020 •

edited

Loading

m-ecry Sep 14, 2020 •

edited

Loading