EmbeddedRelated.com
Forums
The 2024 Embedded Online Conference

SPI with PDC: can't get ISR to fire

Started by Ken Smith January 13, 2010
On Thursday 14 January 2010 07:56:00 pm Ken Smith wrote:
[ ...]

> > Then why are you calling 'AT91F_SPI_ReceiveFrame' ?
>
> I was worried that, since SPI can only transfer and receive
> simultaneously, that having NULL pointers for the receive buffer would
> cause problems. Is this an invalid assumption?
>

Honestly I don't know. I always transmit and receive at the same time. If I
don't need transmit data after transfer I use same buffer for both TX and RX.

By looking at the datasheet it uses consistently the term "pointer registers"
so writing NULL should not break anything. By looking at the Linux code I can
verify this.

If you do not call 'AT91F_SPI_ReceiveFrame' at all, I guess you will get
overrun(OVRES bit in status register) but transmit will work.

Regards,
Caglar
--- In A..., Ken Smith wrote:
>
> On Wed, Jan 13, 2010 at 4:46 PM, Bernd Walter wrote:
>
> > Have you verified that SPI as such is working?
> > Yes. This is a rewrite for an existing driver that doesn't use DMA or
> interrupts. That driver works so I believe the SPI peripheral works.

IOW, you have _not_ verified SPI is working, you _believe_ it is.

Wasn't there a saying about assumptions being the mother of something? ;-)
Regards,

Stef

On Wed, Jan 20, 2010 at 2:41 AM, stef33d wrote:

> >
> > On Wed, Jan 13, 2010 at 4:46 PM, Bernd Walter wrote:
> >
> > > Have you verified that SPI as such is working?
> > >
> >
> > Yes. This is a rewrite for an existing driver that doesn't use DMA or
> > interrupts. That driver works so I believe the SPI peripheral works.
>
> IOW, you have _not_ verified SPI is working, you _believe_ it is.
>
> Wasn't there a saying about assumptions being the mother of something? ;-)
>

Well, a working driver that can talk to a peripheral however flawed
architecturally convinces me that the peripheral is basically working. That
is somewhat stronger than an assumption.

Thanks again for all the help, everyone. I now have a basically working
driver. Ultimately, the code I had actually worked, I just had to move it
later in the boot up sequence for a reason I still haven't yet ascertained.
(It seemed to need to be after configuring the watchdog.) But working is
working and I'm grateful for that.

Now I have another problem. I can write to the transmit buffer and the PDC
transfers the data. ENDTX causes the interrupt to fire but I can only do
this once. When I try to transmit again, the ISR doesn't fire. Here's the
present implementation. I'm no longer using the wrapper inline functions in
order to show more explicitly what I'm doing.

xSemaphoreHandle new_spi1_transmit_buffer_empty = 0;
void init(void)
{
vSemaphoreCreateBinary(new_spi1_transmit_buffer_empty);
*AT91C_PIOA_PDR AT91C_PA24_MISO1
| AT91C_PA23_MOSI1
| AT91C_PA22_SPCK1
| AT91C_PA21_NPCS10
| AT91C_PA25_NPCS11;
*AT91C_PIOA_BSR AT91C_PA24_MISO1
| AT91C_PA23_MOSI1
| AT91C_PA22_SPCK1
| AT91C_PA21_NPCS10
| AT91C_PA25_NPCS11;
*AT91C_PMC_PCER = 1 << AT91C_ID_SPI1;
*AT91C_SPI1_CR = AT91C_SPI_SWRST;
*AT91C_SPI1_CR = AT91C_SPI_SWRST; /* errata */
*AT91C_SPI1_CR = AT91C_SPI_SPIEN;
*AT91C_SPI1_MR (0x20 << 24) // DLYBCS
| (0 << 16) // PCS
| (1 << 7) // LLB
| (0 << 4) // MODFDIS
| (0 << 2) // PCSDEC
| (0 << 1) // PS
| (1 << 0) // MSTR
;
*AT91C_SPI1_IDR = 0x3f;
*(AT91C_SPI1_CSR + 0) (0x40 << 24) // DLYBCT
| (0x20 << 16) // DLYBS
| (MCK / 1000000) // SCBR
| (0 << 4) // BITS
| (0 << 3) // CSAAT TODO is this necessary?
| (0 << 1) // NCPHA
| (0 << 0) // CPOL
;
*AT91C_AIC_IDCR = 1 << AT91C_ID_SPI1;
*(AT91C_AIC_SVR + AT91C_ID_SPI1) = (unsigned long) spi_isr;
*(AT91C_AIC_SMR + AT91C_ID_SPI1) AT91C_AIC_SRCTYPE_INT_HIGH_LEVEL // SRCTYPE
| (7 << 0) // PRIOR
;
*AT91C_AIC_ICCR = 1 << AT91C_ID_SPI1;
*AT91C_AIC_IECR = 1 << AT91C_ID_SPI1;
}

void write(const char* buf, size_t count)
{
enum {transmit_buffer_size = 64};
static char transmit_buffer[transmit_buffer_size];
count = min(count, transmit_buffer_size);

xSemaphoreTake(new_spi1_transmit_buffer_empty, portMAX_DELAY);
memcpy(transmit_buffer, buf, count);

*AT91C_SPI1_PTCR = AT91C_PDC_TXTDIS;
*AT91C_SPI1_TPR = (unsigned long) transmit_buffer;
*AT91C_SPI1_TCR = count;
*AT91C_SPI1_IER = AT91C_SPI_ENDTX;
*AT91C_SPI1_PTCR = AT91C_PDC_TXTEN;
}

void spi_isr(void)
{
portSAVE_CONTEXT();
if (*AT91C_SPI1_SR & AT91C_SPI_ENDTX)
{
*AT91C_SPI1_IDR = AT91C_SPI_ENDTX;
xSemaphoreGiveFromISR(
new_spi1_transmit_buffer_empty,
pdFALSE
);
}
*AT91C_AIC_EOICR = 1;
portRESTORE_CONTEXT();
}

void task(void)
{
init();
const char command = 0xff;
write(&command, 1);
write(&command, 1);
write(&command, 1);
while (1) {}
}

After the first write, the ISR releases (gives) the semaphore and the second
write is able to set up the PDC but when it enables transmission, the ISR
doesn't go off and the PDC buffers don't move. The third call waits forever
on the semaphore. If I call init() between each write they all succeed but
that clearly shouldn't be necessary. But it is an interesting hint that I
need to do more to get the transfer to finish. I feel like I'm really
close. What am I missing? Does the PDC take care of LASTXFER
automatically? I tried setting it inside the ISR to no avail.

ks
On Wed, Jan 20, 2010 at 2:35 PM, Ken Smith wrote:

> | (MCK / 1000000) // SCBR
This needed to be left shifted by 8 to fit. I was setting SCBR to zero.
This and realizing that I couldn't set up the transmit buffer, write, then
set up the receive buffer and read were critical holes in my understanding
that prevented me from getting all this working. Transmit is receive and
receive is transmit. I get it now. I now have a working driver. Thanks
again for all the advice.

ks

The 2024 Embedded Online Conference