Discussion:
[PATCH 01/15] mmc: sdhci: Fix incorrect ADMA2 descriptor table size
Adrian Hunter
2014-10-21 09:26:11 UTC
Permalink
The ADMA2 descriptor table size was being calculated incorrectly
Fix it.

Note that it has been wrong for a long time and likely has not
caused any problems because of a combination of 1) not needing
alignment descriptors for block operations 2) more memory being
allocated than was requested 3) the use of
SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC which does not use an extra
descriptor for the end marker.

Signed-off-by: Adrian Hunter <***@intel.com>
---
drivers/mmc/host/sdhci.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index ada1a3e..c409e87 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -44,7 +44,13 @@

#define MAX_TUNING_LOOP 40

-#define ADMA_SIZE ((128 * 2 + 1) * 4)
+/*
+ * The ADMA2 descriptor table size is calculated as the maximum number of
+ * segments (128), times 2 to allow for an alignment descriptor for each
+ * segment, plus 1 for a nop end descriptor, all multipled by the 32-bit
+ * descriptor size (8).
+ */
+#define ADMA_SIZE ((128 * 2 + 1) * 8)

static unsigned int debug_quirks = 0;
static unsigned int debug_quirks2;
--
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Adrian Hunter
2014-10-21 09:26:12 UTC
Permalink
Bytes are being copied from/to a single page. The intent
of the warning is to warn if the page boundary is crossed.
There are two problems. First, PAGE_MASK is mistaken for
(PAGE_SIZE - 1). Secondly, instead of using the number
of bytes to copy, the warning is using the maximum that
that value could be. Fix both.

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

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c409e87..62d0f5d 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -525,7 +525,8 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,
if (offset) {
if (data->flags & MMC_DATA_WRITE) {
buffer = sdhci_kmap_atomic(sg, &flags);
- WARN_ON(((long)buffer & PAGE_MASK) > (PAGE_SIZE - 3));
+ WARN_ON(((long)buffer & (PAGE_SIZE - 1)) >
+ (PAGE_SIZE - offset));
memcpy(align, buffer, offset);
sdhci_kunmap_atomic(buffer, &flags);
}
@@ -630,7 +631,8 @@ static void sdhci_adma_table_post(struct sdhci_host *host,
size = 4 - (sg_dma_address(sg) & 0x3);

buffer = sdhci_kmap_atomic(sg, &flags);
- WARN_ON(((long)buffer & PAGE_MASK) > (PAGE_SIZE - 3));
+ WARN_ON(((long)buffer & (PAGE_SIZE - 1)) >
+ (PAGE_SIZE - size));
memcpy(buffer, align, size);
sdhci_kunmap_atomic(buffer, &flags);
--
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Adrian Hunter
2014-10-21 09:26:13 UTC
Permalink
The intent of the warning is to warn if the ADMA table
overflows. However there can be one more 'end' entry
so the condition should be adjusted accordingly.

Signed-off-by: Adrian Hunter <***@intel.com>
---
drivers/mmc/host/sdhci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 62d0f5d..700ac22 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -555,7 +555,7 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,
* If this triggers then we have a calculation bug
* somewhere. :/
*/
- WARN_ON((desc - host->adma_desc) > ADMA_SIZE);
+ WARN_ON((desc - host->adma_desc) >= ADMA_SIZE);
}

if (host->quirks & SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC) {
--
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Adrian Hunter
2014-10-21 09:26:14 UTC
Permalink
Rename sdhci_set_adma_desc to sdhci_adma_write_desc and
sdhci_show_adma_error to sdhci_adma_show_error so that
all ADMA functions start with sdhci_adma_.

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

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 700ac22..b61e84b 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -454,7 +454,7 @@ static void sdhci_kunmap_atomic(void *buffer, unsigned long *flags)
local_irq_restore(*flags);
}

-static void sdhci_set_adma_desc(u8 *desc, u32 addr, int len, unsigned cmd)
+static void sdhci_adma_write_desc(u8 *desc, u32 addr, int len, unsigned cmd)
{
__le32 *dataddr = (__le32 __force *)(desc + 4);
__le16 *cmdlen = (__le16 __force *)desc;
@@ -532,7 +532,7 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,
}

/* tran, valid */
- sdhci_set_adma_desc(desc, align_addr, offset, 0x21);
+ sdhci_adma_write_desc(desc, align_addr, offset, 0x21);

BUG_ON(offset > 65536);

@@ -548,7 +548,7 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,
BUG_ON(len > 65536);

/* tran, valid */
- sdhci_set_adma_desc(desc, addr, len, 0x21);
+ sdhci_adma_write_desc(desc, addr, len, 0x21);
desc += 8;

/*
@@ -572,7 +572,7 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,
*/

/* nop, end, valid */
- sdhci_set_adma_desc(desc, 0, 0, 0x3);
+ sdhci_adma_write_desc(desc, 0, 0, 0x3);
}

/*
@@ -2290,7 +2290,7 @@ static void sdhci_cmd_irq(struct sdhci_host *host, u32 intmask, u32 *mask)
}

#ifdef CONFIG_MMC_DEBUG
-static void sdhci_show_adma_error(struct sdhci_host *host)
+static void sdhci_adma_show_error(struct sdhci_host *host)
{
const char *name = mmc_hostname(host->mmc);
u8 *desc = host->adma_desc;
@@ -2315,7 +2315,7 @@ static void sdhci_show_adma_error(struct sdhci_host *host)
}
}
#else
-static void sdhci_show_adma_error(struct sdhci_host *host) { }
+static void sdhci_adma_show_error(struct sdhci_host *host) { }
#endif

static void sdhci_data_irq(struct sdhci_host *host, u32 intmask)
@@ -2378,7 +2378,7 @@ static void sdhci_data_irq(struct sdhci_host *host, u32 intmask)
host->data->error = -EILSEQ;
else if (intmask & SDHCI_INT_ADMA_ERROR) {
pr_err("%s: ADMA error\n", mmc_hostname(host->mmc));
- sdhci_show_adma_error(host);
+ sdhci_adma_show_error(host);
host->data->error = -EIO;
if (host->ops->adma_workaround)
host->ops->adma_workaround(host, intmask);
--
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Adrian Hunter
2014-10-21 09:26:15 UTC
Permalink
In preparation for 64-bit ADMA, rename adma_desc to
adma_table. That is because members will be added
for descriptor size and table size, so using adma_desc
(which is the table) is confusing.

Signed-off-by: Adrian Hunter <***@intel.com>
---
drivers/mmc/host/sdhci.c | 31 ++++++++++++++++---------------
include/linux/mmc/sdhci.h | 2 +-
2 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index b61e84b..b45814e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -505,7 +505,7 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,
if (host->sg_count == 0)
goto unmap_align;

- desc = host->adma_desc;
+ desc = host->adma_table;
align = host->align_buffer;

align_addr = host->align_addr;
@@ -555,14 +555,14 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,
* If this triggers then we have a calculation bug
* somewhere. :/
*/
- WARN_ON((desc - host->adma_desc) >= ADMA_SIZE);
+ WARN_ON((desc - host->adma_table) >= ADMA_SIZE);
}

if (host->quirks & SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC) {
/*
* Mark the last descriptor as the terminating descriptor
*/
- if (desc != host->adma_desc) {
+ if (desc != host->adma_table) {
desc -= 8;
desc[0] |= 0x2; /* end */
}
@@ -2293,7 +2293,7 @@ static void sdhci_cmd_irq(struct sdhci_host *host, u32 intmask, u32 *mask)
static void sdhci_adma_show_error(struct sdhci_host *host)
{
const char *name = mmc_hostname(host->mmc);
- u8 *desc = host->adma_desc;
+ u8 *desc = host->adma_table;
__le32 *dma;
__le16 *len;
u8 attr;
@@ -2874,27 +2874,28 @@ int sdhci_add_host(struct sdhci_host *host)
* (128) and potentially one alignment transfer for
* each of those entries.
*/
- host->adma_desc = dma_alloc_coherent(mmc_dev(mmc),
- ADMA_SIZE, &host->adma_addr,
- GFP_KERNEL);
+ host->adma_table = dma_alloc_coherent(mmc_dev(mmc),
+ ADMA_SIZE,
+ &host->adma_addr,
+ GFP_KERNEL);
host->align_buffer = kmalloc(128 * 4, GFP_KERNEL);
- if (!host->adma_desc || !host->align_buffer) {
+ if (!host->adma_table || !host->align_buffer) {
dma_free_coherent(mmc_dev(mmc), ADMA_SIZE,
- host->adma_desc, host->adma_addr);
+ host->adma_table, host->adma_addr);
kfree(host->align_buffer);
pr_warn("%s: Unable to allocate ADMA buffers - falling back to standard DMA\n",
mmc_hostname(mmc));
host->flags &= ~SDHCI_USE_ADMA;
- host->adma_desc = NULL;
+ host->adma_table = NULL;
host->align_buffer = NULL;
} else if (host->adma_addr & 3) {
pr_warn("%s: unable to allocate aligned ADMA descriptor\n",
mmc_hostname(mmc));
host->flags &= ~SDHCI_USE_ADMA;
dma_free_coherent(mmc_dev(mmc), ADMA_SIZE,
- host->adma_desc, host->adma_addr);
+ host->adma_table, host->adma_addr);
kfree(host->align_buffer);
- host->adma_desc = NULL;
+ host->adma_table = NULL;
host->align_buffer = NULL;
}
}
@@ -3353,12 +3354,12 @@ void sdhci_remove_host(struct sdhci_host *host, int dead)
if (!IS_ERR(mmc->supply.vqmmc))
regulator_disable(mmc->supply.vqmmc);

- if (host->adma_desc)
+ if (host->adma_table)
dma_free_coherent(mmc_dev(mmc), ADMA_SIZE,
- host->adma_desc, host->adma_addr);
+ host->adma_table, host->adma_addr);
kfree(host->align_buffer);

- host->adma_desc = NULL;
+ host->adma_table = NULL;
host->align_buffer = NULL;
}

diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index dba793e..76e0432 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -155,7 +155,7 @@ struct sdhci_host {

int sg_count; /* Mapped sg entries */

- u8 *adma_desc; /* ADMA descriptor table */
+ u8 *adma_table; /* ADMA descriptor table */
u8 *align_buffer; /* Bounce buffer */

dma_addr_t adma_addr; /* Mapped ADMA descr. table */
--
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Adrian Hunter
2014-10-21 09:26:16 UTC
Permalink
In preparation for 64-bit ADMA, separate out code
that touches the ADMA descriptor by adding
sdhci_adma_mark_end().

Signed-off-by: Adrian Hunter <***@intel.com>
---
drivers/mmc/host/sdhci.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index b45814e..833e792 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -468,6 +468,13 @@ static void sdhci_adma_write_desc(u8 *desc, u32 addr, int len, unsigned cmd)
dataddr[0] = cpu_to_le32(addr);
}

+static void sdhci_adma_mark_end(void *desc)
+{
+ u8 *dma_desc = desc;
+
+ dma_desc[0] |= 0x2; /* end */
+}
+
static int sdhci_adma_table_pre(struct sdhci_host *host,
struct mmc_data *data)
{
@@ -564,7 +571,7 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,
*/
if (desc != host->adma_table) {
desc -= 8;
- desc[0] |= 0x2; /* end */
+ sdhci_adma_mark_end(desc);
}
} else {
/*
--
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Adrian Hunter
2014-10-21 09:26:17 UTC
Permalink
It is kernel-style to use 'void *' for anonymous data.
This is being applied to the ADMA bounce buffer which
contains unaligned bytes, and to the ADMA descriptor
table which will contain 32-bit ADMA descriptors
or 64-bit ADMA descriptors when support is added.

Signed-off-by: Adrian Hunter <***@intel.com>
---
drivers/mmc/host/sdhci.c | 10 +++++-----
include/linux/mmc/sdhci.h | 4 ++--
2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 833e792..a4322e2 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -454,7 +454,7 @@ static void sdhci_kunmap_atomic(void *buffer, unsigned long *flags)
local_irq_restore(*flags);
}

-static void sdhci_adma_write_desc(u8 *desc, u32 addr, int len, unsigned cmd)
+static void sdhci_adma_write_desc(void *desc, u32 addr, int len, unsigned cmd)
{
__le32 *dataddr = (__le32 __force *)(desc + 4);
__le16 *cmdlen = (__le16 __force *)desc;
@@ -480,8 +480,8 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,
{
int direction;

- u8 *desc;
- u8 *align;
+ void *desc;
+ void *align;
dma_addr_t addr;
dma_addr_t align_addr;
int len, offset;
@@ -606,7 +606,7 @@ static void sdhci_adma_table_post(struct sdhci_host *host,

struct scatterlist *sg;
int i, size;
- u8 *align;
+ void *align;
char *buffer;
unsigned long flags;
bool has_unaligned;
@@ -2300,7 +2300,7 @@ static void sdhci_cmd_irq(struct sdhci_host *host, u32 intmask, u32 *mask)
static void sdhci_adma_show_error(struct sdhci_host *host)
{
const char *name = mmc_hostname(host->mmc);
- u8 *desc = host->adma_table;
+ void *desc = host->adma_table;
__le32 *dma;
__le16 *len;
u8 attr;
diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index 76e0432..933dbbb 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -155,8 +155,8 @@ struct sdhci_host {

int sg_count; /* Mapped sg entries */

- u8 *adma_table; /* ADMA descriptor table */
- u8 *align_buffer; /* Bounce buffer */
+ void *adma_table; /* ADMA descriptor table */
+ void *align_buffer; /* Bounce buffer */

dma_addr_t adma_addr; /* Mapped ADMA descr. table */
dma_addr_t align_addr; /* Mapped bounce buffer */
--
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Adrian Hunter
2014-10-21 09:26:20 UTC
Permalink
Define all the ADMA constants instead of having numbers
scattered throughout the code.

Signed-off-by: Adrian Hunter <***@intel.com>
---
drivers/mmc/host/sdhci.c | 23 +++++++++++++----------
drivers/mmc/host/sdhci.h | 10 ++++++++++
2 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index ea0ba95..5dd5408 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -464,7 +464,7 @@ static void sdhci_adma_mark_end(void *desc)
{
u8 *dma_desc = desc;

- dma_desc[0] |= 0x2; /* end */
+ dma_desc[0] |= ADMA2_END;
}

static int sdhci_adma_table_pre(struct sdhci_host *host,
@@ -532,7 +532,8 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,
}

/* tran, valid */
- sdhci_adma_write_desc(desc, align_addr, offset, 0x21);
+ sdhci_adma_write_desc(desc, align_addr, offset,
+ ADMA2_TRAN_VALID);

BUG_ON(offset > 65536);

@@ -548,7 +549,7 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,
BUG_ON(len > 65536);

/* tran, valid */
- sdhci_adma_write_desc(desc, addr, len, 0x21);
+ sdhci_adma_write_desc(desc, addr, len, ADMA2_TRAN_VALID);
desc += host->desc_sz;

/*
@@ -572,7 +573,7 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,
*/

/* nop, end, valid */
- sdhci_adma_write_desc(desc, 0, 0, 0x3);
+ sdhci_adma_write_desc(desc, 0, 0, ADMA2_NOP_END_VALID);
}

/*
@@ -2311,7 +2312,7 @@ static void sdhci_adma_show_error(struct sdhci_host *host)

desc += host->desc_sz;

- if (attr & 2)
+ if (attr & ADMA2_END)
break;
}
}
@@ -2876,11 +2877,13 @@ int sdhci_add_host(struct sdhci_host *host)
* descriptor for each segment, plus 1 for a nop end descriptor,
* all multipled by the descriptor size.
*/
- host->adma_table_sz = (SDHCI_MAX_SEGS * 2 + 1) * 8;
- host->align_buffer_sz = SDHCI_MAX_SEGS * 4;
- host->desc_sz = 8;
- host->align_sz = 4;
- host->align_mask = 3;
+ host->adma_table_sz = (SDHCI_MAX_SEGS * 2 + 1) *
+ SDHCI_ADMA2_32_DESC_SZ;
+ host->align_buffer_sz = SDHCI_MAX_SEGS *
+ SDHCI_ADMA2_32_ALIGN;
+ host->desc_sz = SDHCI_ADMA2_32_DESC_SZ;
+ host->align_sz = SDHCI_ADMA2_32_ALIGN;
+ host->align_mask = SDHCI_ADMA2_32_ALIGN - 1;
host->adma_table = dma_alloc_coherent(mmc_dev(mmc),
host->adma_table_sz,
&host->adma_addr,
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 6ae75cd..823cce1 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -266,6 +266,16 @@
#define SDHCI_DEFAULT_BOUNDARY_SIZE (512 * 1024)
#define SDHCI_DEFAULT_BOUNDARY_ARG (ilog2(SDHCI_DEFAULT_BOUNDARY_SIZE) - 12)

+/* ADMA2 32-bit DMA descriptor size */
+#define SDHCI_ADMA2_32_DESC_SZ 8
+
+/* ADMA2 32-bit DMA alignment */
+#define SDHCI_ADMA2_32_ALIGN 4
+
+#define ADMA2_TRAN_VALID 0x21
+#define ADMA2_NOP_END_VALID 0x3
+#define ADMA2_END 0x2
+
/*
* Maximum segments assuming a 512KiB maximum requisition size and a minimum
* 4KiB page size.
--
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Adrian Hunter
2014-10-21 09:26:21 UTC
Permalink
Define the ADMA descriptor structure instead of
using manual offsets and casts.

Signed-off-by: Adrian Hunter <***@intel.com>
---
drivers/mmc/host/sdhci.c | 30 +++++++++++-------------------
drivers/mmc/host/sdhci.h | 7 +++++++
2 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 5dd5408..e503fd4 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -448,23 +448,18 @@ static void sdhci_kunmap_atomic(void *buffer, unsigned long *flags)

static void sdhci_adma_write_desc(void *desc, u32 addr, int len, unsigned cmd)
{
- __le32 *dataddr = (__le32 __force *)(desc + 4);
- __le16 *cmdlen = (__le16 __force *)desc;
+ struct sdhci_adma2_32_desc *dma_desc = desc;

- /* SDHCI specification says ADMA descriptors should be 4 byte
- * aligned, so using 16 or 32bit operations should be safe. */
-
- cmdlen[0] = cpu_to_le16(cmd);
- cmdlen[1] = cpu_to_le16(len);
-
- dataddr[0] = cpu_to_le32(addr);
+ dma_desc->cmd = cpu_to_le16(cmd);
+ dma_desc->len = cpu_to_le16(len);
+ dma_desc->addr = cpu_to_le32(addr);
}

static void sdhci_adma_mark_end(void *desc)
{
- u8 *dma_desc = desc;
+ struct sdhci_adma2_32_desc *dma_desc = desc;

- dma_desc[0] |= ADMA2_END;
+ dma_desc->cmd |= cpu_to_le16(ADMA2_END);
}

static int sdhci_adma_table_pre(struct sdhci_host *host,
@@ -2296,23 +2291,20 @@ static void sdhci_adma_show_error(struct sdhci_host *host)
{
const char *name = mmc_hostname(host->mmc);
void *desc = host->adma_table;
- __le32 *dma;
- __le16 *len;
- u8 attr;

sdhci_dumpregs(host);

while (true) {
- dma = (__le32 *)(desc + 4);
- len = (__le16 *)(desc + 2);
- attr = *desc;
+ struct sdhci_adma2_32_desc *dma_desc = desc;

DBG("%s: %p: DMA 0x%08x, LEN 0x%04x, Attr=0x%02x\n",
- name, desc, le32_to_cpu(*dma), le16_to_cpu(*len), attr);
+ name, desc, le32_to_cpu(dma_desc->addr),
+ le16_to_cpu(dma_desc->len),
+ le16_to_cpu(dma_desc->cmd));

desc += host->desc_sz;

- if (attr & ADMA2_END)
+ if (dma_desc->cmd & cpu_to_le16(ADMA2_END))
break;
}
}
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 823cce1..14c8b67 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -272,6 +272,13 @@
/* ADMA2 32-bit DMA alignment */
#define SDHCI_ADMA2_32_ALIGN 4

+/* ADMA2 32-bit descriptor */
+struct sdhci_adma2_32_desc {
+ __le16 cmd;
+ __le16 len;
+ __le32 addr;
+} __packed __aligned(SDHCI_ADMA2_32_ALIGN);
+
#define ADMA2_TRAN_VALID 0x21
#define ADMA2_NOP_END_VALID 0x3
#define ADMA2_END 0x2
--
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Adrian Hunter
2014-10-21 09:26:19 UTC
Permalink
Define the maximum number of segments instead of
having the constant 128 appearing in the code in
various places.

Signed-off-by: Adrian Hunter <***@intel.com>
---
drivers/mmc/host/sdhci.c | 8 ++++----
drivers/mmc/host/sdhci.h | 6 ++++++
2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 41a3f79..ea0ba95 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2876,8 +2876,8 @@ int sdhci_add_host(struct sdhci_host *host)
* descriptor for each segment, plus 1 for a nop end descriptor,
* all multipled by the descriptor size.
*/
- host->adma_table_sz = (128 * 2 + 1) * 8;
- host->align_buffer_sz = 128 * 4;
+ host->adma_table_sz = (SDHCI_MAX_SEGS * 2 + 1) * 8;
+ host->align_buffer_sz = SDHCI_MAX_SEGS * 4;
host->desc_sz = 8;
host->align_sz = 4;
host->align_mask = 3;
@@ -3191,11 +3191,11 @@ int sdhci_add_host(struct sdhci_host *host)
* can do scatter/gather or not.
*/
if (host->flags & SDHCI_USE_ADMA)
- mmc->max_segs = 128;
+ mmc->max_segs = SDHCI_MAX_SEGS;
else if (host->flags & SDHCI_USE_SDMA)
mmc->max_segs = 1;
else /* PIO */
- mmc->max_segs = 128;
+ mmc->max_segs = SDHCI_MAX_SEGS;

/*
* Maximum number of sectors in one transfer. Limited by DMA boundary
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 31896a7..6ae75cd 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -266,6 +266,12 @@
#define SDHCI_DEFAULT_BOUNDARY_SIZE (512 * 1024)
#define SDHCI_DEFAULT_BOUNDARY_ARG (ilog2(SDHCI_DEFAULT_BOUNDARY_SIZE) - 12)

+/*
+ * Maximum segments assuming a 512KiB maximum requisition size and a minimum
+ * 4KiB page size.
+ */
+#define SDHCI_MAX_SEGS 128
+
struct sdhci_ops {
#ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
u32 (*read_l)(struct sdhci_host *host, int reg);
--
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Adrian Hunter
2014-10-21 09:26:18 UTC
Permalink
In preparation for 64-bit ADMA, parameterize ADMA sizes
and alignment. 64-bit ADMA has a larger descriptor
because it contains a 64-bit address instead of a 32-bit
address. Also data must be 8-byte aligned instead
of 4-byte aligned. Consequently, sdhci_host members
are added for descriptor, table, and buffer sizes
and alignment.

Signed-off-by: Adrian Hunter <***@intel.com>
---
drivers/mmc/host/sdhci.c | 68 +++++++++++++++++++++++------------------------
include/linux/mmc/sdhci.h | 7 +++++
2 files changed, 41 insertions(+), 34 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index a4322e2..41a3f79 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -44,14 +44,6 @@

#define MAX_TUNING_LOOP 40

-/*
- * The ADMA2 descriptor table size is calculated as the maximum number of
- * segments (128), times 2 to allow for an alignment descriptor for each
- * segment, plus 1 for a nop end descriptor, all multipled by the 32-bit
- * descriptor size (8).
- */
-#define ADMA_SIZE ((128 * 2 + 1) * 8)
-
static unsigned int debug_quirks = 0;
static unsigned int debug_quirks2;

@@ -502,10 +494,10 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,
direction = DMA_TO_DEVICE;

host->align_addr = dma_map_single(mmc_dev(host->mmc),
- host->align_buffer, 128 * 4, direction);
+ host->align_buffer, host->align_buffer_sz, direction);
if (dma_mapping_error(mmc_dev(host->mmc), host->align_addr))
goto fail;
- BUG_ON(host->align_addr & 0x3);
+ BUG_ON(host->align_addr & host->align_mask);

host->sg_count = dma_map_sg(mmc_dev(host->mmc),
data->sg, data->sg_len, direction);
@@ -528,7 +520,8 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,
* the (up to three) bytes that screw up the
* alignment.
*/
- offset = (4 - (addr & 0x3)) & 0x3;
+ offset = (host->align_sz - (addr & host->align_mask)) &
+ host->align_mask;
if (offset) {
if (data->flags & MMC_DATA_WRITE) {
buffer = sdhci_kmap_atomic(sg, &flags);
@@ -543,10 +536,10 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,

BUG_ON(offset > 65536);

- align += 4;
- align_addr += 4;
+ align += host->align_sz;
+ align_addr += host->align_sz;

- desc += 8;
+ desc += host->desc_sz;

addr += offset;
len -= offset;
@@ -556,13 +549,13 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,

/* tran, valid */
sdhci_adma_write_desc(desc, addr, len, 0x21);
- desc += 8;
+ desc += host->desc_sz;

/*
* If this triggers then we have a calculation bug
* somewhere. :/
*/
- WARN_ON((desc - host->adma_table) >= ADMA_SIZE);
+ WARN_ON((desc - host->adma_table) >= host->adma_table_sz);
}

if (host->quirks & SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC) {
@@ -570,7 +563,7 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,
* Mark the last descriptor as the terminating descriptor
*/
if (desc != host->adma_table) {
- desc -= 8;
+ desc -= host->desc_sz;
sdhci_adma_mark_end(desc);
}
} else {
@@ -587,14 +580,14 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,
*/
if (data->flags & MMC_DATA_WRITE) {
dma_sync_single_for_device(mmc_dev(host->mmc),
- host->align_addr, 128 * 4, direction);
+ host->align_addr, host->align_buffer_sz, direction);
}

return 0;

unmap_align:
dma_unmap_single(mmc_dev(host->mmc), host->align_addr,
- 128 * 4, direction);
+ host->align_buffer_sz, direction);
fail:
return -EINVAL;
}
@@ -617,12 +610,12 @@ static void sdhci_adma_table_post(struct sdhci_host *host,
direction = DMA_TO_DEVICE;

dma_unmap_single(mmc_dev(host->mmc), host->align_addr,
- 128 * 4, direction);
+ host->align_buffer_sz, direction);

/* Do a quick scan of the SG list for any unaligned mappings */
has_unaligned = false;
for_each_sg(data->sg, sg, host->sg_count, i)
- if (sg_dma_address(sg) & 3) {
+ if (sg_dma_address(sg) & host->align_mask) {
has_unaligned = true;
break;
}
@@ -634,8 +627,9 @@ static void sdhci_adma_table_post(struct sdhci_host *host,
align = host->align_buffer;

for_each_sg(data->sg, sg, host->sg_count, i) {
- if (sg_dma_address(sg) & 0x3) {
- size = 4 - (sg_dma_address(sg) & 0x3);
+ if (sg_dma_address(sg) & host->align_mask) {
+ size = host->align_sz -
+ (sg_dma_address(sg) & host->align_mask);

buffer = sdhci_kmap_atomic(sg, &flags);
WARN_ON(((long)buffer & (PAGE_SIZE - 1)) >
@@ -643,7 +637,7 @@ static void sdhci_adma_table_post(struct sdhci_host *host,
memcpy(buffer, align, size);
sdhci_kunmap_atomic(buffer, &flags);

- align += 4;
+ align += host->align_sz;
}
}
}
@@ -2315,7 +2309,7 @@ static void sdhci_adma_show_error(struct sdhci_host *host)
DBG("%s: %p: DMA 0x%08x, LEN 0x%04x, Attr=0x%02x\n",
name, desc, le32_to_cpu(*dma), le16_to_cpu(*len), attr);

- desc += 8;
+ desc += host->desc_sz;

if (attr & 2)
break;
@@ -2877,17 +2871,23 @@ int sdhci_add_host(struct sdhci_host *host)

if (host->flags & SDHCI_USE_ADMA) {
/*
- * We need to allocate descriptors for all sg entries
- * (128) and potentially one alignment transfer for
- * each of those entries.
+ * The DMA descriptor table size is calculated as the maximum
+ * number of segments times 2, to allow for an alignment
+ * descriptor for each segment, plus 1 for a nop end descriptor,
+ * all multipled by the descriptor size.
*/
+ host->adma_table_sz = (128 * 2 + 1) * 8;
+ host->align_buffer_sz = 128 * 4;
+ host->desc_sz = 8;
+ host->align_sz = 4;
+ host->align_mask = 3;
host->adma_table = dma_alloc_coherent(mmc_dev(mmc),
- ADMA_SIZE,
+ host->adma_table_sz,
&host->adma_addr,
GFP_KERNEL);
- host->align_buffer = kmalloc(128 * 4, GFP_KERNEL);
+ host->align_buffer = kmalloc(host->align_buffer_sz, GFP_KERNEL);
if (!host->adma_table || !host->align_buffer) {
- dma_free_coherent(mmc_dev(mmc), ADMA_SIZE,
+ dma_free_coherent(mmc_dev(mmc), host->adma_table_sz,
host->adma_table, host->adma_addr);
kfree(host->align_buffer);
pr_warn("%s: Unable to allocate ADMA buffers - falling back to standard DMA\n",
@@ -2895,11 +2895,11 @@ int sdhci_add_host(struct sdhci_host *host)
host->flags &= ~SDHCI_USE_ADMA;
host->adma_table = NULL;
host->align_buffer = NULL;
- } else if (host->adma_addr & 3) {
+ } else if (host->adma_addr & host->align_mask) {
pr_warn("%s: unable to allocate aligned ADMA descriptor\n",
mmc_hostname(mmc));
host->flags &= ~SDHCI_USE_ADMA;
- dma_free_coherent(mmc_dev(mmc), ADMA_SIZE,
+ dma_free_coherent(mmc_dev(mmc), host->adma_table_sz,
host->adma_table, host->adma_addr);
kfree(host->align_buffer);
host->adma_table = NULL;
@@ -3362,7 +3362,7 @@ void sdhci_remove_host(struct sdhci_host *host, int dead)
regulator_disable(mmc->supply.vqmmc);

if (host->adma_table)
- dma_free_coherent(mmc_dev(mmc), ADMA_SIZE,
+ dma_free_coherent(mmc_dev(mmc), host->adma_table_sz,
host->adma_table, host->adma_addr);
kfree(host->align_buffer);

diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index 933dbbb..2a72e95 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -158,9 +158,16 @@ struct sdhci_host {
void *adma_table; /* ADMA descriptor table */
void *align_buffer; /* Bounce buffer */

+ size_t adma_table_sz; /* ADMA descriptor table size */
+ size_t align_buffer_sz; /* Bounce buffer size */
+
dma_addr_t adma_addr; /* Mapped ADMA descr. table */
dma_addr_t align_addr; /* Mapped bounce buffer */

+ unsigned int desc_sz; /* ADMA descriptor size */
+ unsigned int align_sz; /* ADMA alignment */
+ unsigned int align_mask; /* ADMA alignment mask */
+
struct tasklet_struct finish_tasklet; /* Tasklet structures */

struct timer_list timer; /* Timer for timeouts */
--
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Adrian Hunter
2014-10-21 09:26:22 UTC
Permalink
Add 64-bit ADMA support including:
- add 64-bit ADMA descriptor
- add SDHCI_USE_64_BIT_DMA flag
- set upper 32-bits of DMA addresses
- ability to select 64-bit ADMA
- ability to use 64-bit ADMA sizes and alignment
- display "ADMA 64-bit" when host is added

It is assumed that a 64-bit capable device has set a 64-bit DMA mask
and *must* do 64-bit DMA. A driver has the opportunity to change
that during the first call to ->enable_dma(). Similarly
SDHCI_QUIRK2_BROKEN_64_BIT_DMA must be left to the drivers to
implement.

Signed-off-by: Adrian Hunter <***@intel.com>
---
drivers/mmc/host/sdhci.c | 109 ++++++++++++++++++++++++++++++++++------------
drivers/mmc/host/sdhci.h | 18 ++++++++
include/linux/mmc/sdhci.h | 3 ++
3 files changed, 102 insertions(+), 28 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index e503fd4..9918225 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -117,10 +117,17 @@ static void sdhci_dumpregs(struct sdhci_host *host)
pr_debug(DRIVER_NAME ": Host ctl2: 0x%08x\n",
sdhci_readw(host, SDHCI_HOST_CONTROL2));

- if (host->flags & SDHCI_USE_ADMA)
- pr_debug(DRIVER_NAME ": ADMA Err: 0x%08x | ADMA Ptr: 0x%08x\n",
- readl(host->ioaddr + SDHCI_ADMA_ERROR),
- readl(host->ioaddr + SDHCI_ADMA_ADDRESS));
+ if (host->flags & SDHCI_USE_ADMA) {
+ if (host->flags & SDHCI_USE_64_BIT_DMA)
+ pr_debug(DRIVER_NAME ": ADMA Err: 0x%08x | ADMA Ptr: 0x%08x%08x\n",
+ readl(host->ioaddr + SDHCI_ADMA_ERROR),
+ readl(host->ioaddr + SDHCI_ADMA_ADDRESS_HI),
+ readl(host->ioaddr + SDHCI_ADMA_ADDRESS));
+ else
+ pr_debug(DRIVER_NAME ": ADMA Err: 0x%08x | ADMA Ptr: 0x%08x\n",
+ readl(host->ioaddr + SDHCI_ADMA_ERROR),
+ readl(host->ioaddr + SDHCI_ADMA_ADDRESS));
+ }

pr_debug(DRIVER_NAME ": ===========================================\n");
}
@@ -446,19 +453,25 @@ static void sdhci_kunmap_atomic(void *buffer, unsigned long *flags)
local_irq_restore(*flags);
}

-static void sdhci_adma_write_desc(void *desc, u32 addr, int len, unsigned cmd)
+static void sdhci_adma_write_desc(struct sdhci_host *host, void *desc,
+ dma_addr_t addr, int len, unsigned cmd)
{
- struct sdhci_adma2_32_desc *dma_desc = desc;
+ struct sdhci_adma2_64_desc *dma_desc = desc;

+ /* 32-bit and 64-bit descriptors have these members in same position */
dma_desc->cmd = cpu_to_le16(cmd);
dma_desc->len = cpu_to_le16(len);
- dma_desc->addr = cpu_to_le32(addr);
+ dma_desc->addr_lo = cpu_to_le32((u32)addr);
+
+ if (host->flags & SDHCI_USE_64_BIT_DMA)
+ dma_desc->addr_hi = cpu_to_le32((u64)addr >> 32);
}

static void sdhci_adma_mark_end(void *desc)
{
- struct sdhci_adma2_32_desc *dma_desc = desc;
+ struct sdhci_adma2_64_desc *dma_desc = desc;

+ /* 32-bit and 64-bit descriptors have 'cmd' in same position */
dma_desc->cmd |= cpu_to_le16(ADMA2_END);
}

@@ -527,7 +540,7 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,
}

/* tran, valid */
- sdhci_adma_write_desc(desc, align_addr, offset,
+ sdhci_adma_write_desc(host, desc, align_addr, offset,
ADMA2_TRAN_VALID);

BUG_ON(offset > 65536);
@@ -544,7 +557,7 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,
BUG_ON(len > 65536);

/* tran, valid */
- sdhci_adma_write_desc(desc, addr, len, ADMA2_TRAN_VALID);
+ sdhci_adma_write_desc(host, desc, addr, len, ADMA2_TRAN_VALID);
desc += host->desc_sz;

/*
@@ -568,7 +581,7 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,
*/

/* nop, end, valid */
- sdhci_adma_write_desc(desc, 0, 0, ADMA2_NOP_END_VALID);
+ sdhci_adma_write_desc(host, desc, 0, 0, ADMA2_NOP_END_VALID);
}

/*
@@ -827,6 +840,10 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd)
} else {
sdhci_writel(host, host->adma_addr,
SDHCI_ADMA_ADDRESS);
+ if (host->flags & SDHCI_USE_64_BIT_DMA)
+ sdhci_writel(host,
+ (u64)host->adma_addr >> 32,
+ SDHCI_ADMA_ADDRESS_HI);
}
} else {
int sg_cnt;
@@ -860,10 +877,14 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd)
ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
ctrl &= ~SDHCI_CTRL_DMA_MASK;
if ((host->flags & SDHCI_REQ_USE_DMA) &&
- (host->flags & SDHCI_USE_ADMA))
- ctrl |= SDHCI_CTRL_ADMA32;
- else
+ (host->flags & SDHCI_USE_ADMA)) {
+ if (host->flags & SDHCI_USE_64_BIT_DMA)
+ ctrl |= SDHCI_CTRL_ADMA64;
+ else
+ ctrl |= SDHCI_CTRL_ADMA32;
+ } else {
ctrl |= SDHCI_CTRL_SDMA;
+ }
sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
}

@@ -2295,12 +2316,19 @@ static void sdhci_adma_show_error(struct sdhci_host *host)
sdhci_dumpregs(host);

while (true) {
- struct sdhci_adma2_32_desc *dma_desc = desc;
-
- DBG("%s: %p: DMA 0x%08x, LEN 0x%04x, Attr=0x%02x\n",
- name, desc, le32_to_cpu(dma_desc->addr),
- le16_to_cpu(dma_desc->len),
- le16_to_cpu(dma_desc->cmd));
+ struct sdhci_adma2_64_desc *dma_desc = desc;
+
+ if (host->flags & SDHCI_USE_64_BIT_DMA)
+ DBG("%s: %p: DMA 0x%08x%08x, LEN 0x%04x, Attr=0x%02x\n",
+ name, desc, le32_to_cpu(dma_desc->addr_hi),
+ le32_to_cpu(dma_desc->addr_lo),
+ le16_to_cpu(dma_desc->len),
+ le16_to_cpu(dma_desc->cmd));
+ else
+ DBG("%s: %p: DMA 0x%08x, LEN 0x%04x, Attr=0x%02x\n",
+ name, desc, le32_to_cpu(dma_desc->addr_lo),
+ le16_to_cpu(dma_desc->len),
+ le16_to_cpu(dma_desc->cmd));

desc += host->desc_sz;

@@ -2851,6 +2879,16 @@ int sdhci_add_host(struct sdhci_host *host)
host->flags &= ~SDHCI_USE_ADMA;
}

+ /*
+ * It is assumed that a 64-bit capable device has set a 64-bit DMA mask
+ * and *must* do 64-bit DMA. A driver has the opportunity to change
+ * that during the first call to ->enable_dma(). Similarly
+ * SDHCI_QUIRK2_BROKEN_64_BIT_DMA must be left to the drivers to
+ * implement.
+ */
+ if (sdhci_readl(host, SDHCI_CAPABILITIES) & SDHCI_CAN_64BIT)
+ host->flags |= SDHCI_USE_64_BIT_DMA;
+
if (host->flags & (SDHCI_USE_SDMA | SDHCI_USE_ADMA)) {
if (host->ops->enable_dma) {
if (host->ops->enable_dma(host)) {
@@ -2862,6 +2900,10 @@ int sdhci_add_host(struct sdhci_host *host)
}
}

+ /* SDMA does not support 64-bit DMA */
+ if (host->flags & SDHCI_USE_64_BIT_DMA)
+ host->flags &= ~SDHCI_USE_SDMA;
+
if (host->flags & SDHCI_USE_ADMA) {
/*
* The DMA descriptor table size is calculated as the maximum
@@ -2869,13 +2911,23 @@ int sdhci_add_host(struct sdhci_host *host)
* descriptor for each segment, plus 1 for a nop end descriptor,
* all multipled by the descriptor size.
*/
- host->adma_table_sz = (SDHCI_MAX_SEGS * 2 + 1) *
- SDHCI_ADMA2_32_DESC_SZ;
- host->align_buffer_sz = SDHCI_MAX_SEGS *
- SDHCI_ADMA2_32_ALIGN;
- host->desc_sz = SDHCI_ADMA2_32_DESC_SZ;
- host->align_sz = SDHCI_ADMA2_32_ALIGN;
- host->align_mask = SDHCI_ADMA2_32_ALIGN - 1;
+ if (host->flags & SDHCI_USE_64_BIT_DMA) {
+ host->adma_table_sz = (SDHCI_MAX_SEGS * 2 + 1) *
+ SDHCI_ADMA2_64_DESC_SZ;
+ host->align_buffer_sz = SDHCI_MAX_SEGS *
+ SDHCI_ADMA2_64_ALIGN;
+ host->desc_sz = SDHCI_ADMA2_64_DESC_SZ;
+ host->align_sz = SDHCI_ADMA2_64_ALIGN;
+ host->align_mask = SDHCI_ADMA2_64_ALIGN - 1;
+ } else {
+ host->adma_table_sz = (SDHCI_MAX_SEGS * 2 + 1) *
+ SDHCI_ADMA2_32_DESC_SZ;
+ host->align_buffer_sz = SDHCI_MAX_SEGS *
+ SDHCI_ADMA2_32_ALIGN;
+ host->desc_sz = SDHCI_ADMA2_32_DESC_SZ;
+ host->align_sz = SDHCI_ADMA2_32_ALIGN;
+ host->align_mask = SDHCI_ADMA2_32_ALIGN - 1;
+ }
host->adma_table = dma_alloc_coherent(mmc_dev(mmc),
host->adma_table_sz,
&host->adma_addr,
@@ -3288,7 +3340,8 @@ int sdhci_add_host(struct sdhci_host *host)

pr_info("%s: SDHCI controller on %s [%s] using %s\n",
mmc_hostname(mmc), host->hw_name, dev_name(mmc_dev(mmc)),
- (host->flags & SDHCI_USE_ADMA) ? "ADMA" :
+ (host->flags & SDHCI_USE_ADMA) ?
+ (host->flags & SDHCI_USE_64_BIT_DMA) ? "ADMA 64-bit" : "ADMA" :
(host->flags & SDHCI_USE_SDMA) ? "DMA" : "PIO");

sdhci_enable_card_detection(host);
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 14c8b67..c2ec7fc 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -227,6 +227,7 @@
/* 55-57 reserved */

#define SDHCI_ADMA_ADDRESS 0x58
+#define SDHCI_ADMA_ADDRESS_HI 0x5C

/* 60-FB reserved */

@@ -279,6 +280,23 @@ struct sdhci_adma2_32_desc {
__le32 addr;
} __packed __aligned(SDHCI_ADMA2_32_ALIGN);

+/* ADMA2 64-bit DMA descriptor size */
+#define SDHCI_ADMA2_64_DESC_SZ 12
+
+/* ADMA2 64-bit DMA alignment */
+#define SDHCI_ADMA2_64_ALIGN 8
+
+/*
+ * ADMA2 64-bit descriptor. Note 12-byte descriptor can't always be 8-byte
+ * aligned.
+ */
+struct sdhci_adma2_64_desc {
+ __le16 cmd;
+ __le16 len;
+ __le32 addr_lo;
+ __le32 addr_hi;
+} __packed __aligned(4);
+
#define ADMA2_TRAN_VALID 0x21
#define ADMA2_NOP_END_VALID 0x3
#define ADMA2_END 0x2
diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index 2a72e95..931ac5e 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -100,6 +100,8 @@ struct sdhci_host {
#define SDHCI_QUIRK2_BROKEN_DDR50 (1<<7)
/* Stop command (CMD12) can set Transfer Complete when not using MMC_RSP_BUSY */
#define SDHCI_QUIRK2_STOP_WITH_TC (1<<8)
+/* Controller does not support 64-bit DMA */
+#define SDHCI_QUIRK2_BROKEN_64_BIT_DMA (1<<9)

int irq; /* Device IRQ */
void __iomem *ioaddr; /* Mapped address */
@@ -130,6 +132,7 @@ struct sdhci_host {
#define SDHCI_SDIO_IRQ_ENABLED (1<<9) /* SDIO irq enabled */
#define SDHCI_SDR104_NEEDS_TUNING (1<<10) /* SDR104/HS200 needs tuning */
#define SDHCI_USING_RETUNING_TIMER (1<<11) /* Host is using a retuning timer for the card */
+#define SDHCI_USE_64_BIT_DMA (1<<12) /* Use 64-bit DMA */

unsigned int version; /* SDHCI spec. version */
--
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Adrian Hunter
2014-10-21 09:26:24 UTC
Permalink
Set a 64-bit DMA mask when using 64-bit DMA.

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

diff --git a/drivers/mmc/host/sdhci-pci.c b/drivers/mmc/host/sdhci-pci.c
index 6119297..c25639b 100644
--- a/drivers/mmc/host/sdhci-pci.c
+++ b/drivers/mmc/host/sdhci-pci.c
@@ -1064,7 +1064,7 @@ static int sdhci_pci_enable_dma(struct sdhci_host *host)
{
struct sdhci_pci_slot *slot;
struct pci_dev *pdev;
- int ret;
+ int ret = -1;

slot = sdhci_priv(host);
pdev = slot->chip->pdev;
@@ -1076,7 +1076,17 @@ static int sdhci_pci_enable_dma(struct sdhci_host *host)
"doesn't fully claim to support it.\n");
}

- ret = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
+ if (host->flags & SDHCI_USE_64_BIT_DMA) {
+ if (host->quirks2 & SDHCI_QUIRK2_BROKEN_64_BIT_DMA) {
+ host->flags &= ~SDHCI_USE_64_BIT_DMA;
+ } else {
+ ret = pci_set_dma_mask(pdev, DMA_BIT_MASK(64));
+ if (ret)
+ dev_warn(&pdev->dev, "Failed to set 64-bit DMA mask\n");
+ }
+ }
+ if (ret)
+ ret = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
if (ret)
return ret;
--
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Adrian Hunter
2014-10-21 09:26:23 UTC
Permalink
Set the DMA mask during the first call to ->enable_dma() to
make use of the SDHCI_USE_64_BIT_DMA flag.

Signed-off-by: Adrian Hunter <***@intel.com>
---
drivers/mmc/host/sdhci-acpi.c | 41 +++++++++++++++++++++++++----------------
1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
index 9cccc0e..637c6c4 100644
--- a/drivers/mmc/host/sdhci-acpi.c
+++ b/drivers/mmc/host/sdhci-acpi.c
@@ -76,6 +76,7 @@ struct sdhci_acpi_host {
const struct sdhci_acpi_slot *slot;
struct platform_device *pdev;
bool use_runtime_pm;
+ bool dma_setup;
};

static inline bool sdhci_acpi_flag(struct sdhci_acpi_host *c, unsigned int flag)
@@ -85,7 +86,30 @@ static inline bool sdhci_acpi_flag(struct sdhci_acpi_host *c, unsigned int flag)

static int sdhci_acpi_enable_dma(struct sdhci_host *host)
{
- return 0;
+ struct sdhci_acpi_host *c = sdhci_priv(host);
+ struct device *dev = &c->pdev->dev;
+ int err = -1;
+
+ if (c->dma_setup)
+ return 0;
+
+ if (host->flags & SDHCI_USE_64_BIT_DMA) {
+ if (host->quirks2 & SDHCI_QUIRK2_BROKEN_64_BIT_DMA) {
+ host->flags &= ~SDHCI_USE_64_BIT_DMA;
+ } else {
+ err = dma_coerce_mask_and_coherent(dev,
+ DMA_BIT_MASK(64));
+ if (err)
+ dev_warn(dev, "Failed to set 64-bit DMA mask\n");
+ }
+ }
+
+ if (err)
+ err = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32));
+
+ c->dma_setup = !err;
+
+ return err;
}

static void sdhci_acpi_int_hw_reset(struct sdhci_host *host)
@@ -305,21 +329,6 @@ static int sdhci_acpi_probe(struct platform_device *pdev)
goto err_free;
}

- if (!dev->dma_mask) {
- u64 dma_mask;
-
- if (sdhci_readl(host, SDHCI_CAPABILITIES) & SDHCI_CAN_64BIT) {
- /* 64-bit DMA is not supported at present */
- dma_mask = DMA_BIT_MASK(32);
- } else {
- dma_mask = DMA_BIT_MASK(32);
- }
-
- err = dma_coerce_mask_and_coherent(dev, dma_mask);
- if (err)
- goto err_free;
- }
-
if (c->slot) {
if (c->slot->probe_slot) {
err = c->slot->probe_slot(pdev, hid, uid);
--
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Arnd Bergmann
2014-10-21 09:33:12 UTC
Permalink
+ return 0;
+
+ if (host->flags & SDHCI_USE_64_BIT_DMA) {
+ if (host->quirks2 & SDHCI_QUIRK2_BROKEN_64_BIT_DMA) {
+ host->flags &= ~SDHCI_USE_64_BIT_DMA;
+ } else {
+ err = dma_coerce_mask_and_coherent(dev,
+ DMA_BIT_MASK(64));
+ if (err)
+ dev_warn(dev, "Failed to set 64-bit DMA mask\n");
+ }
+ }
+
+ if (err)
+ err = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32));
+
+ c->dma_setup = !err;
Never use dma_coerce_mask_and_coherent(), it will ignore limitations of
the upstream bus. Use dma_set_mask_and_coherent instead. If it fails,
I think you need to clear the SDHCI_USE_64_BIT_DMA flag.

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Adrian Hunter
2014-10-21 10:45:20 UTC
Permalink
Post by Arnd Bergmann
+ return 0;
+
+ if (host->flags & SDHCI_USE_64_BIT_DMA) {
+ if (host->quirks2 & SDHCI_QUIRK2_BROKEN_64_BIT_DMA) {
+ host->flags &= ~SDHCI_USE_64_BIT_DMA;
+ } else {
+ err = dma_coerce_mask_and_coherent(dev,
+ DMA_BIT_MASK(64));
+ if (err)
+ dev_warn(dev, "Failed to set 64-bit DMA mask\n");
+ }
+ }
+
+ if (err)
+ err = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32));
+
+ c->dma_setup = !err;
Never use dma_coerce_mask_and_coherent(), it will ignore limitations of
the upstream bus. Use dma_set_mask_and_coherent instead. If it fails,
I think you need to clear the SDHCI_USE_64_BIT_DMA flag.
The sdhci-acpi device is a platform devices created by acpi. dev->dma_mask
is NULL so dma_set_mask_and_coherent will always fail
unless I add:

if (!dev->dma_mask)
dev->dma_mask = &dev->coherent_dma_mask;

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Arnd Bergmann
2014-10-21 11:05:36 UTC
Permalink
Post by Adrian Hunter
Post by Arnd Bergmann
+ return 0;
+
+ if (host->flags & SDHCI_USE_64_BIT_DMA) {
+ if (host->quirks2 & SDHCI_QUIRK2_BROKEN_64_BIT_DMA) {
+ host->flags &= ~SDHCI_USE_64_BIT_DMA;
+ } else {
+ err = dma_coerce_mask_and_coherent(dev,
+ DMA_BIT_MASK(64));
+ if (err)
+ dev_warn(dev, "Failed to set 64-bit DMA mask\n");
+ }
+ }
+
+ if (err)
+ err = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32));
+
+ c->dma_setup = !err;
Never use dma_coerce_mask_and_coherent(), it will ignore limitations of
the upstream bus. Use dma_set_mask_and_coherent instead. If it fails,
I think you need to clear the SDHCI_USE_64_BIT_DMA flag.
The sdhci-acpi device is a platform devices created by acpi. dev->dma_mask
is NULL so dma_set_mask_and_coherent will always fail
if (!dev->dma_mask)
dev->dma_mask = &dev->coherent_dma_mask;
That's a bug in the ACPI code. Fix that instead of working around it in random
drivers.

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Adrian Hunter
2014-10-21 09:26:25 UTC
Permalink
The "Badly aligned" tests, test reading/writing with alignments
of 1,2 and 3. SDHCI now has 64-bit ADMA which has 8-byte
alignment, so extend the tests to test up to 7.

Signed-off-by: Adrian Hunter <***@intel.com>
---
drivers/mmc/card/mmc_test.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/card/mmc_test.c b/drivers/mmc/card/mmc_test.c
index 0c0fc52..d7f986b 100644
--- a/drivers/mmc/card/mmc_test.c
+++ b/drivers/mmc/card/mmc_test.c
@@ -32,6 +32,8 @@
#define BUFFER_ORDER 2
#define BUFFER_SIZE (PAGE_SIZE << BUFFER_ORDER)

+#define TEST_ALIGN_END 8
+
/*
* Limit the test area size to the maximum MMC HC erase group size. Note that
* the maximum SD allocation unit size is just 4MiB.
@@ -1174,7 +1176,7 @@ static int mmc_test_align_write(struct mmc_test_card *test)
int ret, i;
struct scatterlist sg;

- for (i = 1;i < 4;i++) {
+ for (i = 1; i < TEST_ALIGN_END; i++) {
sg_init_one(&sg, test->buffer + i, 512);
ret = mmc_test_transfer(test, &sg, 1, 0, 1, 512, 1);
if (ret)
@@ -1189,7 +1191,7 @@ static int mmc_test_align_read(struct mmc_test_card *test)
int ret, i;
struct scatterlist sg;

- for (i = 1;i < 4;i++) {
+ for (i = 1; i < TEST_ALIGN_END; i++) {
sg_init_one(&sg, test->buffer + i, 512);
ret = mmc_test_transfer(test, &sg, 1, 0, 1, 512, 0);
if (ret)
@@ -1216,7 +1218,7 @@ static int mmc_test_align_multi_write(struct mmc_test_card *test)
if (size < 1024)
return RESULT_UNSUP_HOST;

- for (i = 1;i < 4;i++) {
+ for (i = 1; i < TEST_ALIGN_END; i++) {
sg_init_one(&sg, test->buffer + i, size);
ret = mmc_test_transfer(test, &sg, 1, 0, size/512, 512, 1);
if (ret)
@@ -1243,7 +1245,7 @@ static int mmc_test_align_multi_read(struct mmc_test_card *test)
if (size < 1024)
return RESULT_UNSUP_HOST;

- for (i = 1;i < 4;i++) {
+ for (i = 1; i < TEST_ALIGN_END; i++) {
sg_init_one(&sg, test->buffer + i, size);
ret = mmc_test_transfer(test, &sg, 1, 0, size/512, 512, 0);
if (ret)
--
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Loading...