attiny4 pgm_read_byte problem

Go To Last Post
14 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I am having a strange problem with attiny4 and pgm_read_byte, for some reason the base memory address is compiling to 0x8000 instead of 0x4000, crazy thing is the __AVR_TINY_PM_BASE_ADDRESS__ is defined as 0x4000, the opcode instruction from pgmspace.h compiles to 0x80 instead of 0x40 for the upper register. If I use _SFR_IO8 instead of pgm_read_byte everything works fine...

I even copied out the assembly code from the pgmspace.h and made my own function with it, same issue.

I also hard coded the the 0x4014 address and directly called the assembly function, it still compiled to using 0x8014 instead.

I'm trying to figure out why this is happening, anyone with any ideas I would appreciate it..

I am using tool chain "avr8-gnu-toolchain-3.6.2.1759-linux_x86_64" which is the newest available as of today, tried with and without PACKS for attiny

my compile script is also below at the bottom

Example below...

 

//program defines
#define F_CPU 1000000UL  //we run at the default internal oscillator setting of 1Mhz

//includes
#include <inttypes.h>
#include <avr/io.h>
#include <util/delay.h>
#include <avr/pgmspace.h>

//macros
#define U8                     uint8_t
#define U16                    uint16_t
#define BIT(x)                 (0x01 << (x))

static const U16 g_test_byte[] PROGMEM __attribute__((used)) = {0x55}; //must be word length, bit flip, as a type of weak encryption, to hide actual value

//show define value
//https://stackoverflow.com/questions/1562074/how-do-i-show-the-value-of-a-define-at-compile-time
#define XSTR(x) STR(x)
#define STR(x) #x
#pragma message "The value of define is: " XSTR(__AVR_TINY_PM_BASE_ADDRESS__)

//test to see if pgmspace.h maybe does not have __AVR_TINY_PM_BASE_ADDRESS__ set in time, but not even the below code works
#define my_pgm_read_byte(addr)      \
(__extension__({                \
    uint16_t __addr16 = (uint16_t)(addr) + __AVR_TINY_PM_BASE_ADDRESS__; \
    uint8_t __result;           \
    __asm__                     \
    (                           \
        "ld %0, z" "\n\t"       \
        : "=r" (__result)       \
        : "z" (__addr16)        \
    );                          \
    __result;                   \
}))

//METHOD1 is pgm_read_byte, METHOD2 is _SFR_IO8
#define USE_METHOD1 //comment out to use method 2

#ifdef USE_METHOD1 //comment out to use method 2
void main(void) __attribute__((noreturn));  void main(void)
{

	U16 ptr = (U16)&g_test_byte[0];  //get pointer to flash memory location
	U8 tmp  = my_pgm_read_byte(ptr); //read the byte from flash, even hardcoding 0x4014 does not work!
	PORTB   = tmp;

	while(1);
}
#else
void main(void) __attribute__((noreturn));  void main(void)
{
	U16 ptr = (U16)&g_test_byte[0];
	U8 tmp  = _SFR_IO8(ptr);  //read the byte from flash
	PORTB   = tmp;

	while(1);
}
#endif

 

Problem is here... ldi r31, 0x80 should actually be ldi r31, 0x40

//using pgm_read_byte
void main(void) __attribute__((noreturn));  void main(void)
{
	U16 ptr = (U16)&g_test_byte[0];       //get pointer to flash memory location //178 bytes used
	U8 tmp  = pgm_read_byte(ptr);  //read the byte from flash, bit flip to unencrypt
  28:	e4 e1       	ldi	r30, 0x14	; 20
  2a:	f0 e8       	ldi	r31, 0x80	; 128  <--- THIS IS WRONG, SHOULD BE 0x40
  2c:	e0 81       	ld	r30, Z
	PORTB   = tmp;
  2e:	e2 b9       	out	0x02, r30	; 2
  30:	ff cf       	rjmp	.-2      	; 0x30 <main+0x8>

00000032 <_exit>:
  32:	f8 94       	cli

00000034 <__stop_program>:
  34:	ff cf       	rjmp	.-2      	; 0x34 <__stop_program>

 

 

using _SFR_IO8() macro instead of pgm_read_byte, then everything works fine...

//using    _SFR_IO8(0x4014)
00000026 <main>:
void main(void) __attribute__((noreturn));  void main(void)
{
		U16 ptr = (U16)&g_test_byte[0];
		#define MEM_LOC     _SFR_IO8(ptr)
		U8 tmp  = MEM_LOC;  //read the byte from flash, bit flip to unencrypt
  26:	e4 e1       	ldi	r30, 0x14	; 20
  28:	f0 e4       	ldi	r31, 0x40	; 64
  2a:	40 81       	ld	r20, Z
		PORTB   = tmp;
  2c:	42 b9       	out	0x02, r20	; 2
  2e:	ff cf       	rjmp	.-2      	; 0x2e <main+0x8>

00000030 <_exit>:
  30:	f8 94       	cli

00000032 <__stop_program>:
  32:	ff cf       	rjmp	.-2      	; 0x32 <__stop_program>  

 

Compile script for linux toolchain...

#!/bin/bash

#####################################
# Bash Compile Script
#####################################

#mcu we are compiling for
MCU=attiny4

#variables
PRE=out_
OUT=$PRE$MCU
USR=$USER
ENT=0x0000 #entry point, make sure to pay attention to this when using bootloaders

TOOLCHAIN=/home/$USR/dev/lib_avr/avr8-gnu-toolchain-3.6.2.1759-linux_x86_64
GCC=$TOOLCHAIN/bin
PAC=$TOOLCHAIN/PACKS/Atmel.ATmega_DFP.1.2.272
PACB=$PAC/gcc/dev/$MCU/
PACI=$PAC/include/
#PACa=-I $PACI -B $PACB    #comment out to prevent using PACKS
#PACb=-B $PACB             #comment out to prevent using PACKS
CUR=$(pwd)
EZS=/home/$USR/dev/lib_avr/ezstack/ezstack

#options
OPT='  -DF_CPU=1000000UL '
OPT+=' -Os '
OPT+=' -gdwarf-2 '                     #-g2 same as -gdwarf-2 I think
OPT+=' -DDEBUG '
OPT+=' -std=gnu99 '
OPT+=' -Wall '
OPT+=' -Wshadow '  #usbdrv has a shadowed variable
OPT+=' -funsigned-char '               #pretty sure this is missing from my other programs
OPT+=' -funsigned-bitfields '
OPT+=' -fpack-struct '
OPT+=' -fshort-enums '
OPT+=' -ffunction-sections '
OPT+=' -fdata-sections '
OPT+=' -ffreestanding '
OPT+=' -finline-limit=3 '
OPT+=' -fno-inline-small-functions '

#files to compile #https://stackabuse.com/array-loops-in-bash/
INCS="   "               # custom include directories, but -I before each one
ASMS=(  )                # .S asm files
FILES=( main  ) # .c files

#make output directory
if [ -e "$OUT" ];
then
    "echo found existing bin directory"
else
	"echo creating output directory"
	mkdir "$OUT"
fi

#compile
reset
echo "__________________________________________________________"
echo "STARTING COMPILE"
cd $OUT
#=========================================================== -std=gnu99

#create include directories variable

#reset object linking
OL=""

#compile to object file (if not compiling correctly, echo the command below and make sure all variables are set)
FM=${FILES[0]}
for FL in "${FILES[@]}"
do
echo "  C Compiling... $CUR/$FL.c"
$GCC/avr-gcc -g -x c                              $INCS $PACa $OPT  -mmcu=$MCU -c -MD -MP -MT "$CUR/$FL.o" -MF "$CUR/$FL.d" -o "$CUR/$FL.o" -c "$CUR/$FL.c"
OL+="$CUR/$FL.o " #prep the object linking
done

#compile ASM files
for FL in "${ASMS[@]}"
do
echo "ASM Compiling... $CUR/$FL.S"
$GCC/avr-gcc -g -x assembler-with-cpp -Wa,-gdwarf2 $INCS $PACa $OPT -mmcu=$MCU     -MD -MP -MT "$CUR/$FL.o" -MF "$CUR/$FL.d" -o "$CUR/$FL.o" -c "$CUR/$FL.S"
OL+="$CUR/$FL.o " #prep the object linking
done

#compile objects to elf file, specify entry point
$GCC/avr-gcc -g -o $FM.elf  $OL   -Wl,-Map="$FM.map" -Wl,--start-group -Wl,-lm  -Wl,--end-group -Wl,--gc-sections -Wl,--section-start=.text="$ENT" -mmcu=$MCU $PACb 

#create hex from elf (must remove any extra data when dumping with -R)
$GCC/avr-objcopy -j .text -j .data -O ihex "$FM.elf" "$FM.hex" #extract only .text and .data sections

#create srec from elf
$GCC/avr-objcopy -j .text -j .data -O srec "$FM.elf" "$FM.srec" #extract only .text and .data sections

#create eeprom file from elf
$GCC/avr-objcopy -j .eeprom  --set-section-flags=.eeprom=alloc,load --change-section-lma .eeprom=0  --no-change-warnings -O ihex "$FM.elf" "$FM.eep"

#dump elf to assembly (must have elf that was generated with -g)
$GCC/avr-objdump -h -S "$FM.elf" > "$FM.lss"

#===========================================================
echo "COMPILE FINISHED"
echo "__________________________________________________________"
#display GCC version
$GCC/avr-gcc --version
#display code size
$GCC/avr-objdump -Pmem-usage "$FM.elf"
$GCC/avr-size "$FM.elf"
echo "__________________________________________________________"
cd ..
#move assembly junk to output folder
mv *.o "./$OUT"
mv *.d "./$OUT"

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Just use vanilla C. The linker script locates .rodata in flash and adds the needed offsets to symbol values.
.
__flash is not supported, avr-gcc will error.

avrfreaks does not support Opera. Profile inactive.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

If you use progmem, please read the manual *again*. It works different for ATtiny4. Adding offset by hand is wrong. Apart from that you don't need inline asm for LD.

avrfreaks does not support Opera. Profile inactive.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

But... that's not HIS code; it's a copy out of avr/pgmspace.h, with __AVR_TINY_PM_BASE_ADDRESS__ being defined by the device specs somewhere.

Looks like both the linker and the __LPM_tiny__ are adding the offset to variables tagged PROGMEM...  An actual bug in avr-libc?

 

here's the "canonical" version - it also ends up doing a load from 0x80xx, when compiled for tiny4:

#include <avr/io.h>
#include <avr/pgmspace.h>

const char pdata = 123;

int main() {
    PORTB = pgm_read_byte(&pdata);
}

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I don't use a tiny4, but just use const, no pgmspace needed it seems-

 

#include <avr/io.h>
const char pdata[16]={1};
int main() {
    for(uint8_t i = 0; i < 16; PORTB = pdata[i++] );
}
/*
00000070 <main>:
  70:	e0 e6       	ldi	r30, 0x60	; 96
  72:	f0 e4       	ldi	r31, 0x40	; 64
  74:	41 91       	ld	r20, Z+
  76:	42 b9       	out	0x02, r20	; 2
  78:	40 e4       	ldi	r20, 0x40	; 64
  7a:	e0 37       	cpi	r30, 0x70	; 112
  7c:	f4 07       	cpc	r31, r20
  7e:	d1 f7       	brne	.-12     	; 0x74 <main+0x4>
  80:	80 e0       	ldi	r24, 0x00	; 0
  82:	90 e0       	ldi	r25, 0x00	; 0
  84:	08 95       	ret
 */

 

>I also hard coded the the 0x4014 address and directly called the assembly function, it still compiled to using 0x8014 instead.

 

You are seeing the 0x80xx because the pgmspace macro for the tiny4 is adding another 0x4000. Since the const var is already at an address of 0x804xxx (resulting in a uint16_t of 0x4xxx), adding another 0x4000 gets you to 0x8xxx. It appears if you want to use pgm_read_x macros for some reason (I see none), you need to provide 0 based addresses. The _SFR_IO8() macro simply is casting the address to a uint8_t, which of course turns a 0x4xnn into 0xnn and the add of 0x4000 gets it back to 0x4xnn. That will also start to fail as soon as const data passes over 0x40FF.

 

 

 

Last Edited: Mon. Sep 23, 2019 - 04:33 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

just use const, no pgmspace needed

Yep.  I guess that technically, the behavior of pgm_read_byte() can be considered "correct" - the documentation specifically says: "The address is in the program space."

But it's certainly annoying that the "normal" &(variable_defined_with_PROGMEM) argument doesn't work...

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

>But it's certainly annoying that the "normal" &(variable_defined_with_PROGMEM) argument doesn't work...

 

#undef __AVR_TINY_PM_BASE_ADDRESS__

#define __AVR_TINY_PM_BASE_ADDRESS__ 0

 

now it does :)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

This is a direct note from the toochain pgmspace.h below, (entire pgmspace.h file attached from the toolchain) it appears pgm_read_byte is "supposed to work" for attiny4, so says the comment below inside the pgmspace.h.

The  #undef __AVR_TINY_PM_BASE_ADDRESS__  #define __AVR_TINY_PM_BASE_ADDRESS__ 0  hack should work, I will likely use that for now, but I feel like that should be happening somewhere inside pgmspace.h

/*
Macro to read data from program memory for avr tiny parts(tiny 4/5/9/10/20/40).
why:
- LPM instruction is not available in AVR_TINY instruction set.
- Programs are executed starting from address 0x0000 in program memory.
But it must be addressed starting from 0x4000 when accessed via data memory.
Reference: TINY device (ATTiny 4,5,9,10,20 and 40) datasheets
Bug: avrtc-536
*/

 

Attachment(s): 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Why bother with pgmspace if you are just needing const var's? Its already handled and there is no need for pgm_read_something.

 

If you want to actually read code for some reason (data embedded in code, not a const var), then you need the pgm_read_something as the code address will be 0 based and an offset will need to be added.

 

//constant var in flash

//will need initializing, else ends up in .bss (I think), not .rodata

const char myvar = 1;

 

//if I want to read this code for some reason

//(I'm sure there are better examples/reasons to be reading code space)

void myfunc(){ PORTB = 0; }

//need to use pgm_read_something

uint16_t fcode = pgm_read_word( myfunc );

 

Here is your example (slightly modified)-

const uint16_t g_test_word = 0x0055; //2bytes
int main(void){
    PORTB = g_test_word; //low byte
    while(1);
}

the compiler will optimize this as it already knows what the const value is, but my previous example (with array access) shows the compiler will know how to access the const memory, also.

Last Edited: Mon. Sep 23, 2019 - 03:51 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

curtvm, because I have tons of libraries already written that use pgmspace functions (I'm sure others do too). re-writing them all just for that chip does not make sense. Also it is not just the that, I need to put the variables in flash so I can read them and modify with without have to worry about intermengled instructions. The instructions/opcodes can intermingle with values sometimes. I have a script that can insert serial numbers into precompiled flash hex, I cannot do that if the instructions have intermingled into the const value, I can also specify with section-start where I want the flash variable to reside. it appears that chip was intended to be supported by the toolchain for the pgmspace functions. best to get this resolved so existing libraries will work for that series. I am sure the chip was intended to have those functions work as part of cross compatibility between libraries. I submitted a ticket for the issue to Microchip.

 

Last Edited: Mon. Sep 23, 2019 - 03:53 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

It appears if you use a const array, it ends up at 0x804xxx, but if you make it a 'single' type then it is 0 based.

(change your var from a [] to just a single uint16_t to see)

 

Probably just need to first clear bit14 in the address calculation, so will end up at 0x4xxx in any case-

pgmspace.h tiny macros-

uint16_t __addr16 = ((uint16_t)(addr) & ~0x4000) + __AVR_TINY_PM_BASE_ADDRESS__ 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

As I already said:
.
1) There is no need for PROGMEM.
.
2) Even if you use PROGMEM, do *not* use pgm_read
.
3) If you like pgm_read so much, it's up to you to make your life complicated.
.
4) If everything else fails, consider reading the documentation. GCC manual has code examples and explains how attribute __progmem__ is different for reduced cores.

avrfreaks does not support Opera. Profile inactive.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

He laid out his case.

 

Some of it on the code side could probably done without pgm_read_ -

#define MYSERNUM_ADDR 0x4100 //known address (+0x4000)

uint8_t sn = *(const uint8_t*)MYSERNUM_ADDR;

 

or if do not want to change any code, change pgmspace.h in various ways-

#define pgm_read_byte(addr) *(const uint8_t*)(addr|0x4000)

#define pgm_read_word(addr) *(const uint16_t*)(addr|0x4000)

...
 

or

uint16_t __addr16 = ((uint16_t)(addr) & ~0x4000) + __AVR_TINY_PM_BASE_ADDRESS__

...

or 

uint16_t __addr16 = ((uint16_t)(addr) | __AVR_TINY_PM_BASE_ADDRESS__

...

 

many options. I would probably just do a search/replace on pgmspace.h-

+ __AVR_TINY_PM_BASE_ADDRESS__

| __AVR_TINY_PM_BASE_ADDRESS__

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

curtvm, thanks, all viable solutions, but trying to keep existing libraries working, I did come up with a compiler script solution based on the redefine trick you mentioned earlier.

As westfw mentioned earlier, likely a bug in avr-libc since the __LPM_tiny__ and probably the linker are adding the offset to variables tagged PROGMEM.

 

I did a grep... 
~/dev/lib_avr/avr8-gnu-toolchain-3.6.2.1759-linux_x86_64$ find ./ -type f -exec grep -H '__AVR_TINY_PM_BASE_ADDRESS__' {} \;
./avr/include/avr/pgmspace.h:    uint16_t __addr16 = (uint16_t)(addr) + __AVR_TINY_PM_BASE_ADDRESS__; \
./avr/include/avr/pgmspace.h:    uint16_t __addr16 = (uint16_t)(addr) + __AVR_TINY_PM_BASE_ADDRESS__; \
./avr/include/avr/pgmspace.h:    uint16_t __addr16 = (uint16_t)(addr) + __AVR_TINY_PM_BASE_ADDRESS__; \
./avr/include/avr/pgmspace.h:    uint16_t __addr16 = (uint16_t)(addr) + __AVR_TINY_PM_BASE_ADDRESS__; \
./doc/avr-libc/avr-libc-user-manual/pgmspace_8h_source.html:<a name="l00429"></a>00429 <span class="preprocessor">    uint16_t __addr16 = (uint16_t)(addr) + __AVR_TINY_PM_BASE_ADDRESS__; \</span>
./doc/avr-libc/avr-libc-user-manual/pgmspace_8h_source.html:<a name="l00473"></a>00473 <span class="preprocessor">    uint16_t __addr16 = (uint16_t)(addr) + __AVR_TINY_PM_BASE_ADDRESS__; \</span>
./doc/avr-libc/avr-libc-user-manual/pgmspace_8h_source.html:<a name="l00525"></a>00525 <span class="preprocessor">    uint16_t __addr16 = (uint16_t)(addr) + __AVR_TINY_PM_BASE_ADDRESS__; \</span>
./doc/avr-libc/avr-libc-user-manual/pgmspace_8h_source.html:<a name="l00581"></a>00581 <span class="preprocessor">    uint16_t __addr16 = (uint16_t)(addr) + __AVR_TINY_PM_BASE_ADDRESS__; \</span>
Binary file ./libexec/gcc/avr/5.4.0/cc1plus matches
Binary file ./libexec/gcc/avr/5.4.0/cc1 matches

 

nothing found as to where defined, so I added option to compiler script... 
-D__AVR_TINY_PM_BASE_ADDRESS__=0

got this warning... looks like define is built in....
<command-line>:0:0: warning: "__AVR_TINY_PM_BASE_ADDRESS__" redefined
<built-in>: note: this is the location of the previous definition

 

solution... I had to use the compiler switch -U that probably almost never gets used.
OPT+=' -U__AVR_TINY_PM_BASE_ADDRESS__'   #bug fix for attiny4, undefine built in define
OPT+=' -D__AVR_TINY_PM_BASE_ADDRESS__=0' #bug fix for attiny4, redefine built in define

 

that works, able to compile and use bulit in pgm_read_byte and it points to the correction flash location.