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 ?
Thanks,
Ohad.
>
> Something like:
>
> wl12xx_platform_data.c:
> #include <linux/module.h>
> #include <whatever/required/for/wl12xx.h>
>
> static struct wl12xx_platform_data *platform_data;
>
> int __init wl12xx_set_platform_data(const struct wl12xx_platform_data *data)
> {
> if (platform_data)
> return -EBUSY;
> platform_data = kmemdup(data, sizeof(*data), GFP_KERNEL);
> if (!platform_data)
> return -ENOMEM;
> return 0;
> }
>
> int wl12xx_get_platform_data(struct wl12xx_platform_data *data)
> {
> if (platform_data) {
> memcpy(data, platform_data, sizeof(*data));
> return 0;
> }
> return -ENODEV;
> }
> EXPORT_SYMBOL(wl12xx_get_platform_data);
>
> And in the Kconfig, something like:
>
> config WL12XX_PLATFORM_DATA
> bool
> depends on WL12XX != n
> default y
>
> This means there'll be no confusion over who owns the 'embedded data',
> typechecking is preserved, and no possibility for the wrong driver to
> pick up the data.
>
--
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
