Re: [PATCH 2.6.19] mmc: Add support for SDHC cards (Take 2)

[Philip Langdale]

Andrew Morton wrote:
> On Mon, 01 Jan 2007 07:29:55 -0800
> Philip Langdale <philipl@xxxxxxxxx> wrote:
> 
>>  #define MMC_RSP_R1B	(MMC_RSP_PRESENT|MMC_RSP_CRC|MMC_RSP_OPCODE|MMC_RSP_BUSY)
>>  #define MMC_RSP_R2	(MMC_RSP_PRESENT|MMC_RSP_136|MMC_RSP_CRC)
>>  #define MMC_RSP_R3	(MMC_RSP_PRESENT)
>> -#define MMC_RSP_R6	(MMC_RSP_PRESENT|MMC_RSP_CRC)
>> +#define MMC_RSP_R6	(MMC_RSP_PRESENT|MMC_RSP_CRC|MMC_RSP_OPCODE)
>> +#define MMC_RSP_R7	(MMC_RSP_PRESENT|MMC_RSP_CRC|MMC_RSP_OPCODE)
> 
> This gives MMC_RSP_R1 and MMC_RSP_R6 the same value, so
> 
> drivers/mmc/tifm_sd.c: In function 'tifm_sd_op_flags':
> drivers/mmc/tifm_sd.c:190: error: duplicate case value
> drivers/mmc/tifm_sd.c:181: error: previously used here

This is a bug. The MMC_RSP_R? #defines do not fully characterise the
responses (specically, the way that the response is parsed is not
characterised) and consequently there is no guarantee of uniqueness.
Given this reality - the way that the tifm_sd driver works is unsafe.

If R6 had not been incorrectly defined (the missing RSP_OPCODE should
always have been there), then this code would not have worked. As things
currently stand, it is necessary to also check the command number to
decide on the correct response type - that's suboptimal and it's probably
good to uniquely identify the response in the mmc_command in some other
fashion.

I'm going to remove the R6 fix from my next diff to keep these things
distinct but this needs to be resolved.

--phil
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/