Received: by 2002:ac0:a5a6:0:0:0:0:0 with SMTP id m35-v6csp7187071imm;
        Tue, 28 Aug 2018 07:47:31 -0700 (PDT)
X-Google-Smtp-Source: ANB0VdY3qod0I9maPUpkB+2P3chUv57DBi3ZipuFSdW+TaCQXKVAv69aa5oJhR0CGUvZ31TdtCNn
X-Received: by 2002:a17:902:583:: with SMTP id f3-v6mr1896284plf.115.1535467650964;
        Tue, 28 Aug 2018 07:47:30 -0700 (PDT)
ARC-Seal: i=1; a=rsa-sha256; t=1535467650; cv=none;
        d=google.com; s=arc-20160816;
        b=CJR0Gt3rmwCRktmv6vhxRXARsxNS5kOUGEfgrtaX9LuaxrGMn/W4RJ5EE/4lxAW4dn
         C7qNrkNfB73mopmCRzOXieeOc8pkI3dUspMvqYDORu6xpm/P2LWgbDslmthICoSROr6V
         YtDBTnZkCczoIW0I63Sd+q0KV8VYSzSpieg7YV2Dth9GloL48nAcF2yhQri93pekgl0I
         E7bAd3jhIepPf5T4z8eBdFDkIjIS+FML5bJudeApuK5Gpb93BR/NZLb6LPrFSUv1PQrt
         3uCZ9SxRW39snm//KHLSX9iH5z/S3HmI6VUfvYAgmWeg3r+qLXTsV6K35N7iJZKK5tvx
         hMdQ==
ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816;
        h=list-id:precedence:sender:content-transfer-encoding:mime-version
         :references:in-reply-to:message-id:subject:cc:to:from:date
         :arc-authentication-results;
        bh=9Vrz9TgL15IEMceJFzMUoHhUmsiGd3lkjOQ/ajYV+TQ=;
        b=i53+wSxUOhd8c7d/oVYkTHTIz3jnRq8SH+qMQw98Urpnvru3fGPXTDmOYsEGGA02ko
         zg9cmzUhyxM8V1E7u+dSeVe0asu/ljdqeYPYMz42WwtVr+1Ug4zpJNT3cb9AgX/bHOm2
         viwc1C3g+6g9oail1TxOP4/LgOwUsCtFfyD4iJhzLT7gROvH53umMtTGdUjXn1F/mOgK
         uhl3aZoO0pEVC6egJ7hTj67EZ951y5M4CNrH9fbkYgStlSFIDUh5SS9AN/W5+S01DhQD
         uXrkqOvO2NhZgEQkz4sCA9LA9zasaZPChGFsAjGvl6W2g1dXh6EhRwTGsii6X12lXXpF
         uW5A==
ARC-Authentication-Results: i=1; mx.google.com;
       spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org;
       dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=nvidia.com
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67])
        by mx.google.com with ESMTP id w8-v6si1201282plk.222.2018.08.28.07.47.15;
        Tue, 28 Aug 2018 07:47:30 -0700 (PDT)
Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Authentication-Results: mx.google.com;
       spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org;
       dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=nvidia.com
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1728336AbeH1SiC (ORCPT <rfc822;lans.zhang2008@gmail.com>
        + 99 others); Tue, 28 Aug 2018 14:38:02 -0400
Received: from hqemgate16.nvidia.com ([216.228.121.65]:1370 "EHLO
        hqemgate16.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1727761AbeH1SiC (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Tue, 28 Aug 2018 14:38:02 -0400
Received: from hqpgpgate101.nvidia.com (Not Verified[216.228.121.13]) by hqemgate16.nvidia.com (using TLS: TLSv1.2, DES-CBC3-SHA)
        id <B5b85601e0000>; Tue, 28 Aug 2018 07:45:50 -0700
Received: from HQMAIL101.nvidia.com ([172.20.161.6])
  by hqpgpgate101.nvidia.com (PGP Universal service);
  Tue, 28 Aug 2018 07:46:00 -0700
X-PGP-Universal: processed;
        by hqpgpgate101.nvidia.com on Tue, 28 Aug 2018 07:46:00 -0700
Received: from dhcp-10-21-25-168 (10.21.25.201) by HQMAIL101.nvidia.com
 (172.20.187.10) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Tue, 28 Aug
 2018 14:45:57 +0000
Date:   Tue, 28 Aug 2018 17:45:51 +0300
From:   Aapo Vienamo <avienamo@nvidia.com>
To:     Adrian Hunter <adrian.hunter@intel.com>
CC:     Rob Herring <robh+dt@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Thierry Reding <thierry.reding@gmail.com>,
        Jonathan Hunter <jonathanh@nvidia.com>,
        Ulf Hansson <ulf.hansson@linaro.org>,
        "Mikko Perttunen" <mperttunen@nvidia.com>,
        Stefan Agner <stefan@agner.ch>, <devicetree@vger.kernel.org>,
        <linux-tegra@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <linux-mmc@vger.kernel.org>
Subject: Re: [PATCH v2 11/40] mmc: sdhci: Add a quirk to skip clearing the
 transfer mode register on tuning
Message-ID: <20180828174551.12f1d9bf@dhcp-10-21-25-168>
In-Reply-To: <d603be8f-986a-80e1-d98a-daea9c9684aa@intel.com>
References: <1533924522-1037-1-git-send-email-avienamo@nvidia.com>
        <1533924522-1037-12-git-send-email-avienamo@nvidia.com>
        <d603be8f-986a-80e1-d98a-daea9c9684aa@intel.com>
X-NVConfidentiality: public
MIME-Version: 1.0
Content-Type: text/plain; charset="US-ASCII"
Content-Transfer-Encoding: 7bit
X-Originating-IP: [10.21.25.201]
X-ClientProxiedBy: UKMAIL101.nvidia.com (10.26.138.13) To HQMAIL101.nvidia.com
 (172.20.187.10)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Mon, 27 Aug 2018 14:01:52 +0300
Adrian Hunter <adrian.hunter@intel.com> wrote:

> On 10/08/18 21:08, Aapo Vienamo wrote:
> > Add SDHCI_QUIRK2_TUNE_SKIP_XFERRMODE_REG_PROG to skip programming the
> > SDHCI_TRANSFER_MODE in sdhci_set_transfer_mode() if tuning command is
> > being sent.
> > 
> > On Tegra210 and Tegra186 the tuning sequence hangs if the SDHCI
> > transfer mode register is touched.
> > 
> > Signed-off-by: Aapo Vienamo <avienamo@nvidia.com>
> > ---
> >  drivers/mmc/host/sdhci.c | 6 ++++++
> >  drivers/mmc/host/sdhci.h | 2 ++
> >  2 files changed, 8 insertions(+)
> > 
> > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> > index a7b5602..04dc443 100644
> > --- a/drivers/mmc/host/sdhci.c
> > +++ b/drivers/mmc/host/sdhci.c
> > @@ -1028,6 +1028,12 @@ static void sdhci_set_transfer_mode(struct sdhci_host *host,
> >  
> >  	if (data == NULL) {
> >  		if (host->quirks2 &
> > +			SDHCI_QUIRK2_TUNE_SKIP_XFERMODE_REG_PROG &&
> > +				(cmd->opcode == MMC_SEND_TUNING_BLOCK ||
> > +				 cmd->opcode == MMC_SEND_TUNING_BLOCK_HS200)) {
> > +			return;
> > +		}  
> 
> Rather than introduce a quirk, can't we just dodge the unnecessary write e.g.
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 1b3fbd9bd5c5..68af6a67e397 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1033,10 +1033,19 @@ static void sdhci_set_transfer_mode(struct sdhci_host *host,
>  			if (cmd->opcode != MMC_SEND_TUNING_BLOCK_HS200)
>  				sdhci_writew(host, 0x0, SDHCI_TRANSFER_MODE);
>  		} else {
> -		/* clear Auto CMD settings for no data CMDs */
> +			u16 orig;
> +
> +			/* clear Auto CMD settings for no data CMDs */
>  			mode = sdhci_readw(host, SDHCI_TRANSFER_MODE);
> -			sdhci_writew(host, mode & ~(SDHCI_TRNS_AUTO_CMD12 |
> -				SDHCI_TRNS_AUTO_CMD23), SDHCI_TRANSFER_MODE);
> +			orig = mode;
> +			mode &= ~(SDHCI_TRNS_AUTO_CMD12 | SDHCI_TRNS_AUTO_CMD23);
> +			/*
> +			 * Do not write transfer mode unnecessarily because it
> +			 * can upset some host controllers (e.g. sdhci-tregra)
> +			 * during tuning.
> +			 */
> +			if (mode != orig)
> +				sdhci_writew(host, new_mode, SDHCI_TRANSFER_MODE);
>  		}
>  		return;
>  	}
> 
> 
> Alternatively 
> 
> > +		if (host->quirks2 &
> >  			SDHCI_QUIRK2_CLEAR_TRANSFERMODE_REG_BEFORE_CMD) {
> >  			sdhci_writew(host, 0x0, SDHCI_TRANSFER_MODE);
> >  		} else {
> > diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> > index 23966f8..0a99008 100644
> > --- a/drivers/mmc/host/sdhci.h
> > +++ b/drivers/mmc/host/sdhci.h
> > @@ -450,6 +450,8 @@ struct sdhci_host {
> >   * obtainable timeout.
> >   */
> >  #define SDHCI_QUIRK2_DISABLE_HW_TIMEOUT			(1<<17)
> > +/* Don't clear the SDHCI_TRANSFER_MODE register on tuning commands */
> > +#define SDHCI_QUIRK2_TUNE_SKIP_XFERMODE_REG_PROG	(1<<18)
> >  
> >  	int irq;		/* Device IRQ */
> >  	void __iomem *ioaddr;	/* Mapped address */
> >   
> 

Looks like writes to the transfer mode register are deferred until the
command register is written by tegra_sdhci_writew(). This code was
introduced due to a quirk in Tegra114/124. The Tegra114 ops struct was
probably just copied over when support for Tegra210 was added, so we
end up with this code also getting run on Tegra210.

The issue with tegra_sdhci_writew() is that it breaks successive
read-modify-write operations. The value written to the transfer mode
register isn't returned on read until the command register is also
touched. Removing the write_w accessor from tegra210_sdhci_ops seems to
make the issue originally addressed by this patch to go away.

 -Aapo