All of lore.kernel.org
 help / color / mirror / Atom feed
From: Linus Walleij <linus.walleij@linaro.org>
To: Christoph Hellwig <hch@lst.de>, Jens Axboe <axboe@kernel.dk>,
	 Ming Lei <ming.lei@redhat.com>, Arnd Bergmann <arnd@arndb.de>,
	 Ulf Hansson <ulf.hansson@linaro.org>,
	Nicolas Pitre <nico@fluxnic.net>,
	 Aaro Koskinen <aaro.koskinen@iki.fi>,
	 Adrian Hunter <adrian.hunter@intel.com>,
	 Angelo Dureghello <angelo.dureghello@timesys.com>
Cc: linux-mmc@vger.kernel.org, linux-block@vger.kernel.org,
	 linux-omap@vger.kernel.org,
	Linus Walleij <linus.walleij@linaro.org>
Subject: [PATCH v2 5/9] mmc: mvsdio: Use sg_miter for PIO
Date: Sat, 27 Jan 2024 01:19:52 +0100	[thread overview]
Message-ID: <20240127-mmc-proper-kmap-v2-5-d8e732aa97d1@linaro.org> (raw)
In-Reply-To: <20240127-mmc-proper-kmap-v2-0-d8e732aa97d1@linaro.org>

Use the scatterlist memory iterator instead of just
dereferencing virtual memory using sg_virt().
This make highmem references work properly.

This driver also has a bug in the PIO sglist handling that
is fixed as part of the patch: it does not travers the
list of scatterbuffers: it will just process the first
item in the list. This is fixed by augmenting the logic
such that we do not process more than one sgitem
per IRQ instead of counting down potentially the whole
length of the request.

We can suspect that the PIO path is quite untested.

Suggested-by: Christoph Hellwig <hch@lst.de>
Link: https://lore.kernel.org/linux-mmc/20240122073423.GA25859@lst.de/
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/mmc/host/mvsdio.c | 71 +++++++++++++++++++++++++++++++++++------------
 1 file changed, 53 insertions(+), 18 deletions(-)

diff --git a/drivers/mmc/host/mvsdio.c b/drivers/mmc/host/mvsdio.c
index ca01b7d204ba..af7f21888e27 100644
--- a/drivers/mmc/host/mvsdio.c
+++ b/drivers/mmc/host/mvsdio.c
@@ -38,8 +38,9 @@ struct mvsd_host {
 	unsigned int xfer_mode;
 	unsigned int intr_en;
 	unsigned int ctrl;
+	bool use_pio;
+	struct sg_mapping_iter sg_miter;
 	unsigned int pio_size;
-	void *pio_ptr;
 	unsigned int sg_frags;
 	unsigned int ns_per_clk;
 	unsigned int clock;
@@ -114,11 +115,18 @@ static int mvsd_setup_data(struct mvsd_host *host, struct mmc_data *data)
 		 * data when the buffer is not aligned on a 64 byte
 		 * boundary.
 		 */
+		unsigned int miter_flags = SG_MITER_ATOMIC; /* Used from IRQ */
+
+		if (data->flags & MMC_DATA_READ)
+			miter_flags |= SG_MITER_TO_SG;
+		else
+			miter_flags |= SG_MITER_FROM_SG;
+
 		host->pio_size = data->blocks * data->blksz;
-		host->pio_ptr = sg_virt(data->sg);
+		sg_miter_start(&host->sg_miter, data->sg, data->sg_len, miter_flags);
 		if (!nodma)
-			dev_dbg(host->dev, "fallback to PIO for data at 0x%p size %d\n",
-				host->pio_ptr, host->pio_size);
+			dev_dbg(host->dev, "fallback to PIO for data\n");
+		host->use_pio = true;
 		return 1;
 	} else {
 		dma_addr_t phys_addr;
@@ -129,6 +137,7 @@ static int mvsd_setup_data(struct mvsd_host *host, struct mmc_data *data)
 		phys_addr = sg_dma_address(data->sg);
 		mvsd_write(MVSD_SYS_ADDR_LOW, (u32)phys_addr & 0xffff);
 		mvsd_write(MVSD_SYS_ADDR_HI,  (u32)phys_addr >> 16);
+		host->use_pio = false;
 		return 0;
 	}
 }
@@ -288,8 +297,8 @@ static u32 mvsd_finish_data(struct mvsd_host *host, struct mmc_data *data,
 {
 	void __iomem *iobase = host->base;
 
-	if (host->pio_ptr) {
-		host->pio_ptr = NULL;
+	if (host->use_pio) {
+		sg_miter_stop(&host->sg_miter);
 		host->pio_size = 0;
 	} else {
 		dma_unmap_sg(mmc_dev(host->mmc), data->sg, host->sg_frags,
@@ -344,9 +353,12 @@ static u32 mvsd_finish_data(struct mvsd_host *host, struct mmc_data *data,
 static irqreturn_t mvsd_irq(int irq, void *dev)
 {
 	struct mvsd_host *host = dev;
+	struct sg_mapping_iter *sgm = &host->sg_miter;
 	void __iomem *iobase = host->base;
 	u32 intr_status, intr_done_mask;
 	int irq_handled = 0;
+	u16 *p;
+	int s;
 
 	intr_status = mvsd_read(MVSD_NOR_INTR_STATUS);
 	dev_dbg(host->dev, "intr 0x%04x intr_en 0x%04x hw_state 0x%04x\n",
@@ -370,15 +382,36 @@ static irqreturn_t mvsd_irq(int irq, void *dev)
 	spin_lock(&host->lock);
 
 	/* PIO handling, if needed. Messy business... */
-	if (host->pio_size &&
+	if (host->use_pio) {
+		/*
+		 * As we set sgm->consumed this always gives a valid buffer
+		 * position.
+		 */
+		if (!sg_miter_next(sgm)) {
+			/* This should not happen */
+			dev_err(host->dev, "ran out of scatter segments\n");
+			spin_unlock(&host->lock);
+			host->intr_en &=
+				~(MVSD_NOR_RX_READY | MVSD_NOR_RX_FIFO_8W |
+				  MVSD_NOR_TX_AVAIL | MVSD_NOR_TX_FIFO_8W);
+			mvsd_write(MVSD_NOR_INTR_EN, host->intr_en);
+			return IRQ_HANDLED;
+		}
+		p = sgm->addr;
+		s = sgm->length;
+		if (s > host->pio_size)
+			s = host->pio_size;
+	}
+
+	if (host->use_pio &&
 	    (intr_status & host->intr_en &
 	     (MVSD_NOR_RX_READY | MVSD_NOR_RX_FIFO_8W))) {
-		u16 *p = host->pio_ptr;
-		int s = host->pio_size;
+
 		while (s >= 32 && (intr_status & MVSD_NOR_RX_FIFO_8W)) {
 			readsw(iobase + MVSD_FIFO, p, 16);
 			p += 16;
 			s -= 32;
+			sgm->consumed += 32;
 			intr_status = mvsd_read(MVSD_NOR_INTR_STATUS);
 		}
 		/*
@@ -391,6 +424,7 @@ static irqreturn_t mvsd_irq(int irq, void *dev)
 				put_unaligned(mvsd_read(MVSD_FIFO), p++);
 				put_unaligned(mvsd_read(MVSD_FIFO), p++);
 				s -= 4;
+				sgm->consumed += 4;
 				intr_status = mvsd_read(MVSD_NOR_INTR_STATUS);
 			}
 			if (s && s < 4 && (intr_status & MVSD_NOR_RX_READY)) {
@@ -398,10 +432,13 @@ static irqreturn_t mvsd_irq(int irq, void *dev)
 				val[0] = mvsd_read(MVSD_FIFO);
 				val[1] = mvsd_read(MVSD_FIFO);
 				memcpy(p, ((void *)&val) + 4 - s, s);
+				sgm->consumed += s;
 				s = 0;
 				intr_status = mvsd_read(MVSD_NOR_INTR_STATUS);
 			}
-			if (s == 0) {
+			/* PIO transfer done */
+			host->pio_size -= sgm->consumed;
+			if (host->pio_size == 0) {
 				host->intr_en &=
 				     ~(MVSD_NOR_RX_READY | MVSD_NOR_RX_FIFO_8W);
 				mvsd_write(MVSD_NOR_INTR_EN, host->intr_en);
@@ -413,14 +450,10 @@ static irqreturn_t mvsd_irq(int irq, void *dev)
 		}
 		dev_dbg(host->dev, "pio %d intr 0x%04x hw_state 0x%04x\n",
 			s, intr_status, mvsd_read(MVSD_HW_STATE));
-		host->pio_ptr = p;
-		host->pio_size = s;
 		irq_handled = 1;
-	} else if (host->pio_size &&
+	} else if (host->use_pio &&
 		   (intr_status & host->intr_en &
 		    (MVSD_NOR_TX_AVAIL | MVSD_NOR_TX_FIFO_8W))) {
-		u16 *p = host->pio_ptr;
-		int s = host->pio_size;
 		/*
 		 * The TX_FIFO_8W bit is unreliable. When set, bursting
 		 * 16 halfwords all at once in the FIFO drops data. Actually
@@ -431,6 +464,7 @@ static irqreturn_t mvsd_irq(int irq, void *dev)
 			mvsd_write(MVSD_FIFO, get_unaligned(p++));
 			mvsd_write(MVSD_FIFO, get_unaligned(p++));
 			s -= 4;
+			sgm->consumed += 4;
 			intr_status = mvsd_read(MVSD_NOR_INTR_STATUS);
 		}
 		if (s < 4) {
@@ -439,10 +473,13 @@ static irqreturn_t mvsd_irq(int irq, void *dev)
 				memcpy(((void *)&val) + 4 - s, p, s);
 				mvsd_write(MVSD_FIFO, val[0]);
 				mvsd_write(MVSD_FIFO, val[1]);
+				sgm->consumed += s;
 				s = 0;
 				intr_status = mvsd_read(MVSD_NOR_INTR_STATUS);
 			}
-			if (s == 0) {
+			/* PIO transfer done */
+			host->pio_size -= sgm->consumed;
+			if (host->pio_size == 0) {
 				host->intr_en &=
 				     ~(MVSD_NOR_TX_AVAIL | MVSD_NOR_TX_FIFO_8W);
 				mvsd_write(MVSD_NOR_INTR_EN, host->intr_en);
@@ -450,8 +487,6 @@ static irqreturn_t mvsd_irq(int irq, void *dev)
 		}
 		dev_dbg(host->dev, "pio %d intr 0x%04x hw_state 0x%04x\n",
 			s, intr_status, mvsd_read(MVSD_HW_STATE));
-		host->pio_ptr = p;
-		host->pio_size = s;
 		irq_handled = 1;
 	}
 

-- 
2.34.1


  parent reply	other threads:[~2024-01-27  0:20 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-27  0:19 [PATCH v2 0/9] mmc: Use proper sg_miter for scatterlists Linus Walleij
2024-01-27  0:19 ` [PATCH v2 1/9] mmc: davinci_mmc: Use sg_miter for PIO Linus Walleij
2024-01-27  0:19 ` [PATCH v2 2/9] mmc: moxart-mmc: Factor out moxart_use_dma() helper Linus Walleij
2024-01-27  0:19 ` [PATCH v2 3/9] mmc: moxart-mmc: Fix accounting in DMA transfer Linus Walleij
2024-01-27  0:19 ` [PATCH v2 4/9] mmc: moxart-mmc: Use sg_miter for PIO Linus Walleij
2024-01-27  0:19 ` Linus Walleij [this message]
2024-01-27  3:51   ` [PATCH v2 5/9] mmc: mvsdio: " Nicolas Pitre
2024-01-27 16:33     ` Linus Walleij
2024-01-27 22:23       ` Nicolas Pitre
2024-01-27  0:19 ` [PATCH v2 6/9] mmc: mxcmmc: " Linus Walleij
2024-01-27  0:19 ` [PATCH v2 7/9] mmc: omap: " Linus Walleij
2024-01-27  0:19 ` [PATCH v2 8/9] mmc: sdhci-esdhc-mcf: Use sg_miter for swapping Linus Walleij
2024-02-21  6:30   ` Adrian Hunter
2024-01-27  0:19 ` [PATCH v2 9/9] mmc: sh_mmcif: Use sg_miter for PIO Linus Walleij
2024-02-20 21:03   ` Geert Uytterhoeven
2024-02-20 23:00     ` Linus Walleij
2024-02-21  9:50       ` Geert Uytterhoeven
2024-02-21 21:25         ` Linus Walleij
2024-02-22  9:20           ` Geert Uytterhoeven

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240127-mmc-proper-kmap-v2-5-d8e732aa97d1@linaro.org \
    --to=linus.walleij@linaro.org \
    --cc=aaro.koskinen@iki.fi \
    --cc=adrian.hunter@intel.com \
    --cc=angelo.dureghello@timesys.com \
    --cc=arnd@arndb.de \
    --cc=axboe@kernel.dk \
    --cc=hch@lst.de \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-mmc@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=ming.lei@redhat.com \
    --cc=nico@fluxnic.net \
    --cc=ulf.hansson@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.