Messages in this thread | | | From | David Brownell <> | Subject | Re: [PATCH] mmc_spi: use EILSEQ for possible transmission errors | Date | Wed, 20 May 2009 02:20:46 -0700 |
| |
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?
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |