volatile misbehaves

Lost User

Vaclav_ wrote:

The values of individual terms is irrelevant

On the contrary, they are the most important pieces of information. If you are adding two values and the answer is 1, then we need to know those values.

Vaclav_

OK, but I cannot copy it from IDE when its runing on remote. I am not sure I can post screen shots here. I'll get back to you soon. Here is a copy of the cout debugging without volatile bcm2835_periopherals 0x2000000 BCM28356_GPIO_BASE / 4 80000 bcm2836_gpio 0x20200000 Here is something which MAY explain the issue. Using the GNU Compiler Collection (GCC): Volatiles[^]

Joe Woodbury

In this case, 'volatile' refers to the object to which it is pointing, not the pointer itself. The intent of this code confuses me; why are items being cast to be a pointer?

Vaclav_

As far as I can tell - these are memory addresses and are being passed to functions. The author of the code does not tell much about WHY he does things this way. What I have gather so far - the application writes into "user space" (?) (Linux term) memory, not directly to hardware. The OS does the actual "writing" to hardware.

Vaclav_

STOP THE PRESSES ! I have some serious problems with initial setup. The "code" runs in "debug" mode which should be called "demo" mode and is missing few lines to make the memory management work.

leon de boer

Allow me to help you out ... you are trying to map all the GPIO hardware registers so here is the struct from the manual

#include #include /*--------------------------------------------------------------------------}
{ RASPBERRY PI GPIO HARDWARE REGISTERS - BCM2835.PDF Manual Section 6 }
{--------------------------------------------------------------------------*/
struct __attribute__((__packed__, aligned(4))) GPIORegisters {
uint32_t GPFSEL[6]; // 0x00 GPFSEL0 - GPFSEL5
uint32_t reserved1; // 0x18 reserved
uint32_t GPSET[2]; // 0x1C GPSET0 - GPSET1;
uint32_t reserved2; // 0x24 reserved
uint32_t GPCLR[2]; // 0x28 GPCLR0 - GPCLR1
uint32_t reserved3; // 0x30 reserved
const uint32_t GPLEV[2]; // 0x34 GPLEV0 - GPLEV1 ** Read only hence const
uint32_t reserved4; // 0x3C reserved
uint32_t GPEDS[2]; // 0x40 GPEDS0 - GPEDS1
uint32_t reserved5; // 0x48 reserved
uint32_t GPREN[2]; // 0x4C GPREN0 - GPREN1;
uint32_t reserved6; // 0x54 reserved
uint32_t GPFEN[2]; // 0x58 GPFEN0 - GPFEN1;
uint32_t reserved7; // 0x60 reserved
uint32_t GPHEN[2]; // 0x64 GPHEN0 - GPHEN1;
uint32_t reserved8; // 0x6c reserved
uint32_t GPLEN[2]; // 0x70 GPLEN0 - GPLEN1;
uint32_t reserved9; // 0x78 reserved
uint32_t GPAREN[2]; // 0x7C GPAREN0 - GPAREN1;
uint32_t reserved10; // 0x84 reserved
uint32_t GPAFEN[2]; // 0x88 GPAFEN0 - GPAFEN1;
uint32_t reserved11; // 0x90 reserved
uint32_t GPPUD; // 0x94 GPPUD
uint32_t GPPUDCLK[2]; // 0x98 GPPUDCLK0 - GPPUDCLK1;
};

Now you define the base address of the Pi .. for a Pi1 its 0x20000000, for other models 0x3F000000

#define RPi_IO_Base_Addr 0x20000000

Now what you want is to map the ALL THE REGISTERS to an address with a volatile on the pointer

#define GPIO ((volatile __attribute__((aligned(4))) struct GPIORegisters*)(uintptr_t)(RPi_IO_Base_Addr + 0x200000))

Thats it now its all done you can simply use the pointer to hit the registers ... so lets show you a function

/*-[gpio_output]------------------------------------------------------------}
. Given a valid GPIO port number the output is set high(true) or Low (false)
. RETURN: true for success, false for any failure
.----------------------------

Vaclav_

Thanks, I have not yet looked at your post. You must have spent lots of time on it, and I appreciate it. I have the code running , but... I had to delete all volatiles to make it go. Here is my simplistic view on my code and I am trying to figure out in what point the volatile keyword is causing the issue, I basically see four steps in manipulating the hardware addresses. As soon as I post this I'll study your post. I am sure it will help me to understand this "addressing" mess better. Sorry for the mess, but I have not figured out how to put parts of code under same roof. if ((fp = fopen(BMC2835_RPI2_DT_FILENAME, "rb"))) { unsigned char buf[4]; fseek(fp, BMC2835_RPI2_DT_PERI_BASE_ADDRESS_OFFSET, SEEK_SET); if (fread(buf, 1, sizeof(buf), fp) == sizeof(buf)) bcm2835_peripherals_base = (uint32_t *) (buf[0] << 24 | buf[1] << 16 | buf[2] << 8 | buf[3] << 0); fseek(fp, BMC2835_RPI2_DT_PERI_SIZE_OFFSET, SEEK_SET); if (fread(buf, 1, sizeof(buf), fp) == sizeof(buf)) bcm2835_peripherals_size = (buf[0] << 24 | buf[1] << 16 | buf[2] << 8 | buf[3] << 0); /* Base of the peripherals block is mapped to VM */ bcm2835_peripherals = (uint32_t*) mapmem("gpio", bcm2835_peripherals_size, memfd, (uint32_t) bcm2835_peripherals_base); bcm2835_gpio = (uint32_t*) bcm2835_peripherals + BCM2835_GPIO_BASE / 4; // volatile uint32_t* paddr = bcm2835_gpio + BCM2835_GPFSEL0 / 4 + (pin / 10); paddr = bcm2835_gpio + BCM2835_GPFSEL0 / 4 + (pin / 10);

Vaclav_

OK, the "base" comes from this mess. I am including it all, including my debugging stuff. So no peanut gallery comments, just the facts ma'm. First obstacle in the code is the funky (debug) - it "defaults to zero but I have not found where. Also gpiomem has to be passed as zero (?). The " BMC2835_RPI2_DT_FILENAME " #define BMC2835_RPI2_DT_FILENAME "/proc/device-tree/soc/ranges" is a "device-tree structure (?) " of RPi 3 and up(?) _ - and its access "returns " bcm2835_peripherals_base (pointer) and bcm2835_peripherals_size. Neither use "volatile" keyword. And I have to ask - why not start with volatile at the base? Now gpiomem must have something to do with identifying the actual gpio " tree branch" or what does it do? To be continued.

#define DEBUG_bcm2835_init

int C_BCM2835_SPI_TFT::bcm2835_init(int gpiomem) {

int memfd;
int ok;
FILE \*fp;

if (debug) {
	bcm2835\_peripherals = (uint32\_t\*) BCM2835\_PERI\_BASE;

	bcm2835\_pads = bcm2835\_peripherals + BCM2835\_GPIO\_PADS / 4;
	bcm2835\_clk = bcm2835\_peripherals + BCM2835\_CLOCK\_BASE / 4;
	bcm2835\_gpio = bcm2835\_peripherals + BCM2835\_GPIO\_BASE / 4;
	bcm2835\_pwm = bcm2835\_peripherals + BCM2835\_GPIO\_PWM / 4;
	bcm2835\_spi0 = bcm2835\_peripherals + BCM2835\_SPI0\_BASE / 4;
	bcm2835\_bsc0 = bcm2835\_peripherals + BCM2835\_BSC0\_BASE / 4;
	bcm2835\_bsc1 = bcm2835\_peripherals + BCM2835\_BSC1\_BASE / 4;
	bcm2835\_st = bcm2835\_peripherals + BCM2835\_ST\_BASE / 4;

#ifdef DEBUG_bcm2835_init
cout << "\033[1;32m VERIFY TRACE ENTRY \033[0m\n";
cout << "*** TRACE file " << __FILE__ << endl;
cout << " function " << __FUNCTION__ << endl;
cout << " line " << __LINE__ << endl;
exit(1);
#endif

	return 1; /\* Success \*/
}

/\* Figure out the base and size of the peripheral address block
 // using the device-tree. Required for RPi2, optional for RPi 1
 \* Or actually needed on RPi3 or Zero
 \*/
if ((fp = fopen(BMC2835\_RPI2\_DT\_FILENAME, "rb"))) {
	unsigned char buf\[4\];
	fseek(fp, BMC2835\_RPI2\_DT\_PERI\_BASE\_ADDRESS\_OFFSET, SEEK\_SET);
	if (fread(buf, 1, sizeof(buf), fp) == sizeof(buf))
		bcm2835\_peripherals\_base = (uint32\_t \*) (buf\[0\] << 24 | buf\[1\] << 16
				| buf\[2\] << 8 | buf\[3\] << 0);
	fseek(fp, BMC2835\_RPI2\_DT\_PERI\_SIZE\_OFFSET, SEEK\_SET);
	if (fread(buf, 1, sizeof(buf), fp) == sizeof(buf))
		bcm2835\_peripherals\_size = (buf\[0\] << 24 | buf\[1\] << 16
				| buf\[2\] << 8 | buf\[3\] << 0);

#ifdef DEBUG_bcm2835_init
cout << "\033[1;31mTRACE \033[0m\n";
cout << "*** TR

leon de boer

Haha that is the longest winded writing of these dozen lines of code for the Pi3 .. gpio is the volatile address you use

static volatile uint32_t *gpio;
int fd ;
// Try Obtain handle to physical memory
if ((fd = open ("/dev/mem", O_RDWR | O_SYNC) ) < 0) {
if ((fd = open ("/dev/gpiomem", O_RDWR | O_SYNC) ) < 0){
printf("Unable to open physical memory handle: %s\n", strerror(errno));
return -1;
}
}

//map a page of memory to gpio at offset 0x3F200000 which is where GPIO goodness starts on a Pi3
gpio = (uint32_t *)mmap(0, getpagesize(), PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0x3F200000);

if ((int32_t)gpio < 0){
printf("Mmap failed: %s\n", strerror(errno));
return -1;
}

/* Now you have gpio use and abuse it */

So looking at the code GPIOmem is an entry value >>>> you provide to the function <<<<. If you provide 0 it basically means Autodetect the base address If you provide any number it will use that number as the base address ignoring all the autodetection routines. Okay this pile of rubbish is just reading the device table file (hence the DT). Its just returning a number from a file it's not volatile and this junk should be a function so I would make it one. It will return 0 if it can't find the entry in the DT file and the address if it find it. I am going to dump all the long winded debug code, it's just making a mess of what is actually happening so here is your function

uint32_t BaseAddressFromDTFile (void)
{
uint32_t base_addr = 0;
FILE *fp;
if ((fp = fopen(BMC2835_RPI2_DT_FILENAME, "rb"))) {
uint32_t base_size;
unsigned char buf[4];
fseek(fp, BMC2835_RPI2_DT_PERI_BASE_ADDRESS_OFFSET, SEEK_SET);
if (fread(buf, 1, sizeof(buf), fp) == sizeof(buf))
base_addr = (uint32_t *) (buf[0] << 24 | buf[1] << 16 | buf[2] << 8 | buf[3] << 0);
fseek(fp, BMC2835_RPI2_DT_PERI_SIZE_OFFSET, SEEK_SET);
if (fread(buf, 1, sizeof(buf), fp) == sizeof(buf))
base_size = (buf[0] << 24 | buf[1] << 16 | buf[2] << 8 | buf[3] << 0);
fclose(fp);
}
return (base_addr);
}

If that returns 0 it looks like it assumes it's a Pi1 Next it just tries to Obtain handle to physical memory which the reduce form is above in my code Then if you set GPIOMem to zero it will use that address found otherwise it use the GPIOMem address to map the page. Then it does the page map with

// use memory file descriptor
/* Base of the peripherals block is mapped to VM */
bcm2835_perip

Vaclav_

I am really sorry for the messy post. Being an OF I need to keep track of myself and such code is very difficult to post in public. But if I do not - then I get asked "what is this for etc". O well. I am rewriting the whole mess and getting rid of some fluff. There is one thing I am not sure about The memory can be mapped using /dev/mem which gives "access to all" but has to have user root. I am not sure but my "remote TCF" is probably root. I need to recheck that - this TCF is not too bright to report some problems. Using /dev/gpiomem lets Linux take care of access permissions , BUT is it then limited ONLY to GPIO ? I need access to ALT0 - SPI. Thanks for putting up with my foolishnes. Cheers Vaclav

leon de boer

Each device has it's own name it's basically the name in the process device (look at the /dev directory you can see them all) Now the SPI is a proper block device and you can write to it like a file unlike the GPIO so for SPI it's either /dev/spidev0.0 /dev/spidev0.1 Depending if you want SPI0 or 1 Lets do SPI0

#include
#include //Needed for SPI port
#include //Needed for SPI port
#include //Needed for SPI port
#include //Needed for SPI port

/* This function will open the SPI port (either 0 or 1) with the given parameters */
/* If it fails it will return -1 otherwise it will return handle to opened device */

int OpenSPI( int SPI_Port, int* SPI_WriteMode, int* SPI_ReadMode, int* SPI_WriteLength, int* SPI_ReadLength, int* SPI_WriteSpeed, int* SPI_ReadSpeed)
{
int status;
int spi_handle = -1; // This will be our handle to the SPI

/* OPEN A HANDLE TO THE SPI DEVICE */
if (SPI_Port) == 0){
spi_handle = open ("/dev/spidev0.0", O_RDWR);
} else if (SPI_Port == 1) {
spi_handle = open ("/dev/spidev0.1", O_RDWR)
} else {
printf("Invalid SPI port number specified\n");
return -1;
}

if (spi_handle < 0) {
printf("Unable to open physical memory handle: %s\n", strerror(errno));
return -1;
}

/* SET SPI WRITE MODE */
status = ioctl(spi_handle, SPI_IOC_WR_MODE, SPI_WriteMode);
if(status < 0) {
printf("Could not set SPIMode (WR)...ioctl fail");
return -1;
}

/* SET SPI READ MODE */
status = ioctl(spi_handle, SPI_IOC_RD_MODE, SPI_ReadMode);
if(status < 0) {
printf("Could not set SPIMode (RD)...ioctl fail");
return(-1);
}

/* SET SPI WRITE BITS PER WORD */
status = ioctl(spi_handle, SPI_IOC_WR_BITS_PER_WORD, SPI_WriteLength);
if (status < 0) {
printf("Could not set SPI bitsPerWord (WR)...ioctl fail");
return -1;
}

/* SET SPI READ BITS PER WORD LETS MAKE IT 8 */
status = ioctl(spi_handle, SPI_IOC_RD_BITS_PER_WORD, SPI_ReadLength);
if (status < 0) {
printf("Could not set SPI bitsPerWord(RD)...ioctl fail");
return -1;
}

/* SET SPI WRITE BITS SPEED */
status = ioctl(spi_handle, SPI_IOC_WR_MAX_SPEED_HZ, SPI_WriteSpeed);
if (status < 0) {
printf("Could not set SPI speed (WR)...ioctl fail");
return -1;
}

/* SET SPI READ BITS SPEED */
status = ioctl(spi_handle, SPI_IOC_RD_

Vaclav_

Thanks again, I am making much progress with your help. I think I found the initial problem with using "volatile". Here is my view in pseudo code - using GPIO as an example : pointer * to GPIO block = pointer * to base memory block + offset to gpio Now - what would be of benefit using volatile to POINTER such as volatile pointer * to GPIO block = pointer * to base memory block + offset to gpio In my interpretation - volatile keyword is to protect variable VALUE , not the pointer. It compiles but the resulting pointer is always "1". Leon, I really need to resolve this "volatile" problem. I actually tried few different ways to output data the LCD /TFT device. Using /dev/spidev0.0 (ioctl) writing and reading , using /dev/spidev0.0 and ioctl "writing / reading messages ", plain GPIO access and now this "library". The main issue is that the LCD/TFT is really not true SPI device - it requires switching from writing data to writing command. That is why I am trying to use this BCM library. I figure if I can do the "low level BCM library stuff " I can adopt the ioctl way later.

leon de boer

Sometimes you want to read a status continually in a loop lets show you a sample

#include
#define STATUS_PORT 0x3F000000;
int main (void){
uint32_t* p = (uint32_t*) STATUS_PORT;
do {} while (*p != 1);
}

Now don't worry about the address the code will fail if you have an optimizer turned on ... it will produce this

main:
mov r3, #1056964608
ldr r3, [r3]
cmp r3, #1
beq .L2
.L3:
b .L3
.L2:
mov r0, #0
bx lr

Hopefully you see the problem with the branch L3 loop its a deadloop doing nothing ... so you may ask why does it do it. Well the code read the port at line 2 with ldr r3, [r3] As far as the optimizer is concerned the value never changes so it thinks you are just asking for a deadloop with the while loop. The optimizer has no way to know the port value can change independent of the running code. What the volatile does is tell the optimizer the value can change without it knowing so it must read it everytime so lets do this

#include
#define STATUS_PORT 0x3F000000;
int main (void){
volatile uint32_t* p = (uint32_t*) STATUS_PORT; // We Add volatile to the pointer
do {} while (*p != 1);
}

Now what you get is this

main:
mov r2, #1056964608
.L2:
ldr r3, [r2]
cmp r3, #1
bne .L2
mov r0, #0
bx lr

See the difference in the L2 loop it reads the port everytime So what the volatile is protecting you from is the optimizer making bad assumptions that the registers can't change. That is all it's doing it makes sure that the optimizer knows the register value can change without the optimizer knowing. Now it doesn't have to be a single register so long as you use the pointer anything at the pointer is volatile so consider this

#include
#define STATUS_PORT 0x3F000000;

struct mystruct {
uint32_t port1;
uint32_t port2;
};

int main(void) {
struct mystruct* p = (struct mystruct*)STATUS_PORT;
do {} while (p->port1 != 1);
do {} while (p->port2 != 2);
}

Again it gets it wrong on both ports ... see L3 and L5 .. deadloops again doing nothing

main:
mov r3, #1056964608
ldr r2, [r3]
cmp r2, #1
beq .L2
.L3:
b .L3
.L2:
ldr r3, [r3, #4]
cmp r3, #2
beq .L4
.L5:
b .L5
.L4:
mov r0, #0
bx lr

Now put a volatile on the pointer

#include
#define STATUS_PORT 0x3F000000;

struct mystruct {
uint32_t port1;
uint32_t port2;
};

int main(void) {
volatile struct mystruct* p = (str

supercat9

Vaclav_ wrote:

Here is something which MAY explain the issue. https://gcc.gnu.org/onlinedocs/gcc/Volatiles.html

I don't think that's the issue here, but it does represent something I wish the Standard would address. Although there may be some platforms and application fields for which gcc's behavior would be reasonable in a quality compiler, there are many purposes for which it is not. On many platforms, it is possible for an access to a volatile location to trigger operations that usefully affect other storage (e.g. starting an "in-place" background I/O operation). For an implementation to be suitable for systems-programming on such a platform, it must provide a way of ensuring that such operations are sequenced relative to other operations on non-qualified storage. An implementation can support systems programmings on such platforms without requiring the use of special directives by treating volatile accesses as triggering a call to an unknown function, and I would suggest that quality implementations for such platforms should provide an option to treat them in such fashion. Unfortunately, even though the Standard has to date expressly avoided quality-of-implementation issues, the authors of gcc seem to think either that the Standard fully describes everything necessary to make something a quality implementation, or that users should not expect gcc to behave like a quality implementation when any of its optimizations are enabled. While there might some cases where it might be unnecessarily expensive to treat volatile accesses as sequenced relative to non-qualified accesses to objects that would be accessible by outside code, in most cases the cost would be negligible, and would be less than the cost of adding "volatile" qualifiers and accesses everywhere else that would otherwise be necessary to ensure correct semantics. When the Standard was written, it may have been reasonable to expect compiler writers to exercise good judgment about how quality compilers intended for various purposes should be expected to behave in circumstances beyond those mandated by the Standard, and for programmers to be reliant upon compiler writers' sound judgment. Such expectation and reliance are no longer tenable. If the authors of the Standard don't want to mandate that all compilers treat "volatile" more strongly, they should at minimum specify a predefined macro to allow programmers to say something like:

#if !(\_\_STDC\_VOLATILE\_SEMANTIC