Skip to content

Commit 8ea94ff

Browse files
Fix comments, clean condition checks, save stack
Add more comments and adjust naming to be more informative in transferBytes_ and *aligned_. Save 64bytes of stack in double misaligned case.
1 parent 4875728 commit 8ea94ff

File tree

1 file changed

+22
-28
lines changed

1 file changed

+22
-28
lines changed

libraries/SPI/SPI.cpp

+22-28
Original file line numberDiff line numberDiff line change
@@ -546,20 +546,18 @@ void SPIClass::transferBytesAligned_(const uint8_t * out, uint8_t * in, uint8_t
546546
setDataBits(size * 8);
547547

548548
volatile uint32_t *fifoPtr = &SPI1W0;
549-
uint8_t dataSize = ((size + 3) / 4);
550549

551550
if (out) {
551+
uint8_t outSize = ((size + 3) / 4);
552552
uint32_t *dataPtr = (uint32_t*) out;
553-
while (dataSize--) {
554-
*fifoPtr = *dataPtr;
555-
dataPtr++;
556-
fifoPtr++;
553+
while (outSize--) {
554+
*(fifoPtr++) = *(dataPtr++);
557555
}
558556
} else {
557+
uint8_t outSize = ((size + 3) / 4);
559558
// no out data only read fill with dummy data!
560-
while (dataSize--) {
561-
*fifoPtr = 0xFFFFFFFF;
562-
fifoPtr++;
559+
while (outSize--) {
560+
*(fifoPtr++) = 0xFFFFFFFF;
563561
}
564562
}
565563

@@ -569,14 +567,15 @@ void SPIClass::transferBytesAligned_(const uint8_t * out, uint8_t * in, uint8_t
569567
if (in) {
570568
uint32_t *dataPtr = (uint32_t*) in;
571569
fifoPtr = &SPI1W0;
572-
dataSize = size;
573-
while (dataSize >= 4) {
570+
int inSize = size;
571+
// Unlike outSize above, inSize tracks *bytes* since we must transfer only the requested bytes to the app to avoid overwriting other vars.
572+
while (inSize >= 4) {
574573
*(dataPtr++) = *(fifoPtr++);
575-
dataSize -= 4;
574+
inSize -= 4;
576575
in += 4;
577576
}
578577
volatile uint8_t *fifoPtrB = (volatile uint8_t *)fifoPtr;
579-
while (dataSize--) {
578+
while (inSize--) {
580579
*(in++) = *(fifoPtrB++);
581580
}
582581
}
@@ -587,33 +586,28 @@ void SPIClass::transferBytes_(const uint8_t * out, uint8_t * in, uint8_t size) {
587586
if (!((uint32_t)out & 3) && !((uint32_t)in & 3)) {
588587
// Input and output are both 32b aligned or NULL
589588
transferBytesAligned_(out, in, size);
590-
} else if (!out && ((uint32_t)in & 3)) {
589+
} else if (!out) {
591590
// Input only and misaligned, do bytewise until in aligned
592591
while (size && ((uint32_t)in & 3)) {
593592
*(in++) = transfer(0xff);
594-
size--;
593+
size--;
595594
}
596595
transferBytesAligned_(out, in, size);
597-
} else if (!in && ((uint32_t)out & 3)) {
596+
} else if (!in) {
598597
// Output only and misaligned, bytewise xmit until aligned
599598
while (size && ((uint32_t)out & 3)) {
600599
transfer(*(out++));
601-
size--;
600+
size--;
602601
}
603602
transferBytesAligned_(out, in, size);
604603
} else {
605-
// HW FIFO has 64b limit, so just align in RAM and then send to FIFO aligned
606-
uint8_t outAligned[64]; // Stack vars will be 32b aligned
607-
uint8_t inAligned[64]; // Stack vars will be 32b aligned
608-
if (out) {
609-
memcpy(outAligned, out, size);
610-
} else {
611-
memset(outAligned, 0xff, size); // 0xff = no xmit data
612-
}
613-
transferBytesAligned_(outAligned, inAligned, size);
614-
if (in) {
615-
memcpy(in, inAligned, size);
616-
}
604+
// HW FIFO has 64b limit and ::transferBytes breaks up large xfers into 64byte chunks before calling this function
605+
// We know at this point it is a bidirectional transfer
606+
uint8_t aligned[64]; // Stack vars will be 32b aligned
607+
// No need for separate out and in aligned copies, we can overwrite our out copy with the input data safely
608+
memcpy(aligned, out, size);
609+
transferBytesAligned_(aligned, aligned, size);
610+
memcpy(in, aligned, size);
617611
}
618612
}
619613

0 commit comments

Comments
 (0)