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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
