AVR-Chat

--- In AVR-Chat@yahoogroups.com, "David VanHorn" <microbrix@...> wrote:
>
> Is there a reason that I can't use a char as a pointer in an array
of ints?

An actual pointer you can't but it looks like the pointer you're
referring to is just an index so yes you can use a char but most
compilers are going to promote a char to an integer when doing
arithmetic. You can sometimes override the promotion but it can get
ugly. Usually you need to add "& 0xff" and char casts to do this.

Mike H.

Is there a reason that I can't use a char as a pointer in an array of ints?
My ADC experiment has an array of 8 ints to store the data, and it
does grind my gears to use a 16 bit pointer into an 8 element array.

In ASM, I'd only use three bits of a byte, saving the other five for
flags, and absolutely preventing indexing outside the array, but I
haven't worked out how to do that in C yet.

Also, I did look at the compiled version of my ISR... Eww.

I'm open to the idea that I'm not writing it in the right way for the
compiler to be efficient yet, but I was impressed with how many
instructions it took to do the ISR.

Started with this code:

// Reads all 8 channels even though we only use two.
ISR(ADC_vect)
{	// Store in array of locations per channel flag
	
	// Read the channel data MUST BE IN LOW-HIGH order!
	// low byte of ADC_Reading = ADCL
	// high byte of ADC_Reading = ADCH
	ADC_Data[ADC_Pointer] = ADCW;          // ADC_Reading;

	// Select the next channel
	ADC_Pointer++;  						//Only I ever increment this!
	if (ADC_Pointer > 7) ADC_Pointer=0;
	
	ADMUX = ADMUX & 0xF8;					// Save the other bits
	ADMUX = ADMUX |= ADC_Pointer;			// Copy the pointer into ADMUX
	ADCSRA |= (1<<ADSC);					// Start the next conversion
}


And got this with maximum optimization for size:

// Reads all 8 channels even though we only use two.
ISR(ADC_vect)
{	// Store in array of locations per channel flag
  de:	1f 92       	push	r1
  e0:	0f 92       	push	r0
  e2:	0f b6       	in	r0, 0x3f	; 63
  e4:	0f 92       	push	r0
  e6:	11 24       	eor	r1, r1
  e8:	2f 93       	push	r18
  ea:	3f 93       	push	r19
  ec:	8f 93       	push	r24
  ee:	9f 93       	push	r25
  f0:	ef 93       	push	r30
  f2:	ff 93       	push	r31
	
	// Read the channel data MUST BE IN LOW-HIGH order!
	// low byte of ADC_Reading = ADCL
	// high byte of ADC_Reading = ADCH
	ADC_Data[ADC_Pointer] = ADCW;          // ADC_Reading;
  f4:	20 91 88 00 	lds	r18, 0x0088
  f8:	30 91 89 00 	lds	r19, 0x0089
  fc:	f9 01       	movw	r30, r18
  fe:	e2 0f       	add	r30, r18
 100:	f3 1f       	adc	r31, r19
 102:	e6 57       	subi	r30, 0x76	; 118
 104:	ff 4f       	sbci	r31, 0xFF	; 255
 106:	84 b1       	in	r24, 0x04	; 4
 108:	95 b1       	in	r25, 0x05	; 5
 10a:	91 83       	std	Z+1, r25	; 0x01
 10c:	80 83       	st	Z, r24

	// Select the next channel
	ADC_Pointer++;  						//Only I ever increment this!
 10e:	c9 01       	movw	r24, r18
 110:	01 96       	adiw	r24, 0x01	; 1
 112:	90 93 89 00 	sts	0x0089, r25
 116:	80 93 88 00 	sts	0x0088, r24
	if (ADC_Pointer > 7) ADC_Pointer=0;
 11a:	08 97       	sbiw	r24, 0x08	; 8
 11c:	20 f0       	brcs	.+8      	; 0x126 <__vector_16+0x48>
 11e:	10 92 89 00 	sts	0x0089, r1
 122:	10 92 88 00 	sts	0x0088, r1
	
	ADMUX = ADMUX & 0xF8;					// Save the other bits
 126:	87 b1       	in	r24, 0x07	; 7
 128:	88 7f       	andi	r24, 0xF8	; 248
 12a:	87 b9       	out	0x07, r24	; 7
	ADMUX = ADMUX |= ADC_Pointer;			// Copy the pointer into ADMUX
 12c:	87 b1       	in	r24, 0x07	; 7
 12e:	90 91 88 00 	lds	r25, 0x0088
 132:	89 2b       	or	r24, r25
 134:	87 b9       	out	0x07, r24	; 7
 136:	87 b1       	in	r24, 0x07	; 7
 138:	87 b9       	out	0x07, r24	; 7
	ADCSRA |= (1<<ADSC);					// Start the next conversion
 13a:	36 9a       	sbi	0x06, 6	; 6
 13c:	ff 91       	pop	r31
 13e:	ef 91       	pop	r30
 140:	9f 91       	pop	r25
 142:	8f 91       	pop	r24
 144:	3f 91       	pop	r19
 146:	2f 91       	pop	r18
 148:	0f 90       	pop	r0
 14a:	0f be       	out	0x3f, r0	; 63
 14c:	0f 90       	pop	r0
 14e:	1f 90       	pop	r1
 150:	18 95       	reti


Normally in any app I write in ASM, I dedicate two registers for usage
in the ISR,  ITEMP and ITEMP2, and one to hold SREG, "STEMP"
So getting into this would be something like:

ADC_ISR:
        in STEMP,SREG

     code...

        out SREG,STEMP
     reti


-- 

"The very powerful and the very stupid have one thing in common. Instead of
altering their views to fit the facts, they alter the facts to fit their
views... which can be very uncomfortable if you happen to be one of the
facts that needs altering." Doctor Who, Face of Evil

David
   I do not have an AVR compiler accessible to me right now but please 
let me offer an alternative piece of code.  Also, I did not see your 
declaration of 'ADC_Pointer' which is actually an index, not a pointer.

unsigned char ADC_index;

ISR(ADC_vect)
{
   ADC_Data[ADC_index] = ADCW;

   ADC_index = ((ADC_index + 1) & 7);
   ADMUX = (ADMUX & 0xF8) | ADC_index;
   ADCSRA |= (1 << ADSC);
}

   If you have the time, please give this a try and let me know how (if) 
it works out any better for you.

-Bill Knight
R O Software.

PS - ADC_index declared as an 'unsigned int' (instead of an 'unsigned 
char') might actually produce smaller code.


David VanHorn wrote:

> Is there a reason that I can't use a char as a pointer in an array of ints?
> My ADC experiment has an array of 8 ints to store the data, and it
> does grind my gears to use a 16 bit pointer into an 8 element array.
> 
> In ASM, I'd only use three bits of a byte, saving the other five for
> flags, and absolutely preventing indexing outside the array, but I
> haven't worked out how to do that in C yet.
> 
> Also, I did look at the compiled version of my ISR... Eww.
> 
> I'm open to the idea that I'm not writing it in the right way for the
> compiler to be efficient yet, but I was impressed with how many
> instructions it took to do the ISR.
> 
> Started with this code:
> 
> // Reads all 8 channels even though we only use two.
> ISR(ADC_vect)
> {	// Store in array of locations per channel flag
> 	
> 	// Read the channel data MUST BE IN LOW-HIGH order!
> 	// low byte of ADC_Reading = ADCL
> 	// high byte of ADC_Reading = ADCH
> 	ADC_Data[ADC_Pointer] = ADCW;          // ADC_Reading;
> 
> 	// Select the next channel
> 	ADC_Pointer++;  						//Only I ever increment this!
> 	if (ADC_Pointer > 7) ADC_Pointer=0;
> 	
> 	ADMUX = ADMUX & 0xF8;					// Save the other bits
> 	ADMUX = ADMUX |= ADC_Pointer;			// Copy the pointer into ADMUX
> 	ADCSRA |= (1<<ADSC);					// Start the next conversion
> }
> 
> 
> And got this with maximum optimization for size:
> 
> // Reads all 8 channels even though we only use two.
> ISR(ADC_vect)
> {	// Store in array of locations per channel flag
>   de:	1f 92       	push	r1
>   e0:	0f 92       	push	r0
>   e2:	0f b6       	in	r0, 0x3f	; 63
>   e4:	0f 92       	push	r0
>   e6:	11 24       	eor	r1, r1
>   e8:	2f 93       	push	r18
>   ea:	3f 93       	push	r19
>   ec:	8f 93       	push	r24
>   ee:	9f 93       	push	r25
>   f0:	ef 93       	push	r30
>   f2:	ff 93       	push	r31
> 	
> 	// Read the channel data MUST BE IN LOW-HIGH order!
> 	// low byte of ADC_Reading = ADCL
> 	// high byte of ADC_Reading = ADCH
> 	ADC_Data[ADC_Pointer] = ADCW;          // ADC_Reading;
>   f4:	20 91 88 00 	lds	r18, 0x0088
>   f8:	30 91 89 00 	lds	r19, 0x0089
>   fc:	f9 01       	movw	r30, r18
>   fe:	e2 0f       	add	r30, r18
>  100:	f3 1f       	adc	r31, r19
>  102:	e6 57       	subi	r30, 0x76	; 118
>  104:	ff 4f       	sbci	r31, 0xFF	; 255
>  106:	84 b1       	in	r24, 0x04	; 4
>  108:	95 b1       	in	r25, 0x05	; 5
>  10a:	91 83       	std	Z+1, r25	; 0x01
>  10c:	80 83       	st	Z, r24
> 
> 	// Select the next channel
> 	ADC_Pointer++;  						//Only I ever increment this!
>  10e:	c9 01       	movw	r24, r18
>  110:	01 96       	adiw	r24, 0x01	; 1
>  112:	90 93 89 00 	sts	0x0089, r25
>  116:	80 93 88 00 	sts	0x0088, r24
> 	if (ADC_Pointer > 7) ADC_Pointer=0;
>  11a:	08 97       	sbiw	r24, 0x08	; 8
>  11c:	20 f0       	brcs	.+8      	; 0x126 <__vector_16+0x48>
>  11e:	10 92 89 00 	sts	0x0089, r1
>  122:	10 92 88 00 	sts	0x0088, r1
> 	
> 	ADMUX = ADMUX & 0xF8;					// Save the other bits
>  126:	87 b1       	in	r24, 0x07	; 7
>  128:	88 7f       	andi	r24, 0xF8	; 248
>  12a:	87 b9       	out	0x07, r24	; 7
> 	ADMUX = ADMUX |= ADC_Pointer;			// Copy the pointer into ADMUX
>  12c:	87 b1       	in	r24, 0x07	; 7
>  12e:	90 91 88 00 	lds	r25, 0x0088
>  132:	89 2b       	or	r24, r25
>  134:	87 b9       	out	0x07, r24	; 7
>  136:	87 b1       	in	r24, 0x07	; 7
>  138:	87 b9       	out	0x07, r24	; 7
> 	ADCSRA |= (1<<ADSC);					// Start the next conversion
>  13a:	36 9a       	sbi	0x06, 6	; 6
>  13c:	ff 91       	pop	r31
>  13e:	ef 91       	pop	r30
>  140:	9f 91       	pop	r25
>  142:	8f 91       	pop	r24
>  144:	3f 91       	pop	r19
>  146:	2f 91       	pop	r18
>  148:	0f 90       	pop	r0
>  14a:	0f be       	out	0x3f, r0	; 63
>  14c:	0f 90       	pop	r0
>  14e:	1f 90       	pop	r1
>  150:	18 95       	reti
> 
> 
> Normally in any app I write in ASM, I dedicate two registers for usage
> in the ISR,  ITEMP and ITEMP2, and one to hold SREG, "STEMP"
> So getting into this would be something like:
> 
> ADC_ISR:
>         in STEMP,SREG
> 
>      code...
> 
>         out SREG,STEMP
>      reti
> 
>

--- In AVR-Chat@yahoogroups.com, "David VanHorn" <microbrix@...> wrote:
> Normally in any app I write in ASM, I dedicate two
> registers for usage in the ISR, ITEMP and ITEMP2,
> and one to hold SREG, "STEMP"
That is a perfectly reasonable (partial) set of register allocation 
assumptions for a small set of applications.  The avr-gcc compiler, on 
the other hand, has a set of register allocation assumptions that work 
for all applications.  These assumptions are documented in the FAQ 
(see http://www.nongnu.org/avr-libc/user-
manual/FAQ.html#faq_reg_usage) but are summarized as:

* r1 always has the value zero
* A function can freely modify r0, r18-27, r30-31.
* A function may not modify r2-17, r28-29.  If these registers are 
used they must be saved/restored.

Moreover, an ISR must preserve all register values and cannot rely on 
r1 being zero.

There is a way to inform the compiler that you're going to use certain 
registers for your own special purposes (see http://www.nongnu.org/avr-
libc/user-manual/FAQ.html#faq_regbind) but this practice is 
discouraged.

The code that was generated for the ISR is reasonable with respect to 
these assumptions.  It is a known issue, however, that generated ISR 
code sometimes goes to the trouble of saving and zeroing r1 when it 
doesn't use r1 itself and doesn't call any external functions.

It is also true that as changes are made to the gcc compiler, there 
are sometimes regressesions in the "quality" of the code produced by 
the avr-gcc variant.  These take time to get repaired and it may make 
sense sometimes to continue to use an older version until the newer 
one is improved.  I currently use the 20071221 version and would 
recommend it unless/until there is a specific reason to use a newer 
version.

--- In AVR-Chat@yahoogroups.com, "David VanHorn" <microbrix@...> 
wrote:
> Maybe I'm missing something here, but what's the point of "almost
> always having 0 in R1"
In a word, history.  The avr-gcc register assumptions were defined 
before the advent of the AVR MUL instruction which produces its 
result, unfortunately, in r1:r0.  That means that if an interrupt 
occurs immediately after a MUL instruction, r1 may not be zero.

It is true, of course, that the avr-gcc register assumptions could 
be changed to allocate r2, for example, to be the "zero register".  
This would be a painful change because it would break all existing 
code.  At first blush, you might think "yes, but all you would need 
to do is recompile your code."  While that is true for C code, any 
assembly language code intended to work with C code would need to be 
manually inspected and modified.

> That seems like a trivial thing to fix.
Perhaps.  I've never looked at the source code for the avr-gcc code 
generator.  I suspect that it is more difficult than it at first 
appears otherwise it would already have been done.  It may also be 
that even though it might be relative simple to correct, other 
changes are deemed to be higher priority.  In any event, you can 
always get the source code and fix it yourself if it is important to 
you.

I suspect that the general consensus is that if you have a need for 
an extremely efficient ISR (or any function for that matter), you 
can always code it completely in assembly language.  I have done 
this from time to time.  Using mixed C and assembly language gives 
you the "best of both worlds".   You get the benefits of using C for 
most of your application and, where it is needed, you get the 
benefit of highly tuned, hand-crafted assembly language code.

Don Kinzer
ZBasic Microcontrollers
http://www.zbasic.net

If you use something other than an int as an index then cast it to an int.
Ray

David VanHorn wrote:
> 
> 
> Is there a reason that I can't use a char as a pointer in an array of ints?
> My ADC experiment has an array of 8 ints to store the data, and it
> does grind my gears to use a 16 bit pointer into an 8 element array.
> 
> In ASM, I'd only use three bits of a byte, saving the other five for
> flags, and absolutely preventing indexing outside the array, but I
> haven't worked out how to do that in C yet.
> 
> Also, I did look at the compiled version of my ISR... Eww.
> 
> I'm open to the idea that I'm not writing it in the right way for the
> compiler to be efficient yet, but I was impressed with how many
> instructions it took to do the ISR.
> 
> Started with this code:
> 
> // Reads all 8 channels even though we only use two.
> ISR(ADC_vect)
> { // Store in array of locations per channel flag
> 
> // Read the channel data MUST BE IN LOW-HIGH order!
> // low byte of ADC_Reading = ADCL
> // high byte of ADC_Reading = ADCH
> ADC_Data[ADC_Pointer] = ADCW; // ADC_Reading;
> 
> // Select the next channel
> ADC_Pointer++; //Only I ever increment this!
> if (ADC_Pointer > 7) ADC_Pointer=0;
> 
> ADMUX = ADMUX & 0xF8; // Save the other bits
> ADMUX = ADMUX |= ADC_Pointer; // Copy the pointer into ADMUX
> ADCSRA |= (1<<ADSC); // Start the next conversion
> }
> 
> And got this with maximum optimization for size:
> 
> // Reads all 8 channels even though we only use two.
> ISR(ADC_vect)
> { // Store in array of locations per channel flag
> de: 1f 92 push r1
> e0: 0f 92 push r0
> e2: 0f b6 in r0, 0x3f ; 63
> e4: 0f 92 push r0
> e6: 11 24 eor r1, r1
> e8: 2f 93 push r18
> ea: 3f 93 push r19
> ec: 8f 93 push r24
> ee: 9f 93 push r25
> f0: ef 93 push r30
> f2: ff 93 push r31
> 
> // Read the channel data MUST BE IN LOW-HIGH order!
> // low byte of ADC_Reading = ADCL
> // high byte of ADC_Reading = ADCH
> ADC_Data[ADC_Pointer] = ADCW; // ADC_Reading;
> f4: 20 91 88 00 lds r18, 0x0088
> f8: 30 91 89 00 lds r19, 0x0089
> fc: f9 01 movw r30, r18
> fe: e2 0f add r30, r18
> 100: f3 1f adc r31, r19
> 102: e6 57 subi r30, 0x76 ; 118
> 104: ff 4f sbci r31, 0xFF ; 255
> 106: 84 b1 in r24, 0x04 ; 4
> 108: 95 b1 in r25, 0x05 ; 5
> 10a: 91 83 std Z+1, r25 ; 0x01
> 10c: 80 83 st Z, r24
> 
> // Select the next channel
> ADC_Pointer++; //Only I ever increment this!
> 10e: c9 01 movw r24, r18
> 110: 01 96 adiw r24, 0x01 ; 1
> 112: 90 93 89 00 sts 0x0089, r25
> 116: 80 93 88 00 sts 0x0088, r24
> if (ADC_Pointer > 7) ADC_Pointer=0;
> 11a: 08 97 sbiw r24, 0x08 ; 8
> 11c: 20 f0 brcs .+8 ; 0x126 <__vector_16+0x48>
> 11e: 10 92 89 00 sts 0x0089, r1
> 122: 10 92 88 00 sts 0x0088, r1
> 
> ADMUX = ADMUX & 0xF8; // Save the other bits
> 126: 87 b1 in r24, 0x07 ; 7
> 128: 88 7f andi r24, 0xF8 ; 248
> 12a: 87 b9 out 0x07, r24 ; 7
> ADMUX = ADMUX |= ADC_Pointer; // Copy the pointer into ADMUX
> 12c: 87 b1 in r24, 0x07 ; 7
> 12e: 90 91 88 00 lds r25, 0x0088
> 132: 89 2b or r24, r25
> 134: 87 b9 out 0x07, r24 ; 7
> 136: 87 b1 in r24, 0x07 ; 7
> 138: 87 b9 out 0x07, r24 ; 7
> ADCSRA |= (1<<ADSC); // Start the next conversion
> 13a: 36 9a sbi 0x06, 6 ; 6
> 13c: ff 91 pop r31
> 13e: ef 91 pop r30
> 140: 9f 91 pop r25
> 142: 8f 91 pop r24
> 144: 3f 91 pop r19
> 146: 2f 91 pop r18
> 148: 0f 90 pop r0
> 14a: 0f be out 0x3f, r0 ; 63
> 14c: 0f 90 pop r0
> 14e: 1f 90 pop r1
> 150: 18 95 reti
> 
> Normally in any app I write in ASM, I dedicate two registers for usage
> in the ISR, ITEMP and ITEMP2, and one to hold SREG, "STEMP"
> So getting into this would be something like:
> 
> ADC_ISR:
> in STEMP,SREG
> 
> code...
> 
> out SREG,STEMP
> reti
> 
> -- 
> 
> "The very powerful and the very stupid have one thing in common. Instead of
> altering their views to fit the facts, they alter the facts to fit their
> views... which can be very uncomfortable if you happen to be one of the
> facts that needs altering." Doctor Who, Face of Evil
> 
> 

-- 
Ray Hurst
949-202-6037

> * r1 always has the value zero
> * A function can freely modify r0,

BUT!.....

> Moreover, an ISR must preserve all register values and cannot rely on
> r1 being zero.

Maybe I'm missing something here, but what's the point of "almost
always having 0 in R1"

In ASM, I allocate a register to hold 0, but it is ALWAYS zero, and I
have a sanity check routine that looks for any non-zero values in R0,
stack other than a predefined value in main, as well as other values
that I can know in advance.


> The code that was generated for the ISR is reasonable with respect to
> these assumptions.  It is a known issue, however, that generated ISR
> code sometimes goes to the trouble of saving and zeroing r1 when it
> doesn't use r1 itself and doesn't call any external functions.

That seems like a trivial thing to fix.

> I suspect that the general consensus is that if you have a need for
> an extremely efficient ISR (or any function for that matter), you
> can always code it completely in assembly language.  I have done
> this from time to time.  Using mixed C and assembly language gives
> you the "best of both worlds".   You get the benefits of using C for
> most of your application and, where it is needed, you get the
> benefit of highly tuned, hand-crafted assembly language code.

That's what I expect, and having done a lot of ASM in the past, plus
loads of hardware, I'm trying to position myself as a good all-around
embedded systems guy.  While I haven't yet been asked to do anything
that I couldn't do in ASM, and I have done things that wouldn't be
possible in C, it seems that at this point, all potential employers or
clients expect me to know C....

I do see a class of problems that would be intractable in ASM in any
practical sense, but it's just that I haven't yet hit any of that in
real projects that clients or employers have given me so far..

Eventually I see myself writing most of the app in C, and then writing
ISRs and maybe low level drivers and time-critical code in ASM.

All
   I posted the following message three days ago and have not seen it 
appear in the group.  Did it make it or did I just miss the posting?

Regards
-Bill Knight
R O SoftWare
====================================================================

David
   I do not have an AVR compiler accessible to me right now but please
let me offer an alternative piece of code.  Also, I did not see your
declaration of 'ADC_Pointer' which is actually an index, not a pointer.

unsigned char ADC_index;

ISR(ADC_vect)
{
   ADC_Data[ADC_index] = ADCW;

   ADC_index = ((ADC_index + 1) & 7);
   ADMUX = (ADMUX & 0xF8) | ADC_index;
   ADCSRA |= (1 << ADSC);
}

   If you have the time, please give this a try and let me know how (if)
it works out any better for you.

-Bill Knight
R O Software.

PS - ADC_index declared as an 'unsigned int' (instead of an 'unsigned
char') might actually produce smaller code.


David VanHorn wrote:

> Is there a reason that I can't use a char as a pointer in an array of ints?
> My ADC experiment has an array of 8 ints to store the data, and it
> does grind my gears to use a 16 bit pointer into an 8 element array.
> 
> In ASM, I'd only use three bits of a byte, saving the other five for
> flags, and absolutely preventing indexing outside the array, but I
> haven't worked out how to do that in C yet.
> 
> Also, I did look at the compiled version of my ISR... Eww.
> 
> I'm open to the idea that I'm not writing it in the right way for the
> compiler to be efficient yet, but I was impressed with how many
> instructions it took to do the ISR.
> 
> Started with this code:
> 
> // Reads all 8 channels even though we only use two.
> ISR(ADC_vect)
> {	// Store in array of locations per channel flag
> 	
> 	// Read the channel data MUST BE IN LOW-HIGH order!
> 	// low byte of ADC_Reading = ADCL
> 	// high byte of ADC_Reading = ADCH
> 	ADC_Data[ADC_Pointer] = ADCW;          // ADC_Reading;
> 
> 	// Select the next channel
> 	ADC_Pointer++;  						//Only I ever increment this!
> 	if (ADC_Pointer > 7) ADC_Pointer=0;
> 	
> 	ADMUX = ADMUX & 0xF8;					// Save the other bits
> 	ADMUX = ADMUX |= ADC_Pointer;			// Copy the pointer into ADMUX
> 	ADCSRA |= (1<<ADSC);					// Start the next conversion
> }
> 
> 
> And got this with maximum optimization for size:
> 
> // Reads all 8 channels even though we only use two.
> ISR(ADC_vect)
> {	// Store in array of locations per channel flag
>   de:	1f 92       	push	r1
>   e0:	0f 92       	push	r0
>   e2:	0f b6       	in	r0, 0x3f	; 63
>   e4:	0f 92       	push	r0
>   e6:	11 24       	eor	r1, r1
>   e8:	2f 93       	push	r18
>   ea:	3f 93       	push	r19
>   ec:	8f 93       	push	r24
>   ee:	9f 93       	push	r25
>   f0:	ef 93       	push	r30
>   f2:	ff 93       	push	r31
> 	
> 	// Read the channel data MUST BE IN LOW-HIGH order!
> 	// low byte of ADC_Reading = ADCL
> 	// high byte of ADC_Reading = ADCH
> 	ADC_Data[ADC_Pointer] = ADCW;          // ADC_Reading;
>   f4:	20 91 88 00 	lds	r18, 0x0088
>   f8:	30 91 89 00 	lds	r19, 0x0089
>   fc:	f9 01       	movw	r30, r18
>   fe:	e2 0f       	add	r30, r18
>  100:	f3 1f       	adc	r31, r19
>  102:	e6 57       	subi	r30, 0x76	; 118
>  104:	ff 4f       	sbci	r31, 0xFF	; 255
>  106:	84 b1       	in	r24, 0x04	; 4
>  108:	95 b1       	in	r25, 0x05	; 5
>  10a:	91 83       	std	Z+1, r25	; 0x01
>  10c:	80 83       	st	Z, r24
> 
> 	// Select the next channel
> 	ADC_Pointer++;  						//Only I ever increment this!
>  10e:	c9 01       	movw	r24, r18
>  110:	01 96       	adiw	r24, 0x01	; 1
>  112:	90 93 89 00 	sts	0x0089, r25
>  116:	80 93 88 00 	sts	0x0088, r24
> 	if (ADC_Pointer > 7) ADC_Pointer=0;
>  11a:	08 97       	sbiw	r24, 0x08	; 8
>  11c:	20 f0       	brcs	.+8      	; 0x126 <__vector_16+0x48>
>  11e:	10 92 89 00 	sts	0x0089, r1
>  122:	10 92 88 00 	sts	0x0088, r1
> 	
> 	ADMUX = ADMUX & 0xF8;					// Save the other bits
>  126:	87 b1       	in	r24, 0x07	; 7
>  128:	88 7f       	andi	r24, 0xF8	; 248
>  12a:	87 b9       	out	0x07, r24	; 7
> 	ADMUX = ADMUX |= ADC_Pointer;			// Copy the pointer into ADMUX
>  12c:	87 b1       	in	r24, 0x07	; 7
>  12e:	90 91 88 00 	lds	r25, 0x0088
>  132:	89 2b       	or	r24, r25
>  134:	87 b9       	out	0x07, r24	; 7
>  136:	87 b1       	in	r24, 0x07	; 7
>  138:	87 b9       	out	0x07, r24	; 7
> 	ADCSRA |= (1<<ADSC);					// Start the next conversion
>  13a:	36 9a       	sbi	0x06, 6	; 6
>  13c:	ff 91       	pop	r31
>  13e:	ef 91       	pop	r30
>  140:	9f 91       	pop	r25
>  142:	8f 91       	pop	r24
>  144:	3f 91       	pop	r19
>  146:	2f 91       	pop	r18
>  148:	0f 90       	pop	r0
>  14a:	0f be       	out	0x3f, r0	; 63
>  14c:	0f 90       	pop	r0
>  14e:	1f 90       	pop	r1
>  150:	18 95       	reti
> 
> 
> Normally in any app I write in ASM, I dedicate two registers for usage
> in the ISR,  ITEMP and ITEMP2, and one to hold SREG, "STEMP"
> So getting into this would be something like:
> 
> ADC_ISR:
>         in STEMP,SREG
> 
>      code...
> 
>         out SREG,STEMP
>      reti
> 
>

On Thu, Jan 15, 2009 at 9:55 AM, Bill Knight <BillK@rosw.com> wrote:
> All
>   I posted the following message three days ago and have not seen it
> appear in the group.  Did it make it or did I just miss the posting?

It made it..  Thanks for the hints.. I'm moving along, I have the ADC
now firing by timer0 overflow, and storing the data into an array,
using unsigned chars wherever possible, etc.

Amusing:  The T0 overflow ISR, which should be two instructions,
compiles to this:
// Timer 0 overflow ISR
ISR(TIMER0_OVF_vect)
{
     498:	1f 92       	push	r1
     49a:	0f 92       	push	r0
     49c:	0f b6       	in	r0, 0x3f	; 63
     49e:	0f 92       	push	r0
     4a0:	11 24       	eor	r1, r1
	ADCSRA |= (1<<ADSC);					// Start the next ADC conversion
     4a2:	36 9a       	sbi	0x06, 6	; 6
     4a4:	0f 90       	pop	r0
     4a6:	0f be       	out	0x3f, r0	; 63
     4a8:	0f 90       	pop	r0
     4aa:	1f 90       	pop	r1
     4ac:	18 95       	reti