From patchwork Wed Nov 13 07:53:33 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Linus Walleij X-Patchwork-Id: 11241253 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 2F43E1747 for ; Wed, 13 Nov 2019 07:53:48 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 085CD222C9 for ; Wed, 13 Nov 2019 07:53:48 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="M9EqRpo1" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726179AbfKMHxr (ORCPT ); Wed, 13 Nov 2019 02:53:47 -0500 Received: from mail-lj1-f196.google.com ([209.85.208.196]:42887 "EHLO mail-lj1-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725908AbfKMHxr (ORCPT ); Wed, 13 Nov 2019 02:53:47 -0500 Received: by mail-lj1-f196.google.com with SMTP id n5so1420317ljc.9 for ; Tue, 12 Nov 2019 23:53:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=gdgX+5FPEMWeumBP/L3Kn2WDPUqpVI0CqS6oiSh3P9Y=; b=M9EqRpo1YzQAw5Aym/f7V7i0nozQekQgxJy2pHprR9394Ue0aGkBZ4MEUzkIIHM5e6 +xUdZ2OTmgebD8Jty7okqQ5c0AtB5DtY1xpjB7kyqOsdOot8FldKF/oQErIe5kKofk5t rJSwBW9vG4vEoGBgDOIF4J+2etTtudbHqgveR6kUm1unZ7adMRHUpN+CGvcbYHraILGk eCnHbWJiCmGaou9IxkcmljPnxOYfpFBq/rvwMmAb+dEJUbQTHW1s2epGkbwn/Y4rVaJK hCY49JaBTyZurxQkHobOioY6fZmHrdpOFXhgotVutGIHEt2tBLktTF/2+c6yev/nq9Zz saXA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=gdgX+5FPEMWeumBP/L3Kn2WDPUqpVI0CqS6oiSh3P9Y=; b=BRyprP8CA1nR9Tna10hnmfXleSaUjFLFaeZCxoXnNUGWUFjcA5vgp8s+HgGpveZuH3 iQTy+goKJ2rfCN11pq4oppa8hODetQS527p9DrTb+Io/cINAK7foEH/+6BYZhS7ziWpi t9QKSwgmHzqyxPMfS4T4WNZpbl9nDhEWPUY/0oUokWvFuOj3XDQsGJkMkf4T5qIcCkI6 tvd84/r7wKw/pNYR77HpBXWM93Ai65wGiI/TU7l+vrzB5NIOGI/d6kiegKSe6a0hWG1a PTQchADnh1iQq9cqpE+3HyfMqQegRZ2aU9fjcWWAZJmTz9pgStDA/ROddLgDG25kmYkm hNBA== X-Gm-Message-State: APjAAAUGfvFQYbd03TPWkk76neZ5zRI3SH5B65C+1Qrlb/0wY2QndoRQ 01t5m6pECSYI09wBKc4amM+O/w== X-Google-Smtp-Source: APXvYqxI9u3EcltksVbByILLSxmRN8q0bV2Stmj6PhBTWtyi4NXy3X/Je6aFtgys22hZnZtegZkhCQ== X-Received: by 2002:a2e:4703:: with SMTP id u3mr1385526lja.126.1573631623982; Tue, 12 Nov 2019 23:53:43 -0800 (PST) Received: from localhost.bredbandsbolaget (c-79c8225c.014-348-6c756e10.bbcust.telenor.se. [92.34.200.121]) by smtp.gmail.com with ESMTPSA id j10sm610110lfc.43.2019.11.12.23.53.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 12 Nov 2019 23:53:42 -0800 (PST) From: Linus Walleij To: Ulf Hansson , linux-mmc@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Stephan Gerhold Cc: Russell King , Ludovic Barre , Brian Masney , Niklas Cassel , Russell King , Srinivas Kandagatla , Linus Walleij Subject: [PATCH 1/3] mmc: mmci: Support odd block sizes for ux500v2 and qcom variant Date: Wed, 13 Nov 2019 08:53:33 +0100 Message-Id: <20191113075335.31775-2-linus.walleij@linaro.org> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20191113075335.31775-1-linus.walleij@linaro.org> References: <20191113075335.31775-1-linus.walleij@linaro.org> MIME-Version: 1.0 Sender: linux-mmc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-mmc@vger.kernel.org From: Ulf Hansson This is something like the 5th time this patch is posted, so let's try to fix this now, once and for all. For the ux500v2 variant of the PL18x block, odd block sizes are supported. This is necessary to support some SDIO transfers such as single bytes. This also affects the QCOM MMCI variant. This will work fine for PIO using IRQs: SDIO packets are accepted down to single bytes and the transfers go through just fine. This patch has proven necessary for enabling SDIO for WLAN on PostmarketOS-based Ux500 platforms. This patch is based on Ulf Hansson's patch http://www.spinics.net/lists/linux-mmc/msg12160.html Ulf noted on an earlier iteration in: https://marc.info/?l=linux-mmc&m=140845189316370&w=2 "There are some prerequisites of the data buffers to supports any block size, at least for ux500. (...) The conclusion from the above is that we need to adopt mmci_pio_write() to handle corner cases." This points back to a discussion in 2012. The main point was made by Russell in this message: https://marc.info/?l=linux-arm-kernel&m=135351237018301&w=2 IIUC this pertains to this code (now gone from the patch): if (data->sg->offset & 3) { dev_err(...); return -EINVAL; } This hit Stephan as he noticed that DMA (DMA40) would not work with the MMCI driver, so this patch combined with disabling DMA would do the trick. That way we don't toss unaligned accesses at the DMA engine as SDIO apparently tends to do. (This is not a problem when writing ordinary block device blocks as these are always 512 bytes aligned on a 4-byte boundary.) As Ulf notes, odd SG offsets like this should be handled by the driver even if we run it in DMA mode. I conclude it must be the duty of the DMA driver to say NO to SG offsets it cannot handle, or otherwise bitstuff things around to avoid the situation. So as a first step make sure errors are propagated upward from the DMA engine, and assume the DMA engine will say no to things with weird SG offsets that it cannot handle, and then the driver will fall back to using PIO. It might be that some DMA engines (such as the Ux500 DMA40) do not properly say no to sglists with uneven offsets, or ignore the offset altogether resulting in unpredictable behavior. That is in that case a bug in the DMA driver and needs to be fixed there. I got the impression that the Qualcomm DMA actually can handle these odd alignments without problems. (Make a drive-by fix for datactrl_blocksz, misspelled.) Cc: Ludovic Barre Cc: Brian Masney Cc: Stephan Gerhold Cc: Niklas Cassel Cc: Russell King Tested-by: Stephan Gerhold Signed-off-by: Ulf Hansson Signed-off-by: Srinivas Kandagatla Signed-off-by: Linus Walleij Tested-by: Ludovic Barre --- ChangeLog v2->v3: - Repost with the inclusion of other patches. ChangeLog v1->v2: - Specify odd blocksize field to 1 bit (:1) - Specify that STMMC supports odd block sizes - Collect Stephan's test tag --- drivers/mmc/host/mmci.c | 20 ++++++++++++++++---- drivers/mmc/host/mmci.h | 6 +++++- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index c37e70dbe250..3ffcdf78a428 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -168,6 +168,7 @@ static struct variant_data variant_ux500 = { .cmdreg_srsp = MCI_CPSM_RESPONSE, .datalength_bits = 24, .datactrl_blocksz = 11, + .datactrl_odd_blocksz = true, .datactrl_mask_sdio = MCI_DPSM_ST_SDIOEN, .st_sdio = true, .st_clkdiv = true, @@ -201,6 +202,7 @@ static struct variant_data variant_ux500v2 = { .datactrl_mask_ddrmode = MCI_DPSM_ST_DDRMODE, .datalength_bits = 24, .datactrl_blocksz = 11, + .datactrl_odd_blocksz = true, .datactrl_mask_sdio = MCI_DPSM_ST_SDIOEN, .st_sdio = true, .st_clkdiv = true, @@ -260,6 +262,7 @@ static struct variant_data variant_stm32_sdmmc = { .datacnt_useless = true, .datalength_bits = 25, .datactrl_blocksz = 14, + .datactrl_odd_blocksz = true, .stm32_idmabsize_mask = GENMASK(12, 5), .init = sdmmc_variant_init, }; @@ -279,6 +282,7 @@ static struct variant_data variant_qcom = { .data_cmd_enable = MCI_CPSM_QCOM_DATCMD, .datalength_bits = 24, .datactrl_blocksz = 11, + .datactrl_odd_blocksz = true, .pwrreg_powerup = MCI_PWR_UP, .f_max = 208000000, .explicit_mclk_control = true, @@ -447,10 +451,11 @@ void mmci_dma_setup(struct mmci_host *host) static int mmci_validate_data(struct mmci_host *host, struct mmc_data *data) { + struct variant_data *variant = host->variant; + if (!data) return 0; - - if (!is_power_of_2(data->blksz)) { + if (!is_power_of_2(data->blksz) && !variant->datactrl_odd_blocksz) { dev_err(mmc_dev(host->mmc), "unsupported block size (%d bytes)\n", data->blksz); return -EINVAL; @@ -515,7 +520,9 @@ int mmci_dma_start(struct mmci_host *host, unsigned int datactrl) "Submit MMCI DMA job, sglen %d blksz %04x blks %04x flags %08x\n", data->sg_len, data->blksz, data->blocks, data->flags); - host->ops->dma_start(host, &datactrl); + ret = host->ops->dma_start(host, &datactrl); + if (ret) + return ret; /* Trigger the DMA transfer */ mmci_write_datactrlreg(host, datactrl); @@ -872,9 +879,14 @@ int mmci_dmae_prep_data(struct mmci_host *host, int mmci_dmae_start(struct mmci_host *host, unsigned int *datactrl) { struct mmci_dmae_priv *dmae = host->dma_priv; + int ret; host->dma_in_progress = true; - dmaengine_submit(dmae->desc_current); + ret = dma_submit_error(dmaengine_submit(dmae->desc_current)); + if (ret < 0) { + host->dma_in_progress = false; + return ret; + } dma_async_issue_pending(dmae->cur); *datactrl |= MCI_DPSM_DMAENABLE; diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h index 833236ecb31e..c7f94726eaa1 100644 --- a/drivers/mmc/host/mmci.h +++ b/drivers/mmc/host/mmci.h @@ -278,7 +278,10 @@ struct mmci_host; * @stm32_clkdiv: true if using a STM32-specific clock divider algorithm * @datactrl_mask_ddrmode: ddr mode mask in datactrl register. * @datactrl_mask_sdio: SDIO enable mask in datactrl register - * @datactrl_blksz: block size in power of two + * @datactrl_blocksz: block size in power of two + * @datactrl_odd_blocksz: true if block any sizes are supported, such as one + * single character, as is necessary when using some SDIO + * devices. * @datactrl_first: true if data must be setup before send command * @datacnt_useless: true if you could not use datacnt register to read * remaining data @@ -323,6 +326,7 @@ struct variant_data { unsigned int datactrl_mask_ddrmode; unsigned int datactrl_mask_sdio; unsigned int datactrl_blocksz; + u8 datactrl_odd_blocksz:1; u8 datactrl_first:1; u8 datacnt_useless:1; u8 st_sdio:1; From patchwork Wed Nov 13 07:53:34 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Linus Walleij X-Patchwork-Id: 11241255 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 00F6F15AB for ; Wed, 13 Nov 2019 07:53:50 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id D85C3222C6 for ; Wed, 13 Nov 2019 07:53:49 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="v4Oyosu3" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725908AbfKMHxt (ORCPT ); Wed, 13 Nov 2019 02:53:49 -0500 Received: from mail-lj1-f193.google.com ([209.85.208.193]:43315 "EHLO mail-lj1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726086AbfKMHxt (ORCPT ); Wed, 13 Nov 2019 02:53:49 -0500 Received: by mail-lj1-f193.google.com with SMTP id y23so1410297ljh.10 for ; Tue, 12 Nov 2019 23:53:47 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=Z8pRSrNjPJIG3QeAtwUZocUyaeikI+vC5l6O+U96lQw=; b=v4Oyosu3WiO5hfa96SAV/hmdVCPlCP/p6Hqm+7l1Wv1ykkepsTzPEhEK05HQVeJ9p8 UoNDHpr1vmm5//p9cfaDUfOE+87sXevogKJ/PluCRJil50DDo3/w0jjhxZB+3I9QspEs YiXTvFllQVXiX5S2NyobNj/aZiQ7I4b0IFAufRygReJh8sGRvjFM36vZwKqzCqZByjGY xbqJ8nQUBRM1LZ0NopFf4bt0B1W7yDlBKAUbIUonf+A+HeUWLoj3Sm/EyB4VDnO1uZNP rSTC2hMY+LjBBjOOn/z4m3IPsP2DPmfB8RT0Xx17FhMiMUnzm+8P+KbPkYgRzrMEjhfI eBUw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=Z8pRSrNjPJIG3QeAtwUZocUyaeikI+vC5l6O+U96lQw=; b=MHtdDg6jrk7ui+MOv4dRHSWZmkjWwGewvkLkT3u/y+iyYFl2yurLKFcKVDl3VSQK8w xcOmSnKBo9TMRmnQmEjElE8ESs/WZ/u7+WX8ew5vAVnV87h97lc98vLHdvQWdGcGDmaH 71FeOnf2n2uhFitJs/R6bDYv3kPp17OL+lzX6j4aiWAeBLIwbjKPFLY8LlWS1UYACVhj OpGFS1SHulEjrsx0qDh5p9CUuORpltHnA/RltUPNbeYX1jt26xISOKNRTaQrY1qcsRpm 5XdvceeljULr7R5g58pzcjYnGT3xu3yDaB4Ks6VxWMeU5Gcm90ahTMyoJbk89ZMKYI/U m90Q== X-Gm-Message-State: APjAAAVUW7BfSovxeAMJFk9PHiMApYvEEMwVtQ82o0VsEPyEBwvZOxx3 58KaapMp1q1vHp8HA1IRCtV7sQ== X-Google-Smtp-Source: APXvYqyrkxS30gjbtF0r8GgTAsQTlssm51WKkDZ3hz77IJ0JJm4ESdusnZp+VRTdcRljSfMtR5ZBLQ== X-Received: by 2002:a05:651c:1a1:: with SMTP id c1mr1530078ljn.23.1573631626433; Tue, 12 Nov 2019 23:53:46 -0800 (PST) Received: from localhost.bredbandsbolaget (c-79c8225c.014-348-6c756e10.bbcust.telenor.se. [92.34.200.121]) by smtp.gmail.com with ESMTPSA id j10sm610110lfc.43.2019.11.12.23.53.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 12 Nov 2019 23:53:45 -0800 (PST) From: Linus Walleij To: Ulf Hansson , linux-mmc@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Stephan Gerhold Cc: Russell King , Linus Walleij , Ludovic Barre , Brian Masney , Niklas Cassel , Russell King Subject: [PATCH 2/3] mmc: mmci: Bail out from odd DMA on Ux500 Date: Wed, 13 Nov 2019 08:53:34 +0100 Message-Id: <20191113075335.31775-3-linus.walleij@linaro.org> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20191113075335.31775-1-linus.walleij@linaro.org> References: <20191113075335.31775-1-linus.walleij@linaro.org> MIME-Version: 1.0 Sender: linux-mmc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-mmc@vger.kernel.org The Ux500 (at least) can only deal with DMA transactions starting and ending on an even 4-byte aligned address. The problem isn't in the DMA engine of the system as such: the problem is in the state machine of the MMCI block that has some features to handle single bytes but it seems like it doesn't quite work. This problem is probably caused by most of the testing being done on mass storage, which will be 512-bytes aligned blocks placed neatly in pages and practically never run into this situation. On SDIO (for example in WiFi adapters) this situation is common. By avoiding any such transfers with a special vendor flag, we can bail out to PIO when an odd transfer is detected while keeping DMA for large transfers of evenly aligned packages also for SDIO. Cc: Ludovic Barre Cc: Brian Masney Cc: Stephan Gerhold Cc: Niklas Cassel Cc: Russell King Signed-off-by: Linus Walleij --- ChangeLog v1->v3: - New patch in v3 after discussion with Ulf --- drivers/mmc/host/mmci.c | 21 +++++++++++++++++++++ drivers/mmc/host/mmci.h | 10 ++++++++++ 2 files changed, 31 insertions(+) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index 3ffcdf78a428..a08cd845dddc 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -185,6 +185,7 @@ static struct variant_data variant_ux500 = { .irq_pio_mask = MCI_IRQ_PIO_MASK, .start_err = MCI_STARTBITERR, .opendrain = MCI_OD, + .only_long_aligned_dma = true, .init = mmci_variant_init, }; @@ -219,6 +220,7 @@ static struct variant_data variant_ux500v2 = { .irq_pio_mask = MCI_IRQ_PIO_MASK, .start_err = MCI_STARTBITERR, .opendrain = MCI_OD, + .only_long_aligned_dma = true, .init = ux500v2_variant_init, }; @@ -829,6 +831,25 @@ static int _mmci_dmae_prep_data(struct mmci_host *host, struct mmc_data *data, if (data->blksz * data->blocks <= variant->fifosize) return -EINVAL; + /* + * Handle the variants with DMA that is broken such that start and + * end address must be aligned on a long (32bit) boundary for the DMA + * to work. If this occurs, fall back to PIO. + */ + if (host->variant->only_long_aligned_dma) { + struct scatterlist *sg; + int tmp; + + for_each_sg(data->sg, sg, data->sg_len, tmp) { + /* We start in some odd place, that doesn't work */ + if (sg->offset & 3) + return -EINVAL; + /* We end in some odd place, that doesn't work */ + if (sg->length & 3) + return -EINVAL; + } + } + device = chan->device; nr_sg = dma_map_sg(device->dev, data->sg, data->sg_len, mmc_get_dma_dir(data)); diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h index c7f94726eaa1..e20af17bb313 100644 --- a/drivers/mmc/host/mmci.h +++ b/drivers/mmc/host/mmci.h @@ -307,6 +307,15 @@ struct mmci_host; * register. * @opendrain: bitmask identifying the OPENDRAIN bit inside MMCIPOWER register * @dma_lli: true if variant has dma link list feature. + * @only_long_aligned_dma: it appears that the Ux500 has a broken DMA logic for + * single bytes when either the transfer starts at an odd offset or + * the final DMA burst is an odd (not divisible by 4) address. + * Reading must start and end on an even 4-byte boundary, i.e. an + * even 32bit word in memory. If this is not the case, we need to + * fall back to PIO for that request. For bulk transfers to mass + * storage we are almost exclusively dealing with 512-byte chunks + * allocated at an even address so this is usually only manifesting + * in SDIO. * @stm32_idmabsize_mask: stm32 sdmmc idma buffer size. */ struct variant_data { @@ -350,6 +359,7 @@ struct variant_data { u32 start_err; u32 opendrain; u8 dma_lli:1; + u8 only_long_aligned_dma:1; u32 stm32_idmabsize_mask; void (*init)(struct mmci_host *host); }; From patchwork Wed Nov 13 07:53:35 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Linus Walleij X-Patchwork-Id: 11241259 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 0606D15AB for ; Wed, 13 Nov 2019 07:53:52 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id D292F222C6 for ; Wed, 13 Nov 2019 07:53:51 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="DsChzYTn" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726189AbfKMHxv (ORCPT ); Wed, 13 Nov 2019 02:53:51 -0500 Received: from mail-lf1-f67.google.com ([209.85.167.67]:36167 "EHLO mail-lf1-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726086AbfKMHxv (ORCPT ); Wed, 13 Nov 2019 02:53:51 -0500 Received: by mail-lf1-f67.google.com with SMTP id m6so1127066lfl.3 for ; Tue, 12 Nov 2019 23:53:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=B+Nq0EqiI8iyYesbpyt3jz/qbylH4sylp18sNMPaQEM=; b=DsChzYTn7L0+YXTQOmPxbmVVPiiwiA31DixtYt8VuLWmg+8aa+RL0Fyj0Gq8L68H6L dDHeWXO8atkssCtBo6P9EaVKxnD5sJj+rczIBey5r0Y7Zq5e9eaItL4A69fRiHrHo9Dj i9UWkkYs8i/3E8UJUGrG3ZpiuqXyms9gR5VF+LEOjcKezuBIXil3JzPv3yfwvOq55m7e K+5qKHVvR70hgL9gAezcbhfxhtS++qWH6FejpxvAQoHks515LlhgREnoDgGW836UtrMt veX2fbioHcXs/TAC2zO1P6DATPGRVXsE48Re4GZTWPyThzvNycLH+XoYV/gG7uLNPOlO wYhw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=B+Nq0EqiI8iyYesbpyt3jz/qbylH4sylp18sNMPaQEM=; b=RvTDeUqYZaUUf4+TMZO9PGnihip9W75vQ7mH1YHAcNHBHaM+yGDS8L/zZpt9DTQqJ0 orQWrVjKpUhx+DXrIFe1fe+kUS+zjXx6cZSTGY8tgEF9qDugEdm3wSTfRFQQrOO1WgGr sqG8oWZB2ITPeOajMgEJM6xvQU/nJv3XAAsnzLr2w7yttHuPOEyunZBempZsA+Cr687h ZfSw/Fuo5rTVHbxICwdxz0qlqNz5h056Dm+uyMO5WVA1BNDWUhAV3ZqX8Tw1HGivEOTa O0RwsoYwZiIEXLwNtZT+UGN0IEDdGJLmr8sBl3qlyyGKga+Swkrz5Ma7wA7vOqvEf+1M uXgg== X-Gm-Message-State: APjAAAXVDST9QDKHHqQ/1FRK7+cpd/QUeJ3zcTMvQT+J+X0WpTYDGglg KYf5+nzlYjSn4otqg5tO4qKbQg== X-Google-Smtp-Source: APXvYqxR9l5Gjz4IEu194uSHeGsXPtj30ILrUeGubVYW7wQZHu1q+L78LEMhKEkSdt9qcif7m3dZxQ== X-Received: by 2002:a19:e002:: with SMTP id x2mr1727117lfg.28.1573631628777; Tue, 12 Nov 2019 23:53:48 -0800 (PST) Received: from localhost.bredbandsbolaget (c-79c8225c.014-348-6c756e10.bbcust.telenor.se. [92.34.200.121]) by smtp.gmail.com with ESMTPSA id j10sm610110lfc.43.2019.11.12.23.53.47 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 12 Nov 2019 23:53:47 -0800 (PST) From: Linus Walleij To: Ulf Hansson , linux-mmc@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Stephan Gerhold Cc: Russell King , Linus Walleij , Ludovic Barre , Brian Masney , Niklas Cassel , Russell King Subject: [PATCH 3/3] mmc: mmci: Proper PIO residue handling Date: Wed, 13 Nov 2019 08:53:35 +0100 Message-Id: <20191113075335.31775-4-linus.walleij@linaro.org> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20191113075335.31775-1-linus.walleij@linaro.org> References: <20191113075335.31775-1-linus.walleij@linaro.org> MIME-Version: 1.0 Sender: linux-mmc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-mmc@vger.kernel.org The MMCI PIO write function contains a bug if using any buffers with sglist miter contents that do not add upp to even 32bit writes. The PIO write code currently does this: iowrite32_rep(base + MMCIFIFO, ptr, (count + 3) >> 2); This will make sure that a single buffer passed in gets written into the FIFO even if it is odd, i.e. not evenly divisible by 4. However it has the following problems: - It will read up to three bytes beyond the end of the buffer and fill the FIFO with unpredictable junk and possibly cause segmentation violations if the read passes a page boundary such as at the end of an sglist buffer. - It will be fine if a single buffer is passed in, but the code handles SG lists. There is a while (1) loop in mmci_pio_irq() which repeatedly calls mmci_pio_write() as long as the FIFO is not half-full, if it gets half-full it exits the IRQ and waits for a new IRQ, also the while loop repeatedly calls sg_miter_next() to consume bytes and this means that between subsequent calls to mmci_pio_write() some random junk can be inserted at the end of each call if the buffers passed in do not contain an number of bytes evenly divisible by 4. Fix this by simply doing this: iowrite32_rep(base + MMCIFIFO, ptr, count >> 2); But since the code will then just not write enough bytes if the count is not evenly divisible by 4, we introduce a special residue buffer and keep track of any odd bytes from 1..3 and just write these padded out with new data in a separate statement the next time we get a call of the mmci_pio_write(), or, if there is for example only 1,2 or 3 bytes in the transfer, or we end up with an odd number of bytes in the residue, flushi it out when we end the current data transaction to the host. Cc: Ludovic Barre Cc: Brian Masney Cc: Stephan Gerhold Cc: Niklas Cassel Cc: Russell King Signed-off-by: Linus Walleij --- ChangeLog v1->v3: - New patch in v3 after discussion with Ulf - I'm unable to test this because I cannot provoke these odd reads/writes but hoping for Stephan to take it for a spin. --- drivers/mmc/host/mmci.c | 125 ++++++++++++++++++++++++++++++++++++---- drivers/mmc/host/mmci.h | 2 + 2 files changed, 117 insertions(+), 10 deletions(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index a08cd845dddc..0d01689016f0 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -607,6 +607,7 @@ static void mmci_init_sg(struct mmci_host *host, struct mmc_data *data) flags |= SG_MITER_FROM_SG; sg_miter_start(&host->sg_miter, data->sg, data->sg_len, flags); + host->pio_write_residue_sz = 0; } static u32 mmci_get_dctrl_cfg(struct mmci_host *host) @@ -1422,30 +1423,111 @@ static int mmci_pio_write(struct mmci_host *host, char *buffer, unsigned int rem struct variant_data *variant = host->variant; void __iomem *base = host->base; char *ptr = buffer; + bool wrote_residue = false; + int i; + + /* + * This will normally not happen during block I/O, but can + * happen during SDIO traffic, where odd byte chunks are + * shoveled to the SDIO link. Fill up the residue buffer + * and flush out. + */ + if (host->pio_write_residue_sz) { + int fill = 4 - host->pio_write_residue_sz; + u32 val = 0; + + /* + * If we need more padding that what we have, just stash + * some more in the residue then and continue. This only + * happens if we're passed e.g. 1 or 2 bytes and there is + * just 1 byte residue residue already: we can't fill up! + */ + if (fill > remain) { + for (i = 0; i < fill; i++) { + host->pio_write_residue[host->pio_write_residue_sz + i] = *ptr; + host->pio_write_residue_sz++; + ptr++; + remain--; + } + return ptr - buffer; + } + + /* Pack the residue into a 32bit word */ + for (i = 0; i < host->pio_write_residue_sz; i++) { + val |= host->pio_write_residue[i]; + val <<= 8; + } + /* Top up with new data */ + for (i = 0; i < fill; i++) { + val |= *ptr; + val <<= 8; + ptr++; + remain--; + } + iowrite32(val, base + MMCIFIFO); + host->pio_write_residue_sz = 0; + wrote_residue = true; + } + + /* + * Maybe the residue plus some few bytes was exactly filling a + * 32bit word. + */ + if (!remain) + return ptr - buffer; do { - unsigned int count, maxcnt; + unsigned int count, maxcnt, written, residue; + /* + * If the FIFO is empty just stash it full with data else + * just half-fill it. + */ maxcnt = status & MCI_TXFIFOEMPTY ? - variant->fifosize : variant->fifohalfsize; + variant->fifosize : variant->fifohalfsize; + + /* Watch it to not overfill the FIFO */ + if (wrote_residue) + maxcnt -= 4; + count = min(remain, maxcnt); /* * SDIO especially may want to send something that is * not divisible by 4 (as opposed to card sectors - * etc), and the FIFO only accept full 32-bit writes. - * So compensate by adding +3 on the count, a single - * byte become a 32bit write, 7 bytes will be two - * 32bit writes etc. + * etc), but the FIFO only accepts full 32-bit writes so start by + * just filling up quickly with as much as we can. */ - iowrite32_rep(base + MMCIFIFO, ptr, (count + 3) >> 2); + iowrite32_rep(base + MMCIFIFO, ptr, count >> 2); + residue = count & 3; - ptr += count; - remain -= count; + written = count - residue; + ptr += written; + remain -= written; - if (remain == 0) + /* Handles the common case for block-IO */ + if (!remain) break; + /* + * These were not written out since we only write 32bit + * words to the FIFO. Since this function gets called + * repeatedly and across page boundaries with new pointers + * in *buffer we need to take special care to stash odd + * bytes away and flush them out in next PIO round or at the + * end of the whole data transfer. + */ + if (remain < 4) { + WARN_ON(remain != residue); + host->pio_write_residue_sz = residue; + for (i = 0; i < residue; i++) { + host->pio_write_residue[i] = *ptr; + ptr++; + remain--; + } + break; + } + status = readl(base + MMCISTATUS); } while (status & MCI_TXFIFOHALFEMPTY); @@ -1522,6 +1604,29 @@ static irqreturn_t mmci_pio_irq(int irq, void *dev_id) if (host->size == 0) { mmci_set_mask1(host, 0); writel(readl(base + MMCIMASK0) | MCI_DATAENDMASK, base + MMCIMASK0); + + if ((status & MCI_TXACTIVE) && host->pio_write_residue_sz) { + /* + * If the last pass of the SG miter left some residue after the pio + * write loop, push that out to the card. We are now at the end of + * the transfer so it is OK to pad with zeroes. + */ + int fill = 4 - host->pio_write_residue_sz; + u32 val = 0; + int i; + + /* Pack the residue into a 32bit word */ + for (i = 0; i < host->pio_write_residue_sz; i++) { + val |= host->pio_write_residue[i]; + val <<= 8; + } + /* Top up with zeroes */ + for (i = 0; i < fill; i++) + val <<= 8; + + iowrite32(val, base + MMCIFIFO); + host->pio_write_residue_sz = 0; + } } return IRQ_HANDLED; diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h index e20af17bb313..a7690b64d4cd 100644 --- a/drivers/mmc/host/mmci.h +++ b/drivers/mmc/host/mmci.h @@ -425,6 +425,8 @@ struct mmci_host { /* pio stuff */ struct sg_mapping_iter sg_miter; unsigned int size; + u8 pio_write_residue[4]; + u8 pio_write_residue_sz; int (*get_rx_fifocnt)(struct mmci_host *h, u32 status, int remain); u8 use_dma:1;