Thread (12 messages) 12 messages, 4 authors, 2018-12-11

Re: [PATCH] mmc: core: Lower max_seg_size if too high for DMA

From: Ulf Hansson <hidden>
Date: 2018-12-11 13:40:08
Also in: linux-mmc, linux-omap

On Tue, 11 Dec 2018 at 14:13, Russell King - ARM Linux
[off-list ref] wrote:
On Thu, Nov 29, 2018 at 09:11:17PM +0100, Ulf Hansson wrote:
quoted
On Thu, 29 Nov 2018 at 20:13, Tony Lindgren [off-list ref] wrote:
quoted
* Ulf Hansson [off-list ref] [181119 12:09]:
quoted
On 31 October 2018 at 16:57, Tony Lindgren [off-list ref] wrote:
quoted
With CONFIG_DMA_API_DEBUG_SG a device may produce the following warning:

"DMA-API: mapping sg segment longer than device claims to support"

We default to 64KiB if a DMA engine driver does not initialize dma_parms
and call dma_set_max_seg_size(). This may be lower that what many MMC
drivers do with mmc->max_seg_size = mmc->max_blk_size * mmc->max_blk_count.

Let's do a sanity check for max_seg_size being higher than what DMA
supports in mmc_add_host() and lower it as needed.

Cc: Kishon Vijay Abraham I <redacted>
Cc: Peter Ujfalusi <redacted>
Cc: Russell King <redacted>
Reported-by: Russell King <redacted>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/mmc/core/host.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -13,6 +13,7 @@
  */

 #include <linux/device.h>
+#include <linux/dma-mapping.h>
 #include <linux/err.h>
 #include <linux/idr.h>
 #include <linux/of.h>
@@ -415,6 +416,19 @@ struct mmc_host *mmc_alloc_host(int extra, struct device *dev)

 EXPORT_SYMBOL(mmc_alloc_host);

+static void mmc_check_max_seg_size(struct mmc_host *host)
+{
+       unsigned int max_seg_size = dma_get_max_seg_size(mmc_dev(host));
Is dma_get_max_seg_size() really intended to be called for any struct
device (representing the mmc controller) like this?

My understanding is that the dma_get_max_seg_size() is supposed to be
called by using the DMA engine device, no?
Oh good catch sounds like I'm calling it for the wrong device,
need to check. In that case sounds like this can't be generic?
No, I don't think so as it's only the mmc host driver that knows about
the DMA engine device.
We're nearing the merge window, and this is a regression that is yet
to be solved.  It causes a kernel warning with backtrace, so it's
very annoying.

The error is:

omap-dma-engine 4a056000.dma-controller: DMA-API: mapping sg segment longer than device claims to support [len=69632] [max=65536]

which indicates that we have a SG segment length that exceeds thte
published maximum segment size for a device - in this case the
DMA engine device.  The maximum segment size for the DMA engine comes
from the default per-device setting, in linux/dma-mapping.h, which is
64K.

However, omap_hsmmc sets:

        mmc->max_blk_size = 512;       /* Block Length at max can be 1024 */
        mmc->max_blk_count = 0xFFFF;    /* No. of Blocks is 16 bits */
        mmc->max_req_size = mmc->max_blk_size * mmc->max_blk_count;
        mmc->max_seg_size = mmc->max_req_size;

which ends up telling the block layer that we support a maximum segment
size of 65535*512, so of course it _will_ pass SG lists where a segment
is longer than 64K.

The problem here is that the HSMMC driver doesn't take account of the
DMA engine device's capabilities.

We have something of an odd situation in that the omap-dma device's
maximum SG size depends on the "address width" - it's 64K transfers
of whatever unit "address width" is, so the current implementation of
per-device parameters doesn't exactly work.  That means the default
64K limit at the device-level is reasonable.

The only thing I can think of doing is adding into omap_hsmmc:

        mmc->max_seg_size = min(mmc->max_req_size,
                                min(dma_get_max_seg_size(host->rx_chan->device->dev),
                                    dma_get_max_seg_size(host->tx_chan->device->dev)));

to limit the maximum segment size to that of the device _and_ dma
engine's capabilities.

Doing this solves the problem for me.
Thanks for the suggestion - it sounds like a reasonable way to fix the
problem, at least for now.

Do you want to send to patch or do you expect someone else to do it?

Kind regards
Uffe

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help