* Ohad Ben-Cohen <ohad@xxxxxxxxxx> [121018 10:00]: > Hi Igor, > > On Wed, Oct 17, 2012 at 2:43 PM, Igor Grinberg <grinberg@xxxxxxxxxxxxxx> wrote: > > You are adding declarations inside the common-board-devices.h, > > but the implementation inside the devices.c. > > I can see that devices.c has only on-soc devices in it. > > So, I would expect to have the wl12xx_board_init() function implementation > > inside the common-board-devices.c file. > > I really don't mind. Tony do you have any preference? Yes common-board-devices.c would be better. > > Another minor nit: I don't think you need to mark the declaration as __init, > > only the implementation, or is it for documenting it so? > > It may be, but I don't really mind removing it. Let's remove it if > we'll move to common-board-devices.c, otherwise it probably isn't > worth the noise. OK > > Instead of the above, wouldn't it be better to have: > > #if defined(CONFIG_WL12XX_PLATFORM_DATA) > > int __init wl12xx_board_init(struct wl12xx_platform_data *wlan_data, int irq_gpio) > > { > > ... > > } > > #else > > inline int wl12xx_board_init(struct wl12xx_platform_data *wlan_data, int irq_gpio) > > { > > return 0; > > } > > #endif > > I think readability-wise we're probably better off without the #ifdef. We could optimize away a minimal amount of code for many configurations with the ifdef? :) Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html