mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH v1 0/3] Add displaying a version number in
@ 2023-05-05  7:53 Johannes Zink
  2023-05-05  7:53 ` [PATCH v1 1/3] imx-usb-loader: error with success when displaying help on purpose Johannes Zink
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Johannes Zink @ 2023-05-05  7:53 UTC (permalink / raw)
  To: barebox; +Cc: patchwork-jzi, kernel

This series adds displaying a version number in imx-usb-loader for
easier debugging.

The first two patches add minor cleanup, while the third actually adds
the code for displaying the version number.

The result will look like
$./imx-usb-loader --version
./imx-usb-loader 2023.04.0-00210-g057705fb211a-dirty

Happy hacking
Johannes

Johannes Zink (3):
  imx-usb-loader: error with success when displaying help on purpose
  imx-usb-loader: use proper return code macros
  imx-usb-loader: add commandline option for displaying version number

 scripts/imx/Makefile         |  2 +-
 scripts/imx/imx-usb-loader.c | 29 ++++++++++++++++++++++++-----
 2 files changed, 25 insertions(+), 6 deletions(-)

-- 
2.39.2




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

* [PATCH v1 1/3] imx-usb-loader: error with success when displaying help on purpose
  2023-05-05  7:53 [PATCH v1 0/3] Add displaying a version number in Johannes Zink
@ 2023-05-05  7:53 ` Johannes Zink
  2023-05-05  7:56   ` Ahmad Fatoum
  2023-05-05  7:53 ` [PATCH v1 2/3] imx-usb-loader: use proper return code macros Johannes Zink
  2023-05-05  7:53 ` [PATCH v1 3/3] imx-usb-loader: add commandline option for displaying version number Johannes Zink
  2 siblings, 1 reply; 9+ messages in thread
From: Johannes Zink @ 2023-05-05  7:53 UTC (permalink / raw)
  To: barebox; +Cc: patchwork-jzi, kernel, Johannes Zink

Previously, whenever the usage was displayed, the imx-usb-loader exited
with error code.

When the usage is displayed due to invalid tool invocation, returning an
error code is valid behaviour, but when displaying the usage with the -h
command line option, success should be returned.

Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
---
 scripts/imx/imx-usb-loader.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/imx/imx-usb-loader.c b/scripts/imx/imx-usb-loader.c
index 41d57906c752..17b83b611765 100644
--- a/scripts/imx/imx-usb-loader.c
+++ b/scripts/imx/imx-usb-loader.c
@@ -1527,7 +1527,6 @@ static void usage(const char *prgname)
 		"-s           skip DCD included in image\n"
 		"-v           verbose (give multiple times to increase)\n"
 		"-h           this help\n", prgname);
-	exit(1);
 }
 
 int main(int argc, char *argv[])
@@ -1558,6 +1557,7 @@ int main(int argc, char *argv[])
 			break;
 		case 'h':
 			usage(argv[0]);
+			exit(0);
 		case 'd':
 			devtype = optarg;
 			break;
-- 
2.39.2




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

* [PATCH v1 2/3] imx-usb-loader: use proper return code macros
  2023-05-05  7:53 [PATCH v1 0/3] Add displaying a version number in Johannes Zink
  2023-05-05  7:53 ` [PATCH v1 1/3] imx-usb-loader: error with success when displaying help on purpose Johannes Zink
@ 2023-05-05  7:53 ` Johannes Zink
  2023-05-05  7:53 ` [PATCH v1 3/3] imx-usb-loader: add commandline option for displaying version number Johannes Zink
  2 siblings, 0 replies; 9+ messages in thread
From: Johannes Zink @ 2023-05-05  7:53 UTC (permalink / raw)
  To: barebox; +Cc: patchwork-jzi, kernel, Johannes Zink

It is considered good practice to use EXIT_SUCCESS and EXIT_FAILURE
instead of hardcoding magic values.

No functional changes.

Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
---
 scripts/imx/imx-usb-loader.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/scripts/imx/imx-usb-loader.c b/scripts/imx/imx-usb-loader.c
index 17b83b611765..e49c0bea6ca4 100644
--- a/scripts/imx/imx-usb-loader.c
+++ b/scripts/imx/imx-usb-loader.c
@@ -1557,7 +1557,7 @@ int main(int argc, char *argv[])
 			break;
 		case 'h':
 			usage(argv[0]);
-			exit(0);
+			exit(EXIT_SUCCESS);
 		case 'd':
 			devtype = optarg;
 			break;
@@ -1571,13 +1571,13 @@ int main(int argc, char *argv[])
 			w.do_dcd_once = 0;
 			break;
 		default:
-			exit(1);
+			exit(EXIT_FAILURE);
 		}
 	}
 
 	if (devtype && strcmp(devtype, "list") == 0) {
 		list_imx_device_types();
-		exit(0);
+		exit(EXIT_SUCCESS);
 	}
 
 	if (devtype && !devpath) {
@@ -1587,7 +1587,7 @@ int main(int argc, char *argv[])
 	if (optind == argc) {
 		fprintf(stderr, "no filename given\n");
 		usage(argv[0]);
-		exit(1);
+		exit(EXIT_FAILURE);
 	}
 
 	w.plug = 1;
-- 
2.39.2




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

* [PATCH v1 3/3] imx-usb-loader: add commandline option for displaying version number
  2023-05-05  7:53 [PATCH v1 0/3] Add displaying a version number in Johannes Zink
  2023-05-05  7:53 ` [PATCH v1 1/3] imx-usb-loader: error with success when displaying help on purpose Johannes Zink
  2023-05-05  7:53 ` [PATCH v1 2/3] imx-usb-loader: use proper return code macros Johannes Zink
@ 2023-05-05  7:53 ` Johannes Zink
  2023-05-05  8:02   ` Ahmad Fatoum
  2 siblings, 1 reply; 9+ messages in thread
From: Johannes Zink @ 2023-05-05  7:53 UTC (permalink / raw)
  To: barebox; +Cc: patchwork-jzi, kernel, Johannes Zink

For debugging purposes of the imx-usb-loader it can be helpful to
display the version number as a commandline option.

Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
---
 scripts/imx/Makefile         |  2 +-
 scripts/imx/imx-usb-loader.c | 21 ++++++++++++++++++++-
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/scripts/imx/Makefile b/scripts/imx/Makefile
index dbfa82910a55..43584427d27d 100644
--- a/scripts/imx/Makefile
+++ b/scripts/imx/Makefile
@@ -3,7 +3,7 @@
 hostprogs-always-$(CONFIG_ARCH_IMX_IMXIMAGE)	+= imx-image
 hostprogs-always-$(CONFIG_ARCH_IMX_USBLOADER)	+= imx-usb-loader
 
-HOSTCFLAGS_imx-usb-loader.o = `pkg-config --cflags libusb-1.0`
+HOSTCFLAGS_imx-usb-loader.o = `pkg-config --cflags libusb-1.0` -I$(objtree)/include/generated
 HOSTLDLIBS_imx-usb-loader  = `pkg-config --libs libusb-1.0`
 
 imx-usb-loader-target-userccflags += `$(CROSS_PKG_CONFIG) --cflags libusb-1.0`
diff --git a/scripts/imx/imx-usb-loader.c b/scripts/imx/imx-usb-loader.c
index e49c0bea6ca4..d4f876bb1c99 100644
--- a/scripts/imx/imx-usb-loader.c
+++ b/scripts/imx/imx-usb-loader.c
@@ -32,6 +32,7 @@
 #include <libusb.h>
 #include <getopt.h>
 #include <linux/kernel.h>
+#include <utsrelease.h>
 
 #include "../common.h"
 #include "../compiler.h"
@@ -1526,9 +1527,17 @@ static void usage(const char *prgname)
 		"-p <devpath> Specify device path: <bus>-<port>[.<port>]...\n"
 		"-s           skip DCD included in image\n"
 		"-v           verbose (give multiple times to increase)\n"
+		"--version    display version number\n"
 		"-h           this help\n", prgname);
 }
 
+static void version(const char *prgname)
+{
+	fprintf(stderr, "%s %s\n",
+		prgname, UTS_RELEASE);
+	exit(EXIT_SUCCESS);
+}
+
 int main(int argc, char *argv[])
 {
 	libusb_device **devs;
@@ -1544,10 +1553,20 @@ int main(int argc, char *argv[])
 	char *initfile = NULL;
 	char *devpath = NULL;
 	char *devtype = NULL;
+	int opt_version = 0;
+	struct option long_options[] = {
+		{"version", no_argument, &opt_version, 1},
+		{ }
+	};
 
 	w.do_dcd_once = 1;
 
-	while ((opt = getopt(argc, argv, "cvhd:i:p:s")) != -1) {
+	while ((opt = getopt_long(argc, argv, "cvhd:i:p:s", long_options, NULL)) != -1) {
+		if (opt_version) {
+			version(argv[0]);
+			exit(EXIT_SUCCESS);
+		}
+
 		switch (opt) {
 		case 'c':
 			verify = 1;
-- 
2.39.2




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

* Re: [PATCH v1 1/3] imx-usb-loader: error with success when displaying help on purpose
  2023-05-05  7:53 ` [PATCH v1 1/3] imx-usb-loader: error with success when displaying help on purpose Johannes Zink
@ 2023-05-05  7:56   ` Ahmad Fatoum
  2023-05-05  8:07     ` Johannes Zink
  0 siblings, 1 reply; 9+ messages in thread
From: Ahmad Fatoum @ 2023-05-05  7:56 UTC (permalink / raw)
  To: Johannes Zink, barebox; +Cc: kernel, patchwork-jzi

On 05.05.23 09:53, Johannes Zink wrote:
> Previously, whenever the usage was displayed, the imx-usb-loader exited
> with error code.
> 
> When the usage is displayed due to invalid tool invocation, returning an
> error code is valid behaviour, but when displaying the usage with the -h
> command line option, success should be returned.
> 
> Signed-off-by: Johannes Zink <j.zink@pengutronix.de>

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

The diff doesn't show that the other usage() callsite has an exit(1)
following it. Please note such things in the commit message in the
future.

> ---
>  scripts/imx/imx-usb-loader.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/imx/imx-usb-loader.c b/scripts/imx/imx-usb-loader.c
> index 41d57906c752..17b83b611765 100644
> --- a/scripts/imx/imx-usb-loader.c
> +++ b/scripts/imx/imx-usb-loader.c
> @@ -1527,7 +1527,6 @@ static void usage(const char *prgname)
>  		"-s           skip DCD included in image\n"
>  		"-v           verbose (give multiple times to increase)\n"
>  		"-h           this help\n", prgname);
> -	exit(1);
>  }
>  
>  int main(int argc, char *argv[])
> @@ -1558,6 +1557,7 @@ int main(int argc, char *argv[])
>  			break;
>  		case 'h':
>  			usage(argv[0]);
> +			exit(0);
>  		case 'd':
>  			devtype = optarg;
>  			break;

-- 
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 |




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

* Re: [PATCH v1 3/3] imx-usb-loader: add commandline option for displaying version number
  2023-05-05  7:53 ` [PATCH v1 3/3] imx-usb-loader: add commandline option for displaying version number Johannes Zink
@ 2023-05-05  8:02   ` Ahmad Fatoum
  2023-05-05  8:26     ` Johannes Zink
  0 siblings, 1 reply; 9+ messages in thread
From: Ahmad Fatoum @ 2023-05-05  8:02 UTC (permalink / raw)
  To: Johannes Zink, barebox; +Cc: kernel, patchwork-jzi

On 05.05.23 09:53, Johannes Zink wrote:
> For debugging purposes of the imx-usb-loader it can be helpful to
> display the version number as a commandline option.

This breaks builds outside of Kbuild, which is the only way to build a
(currently non-functional) imx-usb-loader for Windows. See

https://lore.barebox.org/barebox/20230411093844.1297004-1-a.fatoum@pengutronix.de/

What you may be able to do instead is -include $(objtree)/include/generated/utsrelease.h

and then have an

  #ifndef UTS_RELEASE
  #define UTS_RELEASE "external" // or something
  #endif

at the start of the file?

(Not sure how long GCC has had support for __has_include)

Cheers,
Ahmad

> 
> Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
> ---
>  scripts/imx/Makefile         |  2 +-
>  scripts/imx/imx-usb-loader.c | 21 ++++++++++++++++++++-
>  2 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/imx/Makefile b/scripts/imx/Makefile
> index dbfa82910a55..43584427d27d 100644
> --- a/scripts/imx/Makefile
> +++ b/scripts/imx/Makefile
> @@ -3,7 +3,7 @@
>  hostprogs-always-$(CONFIG_ARCH_IMX_IMXIMAGE)	+= imx-image
>  hostprogs-always-$(CONFIG_ARCH_IMX_USBLOADER)	+= imx-usb-loader
>  
> -HOSTCFLAGS_imx-usb-loader.o = `pkg-config --cflags libusb-1.0`
> +HOSTCFLAGS_imx-usb-loader.o = `pkg-config --cflags libusb-1.0` -I$(objtree)/include/generated
>  HOSTLDLIBS_imx-usb-loader  = `pkg-config --libs libusb-1.0`
>  
>  imx-usb-loader-target-userccflags += `$(CROSS_PKG_CONFIG) --cflags libusb-1.0`
> diff --git a/scripts/imx/imx-usb-loader.c b/scripts/imx/imx-usb-loader.c
> index e49c0bea6ca4..d4f876bb1c99 100644
> --- a/scripts/imx/imx-usb-loader.c
> +++ b/scripts/imx/imx-usb-loader.c
> @@ -32,6 +32,7 @@
>  #include <libusb.h>
>  #include <getopt.h>
>  #include <linux/kernel.h>
> +#include <utsrelease.h>
>  
>  #include "../common.h"
>  #include "../compiler.h"
> @@ -1526,9 +1527,17 @@ static void usage(const char *prgname)
>  		"-p <devpath> Specify device path: <bus>-<port>[.<port>]...\n"
>  		"-s           skip DCD included in image\n"
>  		"-v           verbose (give multiple times to increase)\n"
> +		"--version    display version number\n"
>  		"-h           this help\n", prgname);
>  }
>  
> +static void version(const char *prgname)
> +{
> +	fprintf(stderr, "%s %s\n",
> +		prgname, UTS_RELEASE);
> +	exit(EXIT_SUCCESS);
> +}
> +
>  int main(int argc, char *argv[])
>  {
>  	libusb_device **devs;
> @@ -1544,10 +1553,20 @@ int main(int argc, char *argv[])
>  	char *initfile = NULL;
>  	char *devpath = NULL;
>  	char *devtype = NULL;
> +	int opt_version = 0;
> +	struct option long_options[] = {
> +		{"version", no_argument, &opt_version, 1},
> +		{ }
> +	};
>  
>  	w.do_dcd_once = 1;
>  
> -	while ((opt = getopt(argc, argv, "cvhd:i:p:s")) != -1) {
> +	while ((opt = getopt_long(argc, argv, "cvhd:i:p:s", long_options, NULL)) != -1) {
> +		if (opt_version) {
> +			version(argv[0]);
> +			exit(EXIT_SUCCESS);
> +		}
> +
>  		switch (opt) {
>  		case 'c':
>  			verify = 1;

-- 
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 |




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

* Re: [PATCH v1 1/3] imx-usb-loader: error with success when displaying help on purpose
  2023-05-05  7:56   ` Ahmad Fatoum
@ 2023-05-05  8:07     ` Johannes Zink
  0 siblings, 0 replies; 9+ messages in thread
From: Johannes Zink @ 2023-05-05  8:07 UTC (permalink / raw)
  To: Ahmad Fatoum, barebox; +Cc: kernel, patchwork-jzi

Hi Ahmad,

thank you for your review.

On 5/5/23 09:56, Ahmad Fatoum wrote:
> On 05.05.23 09:53, Johannes Zink wrote:
>> Previously, whenever the usage was displayed, the imx-usb-loader exited
>> with error code.
>>
>> When the usage is displayed due to invalid tool invocation, returning an
>> error code is valid behaviour, but when displaying the usage with the -h
>> command line option, success should be returned.
>>
>> Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
> 
> Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> 
> The diff doesn't show that the other usage() callsite has an exit(1)
> following it. Please note such things in the commit message in the
> future.

Ack, gonna add it in v2. Also, I will fix the subject line to "...exit 
with success..." instead of "...error with success...".

Johannes

> 
>> ---
>>   scripts/imx/imx-usb-loader.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/scripts/imx/imx-usb-loader.c b/scripts/imx/imx-usb-loader.c
>> index 41d57906c752..17b83b611765 100644
>> --- a/scripts/imx/imx-usb-loader.c
>> +++ b/scripts/imx/imx-usb-loader.c
>> @@ -1527,7 +1527,6 @@ static void usage(const char *prgname)
>>   		"-s           skip DCD included in image\n"
>>   		"-v           verbose (give multiple times to increase)\n"
>>   		"-h           this help\n", prgname);
>> -	exit(1);
>>   }
>>   
>>   int main(int argc, char *argv[])
>> @@ -1558,6 +1557,7 @@ int main(int argc, char *argv[])
>>   			break;
>>   		case 'h':
>>   			usage(argv[0]);
>> +			exit(0);
>>   		case 'd':
>>   			devtype = optarg;
>>   			break;
> 

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




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

* Re: [PATCH v1 3/3] imx-usb-loader: add commandline option for displaying version number
  2023-05-05  8:02   ` Ahmad Fatoum
@ 2023-05-05  8:26     ` Johannes Zink
  2023-05-05  8:32       ` Ahmad Fatoum
  0 siblings, 1 reply; 9+ messages in thread
From: Johannes Zink @ 2023-05-05  8:26 UTC (permalink / raw)
  To: barebox

Hi Ahmad,

thanks for reviewing this series.

On 5/5/23 10:02, Ahmad Fatoum wrote:
> On 05.05.23 09:53, Johannes Zink wrote:
>> For debugging purposes of the imx-usb-loader it can be helpful to
>> display the version number as a commandline option.
> 
> This breaks builds outside of Kbuild, which is the only way to build a
> (currently non-functional) imx-usb-loader for Windows. See
> 
> https://lore.barebox.org/barebox/20230411093844.1297004-1-a.fatoum@pengutronix.de/
> 

I see, I was not aware that there are users of an out-of-Kbuild build.

> What you may be able to do instead is -include $(objtree)/include/generated/utsrelease.h
> 
> and then have an
> 
>    #ifndef UTS_RELEASE
>    #define UTS_RELEASE "external" // or something
>    #endif
> 
> at the start of the file?
> 

sounds good to me, gonna add this to v2. Out of interest: does the 
outside-of-kBuild use a custom makefile? Otherwise -include would 
probably complain that the file is unknown...

> (Not sure how long GCC has had support for __has_include)

I'd prefer not to use __has_include, as this might not be supported with 
redmondish toolchains, introducing an issue to fix another does not seem 
too appealing to me ;-)

Johannes

> 
> Cheers,
> Ahmad
> 
>>
>> Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
>> ---
>>   scripts/imx/Makefile         |  2 +-
>>   scripts/imx/imx-usb-loader.c | 21 ++++++++++++++++++++-
>>   2 files changed, 21 insertions(+), 2 deletions(-)
>>
>> diff --git a/scripts/imx/Makefile b/scripts/imx/Makefile
>> index dbfa82910a55..43584427d27d 100644
>> --- a/scripts/imx/Makefile
>> +++ b/scripts/imx/Makefile
>> @@ -3,7 +3,7 @@
>>   hostprogs-always-$(CONFIG_ARCH_IMX_IMXIMAGE)	+= imx-image
>>   hostprogs-always-$(CONFIG_ARCH_IMX_USBLOADER)	+= imx-usb-loader
>>   
>> -HOSTCFLAGS_imx-usb-loader.o = `pkg-config --cflags libusb-1.0`
>> +HOSTCFLAGS_imx-usb-loader.o = `pkg-config --cflags libusb-1.0` -I$(objtree)/include/generated
>>   HOSTLDLIBS_imx-usb-loader  = `pkg-config --libs libusb-1.0`
>>   
>>   imx-usb-loader-target-userccflags += `$(CROSS_PKG_CONFIG) --cflags libusb-1.0`
>> diff --git a/scripts/imx/imx-usb-loader.c b/scripts/imx/imx-usb-loader.c
>> index e49c0bea6ca4..d4f876bb1c99 100644
>> --- a/scripts/imx/imx-usb-loader.c
>> +++ b/scripts/imx/imx-usb-loader.c
>> @@ -32,6 +32,7 @@
>>   #include <libusb.h>
>>   #include <getopt.h>
>>   #include <linux/kernel.h>
>> +#include <utsrelease.h>
>>   
>>   #include "../common.h"
>>   #include "../compiler.h"
>> @@ -1526,9 +1527,17 @@ static void usage(const char *prgname)
>>   		"-p <devpath> Specify device path: <bus>-<port>[.<port>]...\n"
>>   		"-s           skip DCD included in image\n"
>>   		"-v           verbose (give multiple times to increase)\n"
>> +		"--version    display version number\n"
>>   		"-h           this help\n", prgname);
>>   }
>>   
>> +static void version(const char *prgname)
>> +{
>> +	fprintf(stderr, "%s %s\n",
>> +		prgname, UTS_RELEASE);
>> +	exit(EXIT_SUCCESS);
>> +}
>> +
>>   int main(int argc, char *argv[])
>>   {
>>   	libusb_device **devs;
>> @@ -1544,10 +1553,20 @@ int main(int argc, char *argv[])
>>   	char *initfile = NULL;
>>   	char *devpath = NULL;
>>   	char *devtype = NULL;
>> +	int opt_version = 0;
>> +	struct option long_options[] = {
>> +		{"version", no_argument, &opt_version, 1},
>> +		{ }
>> +	};
>>   
>>   	w.do_dcd_once = 1;
>>   
>> -	while ((opt = getopt(argc, argv, "cvhd:i:p:s")) != -1) {
>> +	while ((opt = getopt_long(argc, argv, "cvhd:i:p:s", long_options, NULL)) != -1) {
>> +		if (opt_version) {
>> +			version(argv[0]);
>> +			exit(EXIT_SUCCESS);
>> +		}
>> +
>>   		switch (opt) {
>>   		case 'c':
>>   			verify = 1;
> 

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




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

* Re: [PATCH v1 3/3] imx-usb-loader: add commandline option for displaying version number
  2023-05-05  8:26     ` Johannes Zink
@ 2023-05-05  8:32       ` Ahmad Fatoum
  0 siblings, 0 replies; 9+ messages in thread
From: Ahmad Fatoum @ 2023-05-05  8:32 UTC (permalink / raw)
  To: Johannes Zink, barebox

On 05.05.23 10:26, Johannes Zink wrote:
> Hi Ahmad,
> 
> thanks for reviewing this series.
> 
> On 5/5/23 10:02, Ahmad Fatoum wrote:
>> On 05.05.23 09:53, Johannes Zink wrote:
>>> For debugging purposes of the imx-usb-loader it can be helpful to
>>> display the version number as a commandline option.
>>
>> This breaks builds outside of Kbuild, which is the only way to build a
>> (currently non-functional) imx-usb-loader for Windows. See
>>
>> https://lore.barebox.org/barebox/20230411093844.1297004-1-a.fatoum@pengutronix.de/
>>
> 
> I see, I was not aware that there are users of an out-of-Kbuild build.

It's not a nice workaround, but it's better than nothing and I'd
prefer it to not be build-broken (and hopefully it will even be
functional soon).

>> What you may be able to do instead is -include $(objtree)/include/generated/utsrelease.h
>>
>> and then have an
>>
>>    #ifndef UTS_RELEASE
>>    #define UTS_RELEASE "external" // or something
>>    #endif
>>
>> at the start of the file?
>>
> 
> sounds good to me, gonna add this to v2. Out of interest: does the outside-of-kBuild use a custom makefile? Otherwise -include would probably complain that the file is unknown...

Just the commands in the above referenced cover letter. The user would skip the
include line and it would just work.

>> (Not sure how long GCC has had support for __has_include)
> 
> I'd prefer not to use __has_include, as this might not be supported with redmondish toolchains, introducing an issue to fix another does not seem too appealing to me ;-)

That's out of scope anyway. GCC is the only thing you need to consider,
even clang can't build barebox for arm32 for example.

Cheers,
Ahmad

> 
> Johannes
> 
>>
>> Cheers,
>> Ahmad
>>
>>>
>>> Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
>>> ---
>>>   scripts/imx/Makefile         |  2 +-
>>>   scripts/imx/imx-usb-loader.c | 21 ++++++++++++++++++++-
>>>   2 files changed, 21 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/scripts/imx/Makefile b/scripts/imx/Makefile
>>> index dbfa82910a55..43584427d27d 100644
>>> --- a/scripts/imx/Makefile
>>> +++ b/scripts/imx/Makefile
>>> @@ -3,7 +3,7 @@
>>>   hostprogs-always-$(CONFIG_ARCH_IMX_IMXIMAGE)    += imx-image
>>>   hostprogs-always-$(CONFIG_ARCH_IMX_USBLOADER)    += imx-usb-loader
>>>   -HOSTCFLAGS_imx-usb-loader.o = `pkg-config --cflags libusb-1.0`
>>> +HOSTCFLAGS_imx-usb-loader.o = `pkg-config --cflags libusb-1.0` -I$(objtree)/include/generated
>>>   HOSTLDLIBS_imx-usb-loader  = `pkg-config --libs libusb-1.0`
>>>     imx-usb-loader-target-userccflags += `$(CROSS_PKG_CONFIG) --cflags libusb-1.0`
>>> diff --git a/scripts/imx/imx-usb-loader.c b/scripts/imx/imx-usb-loader.c
>>> index e49c0bea6ca4..d4f876bb1c99 100644
>>> --- a/scripts/imx/imx-usb-loader.c
>>> +++ b/scripts/imx/imx-usb-loader.c
>>> @@ -32,6 +32,7 @@
>>>   #include <libusb.h>
>>>   #include <getopt.h>
>>>   #include <linux/kernel.h>
>>> +#include <utsrelease.h>
>>>     #include "../common.h"
>>>   #include "../compiler.h"
>>> @@ -1526,9 +1527,17 @@ static void usage(const char *prgname)
>>>           "-p <devpath> Specify device path: <bus>-<port>[.<port>]...\n"
>>>           "-s           skip DCD included in image\n"
>>>           "-v           verbose (give multiple times to increase)\n"
>>> +        "--version    display version number\n"
>>>           "-h           this help\n", prgname);
>>>   }
>>>   +static void version(const char *prgname)
>>> +{
>>> +    fprintf(stderr, "%s %s\n",
>>> +        prgname, UTS_RELEASE);
>>> +    exit(EXIT_SUCCESS);
>>> +}
>>> +
>>>   int main(int argc, char *argv[])
>>>   {
>>>       libusb_device **devs;
>>> @@ -1544,10 +1553,20 @@ int main(int argc, char *argv[])
>>>       char *initfile = NULL;
>>>       char *devpath = NULL;
>>>       char *devtype = NULL;
>>> +    int opt_version = 0;
>>> +    struct option long_options[] = {
>>> +        {"version", no_argument, &opt_version, 1},
>>> +        { }
>>> +    };
>>>         w.do_dcd_once = 1;
>>>   -    while ((opt = getopt(argc, argv, "cvhd:i:p:s")) != -1) {
>>> +    while ((opt = getopt_long(argc, argv, "cvhd:i:p:s", long_options, NULL)) != -1) {
>>> +        if (opt_version) {
>>> +            version(argv[0]);
>>> +            exit(EXIT_SUCCESS);
>>> +        }
>>> +
>>>           switch (opt) {
>>>           case 'c':
>>>               verify = 1;
>>
> 

-- 
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 |




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

end of thread, other threads:[~2023-05-05  8:34 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-05  7:53 [PATCH v1 0/3] Add displaying a version number in Johannes Zink
2023-05-05  7:53 ` [PATCH v1 1/3] imx-usb-loader: error with success when displaying help on purpose Johannes Zink
2023-05-05  7:56   ` Ahmad Fatoum
2023-05-05  8:07     ` Johannes Zink
2023-05-05  7:53 ` [PATCH v1 2/3] imx-usb-loader: use proper return code macros Johannes Zink
2023-05-05  7:53 ` [PATCH v1 3/3] imx-usb-loader: add commandline option for displaying version number Johannes Zink
2023-05-05  8:02   ` Ahmad Fatoum
2023-05-05  8:26     ` Johannes Zink
2023-05-05  8:32       ` Ahmad Fatoum

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