stm32l4: implement stmclk interface#7469
Conversation
|
@haukepetersen, @vincent-d, @aabadie: what tests were you using for previous verification of stmclk stuff? |
Since this change is related to the clock initialization, having basic features such as UART and timers working on nucleo-l476 and nucleo32-l432 is enough for me: just flash and run some related tests applications. I'll be with the hardware next week, so I'll test. |
|
@MichelRottleuthner please rebase and fix conflicts. @aabadie can you test this one as well? |
be6a11f to
c33985d
Compare
|
rebased and fixed conflicts |
aabadie
left a comment
There was a problem hiding this comment.
Tested on nucleo-l476 and nucleo32-l432, works. There are 3 boards supported with a L4 but you changed the board config to only one, is this intended ?
| /* 0: no external low speed crystal available, | ||
| * 1: external crystal available (always 32.768kHz) */ | ||
| #define CLOCK_LSE (1) | ||
| /* 0: enable MSI only if HSE isn't available |
There was a problem hiding this comment.
Maybe these new defines could also be added to other L4 based boards ? (nucleo32-l432, b-l475e-iotnode)
There was a problem hiding this comment.
I see that LSE is enables for the boards but as we know some boards don't have an external clock source (crystal etc.) So I'd say to better enable LSI as a general rule.
There was a problem hiding this comment.
... you changed the board config to only one, is this intended ?
more or less - b-l475e-iot01 wasn't there when I opened this PR. And I haven't checked (yet) if all three of the L4s support the MSI-LSE sync feature in the same way.
Maybe these new defines could also be added to other L4 based boards ?
Probably yes, I will check the Datasheets now and update the other configs if appropriate.
...as we know some boards don't have an external clock source (crystal etc.) So I'd say to better enable LSI as a general rule.
Generally agree, but this file belongs to the board (nucleo-l476 in this case) and thus it should be clear if it has an external crystal. If another board comes without LSE the *lfclk functions of stmclk_common.c should take care of prefering LSI over LSE.
There was a problem hiding this comment.
Generally agree, but this file belongs to the board (nucleo-l476 in this case) and thus it should be clear if it has an external crystal. If another board comes without LSE the *lfclk functions of stmclk_common.c should take care of prefering LSI over LSE.
The function you mention relies on the define just above the line we're commenting. The problem is actually that some boards have LSE, some not, it depends on the revision and I think it doesn't worth to create another define to know which revision is the board in question.
There was a problem hiding this comment.
So you say that some boards that currently define CLOCK_LSE 1 don't actually have an external crystal? This would be bad indeed. But then it's more of a general problem with the RIOT configuration and not directly related to this PR because I didn't actually change this compared to master.
...some boards have LSE, some not, it depends on the revision
Do you know if this is also the case for one of the currently supported L4s? Is this reflected by their datasheets (e.g. multiple revisions that state differing configuation related to LSE)?
There was a problem hiding this comment.
So you say that some boards that currently define CLOCK_LSE 1 don't actually have an external crystal? This would be bad indeed. But then it's more of a general problem with the RIOT configuration and not directly related to this PR because I didn't actually change this compared to master.
Yes, it's a general problem, but as you're changing the interface to generalise it for the current configuration, it's very possible that boards without crystal will just fail.
Do you know if this is also the case for one of the currently supported L4s? Is this reflected by their datasheets (e.g. multiple revisions that state differing configuation related to LSE)?
Yes, the datasheet (section 6.7) state what is the configuration according to the revision.
There was a problem hiding this comment.
...it's very possible that boards without crystal will just fail
like they would also fail with current master ;)
Nonetheless thanks for pointing out that issue here. I disabled LSE by default on l476 now. The other boards should be safe with LSE enabled by default - see comment below. Do you possess one of the l476 with HW-disabled LSE or do I need to whip out my soldering iron for testing?
|
As I just looked it up, some info about LSE availability on the currently supported L4 boards:
|
aabadie
left a comment
There was a problem hiding this comment.
Tested on all L4 based boards, works.
ACK, and go!
|
nice! thanks for testing guys :) |
Only did a quick test (xtimer_usleep, xtimer_drift, hello world..) on nucleo-l476 for now and had no problems so far.