Re: [PATCH v1 3/3] imx-usb-loader: add commandline option for displaying version number

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

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 |