Re: [Avr-libc-corelib] Style guidelines up! Let's get coding...

[Ruddick Lawrence]

I included multislave mode because Ron mentioned it might be useful to have a callback (that would select the slave) that would be called before transmitting the first byte in a buffer. I think it can probably be removed because people can just select the slave before calling the send function.

spi_change_mode would really only be used for a multimaster system. The idea is that the device can be a slave until something (which the client code would determine) signals that it should become the master. Then spi_change_mode would switch from slave to multimaster mode. Not sure if that's the best way to do it, but it seemed like a fairly flexible way to deal with multimaster systems.

I like the idea of a non-interrupt transfer (maybe call it blocking send or something like that).

I guess the delay would have to use a timer interrupt, which would make it very messy... Any other suggestions?

I forgot to include the interrupts in the API, but basically each module would have a static inline function for each interrupt vector it needs to use. The user would then call each function from the appropriate interrupt vector. This allows us to abstract how the interrupt works, let the user retain control of the ISR, and have it run as quickly as if the code were just in the vector.

Ruddick

2009/12/15 Frédéric Nadeau <address@hidden>

What would be the difference between SPI_MASTER_MODE and
SPI_MASTER_MULTISLAVE_MODE?

Unless you have a really flexible PCB, I don't think it would be
required to have the spi_change_mode. Could you elaborate maybe on a
scenario to give me an idea of how could it be use?

Would be nice to have a spi_master_send(I usually use the term
transfer since data goes in both way, just a though) for pooling mode,
without interrupt.

SPI in Master&Slave is easy. Any slave should have a value in the SPDR
register(usually 0xFF, set wit spi_slave_write). When an interrupt
occurs, meaning end of transmission, interruput handler should check
wheter SS is low or high. If low, it's a slave interrupt, if high it's
a master interrupt.

I think this would be required:
void spi_transfer_isr(uint8_t ubDataOut, uint8_t *pubDataIn)
{
       uint8_t ubDataTmp = SPDR;
       SPDR = ubDataOut;
       if (pubDataIn != (void*) 0)
       {
               *pubDataIn = ubDataTmp;
       }
}
To be used in ISR. Needed for custom ISR, at least.

if spi_master_send is interrupt driven, how do you plan on
implementing the delay? If delay is in us, I would find it weird to
have a, worst case scenario, delayus(UINT16_T_MAX) somewhere in the
interrupt.

My opinion is that Interrupt handler should be left outside the
library. I think of so many ways of  writing the interrupt depending
on the case. One thing we could do though is to provide some
implementation examples. This would help people to use the library as
well. (there was something about weak interrupt, i didn't get that
part)

That is what comes to my mind right now.
Good work.

2009/12/14 Ruddick Lawrence <address@hidden>:

> Below is a starter draft of the API. It's based largely on Ron's proposal
> and the resulting discussion, along with some of my modifications. It's
> presented as an incomplete header file with doxygen comments because I can
> visualize it best like that.
>
> Some things I changed:
> -Uses the concept of a current SPI mode (master or slave) to simplify the
> interface
> -Callbacks are attached separately from initialization.
>
> Although the possible modes includes multimaster and multislave, I have not
> done anything with them. A multislave state seems unnecessary because the
> client code on the master can just do slave selection before calling
> spi_master_send. As for multimaster, it seems complicated and I'm not sure
> it's worth it to deal with that case (I'm thinking at the least we can have
> a callback for when the SS line indicates a master to become a slave).
>
> Some usage questions:
> -Do we want to have the master send be a buffer instead of a single byte?
> Possibly have the single byte option as well?
> -Have people needed to often change SPI settings once a program has started?
> In that case it might make it clearer to have some sort of spi_set function
> (or group of functions) that spi_init calls.
>
> I look forward to seeing your modifications and comments.
> Ruddick
>
> PS Frederic, I have never used CAN, but from what I hear it would be a
> useful module to have. Send out the API to the list (probably in a new
> thread), and I'm sure you'll get some good feedback.
>
> ==
>
> /**
>  * Proposed API for the SPI module.
>  */
>
> /**
>  * Possible SPI clock polarities.
>  */
> typedef enum
> {
>     SPI_CLK_POL_RISING = 1, /**< Rising polarity. */
>     SPI_CLK_POL_FALLING /**< Falling polarity. */
> } spi_clock_pol_t;
>
> /**
>  * Possible SPI clock phases.
>  */
> typedef enum
> {
>     SPI_CLK_PHASE_LEADING = 1, /**< Leading phase. */
>     SPI_CLK_PHASE_TRAILING /**< Trailing phase. */
> } spi_clock_phase_t;
>
> /**
>  * Possible SPI clock rates.
>  */
> typedef enum
> {
>     SPI_CLK_DIV2 = 1, /**< Clock rate of Fosc/2. Note that this rate is too
> fast for AVR to AVR communications. */
>     SPI_CLK_DIV4, /**< Clock rate of Fosc/4. Note that this rate is
> hazardous for AVR to AVR communications. */
>     SPI_CLK_DIV8, /**< Clock rate of Fosc/8. */
>     SPI_CLK_DIV16, /**< Clock rate of Fosc/16. */
>     SPI_CLK_DIV32, /**< Clock rate of Fosc/32. */
>     SPI_CLK_DIV64, /**< Clock rate of Fosc/64. */
>     SPI_CLK_DIV128 /**< Clock rate of Fosc/128. */
> } spi_clock_rate_t;
>
> /**
>  * Possible SPI modes for this AVR.
>  */
> typedef enum
> {
>     SPI_MASTER_MODE = 1, /**< Set as an SPI master. */
>     SPI_SLAVE_MODE, /**< Set as an SPI slave. */
>     SPI_MASTER_MULTISLAVE_MODE, /**< Set as an SPI master in a multi-slave
> system. */
>     SPI_MULTIMASTER_MODE /**< Set as a master in a multi-master system. */
> } spi_mode_t;
>
> /**
>  * The callback to register with the SPI module.
>  * If the
>  */
> typedef void (*spi_callback_t)(uint8_t data);
>
> /**
>  * Initializes the SPI module.
>  * Sets the SPI mode (master or slave), clock rate divisor (not used for
> slave mode), clock phase, and clock polarity. Can be called more than once
> to change the values [do we want to do this through different functions?].
>  */
> SPI_RET spi_init(spi_mode_t mode, spi_clock_rate_t rate, spi_clock_phase_t
> phase, spi_clock_pol_t polarity);
>
> /**
>  * Changes the SPI mode. Could be used for a multimaster system.
>  */
> SPI_RET spi_change_mode(spi_mode_t mode);
>
> /**
>  * Register a callback function with a certain mode.
>  * For master mode, the function is called after all data is done being
> transmitted; the argument is a byte of error flags [we'll have to define
> these]. For slave mode, the function is called when a byte of data is
> received; the argument is that byte. Callbacks need not be registered for
> the library to run, but it will most likely not work as desired.
>  */
> SPI_RET spi_register_callback(spi_mode_t mode, spi_callback_t callback);
>
> /**
>  * Send SPI data from the master.
>  * Transmit count bytes of data from the dataOut buffer and store the
> incoming bytes in the dataIn buffer. Uses the SPI transmit done interrupt so
> it is non-blocking. Waits at least delay clock ticks in between sending
> bytes. It is up to the client code to verify that both buffers have enough
> bytes allocated and remain valid until the transmission is complete. If a
> callback is registered with SPI_MASTER_MODE, it is called when all the bytes
> have been transmitted.
>  */
> SPI_RET spi_master_send(uint16_t count, uint8_t *dataOut, uint8_t *dataIn,
> uint16_t delay);
>
> /**
>  * Reports the state of the SPI module [will have to be defined].
>  * In master mode, reports whether the module is still sending data. Not
> used for slave mode (yet).
>  */
> SPI_STATE spi_get_state(void);
>
> /**
>  * Writes a byte of data to be sent to master on next transmit.
>  */
> SPI_RET spi_slave_write(uint8_t data);
>
>
>
> 2009/12/13 Frédéric Nadeau <address@hidden>
>>
>> Hi there,
>>
>> I just posted a patch #7024. I think you could use it in SPI init.
>> Since not all device as SPI port on same location, this could be use
>> to set directions:
>>
>> DDR_MOSI |= 1<<MOSI_IDX
>> and so on.
>>
>>  As for a comment on the API, I think it would be nice to have the
>> possibility to initiallize the SPI in the 3 following mode:
>> Master only - Leaving the SS pin as a general output
>> Master and Slave - Using the SS pin as input(whit possible pull-up
>> enabled) for slave selection
>> Slave - Well, you know.
>>
>> Whenever I make a project where the AVR is the master with only one
>> SPI slave, I tend to use the SS pin as a mean to select the SPI
>> device.Low count pin issue.
>>
>> I'm currently working on a API for the CAN controller, if that interrest
>> anyone.
>>
>> On Tue, Dec 8, 2009 at 4:44 AM, Ruddick Lawrence <address@hidden>
>> wrote:
>> > Hey everyone,
>> >
>> > I just put up the avr-corelib webpage
>> > (http://www.nongnu.org/avr-libc/avr-corelib/), which has the style
>> > guidelines we all hashed out a few months ago. It's pretty refined,
>> > except
>> > for how error messages work, but I think we'll have a better idea of
>> > that
>> > once we actually code a few modules, which bring us to....
>> >
>> > Our first modules! I think we should start with two modules so that it
>> > gives
>> > us an idea of the differing challenges and requirements that different
>> > functionality will require. We've already started talking a little about
>> > SPI, so let's make that one of the modules. Given that everyone has a
>> > lot of
>> > ideas, but little time, I would like to propose that the second module
>> > be
>> > anything (fairly simple) that someone is passionate about and would be
>> > willing to lead development on. It can range from something as simple as
>> > a
>> > ring buffer to more complex like an A/D module. If possible, I'd like to
>> > get
>> > some more people involved in this because I believe it will create a
>> > better
>> > library (so tell your friends!).
>> >
>> > A good jumping off point for the SPI library is Ron Kreymbourg's thread
>> > about an SPI implementation from this list
>> >
>> > (http://lists.gnu.org/archive/html/avr-libc-corelib/2009-09/msg00022.html).
>> > It's late now, but I'll work on getting out an API that we more-or-less
>> > agree on in the next few days.
>> >
>> > Happy coding!
>> > Ruddick
>> >
>>
>>
>>
>> --
>> Frédéric Nadeau ing. jr
>>
>>
>
>

--
Frédéric Nadeau ing. jr