Hi Linus,
On Sat, 27 Jan 2024, Linus Walleij wrote:
Use sg_miter iterator instead of sg_virt() and custom code
to loop over the scatterlist. The memory iterator will do
bounce buffering if the page happens to be located in high memory,
which the driver may or may not be using.
Suggested-by: Christoph Hellwig <hch@xxxxxx>
Link: https://lore.kernel.org/linux-mmc/20240122073423.GA25859@xxxxxx/
Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
Thanks for your patch, which is now commit 27b57277d9ba3a90 ("mmc:
sh_mmcif: Use sg_miter for PIO") in mmc/next.
I have bisected the following BUGs on R-Mobile APE6 (also seen on
R-Mobile A1 and SH-Mobile AG5) to this commit:
sh_mobile_sdhi ee120000.mmc: mmc1 base at 0xee120000, max clock rate 12 MHz
mmc2: new high speed MMC card at address 0001
sh_mobile_sdhi ee100000.mmc: mmc0 base at 0xee100000, max clock rate 88 MHz
mmcblk2: mmc2:0001 MMC08G 7.33 GiB
BUG: sleeping function called from invalid context at kernel/workqueue.c:3347
in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 35, name: irq/151-ee20000
preempt_count: 1, expected: 0
no locks held by irq/151-ee20000/35.
CPU: 0 PID: 35 Comm: irq/151-ee20000 Not tainted 6.8.0-rc4-ape6evm-00017-g27b57277d9ba #491
Hardware name: Generic R8A73A4 (Flattened Device Tree)
unwind_backtrace from show_stack+0x10/0x14
show_stack from dump_stack_lvl+0x68/0x90
dump_stack_lvl from __might_resched+0x1ac/0x228
__might_resched from __flush_work+0x20c/0x2e4
__flush_work from __cancel_work_timer+0x118/0x198
__cancel_work_timer from sh_mmcif_irqt+0x38/0x8f8
sh_mmcif_irqt from irq_thread_fn+0x1c/0x58
irq_thread_fn from irq_thread+0x10c/0x218
irq_thread from kthread+0xf0/0x100
kthread from ret_from_fork+0x14/0x28
Exception stack(0xf0959fb0 to 0xf0959ff8)
9fa0: 00000000 00000000 00000000 00000000
9fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
9fe0: 00000000 00000000 00000000 00000000 00000013 00000000
BUG: scheduling while atomic: irq/151-ee20000/35/0x00000002
no locks held by irq/151-ee20000/35.
CPU: 0 PID: 35 Comm: irq/151-ee20000 Tainted: G W 6.8.0-rc4-ape6evm-00017-g27b57277d9ba #491
Hardware name: Generic R8A73A4 (Flattened Device Tree)
unwind_backtrace from show_stack+0x10/0x14
show_stack from dump_stack_lvl+0x68/0x90
dump_stack_lvl from __schedule_bug+0x5c/0x7c
__schedule_bug from __schedule+0xa0/0x9bc
__schedule from schedule+0x64/0x94
schedule from irq_thread+0x1dc/0x218
irq_thread from kthread+0xf0/0x100
kthread from ret_from_fork+0x14/0x28
Exception stack(0xf0959fb0 to 0xf0959ff8)
9fa0: 00000000 00000000 00000000 00000000
9fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
9fe0: 00000000 00000000 00000000 00000000 00000013 00000000
sh_mmcif ee200000.mmc: Timeout waiting for 2 on CMD18
Reverting this commit fixes the issue.
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -227,14 +227,12 @@ struct sh_mmcif_host {
bool dying;
long timeout;
void __iomem *addr;
- u32 *pio_ptr;
spinlock_t lock; /* protect sh_mmcif_host::state */
enum sh_mmcif_state state;
enum sh_mmcif_wait_for wait_for;
struct delayed_work timeout_work;
size_t blocksize;
- int sg_idx;
- int sg_blkidx;
+ struct sg_mapping_iter sg_miter;
bool power;
bool ccs_enable; /* Command Completion Signal support */
bool clk_ctrl2_enable;
@@ -600,32 +598,17 @@ static int sh_mmcif_error_manage(struct sh_mmcif_host *host)
return ret;
}
-static bool sh_mmcif_next_block(struct sh_mmcif_host *host, u32 *p)
-{
- struct mmc_data *data = host->mrq->data;
-
- host->sg_blkidx += host->blocksize;
-
- /* data->sg->length must be a multiple of host->blocksize? */
- BUG_ON(host->sg_blkidx > data->sg->length);
-
- if (host->sg_blkidx == data->sg->length) {
- host->sg_blkidx = 0;
- if (++host->sg_idx < data->sg_len)
- host->pio_ptr = sg_virt(++data->sg);
- } else {
- host->pio_ptr = p;
- }
-
- return host->sg_idx != data->sg_len;
-}
-
static void sh_mmcif_single_read(struct sh_mmcif_host *host,
struct mmc_request *mrq)
{
+ struct mmc_data *data = mrq->data;
+
host->blocksize = (sh_mmcif_readl(host->addr, MMCIF_CE_BLOCK_SET) &
BLOCK_SIZE_MASK) + 3;
+ sg_miter_start(&host->sg_miter, data->sg, data->sg_len,
+ SG_MITER_ATOMIC | SG_MITER_TO_SG);
+
host->wait_for = MMCIF_WAIT_FOR_READ;
/* buf read enable */
@@ -634,20 +617,32 @@ static void sh_mmcif_single_read(struct sh_mmcif_host *host,
static bool sh_mmcif_read_block(struct sh_mmcif_host *host)
{
+ struct sg_mapping_iter *sgm = &host->sg_miter;
struct device *dev = sh_mmcif_host_to_dev(host);
struct mmc_data *data = host->mrq->data;
- u32 *p = sg_virt(data->sg);
+ u32 *p;
int i;
if (host->sd_error) {
+ sg_miter_stop(sgm);
data->error = sh_mmcif_error_manage(host);
dev_dbg(dev, "%s(): %d\n", __func__, data->error);
return false;
}
+ if (!sg_miter_next(sgm)) {
+ /* This should not happen on single blocks */
+ sg_miter_stop(sgm);
+ return false;
+ }
+
+ p = sgm->addr;
+
for (i = 0; i < host->blocksize / 4; i++)
*p++ = sh_mmcif_readl(host->addr, MMCIF_CE_DATA);
+ sg_miter_stop(&host->sg_miter);
+
/* buffer read end */
sh_mmcif_bitset(host, MMCIF_CE_INT_MASK, MASK_MBUFRE);
host->wait_for = MMCIF_WAIT_FOR_READ_END;
@@ -666,34 +661,40 @@ static void sh_mmcif_multi_read(struct sh_mmcif_host *host,
host->blocksize = sh_mmcif_readl(host->addr, MMCIF_CE_BLOCK_SET) &
BLOCK_SIZE_MASK;
+ sg_miter_start(&host->sg_miter, data->sg, data->sg_len,
+ SG_MITER_ATOMIC | SG_MITER_TO_SG);
+
host->wait_for = MMCIF_WAIT_FOR_MREAD;
- host->sg_idx = 0;
- host->sg_blkidx = 0;
- host->pio_ptr = sg_virt(data->sg);
sh_mmcif_bitset(host, MMCIF_CE_INT_MASK, MASK_MBUFREN);
}
static bool sh_mmcif_mread_block(struct sh_mmcif_host *host)
{
+ struct sg_mapping_iter *sgm = &host->sg_miter;
struct device *dev = sh_mmcif_host_to_dev(host);
struct mmc_data *data = host->mrq->data;
- u32 *p = host->pio_ptr;
+ u32 *p;
int i;
if (host->sd_error) {
+ sg_miter_stop(sgm);
data->error = sh_mmcif_error_manage(host);
dev_dbg(dev, "%s(): %d\n", __func__, data->error);
return false;
}
- BUG_ON(!data->sg->length);
+ if (!sg_miter_next(sgm)) {
+ sg_miter_stop(sgm);
+ return false;
+ }
+
+ p = sgm->addr;
for (i = 0; i < host->blocksize / 4; i++)
*p++ = sh_mmcif_readl(host->addr, MMCIF_CE_DATA);
- if (!sh_mmcif_next_block(host, p))
- return false;
+ sgm->consumed = host->blocksize;
sh_mmcif_bitset(host, MMCIF_CE_INT_MASK, MASK_MBUFREN);
@@ -703,9 +704,14 @@ static bool sh_mmcif_mread_block(struct sh_mmcif_host *host)
static void sh_mmcif_single_write(struct sh_mmcif_host *host,
struct mmc_request *mrq)
{
+ struct mmc_data *data = mrq->data;
+
host->blocksize = (sh_mmcif_readl(host->addr, MMCIF_CE_BLOCK_SET) &
BLOCK_SIZE_MASK) + 3;
+ sg_miter_start(&host->sg_miter, data->sg, data->sg_len,
+ SG_MITER_ATOMIC | SG_MITER_FROM_SG);
+
host->wait_for = MMCIF_WAIT_FOR_WRITE;
/* buf write enable */
@@ -714,20 +720,32 @@ static void sh_mmcif_single_write(struct sh_mmcif_host *host,
static bool sh_mmcif_write_block(struct sh_mmcif_host *host)
{
+ struct sg_mapping_iter *sgm = &host->sg_miter;
struct device *dev = sh_mmcif_host_to_dev(host);
struct mmc_data *data = host->mrq->data;
- u32 *p = sg_virt(data->sg);
+ u32 *p;
int i;
if (host->sd_error) {
+ sg_miter_stop(sgm);
data->error = sh_mmcif_error_manage(host);
dev_dbg(dev, "%s(): %d\n", __func__, data->error);
return false;
}
+ if (!sg_miter_next(sgm)) {
+ /* This should not happen on single blocks */
+ sg_miter_stop(sgm);
+ return false;
+ }
+
+ p = sgm->addr;
+
for (i = 0; i < host->blocksize / 4; i++)
sh_mmcif_writel(host->addr, MMCIF_CE_DATA, *p++);
+ sg_miter_stop(&host->sg_miter);
+
/* buffer write end */
sh_mmcif_bitset(host, MMCIF_CE_INT_MASK, MASK_MDTRANE);
host->wait_for = MMCIF_WAIT_FOR_WRITE_END;
@@ -746,34 +764,40 @@ static void sh_mmcif_multi_write(struct sh_mmcif_host *host,
host->blocksize = sh_mmcif_readl(host->addr, MMCIF_CE_BLOCK_SET) &
BLOCK_SIZE_MASK;
+ sg_miter_start(&host->sg_miter, data->sg, data->sg_len,
+ SG_MITER_ATOMIC | SG_MITER_FROM_SG);
+
host->wait_for = MMCIF_WAIT_FOR_MWRITE;
- host->sg_idx = 0;
- host->sg_blkidx = 0;
- host->pio_ptr = sg_virt(data->sg);
sh_mmcif_bitset(host, MMCIF_CE_INT_MASK, MASK_MBUFWEN);
}
static bool sh_mmcif_mwrite_block(struct sh_mmcif_host *host)
{
+ struct sg_mapping_iter *sgm = &host->sg_miter;
struct device *dev = sh_mmcif_host_to_dev(host);
struct mmc_data *data = host->mrq->data;
- u32 *p = host->pio_ptr;
+ u32 *p;
int i;
if (host->sd_error) {
+ sg_miter_stop(sgm);
data->error = sh_mmcif_error_manage(host);
dev_dbg(dev, "%s(): %d\n", __func__, data->error);
return false;
}
- BUG_ON(!data->sg->length);
+ if (!sg_miter_next(sgm)) {
+ sg_miter_stop(sgm);
+ return false;
+ }
+
+ p = sgm->addr;
for (i = 0; i < host->blocksize / 4; i++)
sh_mmcif_writel(host->addr, MMCIF_CE_DATA, *p++);
- if (!sh_mmcif_next_block(host, p))
- return false;
+ sgm->consumed = host->blocksize;
sh_mmcif_bitset(host, MMCIF_CE_INT_MASK, MASK_MBUFWEN);
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
