-2

I am trying to read ADC(ADS8320) values using SPI communication. I am using IAR embedded workbench for ARM and SM470R1B1 controller. In the data sheet it says first 6 bits are dummy read and next 16 bits is the actval data. I am trying to read 24 bits and ignore first 6 bits and last 2 bits. I am getting wrong values when i try with the below code.

unsigned int readADC8320(void)  
{                 
   value = AD8320_16(0xFFFFFF);              // read registe    
   return value;
}

unsigned int AD8320_16(unsigned int value) 
{ 
unsigned int data; 
AD8320_CS_Status(0);
SW_Delay(DELAY_10US);
while(GIODOUTA&X2);
data = spi2(value >> 16);     //read high 8 bits
data = (data << 6)| spi2(value >> 8);   //read next 8 bits but 6+8 =14
data = (data << 8)| spi2(value >> 2);   //add last two bits only
SW_Delay(DELAY_10US);
AD8320_CS_Status(1);
return data; 
} 

unsigned char spi2(unsigned char data)
 {
// Write byte to SPI2DAT1 register
SPI2DAT1 = data;    

while(!(SPI2CTRL3 & 0x01)){}        // Wait for RxFlag to get set   
return (SPI2BUF & 0x000000FF);      // Read SPIBUF 
  }

Can anyone suggest me where i am doing wrong. I am quite poor with shift operations.

verendra
  • 223
  • 3
  • 18
  • 2
    Start with describing your system. We don't even know what is the CPU/MCU you are working with. – Eugene Sh. Aug 11 '15 at 15:21
  • You might be quite poor with shift operations, but you are very poor at asking questions and, presumably, testing and debugging. When you stepped through this code with your debugger, what was passed as 'value' and how was the data processed into 'data'? – Martin James Aug 11 '15 at 15:28
  • I mean, why just dump such hardware driver stuff on us as just a code dump? We have no clue whether 'value' is sane, inverted, correct endinanness or what. For all we know, your ADC is not even powered up and you are processing garbage. – Martin James Aug 11 '15 at 15:33
  • Is `unsigned int` 16-bit or 32-bit? – chux - Reinstate Monica Aug 11 '15 at 16:58

1 Answers1

2

Your code looks wonky. For example, what purpose does value have?

The datasheet you linked to indicates that the 22 clock cycles suffice (and that 24 clock cycles are okay), no extra delays are needed, assuming the SPI clock is within limits (at most 2.4 MHz, I believe). So, I'd expect your code to look more like

void AD8320_setup(void)
{
    /* TODO:
     *  - Set SPI2 clock frequency, if programmable (max. 2.4 MHz)
     *  - Set SPI2 transfer size, if programmable (8 bits per byte)
     *  - Set SPI2 to latch data on the falling edge of clock pulse
     *  - Set SPI2 to pull clock high when inactive
     *  - Set AD8320 chip select pin as an output
     *  - Set AD8320 chip select pin high (it is active low)
     *  - Ensure AD8320 is powered 
    */
}

uint16_t AD8320_16(void)
{
    uint16_t result;

    /* TODO: Set AD8320 chip select pin 0/LOW (to select it)
    */

    /* TODO: Clear SPI2 FIFO if it has one,
     *       so that we get actual wire data,
     *       not old cached data.
    */

    result = ((uint16_t)(spi2_read() & 3U)) << 14;
    result |= ((uint16_t)spi2_read()) << 6;
    result |= ((uint16_t)spi2_read()) >> 2;

    /* TODO: Set AD8320 chip select pin 1/HIGH (to deselect it)
    */

    return result;
}

The prototype for spi2_read is typically unsigned char spi2_read(void);, but it might be of any integer type that can represent all eight bit values correctly (0 to 255, inclusive). Above, I cast the result -- after masking out any irrelevant bits, if any, with a binary AND operation -- to uint16_t before shifting it into the correct position in the result, so that regardless of the spi2_read() return type, we can use it to construct our 16-bit value.

The first spi2_read() reads the first 8 bits. I am assuming your device SPI bus sends and receives the most significant bit first; this means that the 6 dummy bits are most significant bits, and the 2 least significant bits are the most significant bits of the result. That's why we only keep the two least significant bits of the first byte, and transfer it 14 places left -- that way we get them in the most significant position.

The second spi2_read() reads the next eight bits. These are bits 13..6 of the data; that's why we shift this byte left by 6 places, and OR with the existing data.

The last spi2_read() reads the next eight bits. The last two bits are to be discarded, because only the first (most significant) 6 bits contain the rest of the data we're interested in. So, we shift this down by 2 places, and OR with the existing data.

Preparing spi2 involves stuff like word size (8 bits), data rate (if programmable), and so on.

The chip select for the AD8320 is just a generic output pin, it's just active low. That's why it's described as ^CS/SHDN in the datasheet: chip selected when low, shutdown when high.

The chip is a micropower one, so it requires less than 2mA, so you could conceivably power it from an output pin on many microcontrollers. If you were to do so, you'd also have to remember to make that pin an output and 1/high in _setup() function.

Nominal Animal
  • 38,216
  • 5
  • 59
  • 86
  • Picky remark about type correctness: for the cast to have any effect, `((uint16_t)(spi2_read() & 3U)) << 14;` should be `( (uint16_t)spi2_read() & 3U) << 14;` The cast to `uint16_t` should be inside the parenthesis, before any implicit integer promotions take place. – Lundin Aug 12 '15 at 08:55
  • @Lundin: Oops! Actually, my wording in that paragraph was atrocious. I wrote that I *"cast the result value"* (of `spi2_read()`) to `uint16_t`, but I meant that I cast the return value, after masking, before shifting. I'll try and fix that wording. I'm happy to let the compiler choose the best way to handle the bit masking here, as long as the value being bit shifted is between 0 and 3, inclusive, and of `uint16_t` type. After all, `spi2_read()` must be able to return unsigned or signed integers 0 to 255, inclusive, and the cast is only needed to ensure we can construct the 16-bit result. – Nominal Animal Aug 12 '15 at 11:44