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: 1824 Return-Path: Delivered-To: unknown Received: from imap.gmail.com (74.125.47.109) by localhost6.localdomain6 with IMAP4-SSL; 14 Jun 2011 16:45:41 -0000 Delivered-To: patches@linaro.org Received: by 10.52.181.10 with SMTP id ds10cs367268vdc; Sat, 11 Jun 2011 06:12:20 -0700 (PDT) Received: by 10.14.47.132 with SMTP id t4mr1433608eeb.66.1307797939326; Sat, 11 Jun 2011 06:12:19 -0700 (PDT) Received: from AM1EHSOBE001.bigfish.com (am1ehsobe001.messaging.microsoft.com [213.199.154.204]) by mx.google.com with ESMTPS id q2si10505661eeh.97.2011.06.11.06.12.17 (version=TLSv1/SSLv3 cipher=OTHER); Sat, 11 Jun 2011 06:12:18 -0700 (PDT) Received-SPF: neutral (google.com: 213.199.154.204 is neither permitted nor denied by best guess record for domain of r65073@freescale.com) client-ip=213.199.154.204; Authentication-Results: mx.google.com; spf=neutral (google.com: 213.199.154.204 is neither permitted nor denied by best guess record for domain of r65073@freescale.com) smtp.mail=r65073@freescale.com 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 CC: Shawn Guo , , Chris Ball , Wolfram Sang , Eric Benard , , , 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 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)); }