Return-path: Received: from mail-gw0-f46.google.com ([74.125.83.46]:46926 "EHLO mail-gw0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932618Ab0HDLZA (ORCPT ); Wed, 4 Aug 2010 07:25:00 -0400 MIME-Version: 1.0 In-Reply-To: References: <1279733634-21974-1-git-send-email-ohad@wizery.com> <1279733634-21974-4-git-send-email-ohad@wizery.com> From: Ohad Ben-Cohen Date: Wed, 4 Aug 2010 14:24:39 +0300 Message-ID: Subject: Re: [PATCH v2 03/20] mmc: support embedded data field in mmc_host To: Vitaly Wool Cc: linux-wireless@vger.kernel.org, linux-mmc@vger.kernel.org, linux-omap@vger.kernel.org, Kalle Valo , Pandita Vikram , linux@arm.linux.org.uk, Nicolas Pitre , Tony Lindgren , Roger Quadros , San Mehat , Chikkature Rajashekar Madhusudhan , Luciano Coelho , akpm@linux-foundation.org, linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-wireless-owner@vger.kernel.org List-ID: Hi Vitaly, On Tue, Aug 3, 2010 at 5:17 PM, Vitaly Wool wrote: > On Mon, Aug 2, 2010 at 11:35 PM, Ohad Ben-Cohen 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): diff --git a/arch/arm/mach-msm/board-trout-mmc.c b/arch/arm/mach-msm/board-trout index 13755f5..df32b2f 100644 --- a/arch/arm/mach-msm/board-trout-mmc.c +++ b/arch/arm/mach-msm/board-trout-mmc.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -297,11 +298,16 @@ int trout_wifi_reset(int on) EXPORT_SYMBOL(trout_wifi_reset); #endif +struct wl12xx_platform_data trout_wlan_data = { + .irq = 62, /* put here your irq number */ + .board_ref_clock = 1, /* put here your ref clock */ +}; + static struct mmc_platform_data trout_wifi_data = { .ocr_mask = MMC_VDD_28_29, .status = trout_wifi_status, .register_status_notify = trout_wifi_status_register, - .embedded_sdio = &trout_wifi_emb_data, + .embedded_sdio = &trout_wlan_data, }; int __init trout_init_mmc(unsigned int sys_rev) diff --git a/drivers/mmc/host/msm_sdcc.c b/drivers/mmc/host/msm_sdcc.c index 1697d42..c40f0d1 100755 --- a/drivers/mmc/host/msm_sdcc.c +++ b/drivers/mmc/host/msm_sdcc.c @@ -1261,6 +1261,7 @@ msmsdcc_probe(struct platform_device *pdev) mmc->f_min = msmsdcc_fmin; mmc->f_max = msmsdcc_fmax; mmc->ocr_avail = plat->ocr_mask; + mmc_set_embedded_data(mmc, plat->embedded_sdio); if (msmsdcc_4bit) mmc->caps |= MMC_CAP_4_BIT_DATA; Is it really that complex ? Thanks, Ohad.