public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Pierre Ossman <pierre@ossman.eu>
To: "Wolfgang Mües" <wolfgang.mues@auerswald.de>
Cc: "David Brownell" <david-b@pacbell.net>,
	"Matt Fleming" <matt@console-pimps.org>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Mike Frysinger" <vapier.adi@gmail.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mmc_spi: use EILSEQ for possible transmission errors
Date: Tue, 9 Jun 2009 20:07:50 +0200	[thread overview]
Message-ID: <20090609200750.2fa38dfe@mjolnir.ossman.eu> (raw)
In-Reply-To: <200905251659.17396.wolfgang.mues@auerswald.de>

[-- Attachment #1: Type: text/plain, Size: 2591 bytes --]

On Mon, 25 May 2009 16:59:17 +0200
Wolfgang Mües <wolfgang.mues@auerswald.de> wrote:

> Pierre,
> 
> Am Montag, 25. Mai 2009 schrieb Pierre Ossman:
> > I don't remember why
> > mmc_spi is looking at responses to begin with.
> 
> Because responses are very different.
> 
> First of all, responses in SD mode and SPI mode are different.
> 

MMC and SD aren't identical either, but we don't delegate that to the
host drivers.

> Second, responses in SD mode are CRC-7 protected, so that it is easy to check 
> for transmission errors.
> 
> SPI mode responses (R1...) are without any CRC.
> 

This however is more relevant. But OTOH, I don't think mmc_spi has a
better idea if the status can be trusted or not than a higher layer.

> Third, if you have SD mode, you use a hardware host controller which manages 
> all command/data/response actions, and what you typically get is the content 
> of the status register of the host controller (which is different from the
> error code of the SD card).
> 
> So it is very natural that every host controller driver mapps the responses 
> from the host controller to a common representation (== error code). The 
> error codes with special meanings for MMC/SD are summed up in mmc/core.h .
> 

But the other drivers don't attempt to interpret the response from the
card. They just give the response back up and let the next layer deal
with it.

> The SPI host driver is a recent addition to the linux kernel, and the fact 
> that SPI responses are not CRC-protected was not accounted for in the code.
> 
> As it makes no sense to code the retry logic in every driver, the prefered way 
> is to code the retry logic in block.c. The non-SPI-mode-drivers will profit 
> from this logic, because they are getting retries for EILSEQ (CRC error) and
> ETIMEDOUT (card has not understand the command).

Indeed. But I don't like the fact that we're hiding interpretation of
responses down in the driver. It should be up to the layer issuing the
MMC request how the response should be interpreted.

But that's an issue that can be left for another day...

> 
> The biggest problem in the error codes is that EINVAL is used by the SPI 
> driver both for recoverable and for non-recoverable errors, which has to be 
> splitted IMHO.
> 

Indeed. I'll queue up the patch.

Rgds
-- 
     -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

  reply	other threads:[~2009-06-09 18:08 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-05-14 11:24 [PATCH] mmc_spi: use EILSEQ for possible transmission errors Wolfgang Mües
2009-05-19 11:29 ` Matt Fleming
2009-05-19 11:47   ` Wolfgang Mües
2009-05-20  7:53     ` Matt Fleming
2009-05-20  4:49   ` David Brownell
2009-05-20  8:35     ` Wolfgang Mües
2009-05-20  9:20       ` David Brownell
2009-05-20 10:08         ` Pierre Ossman
2009-05-21  2:02           ` David Brownell
2009-05-25  9:04             ` Wolfgang Mües
2009-05-25  9:43               ` David Brownell
2009-05-25 10:18                 ` Wolfgang Mües
2009-05-25 11:50                   ` Pierre Ossman
2009-05-25 14:59                     ` Wolfgang Mües
2009-06-09 18:07                       ` Pierre Ossman [this message]
2009-06-10  7:29                         ` Wolfgang Mües
2009-06-10  7:37                           ` Matt Fleming
2009-06-13 10:57                           ` Pierre Ossman
2009-05-25 11:48             ` Pierre Ossman
2009-05-20 10:31         ` Wolfgang Mües

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090609200750.2fa38dfe@mjolnir.ossman.eu \
    --to=pierre@ossman.eu \
    --cc=akpm@linux-foundation.org \
    --cc=david-b@pacbell.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matt@console-pimps.org \
    --cc=vapier.adi@gmail.com \
    --cc=wolfgang.mues@auerswald.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox