Dear Mateusz Zalega,
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.
Cleanup is nice name, yeah.
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.
The advantage would be you won't be mixing two things (value AND value with special meaning) into the "index" parameter.
Best regards, Marek Vasut