Makefile update to ease the loading of PRU firmware #13

deebot · 2020-08-28T12:54:48Z

The existing makefile only compile the code. It was difficult to copy the code first to lib/firmware and then to go in /sys/class/remoteproc and echo the state to start and stop. The new Makefile makes it easier. Now the user just need to do make install_PRU0 after running make and the PRU is loaded with the new firmware. Also there are situations when the firmware is already in place and only a starting or stopping of PRU core is required. In such a case start_PRU0 and stop_PRU0 can be used. For PRU 1 just change the number to 1. Hope this will be of help to other users

dinuxbg

It was difficult to copy the code first to lib/firmware and then to go in /sys/class/remoteproc and echo the state to start and stop

Could you elaborate why it is difficult? I did not expect copy-pasting a few commands to be a hurdle.

dinuxbg · 2020-08-28T20:17:05Z

hc-sr04-range-sensor/Makefile

@@ -61,6 +61,14 @@ OUT := out
 ELF0 := $(OUT)/pru-halt.elf
 ELF1 := $(OUT)/hc-sr04-range-sensor.elf

+#Installation, start stop related variable
+PRU0        :=/sys/class/remoteproc/remoteproc1


Please rename to RPROC_PRU0. Otherwise it could easily be confused with ELF0 output binary for PRU0.

Okay updating this

updated as asked

dinuxbg · 2020-08-28T20:18:36Z

hc-sr04-range-sensor/Makefile

+PRU1        :=/sys/class/remoteproc/remoteproc2
+STATESRUNNING   :=running
+STATESSTOPED   :=offline
+CURRENTSTATE0:=$(shell cat $(PRU0)/state)


This will break building firmware on a PC host.

I don't want sysfs to be poked when I'm building firmware.

ah ok good point. I will add code to check if the make file is being run on Arm or on PC host.

Added a architecture check and when on ARM only then the sysfs is pocked

dinuxbg · 2020-08-28T20:20:03Z

hc-sr04-range-sensor/Makefile

@@ -92,6 +100,70 @@ $(ELF1): $(SRC1) $(HEADERS) | $(OUT)
 clean:
 	$(RM) -fr $(ELF0) $(ELF1) $(OUT)

+install_PRU0:
+	@echo 'Installation Process Initiated'
+ifeq ($(CURRENTSTATE0),$(STATESRUNNING))


Use shell commands to achive conditional logic. Build rule lines are not restricted to simple tool invocation. You can use shell's || or && or []

Added nested conditions using shell commands

dinuxbg · 2020-08-28T20:23:00Z

hc-sr04-range-sensor/Makefile

+	@echo 'PRU0 Already in the stopped state. execute start_PRU0 to start '
+endif
+
+start_PRU0:


Please adhere to the existing convention in the Makefile all rules to be only lower case.

Rules converted to lower case

deebot · 2020-09-01T18:48:33Z

It was difficult to copy the code first to lib/firmware and then to go in /sys/class/remoteproc and echo the state to start and stop

Could you elaborate why it is difficult? I did not expect copy-pasting a few commands to be a hurdle.

So when sombody just wants to run already written code , it is just copy- pasting once. But when i was doing development and trying to write and debug new code, every time i make some changes to the code and want to try out i had to run the commands again and again. In such a case it becomes tedious to copy and paste everytime to /lib/firmware then start from remotroc directory. Also after reboot the history is gone. So just doing make install would be simpler.

dinuxbg · 2020-09-02T18:39:49Z

It was difficult to copy the code first to lib/firmware and then to go in /sys/class/remoteproc and echo the state to start and stop

Could you elaborate why it is difficult? I did not expect copy-pasting a few commands to be a hurdle.

So when sombody just wants to run already written code , it is just copy- pasting once. But when i was doing development and trying to write and debug new code, every time i make some changes to the code and want to try out i had to run the commands again and again. In such a case it becomes tedious to copy and paste everytime to /lib/firmware then start from remotroc directory. Also after reboot the history is gone. So just doing make install would be simpler.

Please include such justification in the commit message and address the rest of the comments. Thanks.

…and starting of PRU cores - These new targets will be specially helpful during development when one needs to make new changes to firmware and load new firmware start ,stop PRU multiple times.

dinuxbg · 2020-09-03T17:32:48Z

hc-sr04-range-sensor/Makefile

@@ -28,7 +31,7 @@

 # Very simple makefile to cross-compile for PRU

-
+#!/bin/bash


No need for shebang here. Please remove.

dinuxbg · 2020-09-03T17:38:33Z

hc-sr04-range-sensor/Makefile

@@ -61,6 +64,18 @@ OUT := out
 ELF0 := $(OUT)/pru-halt.elf
 ELF1 := $(OUT)/hc-sr04-range-sensor.elf

+#Installation, start stop related variable
+ARCHT   :=$(shell uname -m)


Macros and $(shell) invocations should be avoided. Especially if you are writing example code. See below for suggestion how to remove this line.

dinuxbg · 2020-09-03T17:42:11Z

hc-sr04-range-sensor/Makefile

@@ -92,6 +107,98 @@ $(ELF1): $(SRC1) $(HEADERS) | $(OUT)
 clean:
 	$(RM) -fr $(ELF0) $(ELF1) $(OUT)

+stop_pru0:
+	@if [ $(ARCHT) = $(ARM) ]; then\


If I do "make stop_pru0" on x86 it will do nothing but will declare success. That is misleading. I would instead suggest you to do the following:

HOST_CHECK_CMD:=[ `uname -m` = armv7l ] || { echo "ERROR: Host is not a BeagleBone board! Cannot install PRU firmware. Bailing out."; false; } stop_pru0: @$(HOST_CHECK_CMD) @echo "Do some work here." @echo "Next command."

IMHO it is more clear to read, and will fail if unable to service the requested action.

When i ran "make stop_pru0" on x86 i got the message. "Architecture doesnot support functionality. Run on Beaglebone"

https://pastebin.com/mcDgiwFw

But still if you would me to do it as you suggested. I would change and update the code. :)

Yes, sorry, your version would also print error message. But you see, that's why long macros are bad - they are hard to read. I missed the error message at the end. Readability is even more important for example code.

I think that regular Makefile commands are much more readable, as I suggested above.

I am trying to make changes as you suggested and since the last 1 hour i am stuck with a error. It should have been straight forward. Can you please look at this snippet from my make file and help me resolve the error.
https://pastebin.com/00BRQnHx

@deebot , I cannot open the link you have provided.The snippet I provided works - I tested it on beaglebone and x86_64 host.

deebot changed the title ~~New Makefile to ease the loading of PRU firmware~~ Makefile update to ease the loading of PRU firmware Aug 28, 2020

dinuxbg requested changes Aug 28, 2020

View reviewed changes

Updated Readme to use new features of makefile

9bbb073

deebot force-pushed the newMake branch from ccced98 to 51d8ff5 Compare September 3, 2020 04:26

- Added logic to makefile that eases loading of firmware and stoping …

404f0c0

…and starting of PRU cores - These new targets will be specially helpful during development when one needs to make new changes to firmware and load new firmware start ,stop PRU multiple times.

deebot force-pushed the newMake branch from 51d8ff5 to 404f0c0 Compare September 3, 2020 04:43

dinuxbg requested changes Sep 3, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Makefile update to ease the loading of PRU firmware #13

Makefile update to ease the loading of PRU firmware #13

deebot commented Aug 28, 2020

dinuxbg left a comment

dinuxbg Aug 28, 2020

deebot Sep 1, 2020

deebot Sep 3, 2020

dinuxbg Aug 28, 2020

deebot Sep 1, 2020

deebot Sep 3, 2020

dinuxbg Aug 28, 2020

deebot Sep 3, 2020

dinuxbg Aug 28, 2020

deebot Sep 3, 2020

deebot commented Sep 1, 2020 •

edited

Loading

dinuxbg commented Sep 2, 2020

dinuxbg Sep 3, 2020

dinuxbg Sep 3, 2020

dinuxbg Sep 3, 2020

deebot Sep 3, 2020

dinuxbg Sep 4, 2020

deebot Sep 6, 2020

dinuxbg Sep 6, 2020

		@@ -28,7 +31,7 @@

		# Very simple makefile to cross-compile for PRU


		#!/bin/bash

Makefile update to ease the loading of PRU firmware #13

Are you sure you want to change the base?

Makefile update to ease the loading of PRU firmware #13

Conversation

deebot commented Aug 28, 2020

dinuxbg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deebot commented Sep 1, 2020 • edited Loading

dinuxbg commented Sep 2, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deebot commented Sep 1, 2020 •

edited

Loading