Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

boards/stm32l0538-disco: add initial support #9517

Merged
merged 3 commits into from
Jul 9, 2019
Merged

boards/stm32l0538-disco: add initial support #9517

merged 3 commits into from
Jul 9, 2019

Conversation

aabadie
Copy link
Contributor

@aabadie aabadie commented Jul 9, 2018

Contribution description

This PR adds an initial support for the stm32l0538-disco board.

This board embeds an epaper ink display and I could use it with the U8G2 package.

The board can be flashed via st-link but no UART is directly usable via st-link, so a serial to USB adapter is required for stdio.

Issues/PRs references

None

@aabadie aabadie added Type: new feature The issue requests / The PR implemements a new feature for RIOT CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Area: boards Area: Board ports labels Jul 9, 2018
@aabadie aabadie force-pushed the pr/board/stm32l0538-disco branch from 5b2295a to 83a8974 Compare July 9, 2018 11:13
@aabadie aabadie force-pushed the pr/board/stm32l0538-disco branch 2 times, most recently from 33fb9b9 to 7c35a13 Compare August 7, 2018 09:33
@aabadie aabadie mentioned this pull request Mar 5, 2019
Closed
@fjmolinas
Copy link
Contributor

fjmolinas commented Mar 8, 2019 •
edited
Loading

@aabadie Looks good, could you lend me this board to test?

@fjmolinas fjmolinas self-requested a review April 19, 2019 12:02
@aabadie aabadie force-pushed the pr/board/stm32l0538-disco branch 6 times, most recently from 1f29ee9 to d6fa4e5 Compare May 30, 2019 21:06
fjmolinas
fjmolinas requested changes Jun 21, 2019
View reviewed changes
Copy link
Contributor

@fjmolinas fjmolinas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ran:

python dist/tools/compile_and_test_for_board/compile_and_test_for_board.py --jobs 1 . stm32l0538-disco
And got:

- [tests/cpp11_condition_variable](tests/cpp11_condition_variable/test.failed)
- [tests/cpp11_mutex](tests/cpp11_mutex/test.failed)
- [tests/cpp11_thread](tests/cpp11_thread/test.failed)
- [tests/periph_eeprom](tests/periph_eeprom/test.failed)
- [tests/periph_flashpage](tests/periph_flashpage/test.failed)
- [tests/periph_gpio](tests/periph_gpio/test.failed)
- [tests/periph_rtc](tests/periph_rtc/test.failed)
- [tests/periph_rtt](tests/periph_rtt/test.failed)
- [tests/pkg_fatfs_vfs](tests/pkg_fatfs_vfs/test.failed)
- [tests/pthread_barrier](tests/pthread_barrier/test.failed)
- [tests/pthread_cooperation](tests/pthread_cooperation/test.failed)
- [tests/rng](tests/rng/test.failed)
- [tests/shell](tests/shell/test.failed)
- [tests/struct_tm_utility](tests/struct_tm_utility/test.failed)
- [tests/thread_cooperation](tests/thread_cooperation/test.failed)

Havent looked into the tests details, Also could you please add a documentation file?

boards/stm32l0538-disco/include/board.h Outdated
* @file
* @brief Board specific definitions for the STM32L0538-DISCO evaluation board.
*
* @author Alexabdre Abadie <alexandre.abadie@inria.fr>
Copy link
Contributor

@fjmolinas fjmolinas Jun 21, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo here on name

@aabadie aabadie force-pushed the pr/board/stm32l0538-disco branch 2 times, most recently from d8d249e to 400becc Compare June 28, 2019 15:12
@aabadie
Copy link
Contributor Author

aabadie commented Jun 28, 2019

I had a look at this one. For periph_rtc and rtt, actually the board doesn't provide an external LSE, so I had to use a custom clock configuration and no the shared L0 one.

For the other tests, for some I had to blacklist the board, for others there are working (e.g. shell, periph_gpio, rng, struct_tm_utility, periph_eeprom, periph_flashpage). I don't know what happened for those ones.

fjmolinas
fjmolinas reviewed Jul 5, 2019
View reviewed changes
boards/stm32l0538-disco/include/periph_conf.h Outdated
#define PERIPH_CONF_H

#include "periph_cpu.h"
#include "cfg_rtt_default.h"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not valid, it is calculated based on the CLK beeing 32Khz, but here it is 37Khz. I created a branch to patch this https://github.com/fjmolinas/RIOT/tree/pr_stm32_rtt_lsi.

The problem is the test still fails because LSI isn't very precise it can have a pretty big deviation around 37KhZ.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right and your branch is useful and should be PRed I think.

fjmolinas
fjmolinas requested changes Jul 5, 2019
View reviewed changes
Copy link
Contributor

@fjmolinas fjmolinas left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know see what I probably did wrong on my first go, I had the rx pin miss placed. All the tests are passing now except for tests/periph_rtt.

About rtt I don't know what to do in this case, even applying my branch code, the test wont pass because LSI isn't precise enough. I think in this case we might exclude the test from the CI because the frequency has to be measured externally.

I would still add the feature but document that when using LSI manual calibration must be done. The other option is just dropping rtt. What do you think?

@fjmolinas
Copy link
Contributor

Also the documentation needs improving.

@aabadie
Copy link
Contributor Author

aabadie commented Jul 5, 2019

I would still add the feature but document that when using LSI manual calibration must be done. The other option is just dropping rtt. What do you think?

I would drop it for simplicity and until LSI based RTT is correctly supported. What do you mean with manual calibration ?

@fjmolinas
Copy link
Contributor

I would drop it for simplicity and until LSI based RTT is correctly supported. What do you mean with manual calibration ?

You would need to measure the actual frequency and change it in the code.

@fjmolinas
Copy link
Contributor

I would drop it for simplicity and until LSI based RTT is correctly supported.

Ok go ahead and drop it, I didn't PR the RTT changes because I still need to improve some corner cases. Squash right away.

@aabadie aabadie force-pushed the pr/board/stm32l0538-disco branch 2 times, most recently from 55d832c to 9de7371 Compare July 5, 2019 16:01
@aabadie
Copy link
Contributor Author

aabadie commented Jul 5, 2019

Removed the RTT feature and added some documentation. Squashed and rebased.

@aabadie aabadie force-pushed the pr/board/stm32l0538-disco branch from 9de7371 to adc8faa Compare July 5, 2019 16:07
fjmolinas
fjmolinas reviewed Jul 8, 2019
View reviewed changes
Copy link
Contributor

@fjmolinas fjmolinas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Travis is complaining about a trailing whitespace. Otherwise ACK!

@fjmolinas fjmolinas removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jul 8, 2019
@fjmolinas
Copy link
Contributor

@aabadie when you fix the whitespace squash directly.

@aabadie aabadie force-pushed the pr/board/stm32l0538-disco branch from adc8faa to 39fae03 Compare July 8, 2019 07:07
@aabadie
Copy link
Contributor Author

aabadie commented Jul 8, 2019

when you fix the whitespace squash directly.

done

@fjmolinas fjmolinas added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jul 8, 2019
fjmolinas
fjmolinas approved these changes Jul 8, 2019
View reviewed changes
Copy link
Contributor

@fjmolinas fjmolinas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK

@fjmolinas
Copy link
Contributor

@aabadie Do you have time for one last commit? there is still a test with oveflowing RAM

@aabadie
tests: exclude stm32l0538-disco board
4ac2b4e 
because of insufficient memory available
@aabadie aabadie force-pushed the pr/board/stm32l0538-disco branch from 39fae03 to 4ac2b4e Compare July 8, 2019 19:50
@fjmolinas
Copy link
Contributor

@aabadie Thanks! All green with this one, GO!

@fjmolinas fjmolinas merged commit ba5106d into RIOT-OS:master Jul 9, 2019
@aabadie aabadie deleted the pr/board/stm32l0538-disco branch August 4, 2019 14:09
@kb2ma kb2ma added this to the Release 2019.10 milestone Sep 16, 2019
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@fjmolinas fjmolinas fjmolinas approved these changes

Assignees
No one assigned
Labels
Area: boards Area: Board ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Milestone
Release 2019.10
Development

Successfully merging this pull request may close these issues.
3 participants
@aabadie @fjmolinas @kb2ma