From patchwork Sat Jun 11 13:16:38 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Shawn Guo X-Patchwork-Id: 871552 Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by demeter2.kernel.org (8.14.4/8.14.4) with ESMTP id p5BDCXsO028594 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Sat, 11 Jun 2011 13:12:54 GMT Received: from canuck.infradead.org ([2001:4978:20e::1]) by merlin.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1QVNzG-0001AU-Cc; Sat, 11 Jun 2011 13:12:26 +0000 Received: from localhost ([127.0.0.1] helo=canuck.infradead.org) by canuck.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1QVNzF-0006j8-Um; Sat, 11 Jun 2011 13:12:25 +0000 Received: from am1ehsobe001.messaging.microsoft.com ([213.199.154.204] helo=AM1EHSOBE001.bigfish.com) by canuck.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1QVNzB-0006ip-Su for linux-arm-kernel@lists.infradead.org; Sat, 11 Jun 2011 13:12:23 +0000 Received: from mail42-am1-R.bigfish.com (10.3.201.248) by AM1EHSOBE001.bigfish.com (10.3.204.21) with Microsoft SMTP Server id 14.1.225.22; Sat, 11 Jun 2011 13:12:17 +0000 Received: from mail42-am1 (localhost.localdomain [127.0.0.1]) by mail42-am1-R.bigfish.com (Postfix) with ESMTP id 6C7202702E7; Sat, 11 Jun 2011 13:12:17 +0000 (UTC) X-SpamScore: -14 X-BigFish: VS-14(zz148cM1432N98dKzz1202hzz8275bh8275dhz2dh2a8h668h839h61h) X-Spam-TCS-SCL: 0:0 X-Forefront-Antispam-Report: CIP:70.37.183.190; KIP:(null); UIP:(null); IPVD:NLI; H:mail.freescale.net; RD:none; EFVD:NLI Received: from mail42-am1 (localhost.localdomain [127.0.0.1]) by mail42-am1 (MessageSwitch) id 1307797918332489_19075; Sat, 11 Jun 2011 13:11:58 +0000 (UTC) Received: from AM1EHSMHS007.bigfish.com (unknown [10.3.201.253]) by mail42-am1.bigfish.com (Postfix) with ESMTP id BC6D8D4807C; Sat, 11 Jun 2011 13:11:17 +0000 (UTC) Received: from mail.freescale.net (70.37.183.190) by AM1EHSMHS007.bigfish.com (10.3.207.107) with Microsoft SMTP Server (TLS) id 14.1.225.22; Sat, 11 Jun 2011 13:11:17 +0000 Received: from az33smr01.freescale.net (10.64.34.199) by 039-SN1MMR1-002.039d.mgd.msft.net (10.84.1.15) with Microsoft SMTP Server id 14.1.289.8; Sat, 11 Jun 2011 08:11:15 -0500 Received: from S2100-06.ap.freescale.net (S2100-06.ap.freescale.net [10.192.242.125]) by az33smr01.freescale.net (8.13.1/8.13.0) with ESMTP id p5BDBBWQ007186; Sat, 11 Jun 2011 08:11:12 -0500 (CDT) Date: Sat, 11 Jun 2011 21:16:38 +0800 From: Shawn Guo To: Arnaud Patard Subject: Re: [PATCH 4/4] mmc: sdhci-esdhc-imx: extend card_detect and write_protect support Message-ID: <20110611131637.GB29093@S2100-06.ap.freescale.net> References: <1307702572-22066-1-git-send-email-shawn.guo@linaro.org> <1307702572-22066-5-git-send-email-shawn.guo@linaro.org> <87boy4lnp9.fsf@lebrac.rtp-net.org> <20110611115031.GA29093@S2100-06.ap.freescale.net> <877h8slgsl.fsf@lebrac.rtp-net.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <877h8slgsl.fsf@lebrac.rtp-net.org> User-Agent: Mutt/1.5.21 (2010-09-15) X-OriginatorOrg: freescale.com X-CRM114-Version: 20090807-BlameThorstenAndJenny ( TRE 0.7.6 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20110611_091222_212889_B482B0EF X-CRM114-Status: GOOD ( 37.46 ) X-Spam-Score: -0.7 (/) X-Spam-Report: SpamAssassin version 3.3.1 on canuck.infradead.org summary: Content analysis details: (-0.7 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/, low trust [213.199.154.204 listed in list.dnswl.org] Cc: Chris Ball , Eric Benard , patches@linaro.org, linux-mmc@vger.kernel.org, Wolfram Sang , kernel@pengutronix.de, Shawn Guo , linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter2.kernel.org [140.211.167.43]); Sat, 11 Jun 2011 13:12:54 +0000 (UTC) On Sat, Jun 11, 2011 at 01:59:54PM +0200, Arnaud Patard wrote: > Shawn Guo writes: > > > On Sat, Jun 11, 2011 at 11:30:42AM +0200, Arnaud Patard wrote: > >> Shawn Guo writes: > >> > >> Hi, > >> > >> > The patch extends card_detect and write_protect support to get mx5 > >> > family and more scenarios supported. The changes include: > >> > > >> > * Turn platform_data from optional to mandatory > >> > * Add cd_types and wp_types into platform_data to cover more use > >> > cases > >> > * Remove the use of flag ESDHC_FLAG_GPIO_FOR_CD > >> > * Adjust machine codes to adopt the platform_data changes > >> > >> Before I go and test theses patches, I'd like to get some > >> clarification. From what I see, you've modified all over the place the > >> code to provide a platform_data, setting wp/cd type to type "NONE", as > >> if it was the default you choose. Why this default and not considerer > >> the "SIGNAL" type being the default ? Is this choice the safest one when > >> one doesn't know what type to choose or can it have some bad side > >> effects ? > > > > The mx51_babbage is the only board support I'm concerned about in > > this patch. For other boards, I chose to translate the NULL pdata > > into "NONE" for both wp/cd types as the safest one, because I do not > > have (or care to check) the board schematics telling how wp/cd are > > routed on those boards. The patch ensures there is no regression > > for those boards, and let people who have schematics to set up wp/cd > > types later. > > ok. Thanks for making things clear. I see some changes for > loco/imx53qsb. Do you need testers for it too or you've tested it ? > I do not see changes for loco except NULL pdata -> "NONE" types. But testing are always welcomed and appreciated. > > > >> Also, why didn't you modify the imx*_add_sdhci_esdhc_imx() functions to > >> provide the default platform_data by themselves that if the 2nd argument > >> was NULL instead of modifying all theses machines files ? > >> > > As I said above, the wp/cd "NONE" types translated from NULL pdata > > will be set up properly later by people who have schematics. > > You're not answering my question about moving the NULL-> "NONE" type > from *all* modified machine file into the imx*add_sdhci_esdhc_imx(). If > it's the default, why all machines file have to be modified to set it ? > Moreover, *nothing* AFAICS is preventing to call theses functions will > NULL. What will happen ? An oops ? To me, a default is the value set > when nothing is set, and clearly modifying all functions call site due > to having to provide the default seems imho wrong. > Ah, good point. Please review changes below. If it looks good to you, I will incorporate it in the next version of the patch. diff --git a/arch/arm/plat-mxc/devices/platform-sdhci-esdhc-imx.c b/arch/arm/plat-mxc/devices/platform-sdhci-esdhc-imx.c index 6b2940b..a880f73 100644 --- a/arch/arm/plat-mxc/devices/platform-sdhci-esdhc-imx.c +++ b/arch/arm/plat-mxc/devices/platform-sdhci-esdhc-imx.c @@ -65,6 +65,11 @@ imx53_sdhci_esdhc_imx_data[] __initconst = { }; #endif /* ifdef CONFIG_SOC_IMX53 */ +static const struct esdhc_platform_data default_esdhc_pdata __initconst = { + .wp_type = ESDHC_WP_NONE, + .cd_type = ESDHC_CD_NONE, +}; + struct platform_device *__init imx_add_sdhci_esdhc_imx( const struct imx_sdhci_esdhc_imx_data *data, const struct esdhc_platform_data *pdata) @@ -81,6 +86,13 @@ struct platform_device *__init imx_add_sdhci_esdhc_imx( }, }; + /* + * If machine does not provide a pdata, use the default one + * which means none WP/CD support + */ + if (!pdata) + pdata = &default_esdhc_pdata; + return imx_add_platform_device("sdhci-esdhc-imx", data->id, res, ARRAY_SIZE(res), pdata, sizeof(*pdata)); }