Chuck, I'm not sure why you feel that way, but having disagreements and
discussions about technical aspects of a project is usually a good thing.
I am also not sure how I am not being helpful by pointing out something
on the errata sheets that would be a potential source of bugs, some ways
it can happen, and some ways to mitigate it, for different chips, based
on the chip manufacturer's recommendations.
Use any combination of compiler flags to do what? Prove that there will
be a less than 16 instructions delay between clock enable and the first
peripheral register write? Does that really need proving? You don't even
need to change compiler flags for that.
I also have no idea what ST's supplied software has anything to do with
any of this.
Can we go back to technical discussions now?
If so, here is how Chromium OS fixed it:
https://chromium-review.googlesource.com/c/246181/
Matthew
On 03/13/2017 07:41 PM, Chuck McManis wrote:
Matthew, it feels like you are trying to create an argument here.
If you want to be helpful, and this is actually a problem given the
way libopencm3 is built, then *reproduce the problem* with libopencm3.
You can use any combination of compiler flags you want, you just can't
change the source inside of libopencm3.
*If* you can reproduce the problem then that will show what the window
of opportunity is. If you cannot, then you will see that Karl is
correct in that it won't occur in libopencm3.
If having this issue is such a problem for you, then by all means use
the ST Micro supplied software which is written presumably by their
experts.
---Chuck
On Mon, Mar 13, 2017 at 10:57 AM, Matthew Lai <m...@matthewlai.ca
<mailto:m...@matthewlai.ca>> wrote:
On 03/13/2017 01:25 PM, Karl Palsson wrote:
Matthew Lai <m...@matthewlai.ca <mailto:m...@matthewlai.ca>> wrote:
I would ignore it. There's very few places where it
will really matter, and the people who will need it,
will be handling it themselves anyway, not using high
level helper functions.
It would mean things like:
rcc_periph_clock_enable(RCC_TIM3);
timer_set_period(TIM3, 1024);
Would violate this, at high prescalar settings (or low
prescalar settings with LTO). That is actually the example
given here:
https://github.com/libopencm3/libopencm3/blob/master/lib/stm32/common/timer_common_all.c#L70
<https://github.com/libopencm3/libopencm3/blob/master/lib/stm32/common/timer_common_all.c#L70>
It would be a potential source of very difficult to find bugs.
There's plenty of edge cases if you really want to go looking.
I'm not really entirely sure how far to go trying to protect
people. [1]
That is true, but in this case, an user that uses libopencm3
functions to setup clocks, enables clock for a timer, and uses it
right away will be bitten.
I'd say we should protect the user in this case, since 1) it's a
very common sequence, 2) most users probably don't know about it
(it's not mentioned at all in reference manuals until f7), 3)
resulting bugs will be very hard to find, and 4) I think there's a
pretty easy fix (see below).
APB1 is usually AHB/4 on f2, f4, and f7 at least. That means 5
cycles are required for f2 and f4, and 8 cycles on f7. I wouldn't
want to rely on the compiler to happen to generate that much
delay, especially on f7 which is dual-issue (8 cycles can mean 16
instructions).
Since I imagine rcc_periph_clock_enable() to not be
performance-critical in most applications, wouldn't adding the
wait states be better default behaviour, and people who really
need to enable clocks very quickly can go to registers
directly?
It's more that I'm not sure you can do it neatly, and be robust
enough to be worth trying. You'll need to interpret what bus a
peripheral is on, as well as what speed it's currently running
at. Doing this using the rcc_axx_speed globals is... ok, but
they're not set if people didn't use clock helpers to set them
up.
Adding your peripheral speed as a parameter is a gross user API.
For the vast majority of use cases, this is simply never going to
be a problem. Doing any sort of meaningful cycle delay is going
to just add code that most people will never need.
In my opinion at least :)
According to the errata sheets, it's very easy on pre-f7 chips.
All that's required is a DSB barrier instruction. We don't
actually have to do any cycle counting, or determining which bus
the peripheral is on.
Unfortunately that option is not mentioned for f7, which requires
2x peripheral clock cycles (could be because of the new AXIM bus).
However, there's still no need to figure out which bus a
peripheral is on - we can always just do enough delay for APB1.
There's no harm in delaying for longer than necessary, unless
someone needs to enable clocks really quickly. Figuring out APB1
frequency is not easy. I would just use the globals and add a note
in the documentation that users are on their own if they are
setting up their own clocks.
Matthew
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
libopencm3-devel mailing list
libopencm3-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libopencm3-devel
