linux-kernel - Re: [PATCH] Fixes and enhancements for the MMC SPI driver

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] [thread-next>] [day] [month] [year] [list]

Message-Id: <200902091253.57594.wolfgang.mues@auerswald.de>
Date:	Mon, 9 Feb 2009 12:53:57 +0100
From:	Wolfgang Mües <wolfgang.mues@...rswald.de>
To:	Pierre Ossman <drzeus@...eus.cx>
Cc:	"Andrew Morton" <akpm@...ux-foundation.org>,
	"Matt Fleming" <matt@...sole-pimps.org>,
	"David Brownell" <dbrownell@...rs.sourceforge.net>,
	linux-kernel@...r.kernel.org,
	"Mike Frysinger" <vapier.adi@...il.com>
Subject: Re: [PATCH] Fixes and enhancements for the MMC SPI driver - revised

Pierre,

Am Sonntag, 8. Februar 2009 schrieb Pierre Ossman:
> Please split these things up into one patch for each item. It keeps the
> logs sensible and makes bisecting work better in case of problems.

OK. I will do.

> > +	/*
> > +	 * The MMC framework does a good job of computing timeouts
> > +	 * according to the mmc/sd standard. However, we found that in
> > +	 * SPI mode, there are many cards which need a longer timeout
> > +	 * of 1s after receiving a long stream of write data.
> > +	 */
> > +	tv = ktime_to_timeval(timeout);
> > +	if (tv.tv_sec == 0)
> > +		timeout = ktime_set(1, 0);
> >
> >  	if (host->mmc->use_spi_crc)
> >  		scratch->crc_val = cpu_to_be16(
>
> This might as well be handled when we compute the timeout in the core.
> Having things spread out is just confusing.

Hmmm. Inside the core, this timeout value will be calculated as:

timeout = 250ms;
if (driver == SPI)
   timeout = 1s;

I agree that it is confusing to have the timeout value set on two places.
Do you prefer to have it set in the mmc core, different for SPI and non-SPI?

> > -	if (host->mmc->use_spi_crc) {
> > +	/*
> > +	 * Omit the CRC check for CID and CSD reads. There are some SDHC

> Hmmm... I think this also better handled in the core. Reorder things in
> the init to not turn CRC checks on until after those registers have
> been read.

No! CRC checks are ONLY disabled for CID and CSD reads, NOT for the reading of 
data blocks. And I don't want to set use_spi_scc == 0 because I don't want to 
disable the safety of CRC for data blocks.

regards
 
i. A. Wolfgang Mües
-- 
Auerswald GmbH & Co. KG
Hardware Development
Telefon: +49 (0)5306 9219 0
Telefax: +49 (0)5306 9219 94 
E-Mail: Wolfgang.Mues@...rswald.de
Web: http://www.auerswald.de
 
--------------------------------------------------------------
Auerswald GmbH & Co. KG, Vor den Grashöfen 1, 38162 Cremlingen
Registriert beim AG Braunschweig HRA 13289
p.h.G Auerswald Geschäftsführungsges. mbH
Registriert beim AG Braunschweig HRB 7463
Geschäftsführer: Dipl-Ing. Gerhard Auerswald
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/