w_nop2 might do 1-cycle delay in case interrupt handling is disabled

[besteinm]

light_ws2812/light_ws2812_AVR/Light_WS2812/light_ws2812.c

Lines 148 to 150 in 4a3a289

	#if (ws2812_interrupt_handling)
	" clt \n\t"
	#endif

the condition should be negated: #if (!ws2812_interrupt_handling)

otherwise the macro w_nop2 might do a 1 cycle delay in case T happens to be set.

#if (ws2812_interrupt_handling)
#define w_nop2  "brid .+0 \n\t"
#else
#define w_nop2  "brtc .+0 \n\t"
#endif

[besteinm]

light_ws2812/light_ws2812_AVR/Light_WS2812/light_ws2812.c

Lines 173 to 177 in 4a3a289

	#if defined(__LGT8F__)
	" bst %1,7 \n\t" // '1' [02] '0' [02]
	" brts 1f \n\t" // '1' [04] '0' [03]
	" st X,%4 \n\t" // '1' [--] '0' [04] - fe-low
	"1: lsl %1 \n\t" // '1' [05] '0' [05]

additionally the T flag gets clobbered inside the loop when building for LGT8F88A; this might cause w_nop2 to have different delays in each iteration through the loop depending on the data that is sent out.

Maybe simply #define w_nop2 "rjmp .+0 \n\t" regardless of ws2812_interrupt_handling? As far as I know rjmp always takes 2 cycles (at least for the AVRs; I do not know about LGT8F88A though)

[cpldcpu]

This was specifically a fixed for the LGT8F. I don't have such a MCU myself, so i never tested it. Maybe MCU specific code is required? Not sure what the correct defines for that would be...

2218738

[besteinm]

Ah ok thanks, so the rjmp had already been considered and discarded. I did not know about the timing differences, thanks for the clarification. Sticking with brtc .+0 for w_nop2 makes sense then.

Both issues remain however:

• T flag should be cleared (clt) when building with interrupt handling disabled, because only when building with interrupt handling disabled is T considered.
• avoid clobbering of T flag inside the loop

I was wondering if the section of the loop that currently clobbers T could be rewritten like the following:

#if defined(__LGT8F__)
    "       lsl   %1    \n\t"    //  '1' [02] '0' [02]
    "       brcs  1f    \n\t"    //  '1' [04] '0' [03]
    "       st    X,%4  \n\t"    //  '1' [--] '0' [04] - fe-low
    "1:     nop         \n\t"    //  '1' [05] '0' [05]

I have added the nop in order to keep the number of cycles identical; it can be removed and w_fixedlow or w_fixedhigh be adapted instead.

[cpldcpu]

Have you tried this on an actual device? If it works, sure, go ahead and submit a pull request .