SPI daisy chain library

[concon]

Hi,

Is it possible to change the contents of the spi daisy chain library I found on github to be able to run it on atttiny1616?

https://github.com/hideakitai/SPIDaisyChain/tree/master

[SpenceKonde]

What the bloody hell does that library do? The author in deprecating it appears to have burned the readme.

Okay, what the bloody hell is this lunacy?!

// select function for rise/fall CS
	if (_pinCS > 7) {
		_pinCS -= 8;
		fallCS = &SPIDaisyChain::fallCSPORTB;
		riseCS = &SPIDaisyChain::riseCSPORTB;
	} else {
		fallCS = &SPIDaisyChain::fallCSPORTD;
		riseCS = &SPIDaisyChain::riseCSPORTD;
	}

Seriously?! This is an incompatibility right here, and a mindbogglingly stupid one.... With ALMOST EVERY AVR EVER. No consideration of the fact that only a small number of cores give PORTD all of the first few pins and PORTB comes next? Even though cores have always provided a bloody lookup function that does exactly this for them correctly instead of incorrectly like this.

The code for the actual bitflips is also awful. I think the library author is expecting optimizations that will not happen to happen!

This library can clearly be done far more efficiently, and the improved performance of pointer access and the PORTx.OUTSET/CLR registers give performance only 1 cycle slower than the author (falsely) hoped would occur. But that is not what that code generates. It takes at least 4-6 cycles to load the first pointer, 3 cycles to jump there, then uses |= on a register in the Low I/O space, hjoping for a SBI instrtuction, but the argument is not compile time known, so that's not an atomic SBI instruction that's an LDS, ORI, STS ( Change: 2 to 5 on classic, 1 to 6 on modern)
The obvious way is to sacrifice 2 bytes of ram and store the pointer to the port, and use the OUTSET and OUTCLR registers. Depending on how dirty you are with the port pointer, you can

inline void SPIDaisyChain::fallCSPORTD()
{
	PORTD &= ~_BV(_pinCS);
}

inline void SPIDaisyChain::fallCSPORTB()
{
	PORTB &= ~_BV(_pinCS);
}

inline void SPIDaisyChain::riseCSPORTD()
{
	PORTD |= _BV(_pinCS);
}

inline void SPIDaisyChain::riseCSPORTB()
{
	PORTB |= _BV(_pinCS);
}

is what's strictly speaking incompatible.

Direct port is

inline void SPIDaisyChain::fallCSPORTD()
{
	VPORTD.OUT |= ~(1 << _pinCS)
}

inline void SPIDaisyChain::fallCSPORTB()
{
	VPORTB.OUT |= ~(1 << _pinCS)
}

inline void SPIDaisyChain::riseCSPORTD()
{
	VPORTD.OUT |= 1 << _pinCS
}

inline void SPIDaisyChain::riseCSPORTB()
{
	VPORTB.OUT |= 1 << _pinCS;
}

But all this is bloody stupid, and less efficient than the non idiotic solution...

Take all their rise and fall CS functions and flush them down the toilet (they're far too gross for the trash).

static inline void SPIDaisyChain::fallCS(PORT_t* p, uint8_t m)
{
	p -> OUTCLR = m;
}

static inline void SPIDaisyChain::riseCS(PORT_t* p, uint8_t m)
{
	p -> OUTSET = m;
}

Add 2 more member variables to the daisychain class, of types uint8_t and PORT_t*, let's say _CS_bm and _CS_port. Remove _CSpin.
And something liker this in the constructor

SPIDaisyChain(const int pinCS, const int nDaisyChains) {
uint8_t tport=digitalPinToPortStruct(digitalpinToPort(pinCS));
_CS_port = digitalPinToPortStruct(digitalpinToPort(pinCS));
_CS_bm = digitalPinToBitMask(pinCS);
//.....

And all this kinda stuff

*fallCS(); //slow way tto call fallCS - 6 clocks loading 3 clocks icall 2 clocks (classic) cbi 4 clocks ret. 

kill it with fire and use ex
fallCS(PORT_t p, uint8_t CS_bm) we used above.  And as a static function the optimizer can sometimes weave it's magic over it. Still 6 clocks loading  (hand assembly knocks off at least 2 here) - unless any are already in registers, which is much more likely with this construction. Either way, there's no icall,there's no ret, it loads the address of the CS pin, writes out it;s constant to it instead of a 2 cycle classic 1 cycle modern cbi, it's a 1 cycle modern std following an ldi to load in that magic value, or 2-3 if it's not compile time constant. 

But all that nonsense above aside - I'm missing something. Why on earth is that useful? 

I remain unconvinced that there is any purpose to this 




Adapt header file appropriately. 

By making the functions static, we allow more optimization and less duplication.