[PATCH 0/9] Enable dw-mmc multi-card support

[PATCH 0/9] Enable dw-mmc multi-card support

[Liming Sun]

This series of commits enables the multi-card support for the dw-mmc
controller. It includes two parts as below.

The first part (patches 1-7) reverts the series of recent commits that
removed the multi-card support with comments saying there was no such
use case in the real world. Actually this feature is being used in
Mellanox Bluefield SoC and has been requested by customers.

The second part (patches 8-9) fixes the DesignWare multi-card support
according to the dw-mmc databook (synnopsys: DesignWare Cores Mobile
Storage Host Databook, 2.70a). It has changes to set the card number
into the CMD register to multiplex requests to different cards when
working in SD_MMC_CEATA mode, set the CTYPE / CLKENA / CDTHRCTL
registers properly according to the spec, and parse the per-card
configuration to match the Linux Documentation
(bindings/mmc/synopsys-dw-mshc.txt).

Liming Sun (9):
  Revert "Documentation: dw-mshc: deprecate num-slots"
  Revert "mmc: dw_mmc: remove the unnecessary slot variable"
  Revert "mmc: dw_mmc: use the 'slot' instead of 'cur_slot'"
  Revert "mmc: dw_mmc: remove the 'id' arguments about functions
    relevant to slot"
  Revert "mmc: dw_mmc: change the array of slots"
  Revert "mmc: dw_mmc: remove the loop about finding slots"
  Revert "mmc: dw_mmc: deprecated the "num-slots" property"
  mmc: dw_mmc: Support two SD_MMC_CE-ATA cards
  mmc: dw_mmc: Parse slot-specific configuration

 .../devicetree/bindings/mmc/synopsys-dw-mshc.txt   |  16 +-
 drivers/mmc/host/dw_mmc-exynos.c                   |   4 +-
 drivers/mmc/host/dw_mmc.c                          | 277 ++++++++++++++++-----
 drivers/mmc/host/dw_mmc.h                          |  17 +-
 4 files changed, 236 insertions(+), 78 deletions(-)

-- 
1.8.3.1

[PATCH 0/9] Enable dw-mmc multi-card support

[Liming Sun]

This series of commits enables the multi-card support for the dw-mmc
controller. It includes two parts as below.

The first part (patches 1-7) reverts the series of recent commits that
removed the multi-card support with comments saying there was no such
use case in the real world. Actually this feature is being used in
Mellanox Bluefield SoC and has been requested by customers.

The second part (patches 8-9) fixes the DesignWare multi-card support
according to the dw-mmc databook (synnopsys: DesignWare Cores Mobile
Storage Host Databook, 2.70a). It has changes to set the card number
into the CMD register to multiplex requests to different cards when
working in SD_MMC_CEATA mode, set the CTYPE / CLKENA / CDTHRCTL
registers properly according to the spec, and parse the per-card
configuration to match the Linux Documentation
(bindings/mmc/synopsys-dw-mshc.txt).

Liming Sun (9):
  Revert "Documentation: dw-mshc: deprecate num-slots"
  Revert "mmc: dw_mmc: remove the unnecessary slot variable"
  Revert "mmc: dw_mmc: use the 'slot' instead of 'cur_slot'"
  Revert "mmc: dw_mmc: remove the 'id' arguments about functions
    relevant to slot"
  Revert "mmc: dw_mmc: change the array of slots"
  Revert "mmc: dw_mmc: remove the loop about finding slots"
  Revert "mmc: dw_mmc: deprecated the "num-slots" property"
  mmc: dw_mmc: Support two SD_MMC_CE-ATA cards
  mmc: dw_mmc: Parse slot-specific configuration

 .../devicetree/bindings/mmc/synopsys-dw-mshc.txt   |  16 +-
 drivers/mmc/host/dw_mmc-exynos.c                   |   4 +-
 drivers/mmc/host/dw_mmc.c                          | 277 ++++++++++++++++-----
 drivers/mmc/host/dw_mmc.h                          |  17 +-
 4 files changed, 236 insertions(+), 78 deletions(-)

-- 
1.8.3.1

[PATCH 1/9] Revert "Documentation: dw-mshc: deprecate num-slots"

[Liming Sun]

This reverts commit 3f5b4b79d4c0fa71fd7d74c2a44bbc0869c04c9b.

The Mellanox BlueField SoC requires multiple slot dw-mmc support.

Signed-off-by: Liming Sun <lsun@mellanox.com>
---
 .../devicetree/bindings/mmc/synopsys-dw-mshc.txt         | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
index ef3e5f1..9cb55ca 100644
--- a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
+++ b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
@@ -12,12 +12,12 @@ Required Properties:
 * #address-cells: should be 1.
 * #size-cells: should be 0.
 
-# Slots (DEPRECATED): The slot specific information are contained within
-  child-nodes with each child-node representing a supported slot. There should
-  be atleast one child node representing a card slot. The name of the child node
-  representing the slot is recommended to be slot@n where n is the unique number
-  of the slot connected to the controller. The following are optional properties
-  which can be included in the slot child node.
+# Slots: The slot specific information are contained within child-nodes with
+  each child-node representing a supported slot. There should be atleast one
+  child node representing a card slot. The name of the child node representing
+  the slot is recommended to be slot@n where n is the unique number of the slot
+  connected to the controller. The following are optional properties which
+  can be included in the slot child node.
 
 	* reg: specifies the physical slot number. The valid values of this
 	  property is 0 to (num-slots -1), where num-slots is the value
@@ -63,7 +63,7 @@ Optional properties:
   clock(cclk_out). If it's not specified, max is 200MHZ and min is 400KHz by default.
 	  (Use the "max-frequency" instead of "clock-freq-min-max".)
 
-* num-slots (DEPRECATED): specifies the number of slots supported by the controller.
+* num-slots: specifies the number of slots supported by the controller.
   The number of physical slots actually used could be equal or less than the
   value specified by num-slots. If this property is not specified, the value
   of num-slot property is assumed to be 1.
@@ -124,6 +124,7 @@ board specific portions as listed below.
 	dwmmc0@12200000 {
 		clock-frequency = <400000000>;
 		clock-freq-min-max = <400000 200000000>;
+		num-slots = <1>;
 		broken-cd;
 		fifo-depth = <0x80>;
 		card-detect-delay = <200>;
@@ -138,6 +139,7 @@ board specific portions as listed below.
 	dwmmc0@12200000 {
 		clock-frequency = <400000000>;
 		clock-freq-min-max = <400000 200000000>;
+		num-slots = <1>;
 		broken-cd;
 		fifo-depth = <0x80>;
 		card-detect-delay = <200>;
-- 
1.8.3.1

Re: [PATCH 1/9] Revert "Documentation: dw-mshc: deprecate num-slots"

[Rob Herring]

On Fri, Oct 06, 2017 at 03:21:24PM -0400, Liming Sun wrote:
> This reverts commit 3f5b4b79d4c0fa71fd7d74c2a44bbc0869c04c9b.
> 
> The Mellanox BlueField SoC requires multiple slot dw-mmc support.
> 
> Signed-off-by: Liming Sun <lsun@mellanox.com>
> ---
>  .../devicetree/bindings/mmc/synopsys-dw-mshc.txt         | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)

Acked-by: Rob Herring <robh@kernel.org>

[PATCH 2/9] Revert "mmc: dw_mmc: remove the unnecessary slot variable"

[Liming Sun]

This reverts commit e47c0b96678c5fd731c125dca677880e06d6394c.

The Mellanox BlueField SoC requires multiple slot dw-mmc support.

Signed-off-by: Liming Sun <lsun@mellanox.com>
---
 drivers/mmc/host/dw_mmc.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 860313b..c9f81db 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -3040,15 +3040,17 @@ static void dw_mci_enable_cd(struct dw_mci *host)
 {
 	unsigned long irqflags;
 	u32 temp;
+	struct dw_mci_slot *slot;
 
 	/*
 	 * No need for CD if all slots have a non-error GPIO
 	 * as well as broken card detection is found.
 	 */
-	if (host->slot->mmc->caps & MMC_CAP_NEEDS_POLL)
+	slot = host->slot;
+	if (slot->mmc->caps & MMC_CAP_NEEDS_POLL)
 		return;
 
-	if (mmc_gpio_get_cd(host->slot->mmc) < 0) {
+	if (mmc_gpio_get_cd(slot->mmc) < 0) {
 		spin_lock_irqsave(&host->irq_lock, irqflags);
 		temp = mci_readl(host, INTMASK);
 		temp  |= SDMMC_INT_CD;
@@ -3319,6 +3321,7 @@ int dw_mci_runtime_resume(struct device *dev)
 {
 	int ret = 0;
 	struct dw_mci *host = dev_get_drvdata(dev);
+	struct dw_mci_slot *slot = host->slot;
 
 	if (host->slot &&
 	    (mmc_can_gpio_cd(host->slot->mmc) ||
@@ -3358,11 +3361,11 @@ int dw_mci_runtime_resume(struct device *dev)
 	mci_writel(host, CTRL, SDMMC_CTRL_INT_ENABLE);
 
 
-	if (host->slot->mmc->pm_flags & MMC_PM_KEEP_POWER)
-		dw_mci_set_ios(host->slot->mmc, &host->slot->mmc->ios);
+	if (slot->mmc->pm_flags & MMC_PM_KEEP_POWER)
+		dw_mci_set_ios(slot->mmc, &slot->mmc->ios);
 
 	/* Force setup bus to guarantee available clock output */
-	dw_mci_setup_bus(host->slot, true);
+	dw_mci_setup_bus(slot, true);
 
 	/* Now that slots are all setup, we can enable card detect */
 	dw_mci_enable_cd(host);
-- 
1.8.3.1

[PATCH 3/9] Revert "mmc: dw_mmc: use the 'slot' instead of 'cur_slot'"

[Liming Sun]

This reverts commit 42f989c002f235557e3a03feac3b2f16b17d53f6.

The Mellanox BlueField SoC requires multiple slot dw-mmc support.

Signed-off-by: Liming Sun <lsun@mellanox.com>
---
 drivers/mmc/host/dw_mmc-exynos.c |  4 ++--
 drivers/mmc/host/dw_mmc.c        | 33 +++++++++++++++++----------------
 drivers/mmc/host/dw_mmc.h        |  3 +++
 3 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
index 3502679..25691cc 100644
--- a/drivers/mmc/host/dw_mmc-exynos.c
+++ b/drivers/mmc/host/dw_mmc-exynos.c
@@ -157,8 +157,8 @@ static void dw_mci_exynos_set_clksel_timing(struct dw_mci *host, u32 timing)
 	 * HOLD register should be bypassed in case there is no phase shift
 	 * applied on CMD/DATA that is sent to the card.
 	 */
-	if (!SDMMC_CLKSEL_GET_DRV_WD3(clksel) && host->slot)
-		set_bit(DW_MMC_CARD_NO_USE_HOLD, &host->slot->flags);
+	if (!SDMMC_CLKSEL_GET_DRV_WD3(clksel) && host->cur_slot)
+		set_bit(DW_MMC_CARD_NO_USE_HOLD, &host->cur_slot->flags);
 }
 
 #ifdef CONFIG_PM
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index c9f81db..62c0791 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -392,7 +392,7 @@ static u32 dw_mci_prep_stop_abort(struct dw_mci *host, struct mmc_command *cmd)
 	cmdr = stop->opcode | SDMMC_CMD_STOP |
 		SDMMC_CMD_RESP_CRC | SDMMC_CMD_RESP_EXP;
 
-	if (!test_bit(DW_MMC_CARD_NO_USE_HOLD, &host->slot->flags))
+	if (!test_bit(DW_MMC_CARD_NO_USE_HOLD, &host->cur_slot->flags))
 		cmdr |= SDMMC_CMD_USE_HOLD_REG;
 
 	return cmdr;
@@ -499,7 +499,7 @@ static void dw_mci_dmac_complete_dma(void *arg)
 	if ((host->use_dma == TRANS_MODE_EDMAC) &&
 	    data && (data->flags & MMC_DATA_READ))
 		/* Invalidate cache after read */
-		dma_sync_sg_for_cpu(mmc_dev(host->slot->mmc),
+		dma_sync_sg_for_cpu(mmc_dev(host->cur_slot->mmc),
 				    data->sg,
 				    data->sg_len,
 				    DMA_FROM_DEVICE);
@@ -839,7 +839,7 @@ static int dw_mci_edmac_start_dma(struct dw_mci *host,
 
 	/* Flush cache before write */
 	if (host->data->flags & MMC_DATA_WRITE)
-		dma_sync_sg_for_device(mmc_dev(host->slot->mmc), sgl,
+		dma_sync_sg_for_device(mmc_dev(host->cur_slot->mmc), sgl,
 				       sg_elems, DMA_TO_DEVICE);
 
 	dma_async_issue_pending(host->dms->ch);
@@ -1301,6 +1301,7 @@ static void __dw_mci_start_request(struct dw_mci *host,
 
 	mrq = slot->mrq;
 
+	host->cur_slot = slot;
 	host->mrq = mrq;
 
 	host->pending_events = 0;
@@ -1781,7 +1782,7 @@ static bool dw_mci_reset(struct dw_mci *host)
 
 ciu_out:
 	/* After a CTRL reset we need to have CIU set clock registers  */
-	mci_send_cmd(host->slot, SDMMC_CMD_UPD_CLK, 0);
+	mci_send_cmd(host->cur_slot, SDMMC_CMD_UPD_CLK, 0);
 
 	return ret;
 }
@@ -1808,11 +1809,11 @@ static void dw_mci_request_end(struct dw_mci *host, struct mmc_request *mrq)
 	__acquires(&host->lock)
 {
 	struct dw_mci_slot *slot;
-	struct mmc_host	*prev_mmc = host->slot->mmc;
+	struct mmc_host	*prev_mmc = host->cur_slot->mmc;
 
 	WARN_ON(host->cmd || host->data);
 
-	host->slot->mrq = NULL;
+	host->cur_slot->mrq = NULL;
 	host->mrq = NULL;
 	if (!list_empty(&host->queue)) {
 		slot = list_entry(host->queue.next,
@@ -1962,7 +1963,7 @@ static void dw_mci_tasklet_func(unsigned long priv)
 			err = dw_mci_command_complete(host, cmd);
 			if (cmd == mrq->sbc && !err) {
 				prev_state = state = STATE_SENDING_CMD;
-				__dw_mci_start_request(host, host->slot,
+				__dw_mci_start_request(host, host->cur_slot,
 						       mrq->cmd);
 				goto unlock;
 			}
@@ -3308,9 +3309,9 @@ int dw_mci_runtime_suspend(struct device *dev)
 
 	clk_disable_unprepare(host->ciu_clk);
 
-	if (host->slot &&
-	    (mmc_can_gpio_cd(host->slot->mmc) ||
-	     !mmc_card_is_removable(host->slot->mmc)))
+	if (host->cur_slot &&
+	    (mmc_can_gpio_cd(host->cur_slot->mmc) ||
+	     !mmc_card_is_removable(host->cur_slot->mmc)))
 		clk_disable_unprepare(host->biu_clk);
 
 	return 0;
@@ -3323,9 +3324,9 @@ int dw_mci_runtime_resume(struct device *dev)
 	struct dw_mci *host = dev_get_drvdata(dev);
 	struct dw_mci_slot *slot = host->slot;
 
-	if (host->slot &&
-	    (mmc_can_gpio_cd(host->slot->mmc) ||
-	     !mmc_card_is_removable(host->slot->mmc))) {
+	if (host->cur_slot &&
+	    (mmc_can_gpio_cd(host->cur_slot->mmc) ||
+	     !mmc_card_is_removable(host->cur_slot->mmc))) {
 		ret = clk_prepare_enable(host->biu_clk);
 		if (ret)
 			return ret;
@@ -3373,9 +3374,9 @@ int dw_mci_runtime_resume(struct device *dev)
 	return 0;
 
 err:
-	if (host->slot &&
-	    (mmc_can_gpio_cd(host->slot->mmc) ||
-	     !mmc_card_is_removable(host->slot->mmc)))
+	if (host->cur_slot &&
+	    (mmc_can_gpio_cd(host->cur_slot->mmc) ||
+	     !mmc_card_is_removable(host->cur_slot->mmc)))
 		clk_disable_unprepare(host->biu_clk);
 
 	return ret;
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index 34474ad..20a956e 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -133,6 +133,7 @@ struct dw_mci_dma_slave {
  * =======
  *
  * @lock is a softirq-safe spinlock protecting @queue as well as
+ * @cur_slot, @mrq and @state. These must always be updated
  * at the same time while holding @lock.
  *
  * @irq_lock is an irq-safe spinlock protecting the INTMASK register
@@ -168,6 +169,7 @@ struct dw_mci {
 	struct scatterlist	*sg;
 	struct sg_mapping_iter	sg_miter;
 
+	struct dw_mci_slot	*cur_slot;
 	struct mmc_request	*mrq;
 	struct mmc_command	*cmd;
 	struct mmc_data		*data;
@@ -203,6 +205,7 @@ struct dw_mci {
 
 	u32			bus_hz;
 	u32			current_speed;
+	u32			num_slots;
 	u32			fifoth_val;
 	u16			verid;
 	struct device		*dev;
-- 
1.8.3.1

[PATCH 3/9] Revert "mmc: dw_mmc: use the 'slot' instead of 'cur_slot'"

[Liming Sun]

This reverts commit 42f989c002f235557e3a03feac3b2f16b17d53f6.

The Mellanox BlueField SoC requires multiple slot dw-mmc support.

Signed-off-by: Liming Sun <lsun@mellanox.com>
---
 drivers/mmc/host/dw_mmc-exynos.c |  4 ++--
 drivers/mmc/host/dw_mmc.c        | 33 +++++++++++++++++----------------
 drivers/mmc/host/dw_mmc.h        |  3 +++
 3 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
index 3502679..25691cc 100644
--- a/drivers/mmc/host/dw_mmc-exynos.c
+++ b/drivers/mmc/host/dw_mmc-exynos.c
@@ -157,8 +157,8 @@ static void dw_mci_exynos_set_clksel_timing(struct dw_mci *host, u32 timing)
 	 * HOLD register should be bypassed in case there is no phase shift
 	 * applied on CMD/DATA that is sent to the card.
 	 */
-	if (!SDMMC_CLKSEL_GET_DRV_WD3(clksel) && host->slot)
-		set_bit(DW_MMC_CARD_NO_USE_HOLD, &host->slot->flags);
+	if (!SDMMC_CLKSEL_GET_DRV_WD3(clksel) && host->cur_slot)
+		set_bit(DW_MMC_CARD_NO_USE_HOLD, &host->cur_slot->flags);
 }
 
 #ifdef CONFIG_PM
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index c9f81db..62c0791 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -392,7 +392,7 @@ static u32 dw_mci_prep_stop_abort(struct dw_mci *host, struct mmc_command *cmd)
 	cmdr = stop->opcode | SDMMC_CMD_STOP |
 		SDMMC_CMD_RESP_CRC | SDMMC_CMD_RESP_EXP;
 
-	if (!test_bit(DW_MMC_CARD_NO_USE_HOLD, &host->slot->flags))
+	if (!test_bit(DW_MMC_CARD_NO_USE_HOLD, &host->cur_slot->flags))
 		cmdr |= SDMMC_CMD_USE_HOLD_REG;
 
 	return cmdr;
@@ -499,7 +499,7 @@ static void dw_mci_dmac_complete_dma(void *arg)
 	if ((host->use_dma == TRANS_MODE_EDMAC) &&
 	    data && (data->flags & MMC_DATA_READ))
 		/* Invalidate cache after read */
-		dma_sync_sg_for_cpu(mmc_dev(host->slot->mmc),
+		dma_sync_sg_for_cpu(mmc_dev(host->cur_slot->mmc),
 				    data->sg,
 				    data->sg_len,
 				    DMA_FROM_DEVICE);
@@ -839,7 +839,7 @@ static int dw_mci_edmac_start_dma(struct dw_mci *host,
 
 	/* Flush cache before write */
 	if (host->data->flags & MMC_DATA_WRITE)
-		dma_sync_sg_for_device(mmc_dev(host->slot->mmc), sgl,
+		dma_sync_sg_for_device(mmc_dev(host->cur_slot->mmc), sgl,
 				       sg_elems, DMA_TO_DEVICE);
 
 	dma_async_issue_pending(host->dms->ch);
@@ -1301,6 +1301,7 @@ static void __dw_mci_start_request(struct dw_mci *host,
 
 	mrq = slot->mrq;
 
+	host->cur_slot = slot;
 	host->mrq = mrq;
 
 	host->pending_events = 0;
@@ -1781,7 +1782,7 @@ static bool dw_mci_reset(struct dw_mci *host)
 
 ciu_out:
 	/* After a CTRL reset we need to have CIU set clock registers  */
-	mci_send_cmd(host->slot, SDMMC_CMD_UPD_CLK, 0);
+	mci_send_cmd(host->cur_slot, SDMMC_CMD_UPD_CLK, 0);
 
 	return ret;
 }
@@ -1808,11 +1809,11 @@ static void dw_mci_request_end(struct dw_mci *host, struct mmc_request *mrq)
 	__acquires(&host->lock)
 {
 	struct dw_mci_slot *slot;
-	struct mmc_host	*prev_mmc = host->slot->mmc;
+	struct mmc_host	*prev_mmc = host->cur_slot->mmc;
 
 	WARN_ON(host->cmd || host->data);
 
-	host->slot->mrq = NULL;
+	host->cur_slot->mrq = NULL;
 	host->mrq = NULL;
 	if (!list_empty(&host->queue)) {
 		slot = list_entry(host->queue.next,
@@ -1962,7 +1963,7 @@ static void dw_mci_tasklet_func(unsigned long priv)
 			err = dw_mci_command_complete(host, cmd);
 			if (cmd == mrq->sbc && !err) {
 				prev_state = state = STATE_SENDING_CMD;
-				__dw_mci_start_request(host, host->slot,
+				__dw_mci_start_request(host, host->cur_slot,
 						       mrq->cmd);
 				goto unlock;
 			}
@@ -3308,9 +3309,9 @@ int dw_mci_runtime_suspend(struct device *dev)
 
 	clk_disable_unprepare(host->ciu_clk);
 
-	if (host->slot &&
-	    (mmc_can_gpio_cd(host->slot->mmc) ||
-	     !mmc_card_is_removable(host->slot->mmc)))
+	if (host->cur_slot &&
+	    (mmc_can_gpio_cd(host->cur_slot->mmc) ||
+	     !mmc_card_is_removable(host->cur_slot->mmc)))
 		clk_disable_unprepare(host->biu_clk);
 
 	return 0;
@@ -3323,9 +3324,9 @@ int dw_mci_runtime_resume(struct device *dev)
 	struct dw_mci *host = dev_get_drvdata(dev);
 	struct dw_mci_slot *slot = host->slot;
 
-	if (host->slot &&
-	    (mmc_can_gpio_cd(host->slot->mmc) ||
-	     !mmc_card_is_removable(host->slot->mmc))) {
+	if (host->cur_slot &&
+	    (mmc_can_gpio_cd(host->cur_slot->mmc) ||
+	     !mmc_card_is_removable(host->cur_slot->mmc))) {
 		ret = clk_prepare_enable(host->biu_clk);
 		if (ret)
 			return ret;
@@ -3373,9 +3374,9 @@ int dw_mci_runtime_resume(struct device *dev)
 	return 0;
 
 err:
-	if (host->slot &&
-	    (mmc_can_gpio_cd(host->slot->mmc) ||
-	     !mmc_card_is_removable(host->slot->mmc)))
+	if (host->cur_slot &&
+	    (mmc_can_gpio_cd(host->cur_slot->mmc) ||
+	     !mmc_card_is_removable(host->cur_slot->mmc)))
 		clk_disable_unprepare(host->biu_clk);
 
 	return ret;
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index 34474ad..20a956e 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -133,6 +133,7 @@ struct dw_mci_dma_slave {
  * =======
  *
  * @lock is a softirq-safe spinlock protecting @queue as well as
+ * @cur_slot, @mrq and @state. These must always be updated
  * at the same time while holding @lock.
  *
  * @irq_lock is an irq-safe spinlock protecting the INTMASK register
@@ -168,6 +169,7 @@ struct dw_mci {
 	struct scatterlist	*sg;
 	struct sg_mapping_iter	sg_miter;
 
+	struct dw_mci_slot	*cur_slot;
 	struct mmc_request	*mrq;
 	struct mmc_command	*cmd;
 	struct mmc_data		*data;
@@ -203,6 +205,7 @@ struct dw_mci {
 
 	u32			bus_hz;
 	u32			current_speed;
+	u32			num_slots;
 	u32			fifoth_val;
 	u16			verid;
 	struct device		*dev;
-- 
1.8.3.1

[PATCH 4/9] Revert "mmc: dw_mmc: remove the 'id' arguments about functions relevant to slot"

[Liming Sun]

This reverts commit e4a65ef7687b6aaf36bedb497d3fd1480163d2d5.

The Mellanox BlueField SoC requires multiple slot dw-mmc support.

Signed-off-by: Liming Sun <lsun@mellanox.com>
---
 drivers/mmc/host/dw_mmc.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 62c0791..a4356d6 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2709,7 +2709,7 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static int dw_mci_init_slot(struct dw_mci *host)
+static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
 {
 	struct mmc_host *mmc;
 	struct dw_mci_slot *slot;
@@ -2722,8 +2722,8 @@ static int dw_mci_init_slot(struct dw_mci *host)
 		return -ENOMEM;
 
 	slot = mmc_priv(mmc);
-	slot->id = 0;
-	slot->sdio_id = host->sdio_id0 + slot->id;
+	slot->id = id;
+	slot->sdio_id = host->sdio_id0 + id;
 	slot->mmc = mmc;
 	slot->host = host;
 	host->slot = slot;
@@ -2822,7 +2822,7 @@ static int dw_mci_init_slot(struct dw_mci *host)
 	return ret;
 }
 
-static void dw_mci_cleanup_slot(struct dw_mci_slot *slot)
+static void dw_mci_cleanup_slot(struct dw_mci_slot *slot, unsigned int id)
 {
 	/* Debugfs stuff is cleaned up by mmc core */
 	mmc_remove_host(slot->mmc);
@@ -3245,7 +3245,7 @@ int dw_mci_probe(struct dw_mci *host)
 		 host->irq, width, fifo_size);
 
 	/* We need at least one slot to succeed */
-	ret = dw_mci_init_slot(host);
+	ret = dw_mci_init_slot(host, 0);
 	if (ret) {
 		dev_dbg(host->dev, "slot %d init failed\n", i);
 		goto err_dmaunmap;
@@ -3275,9 +3275,11 @@ int dw_mci_probe(struct dw_mci *host)
 
 void dw_mci_remove(struct dw_mci *host)
 {
-	dev_dbg(host->dev, "remove slot\n");
+	int i = 0;
+
+	dev_dbg(host->dev, "remove slot %d\n", i);
 	if (host->slot)
-		dw_mci_cleanup_slot(host->slot);
+		dw_mci_cleanup_slot(host->slot, i);
 
 	mci_writel(host, RINTSTS, 0xFFFFFFFF);
 	mci_writel(host, INTMASK, 0); /* disable all mmc interrupt first */
-- 
1.8.3.1

[PATCH 5/9] Revert "mmc: dw_mmc: change the array of slots"

[Liming Sun]

This reverts commit b23475faed77f6a9016013c8db6b4707466e74a8.

The Mellanox BlueField SoC requires multiple slot dw-mmc support.

Signed-off-by: Liming Sun <lsun@mellanox.com>
---
 drivers/mmc/host/dw_mmc.c | 21 ++++++++++++---------
 drivers/mmc/host/dw_mmc.h |  4 +++-
 2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index a4356d6..d11b8d7 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2582,7 +2582,8 @@ static void dw_mci_cmd_interrupt(struct dw_mci *host, u32 status)
 
 static void dw_mci_handle_cd(struct dw_mci *host)
 {
-	struct dw_mci_slot *slot = host->slot;
+	int i = 0;
+	struct dw_mci_slot *slot = host->slot[i];
 
 	if (slot->mmc->ops->card_event)
 		slot->mmc->ops->card_event(slot->mmc);
@@ -2594,7 +2595,8 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
 {
 	struct dw_mci *host = dev_id;
 	u32 pending;
-	struct dw_mci_slot *slot = host->slot;
+	int i = 0;
+	struct dw_mci_slot *slot = host->slot[i];
 
 	pending = mci_readl(host, MINTSTS); /* read-only mask reg */
 
@@ -2726,7 +2728,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
 	slot->sdio_id = host->sdio_id0 + id;
 	slot->mmc = mmc;
 	slot->host = host;
-	host->slot = slot;
+	host->slot[id] = slot;
 
 	mmc->ops = &dw_mci_ops;
 	if (device_property_read_u32_array(host->dev, "clock-freq-min-max",
@@ -2826,7 +2828,7 @@ static void dw_mci_cleanup_slot(struct dw_mci_slot *slot, unsigned int id)
 {
 	/* Debugfs stuff is cleaned up by mmc core */
 	mmc_remove_host(slot->mmc);
-	slot->host->slot = NULL;
+	slot->host->slot[id] = NULL;
 	mmc_free_host(slot->mmc);
 }
 
@@ -3041,13 +3043,14 @@ static void dw_mci_enable_cd(struct dw_mci *host)
 {
 	unsigned long irqflags;
 	u32 temp;
+	int i = 0;
 	struct dw_mci_slot *slot;
 
 	/*
 	 * No need for CD if all slots have a non-error GPIO
 	 * as well as broken card detection is found.
 	 */
-	slot = host->slot;
+	slot = host->slot[i];
 	if (slot->mmc->caps & MMC_CAP_NEEDS_POLL)
 		return;
 
@@ -3278,8 +3281,8 @@ void dw_mci_remove(struct dw_mci *host)
 	int i = 0;
 
 	dev_dbg(host->dev, "remove slot %d\n", i);
-	if (host->slot)
-		dw_mci_cleanup_slot(host->slot, i);
+	if (host->slot[i])
+		dw_mci_cleanup_slot(host->slot[i], i);
 
 	mci_writel(host, RINTSTS, 0xFFFFFFFF);
 	mci_writel(host, INTMASK, 0); /* disable all mmc interrupt first */
@@ -3322,9 +3325,9 @@ int dw_mci_runtime_suspend(struct device *dev)
 
 int dw_mci_runtime_resume(struct device *dev)
 {
-	int ret = 0;
+	int i = 0, ret = 0;
 	struct dw_mci *host = dev_get_drvdata(dev);
-	struct dw_mci_slot *slot = host->slot;
+	struct dw_mci_slot *slot = host->slot[i];
 
 	if (host->cur_slot &&
 	    (mmc_can_gpio_cd(host->cur_slot->mmc) ||
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index 20a956e..e62cafc 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -20,6 +20,8 @@
 #include <linux/reset.h>
 #include <linux/interrupt.h>
 
+#define MAX_MCI_SLOTS	2
+
 enum dw_mci_state {
 	STATE_IDLE = 0,
 	STATE_SENDING_CMD,
@@ -214,7 +216,7 @@ struct dw_mci {
 	void			*priv;
 	struct clk		*biu_clk;
 	struct clk		*ciu_clk;
-	struct dw_mci_slot	*slot;
+	struct dw_mci_slot	*slot[MAX_MCI_SLOTS];
 
 	/* FIFO push and pull */
 	int			fifo_depth;
-- 
1.8.3.1

[PATCH 6/9] Revert "mmc: dw_mmc: remove the loop about finding slots"

[Liming Sun]

This reverts commit 58870241a67453be7dc9ab368d5a0cdc9c404616.

The Mellanox BlueField SoC requires multiple slot dw-mmc support.

Signed-off-by: Liming Sun <lsun@mellanox.com>
---
 drivers/mmc/host/dw_mmc.c | 115 +++++++++++++++++++++++++++++++---------------
 1 file changed, 79 insertions(+), 36 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index d11b8d7..ef36e05 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2582,21 +2582,26 @@ static void dw_mci_cmd_interrupt(struct dw_mci *host, u32 status)
 
 static void dw_mci_handle_cd(struct dw_mci *host)
 {
-	int i = 0;
-	struct dw_mci_slot *slot = host->slot[i];
+	int i;
+
+	for (i = 0; i < host->num_slots; i++) {
+		struct dw_mci_slot *slot = host->slot[i];
+
+		if (!slot)
+			continue;
 
-	if (slot->mmc->ops->card_event)
-		slot->mmc->ops->card_event(slot->mmc);
-	mmc_detect_change(slot->mmc,
-		msecs_to_jiffies(host->pdata->detect_delay_ms));
+		if (slot->mmc->ops->card_event)
+			slot->mmc->ops->card_event(slot->mmc);
+		mmc_detect_change(slot->mmc,
+			msecs_to_jiffies(host->pdata->detect_delay_ms));
+	}
 }
 
 static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
 {
 	struct dw_mci *host = dev_id;
 	u32 pending;
-	int i = 0;
-	struct dw_mci_slot *slot = host->slot[i];
+	int i;
 
 	pending = mci_readl(host, MINTSTS); /* read-only mask reg */
 
@@ -2675,11 +2680,19 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
 			dw_mci_handle_cd(host);
 		}
 
-		if (pending & SDMMC_INT_SDIO(slot->sdio_id)) {
-			mci_writel(host, RINTSTS,
-				   SDMMC_INT_SDIO(slot->sdio_id));
-			__dw_mci_enable_sdio_irq(slot, 0);
-			sdio_signal_irq(slot->mmc);
+		/* Handle SDIO Interrupts */
+		for (i = 0; i < host->num_slots; i++) {
+			struct dw_mci_slot *slot = host->slot[i];
+
+			if (!slot)
+				continue;
+
+			if (pending & SDMMC_INT_SDIO(slot->sdio_id)) {
+				mci_writel(host, RINTSTS,
+					   SDMMC_INT_SDIO(slot->sdio_id));
+				__dw_mci_enable_sdio_irq(slot, 0);
+				sdio_signal_irq(slot->mmc);
+			}
 		}
 
 	}
@@ -3043,24 +3056,29 @@ static void dw_mci_enable_cd(struct dw_mci *host)
 {
 	unsigned long irqflags;
 	u32 temp;
-	int i = 0;
+	int i;
 	struct dw_mci_slot *slot;
 
 	/*
 	 * No need for CD if all slots have a non-error GPIO
 	 * as well as broken card detection is found.
 	 */
-	slot = host->slot[i];
-	if (slot->mmc->caps & MMC_CAP_NEEDS_POLL)
-		return;
+	for (i = 0; i < host->num_slots; i++) {
+		slot = host->slot[i];
+		if (slot->mmc->caps & MMC_CAP_NEEDS_POLL)
+			return;
 
-	if (mmc_gpio_get_cd(slot->mmc) < 0) {
-		spin_lock_irqsave(&host->irq_lock, irqflags);
-		temp = mci_readl(host, INTMASK);
-		temp  |= SDMMC_INT_CD;
-		mci_writel(host, INTMASK, temp);
-		spin_unlock_irqrestore(&host->irq_lock, irqflags);
+		if (mmc_gpio_get_cd(slot->mmc) < 0)
+			break;
 	}
+	if (i == host->num_slots)
+		return;
+
+	spin_lock_irqsave(&host->irq_lock, irqflags);
+	temp = mci_readl(host, INTMASK);
+	temp  |= SDMMC_INT_CD;
+	mci_writel(host, INTMASK, temp);
+	spin_unlock_irqrestore(&host->irq_lock, irqflags);
 }
 
 int dw_mci_probe(struct dw_mci *host)
@@ -3068,6 +3086,7 @@ int dw_mci_probe(struct dw_mci *host)
 	const struct dw_mci_drv_data *drv_data = host->drv_data;
 	int width, i, ret = 0;
 	u32 fifo_size;
+	int init_slots = 0;
 
 	if (!host->pdata) {
 		host->pdata = dw_mci_parse_dt(host);
@@ -3234,6 +3253,13 @@ int dw_mci_probe(struct dw_mci *host)
 		goto err_dmaunmap;
 
 	/*
+	 * Even though dwmmc IP is provided the multiple slots,
+	 * there is no use case in mmc subsystem.
+	 * dwmmc host controller needs to initialize the one slot per an IP.
+	 */
+	host->num_slots = 1;
+
+	/*
 	 * Enable interrupts for command done, data over, data empty,
 	 * receive ready and error such as transmit, receive timeout, crc error
 	 */
@@ -3248,9 +3274,20 @@ int dw_mci_probe(struct dw_mci *host)
 		 host->irq, width, fifo_size);
 
 	/* We need at least one slot to succeed */
-	ret = dw_mci_init_slot(host, 0);
-	if (ret) {
-		dev_dbg(host->dev, "slot %d init failed\n", i);
+	for (i = 0; i < host->num_slots; i++) {
+		ret = dw_mci_init_slot(host, i);
+		if (ret)
+			dev_dbg(host->dev, "slot %d init failed\n", i);
+		else
+			init_slots++;
+	}
+
+	if (init_slots) {
+		dev_info(host->dev, "%d slots initialized\n", init_slots);
+	} else {
+		dev_dbg(host->dev,
+			"attempted to initialize %d slots, but failed on all\n",
+			host->num_slots);
 		goto err_dmaunmap;
 	}
 
@@ -3278,11 +3315,13 @@ int dw_mci_probe(struct dw_mci *host)
 
 void dw_mci_remove(struct dw_mci *host)
 {
-	int i = 0;
+	int i;
 
-	dev_dbg(host->dev, "remove slot %d\n", i);
-	if (host->slot[i])
-		dw_mci_cleanup_slot(host->slot[i], i);
+	for (i = 0; i < host->num_slots; i++) {
+		dev_dbg(host->dev, "remove slot %d\n", i);
+		if (host->slot[i])
+			dw_mci_cleanup_slot(host->slot[i], i);
+	}
 
 	mci_writel(host, RINTSTS, 0xFFFFFFFF);
 	mci_writel(host, INTMASK, 0); /* disable all mmc interrupt first */
@@ -3325,9 +3364,8 @@ int dw_mci_runtime_suspend(struct device *dev)
 
 int dw_mci_runtime_resume(struct device *dev)
 {
-	int i = 0, ret = 0;
+	int i, ret = 0;
 	struct dw_mci *host = dev_get_drvdata(dev);
-	struct dw_mci_slot *slot = host->slot[i];
 
 	if (host->cur_slot &&
 	    (mmc_can_gpio_cd(host->cur_slot->mmc) ||
@@ -3366,12 +3404,17 @@ int dw_mci_runtime_resume(struct device *dev)
 		   DW_MCI_ERROR_FLAGS);
 	mci_writel(host, CTRL, SDMMC_CTRL_INT_ENABLE);
 
+	for (i = 0; i < host->num_slots; i++) {
+		struct dw_mci_slot *slot = host->slot[i];
 
-	if (slot->mmc->pm_flags & MMC_PM_KEEP_POWER)
-		dw_mci_set_ios(slot->mmc, &slot->mmc->ios);
+		if (!slot)
+			continue;
+		if (slot->mmc->pm_flags & MMC_PM_KEEP_POWER)
+			dw_mci_set_ios(slot->mmc, &slot->mmc->ios);
 
-	/* Force setup bus to guarantee available clock output */
-	dw_mci_setup_bus(slot, true);
+		/* Force setup bus to guarantee available clock output */
+		dw_mci_setup_bus(slot, true);
+	}
 
 	/* Now that slots are all setup, we can enable card detect */
 	dw_mci_enable_cd(host);
-- 
1.8.3.1

[PATCH 7/9] Revert "mmc: dw_mmc: deprecated the "num-slots" property"

[Liming Sun]

This reverts commit d30a8f7bdf6498e47bd3a6f31e5028f239deb208.

The Mellanox BlueField SoC requires multiple slot dw-mmc support.

Signed-off-by: Liming Sun <lsun@mellanox.com>
---
 drivers/mmc/host/dw_mmc.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index ef36e05..150cd05 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -3018,8 +3018,7 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host)
 	}
 
 	/* find out number of slots supported */
-	if (!device_property_read_u32(dev, "num-slots", &pdata->num_slots))
-		dev_info(dev, "'num-slots' was deprecated.\n");
+	device_property_read_u32(dev, "num-slots", &pdata->num_slots);
 
 	if (device_property_read_u32(dev, "fifo-depth", &pdata->fifo_depth))
 		dev_info(dev,
@@ -3252,12 +3251,18 @@ int dw_mci_probe(struct dw_mci *host)
 	if (ret)
 		goto err_dmaunmap;
 
-	/*
-	 * Even though dwmmc IP is provided the multiple slots,
-	 * there is no use case in mmc subsystem.
-	 * dwmmc host controller needs to initialize the one slot per an IP.
-	 */
-	host->num_slots = 1;
+	if (host->pdata->num_slots)
+		host->num_slots = host->pdata->num_slots;
+	else
+		host->num_slots = 1;
+
+	if (host->num_slots < 1 ||
+	    host->num_slots > SDMMC_GET_SLOT_NUM(mci_readl(host, HCON))) {
+		dev_err(host->dev,
+			"Platform data must supply correct num_slots.\n");
+		ret = -ENODEV;
+		goto err_clk_ciu;
+	}
 
 	/*
 	 * Enable interrupts for command done, data over, data empty,
-- 
1.8.3.1

[PATCH 8/9] mmc: dw_mmc: Support two SD_MMC_CE-ATA cards

[Liming Sun]

The dw_mmc controller supports two cards, but the current
driver only supports one card. The issue was found on a system
with two SD_MMC_CE-ATA cards. In such case, none of the cards
could come up when both are enabled.

According to the DesignWare Cores Mobile Storage Host Databook,
DW-MMC supports MMC-Ver3.3-only mode and SD_MMC_CE-ATA mode. In
the latter case, the memory cards are connected in a star topology
where each card has its own bus. In this case, the 'card-number'
in several registers needs to be set so the demux logic of the
controller can send the command or data to only the selected
card. This logic doesn't seem to be fully implemented in the
current dw-mmc driver.

This commit has the following changes to solve the two-card issue.
  - Set the CTYPE register for multi-card case;
  - Set the CLKENA register for multi-card case;
  - Set the 'slot_num' field in CMD register;
  - Program the CDTHRCTL register for the SD_MMC_CE-ATA mode which
    is mentioned as a must in the databook for version 270a.

Signed-off-by: Liming Sun <lsun@mellanox.com>
Reviewed-by: Chris Metcalf <cmetcalf@mellanox.com>
---
 drivers/mmc/host/dw_mmc.c | 75 ++++++++++++++++++++++++++++++++++++++---------
 drivers/mmc/host/dw_mmc.h | 10 +++++++
 2 files changed, 71 insertions(+), 14 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 150cd05..a3188c2 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -251,11 +251,36 @@ static bool dw_mci_ctrl_reset(struct dw_mci *host, u32 reset)
 	return true;
 }
 
-static void dw_mci_wait_while_busy(struct dw_mci *host, u32 cmd_flags)
+static void dw_mci_wait_while_busy(struct dw_mci *host, u32 cmd_flags,
+				   struct dw_mci_slot *slot)
 {
 	u32 status;
 
 	/*
+	 * Only one memory card should be selected at a time for command or data
+	 * transfer. For example, when a data transfer from a card occurs, a new
+	 * command should not be sent to another card; within the same card, a
+	 * new command is allowed to read the status, or to stop or abort the
+	 * current data transfer.
+	 */
+	status = mci_readl(host, CMD);
+	if (SDMMC_GET_CARD_NUM(status) != slot->id) {
+		/* Check the last command of the other card. */
+		if (readl_poll_timeout_atomic(host->regs + SDMMC_CMD,
+					      status,
+					      !(status & SDMMC_CMD_START),
+					      10, 500 * USEC_PER_MSEC))
+			dev_err(host->dev, "CMD busy; trying anyway\n");
+
+		/* Check the busy state of the other card. */
+		if (readl_poll_timeout_atomic(host->regs + SDMMC_STATUS,
+					      status,
+					      !(status & SDMMC_STATUS_BUSY),
+					      10, 500 * USEC_PER_MSEC))
+			dev_err(host->dev, "STATUS busy; trying anyway\n");
+	}
+
+	/*
 	 * Databook says that before issuing a new data transfer command
 	 * we need to check to see if the card is busy.  Data transfer commands
 	 * all have SDMMC_CMD_PRV_DAT_WAIT set, so we'll key off that.
@@ -280,8 +305,9 @@ static void mci_send_cmd(struct dw_mci_slot *slot, u32 cmd, u32 arg)
 
 	mci_writel(host, CMDARG, arg);
 	wmb(); /* drain writebuffer */
-	dw_mci_wait_while_busy(host, cmd);
-	mci_writel(host, CMD, SDMMC_CMD_START | cmd);
+	dw_mci_wait_while_busy(host, cmd, slot);
+	mci_writel(host, CMD, SDMMC_CMD_START | cmd |
+		   SDMMC_CMD_CARD_NUM(slot->id));
 
 	if (readl_poll_timeout_atomic(host->regs + SDMMC_CMD, cmd_status,
 				      !(cmd_status & SDMMC_CMD_START),
@@ -423,13 +449,14 @@ static void dw_mci_start_command(struct dw_mci *host,
 
 	mci_writel(host, CMDARG, cmd->arg);
 	wmb(); /* drain writebuffer */
-	dw_mci_wait_while_busy(host, cmd_flags);
+	dw_mci_wait_while_busy(host, cmd_flags, host->cur_slot);
 
 	/* response expected command only */
 	if (cmd_flags & SDMMC_CMD_RESP_EXP)
 		dw_mci_set_cto(host);
 
-	mci_writel(host, CMD, cmd_flags | SDMMC_CMD_START);
+	mci_writel(host, CMD, cmd_flags | SDMMC_CMD_START |
+		   SDMMC_CMD_CARD_NUM(host->cur_slot->id));
 }
 
 static inline void send_stop_abort(struct dw_mci *host, struct mmc_data *data)
@@ -1069,7 +1096,9 @@ static void dw_mci_ctrl_thld(struct dw_mci *host, struct mmc_data *data)
 		enable = SDMMC_CARD_RD_THR_EN;
 
 	if (host->timing != MMC_TIMING_MMC_HS200 &&
-	    host->timing != MMC_TIMING_UHS_SDR104)
+	    host->timing != MMC_TIMING_UHS_SDR104 &&
+	    (host->timing != MMC_TIMING_MMC_HS ||
+	    host->verid < DW_MMC_270A))
 		goto disable;
 
 	blksz_depth = blksz / (1 << host->data_shift);
@@ -1216,19 +1245,28 @@ static void dw_mci_submit_data(struct dw_mci *host, struct mmc_data *data)
 static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
 {
 	struct dw_mci *host = slot->host;
-	unsigned int clock = slot->clock;
+	unsigned int clock = slot->clock, old_clock;
 	u32 div;
-	u32 clk_en_a;
+	u32 clk_en_a, ctype;
 	u32 sdmmc_cmd_bits = SDMMC_CMD_UPD_CLK | SDMMC_CMD_PRV_DAT_WAIT;
 
 	/* We must continue to set bit 28 in CMD until the change is complete */
 	if (host->state == STATE_WAITING_CMD11_DONE)
 		sdmmc_cmd_bits |= SDMMC_CMD_VOLT_SWITCH;
 
+	/* Save the information for other slots. */
+	clk_en_a = mci_readl(host, CLKENA) &
+		   ~((SDMMC_CLKEN_ENABLE | SDMMC_CLKEN_LOW_PWR) << slot->id);
+	ctype = mci_readl(host, CTYPE) & ~(0x10001 << slot->id);
+
+	/* Only one clock is used in MMC-Ver3.3-only mode. */
+	old_clock = (host->card_type == DW_CARD_TYPE_MMC_ONLY) ?
+			host->current_speed : slot->__clk_old;
+
 	if (!clock) {
-		mci_writel(host, CLKENA, 0);
+		mci_writel(host, CLKENA, clk_en_a);
 		mci_send_cmd(slot, sdmmc_cmd_bits, 0);
-	} else if (clock != host->current_speed || force_clkinit) {
+	} else if (clock != old_clock || force_clkinit) {
 		div = host->bus_hz / clock;
 		if (host->bus_hz % clock && host->bus_hz > clock)
 			/*
@@ -1273,7 +1311,7 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
 		mci_send_cmd(slot, sdmmc_cmd_bits, 0);
 
 		/* enable clock; only low power if no SDIO */
-		clk_en_a = SDMMC_CLKEN_ENABLE << slot->id;
+		clk_en_a |= SDMMC_CLKEN_ENABLE << slot->id;
 		if (!test_bit(DW_MMC_CARD_NO_LOW_PWR, &slot->flags))
 			clk_en_a |= SDMMC_CLKEN_LOW_PWR << slot->id;
 		mci_writel(host, CLKENA, clk_en_a);
@@ -1288,7 +1326,11 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
 	host->current_speed = clock;
 
 	/* Set the current slot bus width */
-	mci_writel(host, CTYPE, (slot->ctype << slot->id));
+	if (slot->ctype == SDMMC_CTYPE_8BIT)
+		ctype |= 0x10000 << slot->id;
+	else
+		ctype |= slot->ctype << slot->id;
+	mci_writel(host, CTYPE, ctype);
 }
 
 static void __dw_mci_start_request(struct dw_mci *host,
@@ -3084,7 +3126,7 @@ int dw_mci_probe(struct dw_mci *host)
 {
 	const struct dw_mci_drv_data *drv_data = host->drv_data;
 	int width, i, ret = 0;
-	u32 fifo_size;
+	u32 fifo_size, hcon;
 	int init_slots = 0;
 
 	if (!host->pdata) {
@@ -3164,11 +3206,14 @@ int dw_mci_probe(struct dw_mci *host)
 	spin_lock_init(&host->irq_lock);
 	INIT_LIST_HEAD(&host->queue);
 
+	/* Read the Hardware Configuration Register (HCON). */
+	hcon = mci_readl(host, HCON);
+
 	/*
 	 * Get the host data width - this assumes that HCON has been set with
 	 * the correct values.
 	 */
-	i = SDMMC_GET_HDATA_WIDTH(mci_readl(host, HCON));
+	i = SDMMC_GET_HDATA_WIDTH(hcon);
 	if (!i) {
 		host->push_data = dw_mci_push_data16;
 		host->pull_data = dw_mci_pull_data16;
@@ -3251,6 +3296,8 @@ int dw_mci_probe(struct dw_mci *host)
 	if (ret)
 		goto err_dmaunmap;
 
+	host->card_type = SDMMC_GET_CARD_TYPE(hcon);
+
 	if (host->pdata->num_slots)
 		host->num_slots = host->pdata->num_slots;
 	else
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index e62cafc..a0a4348 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -106,6 +106,7 @@ struct dw_mci_dma_slave {
  * @num_slots: Number of slots available.
  * @fifoth_val: The value of FIFOTH register.
  * @verid: Denote Version ID.
+ * @card_type: The card type obtained from the HCON register.
  * @dev: Device associated with the MMC controller.
  * @pdata: Platform data associated with the MMC controller.
  * @drv_data: Driver specific data for identified variant of the controller
@@ -210,6 +211,7 @@ struct dw_mci {
 	u32			num_slots;
 	u32			fifoth_val;
 	u16			verid;
+	u8			card_type;
 	struct device		*dev;
 	struct dw_mci_board	*pdata;
 	const struct dw_mci_drv_data	*drv_data;
@@ -280,6 +282,7 @@ struct dw_mci_board {
 };
 
 #define DW_MMC_240A		0x240a
+#define DW_MMC_270A		0x270a
 #define DW_MMC_280A		0x280a
 
 #define SDMMC_CTRL		0x000
@@ -409,6 +412,8 @@ struct dw_mci_board {
 #define SDMMC_CMD_RESP_LONG		BIT(7)
 #define SDMMC_CMD_RESP_EXP		BIT(6)
 #define SDMMC_CMD_INDX(n)		((n) & 0x1F)
+#define SDMMC_CMD_CARD_NUM(n)		((n & 0x1F) << 16)
+#define SDMMC_GET_CARD_NUM(x)		(((x)>>16) & 0x1F)
 /* Status register defines */
 #define SDMMC_GET_FCNT(x)		(((x)>>17) & 0x1FFF)
 #define SDMMC_STATUS_DMA_REQ		BIT(31)
@@ -422,6 +427,7 @@ struct dw_mci_board {
 #define DMA_INTERFACE_DWDMA		(0x1)
 #define DMA_INTERFACE_GDMA		(0x2)
 #define DMA_INTERFACE_NODMA		(0x3)
+#define SDMMC_GET_CARD_TYPE(x)		((x) & 0x1)
 #define SDMMC_GET_TRANS_MODE(x)		(((x)>>16) & 0x3)
 #define SDMMC_GET_SLOT_NUM(x)		((((x)>>1) & 0x1F) + 1)
 #define SDMMC_GET_HDATA_WIDTH(x)	(((x)>>7) & 0x7)
@@ -452,6 +458,10 @@ struct dw_mci_board {
 #define SDMMC_CTRL_ALL_RESET_FLAGS \
 	(SDMMC_CTRL_RESET | SDMMC_CTRL_FIFO_RESET | SDMMC_CTRL_DMA_RESET)
 
+/* Card types */
+#define DW_CARD_TYPE_MMC_ONLY		0
+#define DW_CARD_TYPE_SD_MMC		1
+
 /* FIFO register access macros. These should not change the data endian-ness
  * as they are written to memory to be dealt with by the upper layers */
 #define mci_fifo_readw(__reg)	__raw_readw(__reg)
-- 
1.8.3.1

[PATCH 9/9] mmc: dw_mmc: Parse slot-specific configuration

[Liming Sun]

This commit adds code to parse the child-nodes for slot specific
configuration in the dw_mmc host driver according to
linux/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt

This change is needed when the cards have different configuration
like the 'bus-width'.

Signed-off-by: Liming Sun <lsun@mellanox.com>
Reviewed-by: Chris Metcalf <cmetcalf@mellanox.com>
---
 drivers/mmc/host/dw_mmc.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index a3188c2..9d870db 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2766,6 +2766,39 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static void mmc_parse_slot_cfg(struct mmc_host *mmc)
+{
+	struct dw_mci_slot *slot = mmc_priv(mmc);
+	struct fwnode_handle *child;
+	u32 bus_width, id;
+
+	/* Find the node for this card. */
+	device_for_each_child_node(mmc->parent, child) {
+		if (!fwnode_property_read_u32(child, "reg", &id) &&
+		    id == slot->id)
+			break;
+	}
+
+	if (!child)
+		return;
+
+	if (!fwnode_property_read_u32(child, "bus-width", &bus_width)) {
+		switch (bus_width) {
+		case 8:
+			mmc->caps |= MMC_CAP_8_BIT_DATA;
+			/* Hosts capable of 8-bit can also do 4 bits. */
+		case 4:
+			mmc->caps |= MMC_CAP_4_BIT_DATA;
+			break;
+		default:
+			break;
+		}
+	}
+
+	if (fwnode_property_read_bool(child, "disable-wp"))
+		mmc->caps2 |= MMC_CAP2_NO_WRITE_PROTECT;
+}
+
 static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
 {
 	struct mmc_host *mmc;
@@ -2830,10 +2863,14 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
 	if (host->pdata->caps2)
 		mmc->caps2 = host->pdata->caps2;
 
+	/* Parse common configuration. */
 	ret = mmc_of_parse(mmc);
 	if (ret)
 		goto err_host_allocated;
 
+	/* Check per-slot configuration. */
+	mmc_parse_slot_cfg(mmc);
+
 	/* Process SDIO IRQs through the sdio_irq_work. */
 	if (mmc->caps & MMC_CAP_SDIO_IRQ)
 		mmc->caps2 |= MMC_CAP2_SDIO_IRQ_NOTHREAD;
-- 
1.8.3.1

RE: [PATCH 0/9] Enable dw-mmc multi-card support

[Liming Sun]

So far I saw one comment (an ACK) on PATCH 1/9. There seems no comments on other patches yet. 
Ulf, (or other maintainers), any comments or suggestions how to move forward for this patch series?

Thanks,
Liming

-----Original Message-----
From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel-owner@vger.kernel.org] On Behalf Of Liming Sun
Sent: Friday, October 6, 2017 3:21 PM
To: Ulf Hansson <ulf.hansson@linaro.org>; Rob Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Jaehoon Chung <jh80.chung@samsung.com>; Kukjin Kim <kgene@kernel.org>; Krzysztof Kozlowski <krzk@kernel.org>
Cc: Liming Sun <lsun@mellanox.com>; linux-mmc@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.org
Subject: [PATCH 0/9] Enable dw-mmc multi-card support

This series of commits enables the multi-card support for the dw-mmc controller. It includes two parts as below.

The first part (patches 1-7) reverts the series of recent commits that removed the multi-card support with comments saying there was no such use case in the real world. Actually this feature is being used in Mellanox Bluefield SoC and has been requested by customers.

The second part (patches 8-9) fixes the DesignWare multi-card support according to the dw-mmc databook (synnopsys: DesignWare Cores Mobile Storage Host Databook, 2.70a). It has changes to set the card number into the CMD register to multiplex requests to different cards when working in SD_MMC_CEATA mode, set the CTYPE / CLKENA / CDTHRCTL registers properly according to the spec, and parse the per-card configuration to match the Linux Documentation (bindings/mmc/synopsys-dw-mshc.txt).

Liming Sun (9):
  Revert "Documentation: dw-mshc: deprecate num-slots"
  Revert "mmc: dw_mmc: remove the unnecessary slot variable"
  Revert "mmc: dw_mmc: use the 'slot' instead of 'cur_slot'"
  Revert "mmc: dw_mmc: remove the 'id' arguments about functions
    relevant to slot"
  Revert "mmc: dw_mmc: change the array of slots"
  Revert "mmc: dw_mmc: remove the loop about finding slots"
  Revert "mmc: dw_mmc: deprecated the "num-slots" property"
  mmc: dw_mmc: Support two SD_MMC_CE-ATA cards
  mmc: dw_mmc: Parse slot-specific configuration

 .../devicetree/bindings/mmc/synopsys-dw-mshc.txt   |  16 +-
 drivers/mmc/host/dw_mmc-exynos.c                   |   4 +-
 drivers/mmc/host/dw_mmc.c                          | 277 ++++++++++++++++-----
 drivers/mmc/host/dw_mmc.h                          |  17 +-
 4 files changed, 236 insertions(+), 78 deletions(-)

--
1.8.3.1

[PATCH 0/9] Enable dw-mmc multi-card support

[Liming Sun]

So far I saw one comment (an ACK) on PATCH 1/9. There seems no comments on other patches yet. 
Ulf, (or other maintainers), any comments or suggestions how to move forward for this patch series?

Thanks,
Liming

-----Original Message-----
From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel-owner at vger.kernel.org] On Behalf Of Liming Sun
Sent: Friday, October 6, 2017 3:21 PM
To: Ulf Hansson <ulf.hansson@linaro.org>; Rob Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Jaehoon Chung <jh80.chung@samsung.com>; Kukjin Kim <kgene@kernel.org>; Krzysztof Kozlowski <krzk@kernel.org>
Cc: Liming Sun <lsun@mellanox.com>; linux-mmc at vger.kernel.org; devicetree at vger.kernel.org; linux-kernel at vger.kernel.org; linux-arm-kernel at lists.infradead.org; linux-samsung-soc at vger.kernel.org
Subject: [PATCH 0/9] Enable dw-mmc multi-card support

This series of commits enables the multi-card support for the dw-mmc controller. It includes two parts as below.

The first part (patches 1-7) reverts the series of recent commits that removed the multi-card support with comments saying there was no such use case in the real world. Actually this feature is being used in Mellanox Bluefield SoC and has been requested by customers.

The second part (patches 8-9) fixes the DesignWare multi-card support according to the dw-mmc databook (synnopsys: DesignWare Cores Mobile Storage Host Databook, 2.70a). It has changes to set the card number into the CMD register to multiplex requests to different cards when working in SD_MMC_CEATA mode, set the CTYPE / CLKENA / CDTHRCTL registers properly according to the spec, and parse the per-card configuration to match the Linux Documentation (bindings/mmc/synopsys-dw-mshc.txt).

Liming Sun (9):
  Revert "Documentation: dw-mshc: deprecate num-slots"
  Revert "mmc: dw_mmc: remove the unnecessary slot variable"
  Revert "mmc: dw_mmc: use the 'slot' instead of 'cur_slot'"
  Revert "mmc: dw_mmc: remove the 'id' arguments about functions
    relevant to slot"
  Revert "mmc: dw_mmc: change the array of slots"
  Revert "mmc: dw_mmc: remove the loop about finding slots"
  Revert "mmc: dw_mmc: deprecated the "num-slots" property"
  mmc: dw_mmc: Support two SD_MMC_CE-ATA cards
  mmc: dw_mmc: Parse slot-specific configuration

 .../devicetree/bindings/mmc/synopsys-dw-mshc.txt   |  16 +-
 drivers/mmc/host/dw_mmc-exynos.c                   |   4 +-
 drivers/mmc/host/dw_mmc.c                          | 277 ++++++++++++++++-----
 drivers/mmc/host/dw_mmc.h                          |  17 +-
 4 files changed, 236 insertions(+), 78 deletions(-)

--
1.8.3.1

RE: [PATCH 0/9] Enable dw-mmc multi-card support

[Liming Sun]

So far I saw one comment (an ACK) on PATCH 1/9. There seems no comments on other patches yet. 
Ulf, (or other maintainers), any comments or suggestions how to move forward for this patch series?

Thanks,
Liming

-----Original Message-----
From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel-owner@vger.kernel.org] On Behalf Of Liming Sun
Sent: Friday, October 6, 2017 3:21 PM
To: Ulf Hansson <ulf.hansson@linaro.org>; Rob Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Jaehoon Chung <jh80.chung@samsung.com>; Kukjin Kim <kgene@kernel.org>; Krzysztof Kozlowski <krzk@kernel.org>
Cc: Liming Sun <lsun@mellanox.com>; linux-mmc@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.org
Subject: [PATCH 0/9] Enable dw-mmc multi-card support

This series of commits enables the multi-card support for the dw-mmc controller. It includes two parts as below.

The first part (patches 1-7) reverts the series of recent commits that removed the multi-card support with comments saying there was no such use case in the real world. Actually this feature is being used in Mellanox Bluefield SoC and has been requested by customers.

The second part (patches 8-9) fixes the DesignWare multi-card support according to the dw-mmc databook (synnopsys: DesignWare Cores Mobile Storage Host Databook, 2.70a). It has changes to set the card number into the CMD register to multiplex requests to different cards when working in SD_MMC_CEATA mode, set the CTYPE / CLKENA / CDTHRCTL registers properly according to the spec, and parse the per-card configuration to match the Linux Documentation (bindings/mmc/synopsys-dw-mshc.txt).

Liming Sun (9):
  Revert "Documentation: dw-mshc: deprecate num-slots"
  Revert "mmc: dw_mmc: remove the unnecessary slot variable"
  Revert "mmc: dw_mmc: use the 'slot' instead of 'cur_slot'"
  Revert "mmc: dw_mmc: remove the 'id' arguments about functions
    relevant to slot"
  Revert "mmc: dw_mmc: change the array of slots"
  Revert "mmc: dw_mmc: remove the loop about finding slots"
  Revert "mmc: dw_mmc: deprecated the "num-slots" property"
  mmc: dw_mmc: Support two SD_MMC_CE-ATA cards
  mmc: dw_mmc: Parse slot-specific configuration

 .../devicetree/bindings/mmc/synopsys-dw-mshc.txt   |  16 +-
 drivers/mmc/host/dw_mmc-exynos.c                   |   4 +-
 drivers/mmc/host/dw_mmc.c                          | 277 ++++++++++++++++-----
 drivers/mmc/host/dw_mmc.h                          |  17 +-
 4 files changed, 236 insertions(+), 78 deletions(-)

--
1.8.3.1

Re: [PATCH 0/9] Enable dw-mmc multi-card support

[Shawn Lin]

On 2017/10/7 3:21, Liming Sun wrote:
> This series of commits enables the multi-card support for the dw-mmc
> controller. It includes two parts as below.
> 
> The first part (patches 1-7) reverts the series of recent commits that
> removed the multi-card support with comments saying there was no such
> use case in the real world. Actually this feature is being used in
> Mellanox Bluefield SoC and has been requested by customers.

Hrm.... it's so unlucky that your patchset comes a little late. As
your patch 8 and 9 said, you need them to fix problem for multi-card
support, so definitely there was no such use case, and even the code
was buggy to support it right? That makes the code hard to read and
maintain, so we decide to remove it.

> 
> The second part (patches 8-9) fixes the DesignWare multi-card support
> according to the dw-mmc databook (synnopsys: DesignWare Cores Mobile
> Storage Host Databook, 2.70a). It has changes to set the card number
> into the CMD register to multiplex requests to different cards when
> working in SD_MMC_CEATA mode, set the CTYPE / CLKENA / CDTHRCTL
> registers properly according to the spec, and parse the per-card
> configuration to match the Linux Documentation
> (bindings/mmc/synopsys-dw-mshc.txt).

Havn'e check the databook for details yet, but I think it's ok
to re-introduce multi-slot support if a real user benefits from
it. But you need a new patch to silent the log "num-slots property not
found, assuming 1 slot is available" as we removed all the num-slots
from DT at that time.


> 
> Liming Sun (9):
>    Revert "Documentation: dw-mshc: deprecate num-slots"
>    Revert "mmc: dw_mmc: remove the unnecessary slot variable"
>    Revert "mmc: dw_mmc: use the 'slot' instead of 'cur_slot'"
>    Revert "mmc: dw_mmc: remove the 'id' arguments about functions
>      relevant to slot"
>    Revert "mmc: dw_mmc: change the array of slots"
>    Revert "mmc: dw_mmc: remove the loop about finding slots"
>    Revert "mmc: dw_mmc: deprecated the "num-slots" property"
>    mmc: dw_mmc: Support two SD_MMC_CE-ATA cards
>    mmc: dw_mmc: Parse slot-specific configuration
> 
>   .../devicetree/bindings/mmc/synopsys-dw-mshc.txt   |  16 +-
>   drivers/mmc/host/dw_mmc-exynos.c                   |   4 +-
>   drivers/mmc/host/dw_mmc.c                          | 277 ++++++++++++++++-----
>   drivers/mmc/host/dw_mmc.h                          |  17 +-
>   4 files changed, 236 insertions(+), 78 deletions(-)
> 

[PATCH 0/9] Enable dw-mmc multi-card support

[Shawn Lin]

On 2017/10/7 3:21, Liming Sun wrote:
> This series of commits enables the multi-card support for the dw-mmc
> controller. It includes two parts as below.
> 
> The first part (patches 1-7) reverts the series of recent commits that
> removed the multi-card support with comments saying there was no such
> use case in the real world. Actually this feature is being used in
> Mellanox Bluefield SoC and has been requested by customers.

Hrm.... it's so unlucky that your patchset comes a little late. As
your patch 8 and 9 said, you need them to fix problem for multi-card
support, so definitely there was no such use case, and even the code
was buggy to support it right? That makes the code hard to read and
maintain, so we decide to remove it.

> 
> The second part (patches 8-9) fixes the DesignWare multi-card support
> according to the dw-mmc databook (synnopsys: DesignWare Cores Mobile
> Storage Host Databook, 2.70a). It has changes to set the card number
> into the CMD register to multiplex requests to different cards when
> working in SD_MMC_CEATA mode, set the CTYPE / CLKENA / CDTHRCTL
> registers properly according to the spec, and parse the per-card
> configuration to match the Linux Documentation
> (bindings/mmc/synopsys-dw-mshc.txt).

Havn'e check the databook for details yet, but I think it's ok
to re-introduce multi-slot support if a real user benefits from
it. But you need a new patch to silent the log "num-slots property not
found, assuming 1 slot is available" as we removed all the num-slots
from DT at that time.


> 
> Liming Sun (9):
>    Revert "Documentation: dw-mshc: deprecate num-slots"
>    Revert "mmc: dw_mmc: remove the unnecessary slot variable"
>    Revert "mmc: dw_mmc: use the 'slot' instead of 'cur_slot'"
>    Revert "mmc: dw_mmc: remove the 'id' arguments about functions
>      relevant to slot"
>    Revert "mmc: dw_mmc: change the array of slots"
>    Revert "mmc: dw_mmc: remove the loop about finding slots"
>    Revert "mmc: dw_mmc: deprecated the "num-slots" property"
>    mmc: dw_mmc: Support two SD_MMC_CE-ATA cards
>    mmc: dw_mmc: Parse slot-specific configuration
> 
>   .../devicetree/bindings/mmc/synopsys-dw-mshc.txt   |  16 +-
>   drivers/mmc/host/dw_mmc-exynos.c                   |   4 +-
>   drivers/mmc/host/dw_mmc.c                          | 277 ++++++++++++++++-----
>   drivers/mmc/host/dw_mmc.h                          |  17 +-
>   4 files changed, 236 insertions(+), 78 deletions(-)
> 

Re: [PATCH 0/9] Enable dw-mmc multi-card support

[Shawn Lin]

On 2017/10/7 3:21, Liming Sun wrote:
> This series of commits enables the multi-card support for the dw-mmc
> controller. It includes two parts as below.
> 
> The first part (patches 1-7) reverts the series of recent commits that
> removed the multi-card support with comments saying there was no such
> use case in the real world. Actually this feature is being used in
> Mellanox Bluefield SoC and has been requested by customers.

Hrm.... it's so unlucky that your patchset comes a little late. As
your patch 8 and 9 said, you need them to fix problem for multi-card
support, so definitely there was no such use case, and even the code
was buggy to support it right? That makes the code hard to read and
maintain, so we decide to remove it.

> 
> The second part (patches 8-9) fixes the DesignWare multi-card support
> according to the dw-mmc databook (synnopsys: DesignWare Cores Mobile
> Storage Host Databook, 2.70a). It has changes to set the card number
> into the CMD register to multiplex requests to different cards when
> working in SD_MMC_CEATA mode, set the CTYPE / CLKENA / CDTHRCTL
> registers properly according to the spec, and parse the per-card
> configuration to match the Linux Documentation
> (bindings/mmc/synopsys-dw-mshc.txt).

Havn'e check the databook for details yet, but I think it's ok
to re-introduce multi-slot support if a real user benefits from
it. But you need a new patch to silent the log "num-slots property not
found, assuming 1 slot is available" as we removed all the num-slots
from DT at that time.


> 
> Liming Sun (9):
>    Revert "Documentation: dw-mshc: deprecate num-slots"
>    Revert "mmc: dw_mmc: remove the unnecessary slot variable"
>    Revert "mmc: dw_mmc: use the 'slot' instead of 'cur_slot'"
>    Revert "mmc: dw_mmc: remove the 'id' arguments about functions
>      relevant to slot"
>    Revert "mmc: dw_mmc: change the array of slots"
>    Revert "mmc: dw_mmc: remove the loop about finding slots"
>    Revert "mmc: dw_mmc: deprecated the "num-slots" property"
>    mmc: dw_mmc: Support two SD_MMC_CE-ATA cards
>    mmc: dw_mmc: Parse slot-specific configuration
> 
>   .../devicetree/bindings/mmc/synopsys-dw-mshc.txt   |  16 +-
>   drivers/mmc/host/dw_mmc-exynos.c                   |   4 +-
>   drivers/mmc/host/dw_mmc.c                          | 277 ++++++++++++++++-----
>   drivers/mmc/host/dw_mmc.h                          |  17 +-
>   4 files changed, 236 insertions(+), 78 deletions(-)
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 0/9] Enable dw-mmc multi-card support

[Liming Sun]

>> Hrm.... it's so unlucky that your patchset comes a little late. As your patch 8 and 9 said, you need them to fix problem for multi-card support, so definitely there was no such use case, and even the code was buggy to support it right? That makes the code hard to read and maintain, so we decide to remove it.

Thanks for the feedback. Yes, earlier the multi-card support was buggy indeed. We spent some time to debug it and got it working.

>> Havn'e check the databook for details yet, but I think it's ok to re-introduce multi-slot support if a real user benefits from it. But you need a new patch to silent the log "num-slots property not found, assuming 1 slot is available" as we removed all the num-slots from DT at that time.

The " num-slots property not found..." log message has already been removed by 8a629d26f back in 2016. Looks like we're good on this one. In dw_mci_probe (), it has code to check pdata->num_slots. If 0, the host->num_slots will be set to 1. So the logic of setting default num_slots seems already there. But correct me if I am wrong.

Thanks,
Liming

-----Original Message-----
From: Shawn Lin [mailto:shawn.lin@rock-chips.com] 
Sent: Monday, October 16, 2017 9:36 PM
To: Liming Sun <lsun@mellanox.com>; Jaehoon Chung <jh80.chung@samsung.com>
Cc: Ulf Hansson <ulf.hansson@linaro.org>; Rob Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Kukjin Kim <kgene@kernel.org>; Krzysztof Kozlowski <krzk@kernel.org>; shawn.lin@rock-chips.com; linux-mmc@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.org
Subject: Re: [PATCH 0/9] Enable dw-mmc multi-card support


On 2017/10/7 3:21, Liming Sun wrote:
> This series of commits enables the multi-card support for the dw-mmc 
> controller. It includes two parts as below.
> 
> The first part (patches 1-7) reverts the series of recent commits that 
> removed the multi-card support with comments saying there was no such 
> use case in the real world. Actually this feature is being used in 
> Mellanox Bluefield SoC and has been requested by customers.

Hrm.... it's so unlucky that your patchset comes a little late. As your patch 8 and 9 said, you need them to fix problem for multi-card support, so definitely there was no such use case, and even the code was buggy to support it right? That makes the code hard to read and maintain, so we decide to remove it.

> 
> The second part (patches 8-9) fixes the DesignWare multi-card support 
> according to the dw-mmc databook (synnopsys: DesignWare Cores Mobile 
> Storage Host Databook, 2.70a). It has changes to set the card number 
> into the CMD register to multiplex requests to different cards when 
> working in SD_MMC_CEATA mode, set the CTYPE / CLKENA / CDTHRCTL 
> registers properly according to the spec, and parse the per-card 
> configuration to match the Linux Documentation 
> (bindings/mmc/synopsys-dw-mshc.txt).

Havn'e check the databook for details yet, but I think it's ok to re-introduce multi-slot support if a real user benefits from it. But you need a new patch to silent the log "num-slots property not found, assuming 1 slot is available" as we removed all the num-slots from DT at that time.


> 
> Liming Sun (9):
>    Revert "Documentation: dw-mshc: deprecate num-slots"
>    Revert "mmc: dw_mmc: remove the unnecessary slot variable"
>    Revert "mmc: dw_mmc: use the 'slot' instead of 'cur_slot'"
>    Revert "mmc: dw_mmc: remove the 'id' arguments about functions
>      relevant to slot"
>    Revert "mmc: dw_mmc: change the array of slots"
>    Revert "mmc: dw_mmc: remove the loop about finding slots"
>    Revert "mmc: dw_mmc: deprecated the "num-slots" property"
>    mmc: dw_mmc: Support two SD_MMC_CE-ATA cards
>    mmc: dw_mmc: Parse slot-specific configuration
> 
>   .../devicetree/bindings/mmc/synopsys-dw-mshc.txt   |  16 +-
>   drivers/mmc/host/dw_mmc-exynos.c                   |   4 +-
>   drivers/mmc/host/dw_mmc.c                          | 277 ++++++++++++++++-----
>   drivers/mmc/host/dw_mmc.h                          |  17 +-
>   4 files changed, 236 insertions(+), 78 deletions(-)
> 

[PATCH 0/9] Enable dw-mmc multi-card support

[Liming Sun]

>> Hrm.... it's so unlucky that your patchset comes a little late. As your patch 8 and 9 said, you need them to fix problem for multi-card support, so definitely there was no such use case, and even the code was buggy to support it right? That makes the code hard to read and maintain, so we decide to remove it.

Thanks for the feedback. Yes, earlier the multi-card support was buggy indeed. We spent some time to debug it and got it working.

>> Havn'e check the databook for details yet, but I think it's ok to re-introduce multi-slot support if a real user benefits from it. But you need a new patch to silent the log "num-slots property not found, assuming 1 slot is available" as we removed all the num-slots from DT at that time.

The " num-slots property not found..." log message has already been removed by 8a629d26f back in 2016. Looks like we're good on this one. In dw_mci_probe (), it has code to check pdata->num_slots. If 0, the host->num_slots will be set to 1. So the logic of setting default num_slots seems already there. But correct me if I am wrong.

Thanks,
Liming

-----Original Message-----
From: Shawn Lin [mailto:shawn.lin at rock-chips.com] 
Sent: Monday, October 16, 2017 9:36 PM
To: Liming Sun <lsun@mellanox.com>; Jaehoon Chung <jh80.chung@samsung.com>
Cc: Ulf Hansson <ulf.hansson@linaro.org>; Rob Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Kukjin Kim <kgene@kernel.org>; Krzysztof Kozlowski <krzk@kernel.org>; shawn.lin at rock-chips.com; linux-mmc at vger.kernel.org; devicetree at vger.kernel.org; linux-kernel at vger.kernel.org; linux-arm-kernel at lists.infradead.org; linux-samsung-soc at vger.kernel.org
Subject: Re: [PATCH 0/9] Enable dw-mmc multi-card support


On 2017/10/7 3:21, Liming Sun wrote:
> This series of commits enables the multi-card support for the dw-mmc 
> controller. It includes two parts as below.
> 
> The first part (patches 1-7) reverts the series of recent commits that 
> removed the multi-card support with comments saying there was no such 
> use case in the real world. Actually this feature is being used in 
> Mellanox Bluefield SoC and has been requested by customers.

Hrm.... it's so unlucky that your patchset comes a little late. As your patch 8 and 9 said, you need them to fix problem for multi-card support, so definitely there was no such use case, and even the code was buggy to support it right? That makes the code hard to read and maintain, so we decide to remove it.

> 
> The second part (patches 8-9) fixes the DesignWare multi-card support 
> according to the dw-mmc databook (synnopsys: DesignWare Cores Mobile 
> Storage Host Databook, 2.70a). It has changes to set the card number 
> into the CMD register to multiplex requests to different cards when 
> working in SD_MMC_CEATA mode, set the CTYPE / CLKENA / CDTHRCTL 
> registers properly according to the spec, and parse the per-card 
> configuration to match the Linux Documentation 
> (bindings/mmc/synopsys-dw-mshc.txt).

Havn'e check the databook for details yet, but I think it's ok to re-introduce multi-slot support if a real user benefits from it. But you need a new patch to silent the log "num-slots property not found, assuming 1 slot is available" as we removed all the num-slots from DT at that time.


> 
> Liming Sun (9):
>    Revert "Documentation: dw-mshc: deprecate num-slots"
>    Revert "mmc: dw_mmc: remove the unnecessary slot variable"
>    Revert "mmc: dw_mmc: use the 'slot' instead of 'cur_slot'"
>    Revert "mmc: dw_mmc: remove the 'id' arguments about functions
>      relevant to slot"
>    Revert "mmc: dw_mmc: change the array of slots"
>    Revert "mmc: dw_mmc: remove the loop about finding slots"
>    Revert "mmc: dw_mmc: deprecated the "num-slots" property"
>    mmc: dw_mmc: Support two SD_MMC_CE-ATA cards
>    mmc: dw_mmc: Parse slot-specific configuration
> 
>   .../devicetree/bindings/mmc/synopsys-dw-mshc.txt   |  16 +-
>   drivers/mmc/host/dw_mmc-exynos.c                   |   4 +-
>   drivers/mmc/host/dw_mmc.c                          | 277 ++++++++++++++++-----
>   drivers/mmc/host/dw_mmc.h                          |  17 +-
>   4 files changed, 236 insertions(+), 78 deletions(-)
> 

RE: [PATCH 0/9] Enable dw-mmc multi-card support

[Liming Sun]

>> Hrm.... it's so unlucky that your patchset comes a little late. As your patch 8 and 9 said, you need them to fix problem for multi-card support, so definitely there was no such use case, and even the code was buggy to support it right? That makes the code hard to read and maintain, so we decide to remove it.

Thanks for the feedback. Yes, earlier the multi-card support was buggy indeed. We spent some time to debug it and got it working.

>> Havn'e check the databook for details yet, but I think it's ok to re-introduce multi-slot support if a real user benefits from it. But you need a new patch to silent the log "num-slots property not found, assuming 1 slot is available" as we removed all the num-slots from DT at that time.

The " num-slots property not found..." log message has already been removed by 8a629d26f back in 2016. Looks like we're good on this one. In dw_mci_probe (), it has code to check pdata->num_slots. If 0, the host->num_slots will be set to 1. So the logic of setting default num_slots seems already there. But correct me if I am wrong.

Thanks,
Liming

-----Original Message-----
From: Shawn Lin [mailto:shawn.lin@rock-chips.com] 
Sent: Monday, October 16, 2017 9:36 PM
To: Liming Sun <lsun@mellanox.com>; Jaehoon Chung <jh80.chung@samsung.com>
Cc: Ulf Hansson <ulf.hansson@linaro.org>; Rob Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Kukjin Kim <kgene@kernel.org>; Krzysztof Kozlowski <krzk@kernel.org>; shawn.lin@rock-chips.com; linux-mmc@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.org
Subject: Re: [PATCH 0/9] Enable dw-mmc multi-card support


On 2017/10/7 3:21, Liming Sun wrote:
> This series of commits enables the multi-card support for the dw-mmc 
> controller. It includes two parts as below.
> 
> The first part (patches 1-7) reverts the series of recent commits that 
> removed the multi-card support with comments saying there was no such 
> use case in the real world. Actually this feature is being used in 
> Mellanox Bluefield SoC and has been requested by customers.

Hrm.... it's so unlucky that your patchset comes a little late. As your patch 8 and 9 said, you need them to fix problem for multi-card support, so definitely there was no such use case, and even the code was buggy to support it right? That makes the code hard to read and maintain, so we decide to remove it.

> 
> The second part (patches 8-9) fixes the DesignWare multi-card support 
> according to the dw-mmc databook (synnopsys: DesignWare Cores Mobile 
> Storage Host Databook, 2.70a). It has changes to set the card number 
> into the CMD register to multiplex requests to different cards when 
> working in SD_MMC_CEATA mode, set the CTYPE / CLKENA / CDTHRCTL 
> registers properly according to the spec, and parse the per-card 
> configuration to match the Linux Documentation 
> (bindings/mmc/synopsys-dw-mshc.txt).

Havn'e check the databook for details yet, but I think it's ok to re-introduce multi-slot support if a real user benefits from it. But you need a new patch to silent the log "num-slots property not found, assuming 1 slot is available" as we removed all the num-slots from DT at that time.


> 
> Liming Sun (9):
>    Revert "Documentation: dw-mshc: deprecate num-slots"
>    Revert "mmc: dw_mmc: remove the unnecessary slot variable"
>    Revert "mmc: dw_mmc: use the 'slot' instead of 'cur_slot'"
>    Revert "mmc: dw_mmc: remove the 'id' arguments about functions
>      relevant to slot"
>    Revert "mmc: dw_mmc: change the array of slots"
>    Revert "mmc: dw_mmc: remove the loop about finding slots"
>    Revert "mmc: dw_mmc: deprecated the "num-slots" property"
>    mmc: dw_mmc: Support two SD_MMC_CE-ATA cards
>    mmc: dw_mmc: Parse slot-specific configuration
> 
>   .../devicetree/bindings/mmc/synopsys-dw-mshc.txt   |  16 +-
>   drivers/mmc/host/dw_mmc-exynos.c                   |   4 +-
>   drivers/mmc/host/dw_mmc.c                          | 277 ++++++++++++++++-----
>   drivers/mmc/host/dw_mmc.h                          |  17 +-
>   4 files changed, 236 insertions(+), 78 deletions(-)
> 

Re: [PATCH 0/9] Enable dw-mmc multi-card support

[Jaehoon Chung]

Sorry for late this..

On 10/18/2017 12:52 AM, Liming Sun wrote:
>>> Hrm.... it's so unlucky that your patchset comes a little late. As your patch 8 and 9 said, you need them to fix problem for multi-card support, so definitely there was no such use case, and even the code was buggy to support it right? That makes the code hard to read and maintain, so we decide to remove it.
> 
> Thanks for the feedback. Yes, earlier the multi-card support was buggy indeed. We spent some time to debug it and got it working.
> 
>>> Havn'e check the databook for details yet, but I think it's ok to re-introduce multi-slot support if a real user benefits from it. But you need a new patch to silent the log "num-slots property not found, assuming 1 slot is available" as we removed all the num-slots from DT at that time.
> 
> The " num-slots property not found..." log message has already been removed by 8a629d26f back in 2016. Looks like we're good on this one. In dw_mci_probe (), it has code to check pdata->num_slots. If 0, the host->num_slots will be set to 1. So the logic of setting default num_slots seems already there. But correct me if I am wrong.
> 
> Thanks,
> Liming
> 
> -----Original Message-----
> From: Shawn Lin [mailto:shawn.lin@rock-chips.com] 
> Sent: Monday, October 16, 2017 9:36 PM
> To: Liming Sun <lsun@mellanox.com>; Jaehoon Chung <jh80.chung@samsung.com>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>; Rob Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Kukjin Kim <kgene@kernel.org>; Krzysztof Kozlowski <krzk@kernel.org>; shawn.lin@rock-chips.com; linux-mmc@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.org
> Subject: Re: [PATCH 0/9] Enable dw-mmc multi-card support
> 
> 
> On 2017/10/7 3:21, Liming Sun wrote:
>> This series of commits enables the multi-card support for the dw-mmc 
>> controller. It includes two parts as below.
>>
>> The first part (patches 1-7) reverts the series of recent commits that 
>> removed the multi-card support with comments saying there was no such 
>> use case in the real world. Actually this feature is being used in 
>> Mellanox Bluefield SoC and has been requested by customers.
> 
> Hrm.... it's so unlucky that your patchset comes a little late. As your patch 8 and 9 said, you need them to fix problem for multi-card support, so definitely there was no such use case, and even the code was buggy to support it right? That makes the code hard to read and maintain, so we decide to remove it.

Hmm..
Well, if i missed your reply for my removing patch, it's my fault..but i didn't see any reply..
At that time, we didn't see any usage and also now...

Are there any patches for using multi slot? 
e,g) device-tree file or your own driver

If there are big benefits to revert them,  i don't want to back them..during almost 6 years, never use it..

> 
>>
>> The second part (patches 8-9) fixes the DesignWare multi-card support 
>> according to the dw-mmc databook (synnopsys: DesignWare Cores Mobile 
>> Storage Host Databook, 2.70a). It has changes to set the card number 
>> into the CMD register to multiplex requests to different cards when 
>> working in SD_MMC_CEATA mode, set the CTYPE / CLKENA / CDTHRCTL 
>> registers properly according to the spec, and parse the per-card 
>> configuration to match the Linux Documentation 
>> (bindings/mmc/synopsys-dw-mshc.txt).

the second part seems that it's only support since v2.70a..?

> 
> Havn'e check the databook for details yet, but I think it's ok to re-introduce multi-slot support if a real user benefits from it. But you need a new patch to silent the log "num-slots property not found, assuming 1 slot is available" as we removed all the num-slots from DT at that time.
> 
> 
>>
>> Liming Sun (9):
>>    Revert "Documentation: dw-mshc: deprecate num-slots"
>>    Revert "mmc: dw_mmc: remove the unnecessary slot variable"
>>    Revert "mmc: dw_mmc: use the 'slot' instead of 'cur_slot'"
>>    Revert "mmc: dw_mmc: remove the 'id' arguments about functions
>>      relevant to slot"
>>    Revert "mmc: dw_mmc: change the array of slots"
>>    Revert "mmc: dw_mmc: remove the loop about finding slots"
>>    Revert "mmc: dw_mmc: deprecated the "num-slots" property"
>>    mmc: dw_mmc: Support two SD_MMC_CE-ATA cards
>>    mmc: dw_mmc: Parse slot-specific configuration
>>
>>   .../devicetree/bindings/mmc/synopsys-dw-mshc.txt   |  16 +-
>>   drivers/mmc/host/dw_mmc-exynos.c                   |   4 +-
>>   drivers/mmc/host/dw_mmc.c                          | 277 ++++++++++++++++-----
>>   drivers/mmc/host/dw_mmc.h                          |  17 +-
>>   4 files changed, 236 insertions(+), 78 deletions(-)
>>
> 
> 
> 
> 

[PATCH 0/9] Enable dw-mmc multi-card support

[Jaehoon Chung]

Sorry for late this..

On 10/18/2017 12:52 AM, Liming Sun wrote:
>>> Hrm.... it's so unlucky that your patchset comes a little late. As your patch 8 and 9 said, you need them to fix problem for multi-card support, so definitely there was no such use case, and even the code was buggy to support it right? That makes the code hard to read and maintain, so we decide to remove it.
> 
> Thanks for the feedback. Yes, earlier the multi-card support was buggy indeed. We spent some time to debug it and got it working.
> 
>>> Havn'e check the databook for details yet, but I think it's ok to re-introduce multi-slot support if a real user benefits from it. But you need a new patch to silent the log "num-slots property not found, assuming 1 slot is available" as we removed all the num-slots from DT at that time.
> 
> The " num-slots property not found..." log message has already been removed by 8a629d26f back in 2016. Looks like we're good on this one. In dw_mci_probe (), it has code to check pdata->num_slots. If 0, the host->num_slots will be set to 1. So the logic of setting default num_slots seems already there. But correct me if I am wrong.
> 
> Thanks,
> Liming
> 
> -----Original Message-----
> From: Shawn Lin [mailto:shawn.lin at rock-chips.com] 
> Sent: Monday, October 16, 2017 9:36 PM
> To: Liming Sun <lsun@mellanox.com>; Jaehoon Chung <jh80.chung@samsung.com>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>; Rob Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Kukjin Kim <kgene@kernel.org>; Krzysztof Kozlowski <krzk@kernel.org>; shawn.lin at rock-chips.com; linux-mmc at vger.kernel.org; devicetree at vger.kernel.org; linux-kernel at vger.kernel.org; linux-arm-kernel at lists.infradead.org; linux-samsung-soc at vger.kernel.org
> Subject: Re: [PATCH 0/9] Enable dw-mmc multi-card support
> 
> 
> On 2017/10/7 3:21, Liming Sun wrote:
>> This series of commits enables the multi-card support for the dw-mmc 
>> controller. It includes two parts as below.
>>
>> The first part (patches 1-7) reverts the series of recent commits that 
>> removed the multi-card support with comments saying there was no such 
>> use case in the real world. Actually this feature is being used in 
>> Mellanox Bluefield SoC and has been requested by customers.
> 
> Hrm.... it's so unlucky that your patchset comes a little late. As your patch 8 and 9 said, you need them to fix problem for multi-card support, so definitely there was no such use case, and even the code was buggy to support it right? That makes the code hard to read and maintain, so we decide to remove it.

Hmm..
Well, if i missed your reply for my removing patch, it's my fault..but i didn't see any reply..
At that time, we didn't see any usage and also now...

Are there any patches for using multi slot? 
e,g) device-tree file or your own driver

If there are big benefits to revert them,  i don't want to back them..during almost 6 years, never use it..

> 
>>
>> The second part (patches 8-9) fixes the DesignWare multi-card support 
>> according to the dw-mmc databook (synnopsys: DesignWare Cores Mobile 
>> Storage Host Databook, 2.70a). It has changes to set the card number 
>> into the CMD register to multiplex requests to different cards when 
>> working in SD_MMC_CEATA mode, set the CTYPE / CLKENA / CDTHRCTL 
>> registers properly according to the spec, and parse the per-card 
>> configuration to match the Linux Documentation 
>> (bindings/mmc/synopsys-dw-mshc.txt).

the second part seems that it's only support since v2.70a..?

> 
> Havn'e check the databook for details yet, but I think it's ok to re-introduce multi-slot support if a real user benefits from it. But you need a new patch to silent the log "num-slots property not found, assuming 1 slot is available" as we removed all the num-slots from DT at that time.
> 
> 
>>
>> Liming Sun (9):
>>    Revert "Documentation: dw-mshc: deprecate num-slots"
>>    Revert "mmc: dw_mmc: remove the unnecessary slot variable"
>>    Revert "mmc: dw_mmc: use the 'slot' instead of 'cur_slot'"
>>    Revert "mmc: dw_mmc: remove the 'id' arguments about functions
>>      relevant to slot"
>>    Revert "mmc: dw_mmc: change the array of slots"
>>    Revert "mmc: dw_mmc: remove the loop about finding slots"
>>    Revert "mmc: dw_mmc: deprecated the "num-slots" property"
>>    mmc: dw_mmc: Support two SD_MMC_CE-ATA cards
>>    mmc: dw_mmc: Parse slot-specific configuration
>>
>>   .../devicetree/bindings/mmc/synopsys-dw-mshc.txt   |  16 +-
>>   drivers/mmc/host/dw_mmc-exynos.c                   |   4 +-
>>   drivers/mmc/host/dw_mmc.c                          | 277 ++++++++++++++++-----
>>   drivers/mmc/host/dw_mmc.h                          |  17 +-
>>   4 files changed, 236 insertions(+), 78 deletions(-)
>>
> 
> 
> 
> 

Re: [PATCH 0/9] Enable dw-mmc multi-card support

[Jaehoon Chung]

Sorry for late this..

On 10/18/2017 12:52 AM, Liming Sun wrote:
>>> Hrm.... it's so unlucky that your patchset comes a little late. As your patch 8 and 9 said, you need them to fix problem for multi-card support, so definitely there was no such use case, and even the code was buggy to support it right? That makes the code hard to read and maintain, so we decide to remove it.
> 
> Thanks for the feedback. Yes, earlier the multi-card support was buggy indeed. We spent some time to debug it and got it working.
> 
>>> Havn'e check the databook for details yet, but I think it's ok to re-introduce multi-slot support if a real user benefits from it. But you need a new patch to silent the log "num-slots property not found, assuming 1 slot is available" as we removed all the num-slots from DT at that time.
> 
> The " num-slots property not found..." log message has already been removed by 8a629d26f back in 2016. Looks like we're good on this one. In dw_mci_probe (), it has code to check pdata->num_slots. If 0, the host->num_slots will be set to 1. So the logic of setting default num_slots seems already there. But correct me if I am wrong.
> 
> Thanks,
> Liming
> 
> -----Original Message-----
> From: Shawn Lin [mailto:shawn.lin@rock-chips.com] 
> Sent: Monday, October 16, 2017 9:36 PM
> To: Liming Sun <lsun@mellanox.com>; Jaehoon Chung <jh80.chung@samsung.com>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>; Rob Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Kukjin Kim <kgene@kernel.org>; Krzysztof Kozlowski <krzk@kernel.org>; shawn.lin@rock-chips.com; linux-mmc@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.org
> Subject: Re: [PATCH 0/9] Enable dw-mmc multi-card support
> 
> 
> On 2017/10/7 3:21, Liming Sun wrote:
>> This series of commits enables the multi-card support for the dw-mmc 
>> controller. It includes two parts as below.
>>
>> The first part (patches 1-7) reverts the series of recent commits that 
>> removed the multi-card support with comments saying there was no such 
>> use case in the real world. Actually this feature is being used in 
>> Mellanox Bluefield SoC and has been requested by customers.
> 
> Hrm.... it's so unlucky that your patchset comes a little late. As your patch 8 and 9 said, you need them to fix problem for multi-card support, so definitely there was no such use case, and even the code was buggy to support it right? That makes the code hard to read and maintain, so we decide to remove it.

Hmm..
Well, if i missed your reply for my removing patch, it's my fault..but i didn't see any reply..
At that time, we didn't see any usage and also now...

Are there any patches for using multi slot? 
e,g) device-tree file or your own driver

If there are big benefits to revert them,  i don't want to back them..during almost 6 years, never use it..

> 
>>
>> The second part (patches 8-9) fixes the DesignWare multi-card support 
>> according to the dw-mmc databook (synnopsys: DesignWare Cores Mobile 
>> Storage Host Databook, 2.70a). It has changes to set the card number 
>> into the CMD register to multiplex requests to different cards when 
>> working in SD_MMC_CEATA mode, set the CTYPE / CLKENA / CDTHRCTL 
>> registers properly according to the spec, and parse the per-card 
>> configuration to match the Linux Documentation 
>> (bindings/mmc/synopsys-dw-mshc.txt).

the second part seems that it's only support since v2.70a..?

> 
> Havn'e check the databook for details yet, but I think it's ok to re-introduce multi-slot support if a real user benefits from it. But you need a new patch to silent the log "num-slots property not found, assuming 1 slot is available" as we removed all the num-slots from DT at that time.
> 
> 
>>
>> Liming Sun (9):
>>    Revert "Documentation: dw-mshc: deprecate num-slots"
>>    Revert "mmc: dw_mmc: remove the unnecessary slot variable"
>>    Revert "mmc: dw_mmc: use the 'slot' instead of 'cur_slot'"
>>    Revert "mmc: dw_mmc: remove the 'id' arguments about functions
>>      relevant to slot"
>>    Revert "mmc: dw_mmc: change the array of slots"
>>    Revert "mmc: dw_mmc: remove the loop about finding slots"
>>    Revert "mmc: dw_mmc: deprecated the "num-slots" property"
>>    mmc: dw_mmc: Support two SD_MMC_CE-ATA cards
>>    mmc: dw_mmc: Parse slot-specific configuration
>>
>>   .../devicetree/bindings/mmc/synopsys-dw-mshc.txt   |  16 +-
>>   drivers/mmc/host/dw_mmc-exynos.c                   |   4 +-
>>   drivers/mmc/host/dw_mmc.c                          | 277 ++++++++++++++++-----
>>   drivers/mmc/host/dw_mmc.h                          |  17 +-
>>   4 files changed, 236 insertions(+), 78 deletions(-)
>>
> 
> 
> 
> 

RE: [PATCH 0/9] Enable dw-mmc multi-card support

[Liming Sun]

Thanks Jaehoon. Please also see my response inline.

- Liming

> -----Original Message-----
> From: Jaehoon Chung [mailto:jh80.chung@samsung.com]
> Sent: Friday, October 20, 2017 10:06 AM
> To: Liming Sun <lsun@mellanox.com>; Shawn Lin <shawn.lin@rock-
> chips.com>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>; Rob Herring
> <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Kukjin Kim
> <kgene@kernel.org>; Krzysztof Kozlowski <krzk@kernel.org>; linux-
> mmc@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> samsung-soc@vger.kernel.org; Chris Metcalf <cmetcalf@mellanox.com>
> Subject: Re: [PATCH 0/9] Enable dw-mmc multi-card support
> 
> Sorry for late this..
> 
> On 10/18/2017 12:52 AM, Liming Sun wrote:
> >>> Hrm.... it's so unlucky that your patchset comes a little late. As your patch
> 8 and 9 said, you need them to fix problem for multi-card support, so
> definitely there was no such use case, and even the code was buggy to
> support it right? That makes the code hard to read and maintain, so we
> decide to remove it.
> >
> > Thanks for the feedback. Yes, earlier the multi-card support was buggy
> indeed. We spent some time to debug it and got it working.
> >
> >>> Havn'e check the databook for details yet, but I think it's ok to re-
> introduce multi-slot support if a real user benefits from it. But you need a
> new patch to silent the log "num-slots property not found, assuming 1 slot is
> available" as we removed all the num-slots from DT at that time.
> >
> > The " num-slots property not found..." log message has already been
> removed by 8a629d26f back in 2016. Looks like we're good on this one. In
> dw_mci_probe (), it has code to check pdata->num_slots. If 0, the host-
> >num_slots will be set to 1. So the logic of setting default num_slots seems
> already there. But correct me if I am wrong.
> >
> > Thanks,
> > Liming
> >
> > -----Original Message-----
> > From: Shawn Lin [mailto:shawn.lin@rock-chips.com]
> > Sent: Monday, October 16, 2017 9:36 PM
> > To: Liming Sun <lsun@mellanox.com>; Jaehoon Chung
> > <jh80.chung@samsung.com>
> > Cc: Ulf Hansson <ulf.hansson@linaro.org>; Rob Herring
> > <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Kukjin
> Kim
> > <kgene@kernel.org>; Krzysztof Kozlowski <krzk@kernel.org>;
> > shawn.lin@rock-chips.com; linux-mmc@vger.kernel.org;
> > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> > linux-arm-kernel@lists.infradead.org;
> > linux-samsung-soc@vger.kernel.org
> > Subject: Re: [PATCH 0/9] Enable dw-mmc multi-card support
> >
> >
> > On 2017/10/7 3:21, Liming Sun wrote:
> >> This series of commits enables the multi-card support for the dw-mmc
> >> controller. It includes two parts as below.
> >>
> >> The first part (patches 1-7) reverts the series of recent commits
> >> that removed the multi-card support with comments saying there was no
> >> such use case in the real world. Actually this feature is being used
> >> in Mellanox Bluefield SoC and has been requested by customers.
> >
> > Hrm.... it's so unlucky that your patchset comes a little late. As your patch 8
> and 9 said, you need them to fix problem for multi-card support, so definitely
> there was no such use case, and even the code was buggy to support it right?
> That makes the code hard to read and maintain, so we decide to remove it.
> 
> Hmm..
> Well, if i missed your reply for my removing patch, it's my fault..but i didn't
> see any reply..
> At that time, we didn't see any usage and also now...

[Liming] It is definitely not anybody's fault. My apology that I should have made it very clear.
This is a new SoC which uses two eMMC cards. This feature has been requested by customer.
We made it working with patches, then realized that the support for the 2nd card was removed recently from upstream kernel.
To avoid reinventing the wheel, a better approach appears to bring back the changes.

> 
> Are there any patches for using multi slot?
> e,g) device-tree file or your own driver

[Liming] Patch 9/8 and 9/9 are the changes to fix the multi-slot support.
We follow the original "Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt" for device configuration.
There is no device-tree patches since there is no changes to any of the documentation syntax.

Just in case you might be interested, below is the ACPI table we used to include the two slots. It's defined according to synopsys-dw-mshc.txt and seems working.

    Device (MMC0) {
      Name (_HID, "PRP0001")
      Name (_UID, Zero)
      Name (_CCA, 1)
      Name (_CRS, ResourceTemplate() {
        Memory32Fixed(ReadWrite, 0x6008000, 0x400)
        Interrupt(ResourceConsumer, Level, ActiveHigh, Exclusive) { 64 }
      })

      // Common configuration
      Name(_DSD, Package() {
        ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
        Package() {
          Package () { "compatible", Package () {"snps,dw-mshc"}},
          Package () { "fifo-depth", 0x100 },
          Package () { "clock-frequency", 24000000 },
          Package () { "cap-mmc-highspeed", 1 },
          Package () { "card-detect-delay", 200 },
          Package () { "num-slots", 2 }
        }
      })

      // Slot-0
      Device (SLT0) {
        Name(_DSD, Package() {
          ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
          Package() {
            Package () { "reg", 0 },              // physical slot number
            Package () { "bus-width", 8 }         // bus width (8-bit)
          }
        })
      }

      // Slot-1
      Device (SLT1) {
        Name(_DSD, Package() {
          ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
          Package() {
            Package () { "reg", 1 },              // physical slot number
            Package () { "bus-width", 4 }         // bus width (4-bit)
          }
        })
      }
    }
> 
> If there are big benefits to revert them,  i don't want to back them..during
> almost 6 years, never use it..

[Liming] Yeah, I could imagine it. Earlier the multi-slot didn't work well at all.
We debug it on HW emulation to figure it out. The changes are not big though. 
We have customers asking for it. It'll be very helpful to re-enable the multi-card support.

> 
> >
> >>
> >> The second part (patches 8-9) fixes the DesignWare multi-card support
> >> according to the dw-mmc databook (synnopsys: DesignWare Cores
> Mobile
> >> Storage Host Databook, 2.70a). It has changes to set the card number
> >> into the CMD register to multiplex requests to different cards when
> >> working in SD_MMC_CEATA mode, set the CTYPE / CLKENA / CDTHRCTL
> >> registers properly according to the spec, and parse the per-card
> >> configuration to match the Linux Documentation
> >> (bindings/mmc/synopsys-dw-mshc.txt).
> 
> the second part seems that it's only support since v2.70a..?

[Liming] This one is the spec I used as reference for all the changes and testing.
I didn't go through previous versions due to the limitation of testing environment I have.
As you mentioned, there is no multi-card usage earlier and this SoC is probably the first
product to use it in real-world. So adding a version check seems safe (so it won't
unnecessarily break anything just in case) . Any suggestions?

> 
> >
> > Havn'e check the databook for details yet, but I think it's ok to re-introduce
> multi-slot support if a real user benefits from it. But you need a new patch to
> silent the log "num-slots property not found, assuming 1 slot is available" as
> we removed all the num-slots from DT at that time.
> >
> >
> >>
> >> Liming Sun (9):
> >>    Revert "Documentation: dw-mshc: deprecate num-slots"
> >>    Revert "mmc: dw_mmc: remove the unnecessary slot variable"
> >>    Revert "mmc: dw_mmc: use the 'slot' instead of 'cur_slot'"
> >>    Revert "mmc: dw_mmc: remove the 'id' arguments about functions
> >>      relevant to slot"
> >>    Revert "mmc: dw_mmc: change the array of slots"
> >>    Revert "mmc: dw_mmc: remove the loop about finding slots"
> >>    Revert "mmc: dw_mmc: deprecated the "num-slots" property"
> >>    mmc: dw_mmc: Support two SD_MMC_CE-ATA cards
> >>    mmc: dw_mmc: Parse slot-specific configuration
> >>
> >>   .../devicetree/bindings/mmc/synopsys-dw-mshc.txt   |  16 +-
> >>   drivers/mmc/host/dw_mmc-exynos.c                   |   4 +-
> >>   drivers/mmc/host/dw_mmc.c                          | 277 ++++++++++++++++----
> -
> >>   drivers/mmc/host/dw_mmc.h                          |  17 +-
> >>   4 files changed, 236 insertions(+), 78 deletions(-)
> >>
> >
> >
> >
> >

[PATCH 0/9] Enable dw-mmc multi-card support

[Liming Sun]

Thanks Jaehoon. Please also see my response inline.

- Liming

> -----Original Message-----
> From: Jaehoon Chung [mailto:jh80.chung at samsung.com]
> Sent: Friday, October 20, 2017 10:06 AM
> To: Liming Sun <lsun@mellanox.com>; Shawn Lin <shawn.lin@rock-
> chips.com>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>; Rob Herring
> <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Kukjin Kim
> <kgene@kernel.org>; Krzysztof Kozlowski <krzk@kernel.org>; linux-
> mmc at vger.kernel.org; devicetree at vger.kernel.org; linux-
> kernel at vger.kernel.org; linux-arm-kernel at lists.infradead.org; linux-
> samsung-soc at vger.kernel.org; Chris Metcalf <cmetcalf@mellanox.com>
> Subject: Re: [PATCH 0/9] Enable dw-mmc multi-card support
> 
> Sorry for late this..
> 
> On 10/18/2017 12:52 AM, Liming Sun wrote:
> >>> Hrm.... it's so unlucky that your patchset comes a little late. As your patch
> 8 and 9 said, you need them to fix problem for multi-card support, so
> definitely there was no such use case, and even the code was buggy to
> support it right? That makes the code hard to read and maintain, so we
> decide to remove it.
> >
> > Thanks for the feedback. Yes, earlier the multi-card support was buggy
> indeed. We spent some time to debug it and got it working.
> >
> >>> Havn'e check the databook for details yet, but I think it's ok to re-
> introduce multi-slot support if a real user benefits from it. But you need a
> new patch to silent the log "num-slots property not found, assuming 1 slot is
> available" as we removed all the num-slots from DT at that time.
> >
> > The " num-slots property not found..." log message has already been
> removed by 8a629d26f back in 2016. Looks like we're good on this one. In
> dw_mci_probe (), it has code to check pdata->num_slots. If 0, the host-
> >num_slots will be set to 1. So the logic of setting default num_slots seems
> already there. But correct me if I am wrong.
> >
> > Thanks,
> > Liming
> >
> > -----Original Message-----
> > From: Shawn Lin [mailto:shawn.lin at rock-chips.com]
> > Sent: Monday, October 16, 2017 9:36 PM
> > To: Liming Sun <lsun@mellanox.com>; Jaehoon Chung
> > <jh80.chung@samsung.com>
> > Cc: Ulf Hansson <ulf.hansson@linaro.org>; Rob Herring
> > <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Kukjin
> Kim
> > <kgene@kernel.org>; Krzysztof Kozlowski <krzk@kernel.org>;
> > shawn.lin at rock-chips.com; linux-mmc at vger.kernel.org;
> > devicetree at vger.kernel.org; linux-kernel at vger.kernel.org;
> > linux-arm-kernel at lists.infradead.org;
> > linux-samsung-soc at vger.kernel.org
> > Subject: Re: [PATCH 0/9] Enable dw-mmc multi-card support
> >
> >
> > On 2017/10/7 3:21, Liming Sun wrote:
> >> This series of commits enables the multi-card support for the dw-mmc
> >> controller. It includes two parts as below.
> >>
> >> The first part (patches 1-7) reverts the series of recent commits
> >> that removed the multi-card support with comments saying there was no
> >> such use case in the real world. Actually this feature is being used
> >> in Mellanox Bluefield SoC and has been requested by customers.
> >
> > Hrm.... it's so unlucky that your patchset comes a little late. As your patch 8
> and 9 said, you need them to fix problem for multi-card support, so definitely
> there was no such use case, and even the code was buggy to support it right?
> That makes the code hard to read and maintain, so we decide to remove it.
> 
> Hmm..
> Well, if i missed your reply for my removing patch, it's my fault..but i didn't
> see any reply..
> At that time, we didn't see any usage and also now...

[Liming] It is definitely not anybody's fault. My apology that I should have made it very clear.
This is a new SoC which uses two eMMC cards. This feature has been requested by customer.
We made it working with patches, then realized that the support for the 2nd card was removed recently from upstream kernel.
To avoid reinventing the wheel, a better approach appears to bring back the changes.

> 
> Are there any patches for using multi slot?
> e,g) device-tree file or your own driver

[Liming] Patch 9/8 and 9/9 are the changes to fix the multi-slot support.
We follow the original "Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt" for device configuration.
There is no device-tree patches since there is no changes to any of the documentation syntax.

Just in case you might be interested, below is the ACPI table we used to include the two slots. It's defined according to synopsys-dw-mshc.txt and seems working.

    Device (MMC0) {
      Name (_HID, "PRP0001")
      Name (_UID, Zero)
      Name (_CCA, 1)
      Name (_CRS, ResourceTemplate() {
        Memory32Fixed(ReadWrite, 0x6008000, 0x400)
        Interrupt(ResourceConsumer, Level, ActiveHigh, Exclusive) { 64 }
      })

      // Common configuration
      Name(_DSD, Package() {
        ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
        Package() {
          Package () { "compatible", Package () {"snps,dw-mshc"}},
          Package () { "fifo-depth", 0x100 },
          Package () { "clock-frequency", 24000000 },
          Package () { "cap-mmc-highspeed", 1 },
          Package () { "card-detect-delay", 200 },
          Package () { "num-slots", 2 }
        }
      })

      // Slot-0
      Device (SLT0) {
        Name(_DSD, Package() {
          ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
          Package() {
            Package () { "reg", 0 },              // physical slot number
            Package () { "bus-width", 8 }         // bus width (8-bit)
          }
        })
      }

      // Slot-1
      Device (SLT1) {
        Name(_DSD, Package() {
          ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
          Package() {
            Package () { "reg", 1 },              // physical slot number
            Package () { "bus-width", 4 }         // bus width (4-bit)
          }
        })
      }
    }
> 
> If there are big benefits to revert them,  i don't want to back them..during
> almost 6 years, never use it..

[Liming] Yeah, I could imagine it. Earlier the multi-slot didn't work well at all.
We debug it on HW emulation to figure it out. The changes are not big though. 
We have customers asking for it. It'll be very helpful to re-enable the multi-card support.

> 
> >
> >>
> >> The second part (patches 8-9) fixes the DesignWare multi-card support
> >> according to the dw-mmc databook (synnopsys: DesignWare Cores
> Mobile
> >> Storage Host Databook, 2.70a). It has changes to set the card number
> >> into the CMD register to multiplex requests to different cards when
> >> working in SD_MMC_CEATA mode, set the CTYPE / CLKENA / CDTHRCTL
> >> registers properly according to the spec, and parse the per-card
> >> configuration to match the Linux Documentation
> >> (bindings/mmc/synopsys-dw-mshc.txt).
> 
> the second part seems that it's only support since v2.70a..?

[Liming] This one is the spec I used as reference for all the changes and testing.
I didn't go through previous versions due to the limitation of testing environment I have.
As you mentioned, there is no multi-card usage earlier and this SoC is probably the first
product to use it in real-world. So adding a version check seems safe (so it won't
unnecessarily break anything just in case) . Any suggestions?

> 
> >
> > Havn'e check the databook for details yet, but I think it's ok to re-introduce
> multi-slot support if a real user benefits from it. But you need a new patch to
> silent the log "num-slots property not found, assuming 1 slot is available" as
> we removed all the num-slots from DT at that time.
> >
> >
> >>
> >> Liming Sun (9):
> >>    Revert "Documentation: dw-mshc: deprecate num-slots"
> >>    Revert "mmc: dw_mmc: remove the unnecessary slot variable"
> >>    Revert "mmc: dw_mmc: use the 'slot' instead of 'cur_slot'"
> >>    Revert "mmc: dw_mmc: remove the 'id' arguments about functions
> >>      relevant to slot"
> >>    Revert "mmc: dw_mmc: change the array of slots"
> >>    Revert "mmc: dw_mmc: remove the loop about finding slots"
> >>    Revert "mmc: dw_mmc: deprecated the "num-slots" property"
> >>    mmc: dw_mmc: Support two SD_MMC_CE-ATA cards
> >>    mmc: dw_mmc: Parse slot-specific configuration
> >>
> >>   .../devicetree/bindings/mmc/synopsys-dw-mshc.txt   |  16 +-
> >>   drivers/mmc/host/dw_mmc-exynos.c                   |   4 +-
> >>   drivers/mmc/host/dw_mmc.c                          | 277 ++++++++++++++++----
> -
> >>   drivers/mmc/host/dw_mmc.h                          |  17 +-
> >>   4 files changed, 236 insertions(+), 78 deletions(-)
> >>
> >
> >
> >
> >

RE: [PATCH 0/9] Enable dw-mmc multi-card support

[Liming Sun]

Thanks Jaehoon. Please also see my response inline.

- Liming

> -----Original Message-----
> From: Jaehoon Chung [mailto:jh80.chung@samsung.com]
> Sent: Friday, October 20, 2017 10:06 AM
> To: Liming Sun <lsun@mellanox.com>; Shawn Lin <shawn.lin@rock-
> chips.com>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>; Rob Herring
> <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Kukjin Kim
> <kgene@kernel.org>; Krzysztof Kozlowski <krzk@kernel.org>; linux-
> mmc@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> samsung-soc@vger.kernel.org; Chris Metcalf <cmetcalf@mellanox.com>
> Subject: Re: [PATCH 0/9] Enable dw-mmc multi-card support
> 
> Sorry for late this..
> 
> On 10/18/2017 12:52 AM, Liming Sun wrote:
> >>> Hrm.... it's so unlucky that your patchset comes a little late. As your patch
> 8 and 9 said, you need them to fix problem for multi-card support, so
> definitely there was no such use case, and even the code was buggy to
> support it right? That makes the code hard to read and maintain, so we
> decide to remove it.
> >
> > Thanks for the feedback. Yes, earlier the multi-card support was buggy
> indeed. We spent some time to debug it and got it working.
> >
> >>> Havn'e check the databook for details yet, but I think it's ok to re-
> introduce multi-slot support if a real user benefits from it. But you need a
> new patch to silent the log "num-slots property not found, assuming 1 slot is
> available" as we removed all the num-slots from DT at that time.
> >
> > The " num-slots property not found..." log message has already been
> removed by 8a629d26f back in 2016. Looks like we're good on this one. In
> dw_mci_probe (), it has code to check pdata->num_slots. If 0, the host-
> >num_slots will be set to 1. So the logic of setting default num_slots seems
> already there. But correct me if I am wrong.
> >
> > Thanks,
> > Liming
> >
> > -----Original Message-----
> > From: Shawn Lin [mailto:shawn.lin@rock-chips.com]
> > Sent: Monday, October 16, 2017 9:36 PM
> > To: Liming Sun <lsun@mellanox.com>; Jaehoon Chung
> > <jh80.chung@samsung.com>
> > Cc: Ulf Hansson <ulf.hansson@linaro.org>; Rob Herring
> > <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Kukjin
> Kim
> > <kgene@kernel.org>; Krzysztof Kozlowski <krzk@kernel.org>;
> > shawn.lin@rock-chips.com; linux-mmc@vger.kernel.org;
> > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> > linux-arm-kernel@lists.infradead.org;
> > linux-samsung-soc@vger.kernel.org
> > Subject: Re: [PATCH 0/9] Enable dw-mmc multi-card support
> >
> >
> > On 2017/10/7 3:21, Liming Sun wrote:
> >> This series of commits enables the multi-card support for the dw-mmc
> >> controller. It includes two parts as below.
> >>
> >> The first part (patches 1-7) reverts the series of recent commits
> >> that removed the multi-card support with comments saying there was no
> >> such use case in the real world. Actually this feature is being used
> >> in Mellanox Bluefield SoC and has been requested by customers.
> >
> > Hrm.... it's so unlucky that your patchset comes a little late. As your patch 8
> and 9 said, you need them to fix problem for multi-card support, so definitely
> there was no such use case, and even the code was buggy to support it right?
> That makes the code hard to read and maintain, so we decide to remove it.
> 
> Hmm..
> Well, if i missed your reply for my removing patch, it's my fault..but i didn't
> see any reply..
> At that time, we didn't see any usage and also now...

[Liming] It is definitely not anybody's fault. My apology that I should have made it very clear.
This is a new SoC which uses two eMMC cards. This feature has been requested by customer.
We made it working with patches, then realized that the support for the 2nd card was removed recently from upstream kernel.
To avoid reinventing the wheel, a better approach appears to bring back the changes.

> 
> Are there any patches for using multi slot?
> e,g) device-tree file or your own driver

[Liming] Patch 9/8 and 9/9 are the changes to fix the multi-slot support.
We follow the original "Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt" for device configuration.
There is no device-tree patches since there is no changes to any of the documentation syntax.

Just in case you might be interested, below is the ACPI table we used to include the two slots. It's defined according to synopsys-dw-mshc.txt and seems working.

    Device (MMC0) {
      Name (_HID, "PRP0001")
      Name (_UID, Zero)
      Name (_CCA, 1)
      Name (_CRS, ResourceTemplate() {
        Memory32Fixed(ReadWrite, 0x6008000, 0x400)
        Interrupt(ResourceConsumer, Level, ActiveHigh, Exclusive) { 64 }
      })

      // Common configuration
      Name(_DSD, Package() {
        ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
        Package() {
          Package () { "compatible", Package () {"snps,dw-mshc"}},
          Package () { "fifo-depth", 0x100 },
          Package () { "clock-frequency", 24000000 },
          Package () { "cap-mmc-highspeed", 1 },
          Package () { "card-detect-delay", 200 },
          Package () { "num-slots", 2 }
        }
      })

      // Slot-0
      Device (SLT0) {
        Name(_DSD, Package() {
          ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
          Package() {
            Package () { "reg", 0 },              // physical slot number
            Package () { "bus-width", 8 }         // bus width (8-bit)
          }
        })
      }

      // Slot-1
      Device (SLT1) {
        Name(_DSD, Package() {
          ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
          Package() {
            Package () { "reg", 1 },              // physical slot number
            Package () { "bus-width", 4 }         // bus width (4-bit)
          }
        })
      }
    }
> 
> If there are big benefits to revert them,  i don't want to back them..during
> almost 6 years, never use it..

[Liming] Yeah, I could imagine it. Earlier the multi-slot didn't work well at all.
We debug it on HW emulation to figure it out. The changes are not big though. 
We have customers asking for it. It'll be very helpful to re-enable the multi-card support.

> 
> >
> >>
> >> The second part (patches 8-9) fixes the DesignWare multi-card support
> >> according to the dw-mmc databook (synnopsys: DesignWare Cores
> Mobile
> >> Storage Host Databook, 2.70a). It has changes to set the card number
> >> into the CMD register to multiplex requests to different cards when
> >> working in SD_MMC_CEATA mode, set the CTYPE / CLKENA / CDTHRCTL
> >> registers properly according to the spec, and parse the per-card
> >> configuration to match the Linux Documentation
> >> (bindings/mmc/synopsys-dw-mshc.txt).
> 
> the second part seems that it's only support since v2.70a..?

[Liming] This one is the spec I used as reference for all the changes and testing.
I didn't go through previous versions due to the limitation of testing environment I have.
As you mentioned, there is no multi-card usage earlier and this SoC is probably the first
product to use it in real-world. So adding a version check seems safe (so it won't
unnecessarily break anything just in case) . Any suggestions?

> 
> >
> > Havn'e check the databook for details yet, but I think it's ok to re-introduce
> multi-slot support if a real user benefits from it. But you need a new patch to
> silent the log "num-slots property not found, assuming 1 slot is available" as
> we removed all the num-slots from DT at that time.
> >
> >
> >>
> >> Liming Sun (9):
> >>    Revert "Documentation: dw-mshc: deprecate num-slots"
> >>    Revert "mmc: dw_mmc: remove the unnecessary slot variable"
> >>    Revert "mmc: dw_mmc: use the 'slot' instead of 'cur_slot'"
> >>    Revert "mmc: dw_mmc: remove the 'id' arguments about functions
> >>      relevant to slot"
> >>    Revert "mmc: dw_mmc: change the array of slots"
> >>    Revert "mmc: dw_mmc: remove the loop about finding slots"
> >>    Revert "mmc: dw_mmc: deprecated the "num-slots" property"
> >>    mmc: dw_mmc: Support two SD_MMC_CE-ATA cards
> >>    mmc: dw_mmc: Parse slot-specific configuration
> >>
> >>   .../devicetree/bindings/mmc/synopsys-dw-mshc.txt   |  16 +-
> >>   drivers/mmc/host/dw_mmc-exynos.c                   |   4 +-
> >>   drivers/mmc/host/dw_mmc.c                          | 277 ++++++++++++++++----
> -
> >>   drivers/mmc/host/dw_mmc.h                          |  17 +-
> >>   4 files changed, 236 insertions(+), 78 deletions(-)
> >>
> >
> >
> >
> >

Re: [PATCH 0/9] Enable dw-mmc multi-card support

[Ulf Hansson]

Hi Liming,

Sorry for jumping into the discussion like this.

[...]

>> >> This series of commits enables the multi-card support for the dw-mmc
>> >> controller. It includes two parts as below.
>> >>
>> >> The first part (patches 1-7) reverts the series of recent commits
>> >> that removed the multi-card support with comments saying there was no
>> >> such use case in the real world. Actually this feature is being used
>> >> in Mellanox Bluefield SoC and has been requested by customers.
>> >
>> > Hrm.... it's so unlucky that your patchset comes a little late. As your patch 8
>> and 9 said, you need them to fix problem for multi-card support, so definitely
>> there was no such use case, and even the code was buggy to support it right?
>> That makes the code hard to read and maintain, so we decide to remove it.

Unfortunate patch 8 and 9 does *not* solve the fundamental problem
with multiple slots. You can't solve it without involving the mmc
core, as explained in earlier discussions [1].

>>
>> Hmm..
>> Well, if i missed your reply for my removing patch, it's my fault..but i didn't
>> see any reply..
>> At that time, we didn't see any usage and also now...
>
> [Liming] It is definitely not anybody's fault. My apology that I should have made it very clear.
> This is a new SoC which uses two eMMC cards. This feature has been requested by customer.
> We made it working with patches, then realized that the support for the 2nd card was removed recently from upstream kernel.
> To avoid reinventing the wheel, a better approach appears to bring back the changes.
>
>>

Having multiple slots of eMMC:s, attached to one single mmc
controller, is really a terrible idea, even if one could get it to
somewhat work.

To deal with multiple slots from the mmc core point of view, we would
need to introduce  a kind of "bus slot lock", which needs to be taken
whenever the core access any of the registered two mmc hosts.

However, that then means that the upper block device layers can no
longer perform proper I/O scheduling of requests to the eMMC cards. In
other words, requests on one eMMC device can then starve requests
reaching the other eMMC device. It all falls apart.

Perhaps that is a limitation you think is okay to live with, but I am
really not so sure it's worth adding "multiple slot" support.

[...]

Kind regards
Uffe

[PATCH 0/9] Enable dw-mmc multi-card support

[Ulf Hansson]

Hi Liming,

Sorry for jumping into the discussion like this.

[...]

>> >> This series of commits enables the multi-card support for the dw-mmc
>> >> controller. It includes two parts as below.
>> >>
>> >> The first part (patches 1-7) reverts the series of recent commits
>> >> that removed the multi-card support with comments saying there was no
>> >> such use case in the real world. Actually this feature is being used
>> >> in Mellanox Bluefield SoC and has been requested by customers.
>> >
>> > Hrm.... it's so unlucky that your patchset comes a little late. As your patch 8
>> and 9 said, you need them to fix problem for multi-card support, so definitely
>> there was no such use case, and even the code was buggy to support it right?
>> That makes the code hard to read and maintain, so we decide to remove it.

Unfortunate patch 8 and 9 does *not* solve the fundamental problem
with multiple slots. You can't solve it without involving the mmc
core, as explained in earlier discussions [1].

>>
>> Hmm..
>> Well, if i missed your reply for my removing patch, it's my fault..but i didn't
>> see any reply..
>> At that time, we didn't see any usage and also now...
>
> [Liming] It is definitely not anybody's fault. My apology that I should have made it very clear.
> This is a new SoC which uses two eMMC cards. This feature has been requested by customer.
> We made it working with patches, then realized that the support for the 2nd card was removed recently from upstream kernel.
> To avoid reinventing the wheel, a better approach appears to bring back the changes.
>
>>

Having multiple slots of eMMC:s, attached to one single mmc
controller, is really a terrible idea, even if one could get it to
somewhat work.

To deal with multiple slots from the mmc core point of view, we would
need to introduce  a kind of "bus slot lock", which needs to be taken
whenever the core access any of the registered two mmc hosts.

However, that then means that the upper block device layers can no
longer perform proper I/O scheduling of requests to the eMMC cards. In
other words, requests on one eMMC device can then starve requests
reaching the other eMMC device. It all falls apart.

Perhaps that is a limitation you think is okay to live with, but I am
really not so sure it's worth adding "multiple slot" support.

[...]

Kind regards
Uffe

Re: [PATCH 0/9] Enable dw-mmc multi-card support

[Ulf Hansson]

Hi Liming,

Sorry for jumping into the discussion like this.

[...]

>> >> This series of commits enables the multi-card support for the dw-mmc
>> >> controller. It includes two parts as below.
>> >>
>> >> The first part (patches 1-7) reverts the series of recent commits
>> >> that removed the multi-card support with comments saying there was no
>> >> such use case in the real world. Actually this feature is being used
>> >> in Mellanox Bluefield SoC and has been requested by customers.
>> >
>> > Hrm.... it's so unlucky that your patchset comes a little late. As your patch 8
>> and 9 said, you need them to fix problem for multi-card support, so definitely
>> there was no such use case, and even the code was buggy to support it right?
>> That makes the code hard to read and maintain, so we decide to remove it.

Unfortunate patch 8 and 9 does *not* solve the fundamental problem
with multiple slots. You can't solve it without involving the mmc
core, as explained in earlier discussions [1].

>>
>> Hmm..
>> Well, if i missed your reply for my removing patch, it's my fault..but i didn't
>> see any reply..
>> At that time, we didn't see any usage and also now...
>
> [Liming] It is definitely not anybody's fault. My apology that I should have made it very clear.
> This is a new SoC which uses two eMMC cards. This feature has been requested by customer.
> We made it working with patches, then realized that the support for the 2nd card was removed recently from upstream kernel.
> To avoid reinventing the wheel, a better approach appears to bring back the changes.
>
>>

Having multiple slots of eMMC:s, attached to one single mmc
controller, is really a terrible idea, even if one could get it to
somewhat work.

To deal with multiple slots from the mmc core point of view, we would
need to introduce  a kind of "bus slot lock", which needs to be taken
whenever the core access any of the registered two mmc hosts.

However, that then means that the upper block device layers can no
longer perform proper I/O scheduling of requests to the eMMC cards. In
other words, requests on one eMMC device can then starve requests
reaching the other eMMC device. It all falls apart.

Perhaps that is a limitation you think is okay to live with, but I am
really not so sure it's worth adding "multiple slot" support.

[...]

Kind regards
Uffe

Re: [PATCH 0/9] Enable dw-mmc multi-card support

[Ulf Hansson]

[...]

> Unfortunate patch 8 and 9 does *not* solve the fundamental problem
> with multiple slots. You can't solve it without involving the mmc
> core, as explained in earlier discussions [1].
>

This link got lost, so here it is:

[1]
https://www.spinics.net/lists/linux-mmc/msg46549.html

[...]

Kind regards
Uffe

[PATCH 0/9] Enable dw-mmc multi-card support

[Ulf Hansson]

[...]

> Unfortunate patch 8 and 9 does *not* solve the fundamental problem
> with multiple slots. You can't solve it without involving the mmc
> core, as explained in earlier discussions [1].
>

This link got lost, so here it is:

[1]
https://www.spinics.net/lists/linux-mmc/msg46549.html

[...]

Kind regards
Uffe

Re: [PATCH 0/9] Enable dw-mmc multi-card support

[Ulf Hansson]

[...]

> Unfortunate patch 8 and 9 does *not* solve the fundamental problem
> with multiple slots. You can't solve it without involving the mmc
> core, as explained in earlier discussions [1].
>

This link got lost, so here it is:

[1]
https://www.spinics.net/lists/linux-mmc/msg46549.html

[...]

Kind regards
Uffe