Re: [PATCH] mmc_spi: use EILSEQ for possible transmission errors

[David Brownell <david-b@pacbell.net>]

On Wednesday 20 May 2009, Wolfgang Mües wrote:
> what about the following proposal which is not so aggressive?

Seems plausible to me, though note one comment at the end...


> The current mapping in mmc_spi.c is:
> 
> R1_SPI_PARAMETER => EINVAL
> R1_SPI_ADDRESS => EINVAL
> R1_SPI_ILLEGAL_COMMAND => EINVAL
> R1_SPI_COM_CRC => EILSEQ
> R1_SPI_ERASE_SEQ => EIO
> R1_SPI_ERASE_RESET => EIO

Yeah, that took a bit of head-scratching to come up with.


> R1_SPI_COM_CRC is a transmission error - no doubt. Mapping to EILSEQ is OK.
> 
> R1_SPI_ERASE_SEQ and R1_SPI_ERASE_RESET are responses to a sector erase 
> command. block.c is not sending such commands, so block.c can safely assume 
> that each such response is a transmission error.

Hmm, would hope that's not a long-term plan.  Remember that the
firmware in the card can leverage "that's erased" knowledge for
things like wear leveling.  SSDs and other storage devices would
likewise benefit from such knowledge.  I'm quite certain there's
been discussion about adding support for that in the block layer.


> R1_SPI_PARAMETER and R1_SPI_ADDRESS are errors from the card controller
> because a parameter or an address is out of range/invalid. It is mapped to 
> EINVAL, and so the caller can not distinguish between errors from the host 
> controller and driver or the card. This has to be changed to some other code,
> maybe EFAULT /* Bad address */.

OK.


> R1_SPI_ILLEGAL_COMMAND is mapped to EINVAL, and I suggest it to replace with
> ENOSYS /* Function not implemented */.
> 
> A quick scan in the mmc directory for EINVAL reveals that there are no callers 
> which need to be changed.
> 
> As block.c does not send a command, parameter or address which is invalid/out 
> of range,

That is, *today* it doesn't.  For the moment I suspect just logging
the actual fault code (a pr_debug) may will suffice; this is the kind
of stuff that shouldn't get buried too deeply.


> these errors must be considered transmission errors in the command  
> (if CRC is off) or in the response.

And the key issue is -- correct me if I'm wrong -- that returning
EINVAL makes trouble, so you need to change those three codes.  The
others don't much matter, right?