[PATCH] mci: fix wrong sd/mmc/emmc card size computation for arch where char is signed

[Yann Sionneau]

Woops, forgot about the Signed-off-by. Sorry!

On 9/14/20 5:51 PM, Ahmad Fatoum wrote:
> Hello Yann,
>
> On 9/14/20 5:27 PM, Yann Sionneau wrote:
>> char type can be either signed or unsigned according to C standard.
>> If your arch has signed char, this kind of computation will end up wrong
>> because of sign extension:
>>
>> https://git.pengutronix.de/cgit/barebox/tree/drivers/mci/mci-core.c#n869
>>
>> mci->capacity = mci->ext_csd[EXT_CSD_SEC_COUNT] << 0 |
>> 		mci->ext_csd[EXT_CSD_SEC_COUNT + 1] << 8 |
>> 		mci->ext_csd[EXT_CSD_SEC_COUNT + 2] << 16 |
>> 		mci->ext_csd[EXT_CSD_SEC_COUNT + 3] << 24;
> Good catch! I wondered why this wasn't noticed before,
> so I looked at the OSELAS.Toolchain`s we have on our build
> server. Only x86 and MIPS have signed chars by default.
>
> On x86, we usually do EFI, so barebox mci-core is not involved
> and MIPS apparently doesn't have any MCI drivers yet.
>
> (Of course, you can compile do -fsigned-char or compile
>   your own toolchain with a different default, but I guess
>   not many do that)
>
> Thanks for fixing it.
>
>> Turning the ext_csd field into u8 * fixes the issue.
> Your Signed-off-by is required.
> Please check https://developercertificate.org/
>
>> ---
>>   include/mci.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/mci.h b/include/mci.h
>> index 96547fb39..f6d845440 100644
>> --- a/include/mci.h
>> +++ b/include/mci.h
>> @@ -456,7 +456,7 @@ struct mci {
>>   	uint64_t capacity;	/**< Card's data capacity in bytes */
>>   	int ready_for_use;	/** true if already probed */
>>   	int dsr_imp;		/**< DSR implementation state from CSD */
>> -	char *ext_csd;
>> +	u8 *ext_csd;
>>   	int probe;
>>   	struct param_d *param_boot;
>>   	int bootpart;
>>