Incorrect transfer of SPI packets with BitOrder LSBFIRST when count is >= 4

[gentilkiwi]

When using void transfer(void *buf, size_t count); of the SPI Class, with BitOrder LSBFIRST, group of 4 bytes are reversed.
It seems to be here because of the optimization to handle arrays 4 bytes by 4 bytes (then byte(s) left), but it reverse the whole DWORD, not only bits of the byte.

Captures

MSBFIRST - OK

(left is using void transfer(void *buf, size_t count); right is multiple transfer(uint8_t data);)

LSBFIRST - KO

(left is using void transfer(void *buf, size_t count); right is multiples transfer(uint8_t data);)

Remarks:

• Problem is not present on R3, only on R4 ;
• LSBFIRST is not very common, but NXP PN532 is using it by eg ;
• I'm not sure about this issue, it's too big to have been missed by real users more advanced than me - do not hesitate to close this issue if I'm totally wrong!

[gentilkiwi]

• Did not check on the receive side (but lots of chance it can be the same)

ping @RudolphRiedel

[RudolphRiedel]

I took a brief look at this and I have an idea what is causing it and how to fix it.

[RudolphRiedel]

I can confirm the issue, looks like so far nobody tried to use LSBFIRST with buffer transfers.
And well, that includes me, sorry.

I fixed it here and this is how I currently have it up and running, MSBFIRST:

LSBFIRST:

Note, I am using two decoders to not switch between readings, the top is configured for LSBFIRST, the bottom for MSBFIRST.

I changed line 447 of SPI.cpp from
_spi_ctrl.p_regs->SPDCR2_b.BYSW = 1;

to:
_spi_ctrl.p_regs->SPDCR2_b.BYSW = ~bit_order;

This one does a byte-swap for the 32 bit transfers that happen as optimization when doing buffered transfers.

And for the background, while I was happy to see that Renesas finally released new versions of the datasheet and user manual back in September 2023, the user-manual is at Rev.1.10 now, for the SPI they only fixed the issue that in register SPDCR the bit SPBYT was not documented.
The register SPDCR2 is still not mentioned in the user-manual.
Please fix @renesas.

[gentilkiwi]

Thank you for this quick & efficient analysis @RudolphRiedel !
I can confirm LSB is now ok :)

Will continue to test against real device.

BTW, any reason to use ~bit_order instead of !bit_order for a on/off value ?

[RudolphRiedel]

BTW, any reason to use ~bit_order instead of !bit_order for a on/off value ?

No, I just wanted to inverse the value after I saw that SPCMD_b[0].LSBF needed a 1 to activate LSBFIRST.
And bit_order is not a boolean, it is spi_bit_order_t which is a typedef for an enum, so this is of type int (at least).

I just checked, !bit_order does also work.
And what would be more confusing, "bitorder + 1" and "bitorder - 1" also do work. :-)

Now I am not sure what the most appropriate variant is, especially in the C++ context.

[gentilkiwi]

Just my vision of target bitfield :')

Anyway, I'm now still stuck because Elechouse's PN532 module is a little bit too low for the R4 logic level :( - R3 was fine
>3.1V needed to be considered as a 1 on GPIO

I can't imagine what can be the result on MISO :'), but that's another story...

[RudolphRiedel]

Anyway, I'm now still stuck because Elechouse's PN532 module is a little bit too low for the R4 logic level :( - R3 was fine

3.1V needed to be considered as a 1 on GPIO

I has been a while since I used something like this so I just checked the datasheet for the NXP PN532.
It has a supply voltage range of 2.7V to 5.5V and Elechouse have "VCC: 3.3V~5V" in their documentation.
At 3.3V the required minimum level for the SPI pins for high is 2.31V.

[gentilkiwi]

Thank you for help :) I feel lucky on the PN532, it is very tolerant.

I'm more worried on the Arduino R4 side : I was unable to read a logical value of 1 on a GPIO before reaching >3.1V
And the PN532 is more around 3.0V output...

I don't think I will be able to do without a fast level shifter this time :')

[gentilkiwi]

TXS0108EPWR looks cool :) - https://www.ti.com/lit/ds/symlink/txs0108e.pdf