From patchwork Sun Jan 8 20:37:37 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Chris Ball X-Patchwork-Id: 6095 Return-Path: X-Original-To: patchwork@peony.canonical.com Delivered-To: patchwork@peony.canonical.com Received: from fiordland.canonical.com (fiordland.canonical.com [91.189.94.145]) by peony.canonical.com (Postfix) with ESMTP id 885DB23E01 for ; Sun, 8 Jan 2012 20:37:48 +0000 (UTC) Received: from mail-bk0-f52.google.com (mail-bk0-f52.google.com [209.85.214.52]) by fiordland.canonical.com (Postfix) with ESMTP id 6BCD4A1807D for ; Sun, 8 Jan 2012 20:37:48 +0000 (UTC) Received: by bke17 with SMTP id 17so1564143bke.11 for ; Sun, 08 Jan 2012 12:37:48 -0800 (PST) Received: by 10.204.152.27 with SMTP id e27mr6249902bkw.45.1326055066502; Sun, 08 Jan 2012 12:37:46 -0800 (PST) X-Forwarded-To: linaro-patchwork@canonical.com X-Forwarded-For: patch@linaro.org linaro-patchwork@canonical.com Delivered-To: patches@linaro.org Received: by 10.205.82.144 with SMTP id ac16cs29038bkc; Sun, 8 Jan 2012 12:37:46 -0800 (PST) Received: by 10.180.106.202 with SMTP id gw10mr621056wib.3.1326055064666; Sun, 08 Jan 2012 12:37:44 -0800 (PST) Received: from void.printf.net (void.printf.net. [89.145.121.20]) by mx.google.com with ESMTPS id y84si16380545weq.90.2012.01.08.12.37.44 (version=TLSv1/SSLv3 cipher=OTHER); Sun, 08 Jan 2012 12:37:44 -0800 (PST) Received-SPF: neutral (google.com: 89.145.121.20 is neither permitted nor denied by best guess record for domain of cjb@laptop.org) client-ip=89.145.121.20; Authentication-Results: mx.google.com; spf=neutral (google.com: 89.145.121.20 is neither permitted nor denied by best guess record for domain of cjb@laptop.org) smtp.mail=cjb@laptop.org Received: from 173-166-109-241-newengland.hfc.comcastbusiness.net ([173.166.109.241] helo=bob.laptop.org) by void.printf.net with esmtp (Exim 4.69) (envelope-from ) id 1RjzUt-00017Y-QA; Sun, 08 Jan 2012 20:37:44 +0000 From: Chris Ball To: Girish K S Cc: linux-mmc@vger.kernel.org, patches@linaro.org, linux-samsung-soc@vger.kernel.org Subject: Re: [PATCH V9 2/2] mmc: host: Adds support for eMMC 4.5 HS200 mode References: <1325823999-21770-1-git-send-email-girish.shivananjappa@linaro.org> <1325823999-21770-3-git-send-email-girish.shivananjappa@linaro.org> Date: Sun, 08 Jan 2012 15:37:37 -0500 In-Reply-To: <1325823999-21770-3-git-send-email-girish.shivananjappa@linaro.org> (Girish K. S.'s message of "Fri, 6 Jan 2012 09:56:39 +0530") Message-ID: User-Agent: Gnus/5.110018 (No Gnus v0.18) Emacs/23.2 (gnu/linux) MIME-Version: 1.0 Hi, On Thu, Jan 05 2012, Girish K S wrote: > @@ -1703,10 +1707,13 @@ static int sdhci_execute_tuning(struct mmc_host *mmc) > * Host Controller needs tuning only in case of SDR104 mode > * and for SDR50 mode when Use Tuning for SDR50 is set in > * Capabilities register. > + * If the Host Controller supports the HS200 mode then tuning > + * function has to be executed. > */ > if (((ctrl & SDHCI_CTRL_UHS_MASK) == SDHCI_CTRL_UHS_SDR104) || > (((ctrl & SDHCI_CTRL_UHS_MASK) == SDHCI_CTRL_UHS_SDR50) && > - (host->flags & SDHCI_SDR50_NEEDS_TUNING))) > + (host->flags & SDHCI_SDR50_NEEDS_TUNING)) || > + (host->flags & SDHCI_HS200_NEEDS_TUNING)) > ctrl |= SDHCI_CTRL_EXEC_TUNING; > else { > spin_unlock(&host->lock); Actually, this generates a warning: /home/cjb/git/mmc/drivers/mmc/host/sdhci.c: In function ‘sdhci_execute_tuning’: /home/cjb/git/mmc/drivers/mmc/host/sdhci.c:1716:7: warning: suggest parentheses around ‘&&’ within ‘||’ [-Wparentheses] .. as well as being hard to follow. Shall we split it up as below instead? diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 64febf2..5627354 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -1694,6 +1694,7 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode) int tuning_loop_counter = MAX_TUNING_LOOP; unsigned long timeout; int err = 0; + bool requires_tuning_nonuhs; host = mmc_priv(mmc); @@ -1704,16 +1705,19 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode) ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2); /* - * Host Controller needs tuning only in case of SDR104 mode - * and for SDR50 mode when Use Tuning for SDR50 is set in + * The Host Controller needs tuning only in case of SDR104 mode + * and for SDR50 mode when Use Tuning for SDR50 is set in the * Capabilities register. - * If the Host Controller supports the HS200 mode then tuning - * function has to be executed. + * If the Host Controller supports the HS200 mode then the + * tuning function has to be executed. */ - if ((ctrl & SDHCI_CTRL_UHS_MASK == SDHCI_CTRL_UHS_SDR104) || - ((ctrl & SDHCI_CTRL_UHS_MASK == SDHCI_CTRL_UHS_SDR50) && - host->flags & SDHCI_SDR50_NEEDS_TUNING || + if (((ctrl & SDHCI_CTRL_UHS_MASK) == SDHCI_CTRL_UHS_SDR50) && + (host->flags & SDHCI_SDR50_NEEDS_TUNING || host->flags & SDHCI_HS200_NEEDS_TUNING)) + requires_tuning_nonuhs = true; + + if (((ctrl & SDHCI_CTRL_UHS_MASK) == SDHCI_CTRL_UHS_SDR104) || + requires_tuning_nonuhs) ctrl |= SDHCI_CTRL_EXEC_TUNING; else { spin_unlock(&host->lock);