[U-Boot] [PATCH v3] usb: new board-specific USB init interface

[Mateusz Zalega <m.zalega@samsung.com>]

On 09/05/13 19:51, Marek Vasut wrote:
>>> Why not wrap board_usb_init() and board_usb_init_fail() into single call.
>>> You now pass some flags to board_usb_init() already, so just add another
>>> for the fail case. How does it sound to you?
>>
>> Like overengineering. It would lead to "board_usb_init(USB_INIT_ALL,
>> USB_INIT_DEVICE, USB_CLEANUP)" calls, which are not very readable.
> 
> This is not what I mean, see this:
> 
> int board_usb_init(int index, enum board_usb_init_type init)
> 
> Add a new "init" type (or maybe change the init field to be flags) that will say 
> "OK, do a fail init" ?

As for providing a way to do a selective cleanup if anything gone wrong,
it's a good idea, but adding such functionality to board_usb_init()
would be confusing, especially for newcomers. Why not do this in
board_usb_init_fail(int index, enum board_usb_init_type)?

...and maybe rename board_usb_init_fail to board_usb_cleanup.

>>> Moreover, the 'int index' should likely be unsigned int and the special
>>> value to init all controllers at once should probably then be 0xffffffff
>>
>> Despite our greatest ambitions, I don't think we're likely to use more
>> than 2^31-1 USB controllers at a time. Besides, negative values look
>> better both in code and debugger session.
> 
> Thinking of it further, instead of using negative value here, like I mentioned 
> above, why not make the "board_usb_init_type" into a field of flags , then add 
> flag to init all controllers at once ?

That's unnecessary. It wouldn't lead to any practical advantage over
existing interface.

Best Regards,

-- 
Mateusz Zalega
Samsung R&D Institute Poland