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

--- 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 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

On Friday 15 January 2010 12:19:24 Ken Smith wrote:
> On Thu, Jan 14, 2010 at 2:35 PM, Charles Manning
wrote:
> > > > 9) Is the CPU's interrupt enable set?
> >
> > Look at the CPU register.
>
> I was with you until this point. What is the CPU register on this
> platform?

cpsr (== current processor status register)

This will show up as one of the registers.

Do you have a debugger? If so, do whatever you need to see "show registers".

The I bit is bit 6.

>
> > > I don't know of a CPU-wide interrupt enable bit. Which register is it?
> > > The datasheet for my processor says the AIC is always powered on and
> > > clocked. Perhaps it is always enabled?
> >
> > No.
> > The AIC is an interrupt controller (a peripheral). This is **not** the
> > same as
> > the CPU.
>
> Yes, I understand that the AIC is a peripheral and distinct from the CPU.
> The reason I claim it is always powered on is this line from the
> datasheet.
>
> "The Advanced Interrupt Controller is continuously clocked. The Power
> Management Controller has no effect on the Advanced Interrupt Controller
> behavior."

While it may be on, enabled, and clocking, the AIC has various masks etc that
govern its behavior.
>
> > Hope that helps
> >
> >
> > Charles
>
> It does. Thank you, Charles.
>
> ks

On Thu, Jan 14, 2010 at 2:35 PM, Charles Manning wrote:

> > > 9) Is the CPU's interrupt enable set?
>
> Look at the CPU register.
>

I was with you until this point. What is the CPU register on this platform?
> > I don't know of a CPU-wide interrupt enable bit. Which register is it?
> > The datasheet for my processor says the AIC is always powered on and
> > clocked. Perhaps it is always enabled?
> No.
> The AIC is an interrupt controller (a peripheral). This is **not** the same
> as
> the CPU.
>

Yes, I understand that the AIC is a peripheral and distinct from the CPU.
The reason I claim it is always powered on is this line from the datasheet.

"The Advanced Interrupt Controller is continuously clocked. The Power
Management Controller has no effect on the Advanced Interrupt Controller
behavior."
> Hope that helps
>

> Charles
>

It does. Thank you, Charles.

ks

I was not really expecting a thread from this, but more suggesting a way of
partitioning the problem to hunt down what is happening.

For example, the double reset erratum would show up as the peripheral flags
not being correct.

Even though you are calling the code it is also worth verifying that the
registers actually hold what you expect. Sometimes a sequence of operations
can end up with something important being reset. As Lord Kelvin said: "To
measure is to know".

But, since you ask, see below:

On Friday 15 January 2010 08:04:26 Ken Smith wrote:
> On Wed, Jan 13, 2010 at 4:43 PM, Charles Manning
wrote:
> > 1) Are the flags that should trigger the interrupt set? If not, the
> > peripheral
> > is not getting to the state that triggers the interrupt (eg. buffer full)
>
> The SPI1 SR shows some of the end of transmission flags set. I'll go
> through them all to make sure they transition from unset to set.
>
> > 2) Are the enables set for those flags? If not, the peripheral will not
> > turn
> > these states into an interrupt.
>
> Yes, I've enabled all of the end of transmission and transmit data register
> empty interrupts.
>
> > 3) Is the peripheral interrupt flag set?
>
> I am enabling the peripheral interrupt like this.
>
> AT91F_AIC_EnableIt(AT91C_BASE_AIC, AT91C_ID_SPI1);
> AT91F_SPI_EnableIt(AT91C_BASE_SPI1, AT91C_SPI_TXBUFE | AT91C_SPI_TDRE
>
> | AT91C_SPI_TXEMPTY);
> |
> > 4) Is the interupt controller seeing the interrupt flag?
> > 5) Is the interrupt controller enabling that flag?
>
> How can I determine these?

Look at the various AIC registers.
AIC_ISR: which interrupt is currently being serviced.
AIC_IPR: Ipending register will tell you if the AIC has seen the interrupt.
AIC_IMR: mask register.
AIC_CISR: is the AIC trying to interrupt the core?

>
> > 6) Is the interrupt set up as level sensitive?
>
> Yes. I'm passing the flag AT91C_AIC_SRCTYPE_INT_HIGH_LEVEL
> to AT91F_AIC_ConfigureIt.

Check the relevant AIC_SMRx anyway.

>
> > 7) Is the interrupt controller stuck servicing a different interrupt?
>
> I don't believe so. How can I determine this?
AIC_ISR and AIC_CISR

>
> > 8) Is the interrupt controller interrupting the CPU?
>
> I don't know. How can I check?
AIC_CISR
>
> > 9) Is the CPU's interrupt enable set?

Look at the CPU register.

> I don't know of a CPU-wide interrupt enable bit. Which register is it?
> The datasheet for my processor says the AIC is always powered on and
> clocked. Perhaps it is always enabled?
No.
The AIC is an interrupt controller (a peripheral). This is **not** the same as
the CPU.

The AIC filters the system interrupt stream, then attempts to interrupt the
CPU.
The CPU only listens to the AIC if the CPU interrupt is enabled. If the I bit
in the CPSR is set then the interrupt is disabled.

Another debug thing to do is to use the AIC_ISCR to force the interrupt to
trigger in the AIC. If you see the ISR code fire then this means the the AIC
to CPU part is working and something is wrong in the peripheral set up.

Hope that helps

Charles

On Wed, Jan 13, 2010 at 4:43 PM, Charles Manning wrote:

> 1) Are the flags that should trigger the interrupt set? If not, the
> peripheral
> is not getting to the state that triggers the interrupt (eg. buffer full)
>
The SPI1 SR shows some of the end of transmission flags set. I'll go
through them all to make sure they transition from unset to set.

> 2) Are the enables set for those flags? If not, the peripheral will not
> turn
> these states into an interrupt.
>
Yes, I've enabled all of the end of transmission and transmit data register
empty interrupts.

> 3) Is the peripheral interrupt flag set?
>
I am enabling the peripheral interrupt like this.

AT91F_AIC_EnableIt(AT91C_BASE_AIC, AT91C_ID_SPI1);
AT91F_SPI_EnableIt(AT91C_BASE_SPI1, AT91C_SPI_TXBUFE | AT91C_SPI_TDRE
| AT91C_SPI_TXEMPTY);

> 4) Is the interupt controller seeing the interrupt flag?
> 5) Is the interrupt controller enabling that flag?
>
How can I determine these?

> 6) Is the interrupt set up as level sensitive?
>
Yes. I'm passing the flag AT91C_AIC_SRCTYPE_INT_HIGH_LEVEL
to AT91F_AIC_ConfigureIt.

> 7) Is the interrupt controller stuck servicing a different interrupt?
>
I don't believe so. How can I determine this?

> 8) Is the interrupt controller interrupting the CPU?
>
I don't know. How can I check?

> 9) Is the CPU's interrupt enable set?
>
I don't know of a CPU-wide interrupt enable bit. Which register is it? The
datasheet for my processor says the AIC is always powered on and clocked.
Perhaps it is always enabled?

ks

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.
> The X512 and maybe some X256 revisions need reset to be done twice.
> See Erata section in the datasheet for details.
> I miss setting the SPIEN Flag in CR register.
> Maybe this macro is doing two reset and SPIEN, but I would verify
> to be sure.
>
> Probably it is done with two reset
>

Thanks for pointing this out. Yes, I'm now resetting twice but I still
don't have the interrupts working.
> There are other SPI errata, which you might want to know about.
>

Good point. Most of the other errata involve configurations that I'm not
using but the reset one is definitely interesting.
> > AT91F_SPI_CfgMode(
> > AT91C_BASE_SPI1,
> > ((unsigned int) 0x20 << 24) | /* DLYBCS */
> > ((unsigned int) 0x0 << 16) | /* peripheral 0 */
> > AT91C_SPI_MSTR |
> > AT91C_SPI_PS_FIXED
> > AT91C_SPI_LLB
> > );
> > AT91F_SPI_CfgCs(
> > AT91C_BASE_SPI1,
> > 0,
> > 0 |
> > 0 |
> > AT91C_SPI_CSAAT |
> > AT91C_SPI_BITS_8 |
> > (MCK / 1000000) << 8 |
> > 0x20 << 16 |
> > 0x40 << 24
> > );
> > AT91F_SPI_Enable(AT91C_BASE_SPI1);
>
> I asume this macro is senabling clock for SPI1
> IIRC clock is required for reset to work.
>

The call to AT91F_SPI1_CfgPMC turns on the peripheral clock in my
implementation.
> I also miss setting up the GPIO pin to work with SPI1 functionality,
> but this would just produce a reasonless transfer.
>

The call to AT91F_SPI1_CfgPIO enables the appropriate PIO to work with the
SPI1 peripheral.
> > What I'm trying to do is just prove the technique so I can extend it
> > with a useful driver implementation. When I transmit, I don't care
> > about the received bytes and when I receive, I won't care about the
> > transmitted bytes. In the above code, I set up the receive buffer
> > just so the DMA controller has somewhere to put the bytes but if I can
> > get by without setting up the receive buffer during transmission, I
> > would prefer to do that.
>
> If you don't care about the data and also don't care about the sendbuffer
> being overwritten you can setup DMA for RX and TX to the same buffer.
> In case of large transfers this can save a lot of buffer space.
> If someone knows something like a /dev/null like memory area in
> AT91SAM7* and AT91RM9200 please drop me a note.
>

This is a good point. I have read elsewhere that you can disable reception
at the hardware level by turning the PIO line for MISO back into a GPIO.
This may be safer than using the same buffer for both transfer and receive.

ks

I'll try to address each of the excellent responses.

On Wed, Jan 13, 2010 at 10:39 PM, Caglar Akyuz
wrote:
> On Thursday 14 January 2010 01:20:42 am Ken Smith wrote:
>> I'm using AT91SAM7X256 (and 512) and am trying to use the SPI
>> peripheral with the PDC (DMA controller for the benefit of search
>> engines). When a transfer is finished, I want an ISR to fire to let
>> me know. My toolchain is ARM-ELF-GCC 4.1.1 with Newlib 1.14.0. Here
>> is the code. The ISR never fires. Can someone tell me why?
>> Bernd Walter's errata suggestion is a big possibility. Some minor comments:

I tried adding two resets. The other errata don't seem to affect my
setup. So far no ISR.

>> void spi1_isr(void) __attribute__((naked));
>>
>> void spi1_isr(void)
>> {
>> portSAVE_CONTEXT();
>> while (1)
>> {
>> // Just turn off the LED to prove that we got here.
>> LedOff(1);
>> }
>> AT91F_AIC_AcknowledgeIt(AT91C_BASE_AIC);
>> portRESTORE_CONTEXT();
>> }
>> FYI, C code is not allowed in 'naked' functions. You should wrap your C code
> in another not inlined function.
There are other naked functions that are implemented in C servicing
other interrupts in my code but I'll try your suggestion.

>> // configure and do the transfer.
>> AT91F_SPI1_CfgPIO();
>> AT91F_SPI1_CfgPMC();
>> AT91F_SPI_Reset(AT91C_BASE_SPI1);
>> AT91F_SPI_CfgMode(
>> AT91C_BASE_SPI1,
>> ((unsigned int) 0x20 << 24) | /* DLYBCS */
>> ((unsigned int) 0x0 << 16) | /* peripheral 0 */
>> AT91C_SPI_MSTR |
>> AT91C_SPI_PS_FIXED
>> AT91C_SPI_LLB
>> );
>> AT91F_SPI_CfgCs(
>> AT91C_BASE_SPI1,
>> 0,
>> 0 |
>> 0 |
>> AT91C_SPI_CSAAT |
>> AT91C_SPI_BITS_8 |
>> (MCK / 1000000) << 8 |
>> 0x20 << 16 |
>> 0x40 << 24
>> );
>> AT91F_SPI_Enable(AT91C_BASE_SPI1);
>> enum {transfer_buffer_size = 64};
>> char spi1_transmit_buffer[transfer_buffer_size];
>> char spi1_receive_buffer[transfer_buffer_size];
>> spi1_transmit_buffer[0] = (1 << 7) | 0x11;
>> portENTER_CRITICAL(); // FreeRTOS critical section. Probably unnecessary.
>> AT91F_PDC_Open(AT91C_BASE_PDC_SPI1);
>> AT91F_AIC_ConfigureIt(
>> AT91C_ID_SPI1,
>> AT91C_AIC_PRIOR_HIGHEST,
>> AT91C_AIC_SRCTYPE_INT_HIGH_LEVEL,
>> spi1_isr
>> );
>
> I think 'AT91F_AIC_ConfigureIt' does not touch AT91C_BASE_AIC->AIC_IECR so
> interrupt is enabled by this call. Our library versions are not the same so
> I'm not sure but checking your version may help.

Correct.

>> AT91F_SPI_EnableIt(
>> AT91C_BASE_SPI1,
>> AT91C_SPI_TXBUFE |
>> AT91C_SPI_ENDTX |
>> AT91C_SPI_TDRE |
>> AT91C_SPI_TXEMPTY
>> );

I incorrectly believed this enabled the interrupt in every way. I
have added this line:

AT91F_AIC_EnableIt(AT91C_BASE_AIC, AT91C_ID_SPI1);

immediately after AT91F_AIC_ConfigureIt.

> 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?

ks