SPI Bus Interface

Official FreeEMS vanilla firmware development, the heart and soul of the system!
Post Reply
User avatar
Spudmn
LQFP112 - Up with the play
Posts: 232
Joined: Thu Feb 10, 2011 12:27 am
Location: Auckland, NZ

SPI Bus Interface

Post by Spudmn »

This is my first cut of the SPI interface.

It's not complete or tested but I post it here to document the progress and comments.

It started here viewtopic.php?f=9&t=1686&start=30 but is now more firmware.

Code: Select all

typedef struct
{
  uint16_t CS_Port_Bit;         // Some sort of Port pin mapping for the Chip Select.  Copy how Fred has mapped this.
  uint8_t SPI_Bus;            // Which of the 2 SPI buses do we want to use.
  uint16_t Bus_Speed;
  uint16_t  Bus_Mode;          // There may be other settings and these will be defined when I get into the code.
} SPI_Settings_t;



/** @brief Transmit and Receive bytes over the SPI Bus
 *
 * This will Send and receive a number of bytes over the SPI Bus
 * It will use the data in pSettings to configure the bus.
 * This function will use interrupts to complete the transfer
 * and will return immediately. Use SPI_Complete() to check
 * if the transfer is complete.
 *
 *
 * @author
 *
 * @warning Do not call this function if a transfer is in progress.
 * Use SPI_Complete() to check if the transfer is complete.
 *
 *
 * @param pSettings pointer to setup information for the SPI Bus.
 * @param pTxBuffer pointer to a buffer holding the the bytes to transmit.
 * @param pRxBuffer pointer to a buffer were the received bytes will be stored.
 * @param TxCount The number of bytes to send from the TxBuffer
 * @param RxCount The number of bytes to receive and store in RxBuffer
 *
 *
 * @return An error code. Zero means success, anything else is a failure.
 */

uint8_t SPI_Read_Write(SPI_Settings_t *pSettings,uint8_t  *pTxBuffer, uint8_t  *pRxBuffer, uint16_t TxCount, uint16_t RxCount);


/** @brief Check on the status of the SPI transfer
 *
 * Calling this function will return the status of the
 * interrupt transfer.
 *
 *
 * @author
 *
 * @warning
 *
 * @param SPI_Bus Which SPI Bus to check.
 *
 * @return Zero means Busy. One means complete no error. anything else is a failure.
 */
uint8_t  SPI_Complete(uint8_t SPI_Bus);


/** @brief Transmit and Receive bytes over the SPI Bus and wait till complete
 *
 * This will Send and receive a number of bytes over the SPI Bus
 * It will use the data in pSettings to configure the bus.
 *
 *
 * @author
 *
 * @warning
 *
 *
 * @param pSettings pointer to setup information for the SPI Bus.
 * @param pTxBuffer pointer to a buffer holding the the bytes to transmit.
 * @param pRxBuffer pointer to a buffer were the received bytes will be stored.
 * @param TxCount The number of bytes to send from the TxBuffer
 * @param RxCount The number of bytes to receive and store in RxBuffer
 *
 *
 * @return An error code. Zero means success, anything else is a failure.
 */
uint8_t SPI_Read_Write_Blocking(SPI_Settings_t * pSettings,uint8_t  *pTxBuffe, uint8_t  *pRxBuffer, uint16_t BytesToSend, uint16_t BytesToGet)
{
	uint8_t Result;

	Result = SPI_Read_Write(pSettings,*pTxBuffer,pRxBuffer,TxCount, RxCount);
	if (Result)
	{
		return (Result);
	}

	while (SPI_Complete(pSettings->SPI_Bus) == 0);  // Wait for transfer to complete

	return (SPI_Complete(pSettings->SPI_Bus));

}
User avatar
Fred
Moderator
Posts: 15431
Joined: Tue Jan 15, 2008 2:31 pm
Location: Home sweet home!
Contact:

Re: SPI Bus Interface

Post by Fred »

Thanks for posting this! :-)

Feedback, from the top:

For the time being, port mapping = 16 bit pointer and 8 bit mask.
The spi1 or spi2 thing shouldn't be uint 8, and should be a pointer to the start of a register block, which we'll define a struct for later. This future proofs it.
We'll have to get in the ring and slug it out to decide on uint8_t vs unsigned char vs a custom header which makes uint8_t into uint8, but there is no way in hell that any of OUR types are getting _t on the, or underscores in general. This is just style, though, you can ignore it for now, we can get our gloves on and fight later :-)

Good work on the doxy comments, but the * lines should start with a single leading space to align them with the start line.

Same goes for pSettings being a pointer as it does for type_t bing a type, bad taste in my opinion. There may even be an issue on the tracker for me to remove any existing mistakes of this nature. See 42ddf86d8ec1c9b52a558ecf2d71d3bc6d93976b for me removing my own instance of this. Your IDE gives you this information, if you're not using one, that's your problem :-)

I consider the * as part of the type, so IMO it shouldn't be on the var/param name, rather on the type in front, or next to the type with a space.

Mostly stylistic, but hopefully those first few things help :-)

Fred.
DIYEFI.org - where Open Source means Open Source, and Free means Freedom
FreeEMS.org - the open source engine management system
FreeEMS dev diary and its comments thread and my turbo truck!
n00bs, do NOT PM or email tech questions! Use the forum!
The ever growing list of FreeEMS success stories!
User avatar
Spudmn
LQFP112 - Up with the play
Posts: 232
Joined: Thu Feb 10, 2011 12:27 am
Location: Auckland, NZ

Re: SPI Bus Interface

Post by Spudmn »

Fred wrote: The spi1 or spi2 thing shouldn't be uint 8, and should be a pointer to the start of a register block, which we'll define a struct for later. This future proofs it.
This makes a lot of sense and makes it easier for me in the code. I Like it. :)
We'll have to get in the ring and slug it out to decide on uint8_t vs unsigned char vs a custom header which makes uint8_t into uint8, but there is no way in hell that any of OUR types are getting _t on the, or underscores in general. This is just style, though, you can ignore it for now, we can get our gloves on and fight later :-)
No need for gloves. :lol: I do my best to follow the style of the developer of the code before me. I had every intention to use your style for the coding. The funny thing is that with the example above, I wrote it in the style of the project I was last working on in Eclipse. I just followed it by habit. I don't usually use _t anyway. I have only recently started using uint8 typedefs, because it is easier when doing unit testing on a PC. As long as I code in such away as there is no confusion of the size of an int on a PC and the m68hc11 every thing should be fine.
Good work on the doxy comments, but the * lines should start with a single leading space to align them with the start line.
I have not used doxy before, so thanks for the feed back.

I will fix up the style and if you are happy if the interface I can start coding.
User avatar
Spudmn
LQFP112 - Up with the play
Posts: 232
Joined: Thu Feb 10, 2011 12:27 am
Location: Auckland, NZ

Re: SPI Bus Interface

Post by Spudmn »

User avatar
Fred
Moderator
Posts: 15431
Joined: Tue Jan 15, 2008 2:31 pm
Location: Home sweet home!
Contact:

Re: SPI Bus Interface

Post by Fred »

Re the typedefs, I'm internally torn by them. On the one hand, I know that they make it more portable, but on the other _t is fucking ugly! :-) So likely we'll just have a mapping file and use something less ugly, maybe the same just without the _t. In the mean time long form is likely better to keep with the style. We can do a mass conversion later in a single huge commit.

As for github, any reason why you don't start developing it as a "patch" to the firmware? Just create a branch and apply your changes in a logical commit sequence, then when I push new stuff, rebase your stuff onto my latest (and fix anything incompatible (if nothing, it's automatic)). I see no reason not to do this. It's what Sean has been doing with binutils for a while, and the firmware soon too. If you need help with git, I'm available on IRC, skype, spanish land line, gmail, etc.

Re the /* * thing, that's generally the case, doxygen doesn't care, but I do :-)

Fred.
DIYEFI.org - where Open Source means Open Source, and Free means Freedom
FreeEMS.org - the open source engine management system
FreeEMS dev diary and its comments thread and my turbo truck!
n00bs, do NOT PM or email tech questions! Use the forum!
The ever growing list of FreeEMS success stories!
Post Reply