boards/nucleo-l452: initial support #8441

fjmolinas · 2018-01-25T15:38:56Z

Contribution description

Add initial support for nucleo-l452, tested on nucleo-l452re.

Issues/PRs references

aabadie

Now that #8065 is merged this PR needs to be adapted. See how this is done with other nucleo boards.

I didn't go into the vectors.c details, I'll do that later.

aabadie · 2018-01-25T22:44:50Z

boards/nucleo-l452/board.c

@@ -0,0 +1,34 @@
+/*


with #8065, this is file is no longer needed

aabadie · 2018-01-25T22:47:04Z

cpu/stm32l4/vectors.c

@@ -172,32 +170,37 @@ ISR_VECTOR(1) const isr_t vector_cpu[CPU_IRQ_NUMOF] = {
    [TIM1_TRG_COM_IRQn       ] = isr_tim1_trg_com,         /* [26] TIM1 Trigger and Commutation Interrupt */
    [USB_IRQn                ] = isr_usb,                  /* [67] USB event Interrupt */
    [CRS_IRQn                ] = isr_crs,                  /* [82] CRS global interrupt */
-#elif defined(CPU_MODEL_STM32L476RG) || defined(CPU_MODEL_STM32L475VG)
+#elif defined(CPU_MODEL_STM32L476RG) || defined(CPU_MODEL_STM32L475VG) || defined(CPU_MODEL_STM32L452RE)


This line could be split using \

aabadie · 2018-01-25T22:47:12Z

cpu/stm32l4/vectors.c

    [DFSDM1_FLT2_IRQn        ] = isr_dfsdm1_flt2,          /* [63] DFSDM1 Filter 2 global Interrupt */
    [OTG_FS_IRQn             ] = isr_otg_fs,               /* [67] USB OTG FS global Interrupt */
    [SAI2_IRQn               ] = isr_sai2,                 /* [75] Serial Audio Interface 2 global interrupt */
 #endif
+#if defined(CPU_MODEL_STM32L432KC) || defined(CPU_MODEL_STM32L476RG) || defined(CPU_MODEL_STM32L475VG)


This line could be split using \

fjmolinas · 2018-01-26T15:08:11Z

@aabadie addressed comments.

aabadie

Found another style issue in vectors.c. I need to have a look at the datasheet, just to be sure.

aabadie · 2018-01-26T20:17:04Z

cpu/stm32l4/vectors.c

    [DFSDM1_FLT2_IRQn        ] = isr_dfsdm1_flt2,          /* [63] DFSDM1 Filter 2 global Interrupt */
    [OTG_FS_IRQn             ] = isr_otg_fs,               /* [67] USB OTG FS global Interrupt */
    [SAI2_IRQn               ] = isr_sai2,                 /* [75] Serial Audio Interface 2 global interrupt */
 #endif
+#if defined(CPU_MODEL_STM32L432KC) || defined(CPU_MODEL_STM32L476RG) || \
+    defined(CPU_MODEL_STM32L475VG)
+[TIM7_IRQn               ] = isr_tim7,                 /* [55] TIM7 global interrupt */


The indendation is off here

fjmolinas · 2018-01-26T21:00:49Z

@aabadie about those vector files, I only added the ones I used in the functions implemented, what are the other stuff I should look for? I also excluded the ones that are not used for this specific board. Should I generalize for more cpu familiy's?

fjmolinas · 2018-02-07T21:31:33Z

@aabadie ping

aabadie

I think it would also be great to configure the RTT for this board. See my comment below.

aabadie · 2018-02-08T07:53:08Z

boards/nucleo-l452/Makefile.features

+# Put defined MCU peripherals here (in alphabetical order)
+FEATURES_PROVIDED += periph_gpio
+FEATURES_PROVIDED += periph_pwm
+FEATURES_PROVIDED += periph_rtc


I think you could also add a RTT on this board, see the nucleo-l476 as an example.

I don't think RTT works for L4, at least running tests/periph_rtt doesn't work on L476, doesn't work in master an doesn't work at least up to release 2017.07. I'm trying to look in to this, but it got broken along the way another stm32lx broken feature.... Considering this I would rather not implement this right now, merge the board and then add RTT in the future, what do you think @aabadie ?

aabadie

Just gave it a try with b-l475e-iot01a board:

examples/default: works
tests/periph_timer: crashes

I think the changes in vectors.c are invalid. See my comment below.

aabadie · 2018-02-15T10:17:37Z

cpu/stm32l4/vectors.c

+    [UART4_IRQn              ] = isr_uart4,                /* [52] UART4 global Interrupt */
+    [DFSDM1_FLT0_IRQn        ] = isr_dfsdm1_flt0,          /* [61] DFSDM1 Filter 0 global Interrupt */
+    [DFSDM1_FLT1_IRQn        ] = isr_dfsdm1_flt1,          /* [62] DFSDM1 Filter 1 global Interrupt */
+#elif defined(CPU_MODEL_STM32L476RG) || defined(CPU_MODEL_STM32L475VG)


Not sure this elif is valid. It breaks the tests/periph_timer application with the b-l475e-iot01a board (uses a STM32L475VG).

@aabadie addressed issue, you are right the elif wasn't valid, changed all to just ifdef

aabadie · 2018-02-15T15:01:16Z

cpu/stm32l4/vectors.c

-#elif defined(CPU_MODEL_STM32L476RG) || defined(CPU_MODEL_STM32L475VG)
+#endif
+#if defined(CPU_MODEL_STM32L476RG) || defined(CPU_MODEL_STM32L475VG) || \
+      defined(CPU_MODEL_STM32L452RE)


please align to the defined on previous line (like you did below)

@fjmolinas, can you address my comment here (that is also valid for line 176 below) ? After that, I think it can be merged

aabadie · 2018-02-15T15:08:13Z

cpu/stm32l4/vectors.c

-#elif defined(CPU_MODEL_STM32L476RG) || defined(CPU_MODEL_STM32L475VG)
+#endif
+#if defined(CPU_MODEL_STM32L476RG) || defined(CPU_MODEL_STM32L475VG) || \
+      defined(CPU_MODEL_STM32L452RE)
    [ADC1_2_IRQn             ] = isr_adc1_2,               /* [18] ADC1, ADC2 SAR global Interrupts */


This enum doesn't exist in l452 cmsis file, it's called ADC1_IRQn, like for the l432.

@aabadie there an alias for l452 defined in the cmsis files at line 16034, would you rather have me use the original one?

/* Aliases for __IRQn */ #define ADC1_2_IRQn ADC1_IRQn

there an alias defined in the cmsis files at line 16034

Sorry, I didn't scroll that far ;)

would you rather have me use the original one?

I would say yes, we never know. It may simplify an update of that file if one day there's a new version from ST.

I think I got confused jajajaja, should or should I not use the alias. (I wasn't very clear what I meant by original one jajajaja)

Do you mean the alias was already there (ST code) or you added it for convenience ? If it's the latter, I would avoid this, otherwise, keep it like this.

It was included in the original cmsis file, with this header

//
/ For a painless codes migration between the STM32L4xx device product /
/ lines, the aliases defined below are put in place to overcome the /
/ differences in the interrupt handlers and IRQn definitions. /
/ No need to update developed interrupt code when moving across /
/ product lines within the same STM32L4 Family /
//

So I think it's going to stay in future files too ;) .

aabadie

Found remaining problems with the vectors.

fjmolinas · 2018-03-05T14:46:33Z

@aabadie addressed coments.

aabadie · 2018-03-05T15:30:57Z

boards/nucleo-l452/include/periph_conf.h

+#ifndef CLOCK_LSE
+/* 0: no external low speed crystal available,
+ * 1: external crystal available (always 32.768kHz)
+ * This defaults to 0 because hardware revision 'MB1136 C-01' of the nucleo-64


Looks like this line is a copy paste from the l476, I don't think it is valid for l452re. Maybe simply remove it ?

fjmolinas · 2018-03-05T15:47:23Z

@aabadie removed copy pasta.

aabadie

No other comments on my side, the changes look good now.

Untested-ACK

aabadie · 2018-03-05T15:56:51Z

Please squash the commits in 2:

one for the cpu changes: cpu/stm32l4: add support for stm32l432re
one the for the board: boards/nucleo-l452re: initial support.

Ah, I almost forget about it, would you mind renaming the board to nucleo-l452re (marketting name) ? (See #8649). Be careful with the doxygen comments.

aabadie

Found a remaining doxygen issue.

aabadie · 2018-03-05T17:30:05Z

boards/nucleo-l452re/include/periph_conf.h

+ */
+
+/**
+ * @ingroup     boards_nucleo-l452re


There's a problem here (just noticed that sorry): this doxygen group is never defined. Have a look here for an example.

After this is fixed, we are good.

fjmolinas · 2018-03-05T17:49:15Z

@aabadie I think that would be it, can you confirm the doxygen group thing is ok?, haven't really looked at how that works (will now jaja)

aabadie

Looks really good now ! ACK

aabadie · 2018-03-05T17:55:12Z

Just triggered Murdock: I see problems coming with unittest

fjmolinas · 2018-03-05T18:00:47Z

no problems :)

aabadie · 2018-03-05T18:00:59Z

I see problems coming with unittest

No, it passed ! So let's go @fjmolinas. Well done and thanks for your patience :)

aabadie added Type: new feature The issue requests / The PR implemements a new feature for RIOT Area: boards Area: Board ports labels Jan 25, 2018

aabadie requested changes Jan 25, 2018

View reviewed changes

fjmolinas force-pushed the nucleo-l452 branch from 7ae251f to c12119b Compare January 26, 2018 15:07

aabadie requested changes Jan 26, 2018

View reviewed changes

aabadie requested changes Feb 8, 2018

View reviewed changes

fjmolinas force-pushed the nucleo-l452 branch from e5b7dc0 to 96a0e06 Compare February 8, 2018 17:28

aabadie requested changes Feb 15, 2018

View reviewed changes

aabadie reviewed Feb 15, 2018

View reviewed changes

aabadie requested changes Feb 15, 2018

View reviewed changes

fjmolinas force-pushed the nucleo-l452 branch from b8e7482 to be19504 Compare March 5, 2018 14:45

aabadie reviewed Mar 5, 2018

View reviewed changes

aabadie approved these changes Mar 5, 2018

View reviewed changes

aabadie requested changes Mar 5, 2018

View reviewed changes

cpu/stm32l4: add support for stm32l452re

0fd1248

fjmolinas force-pushed the nucleo-l452 branch from e3de330 to 9c38f31 Compare March 5, 2018 17:41

boards/nucleo-l452re: initial support

dcc0ab8

fjmolinas force-pushed the nucleo-l452 branch from 9c38f31 to dcc0ab8 Compare March 5, 2018 17:45

aabadie approved these changes Mar 5, 2018

View reviewed changes

aabadie added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 5, 2018

aabadie merged commit db55004 into RIOT-OS:master Mar 5, 2018

fjmolinas deleted the nucleo-l452 branch August 7, 2019 15:46

boards/nucleo-l452: initial support #8441

boards/nucleo-l452: initial support #8441

Conversation

fjmolinas commented Jan 25, 2018 • edited Loading

Contribution description

Issues/PRs references

aabadie 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

fjmolinas commented Jan 26, 2018

aabadie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fjmolinas commented Jan 26, 2018

fjmolinas commented Feb 7, 2018

aabadie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fjmolinas Feb 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aabadie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aabadie Feb 15, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fjmolinas Feb 15, 2018 • 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

fjmolinas Feb 15, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aabadie left a comment

Choose a reason for hiding this comment

fjmolinas commented Mar 5, 2018

Choose a reason for hiding this comment

fjmolinas commented Mar 5, 2018

aabadie left a comment

Choose a reason for hiding this comment

aabadie commented Mar 5, 2018

aabadie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fjmolinas commented Mar 5, 2018

aabadie left a comment

Choose a reason for hiding this comment

aabadie commented Mar 5, 2018

fjmolinas commented Mar 5, 2018

aabadie commented Mar 5, 2018

fjmolinas commented Jan 25, 2018 •

edited

Loading

fjmolinas Feb 8, 2018 •

edited

Loading

aabadie Feb 15, 2018 •

edited

Loading

fjmolinas Feb 15, 2018 •

edited

Loading

fjmolinas Feb 15, 2018 •

edited

Loading