adafruit / samd-peripherals

A thin API above SAMD peripherals that is uniform across SAMD21 and SAMD51
MIT License
18 stars 17 forks source link

SAMD51 FREQM: Move Beyond Blocking Single Usage #5

Closed sommersoft closed 5 years ago

sommersoft commented 6 years ago

After getting the initial usage in, and thinking in a broader scope during the PR review, the following dawned on me. It is only available to one "user" at a time, blocks out once it is initialized with no warning, and read() will report with it's current settings no matter the user. The only way around this is to deinit and re-init FREQM, which could rely on a user deinit'ing their object (like my upcoming FreqencyIn).

Init "blocking" check:

if (MCLK->APBAMASK.bit.FREQM_) {
        return;
    }

While writing the FREQM peripheral, I envisioned that a FREQM reading could replace some of the more static/assuming frequency measurements that are returned for the different clocks.

In the short time I've thought about this, the only viable approach I've come up with is to completely wrap init(), read(), and deinit() into a single function. I thought about using some sort of instance tracking, but in my opinion that adds unnecessary overhead and eats code space.

My main concern with an all-in-one function is time of execution. For instance, if the supplied REFNUM is "high", it may start interfering with any other time-sensitive/interrupt driven stuff (this was one of my main reasons for not allowing an option for the reference clock). I did experience consistent HardFaults while calling read() inside an interrupt handler (maybe I should add that note in the function?).

I can/will start to poke at this. I wanted to get a conversation on it first though...

tannewt commented 6 years ago

What are some examples of multi-user scenarios?

sommersoft commented 6 years ago

Well, if common_hal_mcu_processor_get_frequency is updated to use FREQM ("get dynamically" is still a TODO), any of the calls to it would have the init skipped if something else currently owns it and reads will be based on the established MSR_CLOCK. A quick look shows common_hal_mcu_processor_get_frequency being called by PWMOut several places, as well as tick_delay(), current_tick(), and wait_until().

Now, in that instance, the usage inside common_hal_mcu_processor_get_frequency should init and deinit inside the function. Likewise, maybe my usage in FrequencyIn should do the same. But if that is the "expected usage", then why have all broken out to init, read, and deinit steps anyway?

(P.S. thank you for asking that question, forcing me cross-over from conjecture into practical use!)

tannewt commented 6 years ago

Wouldn't using FREQM to measure the CPU speed be like using a clock to time itself? I don't think we need that. FREQM is for measuring external signals against our own clock.

On Thu, Aug 23, 2018 at 8:36 PM sommersoft notifications@github.com wrote:

Well, if common_hal_mcu_processor_get_frequency is updated to use FREQM ("get dynamically" is still a TODO), any of the calls to it would have the init skipped if something else currently owns it and reads will be based on the established MSR_CLOCK. A quick look shows common_hal_mcu_processor_get_frequency being called by PWMOut several places, as well as tick_delay(), current_tick(), and wait_until().

Now, in that instance, the usage inside common_hal_mcu_processor_get_frequency should init and deinit inside the function. Likewise, maybe my usage in FrequencyIn should do the same. But if that is the "expected usage", then why have all broken out to init, read, and deinit steps anyway?

(P.S. thank you for asking that question, forcing me cross-over from conjecture into practical use!)

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/adafruit/samd-peripherals/issues/5#issuecomment-415641339, or mute the thread https://github.com/notifications/unsubscribe-auth/AADNqcrD5kysq5iBCn4FJPI7UhpcBjItks5uT3THgaJpZM4WEzbh .

sommersoft commented 6 years ago

Wouldn't using FREQM to measure the CPU speed be like using a clock to time itself?

I think there is an understandable confusion. FrequencyIn measures an outside signal. The FREQM peripheral uses an internal reference clock (REF_CLOCK) and compares against another internal clock to measure it's frequency (MSR_CLOCK).

I'm currently using the FREQM peripheral in FrequencyIn to capture the frequency of DFLL in open mode, so that the external frequency captured by the TC is calculated more accurately. SAMD51 only, of course.

Clear as mud? :smile:

tannewt commented 6 years ago

So you are comparing two internal sources because the DFLL can't be based on it?

sommersoft commented 6 years ago

Yes, sort of.

TC is sourced from DFLL. DFLL drifts since its in open mode, meaning I can't just divide the TC's count by 48MHz. (TC's count may also be affected itself since DFLL is drifting?)

So, I'm using FREQM to compare DFLL (MSR_CLOCK) to a gclk sourced from OSCULP32K (REF_CLOCK) to get the actual* frequency of DFLL. Then, I use that measurement to divide the result from TC's count to get the detected external frequency.

* The call to freqm_read() is called in the get_item function, so it lags behind when the external reading was taken. Calling freqm_read() inside the interrupt handler was causing HardFaults.

sommersoft commented 5 years ago

Totally forgot to close this moons ago. I've abandoned use of FREQM, and the PR to put it in was reverted anyway.

If FREQM is still beneficial some other way, let me know and I can add it back in.