On Wed, Aug 4, 2010 at 2:42 PM, Ohad Ben-Cohen <ohad@xxxxxxxxxx> wrote: > On Wed, Aug 4, 2010 at 2:41 PM, Russell King - ARM Linux > <linux@xxxxxxxxxxxxxxxx> wrote: >> On Wed, Aug 04, 2010 at 02:24:39PM +0300, Ohad Ben-Cohen wrote: >>> On Tue, Aug 3, 2010 at 5:17 PM, Vitaly Wool <vitalywool@xxxxxxxxx> wrote: >>> > On Mon, Aug 2, 2010 at 11:35 PM, Ohad Ben-Cohen <ohad@xxxxxxxxxx> wrote: >>> >> I'm honestly trying to understand your concerns, but I'm afraid that >>> >> just saying "it's a hack" is not too informative. Can you please >>> >> explain what do you think is technically wrong with the proposed >>> >> solution ? is it not general enough for other use cases ? will it >>> >> break something ? >>> >>> > So if I'd like to set the *same* structure for the *same* WL1271 >>> > driver, provided that I'm working with the other platform, I'll need >>> > to do the following: >>> > - add the pointer to the board-specific header; >>> > - add the structure to the board-specific C file and propagate its >>> > pointer to the controller driver; >>> > - add setting the pointer in the core structure into the controller driver. >>> > >>> > This is far from being intuitive. This means we need to hack, >>> > generally speaking, all the MMC controller drivers to get it working >>> > on all platforms. This is error prone. >>> >>> You make it sound really complex. >>> >>> Let's see what it means to add it to a totally different platform. >>> >>> As an example, let's take Google's ADP1 which is based on a different >>> host controller (msm-sdcc), and add the required support (untested of >>> course, just a quick sketch, patch is based on android's msm git): >> >> What if some other driver gets attached and tries to use this >> platform data? This whole idea sounds extremely silly, cumbersome, >> error prone, and is inviting misuse by normal MMC sockets. >> >> Why not arrange for a small piece of code to be built into the kernel >> when this driver is selected as a module or built-in, which handles >> the passing of platform data to the driver? > > It's interesting. > > The only drawback I can see is that it has an inherent limitation of > having only a single wl1271 device on board, but there are no such > boards outside development/testing labs. > > Vitaly would that work for you too ? Works for me, but I've got a remark. If we try to generalize that idea to handle multiple devices it will be similar to the following: static struct wl12xx_platform_data platform_data[MAX_WL12XX_DEVICES]; int __init wl12xx_set_platform_data(const struct wl12xx_platform_data *data, int id); int wl12xx_get_platform_data(struct wl12xx_platform_data *data, int id); which will look pretty much like... yes, a simplified/customized version board_info applied to a single driver. But anyway I'm fine with that. ~Vitaly -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
