[U-Boot] [PATCH 1/2] Move all command code into its own directory

[Simon Glass]

Hi,

On 19 January 2016 at 08:53, Tom Rini <trini at konsulko.com> wrote:
> On Tue, Jan 19, 2016 at 12:59:10PM +0900, Masahiro Yamada wrote:
>> Hi Simon,
>>
>> 2016-01-18 12:53 GMT+09:00 Simon Glass <sjg at chromium.org>:
>> > There are a lot of unrelated files in common, including all of the commands.
>> > Moving them into their own directory makes them easier to find and is more
>> > logical.
>> >
>> > Some commands include non-command code, such as cmd_scsi.c. This should be
>> > sorted out at some point so that the function can be enabled with or without
>> > the associated command.
>> >
>> > Unfortunately, with m68k I get this error:
>> >
>> > m68k:  +   M5329AFEE
>> > +arch/m68k/cpu/mcf532x/start.o: In function `_start':
>> > +arch/m68k/cpu/mcf532x/start.S:159:(.text+0x452): relocation truncated to fit: R_68K_PC16 against symbol `board_init_f' defined in .text.board_init_f section in common/built-in.o
>> >
>> > I hope someone can shed some light on what this means. I hope it isn't
>> > depending on the position of code in the image.
>> >
>> > Signed-off-by: Simon Glass <sjg at chromium.org>
>>
>>
>> Thanks for working on this!
>>
>> This is a nice improvement,
>> but we might want to think about the best place for device access commands
>> in the future.
>>
>>
>> I mean,
>>
>> cmd_nand.c  in drivers/mtd/nand/ rather than cmd/
>> cmd_mmc.c   in drivers/mmc/ rather than cmd/
>> cmd_usb.c   in drivers/usb/ rather thant cmd/
>>
>> etc.
>
> But we're not going to have anywhere near a 1:1 mapping of commands to
> some subdirectories.  So I would make the inverse point, we would have
> commands in drivers/mtd/nand, drivers/mmc, drivers/usb/host,
> drivers/usb/gadget, drivers/dm, board/foo instead of being able to say
> "If you add a new command, it goes into cmd/" and similarly, "Did you
> see cmd/foo.c?  That sound a whole lot like what you're doing in
> cmd/fauxoo.c" and possibly even short-circuting that problem at the
> design phase when people see that someone else already had a similar
> problem and solved it.

Yes I think it helps to have commands in one directory. There may be
some board-specific commands elsewhere in the tree, but I think it is
useful.

>
>> This patch moves cmd_usb.c, but leaves common/usb*.c.
>>
>> With this patch, USB files are located in three places:
>>   - cmd/cmd_usb.c
>>   - common/usb*.c
>>   - drivers/usb/
>>
>>
>> Is collecting all them into drivers/usb/ more logical?
>
> Skimming common/usb*.c, I think a lot of that needs to move into
> drivers/usb/<various dirs>.

Yes, and in cases where there is non-command logic in cmd/ we should
try to fix it.

>
>> I think this needs closer look and more discussion, though.
>
> I like this approach as it does expose some of the problems with the
> current lack-of-structure, ie all of the non-command stuff in the cmd
> files, and other things we've shoved into common/ that don't quite
> belong there now really.  It will also make it easier to split up the
> current command files where we have a lot of 'hidden' commands.
>
> --
> Tom

That's my feeling too. In principle I think the commands should be a
veneer on top of the implementation. Ideally there should not be a lot
of processing logic in the cmd/ files. We are close to this in most
cases, but things like USB and SCSI need work.

Regards,
Simon