STM32L5 : add DISCO-L562QE board support #12338

jeromecoutant · 2020-01-30T15:20:04Z

Summary of changes

New board addition : DISCO_L562QE

https://www.st.com/en/evaluation-tools/stm32l562e-dk.html

QSPI support OK
STMOD_cellular support OK

NB: Trust Zone is not enabled yet in this version

@ARMmbed/team-st-mcd
@MarceloSalazar
@PLFSTM

Impact of changes

Migration actions required

Documentation

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

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

Reviewers

ciarmcom · 2020-01-30T16:00:41Z

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

bulislaw

Couple of questions.

bulislaw · 2020-02-03T12:03:44Z

targets/TARGET_STM/mbed_overrides.c

@@ -150,5 +150,9 @@ void mbed_sdk_init()
 #endif /* ! MBED_CONF_TARGET_LSE_AVAILABLE */
 #endif /* DEVICE_RTC */

+#if defined (MBED_CONF_TARGET_STMOD_SEL) // DISCO_L562QE


That worries me a bit and it's a bit confusing. stmod_select is only defined for this one DISCO board (not even family or SoC). Here we are conditionally compiling the call to it based on MBED_CONF_TARGET_STMOD_SEL and the function is defined as:

void stmod_select(void) { DigitalOut SEL_12(PF_11); DigitalOut SEL_34(PF_12); #if (MBED_CONF_TARGET_STMOD_SEL) // UART selection SEL_12 = 1; SEL_34 = 1; #else // SPI selection SEL_12 = 0; SEL_34 = 0; #endif }

but it's not called anywhere else. I guess it's acceptable, maybe even desirable, for the build to fail if user defines the macro, but the board doesn't provide the function. But maybe it's worth simplifying it a bit? The stmod_select will only be called when the macro is defined, maybe the function it doesn't need to have the conditional block? Just a comment saying it's only called when the macro is defined and it will force the UART to be selected?

If possible, I'd suggest tweaking the configuration here to pick up the pin names from the STMOD's mbed_lib.json

@bulislaw
Agree. Will propose a new solution.

bulislaw · 2020-02-03T12:03:48Z

targets/TARGET_STM/qspi_api.c

@@ -35,6 +25,14 @@
 #include "pinmap.h"
 #include "PeripheralPins.h"

+#include "mbed_trace.h"


Unifying the "tracing/debug/printf" facilities was an item on my TODO list. Do you know what's the cost of enabling it for builds that didn't use it before? For various build profiles?

Yes, unifying the trace facilities was my goal for mbed_trace use.
I didn't check the cost between now (mbed_trace use) and before (mbed_debug use).

bulislaw · 2020-02-03T12:08:54Z

targets/TARGET_STM/qspi_api.c


    // Reset handle internal state
    obj->handle.State = HAL_OSPI_STATE_RESET;

    // Set default OCTOSPI handle values
    obj->handle.Init.DualQuad = HAL_OSPI_DUALQUAD_DISABLE;
-    obj->handle.Init.MemoryType = HAL_OSPI_MEMTYPE_MICRON;
+    obj->handle.Init.MemoryType = HAL_OSPI_MEMTYPE_MACRONIX;


That's a generic driver, will that work ok across?

You're right, I changed this only while code reviewing.
I am checking with experts the real impact of this.

MarceloSalazar

It would be good to reuse the STMOD config from mbed_lib.json

0xc0170 · 2020-02-05T14:58:39Z

targets/TARGET_STM/TARGET_STM32L5/TARGET_STM32L562xx/TARGET_DISCO_L562QE/PinNames.h

+/* mbed Microcontroller Library
+ *******************************************************************************
+ * Copyright (c) 2018, STMicroelectronics
+ * All rights reserved.


lets add SPDX identifiers to new files

0xc0170 · 2020-02-05T14:59:56Z

targets/targets.json

+                "value": 1
+            }
+        },
+        "overrides": { "lpticker_delay_ticks": 0 },


please fix formatting as the rest of this json (new line should be here?)

Pull request has been modified.

jeromecoutant · 2020-02-10T09:26:58Z

@bulislaw @0xc0170
could you check updates ?
Thx

0xc0170 · 2020-02-11T14:57:39Z

Can you attach test logs for this new target?

jeromecoutant · 2020-02-11T15:26:50Z

Can you attach test logs for this new target?

Yes
report__Check_ARM_DISCO_L562QE__2020_02_10_09_58.txt
report__Check_GCC_DISCO_L562QE__2020_02_10_10_58.txt
report__Check_IAR_DISCO_L562QE__2020_02_10_11_57.txt

0xc0170 · 2020-02-11T15:40:29Z

CI started

mbed-ci · 2020-02-11T16:24:25Z

Test run: FAILED

Summary: 1 of 4 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-IAR

0xc0170 · 2020-02-13T12:33:28Z

CI restarted

mbed-ci · 2020-02-13T14:32:18Z

Test run: FAILED

Summary: 3 of 11 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_mbed2-build-GCC_ARM
jenkins-ci/mbed-os-ci_mbed2-build-ARM
jenkins-ci/mbed-os-ci_mbed2-build-IAR

0xc0170 · 2020-02-13T14:43:07Z

@jeromecoutant there are errors in mbed2 building:

[DEBUG] Output: /builds/ws/mbed-os-ci_mbed2-build-ARM@2/mbed-os/targets/TARGET_STM/qspi_api.c:28:10: fatal error: 'mbed_trace.h' file not found
[DEBUG] Output: #include "mbed_trace.h"

Although there is no mbed 2 release and I'll question this CI stage, what shall we do?

Pull request has been modified.

jeromecoutant · 2020-02-14T16:53:12Z

Hi
QSPI debug feature will be part of an other PR
CI restarted

0xc0170 · 2020-02-17T09:35:27Z

CI restarted

mbed-ci · 2020-02-17T10:55:52Z

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 3
Build artifacts

jeromecoutant · 2020-02-17T12:03:24Z

QSPI debug feature will be part of an other PR

See #12451

ciarmcom requested a review from a team January 30, 2020 16:00

ciarmcom added the needs: review label Jan 30, 2020

bulislaw reviewed Feb 3, 2020

View reviewed changes

MarceloSalazar reviewed Feb 3, 2020

View reviewed changes

0xc0170 added the scope: new-target label Feb 5, 2020

0xc0170 previously requested changes Feb 5, 2020

View reviewed changes

mergify bot added needs: work and removed needs: review labels Feb 5, 2020

0xc0170 added the release-version: 6.0.0-alpha-2 Second pre-release version of 6.0.0 label Feb 7, 2020

jeromecoutant force-pushed the PR_STM32L5_DISCO branch from c8f288f to f587cf6 Compare February 10, 2020 09:24

0xc0170 added needs: review and removed needs: work labels Feb 11, 2020

0xc0170 previously approved these changes Feb 11, 2020

View reviewed changes

mergify bot added needs: CI and removed needs: review labels Feb 11, 2020

mergify bot added needs: work and removed needs: CI labels Feb 11, 2020

0xc0170 added needs: CI and removed needs: work labels Feb 13, 2020

mergify bot added ready for merge and removed needs: CI labels Feb 13, 2020

0xc0170 added needs: work and removed ready for merge labels Feb 13, 2020

jeromecoutant added 3 commits February 14, 2020 17:49

STM32L5 : add QSPI support

f096902

STM32L5 : Add DISCO-L562E support

d66b39d

STMOD_CELLULAR: add DISCO_L562_QE support

c9aead4

jeromecoutant force-pushed the PR_STM32L5_DISCO branch from f587cf6 to c9aead4 Compare February 14, 2020 16:52

0xc0170 added needs: CI and removed needs: work labels Feb 17, 2020

0xc0170 approved these changes Feb 17, 2020

View reviewed changes

0xc0170 removed the needs: CI label Feb 17, 2020

0xc0170 merged commit c12b433 into ARMmbed:master Feb 17, 2020

jeromecoutant deleted the PR_STM32L5_DISCO branch February 17, 2020 11:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

STM32L5 : add DISCO-L562QE board support #12338

STM32L5 : add DISCO-L562QE board support #12338

jeromecoutant commented Jan 30, 2020

ciarmcom commented Jan 30, 2020

bulislaw left a comment

bulislaw Feb 3, 2020

MarceloSalazar Feb 3, 2020

jeromecoutant Feb 7, 2020 •

edited

Loading

bulislaw Feb 3, 2020

jeromecoutant Feb 7, 2020

bulislaw Feb 3, 2020

jeromecoutant Feb 7, 2020

MarceloSalazar left a comment

0xc0170 Feb 5, 2020

jeromecoutant Feb 7, 2020

0xc0170 Feb 5, 2020

jeromecoutant Feb 7, 2020

jeromecoutant commented Feb 10, 2020

0xc0170 commented Feb 11, 2020

jeromecoutant commented Feb 11, 2020

0xc0170 commented Feb 11, 2020

mbed-ci commented Feb 11, 2020

0xc0170 commented Feb 13, 2020

mbed-ci commented Feb 13, 2020

0xc0170 commented Feb 13, 2020

jeromecoutant commented Feb 14, 2020

0xc0170 commented Feb 17, 2020

mbed-ci commented Feb 17, 2020

jeromecoutant commented Feb 17, 2020

STM32L5 : add DISCO-L562QE board support #12338

STM32L5 : add DISCO-L562QE board support #12338

Conversation

jeromecoutant commented Jan 30, 2020

Summary of changes

Impact of changes

Migration actions required

Documentation

Pull request type

Test results

Reviewers

ciarmcom commented Jan 30, 2020

bulislaw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeromecoutant Feb 7, 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

Choose a reason for hiding this comment

MarceloSalazar 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

Choose a reason for hiding this comment

jeromecoutant commented Feb 10, 2020

0xc0170 commented Feb 11, 2020

jeromecoutant commented Feb 11, 2020

0xc0170 commented Feb 11, 2020

mbed-ci commented Feb 11, 2020

0xc0170 commented Feb 13, 2020

mbed-ci commented Feb 13, 2020

0xc0170 commented Feb 13, 2020

jeromecoutant commented Feb 14, 2020

0xc0170 commented Feb 17, 2020

mbed-ci commented Feb 17, 2020

jeromecoutant commented Feb 17, 2020

jeromecoutant Feb 7, 2020 •

edited

Loading