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

[Paul Barker]

On 20/02/2024 11:26, Marek Vasut wrote:
> 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.

+To: Ryder Lee, Weijie Gao, Chunfeng Yun
+Cc: GSS_MTK_Uboot_upstream at mediatek.com

Do you have any input as ARM MEDIATEK maintainers?

-- 
Paul Barker
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_0x27F4B3459F002257.asc
Type: application/pgp-keys
Size: 3520 bytes
Desc: OpenPGP public key
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20240220/aa8c3a8f/attachment.key>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature.asc
Type: application/pgp-signature
Size: 236 bytes
Desc: OpenPGP digital signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20240220/aa8c3a8f/attachment.sig>