[PATCH 0/7] mmc: Some fixes

[PATCH 0/7] mmc: Some fixes

[Adrian Hunter]

Hi

Here are some fixes.  The important ones have cc: stable.
It doesn't matter to me whether you queue them as fixes
or for v4.5.


Adrian Hunter (6):
      mmc: sdhci-pci: Do not default to 33 Ohm driver strength for Intel SPT
      mmc: sdhci: Do not BUG on invalid vdd
      mmc: sdio: Fix invalid vdd in voltage switch power cycle
      mmc: sdhc: Fix DMA descriptor with zero data length
      mmc: sdhci: 64-bit DMA actually has 4-byte alignment
      mmc: sdhci: Fix sdhci_runtime_pm_bus_on/off()

Wenkai Du (1):
      mmc: mmc: Fix incorrect use of driver strength switching HS200 and HS400

 drivers/mmc/core/mmc.c            |  6 ++---
 drivers/mmc/core/sdio.c           |  2 +-
 drivers/mmc/host/sdhci-pci-core.c |  2 +-
 drivers/mmc/host/sdhci.c          | 49 +++++++++++++++++++--------------------
 drivers/mmc/host/sdhci.h          | 21 ++++++++++-------
 5 files changed, 40 insertions(+), 40 deletions(-)


Regards
Adrian

[PATCH 1/7] mmc: mmc: Fix incorrect use of driver strength switching HS200 and HS400

[Adrian Hunter]

From: Wenkai Du <wenkai.du@intel.com>

Commit cc4f414c885c ("mmc: mmc: Add driver strength selection")
added driver strength selection for eMMC HS200 and HS400 modes.
That patch also set the driver stength when transitioning through
High Speed mode to HS200/HS400, but driver strength is not defined
for High Speed mode.  While the JEDEC specification is not clear
on this point it has been observed to cause problems for some eMMC,
and removing the driver strength setting in this case makes it
consistent with the normal use of High Speed mode.

Signed-off-by: Wenkai Du <wenkai.du@intel.com>
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Cc: stable@vger.kernel.org # v4.2+
---
 drivers/mmc/core/mmc.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 66957addf9e4..549c56e8cf6b 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1076,8 +1076,7 @@ static int mmc_select_hs400(struct mmc_card *card)
 	mmc_set_clock(host, max_dtr);
 
 	/* Switch card to HS mode */
-	val = EXT_CSD_TIMING_HS |
-	      card->drive_strength << EXT_CSD_DRV_STR_SHIFT;
+	val = EXT_CSD_TIMING_HS;
 	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
 			   EXT_CSD_HS_TIMING, val,
 			   card->ext_csd.generic_cmd6_time,
@@ -1160,8 +1159,7 @@ int mmc_hs400_to_hs200(struct mmc_card *card)
 	mmc_set_clock(host, max_dtr);
 
 	/* Switch HS400 to HS DDR */
-	val = EXT_CSD_TIMING_HS |
-	      card->drive_strength << EXT_CSD_DRV_STR_SHIFT;
+	val = EXT_CSD_TIMING_HS;
 	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING,
 			   val, card->ext_csd.generic_cmd6_time,
 			   true, send_status, true);
-- 
1.9.1

[PATCH 2/7] mmc: sdhci-pci: Do not default to 33 Ohm driver strength for Intel SPT

[Adrian Hunter]

In some cases, the stronger 33 Ohm driver strength must not be used
so it is not a suitable default.  Change it to the standard default
50 Ohm value.

The patch applies to v4.2+ except the file name changed.  It is
drivers/mmc/host/sdhci-pci.c prior to v.4.4.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Cc: stable@vger.kernel.org # v4.2+
---
 drivers/mmc/host/sdhci-pci-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
index cf7ad458b4f4..08f4a9fe8550 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -277,7 +277,7 @@ static int spt_select_drive_strength(struct sdhci_host *host,
 	if (sdhci_pci_spt_drive_strength > 0)
 		drive_strength = sdhci_pci_spt_drive_strength & 0xf;
 	else
-		drive_strength = 1; /* 33-ohm */
+		drive_strength = 0; /* Default 50-ohm */
 
 	if ((mmc_driver_type_mask(drive_strength) & card_drv) == 0)
 		drive_strength = 0; /* Default 50-ohm */
-- 
1.9.1

[PATCH 3/7] mmc: sdhci: Do not BUG on invalid vdd

[Adrian Hunter]

The driver may not be able to set the power correctly but that
is not a reason to BUG().

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

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 2b17cc1246ca..5f8b0766428c 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1299,7 +1299,10 @@ static void sdhci_set_power(struct sdhci_host *host, unsigned char mode,
 			pwr = SDHCI_POWER_330;
 			break;
 		default:
-			BUG();
+			WARN(1, "%s: Invalid vdd %#x\n",
+			     mmc_hostname(host->mmc), vdd);
+			pwr = 0;
+			break;
 		}
 	}
 
-- 
1.9.1

[PATCH 4/7] mmc: sdio: Fix invalid vdd in voltage switch power cycle

[Adrian Hunter]

The 'ocr' parameter passed to mmc_set_signal_voltage()
defines the power-on voltage used when power cycling
after a failure to set the voltage.  However, in the
case of mmc_sdio_init_card(), the value passed has the
R4_18V_PRESENT flag set which is not valid for power-on
and results in an invalid vdd.  Fix by passing the card's
ocr value which does not have the flag.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Cc: stable@vger.kernel.org # v3.13+
---
 drivers/mmc/core/sdio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index 16d838e6d623..d61ba1a0495e 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -630,7 +630,7 @@ try_again:
 	 */
 	if (!powered_resume && (rocr & ocr & R4_18V_PRESENT)) {
 		err = mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_180,
-					ocr);
+					ocr_card);
 		if (err == -EAGAIN) {
 			sdio_reset(host);
 			mmc_go_idle(host);
-- 
1.9.1

[PATCH 5/7] mmc: sdhc: Fix DMA descriptor with zero data length

[Adrian Hunter]

SDHCI has built-in DMA called ADMA2.  ADMA2 uses a descriptor
table to define DMA scatter-gather.  Each desciptor can specify
a data length up to 65536 bytes, however the length field is
only 16-bits so zero means 65536.  Consequently, putting zero
when the size is zero must not be allowed.  This patch fixes
one case where zero data length could be set inadvertently.

The problem happens because unaligned data gets split and the
code did not consider that the remaining aligned portion might
be zero length.  That case really only happens for SDIO because
SD and eMMC cards transfer blocks that are invariably sector-
aligned.  For SDIO, access to function registers is done by
data transfer (CMD53) when the register is bigger than 1 byte.
Generally registers are 4 bytes but 2-byte registers are possible.
So DMA of 4 bytes or less can happen.  When 32-bit DMA is used,
the data alignment must be 4, so 4-byte transfers won't casue a
problem, but a 2-byte transfer could.  However with the introduction
of 64-bit DMA, the data alignment for 64-bit DMA was made 8 bytes,
so all 4-byte transfers not on 8-byte boundaries get "split" into
a 4-byte chunk and a 0-byte chunk, thereby hitting the bug.

In fact, a closer look at the SDHCI specs indicates that only the
descriptor table requires 8-byte alignment for 64-bit DMA.  That
will be dealt with in a separate patch, but the potential for a
2-byte access remains, so this fix is needed anyway.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Cc: stable@vger.kernel.org # v3.19+
---
 drivers/mmc/host/sdhci.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 5f8b0766428c..6ac5db8e86ab 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -540,9 +540,12 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,
 
 		BUG_ON(len > 65536);
 
-		/* tran, valid */
-		sdhci_adma_write_desc(host, desc, addr, len, ADMA2_TRAN_VALID);
-		desc += host->desc_sz;
+		if (len) {
+			/* tran, valid */
+			sdhci_adma_write_desc(host, desc, addr, len,
+					      ADMA2_TRAN_VALID);
+			desc += host->desc_sz;
+		}
 
 		/*
 		 * If this triggers then we have a calculation bug
-- 
1.9.1

[PATCH 6/7] mmc: sdhci: 64-bit DMA actually has 4-byte alignment

[Adrian Hunter]

The version 3.00 SDHCI spec. was a bit unclear about the
required data alignment for 64-bit DMA, whereas the version
4.10 spec. uses different language and indicates that only
4-byte alignment is required rather than the 8-byte alignment
currently implemented.  That make no difference to SD and EMMC
which invariably transfer data in sector-aligned blocks.
However with SDIO, it results in using more DMA descriptors
than necessary.  Theoretically that slows DMA slightly although
DMA is not the limiting factor for throughput, so there is no
discernable impact on performance.  Nevertheless, the driver
should follw the spec unless there is good reason not to, so
this patch corrects the alignment criterion.

There is a more complicated criterion for the DMA descriptor
table itself.  However the table is allocated by dma_alloc_coherent()
which allocates pages (i.e. aligned to a page boundary).
For simplicity just check it is 8-byte aligned, but add a comment
that some Intel controllers actually require 8-byte alignment
even when using 32-bit DMA.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci.c | 31 ++++++++++++-------------------
 drivers/mmc/host/sdhci.h | 21 ++++++++++++---------
 2 files changed, 24 insertions(+), 28 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 6ac5db8e86ab..7fe326f76fb7 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -492,7 +492,7 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,
 		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 & host->align_mask);
+	BUG_ON(host->align_addr & SDHCI_ADMA2_MASK);
 
 	host->sg_count = sdhci_pre_dma_transfer(host, data);
 	if (host->sg_count < 0)
@@ -514,8 +514,8 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,
 		 * the (up to three) bytes that screw up the
 		 * alignment.
 		 */
-		offset = (host->align_sz - (addr & host->align_mask)) &
-			 host->align_mask;
+		offset = (SDHCI_ADMA2_ALIGN - (addr & SDHCI_ADMA2_MASK)) &
+			 SDHCI_ADMA2_MASK;
 		if (offset) {
 			if (data->flags & MMC_DATA_WRITE) {
 				buffer = sdhci_kmap_atomic(sg, &flags);
@@ -529,8 +529,8 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,
 
 			BUG_ON(offset > 65536);
 
-			align += host->align_sz;
-			align_addr += host->align_sz;
+			align += SDHCI_ADMA2_ALIGN;
+			align_addr += SDHCI_ADMA2_ALIGN;
 
 			desc += host->desc_sz;
 
@@ -611,7 +611,7 @@ static void sdhci_adma_table_post(struct sdhci_host *host,
 	/* 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) & host->align_mask) {
+		if (sg_dma_address(sg) & SDHCI_ADMA2_MASK) {
 			has_unaligned = true;
 			break;
 		}
@@ -623,15 +623,15 @@ 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) & host->align_mask) {
-				size = host->align_sz -
-				       (sg_dma_address(sg) & host->align_mask);
+			if (sg_dma_address(sg) & SDHCI_ADMA2_MASK) {
+				size = SDHCI_ADMA2_ALIGN -
+				       (sg_dma_address(sg) & SDHCI_ADMA2_MASK);
 
 				buffer = sdhci_kmap_atomic(sg, &flags);
 				memcpy(buffer, align, size);
 				sdhci_kunmap_atomic(buffer, &flags);
 
-				align += host->align_sz;
+				align += SDHCI_ADMA2_ALIGN;
 			}
 		}
 	}
@@ -2962,24 +2962,17 @@ int sdhci_add_host(struct sdhci_host *host)
 		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,
 						      GFP_KERNEL);
+		host->align_buffer_sz = SDHCI_MAX_SEGS * SDHCI_ADMA2_ALIGN;
 		host->align_buffer = kmalloc(host->align_buffer_sz, GFP_KERNEL);
 		if (!host->adma_table || !host->align_buffer) {
 			if (host->adma_table)
@@ -2993,7 +2986,7 @@ 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 & host->align_mask) {
+		} else if (host->adma_addr & (SDHCI_ADMA2_DESC_ALIGN - 1)) {
 			pr_warn("%s: unable to allocate aligned ADMA descriptor\n",
 				mmc_hostname(mmc));
 			host->flags &= ~SDHCI_USE_ADMA;
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 9d4aa31b683a..7654ae5d2b4e 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -272,22 +272,27 @@
 /* ADMA2 32-bit DMA descriptor size */
 #define SDHCI_ADMA2_32_DESC_SZ	8
 
-/* 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);
+}  __packed __aligned(4);
+
+/* ADMA2 data alignment */
+#define SDHCI_ADMA2_ALIGN	4
+#define SDHCI_ADMA2_MASK	(SDHCI_ADMA2_ALIGN - 1)
+
+/*
+ * ADMA2 descriptor alignment.  Some controllers (e.g. Intel) require 8 byte
+ * alignment for the descriptor table even in 32-bit DMA mode.  Memory
+ * allocation is at least 8 byte aligned anyway, so just stipulate 8 always.
+ */
+#define SDHCI_ADMA2_DESC_ALIGN	8
 
 /* 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.
@@ -482,8 +487,6 @@ struct sdhci_host {
 	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 */
 
-- 
1.9.1

[PATCH 7/7] mmc: sdhci: Fix sdhci_runtime_pm_bus_on/off()

[Adrian Hunter]

sdhci has a legacy facility to prevent runtime suspend if the
bus power is on.  This is needed in cases where the power to
the card is dependent on the bus power.  It is controlled by
a pair of functions: sdhci_runtime_pm_bus_on() and
sdhci_runtime_pm_bus_off().  These functions use a boolean
variable 'bus_on' to ensure changes are always paired.
There is an additional check for 'runtime_suspended' which is
the problem.  In fact, its use is ill-conceived as the only
requirement for the logic is that 'on' and 'off' are paired,
which is actually broken by the check, for example if the bus
power is turned on during runtime resume.  So remove  the check.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Cc: stable@vger.kernel.org # v3.11+
---
 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 7fe326f76fb7..d82b90928585 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2756,7 +2756,7 @@ static int sdhci_runtime_pm_put(struct sdhci_host *host)
 
 static void sdhci_runtime_pm_bus_on(struct sdhci_host *host)
 {
-	if (host->runtime_suspended || host->bus_on)
+	if (host->bus_on)
 		return;
 	host->bus_on = true;
 	pm_runtime_get_noresume(host->mmc->parent);
@@ -2764,7 +2764,7 @@ static void sdhci_runtime_pm_bus_on(struct sdhci_host *host)
 
 static void sdhci_runtime_pm_bus_off(struct sdhci_host *host)
 {
-	if (host->runtime_suspended || !host->bus_on)
+	if (!host->bus_on)
 		return;
 	host->bus_on = false;
 	pm_runtime_put_noidle(host->mmc->parent);
-- 
1.9.1

RE: [PATCH 3/7] mmc: sdhci: Do not BUG on invalid vdd

[Venu Byravarasu]

> -----Original Message-----
> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
> owner@vger.kernel.org] On Behalf Of Adrian Hunter
> Sent: Thursday, November 26, 2015 5:31 PM
> To: Ulf Hansson
> Cc: linux-mmc; Jaehoon Chung; Chaotian Jing; Wenkai Du
> Subject: [PATCH 3/7] mmc: sdhci: Do not BUG on invalid vdd
> 
> The driver may not be able to set the power correctly but that is not a reason
> to BUG().
> 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>  drivers/mmc/host/sdhci.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index
> 2b17cc1246ca..5f8b0766428c 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1299,7 +1299,10 @@ static void sdhci_set_power(struct sdhci_host
> *host, unsigned char mode,
>  			pwr = SDHCI_POWER_330;
>  			break;
>  		default:
> -			BUG();
> +			WARN(1, "%s: Invalid vdd %#x\n",
> +			     mmc_hostname(host->mmc), vdd);
> +			pwr = 0;

As pwr is initialized to 0 during declaration, don't see a need for above statement.
However agree with your point that BUG can be replaced with a warn message.

> +			break;
>  		}
>  	}
> 
> --
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the
> body of a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html
-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

Re: [PATCH 0/7] mmc: Some fixes

[Ulf Hansson]

On 26 November 2015 at 13:00, Adrian Hunter <adrian.hunter@intel.com> wrote:
> Hi
>
> Here are some fixes.  The important ones have cc: stable.
> It doesn't matter to me whether you queue them as fixes
> or for v4.5.
>
>
> Adrian Hunter (6):
>       mmc: sdhci-pci: Do not default to 33 Ohm driver strength for Intel SPT
>       mmc: sdhci: Do not BUG on invalid vdd
>       mmc: sdio: Fix invalid vdd in voltage switch power cycle
>       mmc: sdhc: Fix DMA descriptor with zero data length
>       mmc: sdhci: 64-bit DMA actually has 4-byte alignment
>       mmc: sdhci: Fix sdhci_runtime_pm_bus_on/off()
>
> Wenkai Du (1):
>       mmc: mmc: Fix incorrect use of driver strength switching HS200 and HS400
>
>  drivers/mmc/core/mmc.c            |  6 ++---
>  drivers/mmc/core/sdio.c           |  2 +-
>  drivers/mmc/host/sdhci-pci-core.c |  2 +-
>  drivers/mmc/host/sdhci.c          | 49 +++++++++++++++++++--------------------
>  drivers/mmc/host/sdhci.h          | 21 ++++++++++-------
>  5 files changed, 40 insertions(+), 40 deletions(-)
>
>
> Regards
> Adrian


Thanks, applied for next!

Kind regards
Uffe

Re: [PATCH 5/7] mmc: sdhc: Fix DMA descriptor with zero data length

[Ulf Hansson]

On 26 November 2015 at 13:00, Adrian Hunter <adrian.hunter@intel.com> wrote:
> SDHCI has built-in DMA called ADMA2.  ADMA2 uses a descriptor
> table to define DMA scatter-gather.  Each desciptor can specify
> a data length up to 65536 bytes, however the length field is
> only 16-bits so zero means 65536.  Consequently, putting zero
> when the size is zero must not be allowed.  This patch fixes
> one case where zero data length could be set inadvertently.
>
> The problem happens because unaligned data gets split and the
> code did not consider that the remaining aligned portion might
> be zero length.  That case really only happens for SDIO because
> SD and eMMC cards transfer blocks that are invariably sector-
> aligned.  For SDIO, access to function registers is done by
> data transfer (CMD53) when the register is bigger than 1 byte.
> Generally registers are 4 bytes but 2-byte registers are possible.
> So DMA of 4 bytes or less can happen.  When 32-bit DMA is used,
> the data alignment must be 4, so 4-byte transfers won't casue a
> problem, but a 2-byte transfer could.  However with the introduction
> of 64-bit DMA, the data alignment for 64-bit DMA was made 8 bytes,
> so all 4-byte transfers not on 8-byte boundaries get "split" into
> a 4-byte chunk and a 0-byte chunk, thereby hitting the bug.
>
> In fact, a closer look at the SDHCI specs indicates that only the
> descriptor table requires 8-byte alignment for 64-bit DMA.  That
> will be dealt with in a separate patch, but the potential for a
> 2-byte access remains, so this fix is needed anyway.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> Cc: stable@vger.kernel.org # v3.19+

I updated the prefix of the commit msg header, /s/sdhc/sdhci

Kind regards
Uffe


> ---
>  drivers/mmc/host/sdhci.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 5f8b0766428c..6ac5db8e86ab 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -540,9 +540,12 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,
>
>                 BUG_ON(len > 65536);
>
> -               /* tran, valid */
> -               sdhci_adma_write_desc(host, desc, addr, len, ADMA2_TRAN_VALID);
> -               desc += host->desc_sz;
> +               if (len) {
> +                       /* tran, valid */
> +                       sdhci_adma_write_desc(host, desc, addr, len,
> +                                             ADMA2_TRAN_VALID);
> +                       desc += host->desc_sz;
> +               }
>
>                 /*
>                  * If this triggers then we have a calculation bug
> --
> 1.9.1
>

Re: [PATCH 3/7] mmc: sdhci: Do not BUG on invalid vdd

[Ulf Hansson]

On 27 November 2015 at 06:08, Venu Byravarasu <vbyravarasu@nvidia.com> wrote:
>
>
>> -----Original Message-----
>> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
>> owner@vger.kernel.org] On Behalf Of Adrian Hunter
>> Sent: Thursday, November 26, 2015 5:31 PM
>> To: Ulf Hansson
>> Cc: linux-mmc; Jaehoon Chung; Chaotian Jing; Wenkai Du
>> Subject: [PATCH 3/7] mmc: sdhci: Do not BUG on invalid vdd
>>
>> The driver may not be able to set the power correctly but that is not a reason
>> to BUG().
>>
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>> ---
>>  drivers/mmc/host/sdhci.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index
>> 2b17cc1246ca..5f8b0766428c 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -1299,7 +1299,10 @@ static void sdhci_set_power(struct sdhci_host
>> *host, unsigned char mode,
>>                       pwr = SDHCI_POWER_330;
>>                       break;
>>               default:
>> -                     BUG();
>> +                     WARN(1, "%s: Invalid vdd %#x\n",
>> +                          mmc_hostname(host->mmc), vdd);
>> +                     pwr = 0;
>
> As pwr is initialized to 0 during declaration, don't see a need for above statement.
> However agree with your point that BUG can be replaced with a warn message.
>

I have updated the patch accordingly and added Venu's reviewed-by tag.

Kind regards
Uffe


>> +                     break;
>>               }
>>       }
>>
>> --
>> 1.9.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the
>> body of a message to majordomo@vger.kernel.org More majordomo info at
>> http://vger.kernel.org/majordomo-info.html
> -----------------------------------------------------------------------------------
> This email message is for the sole use of the intended recipient(s) and may contain
> confidential information.  Any unauthorized review, use, disclosure or distribution
> is prohibited.  If you are not the intended recipient, please contact the sender by
> reply email and destroy all copies of the original message.
> -----------------------------------------------------------------------------------