linux-kernel - Re: [PATCH v2 1/6] mmc: sunxi: Always set signal delay to 0 for A64

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20170111211758.wt24lx5b63rgujia@lukather>
Date:   Wed, 11 Jan 2017 22:17:58 +0100
From:   Maxime Ripard <maxime.ripard@...e-electrons.com>
To:     André Przywara <andre.przywara@....com>
Cc:     Chen-Yu Tsai <wens@...e.org>, Ulf Hansson <ulf.hansson@...aro.org>,
        Rob Herring <robh+dt@...nel.org>, devicetree@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linux-mmc@...r.kernel.org
Subject: Re: [PATCH v2 1/6] mmc: sunxi: Always set signal delay to 0 for A64

On Tue, Jan 10, 2017 at 12:30:41AM +0000, André Przywara wrote:
> On 09/01/17 16:46, Maxime Ripard wrote:
> > Experience have shown that the using the  autocalibration could severely
> > degrade the performances of the MMC bus.
> > 
> > Allwinner is using in its BSP a delay set to 0 for all the modes but HS400.
> > Remove the calibration code for now, and add comments to document our
> > findings.
> > 
> > Signed-off-by: Maxime Ripard <maxime.ripard@...e-electrons.com>
> > ---
> >  drivers/mmc/host/sunxi-mmc.c | 50 ++++++++++++-------------------------
> >  1 file changed, 17 insertions(+), 33 deletions(-)
> > 
> > diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
> > index b1d1303389a7..ea9552a0d820 100644
> > --- a/drivers/mmc/host/sunxi-mmc.c
> > +++ b/drivers/mmc/host/sunxi-mmc.c
> > @@ -683,41 +683,19 @@ static int sunxi_mmc_oclk_onoff(struct sunxi_mmc_host *host, u32 oclk_en)
> >  
> >  static int sunxi_mmc_calibrate(struct sunxi_mmc_host *host, int reg_off)
> >  {
> > -	u32 reg = readl(host->reg_base + reg_off);
> > -	u32 delay;
> > -	unsigned long timeout;
> > -
> >  	if (!host->cfg->can_calibrate)
> >  		return 0;
> >  
> > -	reg &= ~(SDXC_CAL_DL_MASK << SDXC_CAL_DL_SW_SHIFT);
> > -	reg &= ~SDXC_CAL_DL_SW_EN;
> > -
> > -	writel(reg | SDXC_CAL_START, host->reg_base + reg_off);
> > -
> > -	dev_dbg(mmc_dev(host->mmc), "calibration started\n");
> > -
> > -	timeout = jiffies + HZ * SDXC_CAL_TIMEOUT;
> > -
> > -	while (!((reg = readl(host->reg_base + reg_off)) & SDXC_CAL_DONE)) {
> > -		if (time_before(jiffies, timeout))
> > -			cpu_relax();
> > -		else {
> > -			reg &= ~SDXC_CAL_START;
> > -			writel(reg, host->reg_base + reg_off);
> > -
> > -			return -ETIMEDOUT;
> > -		}
> > -	}
> > -
> > -	delay = (reg >> SDXC_CAL_DL_SHIFT) & SDXC_CAL_DL_MASK;
> > -
> > -	reg &= ~SDXC_CAL_START;
> > -	reg |= (delay << SDXC_CAL_DL_SW_SHIFT) | SDXC_CAL_DL_SW_EN;
> > -
> > -	writel(reg, host->reg_base + reg_off);
> > -
> > -	dev_dbg(mmc_dev(host->mmc), "calibration ended, reg is 0x%x\n", reg);
> > +	/*
> > +	 * FIXME:
> > +	 * This is not clear how the calibration is supposed to work
> > +	 * yet. The best rate have been obtained by simply setting the
> > +	 * delay to 0, as Allwinner does in its BSP.
> > +	 *
> > +	 * The only mode that doesn't have such a delay is HS400, that
> > +	 * is in itself a TODO.
> > +	 */
> > +	writel(SDXC_CAL_DL_SW_EN, host->reg_base + reg_off);
> >  
> >  	return 0;
> >  }
> > @@ -806,7 +784,13 @@ static int sunxi_mmc_clk_set_rate(struct sunxi_mmc_host *host,
> >  	if (ret)
> >  		return ret;
> >  
> > -	/* TODO: enable calibrate on sdc2 SDXC_REG_DS_DL_REG of A64 */
> > +	/*
> > +	 * FIXME:
> > +	 *
> > +	 * In HS400 we'll also need to calibrate the data strobe
> > +	 * signal. This should only happen on the MMC2 controller (at
> > +	 * least on the A64 and older SoCs).
> 
> Which older SoCs have this calibration register and a DS signal?
> Is that supposed to mean "other" SoCs?

That was supposed to mean that newer (than A64) SoCs might have that
calibration on other controllers than MMC2. But you're right that it
actually applies only to A64 anyway, I'll remove the and older part.

> Other than that:
> 
> Reviewed-by: Andre Przywara <andre.przywara@....com>

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Download attachment "signature.asc" of type "application/pgp-signature" (802 bytes)