ArduinoShrink icon indicating copy to clipboard operation
ArduinoShrink copied to clipboard

wrong inline asm constraint in delayMicroseconds

Open nerdralph opened this issue 2 years ago • 2 comments

Bug first discovered in MicroCore, which uses the same code. https://github.com/MCUdude/MicroCore/issues/136

nerdralph avatar Jan 03 '23 18:01 nerdralph

What's the reason for the slightly different delayMicroseconds implementation in ArduinoShrink and picoCore?

ArduinoShrink: https://github.com/nerdralph/ArduinoShrink/blob/a95c1c40721834e714f6bfe88fe0a280a7940052/src/as_wiring.c#L25-L41

picoCore:

__attribute((always_inline))
static inline void delayMicroseconds(uint16_t us)
{
    // if us is a compile-time constant result is accurate to 1 cycle
    if (__builtin_constant_p(us)) {
        _delay_us(us);
        return;
    }

    // when us is not known at compile time, delay is accurate to +/- 2us
    // plus an overhead of 3 CPU cycles
    const float fMHz = (F_CPU/1000000.0);
    // subtract two for rounding before dividing by 4
    us -= 2;
    delay4us:
        // delay 4us per loop, less 4 cycles for overhead
        _delay_us(4.0 - (4.0 / fMHz));
        asm volatile ("sbiw %[us], 4" : [us]"+d"(us));
    asm goto( "brpl %l[delay4us]" :::: delay4us);
}

MCUdude avatar Jan 04 '23 17:01 MCUdude

picoCore is designed to work at lower frequencies, such as 4.8Mhz or even 1.2Mhz. ArduinoShink is designed for the common frequencies of 8 and 16Mhz. If you try to build ArduinoShink with F_CPU set to 1Mhz it will fail.

On Wed, Jan 4, 2023, 13:47 Hans @.***> wrote:

What's the reason for the slightly different delayMicroseconds implementation in ArduinoShrink and picoCore?

ArduinoShrink:

https://github.com/nerdralph/ArduinoShrink/blob/a95c1c40721834e714f6bfe88fe0a280a7940052/src/as_wiring.c#L25-L41

picoCore:

__attribute((always_inline)) static inline void delayMicroseconds(uint16_t us) { // if us is a compile-time constant result is accurate to 1 cycle if (__builtin_constant_p(us)) { _delay_us(us); return; }

// when us is not known at compile time, delay is accurate to +/- 2us
// plus an overhead of 3 CPU cycles
const float fMHz = (F_CPU/1000000.0);
// subtract two for rounding before dividing by 4
us -= 2;
delay4us:
    // delay 4us per loop, less 4 cycles for overhead
    _delay_us(4.0 - (4.0 / fMHz));
    asm volatile ("sbiw %[us], 4" : [us]"+d"(us));
asm goto( "brpl %l[delay4us]" :::: delay4us);

}

— Reply to this email directly, view it on GitHub https://github.com/nerdralph/ArduinoShrink/issues/12#issuecomment-1371235209, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABKNZ6TJ6BVL62MV6EEQN5LWQWZRVANCNFSM6AAAAAATP6TSQA . You are receiving this because you authored the thread.Message ID: @.***>

nerdralph avatar Jan 04 '23 19:01 nerdralph