[U-Boot] [PATCH v4 8/8] sandbox: Add basic command line parsing

[Simon Glass]

Hi Mike,

On Mon, Feb 27, 2012 at 10:32 AM, Mike Frysinger <vapier at gentoo.org> wrote:
> On Monday 27 February 2012 00:43:30 Simon Glass wrote:
>> On Sun, Feb 26, 2012 at 8:42 PM, Mike Frysinger wrote:
>> > drivers/mtd/spi/sandbox.c:
>> > static int sb_cmdline_cb_spi_sf(struct sandbox_state *state, const char
>> > *arg) {
>> >    unsigned long bus, cs;
>> >    const char *spec = sb_spi_parse_spec(arg, &bus, &cs);
>> >
>> >    if (!spec)
>> >        return 1;
>> >
>> >    state->spi[bus][cs][0] = &sb_sf_ops;
>> >    state->spi[bus][cs][1] = spec;
>> >    return 0;
>> > }
>> > SB_CMDLINE_OPT(spi_sf, 1, "connect a SPI flash: <bus>:<cs>:<id>:<file>");
>>
>> sandbox...give me your address and I'll send you a cheque to cover the
>> bytes so used :-)
>
> it'll have to be a big one to stop the cascading line wrapping :P
>
>> > and people could do:
>> >        ./u-boot --spi_sf 0:3:m25p16:./some-file.bin
>> > this would connect the spi flash simulation up to the simulated spi bus 0
>> > on cs 3 and have it simulate a m25p16 flash with "some-file.bin" backing
>> > it.
>> >
>> > if someone were to enter the wrong value for
>> > "0:3:m25p16:./some-file.bin", how do you propose we communicate that ?
>> >  the existing code can easily signal the higher parsing logic via return
>> > values, but then we'd just get:
>> >        failed to parse option: 0:3:m25p16:./some-file.bin
>> >
>> > but what exactly did the code not like ?  was it the bus # ?  the cs # ?
>> >  the spi flash id ?  the backing file ?  if the getopt code has access
>> > to printf(), we can clearly communicate to the user what is going wrong.
>>
>> Yes I think printf() is useful in getopt, I just would prefer to avoid
>> using U-Boot's printf. It goes through all the console code, and we
>> might be running a test that deliberately breaks that, perhaps.
>> Actually this could be a pretty important thing to sort out - we need
>> to keep a clear boundary between test code and U-Boot code (as we
>> discussed over GPIO). Having the test code use U-Boot's printf() is
>> blurring that.
>
> we probably need to add architecture details like this to doc/README.sandbox
> so we can stay on the same page ...

Yes I agree.

>
>> To your example, if the syntax is correct perhaps, then it got through
>> initial parsing. If the filename is wrong, or the bus number, then
>> perhaps there will be a message and failure much later. Perhaps the
>> initial parsing just does what it can to avoid running past an obvious
>> syntax error. But we can't really know success until we open the file,
>> or try the bus.
>
> if the bus/cs are valid, the rest of the parsing is delayed until you do "sf
> probe" at which point you'd get an explanation of what is wrong.  i guess we
> can live with this for now.

Yes, I feel like we have done more than enough plumbing in advance of
use. Let's see how it goes.

Regards,
Simon

> -mike