Re: [PATCH master 1/4] Revert "imx-usb-loader: add commandline option for displaying version number"

[Johannes Zink <j.zink@pengutronix.de>]

Hi Ahmad,

On 5/26/23 08:20, Ahmad Fatoum wrote:
[snip]

> 
> Agreed. I am looking forward to a v2 that doesn't break the build. ;)
> 

ack - just sent out a fixup.

>> Do you know of a flag that is set when the files have been generated?
>> Maybe we can include the file conditionally if and only if it has been generated.
> 
> That would mean we accept that the race condition is there and that depending on
> timing/parallelism, we either end up with a version or unknown...
> 

ack

>> Do you think that alternatively we can require the prepare make target as a dependency for scripts/imx?
> 
> AFAICS, scripts/Makefile is called recursively, so it may not be feasible to depend on
> top-level Makefile's prepare for a target defined within. If everything needed for
> Kconfig is part of another target (e.g. config or scripts_basic), then maybe scripts
> could be made to depend on prepare. A worthwhile alternative may be moving stuff not
> needed for the build, like all USB loaders, into a tools/ directory, but then one
> needs to take care of dependencies on files in scripts/.
> 

I had another thought - just using the Version variables from the top 
level makefile is much easier and also does the trick.

Johannes

> Cheers,
> Ahmad
> 
>>
>> Johannes
>>
>>> Until this is resolved, revert commit c1b50061f4b33482ae749f9d6d6c92aa5bf6b37a.
>>>
>>> Cc: Johannes Zink <j.zink@pengutronix.de>
>>> Signed-off-by: Ahmad Fatoum <ahmad@a3f.at>
>>> ---
>>>    scripts/imx/Makefile         |  2 +-
>>>    scripts/imx/imx-usb-loader.c | 22 +---------------------
>>>    2 files changed, 2 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/scripts/imx/Makefile b/scripts/imx/Makefile
>>> index b3be3886d8eb..dbfa82910a55 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` -include $(objtree)/include/generated/utsrelease.h
>>> +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`
>>> diff --git a/scripts/imx/imx-usb-loader.c b/scripts/imx/imx-usb-loader.c
>>> index 676f077c2557..839288f753cc 100644
>>> --- a/scripts/imx/imx-usb-loader.c
>>> +++ b/scripts/imx/imx-usb-loader.c
>>> @@ -46,10 +46,6 @@
>>>    #define FT_DCD    0xee
>>>    #define FT_LOAD_ONLY    0x00
>>>    -#ifndef UTS_RELEASE
>>> -#define UTS_RELEASE "unknown"
>>> -#endif
>>> -
>>>    /*
>>>     * comment from libusb:
>>>     * As per the USB 3.0 specs, the current maximum limit for the depth is 7.
>>> @@ -1530,15 +1526,9 @@ 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);
>>> -}
>>> -
>>>    int main(int argc, char *argv[])
>>>    {
>>>        libusb_device **devs;
>>> @@ -1554,20 +1544,10 @@ 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_long(argc, argv, "cvhd:i:p:s", long_options, NULL)) != -1) {
>>> -        if (opt_version) {
>>> -            version(argv[0]);
>>> -            exit(EXIT_SUCCESS);
>>> -        }
>>> -
>>> +    while ((opt = getopt(argc, argv, "cvhd:i:p:s")) != -1) {
>>>            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    |