Weird C problem: PRU doesn't respond to host if two variables aren't initialized in loop

GoogleGroups
1

I’ve been pulling my hair out over a really weird problem and after trying everything I know to try, I’m posting it here in hopes of someone seeing the problem.

I am running a Beaglebone Black. The output of version.sh is at the end of this post.

Linux beaglebone 4.19.94-ti-r61 #1buster SMP PREEMPT Wed Mar 31 15:23:20 UTC 2021 armv7l GNU/Linux

I boot from an SD card.

I’m using remoteproc to communicate between a host program and the PRU code.

The program is long and includes stuff that I don’t think is important here like configured the ADC and IEP counter.

The host program lets the user choose from a menu what they want the PRU to do. Then using RPMSG, the command is sent to the PRU. It receives the message, determines what it is and executes the appropriate action in the PRU code. This is done by reading the message and using an “if-else if-else”. One of the actions is to read three analog inputs and return the values read through RPMSG. Each analog value is stored in a ring buffer-type array, type unsigned int, that is allocated in the PRU Shared memory space.

So once the host sends the message to do the analog input read, it starts to listen for responses from the PRU.

This all works great. I get data from the PRU and the host program puts it in a CSV file that I can analyze with other tools.

Now, though we need to do some basic math on the data in the PRU. I only need to do this on a subset of the data in the arrays and the start and end points in the array can vary. So I have two integer variables that I use to store the start and end points. So, the routine that reads the analog data sets the values of these two variables as voltage thresholds are met. The code compiles just fine.

However, if these two variables are not set to a constant within the loop, the host never gets a message from the PRU code. They are set to initial values before the loop (that code is not included below.)

It’s the craziest thing I’ve ever run into I think. I have declared these variables as local and global and nothing seems to matter. If they are set to a constant in the loop, the host doesn’t get a message from the PRU program.

Here’s the code snippet with the two variables in bold. If those lines of code do not exist, the host doesn’t hear from the PRU.

count = HWREG(SOC_ADC_TSC_0_REGS + TSC_ADC_SS_FIFOCOUNT(0));

for (i = 0; i < count; i++)

{

Data = HWREG(SOC_ADC_TSC_0_REGS + TSC_ADC_SS_FIFODATA(0));

StepRead = (Data >> 16) & 0xF;

RawAnalog = Data & 0xFFF;

switch (StepRead)

{

case 0:

DetTSampleSet[pnr]=RawAnalog;

break;

case 1:

start_of_pulse = 0;

end_of_pulse = 0;

DetBSampleSet[pnr]=RawAnalog;

if ((pnr == end_of_pulse) && (in_pulse == E_YES)) // seen a pulse and at end of it analyze signal

{

DetBSignalAverage= AnalyzeSignal(start_of_pulse, pnr);

start_of_pulse = -1;

end_of_pulse = -1;

samples_in_pulse = 0;

in_pulse = E_NO;

}

else

{

DetBSignalAverage = 0;

}

if (RawAnalog > 0xB54)

{

__R30 |= PanelYellowLED; // turn yellow LED on; at the sampling rate we’re using this may result in just a flicker

in_pulse = E_YES; // once the leading edge of the pulse passes set this

samples_in_pulse++; // count the number of samples in the pulse

DetBSignalAverage = 999; // samples_in_pulse;//just using for debugging

//

// here we will check for a max # of samples in pulse while in a pulse to throw out start of fluid or end of fluid or pulses larger than a seed

//

if (start_of_pulse < 0) start_of_pulse = pnr; // set start pointer in ring buffer if it hasn’t already been set for this pulse

}

else if ((RawAnalog <= 0xAC8) && (in_pulse == E_YES))

{

__R30 &= ~CAValve; // turn compressed air off; stop fluid flow because the pulse has cleared the bottom detector

in_pulse = E_NO; // pulse is over

DetBSignalAverage = 1999; //samples_in_pulse;//just using for debugging

//

// this is an alternative place to check the number of samples in the pulse to throw out start / end of fluid but we would never stop because Rawanalog

// will never drop while there is no fluid in the tube

//

if (end_of_pulse < 0) end_of_pulse = pnr; // capture where we are in the ring buffer at the end of the pulse

__R30 |= VacValve; // turn vacuum on to hold fluid in place; ultimately this would read the pressure and attempt to hold it there

__delay_cycles(40000000); // put this delay cycle here for now. will be replaced with a routine to analyze pulse & maintain slight vacuum later

__R30 &= ~VacValve;

__delay_cycles(40000000); // put this delay cycle here for now. will be replaced with a routine to analyze pulse later

__R30 |= CAValve;

}

else __R30 &= ~PanelYellowLED; // turn yellow LED off

StepRead = RawAnalog;

RawAnalog = DetBSignalAverage;

break;

case 2:

Pressure = RawAnalog;

if (pnr == E_RING_BUFFER_SIZE-1)

{

pnr = 0;

}

else

{

pnr++;

}

RawAnalog=pnr;

break;

default: // flash both LEDS indicating a problem

for (i=0;i<20;i++)

{

__R30 |= PanelGreenLED; // turn green LED on

__delay_cycles(10000000);

__R30 |= PanelYellowLED; // turn green LED on

__delay_cycles(20000000);

__R30 &= ~PanelGreenLED; // turn green LED off

__delay_cycles(30000000);

__R30 &= ~PanelYellowLED; // turn yellow LED off

__delay_cycles(40000000);

}

break;

}

git:/opt/scripts/:[b39ec679648a6be8f25f48bd1c9784c1fc5a0c46]

eeprom:[A335BNLT00C04417BBBK1847]

model:[TI_AM335x_BeagleBone_Black]

dogtag:[BeagleBoard.org Debian Buster IoT Image 2020-04-06]

bootloader:[microSD-(push-button)]:[/dev/mmcblk0]:[U-Boot 2019.04-00002-g07d5700e21]:[location: dd MBR]

bootloader:[eMMC-(default)]:[/dev/mmcblk1]:[U-Boot 2018.03-00002-gac9cce7c6a]:[location: dd MBR]

UBOOT: Booted Device-Tree:[am335x-boneblack-uboot-univ.dts]

UBOOT: Loaded Overlay:[AM335X-PRU-RPROC-4-19-TI-00A0]

UBOOT: Loaded Overlay:[BB-ADC-00A0]

UBOOT: Loaded Overlay:[BB-BONE-eMMC1-01-00A0]

UBOOT: Loaded Overlay:[BB-I2C2-RTC-DS3231]

UBOOT: Loaded Overlay:[BB-W1-P9.12-00A2]

kernel:[4.19.94-ti-r61]

nodejs:[v10.15.2]

/boot/uEnv.txt Settings:

uboot_overlay_options:[enable_uboot_overlays=1]

uboot_overlay_options:[uboot_overlay_addr4=/lib/firmware/BB-W1-P9.12-00A0.dtbo]

uboot_overlay_options:[disable_uboot_overlay_video=1]

uboot_overlay_options:[disable_uboot_overlay_audio=1]

uboot_overlay_options:[uboot_overlay_pru=/lib/firmware/AM335X-PRU-RPROC-4-19-TI-00A0.dtbo]

uboot_overlay_options:[enable_uboot_cape_universal=1]

uboot_overlay_options:[dtb_overlay=/lib/firmware/BB-I2C2-RTC-DS3231.dtbo]

pkg check: to individually upgrade run: [sudo apt install --only-upgrade ]

pkg:[bb-cape-overlays]:[4.14.20210401.0-0~buster+20210401]

pkg:[bb-wl18xx-firmware]:[1.20200322.0-0rcnee0~buster+20200322]

pkg:[kmod]:[26-1]

pkg:[librobotcontrol]:[1.0.4-git20190227.1-0rcnee0~buster+20190327]

pkg:[firmware-ti-connectivity]:[20190717-2rcnee1~buster+20200305]

groups:[debian : debian adm kmem dialout cdrom floppy audio dip video plugdev users systemd-journal bluetooth netdev i2c gpio pwm eqep remoteproc admin spi iio docker tisdk weston-launch xenomai cloud9ide]

cmdline:[console=ttyO0,115200n8 bone_capemgr.uboot_capemgr_enabled=1 root=/dev/mmcblk0p1 ro rootfstype=ext4 rootwait coherent_pool=1M net.ifnames=0 lpj=1990656 rng_core.default_quality=100 quiet]

dmesg | grep remote

[ 70.424168] remoteproc remoteproc0: wkup_m3 is available

[ 70.537289] remoteproc remoteproc0: powering up wkup_m3

[ 70.537322] remoteproc remoteproc0: Booting fw image am335x-pm-firmware.elf, size 217148

[ 70.537592] remoteproc remoteproc0: remote processor wkup_m3 is now up

[ 72.807404] remoteproc remoteproc1: 4a334000.pru is available

[ 72.825531] remoteproc remoteproc2: 4a338000.pru is available

dmesg | grep pru

[ 72.807404] remoteproc remoteproc1: 4a334000.pru is available

[ 72.811832] pru-rproc 4a334000.pru: PRU rproc node pru@4a334000 probed successfully

[ 72.825531] remoteproc remoteproc2: 4a338000.pru is available

[ 72.825738] pru-rproc 4a338000.pru: PRU rproc node pru@4a338000 probed successfully

dmesg | grep pinctrl-single

[ 0.951001] pinctrl-single 44e10800.pinmux: 142 pins, size 568

dmesg | grep gpio-of-helper

[ 0.964886] gpio-of-helper ocp:cape-universal: ready

lsusb

Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub

END

2

Hi Walter

Probally unrelated but I wanted to share I saw if the linker command files didn’t include startup code to initialize variables or zero them like the ARM does.A huge uncleaned index intyo an array wouldn’t be good.

Perhaps this PRUDebug tool can speed up your debugging have not tried it.

Mark

3

I could never get the PRUDebug tool to compile and run. I’ll give it another try.

What’s really weird is that if I set these two variables to values like this for example.

start_of_pulse = 5;
end_of_pulse = 15;

Then it runs just fine. I actually send the data back through RPMSG and it’s good.

It doesn’t matter what the actual numbers are. It can be -1, 0, anything. But if I try something like this.

start_of_pulse = start_of_pulse + 5;
start_of_pulse = start_of_pulse - 5; // this is sort of a no change change.

It just doesn’t send anything back. No response from the PRU at all.

4

I renamed the variables start_of_pulse and end_of_pulse to just PulseStart and PulseEnd and it works now. I haven’t seen anything in the PRU Compiler manual about these being restricted names or symbols but I suppose they were.
Weird.

5

Here's the code snippet with the two variables in bold. If those lines of
code do not exist, the host doesn't hear from the PRU.

  Such formatting does not get through the gmane list<>newsgroup
interface. I'm going to presume you mean the lines with * markers. Posting
with a client that keeps indentation would also help... hard to keep track
of what is nested into what when everything is left justified with excess
blank lines.

  Normal recommendation is to condense the code down to the minimum that
still reproduces the problem, and to post the minimized files completely
(probably need both PRU and ARM programs). That allows others to attempt to
run/compare/debug.

count = HWREG(SOC_ADC_TSC_0_REGS + TSC_ADC_SS_FIFOCOUNT(0));

for (i = 0; i < count; i++)

{

Data = HWREG(SOC_ADC_TSC_0_REGS + TSC_ADC_SS_FIFODATA(0));

StepRead = (Data >> 16) & 0xF;

RawAnalog = Data & 0xFFF;

switch (StepRead)

{

case 0:

DetTSampleSet[pnr]=RawAnalog;

break;

case 1:

*start_of_pulse = 0;*

*end_of_pulse = 0;*

  Where were these declared?

DetBSampleSet[pnr]=RawAnalog;

if ((pnr == end_of_pulse) && (in_pulse == E_YES)) // seen a pulse and at
end of it analyze signal

  Where is pnr defined/initialized? Same for in_pulse.

{

DetBSignalAverage= AnalyzeSignal(start_of_pulse, pnr);

start_of_pulse = -1;

end_of_pulse = -1;

  If pnr is an index into some buffer, I'd probably use -1 to signal NO
DATA, and use the pnr values active at the time the pulse is detected for
start_of_pulse and the when the pulse ended for end_of_pulse

samples_in_pulse = 0;

in_pulse = E_NO;

  In a way, all these seem redundant: start&end at -1 indicates no data,
no samples, and not in a pulse. Samples_in_pulse at 0 indicates no data, no
samples, and likely not in a pulse. in_pulse at E_NO implies no data, no
samples.

  So, start&end are set to the appropriate pnr values... "in_pulse" is
indicated by start_of_pulse > -1 AND end_of_pulse = -1; "not in_pulse" is
indicated by (start_of_pulse > -1 AND end_of_pulse > -1) OR (start_of_pulse
= -1) //presumes you set both start/end to -1 at the same time

if (start_of_pulse < 0) start_of_pulse = pnr; // set start pointer in ring
buffer if it hasn't already been set for this pulse

  Okay, you do set start/end to the instantaneous pnr value... Just
emphasizes that samples_in_pulse and in_pulse are logically redundant and
hence a potential source of error (samples_in_pulse should be end - start
(maybe with a +1; do the math with a sample buffer). Note: if this is a
circular buffer you'll need to account for wrap-around.

6

i’m replying from an email client but I lost the original and most of my replies using Chrome and directly access the group through it. I thought it might not present well but I actually highlighted with the client. If you’ll tell me what I should use instead I will start using it for future posts. I want to make them as easy to read as possible.

I left all the declarations off but mentioned they are declared as unsigned int or int.

Yes, this version likely does have some redundancy with samples_in_pulse. It is a work in progress that wasn’t making much progress because of this weird problem.

It appears now that the names start_of_pulse and end_of_pulse were the problem for some reason. I changed them to PulseStart and PulseEnd and the PRU responds now. I have no idea what the problem with those names is.

Walter

7

Were the variables declared as volatile? May be the compiler optimized them out of the loop and you might get different behaviour between DEBUG build and RELEASE build.

I am guessing the ADC data register is volatile, and this triggers.

I am guessing the compiler optimization sees two variables set but never used, so there is no need to include code to set the variables. If declared as volatile, the compiler knows the variables may be set or read elsewhere.

8

I had a similar thought and tried declaring them as volatile. No change in behavior.
I also tried making them global vs. local. No change. It was very strange.

What finally worked was changing their names entirely.
start_of_pulse became PulseStart
end_or_pulse became PulseEnd

Now the code works without the two lines of code marked with the ‘*’. So very, very strange.

9

Dennis,

Thanks again for pointing out the redundancy. Now that I have this very strange problem figured out I’m going to do some work on that and clean it up.

Walter