View previous topic :: View next topic |
Author |
Message |
Guest
|
faster bit_set and bit_clear? |
Posted: Tue Sep 29, 2009 10:09 am |
|
|
Hi,
Is there a faster way to do this:
Code: | for (i=12;i;) {
mcp3201_clk = 1;
i--;
if (mcp3201_dout) bit_set(read,i); else bit_clear(read,i);
mcp3201_clk = 0;
} |
I want to keep the timing the same for each "read", hence why there is a bit_clear instead of a read = 0 at the beginning. That one threw me for a while!
It's still quite slow, even at 40MHz. mcp3201_clk, dout are #bit'ed to the port address.
Any ideas much appreciated. |
|
|
treitmey
Joined: 23 Jan 2004 Posts: 1094 Location: Appleton,WI USA
|
|
|
John P
Joined: 17 Sep 2003 Posts: 331
|
|
Posted: Tue Sep 29, 2009 3:08 pm |
|
|
A computed bit-set operation is very slow.
i.e. find a way to make
bit_set(read,i);
become
bit_set(read,3);
Why are you doing this instead of setting/clearing the bottom bit of the variable and then rotating it? Or actually, rotating first and then setting/clearing, because the last bit you'll see is bit 0 and you don't want any rotation after that. Also, start with the variable 'read' set to zero, so there's no need to clear any bit.
Something like (if I'm getting it right):
Code: |
read = 0;
for (i = 12; i; i--)
{
mcp3201_clk = 1;
read <<= 1;
if (mcp3201_dout)
bit_set(read, 0);
mcp3201_clk = 0;
}
|
And yes, if you want to be as fast as possible, don't use a loop at all. Just a sequence of 12 blips to the clock line interspersed with tests of the data line and setting bits.
Note that testing a bit and setting a different bit if it's high (or low) compiles to only 2 assembly instructions. |
|
|
asmboy
Joined: 20 Nov 2007 Posts: 2128 Location: albany ny
|
another way |
Posted: Tue Sep 29, 2009 6:59 pm |
|
|
This is a core fragment of how I read that type of device - even though it was coded for a 16 BIT TI/BB ADC:
Code: |
// ADS8320,25,26 +16 bits unipolar A/D
// ADS8317 is +/- 15 bits
// #define ADCclock PIN_E2
// #define ADCstart PIN_E0
// #define ADCdata PIN_E1
int16 adcresult;
for (i=0;i<15;i++){
output_high(ADCclock); // now after rising edge
ADC_result |= input(ADCdata); // OR input pin with 16bit int
output_low(ADCclock); // produce a new lo-to-high clock
ADC_result<<1; // and shift that bit allong toward MSB
}
// get the last LSB but NO shift
output_high(ADCclock);
ADC_result |= input(ADCdata); // OR input pin with 16bit int
output_low(ADCclock);
|
There is obviously other before and after stuff
but this gets to your question I think. |
|
|
Guest
|
|
Posted: Wed Sep 30, 2009 7:32 am |
|
|
Thanks. Interesting reading. I will investigate your ideas. I will also time the options ideas, the shift idea is elegant.
John, I used to set read = 0 like you suggest but spent rather a long time racking my brain as to why my program slowed down the larger the voltage presented to the ADC! Finally figured that set_bit takes quite a bit of time and the more "1"'s in the data stream the slower the procedure took! I really want a nice constant time no matter what the contents of the data. |
|
|
asmboy
Joined: 20 Nov 2007 Posts: 2128 Location: albany ny
|
|
Posted: Wed Sep 30, 2009 7:45 am |
|
|
i forgot to mention - you of course need to do
ADCresult=0;
before starting the read .
also - the way it is structured - ADC clock has a decently even duty
cycle based on the disitribution of the actual instructions. |
|
|
Guest
|
|
Posted: Wed Sep 30, 2009 7:57 am |
|
|
Code: | .................... for (i=0;i<11;i++) {
033C: CLRF 64
033E: MOVF 64,W
0340: SUBLW 0A
0342: BNC 0356
.................... mcp3201_clk = 1;
0344: BSF F82.3
.................... delay_cycles(1);
0346: NOP
.................... read |= mcp3201_dout;
0348: MOVLW 00
034A: BTFSC F82.4
034C: MOVLW 01
034E: IORWF 4D,F
.................... mcp3201_clk = 0;
0350: BCF F82.3
.................... read<<1;
.................... } |
The compiler warns that "Code has no effect" for the read<<1 line and indeed no code is generated for it? Strange? PIC18F2520 compiled with 4.099. Any ideas? |
|
|
Guest
|
|
Posted: Wed Sep 30, 2009 8:02 am |
|
|
no assignment!
|
|
|
Guest
|
|
Posted: Wed Sep 30, 2009 8:16 am |
|
|
I've moved the shift to get rid of the delay_cycles(1), needed as I am not using the compilers auto TRIS function.
Code: | .................... for (i=0;i<12;i++) {
033C: CLRF 64
033E: MOVF 64,W
0340: SUBLW 0B
0342: BNC 035A
.................... mcp3201_clk = 1;
0344: BSF F82.3
.................... read<<=1;
0346: BCF FD8.0
0348: RLCF 4D,F
034A: RLCF 4E,F
.................... read |= mcp3201_dout;
034C: MOVLW 00
034E: BTFSC F82.4
0350: MOVLW 01
0352: IORWF 4D,F
.................... mcp3201_clk = 0;
0354: BCF F82.3
.................... } |
The advantage of this is all 12 bits can be clocked into read within the for loop (the down side is the first shift is wasted).
It looks like if I get rid of the for loop completely and make the code "messy" I will save 4 instructions each bit read! |
|
|
|