[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <535E3D8D.3000309@realsil.com.cn>
Date: Mon, 28 Apr 2014 19:37:49 +0800
From: micky <micky_ching@...lsil.com.cn>
To: <chris@...ntf.net>, <sameo@...ux.intel.com>
CC: <gregkh@...uxfoundation.org>, <dan.carpenter@...cle.com>,
<ulf.hansson@...aro.org>, <devel@...uxdriverproject.org>,
<linux-kernel@...r.kernel.org>, <linux-mmc@...r.kernel.org>,
<rogerable@...ltek.com>, <wei_wang@...lsil.com.cn>
Subject: Re: [PATCH v2 2/2] mmc: rtsx: modify error handle and remove smatchwarnings
Hi Chris and Ulf,
This patch is denpend on commit c42deffd5b53c9e583d83c7964854ede2f12410d.
<mmc: rtsx: add support for pre_req and post_req>, and we discard the
previous commit.
we want discard this patch also(it it now in linux-next), need I send
another patch to
revert or it will be discard automatically?
Best Regards.
micky.
On 03/27/2014 01:35 PM, micky_ching@...lsil.com.cn wrote:
> From: Micky Ching <micky_ching@...lsil.com.cn>
>
> Using non-DMA dump-regs, which would be more exactly for DMA transfer failed.
>
> More careful handle when cmd/data timeout, add stop(CMD12) cmd before go to
> finish request when multi-rw timeout.
>
> Remove some static checher warings.
> on commit: <mmc: rtsx: add support for pre_req and post_req>
> drivers/mmc/host/rtsx_pci_sdmmc.c:194 sd_finish_request()
> error: we previously assumed 'mrq' could be null (see line 158)
> drivers/mmc/host/rtsx_pci_sdmmc.c:504 sd_get_rsp()
> error: we previously assumed 'cmd' could be null (see line 434)
> drivers/mmc/host/rtsx_pci_sdmmc.c:525 sd_pre_dma_transfer()
> warn: we tested 'next' before and it was 'false'
>
> Signed-off-by: Micky Ching <micky_ching@...lsil.com.cn>
> ---
> drivers/mmc/host/rtsx_pci_sdmmc.c | 119 ++++++++++++++++++++-----------------
> 1 file changed, 65 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/mmc/host/rtsx_pci_sdmmc.c b/drivers/mmc/host/rtsx_pci_sdmmc.c
> index 0d8904a..453e1d4 100644
> --- a/drivers/mmc/host/rtsx_pci_sdmmc.c
> +++ b/drivers/mmc/host/rtsx_pci_sdmmc.c
> @@ -81,25 +81,24 @@ static inline void sd_clear_error(struct realtek_pci_sdmmc *host)
> }
>
> #ifdef DEBUG
> +static inline void sd_print_reg(struct realtek_pci_sdmmc *host, u16 reg)
> +{
> + u8 val = 0;
> +
> + if (rtsx_pci_read_register(host->pcr, reg, &val) < 0)
> + dev_dbg(sdmmc_dev(host), "read 0x%04x failed\n", reg);
> + else
> + dev_dbg(sdmmc_dev(host), "0x%04X: 0x%02x\n", reg, val);
> +}
> +
> static void sd_print_debug_regs(struct realtek_pci_sdmmc *host)
> {
> - struct rtsx_pcr *pcr = host->pcr;
> u16 i;
> - u8 *ptr;
> -
> - /* Print SD host internal registers */
> - rtsx_pci_init_cmd(pcr);
> - for (i = 0xFDA0; i <= 0xFDAE; i++)
> - rtsx_pci_add_cmd(pcr, READ_REG_CMD, i, 0, 0);
> - for (i = 0xFD52; i <= 0xFD69; i++)
> - rtsx_pci_add_cmd(pcr, READ_REG_CMD, i, 0, 0);
> - rtsx_pci_send_cmd(pcr, 100);
>
> - ptr = rtsx_pci_get_cmd_data(pcr);
> for (i = 0xFDA0; i <= 0xFDAE; i++)
> - dev_dbg(sdmmc_dev(host), "0x%04X: 0x%02x\n", i, *(ptr++));
> + sd_print_reg(host, i);
> for (i = 0xFD52; i <= 0xFD69; i++)
> - dev_dbg(sdmmc_dev(host), "0x%04X: 0x%02x\n", i, *(ptr++));
> + sd_print_reg(host, i);
> }
> #else
> #define sd_print_debug_regs(host)
> @@ -125,19 +124,27 @@ static void sd_request_timeout(unsigned long host_addr)
> spin_lock_irqsave(&host->lock, flags);
>
> if (!host->mrq) {
> - dev_err(sdmmc_dev(host), "error: no request exist\n");
> - goto out;
> + dev_err(sdmmc_dev(host), "error: request not exist\n");
> + spin_unlock_irqrestore(&host->lock, flags);
> + return;
> }
>
> - if (host->cmd)
> + if (host->cmd && host->data)
> + dev_err(sdmmc_dev(host), "error: cmd and data conflict\n");
> +
> + if (host->cmd) {
> host->cmd->error = -ETIMEDOUT;
> - if (host->data)
> - host->data->error = -ETIMEDOUT;
> + dev_dbg(sdmmc_dev(host), "timeout for cmd %d\n",
> + host->cmd->opcode);
> + tasklet_schedule(&host->cmd_tasklet);
> + }
>
> - dev_dbg(sdmmc_dev(host), "timeout for request\n");
> + if (host->data) {
> + host->data->error = -ETIMEDOUT;
> + dev_dbg(sdmmc_dev(host), "timeout for data transfer\n");
> + tasklet_schedule(&host->data_tasklet);
> + }
>
> -out:
> - tasklet_schedule(&host->finish_tasklet);
> spin_unlock_irqrestore(&host->lock, flags);
> }
>
> @@ -157,7 +164,8 @@ static void sd_finish_request(unsigned long host_addr)
> mrq = host->mrq;
> if (!mrq) {
> dev_err(sdmmc_dev(host), "error: no request need finish\n");
> - goto out;
> + spin_unlock_irqrestore(&host->lock, flags);
> + return;
> }
>
> cmd = mrq->cmd;
> @@ -167,11 +175,6 @@ static void sd_finish_request(unsigned long host_addr)
> (mrq->stop && mrq->stop->error) ||
> (cmd && cmd->error) || (data && data->error);
>
> - if (any_error) {
> - rtsx_pci_stop_cmd(pcr);
> - sd_clear_error(host);
> - }
> -
> if (data) {
> if (any_error)
> data->bytes_xfered = 0;
> @@ -188,7 +191,6 @@ static void sd_finish_request(unsigned long host_addr)
> host->cmd = NULL;
> host->data = NULL;
>
> -out:
> spin_unlock_irqrestore(&host->lock, flags);
> mutex_unlock(&pcr->pcr_mutex);
> mmc_request_done(host->mmc, mrq);
> @@ -373,8 +375,11 @@ static void sd_send_cmd(struct realtek_pci_sdmmc *host, struct mmc_command *cmd)
> if (cmd->opcode == SD_SWITCH_VOLTAGE) {
> err = rtsx_pci_write_register(pcr, SD_BUS_STAT,
> 0xFF, SD_CLK_TOGGLE_EN);
> - if (err < 0)
> + if (err < 0) {
> + rtsx_pci_write_register(pcr, SD_BUS_STAT,
> + SD_CLK_TOGGLE_EN | SD_CLK_FORCE_STOP, 0);
> goto out;
> + }
> }
>
> rtsx_pci_init_cmd(pcr);
> @@ -436,7 +441,8 @@ static void sd_get_rsp(unsigned long host_addr)
>
> if (!cmd) {
> dev_err(sdmmc_dev(host), "error: cmd not exist\n");
> - goto out;
> + spin_unlock_irqrestore(&host->lock, flags);
> + return;
> }
>
> spin_lock(&pcr->lock);
> @@ -446,16 +452,18 @@ static void sd_get_rsp(unsigned long host_addr)
> err = -EINVAL;
> spin_unlock(&pcr->lock);
>
> - if (err < 0)
> + if (err < 0) {
> + rtsx_pci_stop_cmd(host->pcr);
> + sd_print_debug_regs(host);
> + sd_clear_error(host);
> goto out;
> + }
>
> rsp_type = host->rsp_type;
> stat_idx = host->rsp_len;
>
> - if (rsp_type == SD_RSP_TYPE_R0) {
> - err = 0;
> + if (rsp_type == SD_RSP_TYPE_R0)
> goto out;
> - }
>
> /* Eliminate returned value of CHECK_REG_CMD */
> ptr = rtsx_pci_get_cmd_data(pcr) + 1;
> @@ -498,14 +506,19 @@ static void sd_get_rsp(unsigned long host_addr)
> goto out;
>
> if (cmd->data) {
> - sd_start_multi_rw(host, host->mrq);
> + err = sd_start_multi_rw(host, host->mrq);
> + if (err) {
> + cmd->data->error = err;
> + dev_err(sdmmc_dev(host),
> + "error: start data transfer failed\n");
> + tasklet_schedule(&host->data_tasklet);
> + }
> spin_unlock_irqrestore(&host->lock, flags);
> return;
> }
>
> out:
> cmd->error = err;
> -
> tasklet_schedule(&host->finish_tasklet);
> spin_unlock_irqrestore(&host->lock, flags);
> }
> @@ -525,7 +538,7 @@ static int sd_pre_dma_transfer(struct realtek_pci_sdmmc *host,
> data->host_cookie = 0;
> }
>
> - if (next || (!next && data->host_cookie != host->next_data.cookie))
> + if (next || data->host_cookie != host->next_data.cookie)
> sg_count = rtsx_pci_dma_map_sg(pcr,
> data->sg, data->sg_len, read);
> else
> @@ -580,7 +593,6 @@ static int sd_start_multi_rw(struct realtek_pci_sdmmc *host,
> int uhs = mmc_card_uhs(card);
> int read = data->flags & MMC_DATA_READ;
> u8 cfg2, trans_mode;
> - int err;
> size_t data_len = data->blksz * data->blocks;
>
> if (host->data)
> @@ -641,12 +653,7 @@ static int sd_start_multi_rw(struct realtek_pci_sdmmc *host,
> mod_timer(&host->timer, jiffies + 10 * HZ);
> rtsx_pci_send_cmd_no_wait(pcr);
>
> - err = rtsx_pci_dma_transfer(pcr, data->sg, host->sg_count, read);
> - if (err < 0) {
> - data->error = err;
> - tasklet_schedule(&host->finish_tasklet);
> - }
> - return 0;
> + return rtsx_pci_dma_transfer(pcr, data->sg, host->sg_count, read);
> }
>
> static void sd_finish_multi_rw(unsigned long host_addr)
> @@ -660,8 +667,9 @@ static void sd_finish_multi_rw(unsigned long host_addr)
> spin_lock_irqsave(&host->lock, flags);
>
> if (!host->data) {
> - dev_err(sdmmc_dev(host), "error: no data exist\n");
> - goto out;
> + dev_err(sdmmc_dev(host), "error: data not exist\n");
> + spin_unlock_irqrestore(&host->lock, flags);
> + return;
> }
>
> data = host->data;
> @@ -672,19 +680,22 @@ static void sd_finish_multi_rw(unsigned long host_addr)
> else if (pcr->trans_result != TRANS_RESULT_OK)
> err = -EINVAL;
>
> - if (err < 0) {
> + if (err < 0)
> data->error = err;
> - goto out;
> +
> + if (data->error) {
> + rtsx_pci_stop_cmd(host->pcr);
> + sd_print_debug_regs(host);
> + sd_clear_error(host);
> + dev_dbg(sdmmc_dev(host), "data transfer failed %d\n",
> + data->error);
> }
>
> - if (!host->mrq->sbc && data->stop) {
> + if (!host->mrq->sbc && data->stop)
> sd_send_cmd(host, data->stop);
> - spin_unlock_irqrestore(&host->lock, flags);
> - return;
> - }
> + else
> + tasklet_schedule(&host->finish_tasklet);
>
> -out:
> - tasklet_schedule(&host->finish_tasklet);
> spin_unlock_irqrestore(&host->lock, flags);
> }
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists