[PATCH] mmc: Drop unused mmc_send_tuning() cmd_error parameter

[Marek Vasut]

On 2/20/24 11:50, Paul Barker wrote:
> On 20/02/2024 08:36, Marek Vasut wrote:
>> The cmd_error parameter is not used, remove it.
>>   [snip]
>>
>> diff --git a/drivers/mmc/mtk-sd.c b/drivers/mmc/mtk-sd.c
>> index 5a0c61daed5..296aaee7331 100644
>> --- a/drivers/mmc/mtk-sd.c
>> +++ b/drivers/mmc/mtk-sd.c
>> @@ -1131,7 +1131,7 @@ static int hs400_tune_response(struct udevice *dev, u32 opcode)
>>   				i << PAD_CMD_TUNE_RX_DLY3_S);
>>   
>>   		for (j = 0; j < 3; j++) {
>> -			mmc_send_tuning(mmc, opcode, &cmd_err);
>> +			cmd_err = mmc_send_tuning(mmc, opcode);
>>   			if (!cmd_err) {
>>   				cmd_delay |= (1 << i);
>>   			} else {
>> @@ -1181,7 +1181,7 @@ static int msdc_tune_response(struct udevice *dev, u32 opcode)
>>   				i << MSDC_PAD_TUNE_CMDRDLY_S);
>>   
>>   		for (j = 0; j < 3; j++) {
>> -			mmc_send_tuning(mmc, opcode, &cmd_err);
>> +			cmd_err = mmc_send_tuning(mmc, opcode);
>>   			if (!cmd_err) {
>>   				rise_delay |= (1 << i);
>>   			} else {
>> @@ -1203,7 +1203,7 @@ static int msdc_tune_response(struct udevice *dev, u32 opcode)
>>   				i << MSDC_PAD_TUNE_CMDRDLY_S);
>>   
>>   		for (j = 0; j < 3; j++) {
>> -			mmc_send_tuning(mmc, opcode, &cmd_err);
>> +			cmd_err = mmc_send_tuning(mmc, opcode);
>>   			if (!cmd_err) {
>>   				fall_delay |= (1 << i);
>>   			} else {
>> @@ -1238,7 +1238,7 @@ skip_fall:
>>   		clrsetbits_le32(tune_reg, MSDC_PAD_TUNE_CMDRRDLY_M,
>>   				i << MSDC_PAD_TUNE_CMDRRDLY_S);
>>   
>> -		mmc_send_tuning(mmc, opcode, &cmd_err);
>> +		cmd_err = mmc_send_tuning(mmc, opcode);
>>   		if (!cmd_err)
>>   			internal_delay |= (1 << i);
>>   	}
>> @@ -1264,7 +1264,6 @@ static int msdc_tune_data(struct udevice *dev, u32 opcode)
>>   	struct msdc_delay_phase final_rise_delay, final_fall_delay = { 0, };
>>   	u8 final_delay, final_maxlen;
>>   	void __iomem *tune_reg = &host->base->pad_tune;
>> -	int cmd_err;
>>   	int i, ret;
>>   
>>   	if (host->dev_comp->pad_tune0)
>> @@ -1277,10 +1276,10 @@ static int msdc_tune_data(struct udevice *dev, u32 opcode)
>>   		clrsetbits_le32(tune_reg, MSDC_PAD_TUNE_DATRRDLY_M,
>>   				i << MSDC_PAD_TUNE_DATRRDLY_S);
>>   
>> -		ret = mmc_send_tuning(mmc, opcode, &cmd_err);
>> +		ret = mmc_send_tuning(mmc, opcode);
>>   		if (!ret) {
>>   			rise_delay |= (1 << i);
>> -		} else if (cmd_err) {
>> +		} else {
>>   			/* in this case, retune response is needed */
>>   			ret = msdc_tune_response(dev, opcode);
>>   			if (ret)
>> @@ -1300,10 +1299,10 @@ static int msdc_tune_data(struct udevice *dev, u32 opcode)
>>   		clrsetbits_le32(tune_reg, MSDC_PAD_TUNE_DATRRDLY_M,
>>   				i << MSDC_PAD_TUNE_DATRRDLY_S);
>>   
>> -		ret = mmc_send_tuning(mmc, opcode, &cmd_err);
>> +		ret = mmc_send_tuning(mmc, opcode);
>>   		if (!ret) {
>>   			fall_delay |= (1 << i);
>> -		} else if (cmd_err) {
>> +		} else {
>>   			/* in this case, retune response is needed */
>>   			ret = msdc_tune_response(dev, opcode);
>>   			if (ret)
> 
> This driver (mtk-sd.c) seems to be the only one that really uses the
> `cmd_error` parameter.
> 
> Looking at the implementation of mmc_send_tuning() in Linux, this
> parameter is used so that a caller can differentiate between a command
> error and a data error. I don't know enough details about MMC to
> understand the distinction, but I assume there is some reason for this.
> So I wonder if the mtk-sd driver will still work if those error paths
> are taken for data errors and not just command errors. Has this change
> been tested on some board which uses this driver?

Not by me, so far this driver used uninitialized error value and assumed 
it was initialized as far as I can tell, so it is likely already broken.