[PATCH v2 03/20] mmc: support embedded data field in mmc_host

[Vitaly Wool]

On Wed, Aug 4, 2010 at 2:42 PM, Ohad Ben-Cohen <ohad at wizery.com> wrote:
> On Wed, Aug 4, 2010 at 2:41 PM, Russell King - ARM Linux
> <linux at arm.linux.org.uk> 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 at gmail.com> wrote:
>>> > On Mon, Aug 2, 2010 at 11:35 PM, Ohad Ben-Cohen <ohad at wizery.com> 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