diff mbox

am335x-pru-package: new package

Message ID 1392255830-27810-1-git-send-email-fhunleth@troodon-software.com
State Superseded
Headers show

Commit Message

Frank Hunleth Feb. 13, 2014, 1:43 a.m. UTC 
am335x-pru-package provides an assembler and program loader for Texas
Instrument's AM335x programmable real-time units.

Signed-off-by: Frank Hunleth <fhunleth@troodon-software.com>
---
 package/Config.in                                |  1 +
 package/Config.in.host                           |  1 +
 package/am335x-pru-package/Config.in             |  7 +++++
 package/am335x-pru-package/Config.in.host        |  7 +++++
 package/am335x-pru-package/am335x-pru-package.mk | 40 ++++++++++++++++++++++++
 5 files changed, 56 insertions(+)
 create mode 100644 package/am335x-pru-package/Config.in
 create mode 100644 package/am335x-pru-package/Config.in.host
 create mode 100644 package/am335x-pru-package/am335x-pru-package.mk

Comments

Yann E. MORIN Feb. 23, 2014, 7:19 p.m. UTC | #1 
Franck, All,

On 2014-02-12 20:43 -0500, Frank Hunleth spake thusly:
> am335x-pru-package provides an assembler and program loader for Texas
> Instrument's AM335x programmable real-time units.
> 
> Signed-off-by: Frank Hunleth <fhunleth@troodon-software.com>

Thanks for your contribution, and sorry for the delay.

Here are a few comments, below.

[--SNIP--]
> diff --git a/package/am335x-pru-package/Config.in.host b/package/am335x-pru-package/Config.in.host
> new file mode 100644
> index 0000000..283128e
> --- /dev/null
> +++ b/package/am335x-pru-package/Config.in.host
> @@ -0,0 +1,7 @@
> +config BR2_PACKAGE_HOST_AM335X_PRU_PACKAGE
> +	bool "host am335x-pru-package"
> +	depends on BR2_arm # only relevant for TI am335x
> +	help
> +	  TI AM335X PRU assembler (pasm)
> +
> +	  https://github.com/beagleboard/am335x_pru_package

Is there any special reason this is a user-visible option?

Does it make sense to build the host variant without building the target
variant?

> diff --git a/package/am335x-pru-package/am335x-pru-package.mk b/package/am335x-pru-package/am335x-pru-package.mk
> new file mode 100644
> index 0000000..f70e679
> --- /dev/null
> +++ b/package/am335x-pru-package/am335x-pru-package.mk
> @@ -0,0 +1,40 @@
> +################################################################################
> +#
> +# am335x-pru-package
> +#
> +################################################################################
> +
> +AM335X_PRU_PACKAGE_VERSION = f46d8cb564f492740ccd33a03e8368ee4ecc83ae
> +AM335X_PRU_PACKAGE_SITE = $(call github,beagleboard,am335x_pru_package,$(AM335X_PRU_PACKAGE_VERSION))
> +AM335X_PRU_PACKAGE_LICENSE = BSD-3c
> +AM335X_PRU_PACKAGE_LICENSE_FILES = pru_sw/utils/LICENCE.txt
> +AM335X_PRU_PACKAGE_DEPENDENCIES = host-am335x-pru-package
> +AM335X_PRU_PACKAGE_INSTALL_STAGING = YES
> +
> +define AM335X_PRU_PACKAGE_BUILD_CMDS
> +	$(MAKE) CROSS_COMPILE="$(TARGET_CROSS)" \
> +		-C $(@D)/pru_sw/app_loader/interface all

Usually, 'all' is the default target for make, so it is not required to
specify it. You only need to specify it if it is not the default.

> +endef
> +
> +define AM335X_PRU_PACKAGE_INSTALL_STAGING_CMDS
> +	$(MAKE1) PREFIX="$(STAGING_DIR)/usr" \
> +		-C $(@D)/pru_sw/app_loader/interface install
> +endef
> +
> +define AM335X_PRU_PACKAGE_INSTALL_TARGET_CMDS
> +	$(INSTALL) -m 0755 -D $(@D)/pru_sw/app_loader/lib/libprussdrv.so \
> +		$(TARGET_DIR)/usr/lib

You need to pass the full intall path, not only the directory name.
If $(TARGET_DIR)/usr/lib does not exist, then install would simply copy
libprussdrv.so to the file $(TARGET_DIR)/usr/lib.

    $(INSTALL) -m 0755 -D $(@D)/pru_sw/app_loader/lib/libprussdrv.so \
    	$(TARGET_DIR)/usr/lib/libprussdrv.so

> +endef
> +
> +define HOST_AM335X_PRU_PACKAGE_BUILD_CMDS
> +	cd $(@D)/pru_sw/utils/pasm_source && \
> +		$(HOSTCC) -Wall -D_UNIX_ pasm.c pasmpp.c pasmexp.c pasmop.c \
> +			pasmdot.c pasmstruct.c pasmmacro.c -o ../pasm

Any reason you output in the parent directory?
Also, no indentation on the second line.

Why not:
    cd $(@D)/pru_sw/utils/pasm_source && \
    $(HOSTCC) -Wall -D_UNIX_ pasm.c pasmpp.c pasmexp.c pasmop.c \
    	pasmdot.c pasmstruct.c pasmmacro.c -o pasm

Regards,
Yann E. MORIN.
Frank Hunleth Feb. 25, 2014, 4:31 p.m. UTC | #2 
Hi Yann,

On Sun, Feb 23, 2014 at 2:19 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> Franck, All,
>
> On 2014-02-12 20:43 -0500, Frank Hunleth spake thusly:
>> am335x-pru-package provides an assembler and program loader for Texas
>> Instrument's AM335x programmable real-time units.
>>
>> Signed-off-by: Frank Hunleth <fhunleth@troodon-software.com>
>
> Thanks for your contribution, and sorry for the delay.

No problem regarding the delay. There have been some recent
discussions around this project that may affect the buildroot
integration, so I'm waiting on those to be resolved as well. I'll be
sending the next patch updates hopefully in the next week or so as I
understand more about what will be happening.

>
> Here are a few comments, below.
>
> [--SNIP--]
>> diff --git a/package/am335x-pru-package/Config.in.host b/package/am335x-pru-package/Config.in.host
>> new file mode 100644
>> index 0000000..283128e
>> --- /dev/null
>> +++ b/package/am335x-pru-package/Config.in.host
>> @@ -0,0 +1,7 @@
>> +config BR2_PACKAGE_HOST_AM335X_PRU_PACKAGE
>> +     bool "host am335x-pru-package"
>> +     depends on BR2_arm # only relevant for TI am335x
>> +     help
>> +       TI AM335X PRU assembler (pasm)
>> +
>> +       https://github.com/beagleboard/am335x_pru_package
>
> Is there any special reason this is a user-visible option?
>
> Does it make sense to build the host variant without building the target
> variant?

I don't think so. I'll remove the host variant from being user visible.

>
>> diff --git a/package/am335x-pru-package/am335x-pru-package.mk b/package/am335x-pru-package/am335x-pru-package.mk
>> new file mode 100644
>> index 0000000..f70e679
>> --- /dev/null
>> +++ b/package/am335x-pru-package/am335x-pru-package.mk
>> @@ -0,0 +1,40 @@
>> +################################################################################
>> +#
>> +# am335x-pru-package
>> +#
>> +################################################################################
>> +
>> +AM335X_PRU_PACKAGE_VERSION = f46d8cb564f492740ccd33a03e8368ee4ecc83ae
>> +AM335X_PRU_PACKAGE_SITE = $(call github,beagleboard,am335x_pru_package,$(AM335X_PRU_PACKAGE_VERSION))
>> +AM335X_PRU_PACKAGE_LICENSE = BSD-3c
>> +AM335X_PRU_PACKAGE_LICENSE_FILES = pru_sw/utils/LICENCE.txt
>> +AM335X_PRU_PACKAGE_DEPENDENCIES = host-am335x-pru-package
>> +AM335X_PRU_PACKAGE_INSTALL_STAGING = YES
>> +
>> +define AM335X_PRU_PACKAGE_BUILD_CMDS
>> +     $(MAKE) CROSS_COMPILE="$(TARGET_CROSS)" \
>> +             -C $(@D)/pru_sw/app_loader/interface all
>
> Usually, 'all' is the default target for make, so it is not required to
> specify it. You only need to specify it if it is not the default.

I'll double check whether it's the default now. If it isn't, I'll
submit a fix upstream since I agree that it should be.

>
>> +endef
>> +
>> +define AM335X_PRU_PACKAGE_INSTALL_STAGING_CMDS
>> +     $(MAKE1) PREFIX="$(STAGING_DIR)/usr" \
>> +             -C $(@D)/pru_sw/app_loader/interface install
>> +endef
>> +
>> +define AM335X_PRU_PACKAGE_INSTALL_TARGET_CMDS
>> +     $(INSTALL) -m 0755 -D $(@D)/pru_sw/app_loader/lib/libprussdrv.so \
>> +             $(TARGET_DIR)/usr/lib
>
> You need to pass the full intall path, not only the directory name.
> If $(TARGET_DIR)/usr/lib does not exist, then install would simply copy
> libprussdrv.so to the file $(TARGET_DIR)/usr/lib.
>
>     $(INSTALL) -m 0755 -D $(@D)/pru_sw/app_loader/lib/libprussdrv.so \
>         $(TARGET_DIR)/usr/lib/libprussdrv.so

Thanks for catching that.

>
>> +endef
>> +
>> +define HOST_AM335X_PRU_PACKAGE_BUILD_CMDS
>> +     cd $(@D)/pru_sw/utils/pasm_source && \
>> +             $(HOSTCC) -Wall -D_UNIX_ pasm.c pasmpp.c pasmexp.c pasmop.c \
>> +                     pasmdot.c pasmstruct.c pasmmacro.c -o ../pasm
>
> Any reason you output in the parent directory?

Only to have identical behavior to the non-so-useful build script
included in the project. In hindsight, this doesn't seem that
important anymore. Also, I think that someone is trying to get a
proper Makefile submitted upstream for building pasm. If it gets
accepted in the next couple weeks, I'll hopefully be able to use it.

> Also, no indentation on the second line.
>
> Why not:
>     cd $(@D)/pru_sw/utils/pasm_source && \
>     $(HOSTCC) -Wall -D_UNIX_ pasm.c pasmpp.c pasmexp.c pasmop.c \
>         pasmdot.c pasmstruct.c pasmmacro.c -o pasm
>
> Regards,
> Yann E. MORIN.
>
> --
> .-----------------.--------------------.------------------.--------------------.
> |  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> | +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
> | +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
> '------------------------------^-------^------------------^--------------------'
Yann E. MORIN Oct. 12, 2014, 10:50 a.m. UTC | #3 
Frank, All,

On 2014-02-12 20:43 -0500, Frank Hunleth spake thusly:
> am335x-pru-package provides an assembler and program loader for Texas
> Instrument's AM335x programmable real-time units.

I've just sent an updated version of that patch:
    http://patchwork.ozlabs.org/patch/398921/

Regards,
Yann E. MORIN.
Frank Hunleth Oct. 12, 2014, 1:02 p.m. UTC | #4 
Hi Yann,

On Sun, Oct 12, 2014 at 6:50 AM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>
> Frank, All,
>
> On 2014-02-12 20:43 -0500, Frank Hunleth spake thusly:
> > am335x-pru-package provides an assembler and program loader for Texas
> > Instrument's AM335x programmable real-time units.
>
> I've just sent an updated version of that patch:
>     http://patchwork.ozlabs.org/patch/398921/

Thanks for updating the patch! It seems like it's been a while since I
worked on the project that required it, but it should help anyone
using the PRU get going in the future. I'm really thankful for all of
the work that you and everyone else put into this project.

Frank
diff mbox

Patch

diff --git a/package/Config.in b/package/Config.in
index fca61d6..71bd2d1 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -260,6 +260,7 @@  source "package/zd1211-firmware/Config.in"
 endmenu
 source "package/a10disp/Config.in"
 source "package/acpid/Config.in"
+source "package/am335x-pru-package/Config.in"
 source "package/avrdude/Config.in"
 source "package/cdrkit/Config.in"
 source "package/cryptsetup/Config.in"
diff --git a/package/Config.in.host b/package/Config.in.host
index ac6091f..a364f4e 100644
--- a/package/Config.in.host
+++ b/package/Config.in.host
@@ -1,5 +1,6 @@ 
 menu "Host utilities"
 
+source "package/am335x-pru-package/Config.in.host"
 source "package/dfu-util/Config.in.host"
 source "package/dosfstools/Config.in.host"
 source "package/e2fsprogs/Config.in.host"
diff --git a/package/am335x-pru-package/Config.in b/package/am335x-pru-package/Config.in
new file mode 100644
index 0000000..66d7773
--- /dev/null
+++ b/package/am335x-pru-package/Config.in
@@ -0,0 +1,7 @@ 
+config BR2_PACKAGE_AM335X_PRU_PACKAGE
+	bool "am335x-pru-package"
+	depends on BR2_arm # only relevant for TI am335x
+	help
+	  TI AM335X PRU program loader
+
+	  https://github.com/beagleboard/am335x_pru_package
diff --git a/package/am335x-pru-package/Config.in.host b/package/am335x-pru-package/Config.in.host
new file mode 100644
index 0000000..283128e
--- /dev/null
+++ b/package/am335x-pru-package/Config.in.host
@@ -0,0 +1,7 @@ 
+config BR2_PACKAGE_HOST_AM335X_PRU_PACKAGE
+	bool "host am335x-pru-package"
+	depends on BR2_arm # only relevant for TI am335x
+	help
+	  TI AM335X PRU assembler (pasm)
+
+	  https://github.com/beagleboard/am335x_pru_package
diff --git a/package/am335x-pru-package/am335x-pru-package.mk b/package/am335x-pru-package/am335x-pru-package.mk
new file mode 100644
index 0000000..f70e679
--- /dev/null
+++ b/package/am335x-pru-package/am335x-pru-package.mk
@@ -0,0 +1,40 @@ 
+################################################################################
+#
+# am335x-pru-package
+#
+################################################################################
+
+AM335X_PRU_PACKAGE_VERSION = f46d8cb564f492740ccd33a03e8368ee4ecc83ae
+AM335X_PRU_PACKAGE_SITE = $(call github,beagleboard,am335x_pru_package,$(AM335X_PRU_PACKAGE_VERSION))
+AM335X_PRU_PACKAGE_LICENSE = BSD-3c
+AM335X_PRU_PACKAGE_LICENSE_FILES = pru_sw/utils/LICENCE.txt
+AM335X_PRU_PACKAGE_DEPENDENCIES = host-am335x-pru-package
+AM335X_PRU_PACKAGE_INSTALL_STAGING = YES
+
+define AM335X_PRU_PACKAGE_BUILD_CMDS
+	$(MAKE) CROSS_COMPILE="$(TARGET_CROSS)" \
+		-C $(@D)/pru_sw/app_loader/interface all
+endef
+
+define AM335X_PRU_PACKAGE_INSTALL_STAGING_CMDS
+	$(MAKE1) PREFIX="$(STAGING_DIR)/usr" \
+		-C $(@D)/pru_sw/app_loader/interface install
+endef
+
+define AM335X_PRU_PACKAGE_INSTALL_TARGET_CMDS
+	$(INSTALL) -m 0755 -D $(@D)/pru_sw/app_loader/lib/libprussdrv.so \
+		$(TARGET_DIR)/usr/lib
+endef
+
+define HOST_AM335X_PRU_PACKAGE_BUILD_CMDS
+	cd $(@D)/pru_sw/utils/pasm_source && \
+		$(HOSTCC) -Wall -D_UNIX_ pasm.c pasmpp.c pasmexp.c pasmop.c \
+			pasmdot.c pasmstruct.c pasmmacro.c -o ../pasm
+endef
+
+define HOST_AM335X_PRU_PACKAGE_INSTALL_CMDS
+	$(INSTALL) -m 0755 -D $(@D)/pru_sw/utils/pasm $(HOST_DIR)/usr/bin/pasm
+endef
+
+$(eval $(generic-package))
+$(eval $(host-generic-package))