mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 1/4] common: remove !SANDBOX dependency for target tools
@ 2021-09-14 13:20 Ahmad Fatoum
  2021-09-14 13:20 ` [PATCH 2/4] common: add new menu " Ahmad Fatoum
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Ahmad Fatoum @ 2021-09-14 13:20 UTC (permalink / raw)
  To: barebox; +Cc: ejo, rhi, uol, renaud.barbier, Ahmad Fatoum

d4aa01503348 ("common: add dependency !SANDBOX on target tools") and
later commits disabled building the target tools with sandbox, because
the build failed when they were enabled.

This has been fixed since then. Remove the limitation, so target tools
can be cross-compiled when using the ARCH=sandbox.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/Kconfig | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/common/Kconfig b/common/Kconfig
index a9feae2ae866..222a438ee545 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -740,7 +740,6 @@ config IMD_TARGET
 
 config KERNEL_INSTALL_TARGET
 	bool
-	depends on !SANDBOX
 	prompt "Build kernel-install utility for the target"
 	help
 	  Enable this to compile the kernel-install script using the cross
@@ -983,7 +982,6 @@ config DEFAULT_ENVIRONMENT_PATH
 
 config BAREBOXENV_TARGET
 	bool
-	depends on !SANDBOX
 	prompt "build bareboxenv tool for target"
 	help
 	  'bareboxenv' is a tool to access the barebox environment from a running Linux
@@ -992,7 +990,6 @@ config BAREBOXENV_TARGET
 config BAREBOXCRC32_TARGET
 	bool
 	prompt "build bareboxcrc32 tool for target"
-	depends on !SANDBOX
 	help
 	  'bareboxcrc32' is a userspacetool to generate the crc32 checksums the same way
 	  barebox does. Say yes here to build it for the target.
-- 
2.30.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/4] common: add new menu for target tools
  2021-09-14 13:20 [PATCH 1/4] common: remove !SANDBOX dependency for target tools Ahmad Fatoum
@ 2021-09-14 13:20 ` Ahmad Fatoum
  2021-09-14 16:00   ` Roland Hieber
  2021-09-14 13:20 ` [PATCH 3/4] scripts: unify libusb.h inclusion Ahmad Fatoum
  2021-09-14 13:20 ` [PATCH 4/4] scripts: allow building USB loader tools for target as well Ahmad Fatoum
  2 siblings, 1 reply; 12+ messages in thread
From: Ahmad Fatoum @ 2021-09-14 13:20 UTC (permalink / raw)
  To: barebox; +Cc: ejo, rhi, uol, renaud.barbier, Ahmad Fatoum

We have four target tools and will add 3 more in a follow up commit.
Add a new menu to collect them.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/Kconfig  | 26 --------------------------
 scripts/Kconfig | 30 ++++++++++++++++++++++++++++++
 2 files changed, 30 insertions(+), 26 deletions(-)

diff --git a/common/Kconfig b/common/Kconfig
index 222a438ee545..9dab7532e15e 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -734,18 +734,6 @@ config IMD
 	select CRC32
 	bool "barebox metadata support"
 
-config IMD_TARGET
-	bool "build bareboximd target tool"
-	depends on IMD
-
-config KERNEL_INSTALL_TARGET
-	bool
-	prompt "Build kernel-install utility for the target"
-	help
-	  Enable this to compile the kernel-install script using the cross
-	  compiler. The utility for the target will be under
-	  scripts/kernel-install-target
-
 choice
 	prompt "console support"
 	default CONSOLE_FULL
@@ -980,20 +968,6 @@ config DEFAULT_ENVIRONMENT_PATH
 	  be taken. Relative paths will be relative to the barebox top-level
 	  directory, but absolute paths are fine as well.
 
-config BAREBOXENV_TARGET
-	bool
-	prompt "build bareboxenv tool for target"
-	help
-	  'bareboxenv' is a tool to access the barebox environment from a running Linux
-	  system. Say yes here to build it for the target.
-
-config BAREBOXCRC32_TARGET
-	bool
-	prompt "build bareboxcrc32 tool for target"
-	help
-	  'bareboxcrc32' is a userspacetool to generate the crc32 checksums the same way
-	  barebox does. Say yes here to build it for the target.
-
 config HAS_SCHED
 	bool
 
diff --git a/scripts/Kconfig b/scripts/Kconfig
index b903486ecd8f..f7ed775fbc66 100644
--- a/scripts/Kconfig
+++ b/scripts/Kconfig
@@ -40,3 +40,33 @@ config OMAP4_HOSTTOOL_USBBOOT
 	  You need libusb-1.0 to compile this tool.
 
 endmenu
+
+menu "Target Tools"
+
+config IMD_TARGET
+	bool "build bareboximd target tool"
+	depends on IMD
+
+config KERNEL_INSTALL_TARGET
+	bool
+	prompt "Build kernel-install utility for the target"
+	help
+	  Enable this to compile the kernel-install script using the cross
+	  compiler. The utility for the target will be under
+	  scripts/kernel-install-target
+
+config BAREBOXENV_TARGET
+	bool
+	prompt "build bareboxenv tool for target"
+	help
+	  'bareboxenv' is a tool to access the barebox environment from a running Linux
+	  system. Say yes here to build it for the target.
+
+config BAREBOXCRC32_TARGET
+	bool
+	prompt "build bareboxcrc32 tool for target"
+	help
+	  'bareboxcrc32' is a userspacetool to generate the crc32 checksums the same way
+	  barebox does. Say yes here to build it for the target.
+
+endmenu
-- 
2.30.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 3/4] scripts: unify libusb.h inclusion
  2021-09-14 13:20 [PATCH 1/4] common: remove !SANDBOX dependency for target tools Ahmad Fatoum
  2021-09-14 13:20 ` [PATCH 2/4] common: add new menu " Ahmad Fatoum
@ 2021-09-14 13:20 ` Ahmad Fatoum
  2021-09-14 19:22   ` Trent Piepho
  2021-09-14 13:20 ` [PATCH 4/4] scripts: allow building USB loader tools for target as well Ahmad Fatoum
  2 siblings, 1 reply; 12+ messages in thread
From: Ahmad Fatoum @ 2021-09-14 13:20 UTC (permalink / raw)
  To: barebox; +Cc: ejo, rhi, uol, renaud.barbier, Ahmad Fatoum

We have three host tools using libusb:
imx-usb-loader omap3-usb-loader and omap4_usbboot

All three link against it using `pkg-config --libs libusb-1.0`. All
three also do `pkg-config --cflags libusb-1.0`, but omap3-usb-loader
doesn't actually use it and assumes that <libusb-1.0/libusb.h> will resolve
to something that's correct.

I've also run into downstream patches around broken toolchains that
change <libusb.h> to <libusb-1.0/libusb.h>, so the header is found.

Fix the first issue and workaround the broken toolchains by using GCC 5.0+
__has_include to fallback to <libusb-1.0/libusb.h> if it doesn't exist
or if GCC is older than 5.0.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 scripts/imx/imx-usb-loader.c |  2 +-
 scripts/libusb.h             | 17 +++++++++++++++++
 scripts/omap3-usb-loader.c   |  2 +-
 scripts/omap4_usbboot.c      |  2 +-
 4 files changed, 20 insertions(+), 3 deletions(-)
 create mode 100644 scripts/libusb.h

diff --git a/scripts/imx/imx-usb-loader.c b/scripts/imx/imx-usb-loader.c
index cff77f27f264..78af2b61bb06 100644
--- a/scripts/imx/imx-usb-loader.c
+++ b/scripts/imx/imx-usb-loader.c
@@ -29,12 +29,12 @@
 #include <errno.h>
 #include <string.h>
 #include <stdlib.h>
-#include <libusb.h>
 #include <getopt.h>
 #include <arpa/inet.h>
 #include <linux/kernel.h>
 
 #include "../compiler.h"
+#include "../libusb.h"
 #include "imx.h"
 
 #define get_min(a, b) (((a) < (b)) ? (a) : (b))
diff --git a/scripts/libusb.h b/scripts/libusb.h
new file mode 100644
index 000000000000..892d5422c451
--- /dev/null
+++ b/scripts/libusb.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef SCRIPTS_LIBUSB_H_
+#define SCRIPTS_LIBUSB_H_
+
+#ifdef __has_include
+/* not using && to keep compatibility with older compilers */
+#if __has_include("<libusb.h>")
+#include <libusb.h>
+#define HAS_LIBUSB_H
+#endif
+#endif
+
+#ifndef HAS_LIBUSB_H
+#include <libusb-1.0/libusb.h>
+#endif
+
+#endif
diff --git a/scripts/omap3-usb-loader.c b/scripts/omap3-usb-loader.c
index 599a93856ac3..edc46938d9ba 100644
--- a/scripts/omap3-usb-loader.c
+++ b/scripts/omap3-usb-loader.c
@@ -28,7 +28,7 @@
 #include <errno.h>
 #include <libgen.h>		/* for basename */
 
-#include <libusb-1.0/libusb.h>		/* the main event */
+#include "libusb.h"
 
 /* Device specific defines (OMAP)
  * Primary source: http://www.ti.com/lit/pdf/sprugn4
diff --git a/scripts/omap4_usbboot.c b/scripts/omap4_usbboot.c
index 342efd0c9a40..71e4f0acd1fd 100644
--- a/scripts/omap4_usbboot.c
+++ b/scripts/omap4_usbboot.c
@@ -10,7 +10,7 @@
 #include <fcntl.h>
 #include <string.h>
 #include <sys/mman.h>
-#include <libusb.h>
+#include "libusb.h"
 #include <pthread.h>
 #include <termios.h>
 
-- 
2.30.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 4/4] scripts: allow building USB loader tools for target as well
  2021-09-14 13:20 [PATCH 1/4] common: remove !SANDBOX dependency for target tools Ahmad Fatoum
  2021-09-14 13:20 ` [PATCH 2/4] common: add new menu " Ahmad Fatoum
  2021-09-14 13:20 ` [PATCH 3/4] scripts: unify libusb.h inclusion Ahmad Fatoum
@ 2021-09-14 13:20 ` Ahmad Fatoum
  2021-09-14 19:11   ` Trent Piepho
  2021-09-15  8:44   ` Enrico Jörns
  2 siblings, 2 replies; 12+ messages in thread
From: Ahmad Fatoum @ 2021-09-14 13:20 UTC (permalink / raw)
  To: barebox; +Cc: ejo, rhi, uol, renaud.barbier

We currently build the USB loader tools only for the host (build) system,
but it can be useful to cross compile them as well for the target.

We already have some target tools, but support for those is easier,
because they don't link against libraries. We use pkg-config to get
cc and ld flags, but we always assume that pkg-config is for the host
system and there is no well-defined way to request pkg-config for the
target system.

Support this by introducing a new CROSS_PKG_CONFIG. This will be
consulted only for target tools and default to
$(CROSS_COMPILE)pkgconfig.

Users can override it as necessary, for example, with Yocto, pkg-config
will be for the cross environment, so target tools can now be built
with:

  export ARCH=sandbox CROSS_COMPILE=aarch64-linux-gnu-
  export CROSS_PKG_CONFIG=pkg-config scripts
  make targettools_defconfig
  make scripts

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronxi.de>
---
 Documentation/user/barebox.rst             | 45 ++++++++++++++++++++++
 Makefile                                   | 10 ++++-
 arch/sandbox/configs/targettools_defconfig |  9 +++++
 scripts/.gitignore                         |  2 +
 scripts/Kconfig                            | 29 ++++++++++++++
 scripts/Makefile                           |  9 ++++-
 scripts/imx/.gitignore                     |  1 +
 scripts/imx/Makefile                       | 10 +++++
 scripts/imx/imx-target.c                   |  1 +
 scripts/imx/imx-usb-loader-target.c        |  1 +
 scripts/omap3-usb-loader-target.c          |  1 +
 scripts/omap4_usbboot-target.c             |  1 +
 12 files changed, 117 insertions(+), 2 deletions(-)
 create mode 100644 arch/sandbox/configs/targettools_defconfig
 create mode 100644 scripts/imx/imx-target.c
 create mode 100644 scripts/imx/imx-usb-loader-target.c
 create mode 100644 scripts/omap3-usb-loader-target.c
 create mode 100644 scripts/omap4_usbboot-target.c

diff --git a/Documentation/user/barebox.rst b/Documentation/user/barebox.rst
index 503f0b979777..b6b7a57af300 100644
--- a/Documentation/user/barebox.rst
+++ b/Documentation/user/barebox.rst
@@ -262,3 +262,48 @@ the usage for a particular command. barebox has tab completion which will comple
 your command. Arguments to commands are also completed depending on the command. If
 a command expects a file argument only files will be offered as completion. Other
 commands will only complete devices or devicetree nodes.
+
+Building barebox tools
+----------------------
+
+The normal barebox build results in one or more barebox images (cf. :ref:`multi_image`)
+and a number of tools built from its ``scripts/`` directory.
+
+Most tools are used for the barebox build itself: e.g. the device tree compiler,
+the Kconfig machinery and the different image formatting tools that wrap barebox,
+so it may be loaded by the boot ROM of the relevant SoCs.
+
+In addition to these barebox also builds host and target tools that are useful
+outside of barebox build: e.g. to manipulate the environment or to load an
+image over a boot ROM's USB recovery protocol.
+
+There are two ``ARCH=sandbox`` to make this more straight forward:
+
+Host Tools
+^^^^^^^^^^
+
+The ``hosttools_defconfig`` will compile standalone host tools for the
+host (build) system. To build the USB loaders, ``pkg-config`` needs to know
+about ``libusb-1.0``.
+
+.. code-block:: console
+
+  export ARCH=sandbox
+  make hosttools_defconfig
+  make scripts
+
+Target Tools
+^^^^^^^^^^^^
+
+The ``targettools_defconfig`` will cross-compile standalone target tools for the
+target system.  To build the USB loaders, ``CROSS_PKG_CONFIG`` needs to know
+about ``libusb-1.0``. This config won't built any host tools, so it's ok to
+set ``CROSS_PKG_CONFIG=pkg-config`` if ``pkg-config`` is primed for target
+use. The default is ``CROSS_PKG_CONFIG=$(CROSS_COMPILE)pkg-config``. Example:
+
+.. code-block:: console
+
+  export ARCH=sandbox CROSS_COMPILE=aarch64-linux-gnu-
+  export CROSS_PKG_CONFIG=pkg-config
+  make targettools_defconfig
+  make scripts
diff --git a/Makefile b/Makefile
index 74e4893f02c8..77ece120f0c7 100644
--- a/Makefile
+++ b/Makefile
@@ -362,6 +362,8 @@ endif
 
 KCONFIG_CONFIG	?= .config
 
+CROSS_PKG_CONFIG ?= $(CROSS_COMPILE)pkg-config
+
 export KCONFIG_CONFIG
 
 # SHELL used by kbuild
@@ -1126,11 +1128,17 @@ CLEAN_DIRS  += $(MODVERDIR)
 CLEAN_FILES +=	barebox System.map stickypage.bin include/generated/barebox_default_env.h \
                 .tmp_version .tmp_barebox* barebox.bin barebox.map \
 		.tmp_kallsyms* barebox.ldr compile_commands.json \
-		scripts/bareboxenv-target barebox-flash-image \
+		barebox-flash-image \
 		barebox.srec barebox.s5p barebox.ubl barebox.zynq \
 		barebox.uimage barebox.spi barebox.kwb barebox.kwbuart \
 		barebox.efi barebox.canon-a1100.bin
 
+CLEAN_FILES +=	\
+		scripts/bareboxenv-target scripts/kernel-install-target \
+		scripts/bareboxcrc32-target scripts/bareboximd-target \
+		scripts/omap3-usb-loader-target scripts/omap4_usbboot-target \
+		scripts/imx-usb-loader-target
+
 # Directories & files removed with 'make mrproper'
 MRPROPER_DIRS  += include/config usr/include include/generated Documentation/commands
 MRPROPER_FILES += .config .config.old .version .old_version \
diff --git a/arch/sandbox/configs/targettools_defconfig b/arch/sandbox/configs/targettools_defconfig
new file mode 100644
index 000000000000..cba0de4a2aaa
--- /dev/null
+++ b/arch/sandbox/configs/targettools_defconfig
@@ -0,0 +1,9 @@
+CONFIG_IMD=y
+CONFIG_IMD_TARGET=y
+CONFIG_KERNEL_INSTALL_TARGET=y
+CONFIG_BAREBOXENV_TARGET=y
+CONFIG_BAREBOXCRC32_TARGET=y
+CONFIG_COMPILE_HOST_TOOLS=y
+CONFIG_ARCH_IMX_USBLOADER_TARGET=y
+CONFIG_CONFIG_OMAP3_USB_LOADER_TARGET=y
+CONFIG_OMAP4_HOSTTOOL_USBBOOT_TARGET=y
diff --git a/scripts/.gitignore b/scripts/.gitignore
index 9577d568edd0..0b461ea7ff8a 100644
--- a/scripts/.gitignore
+++ b/scripts/.gitignore
@@ -27,7 +27,9 @@ mxsboot
 mxs-usb-loader
 /omap3-usb-loader
 omap4_usbboot
+omap4_usbboot-target
 omap3-usb-loader
+omap3-usb-loader-target
 mips-relocs
 rsatoc
 stm32image
diff --git a/scripts/Kconfig b/scripts/Kconfig
index f7ed775fbc66..e0adb5c4a311 100644
--- a/scripts/Kconfig
+++ b/scripts/Kconfig
@@ -69,4 +69,33 @@ config BAREBOXCRC32_TARGET
 	  'bareboxcrc32' is a userspacetool to generate the crc32 checksums the same way
 	  barebox does. Say yes here to build it for the target.
 
+config HAS_TARGET_LIBUSB_1_0
+	def_bool $(success,$(CROSS_PKG_CONFIG) --exists libusb-1.0)
+	help
+	  Ensure $(CROSS_PKG_CONFIG) is set to a valid pkg-config
+	  binary that knows about libusb-1.0 compiled for the
+	  target architecture.
+
+config ARCH_IMX_USBLOADER_TARGET
+	depends on HAS_TARGET_LIBUSB_1_0
+	bool "imx-usb-loader for target"
+	help
+	  Say Y here to build the imx-usb-loader tool for the target.
+	  The cross toolchain needs libusb-1.0 to compile this tool.
+
+config CONFIG_OMAP3_USB_LOADER_TARGET
+	bool "omap3 USB loader for target"
+	depends on HAS_TARGET_LIBUSB_1_0
+	help
+	  Say Y here to build the omap3 usb loader tool for the target.
+	  The cross toolchain needs libusb-1.0 to compile this tool.
+
+
+config OMAP4_HOSTTOOL_USBBOOT_TARGET
+	bool "omap4 usbboot for target"
+	depends on HAS_TARGET_LIBUSB_1_0
+	help
+	  Say Y here to build the omap4 usb loader tool for the target.
+	  The cross toolchain needs libusb-1.0 to compile this tool.
+
 endmenu
diff --git a/scripts/Makefile b/scripts/Makefile
index eb0f5c5805bb..d97f00d9a792 100644
--- a/scripts/Makefile
+++ b/scripts/Makefile
@@ -41,8 +41,15 @@ userprogs-always-$(CONFIG_BAREBOXENV_TARGET)		+= bareboxenv-target
 userprogs-always-$(CONFIG_KERNEL_INSTALL_TARGET)	+= kernel-install-target
 userprogs-always-$(CONFIG_BAREBOXCRC32_TARGET)		+= bareboxcrc32-target
 userprogs-always-$(CONFIG_IMD_TARGET)			+= bareboximd-target
+userprogs-always-$(CONFIG_OMAP3_USB_LOADER_TARGET)	+= omap3-usb-loader-target
+userprogs-always-$(CONFIG_OMAP4_HOSTTOOL_USBBOOT)	+= omap4_usbboot-target
 
-userccflags += -I $(srctree)/$(src)/include
+omap3-usb-loader-target-userccflags += `$(CROSS_PKG_CONFIG) --cflags libusb-1.0`
+omap3-usb-loader-target-userldlibs += `$(CROSS_PKG_CONFIG) --libs libusb-1.0`
+omap4_usbboot-target-userccflags += `$(CROSS_PKG_CONFIG) --cflags libusb-1.0`
+omap4_usbboot-target-userldlibs += -lpthread `$(CROSS_PKG_CONFIG) --libs libusb-1.0`
+
+userccflags += -I $(srctree)/$(src)/include -isystem $(srctree)/scripts/include
 
 subdir-y			+= mod
 subdir-y			+= imx
diff --git a/scripts/imx/.gitignore b/scripts/imx/.gitignore
index 84e6f2b40617..341aec9ee435 100644
--- a/scripts/imx/.gitignore
+++ b/scripts/imx/.gitignore
@@ -1,2 +1,3 @@
 imx-usb-loader
+imx-usb-loader-target
 imx-image
diff --git a/scripts/imx/Makefile b/scripts/imx/Makefile
index 029f9ca9f8af..a7f487a0452c 100644
--- a/scripts/imx/Makefile
+++ b/scripts/imx/Makefile
@@ -4,13 +4,23 @@ hostprogs-always-$(CONFIG_ARCH_IMX_USBLOADER)	+= imx-usb-loader
 HOSTCFLAGS_imx-usb-loader.o = `pkg-config --cflags libusb-1.0`
 HOSTLDLIBS_imx-usb-loader  = `pkg-config --libs libusb-1.0`
 
+imx-usb-loader-target-userccflags += `$(CROSS_PKG_CONFIG) --cflags libusb-1.0`
+imx-usb-loader-target-userldlibs += `$(CROSS_PKG_CONFIG) --libs libusb-1.0`
+
 HOSTCFLAGS_imx.o = -I$(srctree)/arch/arm/mach-imx/include
+imx-target-userccflags += -I$(srctree)/arch/arm/mach-imx/include
 HOSTCFLAGS_imx-image.o = -I$(srctree) -I$(srctree)/arch/arm/mach-imx/include
 HOSTCFLAGS_imx-usb-loader.o += -I$(srctree) -I$(srctree)/arch/arm/mach-imx/include
+imx-usb-loader-target-userccflags += -I$(srctree) -I$(srctree)/arch/arm/mach-imx/include
 ifdef CONFIG_ARCH_IMX_IMXIMAGE_SSL_SUPPORT
 HOSTCFLAGS_imx-image.o += -DIMXIMAGE_SSL_SUPPORT
 HOSTLDLIBS_imx-image  = `pkg-config --libs openssl`
 endif
 
 imx-usb-loader-objs := imx-usb-loader.o imx.o
+imx-usb-loader-target-objs := imx-usb-loader-target.o imx-target.o
 imx-image-objs := imx-image.o imx.o
+
+userprogs-always-$(CONFIG_ARCH_IMX_USBLOADER_TARGET)	+= imx-usb-loader-target
+
+userccflags += -I $(srctree)/$(src)/include -isystem $(srctree)/scripts/include
diff --git a/scripts/imx/imx-target.c b/scripts/imx/imx-target.c
new file mode 100644
index 000000000000..4062eed6f60f
--- /dev/null
+++ b/scripts/imx/imx-target.c
@@ -0,0 +1 @@
+#include "imx.c"
diff --git a/scripts/imx/imx-usb-loader-target.c b/scripts/imx/imx-usb-loader-target.c
new file mode 100644
index 000000000000..f2050aec1791
--- /dev/null
+++ b/scripts/imx/imx-usb-loader-target.c
@@ -0,0 +1 @@
+#include "imx-usb-loader.c"
diff --git a/scripts/omap3-usb-loader-target.c b/scripts/omap3-usb-loader-target.c
new file mode 100644
index 000000000000..c99c261a0253
--- /dev/null
+++ b/scripts/omap3-usb-loader-target.c
@@ -0,0 +1 @@
+#include "omap3-usb-loader.c"
diff --git a/scripts/omap4_usbboot-target.c b/scripts/omap4_usbboot-target.c
new file mode 100644
index 000000000000..3dd606533749
--- /dev/null
+++ b/scripts/omap4_usbboot-target.c
@@ -0,0 +1 @@
+#include "omap4_usbboot.c"
-- 
2.30.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/4] common: add new menu for target tools
  2021-09-14 13:20 ` [PATCH 2/4] common: add new menu " Ahmad Fatoum
@ 2021-09-14 16:00   ` Roland Hieber
  0 siblings, 0 replies; 12+ messages in thread
From: Roland Hieber @ 2021-09-14 16:00 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox, ejo, uol, renaud.barbier

On Tue, Sep 14, 2021 at 03:20:14PM +0200, Ahmad Fatoum wrote:
> We have four target tools and will add 3 more in a follow up commit.
> Add a new menu to collect them.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

Looks very useful to me.

Acked-by: Roland Hieber <rhi@pengutronix.de>

> ---
>  common/Kconfig  | 26 --------------------------
>  scripts/Kconfig | 30 ++++++++++++++++++++++++++++++
>  2 files changed, 30 insertions(+), 26 deletions(-)
> 
> diff --git a/common/Kconfig b/common/Kconfig
> index 222a438ee545..9dab7532e15e 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -734,18 +734,6 @@ config IMD
>  	select CRC32
>  	bool "barebox metadata support"
>  
> -config IMD_TARGET
> -	bool "build bareboximd target tool"
> -	depends on IMD
> -
> -config KERNEL_INSTALL_TARGET
> -	bool
> -	prompt "Build kernel-install utility for the target"
> -	help
> -	  Enable this to compile the kernel-install script using the cross
> -	  compiler. The utility for the target will be under
> -	  scripts/kernel-install-target
> -
>  choice
>  	prompt "console support"
>  	default CONSOLE_FULL
> @@ -980,20 +968,6 @@ config DEFAULT_ENVIRONMENT_PATH
>  	  be taken. Relative paths will be relative to the barebox top-level
>  	  directory, but absolute paths are fine as well.
>  
> -config BAREBOXENV_TARGET
> -	bool
> -	prompt "build bareboxenv tool for target"
> -	help
> -	  'bareboxenv' is a tool to access the barebox environment from a running Linux
> -	  system. Say yes here to build it for the target.
> -
> -config BAREBOXCRC32_TARGET
> -	bool
> -	prompt "build bareboxcrc32 tool for target"
> -	help
> -	  'bareboxcrc32' is a userspacetool to generate the crc32 checksums the same way
> -	  barebox does. Say yes here to build it for the target.
> -
>  config HAS_SCHED
>  	bool
>  
> diff --git a/scripts/Kconfig b/scripts/Kconfig
> index b903486ecd8f..f7ed775fbc66 100644
> --- a/scripts/Kconfig
> +++ b/scripts/Kconfig
> @@ -40,3 +40,33 @@ config OMAP4_HOSTTOOL_USBBOOT
>  	  You need libusb-1.0 to compile this tool.
>  
>  endmenu
> +
> +menu "Target Tools"
> +
> +config IMD_TARGET
> +	bool "build bareboximd target tool"
> +	depends on IMD
> +
> +config KERNEL_INSTALL_TARGET
> +	bool
> +	prompt "Build kernel-install utility for the target"
> +	help
> +	  Enable this to compile the kernel-install script using the cross
> +	  compiler. The utility for the target will be under
> +	  scripts/kernel-install-target
> +
> +config BAREBOXENV_TARGET
> +	bool
> +	prompt "build bareboxenv tool for target"
> +	help
> +	  'bareboxenv' is a tool to access the barebox environment from a running Linux
> +	  system. Say yes here to build it for the target.
> +
> +config BAREBOXCRC32_TARGET
> +	bool
> +	prompt "build bareboxcrc32 tool for target"
> +	help
> +	  'bareboxcrc32' is a userspacetool to generate the crc32 checksums the same way
> +	  barebox does. Say yes here to build it for the target.
> +
> +endmenu
> -- 
> 2.30.2
> 
> 

-- 
Roland Hieber, Pengutronix e.K.          | r.hieber@pengutronix.de     |
Steuerwalder Str. 21                     | https://www.pengutronix.de/ |
31137 Hildesheim, Germany                | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686         | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/4] scripts: allow building USB loader tools for target as well
  2021-09-14 13:20 ` [PATCH 4/4] scripts: allow building USB loader tools for target as well Ahmad Fatoum
@ 2021-09-14 19:11   ` Trent Piepho
  2021-09-15  8:50     ` Ahmad Fatoum
  2021-09-15  8:44   ` Enrico Jörns
  1 sibling, 1 reply; 12+ messages in thread
From: Trent Piepho @ 2021-09-14 19:11 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: Barebox List, ejo, rhi, uol, renaud.barbier

On Tue, Sep 14, 2021 at 6:21 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>
> We currently build the USB loader tools only for the host (build) system,
> but it can be useful to cross compile them as well for the target.
>
> We already have some target tools, but support for those is easier,
> because they don't link against libraries. We use pkg-config to get
> cc and ld flags, but we always assume that pkg-config is for the host
> system and there is no well-defined way to request pkg-config for the
> target system.
>
> Support this by introducing a new CROSS_PKG_CONFIG. This will be
> consulted only for target tools and default to
> $(CROSS_COMPILE)pkgconfig.
>
> Users can override it as necessary, for example, with Yocto, pkg-config
> will be for the cross environment, so target tools can now be built
> with:

I just added support to Buildroot for building imx-usb-loader from
Barebox, since it's nicer than the standalone version of the program.

Since pkgconfig was only used for host tools, I didn't need to make
both host and target pkgconfig work.  But of course that will no
longer be true after this patch.

There is a problem with only supplying CROSS_PKG_CONFIG.  To get both
host and target pkgconfig to work, I also need to supply the env
variables used by pkgconfig, PKG_CONFIG_SYSROOT and PKG_CONFIG_LIBDIR.

The former makes the paths returned by pkgconfig correct and the
latter controls which set, target or host, of .pc files will be used.

Maybe something like this in the Makefile:

CROSS_PKG_CONFIG ?= $(CROSS_COMPILE)pkg-config
CROSS_PKG_CONFIG_SYSROOT ?= $(PKG_CONFIG_SYSROOT)
CROSS_PKG_CONFIG_LIBDIR ?= $(PKG_CONFIG_LIBDIR)
CROSS_PKG_CONFIG_ENV := \
       PKG_CONFIG_LIBDIR=$(CROSS_PKG_CONFIG_LIBDIR) \
       PKG_CONFIG_SYSROOT=$(CROSS_PKG_CONFIG_SYSROOT)

HOST_LIBUSB_CFLAGS := $(shell $(PKG_CONFIG) --cflags)
CROSS_LIBUSB_CFLAGS := $(shell $(CROSS_PKG_CONFIG_ENV)
$(CROSS_PKG_CONFIG) --cflags)

Then use those everywhere someone wants the libusb cflags.  Repeat for LDFLAGS.

You'll get fewer repeated invocations of pkg-config this way too.

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/4] scripts: unify libusb.h inclusion
  2021-09-14 13:20 ` [PATCH 3/4] scripts: unify libusb.h inclusion Ahmad Fatoum
@ 2021-09-14 19:22   ` Trent Piepho
  2021-09-15  8:55     ` Ahmad Fatoum
  0 siblings, 1 reply; 12+ messages in thread
From: Trent Piepho @ 2021-09-14 19:22 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: Barebox List, ejo, rhi, uol, renaud.barbier

On Tue, Sep 14, 2021 at 6:21 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>
> I've also run into downstream patches around broken toolchains that
> change <libusb.h> to <libusb-1.0/libusb.h>, so the header is found.

How does this become a problem exactly?  If the toolchain has the file
in the location <includedir>/ibusb-1.0/libusb.h, then shouldn't
pkg-config --cflags have returned -I<includedir>/libusb-1.0 and it
would then find the file just fine?

Or is the problem really that the cross building setup does not have
correct .pc files?

If it's the latter, then I'll point out that my previous email would
let someone put CROSS_LIBUSB_CFLAGS=-I<cross-include-dir>/libusb-1.0
on the make command line and get a build without having .pc files.
This is pretty much what you get with autoconf configure scripts when
you put headers in a non-standard place and don't have .pc files setup
to find them.  You need to supply CFLAGS, LDFLAGS, etc. variables with
the proper -L and -I options to find them.


> +++ b/scripts/libusb.h

By naming this libusb.h, it requires that "libusb.h" find this file
and <libusb.h> find a different header of the same name.  That's
certainly possible by being careful with the "" and <> search paths.
But it seems unnecessarily fragile.  I suggest using libusb-wrapper.h
for this file so you avoid name collisions with the real header file.

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/4] scripts: allow building USB loader tools for target as well
  2021-09-14 13:20 ` [PATCH 4/4] scripts: allow building USB loader tools for target as well Ahmad Fatoum
  2021-09-14 19:11   ` Trent Piepho
@ 2021-09-15  8:44   ` Enrico Jörns
  1 sibling, 0 replies; 12+ messages in thread
From: Enrico Jörns @ 2021-09-15  8:44 UTC (permalink / raw)
  To: Ahmad Fatoum, barebox; +Cc: rhi, uol, renaud.barbier, ejo

Hi Ahmad,

Am Dienstag, dem 14.09.2021 um 15:20 +0200 schrieb Ahmad Fatoum:
> We currently build the USB loader tools only for the host (build) system,
> but it can be useful to cross compile them as well for the target.

many thanks for this great enhancement first of all!

> +CLEAN_FILES +=	\
> +		scripts/bareboxenv-target scripts/kernel-install-target \
> +		scripts/bareboxcrc32-target scripts/bareboximd-target \
> +		scripts/omap3-usb-loader-target scripts/omap4_usbboot-target \
> +		scripts/imx-usb-loader-target
> +
>  # Directories & files removed with 'make mrproper'
>  MRPROPER_DIRS  += include/config usr/include include/generated Documentation/commands
>  MRPROPER_FILES += .config .config.old .version .old_version \
> diff --git a/arch/sandbox/configs/targettools_defconfig b/arch/sandbox/configs/targettools_defconfig
> new file mode 100644
> index 000000000000..cba0de4a2aaa
> --- /dev/null
> +++ b/arch/sandbox/configs/targettools_defconfig
> @@ -0,0 +1,9 @@
> +CONFIG_IMD=y
> +CONFIG_IMD_TARGET=y
> +CONFIG_KERNEL_INSTALL_TARGET=y
> +CONFIG_BAREBOXENV_TARGET=y
> +CONFIG_BAREBOXCRC32_TARGET=y
> +CONFIG_COMPILE_HOST_TOOLS=y
> +CONFIG_ARCH_IMX_USBLOADER_TARGET=y
> +CONFIG_CONFIG_OMAP3_USB_LOADER_TARGET=y

A single CONFIG_ should be sufficient here when renaming the kconfig symbol ;)

(see below)

> +CONFIG_OMAP4_HOSTTOOL_USBBOOT_TARGET=y
> diff --git a/scripts/.gitignore b/scripts/.gitignore
> index 9577d568edd0..0b461ea7ff8a 100644
> --- a/scripts/.gitignore
> +++ b/scripts/.gitignore
> @@ -27,7 +27,9 @@ mxsboot
>  mxs-usb-loader
>  /omap3-usb-loader
>  omap4_usbboot
> +omap4_usbboot-target
>  omap3-usb-loader
> +omap3-usb-loader-target
>  mips-relocs
>  rsatoc
>  stm32image
> diff --git a/scripts/Kconfig b/scripts/Kconfig
> index f7ed775fbc66..e0adb5c4a311 100644
> --- a/scripts/Kconfig
> +++ b/scripts/Kconfig
> @@ -69,4 +69,33 @@ config BAREBOXCRC32_TARGET
>  	  'bareboxcrc32' is a userspacetool to generate the crc32 checksums the same way
>  	  barebox does. Say yes here to build it for the target.
>  
> +config HAS_TARGET_LIBUSB_1_0
> +	def_bool $(success,$(CROSS_PKG_CONFIG) --exists libusb-1.0)
> +	help
> +	  Ensure $(CROSS_PKG_CONFIG) is set to a valid pkg-config
> +	  binary that knows about libusb-1.0 compiled for the
> +	  target architecture.
> +
> +config ARCH_IMX_USBLOADER_TARGET
> +	depends on HAS_TARGET_LIBUSB_1_0
> +	bool "imx-usb-loader for target"
> +	help
> +	  Say Y here to build the imx-usb-loader tool for the target.
> +	  The cross toolchain needs libusb-1.0 to compile this tool.
> +
> +config CONFIG_OMAP3_USB_LOADER_TARGET
> +	bool "omap3 USB loader for target"
> +	depends on HAS_TARGET_LIBUSB_1_0
> +	help
> +	  Say Y here to build the omap3 usb loader tool for the target.
> +	  The cross toolchain needs libusb-1.0 to compile this tool.
> 

CONFIG_ should be omitted here.


About 'kwboot' I was not sure if that should be available as target tool, too.
What do you think? I have never used it but it appears to me that it is useful
for booting mvebu devices and thus might be used on TACs, too.


Regards

Enrico


> +
> +config OMAP4_HOSTTOOL_USBBOOT_TARGET
> +	bool "omap4 usbboot for target"
> +	depends on HAS_TARGET_LIBUSB_1_0
> +	help
> +	  Say Y here to build the omap4 usb loader tool for the target.
> +	  The cross toolchain needs libusb-1.0 to compile this tool.
> +
>  endmenu
> diff --git a/scripts/Makefile b/scripts/Makefile
> index eb0f5c5805bb..d97f00d9a792 100644
> --- a/scripts/Makefile
> +++ b/scripts/Makefile
> @@ -41,8 +41,15 @@ userprogs-always-$(CONFIG_BAREBOXENV_TARGET)		+= bareboxenv-target
>  userprogs-always-$(CONFIG_KERNEL_INSTALL_TARGET)	+= kernel-install-target
>  userprogs-always-$(CONFIG_BAREBOXCRC32_TARGET)		+= bareboxcrc32-target
>  userprogs-always-$(CONFIG_IMD_TARGET)			+= bareboximd-target
> +userprogs-always-$(CONFIG_OMAP3_USB_LOADER_TARGET)	+= omap3-usb-loader-target
> +userprogs-always-$(CONFIG_OMAP4_HOSTTOOL_USBBOOT)	+= omap4_usbboot-target
>  
> 
> 
> 
> 
> 
> 
> 
> -userccflags += -I $(srctree)/$(src)/include
> +omap3-usb-loader-target-userccflags += `$(CROSS_PKG_CONFIG) --cflags libusb-1.0`
> +omap3-usb-loader-target-userldlibs += `$(CROSS_PKG_CONFIG) --libs libusb-1.0`
> +omap4_usbboot-target-userccflags += `$(CROSS_PKG_CONFIG) --cflags libusb-1.0`
> +omap4_usbboot-target-userldlibs += -lpthread `$(CROSS_PKG_CONFIG) --libs libusb-1.0`
> +
> +userccflags += -I $(srctree)/$(src)/include -isystem $(srctree)/scripts/include
>  
> 
> 
> 
> 
> 
> 
> 
>  subdir-y			+= mod
>  subdir-y			+= imx
> diff --git a/scripts/imx/.gitignore b/scripts/imx/.gitignore
> index 84e6f2b40617..341aec9ee435 100644
> --- a/scripts/imx/.gitignore
> +++ b/scripts/imx/.gitignore
> @@ -1,2 +1,3 @@
>  imx-usb-loader
> +imx-usb-loader-target
>  imx-image
> diff --git a/scripts/imx/Makefile b/scripts/imx/Makefile
> index 029f9ca9f8af..a7f487a0452c 100644
> --- a/scripts/imx/Makefile
> +++ b/scripts/imx/Makefile
> @@ -4,13 +4,23 @@ hostprogs-always-$(CONFIG_ARCH_IMX_USBLOADER)	+= imx-usb-loader
>  HOSTCFLAGS_imx-usb-loader.o = `pkg-config --cflags libusb-1.0`
>  HOSTLDLIBS_imx-usb-loader  = `pkg-config --libs libusb-1.0`
>  
> 
> 
> 
> 
> 
> 
> 
> +imx-usb-loader-target-userccflags += `$(CROSS_PKG_CONFIG) --cflags libusb-1.0`
> +imx-usb-loader-target-userldlibs += `$(CROSS_PKG_CONFIG) --libs libusb-1.0`
> +
>  HOSTCFLAGS_imx.o = -I$(srctree)/arch/arm/mach-imx/include
> +imx-target-userccflags += -I$(srctree)/arch/arm/mach-imx/include
>  HOSTCFLAGS_imx-image.o = -I$(srctree) -I$(srctree)/arch/arm/mach-imx/include
>  HOSTCFLAGS_imx-usb-loader.o += -I$(srctree) -I$(srctree)/arch/arm/mach-imx/include
> +imx-usb-loader-target-userccflags += -I$(srctree) -I$(srctree)/arch/arm/mach-imx/include
>  ifdef CONFIG_ARCH_IMX_IMXIMAGE_SSL_SUPPORT
>  HOSTCFLAGS_imx-image.o += -DIMXIMAGE_SSL_SUPPORT
>  HOSTLDLIBS_imx-image  = `pkg-config --libs openssl`
>  endif
>  
> 
> 
> 
> 
> 
> 
> 
>  imx-usb-loader-objs := imx-usb-loader.o imx.o
> +imx-usb-loader-target-objs := imx-usb-loader-target.o imx-target.o
>  imx-image-objs := imx-image.o imx.o
> +
> +userprogs-always-$(CONFIG_ARCH_IMX_USBLOADER_TARGET)	+= imx-usb-loader-target
> +
> +userccflags += -I $(srctree)/$(src)/include -isystem $(srctree)/scripts/include
> diff --git a/scripts/imx/imx-target.c b/scripts/imx/imx-target.c
> new file mode 100644
> index 000000000000..4062eed6f60f
> --- /dev/null
> +++ b/scripts/imx/imx-target.c
> @@ -0,0 +1 @@
> +#include "imx.c"
> diff --git a/scripts/imx/imx-usb-loader-target.c b/scripts/imx/imx-usb-loader-target.c
> new file mode 100644
> index 000000000000..f2050aec1791
> --- /dev/null
> +++ b/scripts/imx/imx-usb-loader-target.c
> @@ -0,0 +1 @@
> +#include "imx-usb-loader.c"
> diff --git a/scripts/omap3-usb-loader-target.c b/scripts/omap3-usb-loader-target.c
> new file mode 100644
> index 000000000000..c99c261a0253
> --- /dev/null
> +++ b/scripts/omap3-usb-loader-target.c
> @@ -0,0 +1 @@
> +#include "omap3-usb-loader.c"
> diff --git a/scripts/omap4_usbboot-target.c b/scripts/omap4_usbboot-target.c
> new file mode 100644
> index 000000000000..3dd606533749
> --- /dev/null
> +++ b/scripts/omap4_usbboot-target.c
> @@ -0,0 +1 @@
> +#include "omap4_usbboot.c"

-- 
Pengutronix e.K.                           | Enrico Jörns                |
Embedded Linux Consulting & Support        | https://www.pengutronix.de/ |
Steuerwalder Str. 21                       | Phone: +49-5121-206917-180  |
31137 Hildesheim, Germany                  | Fax:   +49-5121-206917-9    |


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/4] scripts: allow building USB loader tools for target as well
  2021-09-14 19:11   ` Trent Piepho
@ 2021-09-15  8:50     ` Ahmad Fatoum
  2021-09-15  9:38       ` Trent Piepho
  0 siblings, 1 reply; 12+ messages in thread
From: Ahmad Fatoum @ 2021-09-15  8:50 UTC (permalink / raw)
  To: Trent Piepho; +Cc: Barebox List, ejo, rhi, uol, renaud.barbier

Hello Trent,

On 14.09.21 21:11, Trent Piepho wrote:
> On Tue, Sep 14, 2021 at 6:21 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>
>> We currently build the USB loader tools only for the host (build) system,
>> but it can be useful to cross compile them as well for the target.
>>
>> We already have some target tools, but support for those is easier,
>> because they don't link against libraries. We use pkg-config to get
>> cc and ld flags, but we always assume that pkg-config is for the host
>> system and there is no well-defined way to request pkg-config for the
>> target system.
>>
>> Support this by introducing a new CROSS_PKG_CONFIG. This will be
>> consulted only for target tools and default to
>> $(CROSS_COMPILE)pkgconfig.
>>
>> Users can override it as necessary, for example, with Yocto, pkg-config
>> will be for the cross environment, so target tools can now be built
>> with:
> 
> I just added support to Buildroot for building imx-usb-loader from
> Barebox, since it's nicer than the standalone version of the program.
> 
> Since pkgconfig was only used for host tools, I didn't need to make
> both host and target pkgconfig work.  But of course that will no
> longer be true after this patch.

It still wouldn't break your workflow, imx-usb-loader wasn't built
for target so far.

> There is a problem with only supplying CROSS_PKG_CONFIG.  To get both
> host and target pkgconfig to work, I also need to supply the env
> variables used by pkgconfig, PKG_CONFIG_SYSROOT and PKG_CONFIG_LIBDIR.

You can set these on the environment before starting barebox build.
If you have a $(CROSS_COMPILE)pkg-config that doesn't need any further
configuration, you can use that.

> The former makes the paths returned by pkgconfig correct and the
> latter controls which set, target or host, of .pc files will be used.
> 
> Maybe something like this in the Makefile:
> 
> CROSS_PKG_CONFIG ?= $(CROSS_COMPILE)pkg-config
> CROSS_PKG_CONFIG_SYSROOT ?= $(PKG_CONFIG_SYSROOT)
> CROSS_PKG_CONFIG_LIBDIR ?= $(PKG_CONFIG_LIBDIR)
> CROSS_PKG_CONFIG_ENV := \
>        PKG_CONFIG_LIBDIR=$(CROSS_PKG_CONFIG_LIBDIR) \
>        PKG_CONFIG_SYSROOT=$(CROSS_PKG_CONFIG_SYSROOT)
> 
> HOST_LIBUSB_CFLAGS := $(shell $(PKG_CONFIG) --cflags)
> CROSS_LIBUSB_CFLAGS := $(shell $(CROSS_PKG_CONFIG_ENV)
> $(CROSS_PKG_CONFIG) --cflags)
> 
> Then use those everywhere someone wants the libusb cflags.  Repeat for LDFLAGS.

I am not really sold on this. Linux doesn't mess with PKG_CONFIG_ variables
either. For perf the assume $(CROSS_COMPILE)pkg-config to be available.
I think it's a suitbale default for us too. The lines above can go into
a shell script wrapper.

> You'll get fewer repeated invocations of pkg-config this way too.

Ye, I thought about that as well, but we do it like this for the host tools,
so I left it for now. It's not much overhead and it makes it a bit easier
to follow what is used.

Cheers,
Ahmad
 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/4] scripts: unify libusb.h inclusion
  2021-09-14 19:22   ` Trent Piepho
@ 2021-09-15  8:55     ` Ahmad Fatoum
  0 siblings, 0 replies; 12+ messages in thread
From: Ahmad Fatoum @ 2021-09-15  8:55 UTC (permalink / raw)
  To: Trent Piepho; +Cc: Barebox List, ejo, rhi, uol, renaud.barbier

On 14.09.21 21:22, Trent Piepho wrote:
> On Tue, Sep 14, 2021 at 6:21 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>
>> I've also run into downstream patches around broken toolchains that
>> change <libusb.h> to <libusb-1.0/libusb.h>, so the header is found.
> 
> How does this become a problem exactly?  If the toolchain has the file
> in the location <includedir>/ibusb-1.0/libusb.h, then shouldn't
> pkg-config --cflags have returned -I<includedir>/libusb-1.0 and it
> would then find the file just fine?
>
> Or is the problem really that the cross building setup does not have
> correct .pc files?

I ran twice into downstream patches patching imx-usb-loader. So apparently

-lusb1.0 was returned but the CFLAGS were correctl set.

> If it's the latter, then I'll point out that my previous email would
> let someone put CROSS_LIBUSB_CFLAGS=-I<cross-include-dir>/libusb-1.0
> on the make command line and get a build without having .pc files.
> This is pretty much what you get with autoconf configure scripts when
> you put headers in a non-standard place and don't have .pc files setup
> to find them.  You need to supply CFLAGS, LDFLAGS, etc. variables with
> the proper -L and -I options to find them.

I wrote this patch initially to support a pkg-config less build:
If CROSS_PKG_CONFIG wasn't set (no default), we would just use -lusb-1.0
and hope it works. For the header __has_include would try the two paths
that most probably would work.

Now that I dropped that fallback, I should probably drop this patch
as well and let people with broken pkg-config files just fix their
brokenness.

>> +++ b/scripts/libusb.h
> 
> By naming this libusb.h, it requires that "libusb.h" find this file
> and <libusb.h> find a different header of the same name.  That's
> certainly possible by being careful with the "" and <> search paths.
> But it seems unnecessarily fragile.  I suggest using libusb-wrapper.h
> for this file so you avoid name collisions with the real header file.

That's a good point. I'll drop this patch though for v2.

Thanks for your feedback,
Ahmad


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/4] scripts: allow building USB loader tools for target as well
  2021-09-15  8:50     ` Ahmad Fatoum
@ 2021-09-15  9:38       ` Trent Piepho
  2021-09-15 10:23         ` Ahmad Fatoum
  0 siblings, 1 reply; 12+ messages in thread
From: Trent Piepho @ 2021-09-15  9:38 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: Barebox List, ejo, rhi, uol, renaud.barbier

On Wed, Sep 15, 2021 at 1:50 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> On 14.09.21 21:11, Trent Piepho wrote:
> > On Tue, Sep 14, 2021 at 6:21 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> >>
> >> Users can override it as necessary, for example, with Yocto, pkg-config
> >> will be for the cross environment, so target tools can now be built
> >> with:
> >
> > I just added support to Buildroot for building imx-usb-loader from
> > Barebox, since it's nicer than the standalone version of the program.
> >
> > Since pkgconfig was only used for host tools, I didn't need to make
> > both host and target pkgconfig work.  But of course that will no
> > longer be true after this patch.
>
> It still wouldn't break your workflow, imx-usb-loader wasn't built
> for target so far.

But it wouldn't work for anyone who wanted both the host and target versions.

> > There is a problem with only supplying CROSS_PKG_CONFIG.  To get both
> > host and target pkgconfig to work, I also need to supply the env
> > variables used by pkgconfig, PKG_CONFIG_SYSROOT and PKG_CONFIG_LIBDIR.
>
> You can set these on the environment before starting barebox build.
> If you have a $(CROSS_COMPILE)pkg-config that doesn't need any further
> configuration, you can use that.

The problem is they need different values for the target and for the
host version of the package config.  The way you have done this it is
only possible for there to be one value that is used both when the
host version is called and the same values when the cross pkg-config
is called.

> > The former makes the paths returned by pkgconfig correct and the
> > latter controls which set, target or host, of .pc files will be used.
> >
> > Maybe something like this in the Makefile:
> >
> > CROSS_PKG_CONFIG ?= $(CROSS_COMPILE)pkg-config
> > CROSS_PKG_CONFIG_SYSROOT ?= $(PKG_CONFIG_SYSROOT)
> > CROSS_PKG_CONFIG_LIBDIR ?= $(PKG_CONFIG_LIBDIR)
> > CROSS_PKG_CONFIG_ENV := \
> >        PKG_CONFIG_LIBDIR=$(CROSS_PKG_CONFIG_LIBDIR) \
> >        PKG_CONFIG_SYSROOT=$(CROSS_PKG_CONFIG_SYSROOT)
> >
> > HOST_LIBUSB_CFLAGS := $(shell $(PKG_CONFIG) --cflags)
> > CROSS_LIBUSB_CFLAGS := $(shell $(CROSS_PKG_CONFIG_ENV)
> > $(CROSS_PKG_CONFIG) --cflags)
> >
> > Then use those everywhere someone wants the libusb cflags.  Repeat for LDFLAGS.
>
> I am not really sold on this. Linux doesn't mess with PKG_CONFIG_ variables
> either. For perf the assume $(CROSS_COMPILE)pkg-config to be available.
> I think it's a suitbale default for us too. The lines above can go into
> a shell script wrapper.

Linux allows you to build the tools individually,  e.g. for tmon we have this:

make -C $(LINUX_DIR)/tools CC=$(TARGET_CC)
PKG_CONFIG_PATH=$(STAGING_DIR)/usr/lib/pkgconfig tmon

Can Barebox build do this?  This way there are multiple make calls,
with different options, to build a kernel for the target, dtc for the
host, tmon for the target, and so on.

So Linux build may not be an ideal pattern to copy.  It can not build
a tool for both host and target in one build.

> > You'll get fewer repeated invocations of pkg-config this way too.
>
> Ye, I thought about that as well, but we do it like this for the host tools,
> so I left it for now. It's not much overhead and it makes it a bit easier
> to follow what is used.

It looks like Linux build assigns pkg-config output to a variable in
every instance but dtc's build.  I think the existing Barebox
makefiles are just a bit sloppy in how they call pkg-config.

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/4] scripts: allow building USB loader tools for target as well
  2021-09-15  9:38       ` Trent Piepho
@ 2021-09-15 10:23         ` Ahmad Fatoum
  0 siblings, 0 replies; 12+ messages in thread
From: Ahmad Fatoum @ 2021-09-15 10:23 UTC (permalink / raw)
  To: Trent Piepho; +Cc: Barebox List, ejo, rhi, uol, renaud.barbier

On 15.09.21 11:38, Trent Piepho wrote:
> On Wed, Sep 15, 2021 at 1:50 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>> On 14.09.21 21:11, Trent Piepho wrote:
>>> On Tue, Sep 14, 2021 at 6:21 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>>>
>>>> Users can override it as necessary, for example, with Yocto, pkg-config
>>>> will be for the cross environment, so target tools can now be built
>>>> with:
>>>
>>> I just added support to Buildroot for building imx-usb-loader from
>>> Barebox, since it's nicer than the standalone version of the program.
>>>
>>> Since pkgconfig was only used for host tools, I didn't need to make
>>> both host and target pkgconfig work.  But of course that will no
>>> longer be true after this patch.
>>
>> It still wouldn't break your workflow, imx-usb-loader wasn't built
>> for target so far.
> 
> But it wouldn't work for anyone who wanted both the host and target versions.

You have two options:

 - provide ${CROSS_COMPILE}pkg-config that calls your wrapper that has
   the necessary options set

 - export CROSS_PKG_CONFIG="YOUR_ENV_VARS pkg-config"

>>> There is a problem with only supplying CROSS_PKG_CONFIG.  To get both
>>> host and target pkgconfig to work, I also need to supply the env
>>> variables used by pkgconfig, PKG_CONFIG_SYSROOT and PKG_CONFIG_LIBDIR.
>>
>> You can set these on the environment before starting barebox build.
>> If you have a $(CROSS_COMPILE)pkg-config that doesn't need any further
>> configuration, you can use that.
> 
> The problem is they need different values for the target and for the
> host version of the package config.  The way you have done this it is
> only possible for there to be one value that is used both when the
> host version is called and the same values when the cross pkg-config
> is called.

The example in the commit message is one way to do it with Yocto.
You could still build both host and target tools in the same go.

>>> The former makes the paths returned by pkgconfig correct and the
>>> latter controls which set, target or host, of .pc files will be used.
>>>
>>> Maybe something like this in the Makefile:
>>>
>>> CROSS_PKG_CONFIG ?= $(CROSS_COMPILE)pkg-config
>>> CROSS_PKG_CONFIG_SYSROOT ?= $(PKG_CONFIG_SYSROOT)
>>> CROSS_PKG_CONFIG_LIBDIR ?= $(PKG_CONFIG_LIBDIR)
>>> CROSS_PKG_CONFIG_ENV := \
>>>        PKG_CONFIG_LIBDIR=$(CROSS_PKG_CONFIG_LIBDIR) \
>>>        PKG_CONFIG_SYSROOT=$(CROSS_PKG_CONFIG_SYSROOT)
>>>
>>> HOST_LIBUSB_CFLAGS := $(shell $(PKG_CONFIG) --cflags)
>>> CROSS_LIBUSB_CFLAGS := $(shell $(CROSS_PKG_CONFIG_ENV)
>>> $(CROSS_PKG_CONFIG) --cflags)
>>>
>>> Then use those everywhere someone wants the libusb cflags.  Repeat for LDFLAGS.
>>
>> I am not really sold on this. Linux doesn't mess with PKG_CONFIG_ variables
>> either. For perf the assume $(CROSS_COMPILE)pkg-config to be available.
>> I think it's a suitbale default for us too. The lines above can go into
>> a shell script wrapper.
> 
> Linux allows you to build the tools individually,  e.g. for tmon we have this:
> 
> make -C $(LINUX_DIR)/tools CC=$(TARGET_CC)
> PKG_CONFIG_PATH=$(STAGING_DIR)/usr/lib/pkgconfig tmon
> 
> Can Barebox build do this?  This way there are multiple make calls,
> with different options, to build a kernel for the target, dtc for the
> host, tmon for the target, and so on.

I don't need this flexibility. I am fine with changing the config
if only specific applications sohuld be built.
 
> So Linux build may not be an ideal pattern to copy.  It can not build
> a tool for both host and target in one build.

And barebox can do this now. So all good?

>>> You'll get fewer repeated invocations of pkg-config this way too.
>>
>> Ye, I thought about that as well, but we do it like this for the host tools,
>> so I left it for now. It's not much overhead and it makes it a bit easier
>> to follow what is used.
> 
> It looks like Linux build assigns pkg-config output to a variable in
> every instance but dtc's build.  I think the existing Barebox
> makefiles are just a bit sloppy in how they call pkg-config.

Send patches?

Cheers,
Ahmad


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2021-09-15 10:25 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-14 13:20 [PATCH 1/4] common: remove !SANDBOX dependency for target tools Ahmad Fatoum
2021-09-14 13:20 ` [PATCH 2/4] common: add new menu " Ahmad Fatoum
2021-09-14 16:00   ` Roland Hieber
2021-09-14 13:20 ` [PATCH 3/4] scripts: unify libusb.h inclusion Ahmad Fatoum
2021-09-14 19:22   ` Trent Piepho
2021-09-15  8:55     ` Ahmad Fatoum
2021-09-14 13:20 ` [PATCH 4/4] scripts: allow building USB loader tools for target as well Ahmad Fatoum
2021-09-14 19:11   ` Trent Piepho
2021-09-15  8:50     ` Ahmad Fatoum
2021-09-15  9:38       ` Trent Piepho
2021-09-15 10:23         ` Ahmad Fatoum
2021-09-15  8:44   ` Enrico Jörns

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox