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);@@ -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);
} 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);
} 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.