[PATCH 00/14] mmc: Replace kmap_atomic() with kmap_local_page()

[PATCH 00/14] mmc: Replace kmap_atomic() with kmap_local_page()

[Adrian Hunter]

Hi

Here are patches primarily aimed at replacing kmap_atomic() with
kmap_local_page().

kmap_local_page() is equivalent to kmap_atomic() except that it does not
disable page faults or preemption. Where possible kmap_local_page() is
preferred to kmap_atomic() - refer kernel highmem documentation.

In these cases, there is no need to disable page faults or preemption, so
kmap_atomic() is replaced with kmap_local_page(), and, correspondingly,
kunmap_atomic() with kunmap_local().

That work raised the question of why local_irq_{save,restore}() was being
used with k[un]map_atomic().  It turns out to be for legacy reasons that
have gone away.

A long time ago the kmap_atomic API required a slot to be provided which
risked the possibility that other code might use the same slot at the
same time. Disabling interrupts prevented the possibility of an interrupt
handler doing that. However, that went away with
commit 3e4d3af501cc ("mm: stack based kmap_atomic()").

The work is mostly divided into separate patches, to enable separate review
for driver reviewers and to enable separate reverts if necessary.


Adrian Hunter (14):
      mmc: sdhci: Remove local_irq_{save,restore}() around k[un]map_atomic()
      mmc: sdhci: Remove local_irq_{save,restore}() around sg_miter_{next,stop}()
      mmc: sdhci: Replace kmap_atomic() with kmap_local_page()
      mmc: bcm2835: Remove local_irq_{save,restore}() around k[un]map_atomic()
      mmc: bcm2835: Remove local_irq_{save,restore}() around sg_miter_{next,stop}()
      mmc: bcm2835: Replace kmap_atomic() with kmap_local_page()
      mmc: mmc_test: Remove local_irq_{save,restore}() around sg_copy_{from,to}_buffer()
      mmc: tifm_sd: Remove local_irq_{save,restore}() around tifm_sd_transfer_data()
      mmc: tifm_sd: Remove local_irq_{save,restore}() around tifm_sd_bounce_block()
      mmc: tifm_sd: Replace kmap_atomic() with kmap_local_page()
      mmc: tmio_mmc_core: Remove local_irq_{save,restore}() around k[un]map_atomic()
      mmc: tmio_mmc_core: Replace kmap_atomic() with kmap_local_page()
      mmc: au1xmmc: Replace kmap_atomic() with kmap_local_page()
      mmc: wbsd: Replace kmap_atomic() with kmap_local_page()

 drivers/mmc/core/mmc_test.c              |  5 -----
 drivers/mmc/host/au1xmmc.c               |  8 ++++----
 drivers/mmc/host/bcm2835.c               | 12 ++----------
 drivers/mmc/host/renesas_sdhi_sys_dmac.c |  5 ++---
 drivers/mmc/host/sdhci.c                 | 30 ++++++++----------------------
 drivers/mmc/host/tifm_sd.c               | 28 ++++++++++------------------
 drivers/mmc/host/tmio_mmc.h              | 11 ++++-------
 drivers/mmc/host/tmio_mmc_core.c         | 10 ++++------
 drivers/mmc/host/wbsd.c                  | 10 +++++-----
 9 files changed, 39 insertions(+), 80 deletions(-)


Regards
Adrian

[PATCH 01/14] mmc: sdhci: Remove local_irq_{save,restore}() around k[un]map_atomic()

[Adrian Hunter]

A long time ago the kmap_atomic API required a slot to be provided which
risked the possibility that other code might use the same slot at the
same time. Disabling interrupts prevented the possibility of an interrupt
handler doing that. However, that went away with
commit 3e4d3af501cc ("mm: stack based kmap_atomic()").

When the second argument to kmap_atomic was removed by commit 482fce997e14
("mmc: remove the second argument of k[un]map_atomic()"),
local_irq_{save,restore}() should have been removed also.

Remove it now.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index fef03de85b99..0ee20f4beebf 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -705,16 +705,14 @@ static int sdhci_pre_dma_transfer(struct sdhci_host *host,
 	return sg_count;
 }
 
-static char *sdhci_kmap_atomic(struct scatterlist *sg, unsigned long *flags)
+static char *sdhci_kmap_atomic(struct scatterlist *sg)
 {
-	local_irq_save(*flags);
 	return kmap_atomic(sg_page(sg)) + sg->offset;
 }
 
-static void sdhci_kunmap_atomic(void *buffer, unsigned long *flags)
+static void sdhci_kunmap_atomic(void *buffer)
 {
 	kunmap_atomic(buffer);
-	local_irq_restore(*flags);
 }
 
 void sdhci_adma_write_desc(struct sdhci_host *host, void **desc,
@@ -756,7 +754,6 @@ static void sdhci_adma_table_pre(struct sdhci_host *host,
 	struct mmc_data *data, int sg_count)
 {
 	struct scatterlist *sg;
-	unsigned long flags;
 	dma_addr_t addr, align_addr;
 	void *desc, *align;
 	char *buffer;
@@ -788,9 +785,9 @@ static void sdhci_adma_table_pre(struct sdhci_host *host,
 			 SDHCI_ADMA2_MASK;
 		if (offset) {
 			if (data->flags & MMC_DATA_WRITE) {
-				buffer = sdhci_kmap_atomic(sg, &flags);
+				buffer = sdhci_kmap_atomic(sg);
 				memcpy(align, buffer, offset);
-				sdhci_kunmap_atomic(buffer, &flags);
+				sdhci_kunmap_atomic(buffer);
 			}
 
 			/* tran, valid */
@@ -851,7 +848,6 @@ static void sdhci_adma_table_post(struct sdhci_host *host,
 	int i, size;
 	void *align;
 	char *buffer;
-	unsigned long flags;
 
 	if (data->flags & MMC_DATA_READ) {
 		bool has_unaligned = false;
@@ -874,9 +870,9 @@ static void sdhci_adma_table_post(struct sdhci_host *host,
 					size = SDHCI_ADMA2_ALIGN -
 					       (sg_dma_address(sg) & SDHCI_ADMA2_MASK);
 
-					buffer = sdhci_kmap_atomic(sg, &flags);
+					buffer = sdhci_kmap_atomic(sg);
 					memcpy(buffer, align, size);
-					sdhci_kunmap_atomic(buffer, &flags);
+					sdhci_kunmap_atomic(buffer);
 
 					align += SDHCI_ADMA2_ALIGN;
 				}
-- 
2.25.1

[PATCH 02/14] mmc: sdhci: Remove local_irq_{save,restore}() around sg_miter_{next,stop}()

[Adrian Hunter]

sg_miter_next() using an sg_mapping_iter with flag SG_MITER_ATOMIC uses
kmap_atomic() to map pages.

A long time ago the kmap_atomic API required a slot to be provided which
risked the possibility that other code might use the same slot at the
same time. Disabling interrupts prevented the possibility of an interrupt
handler doing that. However, that went away with
commit 3e4d3af501cc ("mm: stack based kmap_atomic()").

Remove local_irq_{save,restore}() around sg_miter_{next,stop}().

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 0ee20f4beebf..31d87ec7d055 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -525,7 +525,6 @@ static inline bool sdhci_has_requests(struct sdhci_host *host)
 
 static void sdhci_read_block_pio(struct sdhci_host *host)
 {
-	unsigned long flags;
 	size_t blksize, len, chunk;
 	u32 scratch;
 	u8 *buf;
@@ -535,8 +534,6 @@ static void sdhci_read_block_pio(struct sdhci_host *host)
 	blksize = host->data->blksz;
 	chunk = 0;
 
-	local_irq_save(flags);
-
 	while (blksize) {
 		BUG_ON(!sg_miter_next(&host->sg_miter));
 
@@ -563,13 +560,10 @@ static void sdhci_read_block_pio(struct sdhci_host *host)
 	}
 
 	sg_miter_stop(&host->sg_miter);
-
-	local_irq_restore(flags);
 }
 
 static void sdhci_write_block_pio(struct sdhci_host *host)
 {
-	unsigned long flags;
 	size_t blksize, len, chunk;
 	u32 scratch;
 	u8 *buf;
@@ -580,8 +574,6 @@ static void sdhci_write_block_pio(struct sdhci_host *host)
 	chunk = 0;
 	scratch = 0;
 
-	local_irq_save(flags);
-
 	while (blksize) {
 		BUG_ON(!sg_miter_next(&host->sg_miter));
 
@@ -608,8 +600,6 @@ static void sdhci_write_block_pio(struct sdhci_host *host)
 	}
 
 	sg_miter_stop(&host->sg_miter);
-
-	local_irq_restore(flags);
 }
 
 static void sdhci_transfer_pio(struct sdhci_host *host)
-- 
2.25.1

[PATCH 03/14] mmc: sdhci: Replace kmap_atomic() with kmap_local_page()

[Adrian Hunter]

kmap_local_page() is equivalent to kmap_atomic() except that it does not
disable page faults or preemption. Where possible kmap_local_page() is
preferred to kmap_atomic() - refer kernel highmem documentation.

In this case, there is no need to disable page faults or preemption, so
replace kmap_atomic() with kmap_local_page(), and, correspondingly,
kunmap_atomic() with kunmap_local().

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 31d87ec7d055..fb6e9a81f198 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -697,12 +697,12 @@ static int sdhci_pre_dma_transfer(struct sdhci_host *host,
 
 static char *sdhci_kmap_atomic(struct scatterlist *sg)
 {
-	return kmap_atomic(sg_page(sg)) + sg->offset;
+	return kmap_local_page(sg_page(sg)) + sg->offset;
 }
 
 static void sdhci_kunmap_atomic(void *buffer)
 {
-	kunmap_atomic(buffer);
+	kunmap_local(buffer);
 }
 
 void sdhci_adma_write_desc(struct sdhci_host *host, void **desc,
-- 
2.25.1

[PATCH 04/14] mmc: bcm2835: Remove local_irq_{save,restore}() around k[un]map_atomic()

[Adrian Hunter]

A long time ago the kmap_atomic API required a slot to be provided which
risked the possibility that other code might use the same slot at the
same time. Disabling interrupts prevented the possibility of an interrupt
handler doing that. However, that went away with
commit 3e4d3af501cc ("mm: stack based kmap_atomic()").

Unfortunately, that unnecessary pattern of code has been copied since
and persists in bcm2385.c.

Remove it.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/bcm2835.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/mmc/host/bcm2835.c b/drivers/mmc/host/bcm2835.c
index 641ab4f42125..49acbfa87ab8 100644
--- a/drivers/mmc/host/bcm2835.c
+++ b/drivers/mmc/host/bcm2835.c
@@ -1068,7 +1068,6 @@ static void bcm2835_dma_complete_work(struct work_struct *work)
 	}
 
 	if (host->drain_words) {
-		unsigned long flags;
 		void *page;
 		u32 *buf;
 
@@ -1076,7 +1075,6 @@ static void bcm2835_dma_complete_work(struct work_struct *work)
 			host->drain_page += host->drain_offset >> PAGE_SHIFT;
 			host->drain_offset &= ~PAGE_MASK;
 		}
-		local_irq_save(flags);
 		page = kmap_atomic(host->drain_page);
 		buf = page + host->drain_offset;
 
@@ -1089,7 +1087,6 @@ static void bcm2835_dma_complete_work(struct work_struct *work)
 		}
 
 		kunmap_atomic(page);
-		local_irq_restore(flags);
 	}
 
 	bcm2835_finish_data(host);
-- 
2.25.1

[PATCH 05/14] mmc: bcm2835: Remove local_irq_{save,restore}() around sg_miter_{next,stop}()

[Adrian Hunter]

sg_miter_next() using an sg_mapping_iter with flag SG_MITER_ATOMIC uses
kmap_atomic() to map pages.

A long time ago the kmap_atomic API required a slot to be provided which
risked the possibility that other code might use the same slot at the
same time. Disabling interrupts prevented the possibility of an interrupt
handler doing that. However, that went away with
commit 3e4d3af501cc ("mm: stack based kmap_atomic()").

Remove local_irq_{save,restore}() around sg_miter_{next,stop}().

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/bcm2835.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/mmc/host/bcm2835.c b/drivers/mmc/host/bcm2835.c
index 49acbfa87ab8..440773e3ba55 100644
--- a/drivers/mmc/host/bcm2835.c
+++ b/drivers/mmc/host/bcm2835.c
@@ -327,7 +327,6 @@ static void bcm2835_dma_complete(void *param)
 
 static void bcm2835_transfer_block_pio(struct bcm2835_host *host, bool is_read)
 {
-	unsigned long flags;
 	size_t blksize;
 	unsigned long wait_max;
 
@@ -335,8 +334,6 @@ static void bcm2835_transfer_block_pio(struct bcm2835_host *host, bool is_read)
 
 	wait_max = jiffies + msecs_to_jiffies(500);
 
-	local_irq_save(flags);
-
 	while (blksize) {
 		int copy_words;
 		u32 hsts = 0;
@@ -421,8 +418,6 @@ static void bcm2835_transfer_block_pio(struct bcm2835_host *host, bool is_read)
 	}
 
 	sg_miter_stop(&host->sg_miter);
-
-	local_irq_restore(flags);
 }
 
 static void bcm2835_transfer_pio(struct bcm2835_host *host)
-- 
2.25.1

[PATCH 06/14] mmc: bcm2835: Replace kmap_atomic() with kmap_local_page()

[Adrian Hunter]

kmap_local_page() is equivalent to kmap_atomic() except that it does not
disable page faults or preemption. Where possible kmap_local_page() is
preferred to kmap_atomic() - refer kernel highmem documentation.

In this case, there is no need to disable page faults or preemption, so
replace kmap_atomic() with kmap_local_page(), and, correspondingly,
kunmap_atomic() with kunmap_local().

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/bcm2835.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/bcm2835.c b/drivers/mmc/host/bcm2835.c
index 440773e3ba55..8648f7e63ca1 100644
--- a/drivers/mmc/host/bcm2835.c
+++ b/drivers/mmc/host/bcm2835.c
@@ -1070,7 +1070,7 @@ static void bcm2835_dma_complete_work(struct work_struct *work)
 			host->drain_page += host->drain_offset >> PAGE_SHIFT;
 			host->drain_offset &= ~PAGE_MASK;
 		}
-		page = kmap_atomic(host->drain_page);
+		page = kmap_local_page(host->drain_page);
 		buf = page + host->drain_offset;
 
 		while (host->drain_words) {
@@ -1081,7 +1081,7 @@ static void bcm2835_dma_complete_work(struct work_struct *work)
 			host->drain_words--;
 		}
 
-		kunmap_atomic(page);
+		kunmap_local(page);
 	}
 
 	bcm2835_finish_data(host);
-- 
2.25.1

[PATCH 07/14] mmc: mmc_test: Remove local_irq_{save,restore}() around sg_copy_{from,to}_buffer()

[Adrian Hunter]

sg_copy_{from,to}_buffer() call sg_copy_buffer() which uses an
sg_mapping_iter with flag SG_MITER_ATOMIC, so then sg_miter_next() uses
kmap_atomic() to map pages.

A long time ago the kmap_atomic API required a slot to be provided which
risked the possibility that other code might use the same slot at the
same time. Disabling interrupts prevented the possibility of an interrupt
handler doing that. However, that went away with
commit 3e4d3af501cc ("mm: stack based kmap_atomic()").

Remove local_irq_{save,restore}() around sg_copy_{from,to}_buffer().

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/core/mmc_test.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/mmc/core/mmc_test.c b/drivers/mmc/core/mmc_test.c
index 8d9bceeff986..6cd6f8a94a71 100644
--- a/drivers/mmc/core/mmc_test.c
+++ b/drivers/mmc/core/mmc_test.c
@@ -932,7 +932,6 @@ static int mmc_test_transfer(struct mmc_test_card *test,
 	unsigned blocks, unsigned blksz, int write)
 {
 	int ret, i;
-	unsigned long flags;
 
 	if (write) {
 		for (i = 0; i < blocks * blksz; i++)
@@ -940,9 +939,7 @@ static int mmc_test_transfer(struct mmc_test_card *test,
 	} else {
 		memset(test->scratch, 0, BUFFER_SIZE);
 	}
-	local_irq_save(flags);
 	sg_copy_from_buffer(sg, sg_len, test->scratch, BUFFER_SIZE);
-	local_irq_restore(flags);
 
 	ret = mmc_test_set_blksize(test, blksz);
 	if (ret)
@@ -987,9 +984,7 @@ static int mmc_test_transfer(struct mmc_test_card *test,
 				return RESULT_FAIL;
 		}
 	} else {
-		local_irq_save(flags);
 		sg_copy_to_buffer(sg, sg_len, test->scratch, BUFFER_SIZE);
-		local_irq_restore(flags);
 		for (i = 0; i < blocks * blksz; i++) {
 			if (test->scratch[i] != (u8)i)
 				return RESULT_FAIL;
-- 
2.25.1

[PATCH 08/14] mmc: tifm_sd: Remove local_irq_{save,restore}() around tifm_sd_transfer_data()

[Adrian Hunter]

tifm_sd_transfer_data() calls functions that ultimate use kmap_atomic() to
map pages.

A long time ago the kmap_atomic API required a slot to be provided which
risked the possibility that other code might use the same slot at the
same time. Disabling interrupts prevented the possibility of an interrupt
handler doing that. However, that went away with
commit 3e4d3af501cc ("mm: stack based kmap_atomic()").

When the second argument to kmap_atomic was removed by commit 482fce997e14
("mmc: remove the second argument of k[un]map_atomic()"),
local_irq_{save,restore}() should have been removed also.

Remove it now.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/tifm_sd.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/mmc/host/tifm_sd.c b/drivers/mmc/host/tifm_sd.c
index 63917070b1a7..ed1685add446 100644
--- a/drivers/mmc/host/tifm_sd.c
+++ b/drivers/mmc/host/tifm_sd.c
@@ -506,7 +506,6 @@ static void tifm_sd_card_event(struct tifm_dev *sock)
 	unsigned int host_status = 0;
 	int cmd_error = 0;
 	struct mmc_command *cmd = NULL;
-	unsigned long flags;
 
 	spin_lock(&sock->lock);
 	host = mmc_priv((struct mmc_host*)tifm_get_drvdata(sock));
@@ -570,9 +569,7 @@ static void tifm_sd_card_event(struct tifm_dev *sock)
 
 			if (host_status & (TIFM_MMCSD_AE | TIFM_MMCSD_AF
 					   | TIFM_MMCSD_BRS)) {
-				local_irq_save(flags);
 				tifm_sd_transfer_data(host);
-				local_irq_restore(flags);
 				host_status &= ~TIFM_MMCSD_AE;
 			}
 		}
-- 
2.25.1

[PATCH 09/14] mmc: tifm_sd: Remove local_irq_{save,restore}() around tifm_sd_bounce_block()

[Adrian Hunter]

tifm_sd_bounce_block() calls functions that ultimate use kmap_atomic() to
map pages.

A long time ago the kmap_atomic API required a slot to be provided which
risked the possibility that other code might use the same slot at the
same time. Disabling interrupts prevented the possibility of an interrupt
handler doing that. However, that went away with
commit 3e4d3af501cc ("mm: stack based kmap_atomic()").

When the second argument to kmap_atomic was removed by commit 482fce997e14
("mmc: remove the second argument of k[un]map_atomic()"),
local_irq_{save,restore}() should have been removed also.

Remove it now.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/tifm_sd.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/host/tifm_sd.c b/drivers/mmc/host/tifm_sd.c
index ed1685add446..d539f9b48422 100644
--- a/drivers/mmc/host/tifm_sd.c
+++ b/drivers/mmc/host/tifm_sd.c
@@ -264,16 +264,13 @@ static int tifm_sd_set_dma_data(struct tifm_sd *host, struct mmc_data *r_data)
 	unsigned int t_size = TIFM_DMA_TSIZE * r_data->blksz;
 	unsigned int dma_len, dma_blk_cnt, dma_off;
 	struct scatterlist *sg = NULL;
-	unsigned long flags;
 
 	if (host->sg_pos == host->sg_len)
 		return 1;
 
 	if (host->cmd_flags & DATA_CARRY) {
 		host->cmd_flags &= ~DATA_CARRY;
-		local_irq_save(flags);
 		tifm_sd_bounce_block(host, r_data);
-		local_irq_restore(flags);
 		if (host->sg_pos == host->sg_len)
 			return 1;
 	}
@@ -300,11 +297,9 @@ static int tifm_sd_set_dma_data(struct tifm_sd *host, struct mmc_data *r_data)
 	if (dma_blk_cnt)
 		sg = &r_data->sg[host->sg_pos];
 	else if (dma_len) {
-		if (r_data->flags & MMC_DATA_WRITE) {
-			local_irq_save(flags);
+		if (r_data->flags & MMC_DATA_WRITE)
 			tifm_sd_bounce_block(host, r_data);
-			local_irq_restore(flags);
-		} else
+		else
 			host->cmd_flags |= DATA_CARRY;
 
 		sg = &host->bounce_buf;
-- 
2.25.1

[PATCH 10/14] mmc: tifm_sd: Replace kmap_atomic() with kmap_local_page()

[Adrian Hunter]

kmap_local_page() is equivalent to kmap_atomic() except that it does not
disable page faults or preemption. Where possible kmap_local_page() is
preferred to kmap_atomic() - refer kernel highmem documentation.

In this case, there is no need to disable page faults or preemption, so
replace kmap_atomic() with kmap_local_page(), and, correspondingly,
kunmap_atomic() with kunmap_local().

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/tifm_sd.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/tifm_sd.c b/drivers/mmc/host/tifm_sd.c
index d539f9b48422..b5a2f2f25ad9 100644
--- a/drivers/mmc/host/tifm_sd.c
+++ b/drivers/mmc/host/tifm_sd.c
@@ -116,7 +116,7 @@ static void tifm_sd_read_fifo(struct tifm_sd *host, struct page *pg,
 	unsigned char *buf;
 	unsigned int pos = 0, val;
 
-	buf = kmap_atomic(pg) + off;
+	buf = kmap_local_page(pg) + off;
 	if (host->cmd_flags & DATA_CARRY) {
 		buf[pos++] = host->bounce_buf_data[0];
 		host->cmd_flags &= ~DATA_CARRY;
@@ -132,7 +132,7 @@ static void tifm_sd_read_fifo(struct tifm_sd *host, struct page *pg,
 		}
 		buf[pos++] = (val >> 8) & 0xff;
 	}
-	kunmap_atomic(buf - off);
+	kunmap_local(buf - off);
 }
 
 static void tifm_sd_write_fifo(struct tifm_sd *host, struct page *pg,
@@ -142,7 +142,7 @@ static void tifm_sd_write_fifo(struct tifm_sd *host, struct page *pg,
 	unsigned char *buf;
 	unsigned int pos = 0, val;
 
-	buf = kmap_atomic(pg) + off;
+	buf = kmap_local_page(pg) + off;
 	if (host->cmd_flags & DATA_CARRY) {
 		val = host->bounce_buf_data[0] | ((buf[pos++] << 8) & 0xff00);
 		writel(val, sock->addr + SOCK_MMCSD_DATA);
@@ -159,7 +159,7 @@ static void tifm_sd_write_fifo(struct tifm_sd *host, struct page *pg,
 		val |= (buf[pos++] << 8) & 0xff00;
 		writel(val, sock->addr + SOCK_MMCSD_DATA);
 	}
-	kunmap_atomic(buf - off);
+	kunmap_local(buf - off);
 }
 
 static void tifm_sd_transfer_data(struct tifm_sd *host)
@@ -210,13 +210,13 @@ static void tifm_sd_copy_page(struct page *dst, unsigned int dst_off,
 			      struct page *src, unsigned int src_off,
 			      unsigned int count)
 {
-	unsigned char *src_buf = kmap_atomic(src) + src_off;
-	unsigned char *dst_buf = kmap_atomic(dst) + dst_off;
+	unsigned char *src_buf = kmap_local_page(src) + src_off;
+	unsigned char *dst_buf = kmap_local_page(dst) + dst_off;
 
 	memcpy(dst_buf, src_buf, count);
 
-	kunmap_atomic(dst_buf - dst_off);
-	kunmap_atomic(src_buf - src_off);
+	kunmap_local(dst_buf - dst_off);
+	kunmap_local(src_buf - src_off);
 }
 
 static void tifm_sd_bounce_block(struct tifm_sd *host, struct mmc_data *r_data)
-- 
2.25.1

[PATCH 11/14] mmc: tmio_mmc_core: Remove local_irq_{save,restore}() around k[un]map_atomic()

[Adrian Hunter]

A long time ago the kmap_atomic API required a slot to be provided which
risked the possibility that other code might use the same slot at the
same time. Disabling interrupts prevented the possibility of an interrupt
handler doing that. However, that went away with
commit 3e4d3af501cc ("mm: stack based kmap_atomic()").

When the second argument to kmap_atomic was removed by commit 482fce997e14
("mmc: remove the second argument of k[un]map_atomic()"),
local_irq_{save,restore}() should have been removed also.

Remove it now.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/renesas_sdhi_sys_dmac.c |  5 ++---
 drivers/mmc/host/tmio_mmc.h              |  7 ++-----
 drivers/mmc/host/tmio_mmc_core.c         | 10 ++++------
 3 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/mmc/host/renesas_sdhi_sys_dmac.c b/drivers/mmc/host/renesas_sdhi_sys_dmac.c
index 99e3426df702..e9cc6c15d229 100644
--- a/drivers/mmc/host/renesas_sdhi_sys_dmac.c
+++ b/drivers/mmc/host/renesas_sdhi_sys_dmac.c
@@ -254,12 +254,11 @@ static void renesas_sdhi_sys_dmac_start_dma_tx(struct tmio_mmc_host *host)
 
 	/* The only sg element can be unaligned, use our bounce buffer then */
 	if (!aligned) {
-		unsigned long flags;
-		void *sg_vaddr = tmio_mmc_kmap_atomic(sg, &flags);
+		void *sg_vaddr = tmio_mmc_kmap_atomic(sg);
 
 		sg_init_one(&host->bounce_sg, host->bounce_buf, sg->length);
 		memcpy(host->bounce_buf, sg_vaddr, host->bounce_sg.length);
-		tmio_mmc_kunmap_atomic(sg, &flags, sg_vaddr);
+		tmio_mmc_kunmap_atomic(sg, sg_vaddr);
 		host->sg_ptr = &host->bounce_sg;
 		sg = host->sg_ptr;
 	}
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 501613c74406..22375790b57b 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -204,18 +204,15 @@ void tmio_mmc_enable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
 void tmio_mmc_disable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
 irqreturn_t tmio_mmc_irq(int irq, void *devid);
 
-static inline char *tmio_mmc_kmap_atomic(struct scatterlist *sg,
-					 unsigned long *flags)
+static inline char *tmio_mmc_kmap_atomic(struct scatterlist *sg)
 {
-	local_irq_save(*flags);
 	return kmap_atomic(sg_page(sg)) + sg->offset;
 }
 
 static inline void tmio_mmc_kunmap_atomic(struct scatterlist *sg,
-					  unsigned long *flags, void *virt)
+					  void *virt)
 {
 	kunmap_atomic(virt - sg->offset);
-	local_irq_restore(*flags);
 }
 
 #ifdef CONFIG_PM
diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index 437048bb8027..6d50c0dd53fe 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -412,7 +412,6 @@ static void tmio_mmc_pio_irq(struct tmio_mmc_host *host)
 	void *sg_virt;
 	unsigned short *buf;
 	unsigned int count;
-	unsigned long flags;
 
 	if (host->dma_on) {
 		pr_err("PIO IRQ in DMA mode!\n");
@@ -422,7 +421,7 @@ static void tmio_mmc_pio_irq(struct tmio_mmc_host *host)
 		return;
 	}
 
-	sg_virt = tmio_mmc_kmap_atomic(host->sg_ptr, &flags);
+	sg_virt = tmio_mmc_kmap_atomic(host->sg_ptr);
 	buf = (unsigned short *)(sg_virt + host->sg_off);
 
 	count = host->sg_ptr->length - host->sg_off;
@@ -437,7 +436,7 @@ static void tmio_mmc_pio_irq(struct tmio_mmc_host *host)
 
 	host->sg_off += count;
 
-	tmio_mmc_kunmap_atomic(host->sg_ptr, &flags, sg_virt);
+	tmio_mmc_kunmap_atomic(host->sg_ptr, sg_virt);
 
 	if (host->sg_off == host->sg_ptr->length)
 		tmio_mmc_next_sg(host);
@@ -446,11 +445,10 @@ static void tmio_mmc_pio_irq(struct tmio_mmc_host *host)
 static void tmio_mmc_check_bounce_buffer(struct tmio_mmc_host *host)
 {
 	if (host->sg_ptr == &host->bounce_sg) {
-		unsigned long flags;
-		void *sg_vaddr = tmio_mmc_kmap_atomic(host->sg_orig, &flags);
+		void *sg_vaddr = tmio_mmc_kmap_atomic(host->sg_orig);
 
 		memcpy(sg_vaddr, host->bounce_buf, host->bounce_sg.length);
-		tmio_mmc_kunmap_atomic(host->sg_orig, &flags, sg_vaddr);
+		tmio_mmc_kunmap_atomic(host->sg_orig, sg_vaddr);
 	}
 }
 
-- 
2.25.1

[PATCH 12/14] mmc: tmio_mmc_core: Replace kmap_atomic() with kmap_local_page()

[Adrian Hunter]

kmap_local_page() is equivalent to kmap_atomic() except that it does not
disable page faults or preemption. Where possible kmap_local_page() is
preferred to kmap_atomic() - refer kernel highmem documentation.

In this case, there is no need to disable page faults or preemption, so
replace kmap_atomic() with kmap_local_page(), and, correspondingly,
kunmap_atomic() with kunmap_local().

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/tmio_mmc.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 22375790b57b..e36ff80108e6 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -206,13 +206,13 @@ irqreturn_t tmio_mmc_irq(int irq, void *devid);
 
 static inline char *tmio_mmc_kmap_atomic(struct scatterlist *sg)
 {
-	return kmap_atomic(sg_page(sg)) + sg->offset;
+	return kmap_local_page(sg_page(sg)) + sg->offset;
 }
 
 static inline void tmio_mmc_kunmap_atomic(struct scatterlist *sg,
 					  void *virt)
 {
-	kunmap_atomic(virt - sg->offset);
+	kunmap_local(virt - sg->offset);
 }
 
 #ifdef CONFIG_PM
-- 
2.25.1

[PATCH 13/14] mmc: au1xmmc: Replace kmap_atomic() with kmap_local_page()

[Adrian Hunter]

kmap_local_page() is equivalent to kmap_atomic() except that it does not
disable page faults or preemption. Where possible kmap_local_page() is
preferred to kmap_atomic() - refer kernel highmem documentation.

In this case, there is no need to disable page faults or preemption, so
replace kmap_atomic() with kmap_local_page(), and, correspondingly,
kunmap_atomic() with kunmap_local().

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/au1xmmc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/au1xmmc.c b/drivers/mmc/host/au1xmmc.c
index c88b039dc9fb..82dd0ae40305 100644
--- a/drivers/mmc/host/au1xmmc.c
+++ b/drivers/mmc/host/au1xmmc.c
@@ -388,7 +388,7 @@ static void au1xmmc_send_pio(struct au1xmmc_host *host)
 
 	/* This is the pointer to the data buffer */
 	sg = &data->sg[host->pio.index];
-	sg_ptr = kmap_atomic(sg_page(sg)) + sg->offset + host->pio.offset;
+	sg_ptr = kmap_local_page(sg_page(sg)) + sg->offset + host->pio.offset;
 
 	/* This is the space left inside the buffer */
 	sg_len = data->sg[host->pio.index].length - host->pio.offset;
@@ -409,7 +409,7 @@ static void au1xmmc_send_pio(struct au1xmmc_host *host)
 		__raw_writel((unsigned long)val, HOST_TXPORT(host));
 		wmb(); /* drain writebuffer */
 	}
-	kunmap_atomic(sg_ptr);
+	kunmap_local(sg_ptr);
 
 	host->pio.len -= count;
 	host->pio.offset += count;
@@ -446,7 +446,7 @@ static void au1xmmc_receive_pio(struct au1xmmc_host *host)
 
 	if (host->pio.index < host->dma.len) {
 		sg = &data->sg[host->pio.index];
-		sg_ptr = kmap_atomic(sg_page(sg)) + sg->offset + host->pio.offset;
+		sg_ptr = kmap_local_page(sg_page(sg)) + sg->offset + host->pio.offset;
 
 		/* This is the space left inside the buffer */
 		sg_len = sg_dma_len(&data->sg[host->pio.index]) - host->pio.offset;
@@ -488,7 +488,7 @@ static void au1xmmc_receive_pio(struct au1xmmc_host *host)
 			sg_ptr[count] = (unsigned char)(val & 0xFF);
 	}
 	if (sg_ptr)
-		kunmap_atomic(sg_ptr);
+		kunmap_local(sg_ptr);
 
 	host->pio.len -= count;
 	host->pio.offset += count;
-- 
2.25.1

[PATCH 14/14] mmc: wbsd: Replace kmap_atomic() with kmap_local_page()

[Adrian Hunter]

kmap_local_page() is equivalent to kmap_atomic() except that it does not
disable page faults or preemption. Where possible kmap_local_page() is
preferred to kmap_atomic() - refer kernel highmem documentation.

In this case, there is no need to disable page faults or preemption, so
replace kmap_atomic() with kmap_local_page(), and, correspondingly,
kunmap_atomic() with kunmap_local().

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/wbsd.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/wbsd.c b/drivers/mmc/host/wbsd.c
index 67ecd342fe5f..2219144c166a 100644
--- a/drivers/mmc/host/wbsd.c
+++ b/drivers/mmc/host/wbsd.c
@@ -267,7 +267,7 @@ static inline int wbsd_next_sg(struct wbsd_host *host)
 
 static inline char *wbsd_map_sg(struct wbsd_host *host)
 {
-	return kmap_atomic(sg_page(host->cur_sg)) + host->cur_sg->offset;
+	return kmap_local_page(sg_page(host->cur_sg)) + host->cur_sg->offset;
 }
 
 static inline void wbsd_sg_to_dma(struct wbsd_host *host, struct mmc_data *data)
@@ -439,7 +439,7 @@ static void wbsd_empty_fifo(struct wbsd_host *host)
 			 * End of scatter list entry?
 			 */
 			if (host->remain == 0) {
-				kunmap_atomic(buffer);
+				kunmap_local(buffer);
 				/*
 				 * Get next entry. Check if last.
 				 */
@@ -451,7 +451,7 @@ static void wbsd_empty_fifo(struct wbsd_host *host)
 			}
 		}
 	}
-	kunmap_atomic(buffer);
+	kunmap_local(buffer);
 
 	/*
 	 * This is a very dirty hack to solve a
@@ -505,7 +505,7 @@ static void wbsd_fill_fifo(struct wbsd_host *host)
 			 * End of scatter list entry?
 			 */
 			if (host->remain == 0) {
-				kunmap_atomic(buffer);
+				kunmap_local(buffer);
 				/*
 				 * Get next entry. Check if last.
 				 */
@@ -517,7 +517,7 @@ static void wbsd_fill_fifo(struct wbsd_host *host)
 			}
 		}
 	}
-	kunmap_atomic(buffer);
+	kunmap_local(buffer);
 
 	/*
 	 * The controller stops sending interrupts for
-- 
2.25.1

RE: [PATCH 03/14] mmc: sdhci: Replace kmap_atomic() with kmap_local_page()

[Avri Altman]

> kmap_local_page() is equivalent to kmap_atomic() except that it does not
> disable page faults or preemption. Where possible kmap_local_page() is
> preferred to kmap_atomic() - refer kernel highmem documentation.
> 
> In this case, there is no need to disable page faults or preemption, so replace
> kmap_atomic() with kmap_local_page(), and, correspondingly,
> kunmap_atomic() with kunmap_local().
> 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>  drivers/mmc/host/sdhci.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index
> 31d87ec7d055..fb6e9a81f198 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -697,12 +697,12 @@ static int sdhci_pre_dma_transfer(struct sdhci_host
> *host,
> 
>  static char *sdhci_kmap_atomic(struct scatterlist *sg)  {
> -       return kmap_atomic(sg_page(sg)) + sg->offset;
> +       return kmap_local_page(sg_page(sg)) + sg->offset;
>  }
> 
>  static void sdhci_kunmap_atomic(void *buffer)  {
> -       kunmap_atomic(buffer);
> +       kunmap_local(buffer);
kmap_local_page documentation, indicates that the unmapping should be done in reverse order.
Isn't it something that the callers of sdhci_kunmap_atomic now needs to attend?

Thanks,
Avri

>  }
> 
>  void sdhci_adma_write_desc(struct sdhci_host *host, void **desc,
> --
> 2.25.1

Re: [PATCH 03/14] mmc: sdhci: Replace kmap_atomic() with kmap_local_page()

[Adrian Hunter]

On 6/10/22 10:03, Avri Altman wrote:
>> kmap_local_page() is equivalent to kmap_atomic() except that it does not
>> disable page faults or preemption. Where possible kmap_local_page() is
>> preferred to kmap_atomic() - refer kernel highmem documentation.
>>
>> In this case, there is no need to disable page faults or preemption, so replace
>> kmap_atomic() with kmap_local_page(), and, correspondingly,
>> kunmap_atomic() with kunmap_local().
>>
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>> ---
>>  drivers/mmc/host/sdhci.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index
>> 31d87ec7d055..fb6e9a81f198 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -697,12 +697,12 @@ static int sdhci_pre_dma_transfer(struct sdhci_host
>> *host,
>>
>>  static char *sdhci_kmap_atomic(struct scatterlist *sg)  {
>> -       return kmap_atomic(sg_page(sg)) + sg->offset;
>> +       return kmap_local_page(sg_page(sg)) + sg->offset;
>>  }
>>
>>  static void sdhci_kunmap_atomic(void *buffer)  {
>> -       kunmap_atomic(buffer);
>> +       kunmap_local(buffer);
> kmap_local_page documentation, indicates that the unmapping should be done in reverse order.
> Isn't it something that the callers of sdhci_kunmap_atomic now needs to attend?

Nesting is strictly ordered in either case, so there is no change there.
i.e. the map management is stack based

Re: [PATCH 00/14] mmc: Replace kmap_atomic() with kmap_local_page()

[Ulf Hansson]

On Wed, 5 Oct 2022 at 12:20, Adrian Hunter <adrian.hunter@intel.com> wrote:
>
> Hi
>
> Here are patches primarily aimed at replacing kmap_atomic() with
> kmap_local_page().
>
> kmap_local_page() is equivalent to kmap_atomic() except that it does not
> disable page faults or preemption. Where possible kmap_local_page() is
> preferred to kmap_atomic() - refer kernel highmem documentation.
>
> In these cases, there is no need to disable page faults or preemption, so
> kmap_atomic() is replaced with kmap_local_page(), and, correspondingly,
> kunmap_atomic() with kunmap_local().
>
> That work raised the question of why local_irq_{save,restore}() was being
> used with k[un]map_atomic().  It turns out to be for legacy reasons that
> have gone away.
>
> A long time ago the kmap_atomic API required a slot to be provided which
> risked the possibility that other code might use the same slot at the
> same time. Disabling interrupts prevented the possibility of an interrupt
> handler doing that. However, that went away with
> commit 3e4d3af501cc ("mm: stack based kmap_atomic()").
>
> The work is mostly divided into separate patches, to enable separate review
> for driver reviewers and to enable separate reverts if necessary.
>
>
> Adrian Hunter (14):
>       mmc: sdhci: Remove local_irq_{save,restore}() around k[un]map_atomic()
>       mmc: sdhci: Remove local_irq_{save,restore}() around sg_miter_{next,stop}()
>       mmc: sdhci: Replace kmap_atomic() with kmap_local_page()
>       mmc: bcm2835: Remove local_irq_{save,restore}() around k[un]map_atomic()
>       mmc: bcm2835: Remove local_irq_{save,restore}() around sg_miter_{next,stop}()
>       mmc: bcm2835: Replace kmap_atomic() with kmap_local_page()
>       mmc: mmc_test: Remove local_irq_{save,restore}() around sg_copy_{from,to}_buffer()
>       mmc: tifm_sd: Remove local_irq_{save,restore}() around tifm_sd_transfer_data()
>       mmc: tifm_sd: Remove local_irq_{save,restore}() around tifm_sd_bounce_block()
>       mmc: tifm_sd: Replace kmap_atomic() with kmap_local_page()
>       mmc: tmio_mmc_core: Remove local_irq_{save,restore}() around k[un]map_atomic()
>       mmc: tmio_mmc_core: Replace kmap_atomic() with kmap_local_page()
>       mmc: au1xmmc: Replace kmap_atomic() with kmap_local_page()
>       mmc: wbsd: Replace kmap_atomic() with kmap_local_page()
>
>  drivers/mmc/core/mmc_test.c              |  5 -----
>  drivers/mmc/host/au1xmmc.c               |  8 ++++----
>  drivers/mmc/host/bcm2835.c               | 12 ++----------
>  drivers/mmc/host/renesas_sdhi_sys_dmac.c |  5 ++---
>  drivers/mmc/host/sdhci.c                 | 30 ++++++++----------------------
>  drivers/mmc/host/tifm_sd.c               | 28 ++++++++++------------------
>  drivers/mmc/host/tmio_mmc.h              | 11 ++++-------
>  drivers/mmc/host/tmio_mmc_core.c         | 10 ++++------
>  drivers/mmc/host/wbsd.c                  | 10 +++++-----
>  9 files changed, 39 insertions(+), 80 deletions(-)
>
>
> Regards
> Adrian

Applied for next, thanks!

Kind regards
Uffe

Re: [PATCH 11/14] mmc: tmio_mmc_core: Remove local_irq_{save,restore}() around k[un]map_atomic()

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 873 bytes --]

On Wed, Oct 05, 2022 at 01:19:48PM +0300, Adrian Hunter wrote:
> A long time ago the kmap_atomic API required a slot to be provided which
> risked the possibility that other code might use the same slot at the
> same time. Disabling interrupts prevented the possibility of an interrupt
> handler doing that. However, that went away with
> commit 3e4d3af501cc ("mm: stack based kmap_atomic()").
> 
> When the second argument to kmap_atomic was removed by commit 482fce997e14
> ("mmc: remove the second argument of k[un]map_atomic()"),
> local_irq_{save,restore}() should have been removed also.
> 
> Remove it now.
> 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

Thank you for cleaning this cruft! Looks good.

Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

I should be able to set up a board for testing this week as well.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 12/14] mmc: tmio_mmc_core: Replace kmap_atomic() with kmap_local_page()

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 641 bytes --]

On Wed, Oct 05, 2022 at 01:19:49PM +0300, Adrian Hunter wrote:
> kmap_local_page() is equivalent to kmap_atomic() except that it does not
> disable page faults or preemption. Where possible kmap_local_page() is
> preferred to kmap_atomic() - refer kernel highmem documentation.
> 
> In this case, there is no need to disable page faults or preemption, so
> replace kmap_atomic() with kmap_local_page(), and, correspondingly,
> kunmap_atomic() with kunmap_local().
> 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

Not disabling preemption sounds good!

Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 11/14] mmc: tmio_mmc_core: Remove local_irq_{save,restore}() around k[un]map_atomic()

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 315 bytes --]


Okay, testing turned out to be not so easy because injecting working
non-aligned buffers seems not as trivial as I thought and would require
more time than I can afford for these old systems. Some light testing
did work, though, but this is not enough for a Tested-by tag. The Rev-by
tags still stand, of course.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]