Hi Simon,
nice you got it working with upstream! Thanks. It still looks a bit too
much like BSP code, though, so we need to clean it up. I found 'git log
--grep=tuning drivers/mmc/host' to be useful to get an idea of current
best practices.
> + bool (*inquiry_tuning)(struct tmio_mmc_host *host);
Do we really need this? I'd think the core will check that tuning only
gets called when needed.
> -static void tmio_mmc_data_irq(struct tmio_mmc_host *host)
> +static void tmio_mmc_data_irq(struct tmio_mmc_host *host, unsigned int stat)
> {
> struct mmc_data *data;
> spin_lock(&host->lock);
> @@ -529,6 +558,9 @@ static void tmio_mmc_data_irq(struct tmio_mmc_host *host)
> if (!data)
> goto out;
>
> + if (stat & TMIO_STAT_CRCFAIL || stat & TMIO_STAT_STOPBIT_ERR ||
> + stat & TMIO_STAT_TXUNDERRUN)
> + data->error = -EILSEQ;
> if (host->chan_tx && (data->flags & MMC_DATA_WRITE) && !host->force_pio) {
> u32 status = sd_ctrl_read16_and_16_as_32(host, CTL_STATUS);
> bool done = false;
> @@ -577,8 +609,6 @@ static void tmio_mmc_cmd_irq(struct tmio_mmc_host *host,
> goto out;
> }
>
> - host->cmd = NULL;
> -
> /* This controller is sicker than the PXA one. Not only do we need to
> * drop the top 8 bits of the first response word, we also need to
> * modify the order of the response for short response command types.
> @@ -598,14 +628,16 @@ static void tmio_mmc_cmd_irq(struct tmio_mmc_host *host,
>
> if (stat & TMIO_STAT_CMDTIMEOUT)
> cmd->error = -ETIMEDOUT;
> - else if (stat & TMIO_STAT_CRCFAIL && cmd->flags & MMC_RSP_CRC)
> + else if ((stat & TMIO_STAT_CRCFAIL && cmd->flags & MMC_RSP_CRC) ||
> + stat & TMIO_STAT_STOPBIT_ERR ||
> + stat & TMIO_STAT_CMD_IDX_ERR)
> cmd->error = -EILSEQ;
>
> /* If there is data to handle we enable data IRQs here, and
> * we will ultimatley finish the request in the data_end handler.
> * If theres no data or we encountered an error, finish now.
> */
> - if (host->data && !cmd->error) {
> + if (host->data && (!cmd->error || cmd->error == -EILSEQ)) {
> if (host->data->flags & MMC_DATA_READ) {
> if (host->force_pio || !host->chan_rx)
> tmio_mmc_enable_mmc_irqs(host, TMIO_MASK_READOP);
> @@ -666,7 +698,7 @@ static bool __tmio_mmc_sdcard_irq(struct tmio_mmc_host *host,
> /* Data transfer completion */
> if (ireg & TMIO_STAT_DATAEND) {
> tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_DATAEND);
> - tmio_mmc_data_irq(host);
> + tmio_mmc_data_irq(host, status);
> return true;
> }
I wonder if the changes to this tmio_mmc_*_irq are a seperate patch?
I'd think they need a seperate description.
>
> @@ -753,6 +785,157 @@ static int tmio_mmc_start_data(struct tmio_mmc_host *host,
> return 0;
> }
>
> +#define TMIO_MMC_MAX_TUNING_LOOP 40
> +
> +static int tmio_mmc_execute_tuning(struct mmc_host *mmc, u32 opcode)
> +{
I'd think we can use mmc_send_tuning() from the mmc core here in here.
> + struct tmio_mmc_host *host = mmc_priv(mmc);
> + struct mmc_ios *ios = &mmc->ios;
> +
> + unsigned long timeout, val;
> + unsigned long *tap;
> + int tuning_loop_counter = TMIO_MMC_MAX_TUNING_LOOP;
> + int ret, timeleft;
> +
> + struct mmc_request mrq = {NULL};
> + struct mmc_command cmd = {0};
> + struct mmc_data data = {0};
> + struct scatterlist sg;
> + u8 *data_buf;
> + unsigned int num, tm = CMDREQ_TIMEOUT;
> + unsigned long flags;
> +
> + if ((ios->timing != MMC_TIMING_UHS_SDR50 &&
> + ios->timing != MMC_TIMING_UHS_SDR104) ||
> + (host->inquiry_tuning && !host->inquiry_tuning(host)) ||
> + host->done_tuning || !host->init_tuning)
> + return 0;
> +
> + num = host->init_tuning(host);
> +
> + tap = kmalloc(num * 2, GFP_KERNEL);
> + if (tap == NULL) {
> + ret = -ENOMEM;
> + goto err_tap;
> + }
> +
> + data_buf = kmalloc(64, GFP_KERNEL);
> + if (data_buf == NULL) {
> + ret = -ENOMEM;
> + goto err_data;
> + }
> +
> + val = 0;
> +
> + /*
> + * Issue CMD19 repeatedly till Execute Tuning is set to 0 or the number
> + * of loops reaches 40 times or a timeout of 150ms occurs.
> + */
Note to self: this is copied from sdhci.c
So far for now!
Thanks,
Wolfram
Attachment:
signature.asc
Description: PGP signature
