Is there any limitation in the code size, in Events.c

cancel
Showing results for 
Show  only  | Search instead for 
Did you mean: 

Is there any limitation in the code size, in Events.c

Jump to solution
2,580 Views
B_Conf
Contributor I
Hi,
  I´m using CW6.1 and I`m working with 908AP32, I'm using two timers, the TimeBase, SPI, IIC, and SCI ports, also I´m using KBI interrupts.

The timers are used to launch SPI adcquisition. For each interrupt, clock on SPI pin is generated, and the SPDR register is filled with data output of an ADC 12bits (MCP3204).

The code of one of the interrupts is next.

I can see the clock on SPI pin, so the code is working, but I notice that the size, for example, in KBI interrupt produce a strange execution.
The problem is all information saved (if it is saved!) in Datos[x][y].wordsep.alto/bajo, it isn´t!.
For those reasons, I`m asking if  is there any limitation, and why is it?

Please, I will appreciate any suggestion!

Thanks in advance!

B_Conf

void Muestreo_C_fase_OnInterrupt(void)
{

     byte aux1;
   
        
    clrReg8Bits(PTA, CS_3204Ten);        
   
    asm
    {                                                                                 //258 ciclos `por simulación
       LDHX #@aux                                                          //#C=3
       adquiere:
           mov #$00,SPDR                                                //#C=4
       reviso: brclr 7,SPSCR,reviso                                   //#C=5    
           lda SPDR                                                           //#C=3 cargo spdr en a, fijate que uso el index arriba no ACCA
           sta ,X                                                                  //#C=2 después cargo A en la posición aux+Index
           incx                                                                    //#C=1
           cpx #@aux+3                                                     //#C=2
           bne adquiere                                                     //#C=3
                                                                                     //total= 23*3=69 ciclos de reloj
    }
   
    setReg8Bits(PTA, CS_3204Ten);                             //Deshabilito A/D de tensión y pongo el buffer en Tri State 
    
    aux[2]=aux[2]>>5;                                                   //#15
    aux1=aux[1];                                                           //#6
    aux1=aux1<<3;                                                      //#15
    aux[2]=aux[2]|aux1;                                                //#7
    aux[1]=aux[1]>>5;
    aux[0]=aux[0]<<3;
    aux[1]=aux[1]|aux[0]|0xF0;                                      //0xF0: cuando niego el dato queda bien!.
      
    Datos[0][ind].wordsep.alto= aux[1];
    Datos[0][ind].wordsep.bajo=aux[2];
 
 //  ADQUISICIÖN DE CORRIENTE  
  
    clrReg8Bits(PTA, CS_3204Cor);
    asm
    {                                                                                   //258 ciclos `por simulación
       LDHX #@aux                                                            //#C=3
       adquiereI:
         mov #$00,SPDR                                                     //#C=4
       revisoI: brclr 7,SPSCR,revisoI                                   //#C=5    
         lda SPDR                                                               //#C=3 cargo spdr en a, fijate que uso el index arriba no ACCA
         sta ,X                                                                     //#C=2 después cargo A en la posición aux+Index
         incx                                                                       //#C=1
         cpx #@aux+3                                                        //#C=2
         bne adquiereI                                                      //#C=3
                                 
    }
    setReg8Bits(PTA, CS_3204Cor);
    //Control_Placas_PutBit(6,1);  //Deshabilito A/D de corriente
   
    aux[2]=aux[2]>>5;                                                       // #15
    aux1=aux[1];                                                                //   #6
    aux1=aux1<<3;                                                           //#15
    aux[2]=aux[2]|aux1;                                                     //#7
    aux[1]=aux[1]>>5;
    aux[0]=aux[0]<<3;
    aux[1]=aux[1]|aux[0]|0xF0;                                        //0xF0: cuando niego el dato queda bien!.
 
    Datos[1][ind].wordsep.alto=aux[1];        
    Datos[1][ind].wordsep.bajo=aux[2];                            // Evita que se vaya de rango.
    ind++;
    if(ind==260)
   {
        aux1=Muestreo_C_fase_Disable();                        /*DESHABILITO INTERRUPCIÓN POR timer*/
        Cpu_Delay100US(1000);
        ind=0;
        aux1=AD3204_Disable();
        Cpu_Delay100US(1000);
   }


}    
      

Labels (1)
Tags (1)
0 Kudos
Reply
1 Solution
1,189 Views
bigmac
Specialist III
Hello,
 
I can see some problems with your inline assembly code, in those areas where there are differences between conventional assembly and inline assembly, namely the instructions that refer to @aux operand.  However, I can't see that the use of inline code will give significant benefit, and might be replaced by the following C code -
 
byte i;
 
for (i = 0; i < 3; i++) {
   SPDR = 0;            /* Send dummy byte */
   while (!SPSCR_SPRF); /* Wait for receive flag set */
   aux[i] = SPDR;
}
 
A secondary issue, that won't affect the operation of the code, but will streamline its presentation - the following code:
 
aux[2] = aux[2] >> 5;
aux1 = aux[1];
aux1 = aux1 << 3;
aux[2] = aux[2] | aux1;
aux[1] = aux[1] >> 5;
aux[0] = aux[0] << 3;
aux[1] = aux[1] | aux[0] | 0xF0; 
 
might be replaced by -
 
aux[2] >>= 5;
aux[2] |= (aux[1] << 3);
aux[1] >>= 5;
aux[1] |= (aux[0] << 3) | 0xF0; 
 
Regards,
Mac
 

View solution in original post

0 Kudos
Reply
5 Replies
1,190 Views
bigmac
Specialist III
Hello,
 
I can see some problems with your inline assembly code, in those areas where there are differences between conventional assembly and inline assembly, namely the instructions that refer to @aux operand.  However, I can't see that the use of inline code will give significant benefit, and might be replaced by the following C code -
 
byte i;
 
for (i = 0; i < 3; i++) {
   SPDR = 0;            /* Send dummy byte */
   while (!SPSCR_SPRF); /* Wait for receive flag set */
   aux[i] = SPDR;
}
 
A secondary issue, that won't affect the operation of the code, but will streamline its presentation - the following code:
 
aux[2] = aux[2] >> 5;
aux1 = aux[1];
aux1 = aux1 << 3;
aux[2] = aux[2] | aux1;
aux[1] = aux[1] >> 5;
aux[0] = aux[0] << 3;
aux[1] = aux[1] | aux[0] | 0xF0; 
 
might be replaced by -
 
aux[2] >>= 5;
aux[2] |= (aux[1] << 3);
aux[1] >>= 5;
aux[1] |= (aux[0] << 3) | 0xF0; 
 
Regards,
Mac
 
0 Kudos
Reply
1,189 Views
B_Conf
Contributor I
Hello bigmac:
    Thank you for the response!.
    The assembly code is because, the option that you have mentioned, consumes more Clock cycles and I need reduce that because the interruption occurs  every 200uS, and the bus frequency is  7.3728MHz.
However I appreciate your suggestion, On the other hand, I'm going to replace the other code, I learn C by programming this application, and I have several mistakes of this kind.

Thank you

B_Conf
0 Kudos
Reply
1,189 Views
bigmac
Specialist III
Hello,
 
I have now examined the datasheet for the MCP3204 device, and am of the opinion that you are not using the correct data format.  Fig. 6.1 and Fig. 6.2 provide the information for the SPI interface. 
  1. The first byte sent will need the start bit and the single/diff bit correctly positioned, and the return data would be ignored.
  2. The second byte sent will need the two channel select bits positioned at the two most significant positions.  The lower four bits of the return byte will contain the MS four bits of the 12-bit reading.
  3. The third byte sent will be a dummy byte of non-specific value, and the return byte will contain the remaining bits of the 12-bit reading.
With a bus clock of 7.3728 MHz, and a timer interrupt period of 200 microseconds, this will correspond to 1474 bus cycles between interrupts.  The ISR processing should take considerably less than this number of cycles, to allow for other activities to occur.  In fact all other ISRs should also take considerably less than that number of cycles, so that the critical timer interrupt may occur with minimal latency.
 
With the current ISR code, the code enters a tight (5-cycle) loop, waiting for the SPRF flag to become set.  The period within the loop will therefore be a 5-cycle multiple.  The SPI clock should be as fast as possible. However, the minimum available division ratio is 2, and the next ratio is 8.  A division ratio of 2 would put the SPI clock at 3.69 MHz, which is too high for the ADC device, so a division ration of 8 must be used.  This will give a SPI clock of 0.92 MHz, which is one half the maximum rate of the ADC.  So each byte transfer will take 64 bus cycles, and the wait loop will occupy 65 cycles.
 
I notice that you are storing the data in a two-dimensional array of structures.  Handling this would seem to require an excessive number of cycles.  By extending the structure, it is possible to have a single dimension array of structures that will be considerably more efficient to handle.
 
The following code addresses some of these issues, and requires about 656 cycles to execute, occupying about 45 percent of the available time.
 
#define clrReg8Bits(reg,bit)  __asm bclr (bit),(reg)
#define setReg8Bits(reg,bit)  __asm bset (bit),(reg)
 
#define CS_3204  7
#define START    0x06  /* Single mode */
 
typedef union {
  word value;
  byte val[2];
} two_byte;
 
typedef struct {
  two_byte dat0;
  two_byte dat1;
} two_val;
 
byte ind;
byte chan;
two_val Datos[10];
 
byte SPI_trans( byte val);  /* Function prototype */
 
/****************************************************************************/
/* TIM Ch0 interrupt (output compare) */
interrupt 5 void Muestreo_C_fase_OnInterrupt(void)
{                                       // [13] ISR entry
   T1SC0_CH0F = 0;                      // [4]  Clear flag
   T1CH0 += INCRVAL;                    // [21]  Set next interval
 
   if (ind < 10) {                      // [9]
      clrReg8Bits(PTA, CS_3204);        // [4]      
 
      (void)SPI_trans( START);          // [37 or 82<-]
      Datos[ind].dat0.val[0] =
         SPI_trans( chan << 6) & 0x0F;  // [63 or 108<-]       
      Datos[ind].dat0.val[1] =
         SPI_trans( 0);                 // [50 or 95<-]
 
      setReg8Bits(PTA, CS_3204);        // [4]
      __asm nop; __asm nop;             // [2]
      setReg8Bits(PTA, CS_3204);        // [4]
 
      (void)SPI_trans( START);          // [37 or 82<-]
      Datos[ind].dat1.val[0] =
         SPI_trans(( chan + 1) << 6) & 0x0F; // [63 or 108<-]
      Datos[ind].dat1.val[1] =
         SPI_trans( 0);                 // [50 or 95<-]
 
      setReg8Bits(PTA, CS_3204);        // [4]
      ind++;                            // [10]
   }
}                                       // [11] ISR exit
 
/****************************************************************************/
/* SPI transfer */
byte SPI_trans( byte val)
{
   SPDR = val;
   while (!SPSCR_SPRF);  /* Wait for transfer complete */
   return SPDR;
}                        // Total cycles: 15+(5*n)
                         // SPI_clk divisor = 2:  n = 4
                         // SPI_clk divisor = 8:  n = 13  /*
 
For each 200 microsecond interrupt, you are processing two ADC readings.  The ISR execution time would be more managable if only a single reading were processed for each interrupt.
 
It is possible to utilize the SPI interrupt to avoid waiting for each SPI transfer to complete.  However, the coding would be more complex.  With minimum ISR entry/exit overhead of 28 cycles, and the more complex code, any savings would probably be quite minimal.
 
Regards,
Mac
 
0 Kudos
Reply
1,189 Views
B_Conf
Contributor I
Hello BigMac:
  I'm really sorry, I can't answer you before, but I saw all your suggestions and I saw a "couple" of things to improve the code. In fact, I didn't expect so many details and dedication, so, I'm sorry again, and thank you very much.
  You are right when you say that with that code the MCP3204 will not work, but I'm using the ADC by Optocouplers and to save space on the board I connect directly the "Data in" Pin to Vcc, for that reason, I have to read three bytes. The other thing is that I can't use a 0.92MHz SPI Clk, because of the optocouplers, and I had to use 0.234 MHz SPI Clk.
However I learned several things, and when it is functioning the adquistion, I´m going to send the code.

Best regards
Pablo.
0 Kudos
Reply
1,189 Views
bigmac
Specialist III
Hello Pablo,
 
If you are stuck with a SPI clock of 234 kHz, I can only suggest that there will be insufficient time for two ADC readings for a TIM interrupt every 200 us.  Using the sample code that I posted, the ISR processing period would increase from 656 cycles to 1826 cycles (248 us).  For a single ADC reading per interrupt, the ISR processing period would be 946 cycles (128 us), or 64 percent of the available time.
 
With the much slower SPI clock rate, you would potentially get greater benefit using SPI interrupts for the completion of each data transfer.  However, to avoid the additional complexity and overheads needed for nested interrupts, the communications with the ADC device would need to occur outside of the ISR code for the TIM channel.
 
Regards,
Mac
 
0 Kudos
Reply