bugs in AVR/hal_spi_lld.*

ChibiOS public support forum for topics related to the Atmel AVR family of micro-controllers.

Moderators: utzig, tfAteba

Marcello
Posts: 8
Joined: Tue Apr 05, 2016 2:52 pm
Location: Torgon, Switzerland
Been thanked: 1 time

bugs in AVR/hal_spi_lld.*

Postby Marcello » Fri Jul 01, 2016 9:53 pm

Hi,
I tried to get mmc_spi with FATFS in ChibiOS master (3b1848e on github) running on an Arduino UNO, but it didn't work. First I discovered that the card wouldn't connect because the AVR SPI implementation sends 0 as a dummy variable when it receives and ignores data, whereas SD cards want to hear a 0xFF value. I defined a dummy send value, but maybe it's a better idea if the value of the dummy variable becomes a field in the SPI_config structure? This allowed the card to connect, but still, writing to the card failed. I couldn't exactly pinpoint the problem without a debugger, but I have a strong suspicion that the existing spi interrupt routine corrupts memory by writing to buffers outside of their range. The routine is also needlessly complex, so I rewrote it to fix the problem. I also replaced the send, receive and ignore functions with macros which saves a few bytes of program memory. Attached is a patch with the changes to the AVR/hal_spi_lld.* files and a test project for your review.
Kind regards, Marcello.
Attachments
ChibiOS.patch.zip
(9.55 KiB) Downloaded 21 times

User avatar
Giovanni
Site Admin
Posts: 9409
Joined: Wed May 27, 2009 8:48 am
Location: Salerno, Italy
Has thanked: 112 times
Been thanked: 100 times
Contact:

Re: bugs in AVR/hal_spi_lld.*

Postby Giovanni » Sat Jul 16, 2016 11:06 am

Duplicating the AVR forum.

User avatar
Giovanni
Site Admin
Posts: 9409
Joined: Wed May 27, 2009 8:48 am
Location: Salerno, Italy
Has thanked: 112 times
Been thanked: 100 times
Contact:

Re: bugs in AVR/hal_spi_lld.*

Postby Giovanni » Tue Dec 13, 2016 10:51 am

Is this still relevant?

Giovanni

Marco
Posts: 99
Joined: Tue Apr 16, 2013 8:22 pm
Has thanked: 2 times
Been thanked: 8 times

Re: bugs in AVR/hal_spi_lld.*

Postby Marco » Tue Dec 13, 2016 12:24 pm

I have no problems using the SPI driver so far so the modifications to the interrupt routine are not needed IMO. I checked the SPI drivers for the STM32 port and they send 0xFF as dummy byte. I think the AVR port should to this as well.

Marco

User avatar
Giovanni
Site Admin
Posts: 9409
Joined: Wed May 27, 2009 8:48 am
Location: Salerno, Italy
Has thanked: 112 times
Been thanked: 100 times
Contact:

Re: bugs in AVR/hal_spi_lld.*

Postby Giovanni » Tue Dec 13, 2016 12:44 pm

Just to know if I should mark it as "solved", there is an entry of this topic also in the bug reports forum.

Giovanni

Marcello
Posts: 8
Joined: Tue Apr 05, 2016 2:52 pm
Location: Torgon, Switzerland
Been thanked: 1 time

Re: bugs in AVR/hal_spi_lld.*

Postby Marcello » Wed Dec 28, 2016 4:37 pm

Hurray! Somebody finally took the time to have a look at my solution to a bug which I posted 8 months ago. You took your time guys! :|

And yes, I think my initial post is still relevant:
Apart from (unknowingly) solving the "Write Collision" bug that Marco discovered, the solution I propose has some other advantages:
* It solves another bug: spiIgnore() always sends out 1 byte too much (this causes problems in mmc_spi).
* the code of the interrupt routine is considerably simpler, smaller and therefore faster. I also think it's easier to read and understand.
* the driver uses slightly less RAM memory.
* overall the code is slightly smaller; also because spiRead(), spiIgnore() and spiSend() are now macros that are translated into different invocations of spiExchange()

With regard to the dummy variable, I think it needs to become a mandatory config field. Different spi-devices might have different requirements regarding the dummy variable. If you need to receive an array of 200 bytes from a particular device and the dummy variable is not correct, you end up doing a spiExchange() with a txBuf of 200 dummy values.

I included the hal_spi_lld files once again. Let me know if you want to use them, than I will clean up the code and fix comments where necessary.

Kind regards,

Marcello
Attachments
AVR_hall_spi_lld.zip
(4.91 KiB) Downloaded 2 times

User avatar
Giovanni
Site Admin
Posts: 9409
Joined: Wed May 27, 2009 8:48 am
Location: Salerno, Italy
Has thanked: 112 times
Been thanked: 100 times
Contact:

Re: bugs in AVR/hal_spi_lld.*

Postby Giovanni » Wed Dec 28, 2016 6:16 pm

Hi Marcello,

I am not directly involved with the AVR port and the maintainer are volunteering their time for the AVR sub-project, you can't expect an immediate reaction but bug reports usually don't get lost.

Giovanni

utzig
Posts: 318
Joined: Sat Jan 07, 2012 6:22 pm
Location: Brazil
Been thanked: 6 times
Contact:

Re: bugs in AVR/hal_spi_lld.*

Postby utzig » Sat Dec 31, 2016 10:48 pm

I applied the patch with some style changes. I have no AVR board to test it right now but the patch looks ok. If you can re-test and post updates, please do.

Fabio Utzig

Marcello
Posts: 8
Joined: Tue Apr 05, 2016 2:52 pm
Location: Torgon, Switzerland
Been thanked: 1 time

Re: bugs in AVR/hal_spi_lld.*

Postby Marcello » Tue Jan 03, 2017 1:39 pm

Hi Fabio,

Thanks for applying the patch! I have re-tested it and it works fine.

Kind regards, Marcello


Return to “AVR Support”

Who is online

Users browsing this forum: No registered users and 1 guest