[PATCH RFC 00/12] Add Qualcomm SD Card Controller support.

[PATCH RFC 00/12] Add Qualcomm SD Card Controller support.

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

This patch series adds Qualcomm SD Card Controller support in pl180 mmci
driver. QCom SDCC is basically a pl180, but bit more customized, some of the
register layouts and offsets are different to the ones mentioned in pl180
datasheet. The plan is to totally remove the standalone SDCC driver
drivers/mmc/host/msm_sdcc.* and start using generic mmci driver for all
Qualcomm parts, as we get chance to test on other Qcom boards.

To start using the existing mmci driver, a fake amba id for Qualcomm is added
in patches:
 ARM: amba: Add Qualcomm vendor ID.
 mmc: mmci: Add Qualcomm Id to amba id table.

Second change is, adding a 3 clock cycle delay for register writes on QCOM SDCC
registers, which is done in patches:
  mmc: mmci: Add register read/write wrappers.
  mmc: mmci: Add write delay to variant structure.
  mmc: mmci: Qcomm: Add 3 clock cycle delay after each register write

Third change was to accommodate DATCTRL and MMCICLK register layout changes in
Qcom SDCC. Which is done in patches:
  mmc: mmci: Add Qcom datactrl register variant
  mmc: mmci: Add Qcom variations to MCICommand register.
  mmc: mmci: Qcom fix MCICLK register settings.
  mmc: mmci: Add clock support for Qualcomm.

Fourth major change was to add qcom specfic pio read function, the need for
this is because the way MCIFIFOCNT register behaved in QCOM SDCC is very
 different to the one in pl180. This change is done in patch:
  mmc: mmci: Add Qcom specific pio_read function.

Last some Qcom unrelated changes to support Qcom are done in patches:
  mmc: mmci: use NSEC_PER_SEC macro
  mmc: mmci: move ST specific register extensions access under condition.

This patches are tested in PIO mode on IFC8064 board with both eMMC and
external SD card. I would appreciate any feedback/suggestions on the overall
approach.

Thanks,
srini

Srinivas Kandagatla (12):
  ARM: amba: Add Qualcomm vendor ID.
  mmc: mmci: Add Qualcomm Id to amba id table
  mmc: mmci: Add Qcom datactrl register variant
  mmc: mmci: Add register read/write wrappers.
  mmc: mmci: use NSEC_PER_SEC macro
  mmc: mmci: Add write delay to variant structure.
  mmc: mmci: Qcomm: Add 3 clock cycle delay after each register write
  mmc: mmci: move ST specific register extensions access under condition.
  mmc: mmci: Qcom fix MCICLK register settings.
  mmc: mmci: Add clock support for Qualcomm.
  mmc: mmci: Add Qcom variations to MCICommand register.
  mmc: mmci: Add Qcom specific pio_read function.

 drivers/mmc/host/mmci.c  |  239 +++++++++++++++++++++++++++++++++-------------
 drivers/mmc/host/mmci.h  |   28 ++++++
 include/linux/amba/bus.h |    1 +
 3 files changed, 202 insertions(+), 66 deletions(-)

-- 
1.7.9.5

[PATCH RFC 01/12] ARM: amba: Add Qualcomm vendor ID.

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

This patch adds Qualcomm amba vendor Id to the list. This ID is used in mmci
driver.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 include/linux/amba/bus.h |    1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/amba/bus.h b/include/linux/amba/bus.h
index 63b5eff..fdd7e1b 100644
--- a/include/linux/amba/bus.h
+++ b/include/linux/amba/bus.h
@@ -47,6 +47,7 @@ struct amba_driver {
 enum amba_vendor {
 	AMBA_VENDOR_ARM = 0x41,
 	AMBA_VENDOR_ST = 0x80,
+	AMBA_VENDOR_QCOM = 0x51,
 };
 
 extern struct bus_type amba_bustype;
-- 
1.7.9.5

[PATCH RFC 02/12] mmc: mmci: Add Qualcomm Id to amba id table

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

This patch adds a fake Qualcomm ID 0x00051180 to the amba_ids, as Qualcomm
SDCC controller is pl180, but amba id registers read 0x0's.
The plan is to remove SDCC driver totally and use mmci as the main SD
controller driver for Qualcomm SOCs.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/mmc/host/mmci.c |   15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 771c60a..391e8d4 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -158,6 +158,15 @@ static struct variant_data variant_ux500v2 = {
 	.pwrreg_nopower		= true,
 };
 
+static struct variant_data variant_qcom = {
+	.fifosize		= 16 * 4,
+	.fifohalfsize		= 8 * 4,
+	.clkreg			= MCI_CLK_ENABLE,
+	.datalength_bits	= 24,
+	.blksz_datactrl4	= true,
+	.pwrreg_powerup		= MCI_PWR_UP,
+};
+
 static int mmci_card_busy(struct mmc_host *mmc)
 {
 	struct mmci_host *host = mmc_priv(mmc);
@@ -1908,6 +1917,12 @@ static struct amba_id mmci_ids[] = {
 		.mask   = 0xf0ffffff,
 		.data	= &variant_ux500v2,
 	},
+	/* Qualcomm variants */
+	{
+		.id     = 0x00051180,
+		.mask	= 0x000fffff,
+		.data	= &variant_qcom,
+	},
 	{ 0, 0 },
 };
 
-- 
1.7.9.5

[PATCH RFC 03/12] mmc: mmci: Add Qcom datactrl register variant

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Instance of this IP on Qualcomm's SOCs has bit different layout for datactrl
register. Bit postion datactrl[16:4] hold the true block size instead of power
of 2.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/mmc/host/mmci.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 391e8d4..19d6b6f 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -58,6 +58,8 @@ static unsigned int fmax = 515633;
  * @sdio: variant supports SDIO
  * @st_clkdiv: true if using a ST-specific clock divider algorithm
  * @blksz_datactrl16: true if Block size is at b16..b30 position in datactrl register
+ * @blksz_datactrl4: true if Block size is at b4..b16 position in datactrl
+ *		     register
  * @pwrreg_powerup: power up value for MMCIPOWER register
  * @signal_direction: input/out direction of bus signals can be indicated
  * @pwrreg_clkgate: MMCIPOWER register must be used to gate the clock
@@ -73,6 +75,7 @@ struct variant_data {
 	bool			sdio;
 	bool			st_clkdiv;
 	bool			blksz_datactrl16;
+	bool			blksz_datactrl4;
 	u32			pwrreg_powerup;
 	bool			signal_direction;
 	bool			pwrreg_clkgate;
@@ -162,6 +165,7 @@ static struct variant_data variant_qcom = {
 	.fifosize		= 16 * 4,
 	.fifohalfsize		= 8 * 4,
 	.clkreg			= MCI_CLK_ENABLE,
+	.blksz_datactrl4	= true,
 	.datalength_bits	= 24,
 	.blksz_datactrl4	= true,
 	.pwrreg_powerup		= MCI_PWR_UP,
@@ -760,6 +764,8 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 
 	if (variant->blksz_datactrl16)
 		datactrl = MCI_DPSM_ENABLE | (data->blksz << 16);
+	else if (variant->blksz_datactrl4)
+		datactrl = MCI_DPSM_ENABLE | (data->blksz << 4);
 	else
 		datactrl = MCI_DPSM_ENABLE | blksz_bits << 4;
 
-- 
1.7.9.5

[PATCH RFC 04/12] mmc: mmci: Add register read/write wrappers.

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

This patch adds wrappers for readl/writel functions used in the driver. The
reason for this wrappers is to accommodate SOCs like Qualcomm which has
requirement for delaying the write for few cycles when writing to its SD Card
Controller registers.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/mmc/host/mmci.c |  114 +++++++++++++++++++++++++----------------------
 1 file changed, 61 insertions(+), 53 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 19d6b6f..36db31e 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -171,6 +171,16 @@ static struct variant_data variant_qcom = {
 	.pwrreg_powerup		= MCI_PWR_UP,
 };
 
+static inline u32 mmci_readl(struct mmci_host *host, u32 off)
+{
+	return readl(host->base  + off);
+}
+
+static inline void mmci_writel(struct mmci_host *host, u32 data, u32 off)
+{
+	writel(data, host->base + off);
+}
+
 static int mmci_card_busy(struct mmc_host *mmc)
 {
 	struct mmci_host *host = mmc_priv(mmc);
@@ -180,7 +190,7 @@ static int mmci_card_busy(struct mmc_host *mmc)
 	pm_runtime_get_sync(mmc_dev(mmc));
 
 	spin_lock_irqsave(&host->lock, flags);
-	if (readl(host->base + MMCISTATUS) & MCI_ST_CARDBUSY)
+	if (mmci_readl(host, MMCISTATUS) & MCI_ST_CARDBUSY)
 		busy = 1;
 	spin_unlock_irqrestore(&host->lock, flags);
 
@@ -230,7 +240,7 @@ static void mmci_write_clkreg(struct mmci_host *host, u32 clk)
 {
 	if (host->clk_reg != clk) {
 		host->clk_reg = clk;
-		writel(clk, host->base + MMCICLOCK);
+		mmci_writel(host, clk, MMCICLOCK);
 	}
 }
 
@@ -241,7 +251,7 @@ static void mmci_write_pwrreg(struct mmci_host *host, u32 pwr)
 {
 	if (host->pwr_reg != pwr) {
 		host->pwr_reg = pwr;
-		writel(pwr, host->base + MMCIPOWER);
+		mmci_writel(host, pwr, MMCIPOWER);
 	}
 }
 
@@ -255,7 +265,7 @@ static void mmci_write_datactrlreg(struct mmci_host *host, u32 datactrl)
 
 	if (host->datactrl_reg != datactrl) {
 		host->datactrl_reg = datactrl;
-		writel(datactrl, host->base + MMCIDATACTRL);
+		mmci_writel(host, datactrl, MMCIDATACTRL);
 	}
 }
 
@@ -321,7 +331,7 @@ static void mmci_set_clkreg(struct mmci_host *host, unsigned int desired)
 static void
 mmci_request_end(struct mmci_host *host, struct mmc_request *mrq)
 {
-	writel(0, host->base + MMCICOMMAND);
+	mmci_writel(host, 0, MMCICOMMAND);
 
 	BUG_ON(host->data);
 
@@ -336,18 +346,16 @@ mmci_request_end(struct mmci_host *host, struct mmc_request *mrq)
 
 static void mmci_set_mask1(struct mmci_host *host, unsigned int mask)
 {
-	void __iomem *base = host->base;
-
 	if (host->singleirq) {
-		unsigned int mask0 = readl(base + MMCIMASK0);
+		unsigned int mask0 = mmci_readl(host, MMCIMASK0);
 
 		mask0 &= ~MCI_IRQ1MASK;
 		mask0 |= mask;
 
-		writel(mask0, base + MMCIMASK0);
+		mmci_writel(host, mask0, MMCIMASK0);
 	}
 
-	writel(mask, base + MMCIMASK1);
+	mmci_writel(host, mask, MMCIMASK1);
 }
 
 static void mmci_stop_data(struct mmci_host *host)
@@ -498,7 +506,7 @@ static void mmci_dma_finalize(struct mmci_host *host, struct mmc_data *data)
 
 	/* Wait up to 1ms for the DMA to complete */
 	for (i = 0; ; i++) {
-		status = readl(host->base + MMCISTATUS);
+		status = mmci_readl(host, MMCISTATUS);
 		if (!(status & MCI_RXDATAAVLBLMASK) || i >= 100)
 			break;
 		udelay(10);
@@ -637,8 +645,8 @@ static int mmci_dma_start_data(struct mmci_host *host, unsigned int datactrl)
 	 * to fire next DMA request. When that happens, MMCI will
 	 * call mmci_data_end()
 	 */
-	writel(readl(host->base + MMCIMASK0) | MCI_DATAENDMASK,
-	       host->base + MMCIMASK0);
+	mmci_writel(host, mmci_readl(host, MMCIMASK0) | MCI_DATAENDMASK,
+		    MMCIMASK0);
 	return 0;
 }
 
@@ -756,8 +764,8 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 	timeout = data->timeout_clks + (unsigned int)clks;
 
 	base = host->base;
-	writel(timeout, base + MMCIDATATIMER);
-	writel(host->size, base + MMCIDATALENGTH);
+	mmci_writel(host, timeout, MMCIDATATIMER);
+	mmci_writel(host, host->size, MMCIDATALENGTH);
 
 	blksz_bits = ffs(data->blksz) - 1;
 	BUG_ON(1 << blksz_bits != data->blksz);
@@ -831,20 +839,19 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 	}
 
 	mmci_write_datactrlreg(host, datactrl);
-	writel(readl(base + MMCIMASK0) & ~MCI_DATAENDMASK, base + MMCIMASK0);
+	mmci_writel(host, mmci_readl(host, MMCIMASK0) & ~MCI_DATAENDMASK,
+		    MMCIMASK0);
 	mmci_set_mask1(host, irqmask);
 }
 
 static void
 mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c)
 {
-	void __iomem *base = host->base;
-
 	dev_dbg(mmc_dev(host->mmc), "op %02x arg %08x flags %08x\n",
 	    cmd->opcode, cmd->arg, cmd->flags);
 
-	if (readl(base + MMCICOMMAND) & MCI_CPSM_ENABLE) {
-		writel(0, base + MMCICOMMAND);
+	if (mmci_readl(host, MMCICOMMAND) & MCI_CPSM_ENABLE) {
+		mmci_writel(host, 0, MMCICOMMAND);
 		udelay(1);
 	}
 
@@ -859,8 +866,8 @@ mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c)
 
 	host->cmd = cmd;
 
-	writel(cmd->arg, base + MMCIARGUMENT);
-	writel(c, base + MMCICOMMAND);
+	mmci_writel(host, cmd->arg, MMCIARGUMENT);
+	mmci_writel(host, c, MMCICOMMAND);
 }
 
 static void
@@ -885,7 +892,7 @@ mmci_data_irq(struct mmci_host *host, struct mmc_data *data,
 		 * can be as much as a FIFO-worth of data ahead.  This
 		 * matters for FIFO overruns only.
 		 */
-		remain = readl(host->base + MMCIDATACNT);
+		remain = mmci_readl(host, MMCIDATACNT);
 		success = data->blksz * data->blocks - remain;
 
 		dev_dbg(mmc_dev(host->mmc), "MCI ERROR IRQ, status 0x%08x at 0x%08x\n",
@@ -967,10 +974,10 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd,
 	} else if (status & MCI_CMDCRCFAIL && cmd->flags & MMC_RSP_CRC) {
 		cmd->error = -EILSEQ;
 	} else {
-		cmd->resp[0] = readl(base + MMCIRESPONSE0);
-		cmd->resp[1] = readl(base + MMCIRESPONSE1);
-		cmd->resp[2] = readl(base + MMCIRESPONSE2);
-		cmd->resp[3] = readl(base + MMCIRESPONSE3);
+		cmd->resp[0] = mmci_readl(host, MMCIRESPONSE0);
+		cmd->resp[1] = mmci_readl(host, MMCIRESPONSE1);
+		cmd->resp[2] = mmci_readl(host, MMCIRESPONSE2);
+		cmd->resp[3] = mmci_readl(host, MMCIRESPONSE3);
 	}
 
 	if ((!sbc && !cmd->data) || cmd->error) {
@@ -1081,11 +1088,10 @@ static irqreturn_t mmci_pio_irq(int irq, void *dev_id)
 	struct mmci_host *host = dev_id;
 	struct sg_mapping_iter *sg_miter = &host->sg_miter;
 	struct variant_data *variant = host->variant;
-	void __iomem *base = host->base;
 	unsigned long flags;
 	u32 status;
 
-	status = readl(base + MMCISTATUS);
+	status = mmci_readl(host, MMCISTATUS);
 
 	dev_dbg(mmc_dev(host->mmc), "irq1 (pio) %08x\n", status);
 
@@ -1125,7 +1131,7 @@ static irqreturn_t mmci_pio_irq(int irq, void *dev_id)
 		if (remain)
 			break;
 
-		status = readl(base + MMCISTATUS);
+		status = mmci_readl(host, MMCISTATUS);
 	} while (1);
 
 	sg_miter_stop(sg_miter);
@@ -1147,7 +1153,9 @@ static irqreturn_t mmci_pio_irq(int irq, void *dev_id)
 	 */
 	if (host->size == 0) {
 		mmci_set_mask1(host, 0);
-		writel(readl(base + MMCIMASK0) | MCI_DATAENDMASK, base + MMCIMASK0);
+		mmci_writel(host,
+			    mmci_readl(host, MMCIMASK0) | MCI_DATAENDMASK,
+			    MMCIMASK0);
 	}
 
 	return IRQ_HANDLED;
@@ -1168,10 +1176,10 @@ static irqreturn_t mmci_irq(int irq, void *dev_id)
 		struct mmc_command *cmd;
 		struct mmc_data *data;
 
-		status = readl(host->base + MMCISTATUS);
+		status = mmci_readl(host, MMCISTATUS);
 
 		if (host->singleirq) {
-			if (status & readl(host->base + MMCIMASK1))
+			if (status & mmci_readl(host, MMCIMASK1))
 				mmci_pio_irq(irq, dev_id);
 
 			status &= ~MCI_IRQ1MASK;
@@ -1182,8 +1190,8 @@ static irqreturn_t mmci_irq(int irq, void *dev_id)
 		 * enabled) since the HW seems to be triggering the IRQ on both
 		 * edges while monitoring DAT0 for busy completion.
 		 */
-		status &= readl(host->base + MMCIMASK0);
-		writel(status, host->base + MMCICLEAR);
+		status &= mmci_readl(host, MMCIMASK0);
+		mmci_writel(host, status, MMCICLEAR);
 
 		dev_dbg(mmc_dev(host->mmc), "irq0 (data+cmd) %08x\n", status);
 
@@ -1627,9 +1635,9 @@ static int mmci_probe(struct amba_device *dev,
 
 	spin_lock_init(&host->lock);
 
-	writel(0, host->base + MMCIMASK0);
-	writel(0, host->base + MMCIMASK1);
-	writel(0xfff, host->base + MMCICLEAR);
+	mmci_writel(host, 0, MMCIMASK0);
+	mmci_writel(host, 0, MMCIMASK1);
+	mmci_writel(host, 0xfff, MMCICLEAR);
 
 	if (plat->gpio_cd == -EPROBE_DEFER) {
 		ret = -EPROBE_DEFER;
@@ -1689,7 +1697,7 @@ static int mmci_probe(struct amba_device *dev,
 			goto irq0_free;
 	}
 
-	writel(MCI_IRQENABLE, host->base + MMCIMASK0);
+	mmci_writel(host, MCI_IRQENABLE, MMCIMASK0);
 
 	amba_set_drvdata(dev, mmc);
 
@@ -1745,11 +1753,11 @@ static int mmci_remove(struct amba_device *dev)
 
 		mmc_remove_host(mmc);
 
-		writel(0, host->base + MMCIMASK0);
-		writel(0, host->base + MMCIMASK1);
+		mmci_writel(host, 0, MMCIMASK0);
+		mmci_writel(host, 0, MMCIMASK1);
 
-		writel(0, host->base + MMCICOMMAND);
-		writel(0, host->base + MMCIDATACTRL);
+		mmci_writel(host, 0, MMCICOMMAND);
+		mmci_writel(host, 0, MMCIDATACTRL);
 
 		mmci_dma_release(host);
 		free_irq(dev->irq[0], host);
@@ -1783,7 +1791,7 @@ static int mmci_suspend(struct device *dev)
 	if (mmc) {
 		struct mmci_host *host = mmc_priv(mmc);
 		pm_runtime_get_sync(dev);
-		writel(0, host->base + MMCIMASK0);
+		mmci_writel(host, 0, MMCIMASK0);
 	}
 
 	return 0;
@@ -1796,7 +1804,7 @@ static int mmci_resume(struct device *dev)
 
 	if (mmc) {
 		struct mmci_host *host = mmc_priv(mmc);
-		writel(MCI_IRQENABLE, host->base + MMCIMASK0);
+		mmci_writel(host, MCI_IRQENABLE, MMCIMASK0);
 		pm_runtime_put(dev);
 	}
 
@@ -1812,10 +1820,10 @@ static void mmci_save(struct mmci_host *host)
 	if (host->variant->pwrreg_nopower) {
 		spin_lock_irqsave(&host->lock, flags);
 
-		writel(0, host->base + MMCIMASK0);
-		writel(0, host->base + MMCIDATACTRL);
-		writel(0, host->base + MMCIPOWER);
-		writel(0, host->base + MMCICLOCK);
+		mmci_writel(host, 0, MMCIMASK0);
+		mmci_writel(host, 0, MMCIDATACTRL);
+		mmci_writel(host, 0, MMCIPOWER);
+		mmci_writel(host, 0, MMCICLOCK);
 		mmci_reg_delay(host);
 
 		spin_unlock_irqrestore(&host->lock, flags);
@@ -1830,10 +1838,10 @@ static void mmci_restore(struct mmci_host *host)
 	if (host->variant->pwrreg_nopower) {
 		spin_lock_irqsave(&host->lock, flags);
 
-		writel(host->clk_reg, host->base + MMCICLOCK);
-		writel(host->datactrl_reg, host->base + MMCIDATACTRL);
-		writel(host->pwr_reg, host->base + MMCIPOWER);
-		writel(MCI_IRQENABLE, host->base + MMCIMASK0);
+		mmci_writel(host, host->clk_reg, MMCICLOCK);
+		mmci_writel(host, host->datactrl_reg, MMCIDATACTRL);
+		mmci_writel(host, host->pwr_reg, MMCIPOWER);
+		mmci_writel(host, MCI_IRQENABLE, MMCIMASK0);
 		mmci_reg_delay(host);
 
 		spin_unlock_irqrestore(&host->lock, flags);
-- 
1.7.9.5

[PATCH RFC 05/12] mmc: mmci: use NSEC_PER_SEC macro

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

This patch replaces a constant used in calculating timeout with a proper
macro. This is make code more readable.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/mmc/host/mmci.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 36db31e..4f8d0ba 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -759,7 +759,7 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 	data->bytes_xfered = 0;
 
 	clks = (unsigned long long)data->timeout_ns * host->cclk;
-	do_div(clks, 1000000000UL);
+	do_div(clks, NSEC_PER_SEC);
 
 	timeout = data->timeout_clks + (unsigned int)clks;
 
-- 
1.7.9.5

[PATCH RFC 06/12] mmc: mmci: Add write delay to variant structure.

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

This patch adds write delay parameter required after each write to controller
registers on some of the SOCs like Qualcomm ones. The delay parameter will
provide information on how many clock cycle delay required after each write.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/mmc/host/mmci.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 4f8d0ba..86bf330 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -55,6 +55,8 @@ static unsigned int fmax = 515633;
  *	      is asserted (likewise for RX)
  * @fifohalfsize: number of bytes that can be written when MCI_TXFIFOHALFEMPTY
  *		  is asserted (likewise for RX)
+ * @reg_write_delay: delay in number of clock cycles required after each write
+ *		     to controller registers.
  * @sdio: variant supports SDIO
  * @st_clkdiv: true if using a ST-specific clock divider algorithm
  * @blksz_datactrl16: true if Block size is at b16..b30 position in datactrl register
@@ -72,6 +74,7 @@ struct variant_data {
 	unsigned int		datalength_bits;
 	unsigned int		fifosize;
 	unsigned int		fifohalfsize;
+	unsigned int		reg_write_delay;
 	bool			sdio;
 	bool			st_clkdiv;
 	bool			blksz_datactrl16;
@@ -178,7 +181,12 @@ static inline u32 mmci_readl(struct mmci_host *host, u32 off)
 
 static inline void mmci_writel(struct mmci_host *host, u32 data, u32 off)
 {
+	struct variant_data *var = host->variant;
+
 	writel(data, host->base + off);
+
+	if (var->reg_write_delay && host->mclk)
+		udelay(1 + ((var->reg_write_delay * USEC_PER_SEC)/host->mclk));
 }
 
 static int mmci_card_busy(struct mmc_host *mmc)
-- 
1.7.9.5

Re: [PATCH RFC 06/12] mmc: mmci: Add write delay to variant structure.

[Felipe Balbi]

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

Hi,

On Mon, Apr 21, 2014 at 10:48:18PM +0100, srinivas.kandagatla@linaro.org wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> 
> This patch adds write delay parameter required after each write to controller
> registers on some of the SOCs like Qualcomm ones. The delay parameter will
> provide information on how many clock cycle delay required after each write.
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>  drivers/mmc/host/mmci.c |    8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index 4f8d0ba..86bf330 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -55,6 +55,8 @@ static unsigned int fmax = 515633;
>   *	      is asserted (likewise for RX)
>   * @fifohalfsize: number of bytes that can be written when MCI_TXFIFOHALFEMPTY
>   *		  is asserted (likewise for RX)
> + * @reg_write_delay: delay in number of clock cycles required after each write
> + *		     to controller registers.
>   * @sdio: variant supports SDIO
>   * @st_clkdiv: true if using a ST-specific clock divider algorithm
>   * @blksz_datactrl16: true if Block size is at b16..b30 position in datactrl register
> @@ -72,6 +74,7 @@ struct variant_data {
>  	unsigned int		datalength_bits;
>  	unsigned int		fifosize;
>  	unsigned int		fifohalfsize;
> +	unsigned int		reg_write_delay;
>  	bool			sdio;
>  	bool			st_clkdiv;
>  	bool			blksz_datactrl16;
> @@ -178,7 +181,12 @@ static inline u32 mmci_readl(struct mmci_host *host, u32 off)
>  
>  static inline void mmci_writel(struct mmci_host *host, u32 data, u32 off)
>  {
> +	struct variant_data *var = host->variant;
> +
>  	writel(data, host->base + off);
> +
> +	if (var->reg_write_delay && host->mclk)
> +		udelay(1 + ((var->reg_write_delay * USEC_PER_SEC)/host->mclk));

looks like this should be quirk flag instead of a write delay... No
strong feelings though, but it looks like the following would be better,
perhaps:

	if (host_is_qualcom(host))
		udelay(1 + ((3 * USEC_PER_SEC)/host->mclk));

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH RFC 06/12] mmc: mmci: Add write delay to variant structure.

[Srinivas Kandagatla]

Thanks Felipe,

On 21/04/14 23:08, Felipe Balbi wrote:
>> +	if (var->reg_write_delay && host->mclk)
>> >+		udelay(1 + ((var->reg_write_delay * USEC_PER_SEC)/host->mclk));
> looks like this should be quirk flag instead of a write delay... No
> strong feelings though, but it looks like the following would be better,
> perhaps:
>
> 	if (host_is_qualcom(host))
> 		udelay(1 + ((3 * USEC_PER_SEC)/host->mclk));
Am ok with your proposal. I was wondering if someone else might need it 
in future.
If not I could change it as you suggested.

Thanks,
srini
>
> --

[PATCH RFC 07/12] mmc: mmci: Qcomm: Add 3 clock cycle delay after each register write

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

This patch adds a 3 clock cycle delay required after writing to controller
registers on Qualcomm SOCs. Without this delay cards are either not detected
or fails as soon as card is put into data transfer mode.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/mmc/host/mmci.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 86bf330..2dc7581 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -168,6 +168,7 @@ static struct variant_data variant_qcom = {
 	.fifosize		= 16 * 4,
 	.fifohalfsize		= 8 * 4,
 	.clkreg			= MCI_CLK_ENABLE,
+	.reg_write_delay	= 3,
 	.blksz_datactrl4	= true,
 	.datalength_bits	= 24,
 	.blksz_datactrl4	= true,
-- 
1.7.9.5

[PATCH RFC 08/12] mmc: mmci: move ST specific register extensions access under condition.

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

This patch moves some of the ST specific register extensions access under
condition, so that other SOCs like Qualcomm or ARM would not a side effect of
writing to those reserved/different purpose bits.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/mmc/host/mmci.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 2dc7581..179abfb 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -270,7 +270,8 @@ static void mmci_write_pwrreg(struct mmci_host *host, u32 pwr)
 static void mmci_write_datactrlreg(struct mmci_host *host, u32 datactrl)
 {
 	/* Keep ST Micro busy mode if enabled */
-	datactrl |= host->datactrl_reg & MCI_ST_DPSM_BUSYMODE;
+	if (host->hw_designer == AMBA_VENDOR_ST)
+		datactrl |= host->datactrl_reg & MCI_ST_DPSM_BUSYMODE;
 
 	if (host->datactrl_reg != datactrl) {
 		host->datactrl_reg = datactrl;
@@ -816,7 +817,8 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 			mmci_write_clkreg(host, clk);
 		}
 
-	if (host->mmc->ios.timing == MMC_TIMING_UHS_DDR50)
+	if (host->mmc->ios.timing == MMC_TIMING_UHS_DDR50 &&
+	    host->hw_designer == AMBA_VENDOR_ST)
 		datactrl |= MCI_ST_DPSM_DDRMODE;
 
 	/*
-- 
1.7.9.5

[PATCH RFC 09/12] mmc: mmci: Qcom fix MCICLK register settings.

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

MCICLK register layout is bit different to the standard pl180 register layout.
Qcom SDCC controller some setup in MCICLK register to get it going. So this
patch adds new setup and makes it specific to Qcom hw designer.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/mmc/host/mmci.c |   36 ++++++++++++++++++++++++++++++------
 drivers/mmc/host/mmci.h |   21 +++++++++++++++++++++
 2 files changed, 51 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 179abfb..f465eb5 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -327,13 +327,37 @@ static void mmci_set_clkreg(struct mmci_host *host, unsigned int desired)
 	/* Set actual clock for debug */
 	host->mmc->actual_clock = host->cclk;
 
-	if (host->mmc->ios.bus_width == MMC_BUS_WIDTH_4)
-		clk |= MCI_4BIT_BUS;
-	if (host->mmc->ios.bus_width == MMC_BUS_WIDTH_8)
-		clk |= MCI_ST_8BIT_BUS;
+	if (host->hw_designer == AMBA_VENDOR_QCOM) {
+		clk |= MCI_CLK_QCOM_FLOWENA;
+		clk |= (MCI_CLK_QCOM_SEL_FEEDBACK_CLK <<
+				MCI_CLK_QCOM_SEL_IN_SHIFT); /* feedback clk */
+		if (host->mmc->ios.bus_width == MMC_BUS_WIDTH_8)
+			clk |= MCI_CLK_QCOM_WIDEBUS_8;
+		else if (host->mmc->ios.bus_width == MMC_BUS_WIDTH_4)
+			clk |= MCI_CLK_QCOM_WIDEBUS_4;
+		else
+			clk |= MCI_CLK_QCOM_WIDEBUS_1;
+
+		if (host->mmc->ios.timing == MMC_TIMING_UHS_DDR50) {
+			/* clear SELECT_IN field */
+			clk &= ~(MCI_CLK_QCOM_SEL_MASK <<
+					MCI_CLK_QCOM_SEL_IN_SHIFT);
+			/* set DDR timing mode */
+			clk |= (MCI_CLK_QCOM_SEL_DDR_MODE <<
+					MCI_CLK_QCOM_SEL_IN_SHIFT);
+		}
+		clk |= (MCI_CLK_SDC4_MCLK_SEL_MCLK <<
+				MCI_CLK_SDC4_MCLK_SEL_SHIFT);
 
-	if (host->mmc->ios.timing == MMC_TIMING_UHS_DDR50)
-		clk |= MCI_ST_UX500_NEG_EDGE;
+	} else {
+		if (host->mmc->ios.bus_width == MMC_BUS_WIDTH_4)
+			clk |= MCI_4BIT_BUS;
+		if (host->mmc->ios.bus_width == MMC_BUS_WIDTH_8)
+			clk |= MCI_ST_8BIT_BUS;
+
+		if (host->mmc->ios.timing == MMC_TIMING_UHS_DDR50)
+			clk |= MCI_ST_UX500_NEG_EDGE;
+	}
 
 	mmci_write_clkreg(host, clk);
 }
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 58b1b88..0a6de1c 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -31,6 +31,27 @@
 /* Modified PL180 on Versatile Express platform */
 #define MCI_ARM_HWFCEN		(1 << 12)
 
+/* Modified on Qualcomm Integrations */
+#define MCI_CLK_QCOM_WIDEBUS_1	(0 << 10)
+#define MCI_CLK_QCOM_WIDEBUS_4	(2 << 10)
+#define MCI_CLK_QCOM_WIDEBUS_8	(3 << 10)
+#define MCI_CLK_QCOM_FLOWENA	(1 << 12)
+#define MCI_CLK_QCOM_INVERTOUT	(1 << 13)
+
+/* select in latch data and command */
+#define MCI_CLK_QCOM_SEL_IN_SHIFT	(14)
+#define MCI_CLK_QCOM_SEL_MASK		(0x3)
+#define MCI_CLK_QCOM_SEL_RISING_EDGE	(1)
+#define MCI_CLK_QCOM_SEL_FEEDBACK_CLK	(2)
+#define MCI_CLK_QCOM_SEL_DDR_MODE	(3)
+
+/* mclk selection */
+#define MCI_CLK_SDC4_MCLK_SEL_SHIFT	(23)
+#define MCI_CLK_SDC4_MCLK_SEL_MASK	(0x3)
+#define MCI_CLK_SDC4_MCLK_SEL_FB_CLK	(1)
+#define MCI_CLK_SDC4_MCLK_SEL_MCLK	(2)
+
+
 #define MMCIARGUMENT		0x008
 #define MMCICOMMAND		0x00c
 #define MCI_CPSM_RESPONSE	(1 << 6)
-- 
1.7.9.5

[PATCH RFC 10/12] mmc: mmci: Add clock support for Qualcomm.

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

MCICLK going to card bus is directly driven by the clock controller, so the
driver has to set the required rates depending on the state of the card. This
bit of support is very much similar to bypass mode but there is no such thing
called bypass mode in MCICLK register of Qcom SD card controller. By default
the clock is directly driven by the clk controller.

This patch adds clock support for Qualcomm SDCC in the driver. This bit of
code is conditioned on hw designer.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/mmc/host/mmci.c |   16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index f465eb5..2cd3a8f 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -291,7 +291,18 @@ static void mmci_set_clkreg(struct mmci_host *host, unsigned int desired)
 	host->cclk = 0;
 
 	if (desired) {
-		if (desired >= host->mclk) {
+		if (desired != host->mclk &&
+		    host->hw_designer == AMBA_VENDOR_QCOM) {
+			/* Qcom MCLKCLK register does not define bypass bits */
+			int rc = clk_set_rate(host->clk, desired);
+			if (rc < 0) {
+				dev_err(mmc_dev(host->mmc),
+					"Error setting clock rate (%d)\n", rc);
+			} else {
+				host->mclk = clk_get_rate(host->clk);
+				host->cclk = host->mclk;
+			}
+		} else if (desired >= host->mclk) {
 			clk = MCI_CLK_BYPASS;
 			if (variant->st_clkdiv)
 				clk |= MCI_ST_UX500_NEG_EDGE;
@@ -1612,7 +1623,8 @@ static int mmci_probe(struct amba_device *dev,
 	 * of course.
 	 */
 	if (plat->f_max)
-		mmc->f_max = min(host->mclk, plat->f_max);
+		mmc->f_max = (host->hw_designer == AMBA_VENDOR_QCOM) ?
+				plat->f_max : min(host->mclk, plat->f_max);
 	else
 		mmc->f_max = min(host->mclk, fmax);
 	dev_dbg(mmc_dev(mmc), "clocking block at %u Hz\n", mmc->f_max);
-- 
1.7.9.5

Re: [PATCH RFC 10/12] mmc: mmci: Add clock support for Qualcomm.

[Stephen Boyd]

On 04/21/14 14:49, srinivas.kandagatla@linaro.org wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> MCICLK going to card bus is directly driven by the clock controller, so the
> driver has to set the required rates depending on the state of the card. This
> bit of support is very much similar to bypass mode but there is no such thing
> called bypass mode in MCICLK register of Qcom SD card controller. By default
> the clock is directly driven by the clk controller.
>
> This patch adds clock support for Qualcomm SDCC in the driver. This bit of
> code is conditioned on hw designer.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>  drivers/mmc/host/mmci.c |   16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index f465eb5..2cd3a8f 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -291,7 +291,18 @@ static void mmci_set_clkreg(struct mmci_host *host, unsigned int desired)
>  	host->cclk = 0;
>  
>  	if (desired) {
> -		if (desired >= host->mclk) {
> +		if (desired != host->mclk &&
> +		    host->hw_designer == AMBA_VENDOR_QCOM) {
> +			/* Qcom MCLKCLK register does not define bypass bits */
> +			int rc = clk_set_rate(host->clk, desired);

Please turn on lockdep (PROVE_LOCKING) and sleeping while atomic checks
(DEBUG_ATOMIC_SLEEP). You cannot call clk_set_rate() in atomic context.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH RFC 10/12] mmc: mmci: Add clock support for Qualcomm.

[Srinivas Kandagatla]

Thankyou Stephen,

On 21/04/14 23:20, Stephen Boyd wrote:
> Please turn on lockdep (PROVE_LOCKING) and sleeping while atomic checks
> (DEBUG_ATOMIC_SLEEP). You cannot call clk_set_rate() in atomic context.
You are correct, there is a spinlock taken just before entering this 
code. I will fix this in next version by moving clk_set_rate out of this 
lock.

Thanks,
srini

[PATCH RFC 11/12] mmc: mmci: Add Qcom variations to MCICommand register.

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Some bits which control Command Path State Machine (CPSM) are new in Qcom
integration, so this patch adds support to those bits.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/mmc/host/mmci.c |    4 ++++
 drivers/mmc/host/mmci.h |    7 +++++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 2cd3a8f..8fcd8ef 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -910,6 +910,10 @@ mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c)
 	if (/*interrupt*/0)
 		c |= MCI_CPSM_INTERRUPT;
 
+	if (host->hw_designer == AMBA_VENDOR_QCOM &&
+	    mmc_cmd_type(cmd) == MMC_CMD_ADTC)
+		c |= MCI_CSPM_QCOM_DATCMD;
+
 	host->cmd = cmd;
 
 	mmci_writel(host, cmd->arg, MMCIARGUMENT);
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 0a6de1c..2ba0834 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -64,6 +64,13 @@
 #define MCI_ST_ENCMD_COMPL	(1 << 12)
 #define MCI_ST_NIEN		(1 << 13)
 #define MCI_ST_CE_ATACMD	(1 << 14)
+/* Modified on Qualcomm Integrations */
+#define MCI_CSPM_QCOM_DATCMD		(1 << 12)
+#define MCI_CSPM_QCOM_MCIABORT		(1 << 13)
+#define MCI_CSPM_QCOM_CCSENABLE		(1 << 14)
+#define MCI_CSPM_QCOM_CCSDISABLE	(1 << 15)
+#define MCI_CSPM_QCOM_AUTO_CMD19	(1 << 16)
+#define MCI_CSPM_QCOM_AUTO_CMD21	(1 << 21)
 
 #define MMCIRESPCMD		0x010
 #define MMCIRESPONSE0		0x014
-- 
1.7.9.5

[PATCH RFC 12/12] mmc: mmci: Add Qcom specific pio_read function.

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

MCIFIFOCNT register behaviour on Qcom chips is very different than the other
pl180 integrations. MCIFIFOCNT register contains the number of
words that are still waiting to be transferred through the FIFO. It keeps
decrementing once the host CPU reads the MCIFIFO. With the existing logic and
the MCIFIFOCNT behaviour, mmci_pio_read will loop forever, as the FIFOCNT
register will always return transfer size before reading the FIFO.

This patch implements qcom_pio_read function so as existing mmci_pio_read is
not suitable for Qcom SOCs.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/mmc/host/mmci.c |   31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 8fcd8ef..585888e 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -1047,6 +1047,29 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd,
 	}
 }
 
+static int mmci_qcom_pio_read(struct mmci_host *host, char *buffer,
+			 unsigned int remain)
+{
+	uint32_t	*ptr = (uint32_t *) buffer;
+	int		count = 0;
+	struct variant_data *variant = host->variant;
+	int		fifo_size = variant->fifosize;
+
+	if (remain % 4)
+		remain = ((remain >> 2) + 1) << 2;
+
+	while (readl(host->base + MMCISTATUS) & MCI_RXDATAAVLBL) {
+		*ptr = readl(host->base + MMCIFIFO + (count % fifo_size));
+		ptr++;
+		count += sizeof(uint32_t);
+
+		remain -=  sizeof(uint32_t);
+		if (remain == 0)
+			break;
+	}
+	return count;
+}
+
 static int mmci_pio_read(struct mmci_host *host, char *buffer, unsigned int remain)
 {
 	void __iomem *base = host->base;
@@ -1168,8 +1191,12 @@ static irqreturn_t mmci_pio_irq(int irq, void *dev_id)
 		remain = sg_miter->length;
 
 		len = 0;
-		if (status & MCI_RXACTIVE)
-			len = mmci_pio_read(host, buffer, remain);
+		if (status & MCI_RXACTIVE) {
+			if (host->hw_designer == AMBA_VENDOR_QCOM)
+				len = mmci_qcom_pio_read(host, buffer, remain);
+			else
+				len = mmci_pio_read(host, buffer, remain);
+		}
 		if (status & MCI_TXACTIVE)
 			len = mmci_pio_write(host, buffer, remain, status);
 
-- 
1.7.9.5

Re: [PATCH RFC 00/12] Add Qualcomm SD Card Controller support.

[Christopher Covington]

Hi Srini,

On 04/21/2014 05:43 PM, srinivas.kandagatla@linaro.org wrote:

> This patches are tested in PIO mode on IFC8064 board with both eMMC and
> external SD card. I would appreciate any feedback/suggestions on the overall
> approach.

Is a non-upstream device tree needed to reproduce your test setup? If so could
you please point to or post it?

Thanks,
Christopher

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by the Linux Foundation.

Re: [PATCH RFC 00/12] Add Qualcomm SD Card Controller support.

[Srinivas Kandagatla]

Hi Christopher,


On 22/04/14 13:58, Christopher Covington wrote:
> Hi Srini,
>
> On 04/21/2014 05:43 PM, srinivas.kandagatla@linaro.org wrote:
>
>> This patches are tested in PIO mode on IFC8064 board with both eMMC and
>> external SD card. I would appreciate any feedback/suggestions on the overall
>> approach.
>
I pushed a temporary tag "mmci-pio-srini-wip" for you on my git tree 
git://git.linaro.org/landing-teams/working/qualcomm/kernel.git

> Is a non-upstream device tree needed to reproduce your test setup? If so could
> you please point to or post it?

You will need to use multi_v7_defconfig.

Thanks,
srini
>
> Thanks,
> Christopher
>

[PATCH v1 00/11] Add Qualcomm SD Card Controller support.

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Hi Russell,

This patch series adds Qualcomm SD Card Controller support in pl180 mmci
driver. QCom SDCC is basically a pl180, but bit more customized, some of the
register layouts and offsets are different to the ones mentioned in pl180
datasheet. The plan is to totally remove the standalone SDCC driver
drivers/mmc/host/msm_sdcc.* and start using generic mmci driver for all
Qualcomm parts, as we get chance to test on other Qcom boards.

To start using the existing mmci driver, a fake amba id for Qualcomm is added
in patches:
 ARM: amba: Add Qualcomm vendor ID.
 mmc: mmci: Add Qualcomm Id to amba id table.

Second change is, adding a 3 clock cycle delay for register writes on QCOM
SDCC
registers, which is done in patches:
  mmc: mmci: Add register read/write wrappers.
  mmc: mmci: Qcomm: Add 3 clock cycle delay after each register write

Third change was to accommodate DATCTRL and MMCICLK register layout changes in
Qcom SDCC. Which is done in patches:
  mmc: mmci: Add Qcom datactrl register variant
  mmc: mmci: Add Qcom variations to MCICommand register.
  mmc: mmci: Qcom fix MCICLK register settings.
  mmc: mmci: Add clock support for Qualcomm.

Fourth major change was to add qcom specfic pio read function, the need for
this is because the way MCIFIFOCNT register behaved in QCOM SDCC is very
 different to the one in pl180. This change is done in patch:
  mmc: mmci: Add Qcom specific pio_read function.

Last some Qcom unrelated changes to support Qcom are done in patches:
  mmc: mmci: use NSEC_PER_SEC macro
  mmc: mmci: move ST specific register extensions access under condition.

This patches are tested  v3.15-rc3 in PIO mode on IFC6410 board with both eMMC
and external SD card. I would appreciate any feedback on the patches.
I would like to get this for v3.16.

Thanks,
srini

Srinivas Kandagatla (11):
  ARM: amba: Add Qualcomm vendor ID.
  mmc: mmci: Add Qualcomm Id to amba id table
  mmc: mmci: Add Qcom datactrl register variant
  mmc: mmci: Add register read/write wrappers.
  mmc: mmci: use NSEC_PER_SEC macro
  mmc: mmci: Qcomm: Add 3 clock cycle delay after register write
  mmc: mmci: move ST specific register extensions access under
    condition.
  mmc: mmci: Qcom fix MCICLK register settings.
  mmc: mmci: Add clock support for Qualcomm.
  mmc: mmci: Add Qcom variations to MCICommand register.
  mmc: mmci: Add Qcom specific pio_read function.

 drivers/mmc/host/mmci.c  |  243 +++++++++++++++++++++++++++++++++-------------
 drivers/mmc/host/mmci.h  |   28 ++++++
 include/linux/amba/bus.h |    1 +
 3 files changed, 206 insertions(+), 66 deletions(-)

-- 
1.7.9.5

[PATCH v1 01/11] ARM: amba: Add Qualcomm vendor ID.

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

This patch adds Qualcomm amba vendor Id to the list. This ID is used in mmci
driver.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 include/linux/amba/bus.h |    1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/amba/bus.h b/include/linux/amba/bus.h
index 63b5eff..fdd7e1b 100644
--- a/include/linux/amba/bus.h
+++ b/include/linux/amba/bus.h
@@ -47,6 +47,7 @@ struct amba_driver {
 enum amba_vendor {
 	AMBA_VENDOR_ARM = 0x41,
 	AMBA_VENDOR_ST = 0x80,
+	AMBA_VENDOR_QCOM = 0x51,
 };
 
 extern struct bus_type amba_bustype;
-- 
1.7.9.5

Re: [PATCH v1 01/11] ARM: amba: Add Qualcomm vendor ID.

[Linus Walleij]

On Tue, Apr 29, 2014 at 10:19 AM,  <srinivas.kandagatla@linaro.org> wrote:

> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> This patch adds Qualcomm amba vendor Id to the list. This ID is used in mmci
> driver.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
(...)
> +       AMBA_VENDOR_QCOM = 0x51,

Yeah it's a "Q", like 0x41 is "A" for ARM. You could as well mention
this in the commit message.

And you can probably just put this patch into Russell's patch tracker
already.

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH v1 01/11] ARM: amba: Add Qualcomm vendor ID.

[Srinivas Kandagatla]

Thanks Linus W,

On 13/05/14 08:16, Linus Walleij wrote:
> On Tue, Apr 29, 2014 at 10:19 AM,  <srinivas.kandagatla@linaro.org> wrote:
>
>> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>
>> This patch adds Qualcomm amba vendor Id to the list. This ID is used in mmci
>> driver.
>>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> (...)
>> +       AMBA_VENDOR_QCOM = 0x51,
>
> Yeah it's a "Q", like 0x41 is "A" for ARM. You could as well mention
> this in the commit message.
>
Sounds good, I will fix it in next version and send it to Russell's 
patch tracker.


--srini
> And you can probably just put this patch into Russell's patch tracker
> already.
>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
>
> Yours,
> Linus Walleij
>

Re: [PATCH v1 01/11] ARM: amba: Add Qualcomm vendor ID.

[Stephen Boyd]

On 05/13, Srinivas Kandagatla wrote:
> Thanks Linus W,
> 
> On 13/05/14 08:16, Linus Walleij wrote:
> >On Tue, Apr 29, 2014 at 10:19 AM,  <srinivas.kandagatla@linaro.org> wrote:
> >
> >>From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> >>
> >>This patch adds Qualcomm amba vendor Id to the list. This ID is used in mmci
> >>driver.
> >>
> >>Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> >(...)
> >>+       AMBA_VENDOR_QCOM = 0x51,
> >
> >Yeah it's a "Q", like 0x41 is "A" for ARM. You could as well mention
> >this in the commit message.
> >
> Sounds good, I will fix it in next version and send it to Russell's
> patch tracker.
> 

Please add a note that this id is fake in the commit text or in
the code as well.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH v1 01/11] ARM: amba: Add Qualcomm vendor ID.

[Srinivas Kandagatla]

On 13/05/14 23:13, Stephen Boyd wrote:
> On 05/13, Srinivas Kandagatla wrote:
>> Thanks Linus W,
>>
>> On 13/05/14 08:16, Linus Walleij wrote:
>>> On Tue, Apr 29, 2014 at 10:19 AM,  <srinivas.kandagatla@linaro.org> wrote:
>>>
>>>> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>>>
>>>> This patch adds Qualcomm amba vendor Id to the list. This ID is used in mmci
>>>> driver.
>>>>
>>>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>> (...)
>>>> +       AMBA_VENDOR_QCOM = 0x51,
>>>
>>> Yeah it's a "Q", like 0x41 is "A" for ARM. You could as well mention
>>> this in the commit message.
>>>
>> Sounds good, I will fix it in next version and send it to Russell's
>> patch tracker.
>>
>
> Please add a note that this id is fake in the commit text or in
> the code as well.
Sure, I will add it in commit text.


>

Re: [PATCH v1 01/11] ARM: amba: Add Qualcomm vendor ID.

[Linus Walleij]

On Wed, May 14, 2014 at 12:13 AM, Stephen Boyd <sboyd@codeaurora.org> wrote:

> Please add a note that this id is fake in the commit text or in
> the code as well.

You just need one of your hardware engineers to add is to one instance
of some test chip round so it exists in any hardware whatsoever and it
is not "fake" anymore...

This is a bit like device tree, it's defined by the kernel community as much
as by the hardware engineers and it's a little bit of back-and-forth.

Yours,
Linus Walleij

Re: [PATCH v1 01/11] ARM: amba: Add Qualcomm vendor ID.

[Stephen Boyd]

On 05/16/14 09:24, Linus Walleij wrote:
> On Wed, May 14, 2014 at 12:13 AM, Stephen Boyd <sboyd@codeaurora.org> wrote:
>
>> Please add a note that this id is fake in the commit text or in
>> the code as well.
> You just need one of your hardware engineers to add is to one instance
> of some test chip round so it exists in any hardware whatsoever and it
> is not "fake" anymore...
>
> This is a bit like device tree, it's defined by the kernel community as much
> as by the hardware engineers and it's a little bit of back-and-forth.
>

Sure, but hardware teams have moved on from this IP so I doubt we can
even get them to put it into a test chip. In fact, the devices a
generation before the ones Srini is working on (msm8660 for example)
have a periph ID of 0x41180 so we need to override that ID in DT with
this fake ID for things to even work.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH v1 01/11] ARM: amba: Add Qualcomm vendor ID.

[Linus Walleij]

On Fri, May 16, 2014 at 6:43 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 05/16/14 09:24, Linus Walleij wrote:
>> On Wed, May 14, 2014 at 12:13 AM, Stephen Boyd <sboyd@codeaurora.org> wrote:
>>
>>> Please add a note that this id is fake in the commit text or in
>>> the code as well.
>> You just need one of your hardware engineers to add is to one instance
>> of some test chip round so it exists in any hardware whatsoever and it
>> is not "fake" anymore...
>>
>> This is a bit like device tree, it's defined by the kernel community as much
>> as by the hardware engineers and it's a little bit of back-and-forth.
>>
>
> Sure, but hardware teams have moved on from this IP so I doubt we can
> even get them to put it into a test chip. In fact, the devices a
> generation before the ones Srini is working on (msm8660 for example)
> have a periph ID of 0x41180 so we need to override that ID in DT with
> this fake ID for things to even work.

Hehe I wasn't really suggesting doing that.

The point was to illustrate how fragile the border between "real"
and "fake" is in these cases.

Yours,
Linus Walleij

[PATCH v1 02/11] mmc: mmci: Add Qualcomm Id to amba id table

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

This patch adds a fake Qualcomm ID 0x00051180 to the amba_ids, as Qualcomm
SDCC controller is pl180, but amba id registers read 0x0's.
The plan is to remove SDCC driver totally and use mmci as the main SD
controller driver for Qualcomm SOCs.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/mmc/host/mmci.c |   15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 771c60a..391e8d4 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -158,6 +158,15 @@ static struct variant_data variant_ux500v2 = {
 	.pwrreg_nopower		= true,
 };
 
+static struct variant_data variant_qcom = {
+	.fifosize		= 16 * 4,
+	.fifohalfsize		= 8 * 4,
+	.clkreg			= MCI_CLK_ENABLE,
+	.datalength_bits	= 24,
+	.blksz_datactrl4	= true,
+	.pwrreg_powerup		= MCI_PWR_UP,
+};
+
 static int mmci_card_busy(struct mmc_host *mmc)
 {
 	struct mmci_host *host = mmc_priv(mmc);
@@ -1908,6 +1917,12 @@ static struct amba_id mmci_ids[] = {
 		.mask   = 0xf0ffffff,
 		.data	= &variant_ux500v2,
 	},
+	/* Qualcomm variants */
+	{
+		.id     = 0x00051180,
+		.mask	= 0x000fffff,
+		.data	= &variant_qcom,
+	},
 	{ 0, 0 },
 };
 
-- 
1.7.9.5

Re: [PATCH v1 02/11] mmc: mmci: Add Qualcomm Id to amba id table

[Linus Walleij]

On Tue, Apr 29, 2014 at 10:19 AM,  <srinivas.kandagatla@linaro.org> wrote:

> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> This patch adds a fake Qualcomm ID 0x00051180 to the amba_ids, as Qualcomm
> SDCC controller is pl180, but amba id registers read 0x0's.
> The plan is to remove SDCC driver totally and use mmci as the main SD
> controller driver for Qualcomm SOCs.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Re: [PATCH v1 02/11] mmc: mmci: Add Qualcomm Id to amba id table

[Srinivas Kandagatla]

Thanks Linus W.

On 13/05/14 08:17, Linus Walleij wrote:
> On Tue, Apr 29, 2014 at 10:19 AM,  <srinivas.kandagatla@linaro.org> wrote:
>
>> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>
>> This patch adds a fake Qualcomm ID 0x00051180 to the amba_ids, as Qualcomm
>> SDCC controller is pl180, but amba id registers read 0x0's.
>> The plan is to remove SDCC driver totally and use mmci as the main SD
>> controller driver for Qualcomm SOCs.
>>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>

[PATCH v1 03/11] mmc: mmci: Add Qcom datactrl register variant

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Instance of this IP on Qualcomm's SOCs has bit different layout for datactrl
register. Bit postion datactrl[16:4] hold the true block size instead of power
of 2.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/mmc/host/mmci.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 391e8d4..19d6b6f 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -58,6 +58,8 @@ static unsigned int fmax = 515633;
  * @sdio: variant supports SDIO
  * @st_clkdiv: true if using a ST-specific clock divider algorithm
  * @blksz_datactrl16: true if Block size is at b16..b30 position in datactrl register
+ * @blksz_datactrl4: true if Block size is at b4..b16 position in datactrl
+ *		     register
  * @pwrreg_powerup: power up value for MMCIPOWER register
  * @signal_direction: input/out direction of bus signals can be indicated
  * @pwrreg_clkgate: MMCIPOWER register must be used to gate the clock
@@ -73,6 +75,7 @@ struct variant_data {
 	bool			sdio;
 	bool			st_clkdiv;
 	bool			blksz_datactrl16;
+	bool			blksz_datactrl4;
 	u32			pwrreg_powerup;
 	bool			signal_direction;
 	bool			pwrreg_clkgate;
@@ -162,6 +165,7 @@ static struct variant_data variant_qcom = {
 	.fifosize		= 16 * 4,
 	.fifohalfsize		= 8 * 4,
 	.clkreg			= MCI_CLK_ENABLE,
+	.blksz_datactrl4	= true,
 	.datalength_bits	= 24,
 	.blksz_datactrl4	= true,
 	.pwrreg_powerup		= MCI_PWR_UP,
@@ -760,6 +764,8 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 
 	if (variant->blksz_datactrl16)
 		datactrl = MCI_DPSM_ENABLE | (data->blksz << 16);
+	else if (variant->blksz_datactrl4)
+		datactrl = MCI_DPSM_ENABLE | (data->blksz << 4);
 	else
 		datactrl = MCI_DPSM_ENABLE | blksz_bits << 4;
 
-- 
1.7.9.5

Re: [PATCH v1 03/11] mmc: mmci: Add Qcom datactrl register variant

[Linus Walleij]

On Tue, Apr 29, 2014 at 10:19 AM,  <srinivas.kandagatla@linaro.org> wrote:

> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> Instance of this IP on Qualcomm's SOCs has bit different layout for datactrl
> register. Bit postion datactrl[16:4] hold the true block size instead of power
> of 2.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

This is probably something I got wrong when I attempted to use
this driver on the Dragon board. Now I see what I missed...

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH v1 04/11] mmc: mmci: Add register read/write wrappers.

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

This patch adds wrappers for readl/writel functions used in the driver. The
reason for this wrappers is to accommodate SOCs like Qualcomm which has
requirement for delaying the write for few cycles when writing to its SD Card
Controller registers.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/mmc/host/mmci.c |  114 +++++++++++++++++++++++++----------------------
 1 file changed, 61 insertions(+), 53 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 19d6b6f..36db31e 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -171,6 +171,16 @@ static struct variant_data variant_qcom = {
 	.pwrreg_powerup		= MCI_PWR_UP,
 };
 
+static inline u32 mmci_readl(struct mmci_host *host, u32 off)
+{
+	return readl(host->base  + off);
+}
+
+static inline void mmci_writel(struct mmci_host *host, u32 data, u32 off)
+{
+	writel(data, host->base + off);
+}
+
 static int mmci_card_busy(struct mmc_host *mmc)
 {
 	struct mmci_host *host = mmc_priv(mmc);
@@ -180,7 +190,7 @@ static int mmci_card_busy(struct mmc_host *mmc)
 	pm_runtime_get_sync(mmc_dev(mmc));
 
 	spin_lock_irqsave(&host->lock, flags);
-	if (readl(host->base + MMCISTATUS) & MCI_ST_CARDBUSY)
+	if (mmci_readl(host, MMCISTATUS) & MCI_ST_CARDBUSY)
 		busy = 1;
 	spin_unlock_irqrestore(&host->lock, flags);
 
@@ -230,7 +240,7 @@ static void mmci_write_clkreg(struct mmci_host *host, u32 clk)
 {
 	if (host->clk_reg != clk) {
 		host->clk_reg = clk;
-		writel(clk, host->base + MMCICLOCK);
+		mmci_writel(host, clk, MMCICLOCK);
 	}
 }
 
@@ -241,7 +251,7 @@ static void mmci_write_pwrreg(struct mmci_host *host, u32 pwr)
 {
 	if (host->pwr_reg != pwr) {
 		host->pwr_reg = pwr;
-		writel(pwr, host->base + MMCIPOWER);
+		mmci_writel(host, pwr, MMCIPOWER);
 	}
 }
 
@@ -255,7 +265,7 @@ static void mmci_write_datactrlreg(struct mmci_host *host, u32 datactrl)
 
 	if (host->datactrl_reg != datactrl) {
 		host->datactrl_reg = datactrl;
-		writel(datactrl, host->base + MMCIDATACTRL);
+		mmci_writel(host, datactrl, MMCIDATACTRL);
 	}
 }
 
@@ -321,7 +331,7 @@ static void mmci_set_clkreg(struct mmci_host *host, unsigned int desired)
 static void
 mmci_request_end(struct mmci_host *host, struct mmc_request *mrq)
 {
-	writel(0, host->base + MMCICOMMAND);
+	mmci_writel(host, 0, MMCICOMMAND);
 
 	BUG_ON(host->data);
 
@@ -336,18 +346,16 @@ mmci_request_end(struct mmci_host *host, struct mmc_request *mrq)
 
 static void mmci_set_mask1(struct mmci_host *host, unsigned int mask)
 {
-	void __iomem *base = host->base;
-
 	if (host->singleirq) {
-		unsigned int mask0 = readl(base + MMCIMASK0);
+		unsigned int mask0 = mmci_readl(host, MMCIMASK0);
 
 		mask0 &= ~MCI_IRQ1MASK;
 		mask0 |= mask;
 
-		writel(mask0, base + MMCIMASK0);
+		mmci_writel(host, mask0, MMCIMASK0);
 	}
 
-	writel(mask, base + MMCIMASK1);
+	mmci_writel(host, mask, MMCIMASK1);
 }
 
 static void mmci_stop_data(struct mmci_host *host)
@@ -498,7 +506,7 @@ static void mmci_dma_finalize(struct mmci_host *host, struct mmc_data *data)
 
 	/* Wait up to 1ms for the DMA to complete */
 	for (i = 0; ; i++) {
-		status = readl(host->base + MMCISTATUS);
+		status = mmci_readl(host, MMCISTATUS);
 		if (!(status & MCI_RXDATAAVLBLMASK) || i >= 100)
 			break;
 		udelay(10);
@@ -637,8 +645,8 @@ static int mmci_dma_start_data(struct mmci_host *host, unsigned int datactrl)
 	 * to fire next DMA request. When that happens, MMCI will
 	 * call mmci_data_end()
 	 */
-	writel(readl(host->base + MMCIMASK0) | MCI_DATAENDMASK,
-	       host->base + MMCIMASK0);
+	mmci_writel(host, mmci_readl(host, MMCIMASK0) | MCI_DATAENDMASK,
+		    MMCIMASK0);
 	return 0;
 }
 
@@ -756,8 +764,8 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 	timeout = data->timeout_clks + (unsigned int)clks;
 
 	base = host->base;
-	writel(timeout, base + MMCIDATATIMER);
-	writel(host->size, base + MMCIDATALENGTH);
+	mmci_writel(host, timeout, MMCIDATATIMER);
+	mmci_writel(host, host->size, MMCIDATALENGTH);
 
 	blksz_bits = ffs(data->blksz) - 1;
 	BUG_ON(1 << blksz_bits != data->blksz);
@@ -831,20 +839,19 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 	}
 
 	mmci_write_datactrlreg(host, datactrl);
-	writel(readl(base + MMCIMASK0) & ~MCI_DATAENDMASK, base + MMCIMASK0);
+	mmci_writel(host, mmci_readl(host, MMCIMASK0) & ~MCI_DATAENDMASK,
+		    MMCIMASK0);
 	mmci_set_mask1(host, irqmask);
 }
 
 static void
 mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c)
 {
-	void __iomem *base = host->base;
-
 	dev_dbg(mmc_dev(host->mmc), "op %02x arg %08x flags %08x\n",
 	    cmd->opcode, cmd->arg, cmd->flags);
 
-	if (readl(base + MMCICOMMAND) & MCI_CPSM_ENABLE) {
-		writel(0, base + MMCICOMMAND);
+	if (mmci_readl(host, MMCICOMMAND) & MCI_CPSM_ENABLE) {
+		mmci_writel(host, 0, MMCICOMMAND);
 		udelay(1);
 	}
 
@@ -859,8 +866,8 @@ mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c)
 
 	host->cmd = cmd;
 
-	writel(cmd->arg, base + MMCIARGUMENT);
-	writel(c, base + MMCICOMMAND);
+	mmci_writel(host, cmd->arg, MMCIARGUMENT);
+	mmci_writel(host, c, MMCICOMMAND);
 }
 
 static void
@@ -885,7 +892,7 @@ mmci_data_irq(struct mmci_host *host, struct mmc_data *data,
 		 * can be as much as a FIFO-worth of data ahead.  This
 		 * matters for FIFO overruns only.
 		 */
-		remain = readl(host->base + MMCIDATACNT);
+		remain = mmci_readl(host, MMCIDATACNT);
 		success = data->blksz * data->blocks - remain;
 
 		dev_dbg(mmc_dev(host->mmc), "MCI ERROR IRQ, status 0x%08x at 0x%08x\n",
@@ -967,10 +974,10 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd,
 	} else if (status & MCI_CMDCRCFAIL && cmd->flags & MMC_RSP_CRC) {
 		cmd->error = -EILSEQ;
 	} else {
-		cmd->resp[0] = readl(base + MMCIRESPONSE0);
-		cmd->resp[1] = readl(base + MMCIRESPONSE1);
-		cmd->resp[2] = readl(base + MMCIRESPONSE2);
-		cmd->resp[3] = readl(base + MMCIRESPONSE3);
+		cmd->resp[0] = mmci_readl(host, MMCIRESPONSE0);
+		cmd->resp[1] = mmci_readl(host, MMCIRESPONSE1);
+		cmd->resp[2] = mmci_readl(host, MMCIRESPONSE2);
+		cmd->resp[3] = mmci_readl(host, MMCIRESPONSE3);
 	}
 
 	if ((!sbc && !cmd->data) || cmd->error) {
@@ -1081,11 +1088,10 @@ static irqreturn_t mmci_pio_irq(int irq, void *dev_id)
 	struct mmci_host *host = dev_id;
 	struct sg_mapping_iter *sg_miter = &host->sg_miter;
 	struct variant_data *variant = host->variant;
-	void __iomem *base = host->base;
 	unsigned long flags;
 	u32 status;
 
-	status = readl(base + MMCISTATUS);
+	status = mmci_readl(host, MMCISTATUS);
 
 	dev_dbg(mmc_dev(host->mmc), "irq1 (pio) %08x\n", status);
 
@@ -1125,7 +1131,7 @@ static irqreturn_t mmci_pio_irq(int irq, void *dev_id)
 		if (remain)
 			break;
 
-		status = readl(base + MMCISTATUS);
+		status = mmci_readl(host, MMCISTATUS);
 	} while (1);
 
 	sg_miter_stop(sg_miter);
@@ -1147,7 +1153,9 @@ static irqreturn_t mmci_pio_irq(int irq, void *dev_id)
 	 */
 	if (host->size == 0) {
 		mmci_set_mask1(host, 0);
-		writel(readl(base + MMCIMASK0) | MCI_DATAENDMASK, base + MMCIMASK0);
+		mmci_writel(host,
+			    mmci_readl(host, MMCIMASK0) | MCI_DATAENDMASK,
+			    MMCIMASK0);
 	}
 
 	return IRQ_HANDLED;
@@ -1168,10 +1176,10 @@ static irqreturn_t mmci_irq(int irq, void *dev_id)
 		struct mmc_command *cmd;
 		struct mmc_data *data;
 
-		status = readl(host->base + MMCISTATUS);
+		status = mmci_readl(host, MMCISTATUS);
 
 		if (host->singleirq) {
-			if (status & readl(host->base + MMCIMASK1))
+			if (status & mmci_readl(host, MMCIMASK1))
 				mmci_pio_irq(irq, dev_id);
 
 			status &= ~MCI_IRQ1MASK;
@@ -1182,8 +1190,8 @@ static irqreturn_t mmci_irq(int irq, void *dev_id)
 		 * enabled) since the HW seems to be triggering the IRQ on both
 		 * edges while monitoring DAT0 for busy completion.
 		 */
-		status &= readl(host->base + MMCIMASK0);
-		writel(status, host->base + MMCICLEAR);
+		status &= mmci_readl(host, MMCIMASK0);
+		mmci_writel(host, status, MMCICLEAR);
 
 		dev_dbg(mmc_dev(host->mmc), "irq0 (data+cmd) %08x\n", status);
 
@@ -1627,9 +1635,9 @@ static int mmci_probe(struct amba_device *dev,
 
 	spin_lock_init(&host->lock);
 
-	writel(0, host->base + MMCIMASK0);
-	writel(0, host->base + MMCIMASK1);
-	writel(0xfff, host->base + MMCICLEAR);
+	mmci_writel(host, 0, MMCIMASK0);
+	mmci_writel(host, 0, MMCIMASK1);
+	mmci_writel(host, 0xfff, MMCICLEAR);
 
 	if (plat->gpio_cd == -EPROBE_DEFER) {
 		ret = -EPROBE_DEFER;
@@ -1689,7 +1697,7 @@ static int mmci_probe(struct amba_device *dev,
 			goto irq0_free;
 	}
 
-	writel(MCI_IRQENABLE, host->base + MMCIMASK0);
+	mmci_writel(host, MCI_IRQENABLE, MMCIMASK0);
 
 	amba_set_drvdata(dev, mmc);
 
@@ -1745,11 +1753,11 @@ static int mmci_remove(struct amba_device *dev)
 
 		mmc_remove_host(mmc);
 
-		writel(0, host->base + MMCIMASK0);
-		writel(0, host->base + MMCIMASK1);
+		mmci_writel(host, 0, MMCIMASK0);
+		mmci_writel(host, 0, MMCIMASK1);
 
-		writel(0, host->base + MMCICOMMAND);
-		writel(0, host->base + MMCIDATACTRL);
+		mmci_writel(host, 0, MMCICOMMAND);
+		mmci_writel(host, 0, MMCIDATACTRL);
 
 		mmci_dma_release(host);
 		free_irq(dev->irq[0], host);
@@ -1783,7 +1791,7 @@ static int mmci_suspend(struct device *dev)
 	if (mmc) {
 		struct mmci_host *host = mmc_priv(mmc);
 		pm_runtime_get_sync(dev);
-		writel(0, host->base + MMCIMASK0);
+		mmci_writel(host, 0, MMCIMASK0);
 	}
 
 	return 0;
@@ -1796,7 +1804,7 @@ static int mmci_resume(struct device *dev)
 
 	if (mmc) {
 		struct mmci_host *host = mmc_priv(mmc);
-		writel(MCI_IRQENABLE, host->base + MMCIMASK0);
+		mmci_writel(host, MCI_IRQENABLE, MMCIMASK0);
 		pm_runtime_put(dev);
 	}
 
@@ -1812,10 +1820,10 @@ static void mmci_save(struct mmci_host *host)
 	if (host->variant->pwrreg_nopower) {
 		spin_lock_irqsave(&host->lock, flags);
 
-		writel(0, host->base + MMCIMASK0);
-		writel(0, host->base + MMCIDATACTRL);
-		writel(0, host->base + MMCIPOWER);
-		writel(0, host->base + MMCICLOCK);
+		mmci_writel(host, 0, MMCIMASK0);
+		mmci_writel(host, 0, MMCIDATACTRL);
+		mmci_writel(host, 0, MMCIPOWER);
+		mmci_writel(host, 0, MMCICLOCK);
 		mmci_reg_delay(host);
 
 		spin_unlock_irqrestore(&host->lock, flags);
@@ -1830,10 +1838,10 @@ static void mmci_restore(struct mmci_host *host)
 	if (host->variant->pwrreg_nopower) {
 		spin_lock_irqsave(&host->lock, flags);
 
-		writel(host->clk_reg, host->base + MMCICLOCK);
-		writel(host->datactrl_reg, host->base + MMCIDATACTRL);
-		writel(host->pwr_reg, host->base + MMCIPOWER);
-		writel(MCI_IRQENABLE, host->base + MMCIMASK0);
+		mmci_writel(host, host->clk_reg, MMCICLOCK);
+		mmci_writel(host, host->datactrl_reg, MMCIDATACTRL);
+		mmci_writel(host, host->pwr_reg, MMCIPOWER);
+		mmci_writel(host, MCI_IRQENABLE, MMCIMASK0);
 		mmci_reg_delay(host);
 
 		spin_unlock_irqrestore(&host->lock, flags);
-- 
1.7.9.5

[PATCH v1 05/11] mmc: mmci: use NSEC_PER_SEC macro

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

This patch replaces a constant used in calculating timeout with a proper
macro. This is make code more readable.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/mmc/host/mmci.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 36db31e..4f8d0ba 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -759,7 +759,7 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 	data->bytes_xfered = 0;
 
 	clks = (unsigned long long)data->timeout_ns * host->cclk;
-	do_div(clks, 1000000000UL);
+	do_div(clks, NSEC_PER_SEC);
 
 	timeout = data->timeout_clks + (unsigned int)clks;
 
-- 
1.7.9.5

Re: [PATCH v1 05/11] mmc: mmci: use NSEC_PER_SEC macro

[Linus Walleij]

On Tue, Apr 29, 2014 at 10:20 AM,  <srinivas.kandagatla@linaro.org> wrote:

> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> This patch replaces a constant used in calculating timeout with a proper
> macro. This is make code more readable.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

This can be merged out-of-order just as-is.

Yours,
Linus Walleij

Re: [PATCH v1 05/11] mmc: mmci: use NSEC_PER_SEC macro

[Srinivas Kandagatla]

On 13/05/14 08:20, Linus Walleij wrote:
> On Tue, Apr 29, 2014 at 10:20 AM,  <srinivas.kandagatla@linaro.org> wrote:
>
>> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>
>> This patch replaces a constant used in calculating timeout with a proper
>> macro. This is make code more readable.
>>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>
> This can be merged out-of-order just as-is.
Agreed, I can take this patch out of this series.

thanks,
srini
>
> Yours,
> Linus Walleij
>

[PATCH v1 06/11] mmc: mmci: Qcomm: Add 3 clock cycle delay after register write

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Most of the Qcomm SD card controller registers must be updated to the MCLK
domain so subsequent writes to registers will be ignored until 3 clock cycles
have passed.

This patch adds a 3 clock cycle delay required after writing to controller
registers on Qualcomm SOCs. Without this delay all the register writes are not
successfull, resulting in not detecting cards.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/mmc/host/mmci.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 4f8d0ba..f73dc48 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -179,6 +179,14 @@ static inline u32 mmci_readl(struct mmci_host *host, u32 off)
 static inline void mmci_writel(struct mmci_host *host, u32 data, u32 off)
 {
 	writel(data, host->base + off);
+
+	/*
+	 * On QCom SD card controller, registers must be updated to the
+	 * MCLK domain so subsequent writes to this register will be ignored
+	 * for 3 clk cycles.
+	 */
+	if (host->hw_designer == AMBA_VENDOR_QCOM)
+		udelay(1 + ((3 * USEC_PER_SEC)/host->mclk));
 }
 
 static int mmci_card_busy(struct mmc_host *mmc)
-- 
1.7.9.5

Re: [PATCH v1 06/11] mmc: mmci: Qcomm: Add 3 clock cycle delay after register write

[Linus Walleij]

On Tue, Apr 29, 2014 at 10:20 AM,  <srinivas.kandagatla@linaro.org> wrote:

> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> Most of the Qcomm SD card controller registers must be updated to the MCLK
> domain so subsequent writes to registers will be ignored until 3 clock cycles
> have passed.
>
> This patch adds a 3 clock cycle delay required after writing to controller
> registers on Qualcomm SOCs. Without this delay all the register writes are not
> successfull, resulting in not detecting cards.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Sounds like someone decided to clock the internal state machine
in the MMCI using MCLK instead of PCLK :-(

A bit nasty if this ends up in the fastpath (irq) though. Which it
invariably does, right?

> +       /*
> +        * On QCom SD card controller, registers must be updated to the
> +        * MCLK domain so subsequent writes to this register will be ignored
> +        * for 3 clk cycles.
> +        */
> +       if (host->hw_designer == AMBA_VENDOR_QCOM)
> +               udelay(1 + ((3 * USEC_PER_SEC)/host->mclk));

Add a new field in vendor data instead, and use DIV_ROUND_UP():

static struct variant_data variant_qcom = {
         .mclk_delayed_writes  = true,
(...)

if (host->vendor->mclk_delayed_writes)
    udelay(DIV_ROUND_UP((3 * USEC_PER_SEC), host->mclk));

You get the idea.

Yours,
Linus Walleij

Re: [PATCH v1 06/11] mmc: mmci: Qcomm: Add 3 clock cycle delay after register write

[Srinivas Kandagatla]

Thanks Linus W for reviewing the patches.

On 13/05/14 08:29, Linus Walleij wrote:
> On Tue, Apr 29, 2014 at 10:20 AM,  <srinivas.kandagatla@linaro.org> wrote:
>
>> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>
>> Most of the Qcomm SD card controller registers must be updated to the MCLK
>> domain so subsequent writes to registers will be ignored until 3 clock cycles
>> have passed.
>>
>> This patch adds a 3 clock cycle delay required after writing to controller
>> registers on Qualcomm SOCs. Without this delay all the register writes are not
>> successfull, resulting in not detecting cards.
>>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> Sounds like someone decided to clock the internal state machine
> in the MMCI using MCLK instead of PCLK :-(
>
> A bit nasty if this ends up in the fastpath (irq) though. Which it
> invariably does, right?
yes, Its going to for Qcom SOC.
>
>> +       /*
>> +        * On QCom SD card controller, registers must be updated to the
>> +        * MCLK domain so subsequent writes to this register will be ignored
>> +        * for 3 clk cycles.
>> +        */
>> +       if (host->hw_designer == AMBA_VENDOR_QCOM)
>> +               udelay(1 + ((3 * USEC_PER_SEC)/host->mclk));
>
> Add a new field in vendor data instead, and use DIV_ROUND_UP():
>
yes, that makes sense..

> static struct variant_data variant_qcom = {
>           .mclk_delayed_writes  = true,
> (...)
>
> if (host->vendor->mclk_delayed_writes)
>      udelay(DIV_ROUND_UP((3 * USEC_PER_SEC), host->mclk));
>
> You get the idea.
>
Got it.

> Yours,
> Linus Walleij
>

[PATCH v1 07/11] mmc: mmci: move ST specific register extensions access under condition.

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

This patch moves some of the ST specific register extensions access under
condition, so that other SOCs like Qualcomm or ARM would not a side effect of
writing to those reserved/different purpose bits.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/mmc/host/mmci.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index f73dc48..306e0c8 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -269,7 +269,8 @@ static void mmci_write_pwrreg(struct mmci_host *host, u32 pwr)
 static void mmci_write_datactrlreg(struct mmci_host *host, u32 datactrl)
 {
 	/* Keep ST Micro busy mode if enabled */
-	datactrl |= host->datactrl_reg & MCI_ST_DPSM_BUSYMODE;
+	if (host->hw_designer == AMBA_VENDOR_ST)
+		datactrl |= host->datactrl_reg & MCI_ST_DPSM_BUSYMODE;
 
 	if (host->datactrl_reg != datactrl) {
 		host->datactrl_reg = datactrl;
@@ -815,7 +816,8 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 			mmci_write_clkreg(host, clk);
 		}
 
-	if (host->mmc->ios.timing == MMC_TIMING_UHS_DDR50)
+	if (host->mmc->ios.timing == MMC_TIMING_UHS_DDR50 &&
+	    host->hw_designer == AMBA_VENDOR_ST)
 		datactrl |= MCI_ST_DPSM_DDRMODE;
 
 	/*
-- 
1.7.9.5

Re: [PATCH v1 07/11] mmc: mmci: move ST specific register extensions access under condition.

[Linus Walleij]

On Tue, Apr 29, 2014 at 10:20 AM,  <srinivas.kandagatla@linaro.org> wrote:

> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> This patch moves some of the ST specific register extensions access under
> condition, so that other SOCs like Qualcomm or ARM would not a side effect of
> writing to those reserved/different purpose bits.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
(...)

>         /* Keep ST Micro busy mode if enabled */
> -       datactrl |= host->datactrl_reg & MCI_ST_DPSM_BUSYMODE;
> +       if (host->hw_designer == AMBA_VENDOR_ST)
> +               datactrl |= host->datactrl_reg & MCI_ST_DPSM_BUSYMODE;

Do not hard-check the hw_designer everywhere, follow the pattern
if storing special stuff in the variant data.

struct variant_data {
        u32 datactrl_mask_busymode;
(...)

static struct variant_data variant_u300 = {
        .datactrl_mask_busymode = MCI_ST_DPSM_BUSYMODE,
(...)
static struct variant_data variant_nomadik = {
        .datactrl_mask_busymode = MCI_ST_DPSM_BUSYMODE,
(...)
static struct variant_data variant_ux500 = {
        .datactrl_mask_busymode = MCI_ST_DPSM_BUSYMODE,
(...)
static struct variant_data variant_ux500v2 = {
        .datactrl_mask_busymode = MCI_ST_DPSM_BUSYMODE,
(...)

Then end up like this:

>         /* Keep ST Micro busy mode if enabled */
> -       datactrl |= host->datactrl_reg & MCI_ST_DPSM_BUSYMODE;
> +      datactrl |= host->datactrl_reg & host->vendor->datactrl_mask_busymode;

OK we know this should have been done like this from the beginning
but that is not an excuse not to fix it up now.

> -       if (host->mmc->ios.timing == MMC_TIMING_UHS_DDR50)
> +       if (host->mmc->ios.timing == MMC_TIMING_UHS_DDR50 &&
> +           host->hw_designer == AMBA_VENDOR_ST)
>                 datactrl |= MCI_ST_DPSM_DDRMODE;

Same pattern here.

Actually I think this is only available on Ux500v2 (Ulf? Can you verify
this) so it is probably plain wrong to do it for other variants.

struct variant_data {
        u32 datactrl_ddrmode;
(...)
static struct variant_data variant_ux500v2 = {
        .datactrl_ddrmode = MCI_ST_DPSM_DDRMODE,
(...)

Then end up like this:

>        if (host->mmc->ios.timing == MMC_TIMING_UHS_DDR50)
> -                datactrl |= MCI_ST_DPSM_DDRMODE;
> +               datactrl |= host->vendor->datactrl_ddrmode;

Yours,
Linus Walleij

Re: [PATCH v1 07/11] mmc: mmci: move ST specific register extensions access under condition.

[Srinivas Kandagatla]

On 13/05/14 09:08, Linus Walleij wrote:
>> >         /* Keep ST Micro busy mode if enabled */
>> >-       datactrl |= host->datactrl_reg & MCI_ST_DPSM_BUSYMODE;
>> >+       if (host->hw_designer == AMBA_VENDOR_ST)
>> >+               datactrl |= host->datactrl_reg & MCI_ST_DPSM_BUSYMODE;
> Do not hard-check the hw_designer everywhere, follow the pattern
> if storing special stuff in the variant data.
>
Got it, I will fix this across other patches too.

> struct variant_data {
>          u32 datactrl_mask_busymode;
> (...)
>
> static struct variant_data variant_u300 = {
>          .datactrl_mask_busymode = MCI_ST_DPSM_BUSYMODE,
> (...)
> static struct variant_data variant_nomadik = {

[PATCH v1 08/11] mmc: mmci: Qcom fix MCICLK register settings.

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

MCICLK register layout is bit different to the standard pl180 register layout.
Qcom SDCC controller some setup in MCICLK register to get it going. So this
patch adds new setup and makes it specific to Qcom hw designer.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/mmc/host/mmci.c |   36 ++++++++++++++++++++++++++++++------
 drivers/mmc/host/mmci.h |   21 +++++++++++++++++++++
 2 files changed, 51 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 306e0c8..35aed38 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -326,13 +326,37 @@ static void mmci_set_clkreg(struct mmci_host *host, unsigned int desired)
 	/* Set actual clock for debug */
 	host->mmc->actual_clock = host->cclk;
 
-	if (host->mmc->ios.bus_width == MMC_BUS_WIDTH_4)
-		clk |= MCI_4BIT_BUS;
-	if (host->mmc->ios.bus_width == MMC_BUS_WIDTH_8)
-		clk |= MCI_ST_8BIT_BUS;
+	if (host->hw_designer == AMBA_VENDOR_QCOM) {
+		clk |= MCI_CLK_QCOM_FLOWENA;
+		clk |= (MCI_CLK_QCOM_SEL_FEEDBACK_CLK <<
+				MCI_CLK_QCOM_SEL_IN_SHIFT); /* feedback clk */
+		if (host->mmc->ios.bus_width == MMC_BUS_WIDTH_8)
+			clk |= MCI_CLK_QCOM_WIDEBUS_8;
+		else if (host->mmc->ios.bus_width == MMC_BUS_WIDTH_4)
+			clk |= MCI_CLK_QCOM_WIDEBUS_4;
+		else
+			clk |= MCI_CLK_QCOM_WIDEBUS_1;
+
+		if (host->mmc->ios.timing == MMC_TIMING_UHS_DDR50) {
+			/* clear SELECT_IN field */
+			clk &= ~(MCI_CLK_QCOM_SEL_MASK <<
+					MCI_CLK_QCOM_SEL_IN_SHIFT);
+			/* set DDR timing mode */
+			clk |= (MCI_CLK_QCOM_SEL_DDR_MODE <<
+					MCI_CLK_QCOM_SEL_IN_SHIFT);
+		}
+		clk |= (MCI_CLK_SDC4_MCLK_SEL_MCLK <<
+				MCI_CLK_SDC4_MCLK_SEL_SHIFT);
 
-	if (host->mmc->ios.timing == MMC_TIMING_UHS_DDR50)
-		clk |= MCI_ST_UX500_NEG_EDGE;
+	} else {
+		if (host->mmc->ios.bus_width == MMC_BUS_WIDTH_4)
+			clk |= MCI_4BIT_BUS;
+		if (host->mmc->ios.bus_width == MMC_BUS_WIDTH_8)
+			clk |= MCI_ST_8BIT_BUS;
+
+		if (host->mmc->ios.timing == MMC_TIMING_UHS_DDR50)
+			clk |= MCI_ST_UX500_NEG_EDGE;
+	}
 
 	mmci_write_clkreg(host, clk);
 }
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 58b1b88..0a6de1c 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -31,6 +31,27 @@
 /* Modified PL180 on Versatile Express platform */
 #define MCI_ARM_HWFCEN		(1 << 12)
 
+/* Modified on Qualcomm Integrations */
+#define MCI_CLK_QCOM_WIDEBUS_1	(0 << 10)
+#define MCI_CLK_QCOM_WIDEBUS_4	(2 << 10)
+#define MCI_CLK_QCOM_WIDEBUS_8	(3 << 10)
+#define MCI_CLK_QCOM_FLOWENA	(1 << 12)
+#define MCI_CLK_QCOM_INVERTOUT	(1 << 13)
+
+/* select in latch data and command */
+#define MCI_CLK_QCOM_SEL_IN_SHIFT	(14)
+#define MCI_CLK_QCOM_SEL_MASK		(0x3)
+#define MCI_CLK_QCOM_SEL_RISING_EDGE	(1)
+#define MCI_CLK_QCOM_SEL_FEEDBACK_CLK	(2)
+#define MCI_CLK_QCOM_SEL_DDR_MODE	(3)
+
+/* mclk selection */
+#define MCI_CLK_SDC4_MCLK_SEL_SHIFT	(23)
+#define MCI_CLK_SDC4_MCLK_SEL_MASK	(0x3)
+#define MCI_CLK_SDC4_MCLK_SEL_FB_CLK	(1)
+#define MCI_CLK_SDC4_MCLK_SEL_MCLK	(2)
+
+
 #define MMCIARGUMENT		0x008
 #define MMCICOMMAND		0x00c
 #define MCI_CPSM_RESPONSE	(1 << 6)
-- 
1.7.9.5

Re: [PATCH v1 08/11] mmc: mmci: Qcom fix MCICLK register settings.

[Linus Walleij]

On Tue, Apr 29, 2014 at 10:20 AM,  <srinivas.kandagatla@linaro.org> wrote:

> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> MCICLK register layout is bit different to the standard pl180 register layout.
> Qcom SDCC controller some setup in MCICLK register to get it going. So this
> patch adds new setup and makes it specific to Qcom hw designer.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
(...)

> -       if (host->mmc->ios.bus_width == MMC_BUS_WIDTH_4)
> -               clk |= MCI_4BIT_BUS;
> -       if (host->mmc->ios.bus_width == MMC_BUS_WIDTH_8)
> -               clk |= MCI_ST_8BIT_BUS;
> +       if (host->hw_designer == AMBA_VENDOR_QCOM) {
> +               clk |= MCI_CLK_QCOM_FLOWENA;
> +               clk |= (MCI_CLK_QCOM_SEL_FEEDBACK_CLK <<
> +                               MCI_CLK_QCOM_SEL_IN_SHIFT); /* feedback clk */
> +               if (host->mmc->ios.bus_width == MMC_BUS_WIDTH_8)
> +                       clk |= MCI_CLK_QCOM_WIDEBUS_8;
> +               else if (host->mmc->ios.bus_width == MMC_BUS_WIDTH_4)
> +                       clk |= MCI_CLK_QCOM_WIDEBUS_4;
> +               else
> +                       clk |= MCI_CLK_QCOM_WIDEBUS_1;
> +
> +               if (host->mmc->ios.timing == MMC_TIMING_UHS_DDR50) {
> +                       /* clear SELECT_IN field */
> +                       clk &= ~(MCI_CLK_QCOM_SEL_MASK <<
> +                                       MCI_CLK_QCOM_SEL_IN_SHIFT);
> +                       /* set DDR timing mode */
> +                       clk |= (MCI_CLK_QCOM_SEL_DDR_MODE <<
> +                                       MCI_CLK_QCOM_SEL_IN_SHIFT);
> +               }
> +               clk |= (MCI_CLK_SDC4_MCLK_SEL_MCLK <<
> +                               MCI_CLK_SDC4_MCLK_SEL_SHIFT);
>
> -       if (host->mmc->ios.timing == MMC_TIMING_UHS_DDR50)
> -               clk |= MCI_ST_UX500_NEG_EDGE;
> +       } else {
> +               if (host->mmc->ios.bus_width == MMC_BUS_WIDTH_4)
> +                       clk |= MCI_4BIT_BUS;
> +               if (host->mmc->ios.bus_width == MMC_BUS_WIDTH_8)
> +                       clk |= MCI_ST_8BIT_BUS;
> +
> +               if (host->mmc->ios.timing == MMC_TIMING_UHS_DDR50)
> +                       clk |= MCI_ST_UX500_NEG_EDGE;
> +       }

Again follow the pattern of storing register templates in the vendor_data
struct. I think you will quickly realize how this can be cut down with
new fields like .clk_4bitmode etc.

>  /* Modified PL180 on Versatile Express platform */
>  #define MCI_ARM_HWFCEN         (1 << 12)
>
> +/* Modified on Qualcomm Integrations */

First: follow the convention set for the ST-specific registers that
look e.g. like this:

MCI_ST_8BIT_BUS i.e. MCI_<vendor>_SPECIFIER
So the below becomes MCI_QCOM_CLK_WIDEBUS... etc.

> +#define MCI_CLK_QCOM_WIDEBUS_1 (0 << 10)
> +#define MCI_CLK_QCOM_WIDEBUS_4 (2 << 10)
> +#define MCI_CLK_QCOM_WIDEBUS_8 (3 << 10)

Compare to what we have:

#define MMCICLOCK               0x004
#define MCI_CLK_ENABLE          (1 << 8)
#define MCI_CLK_PWRSAVE         (1 << 9)
#define MCI_CLK_BYPASS          (1 << 10)
#define MCI_4BIT_BUS            (1 << 11)

MCI_CLK_QCOM_WIDEBUS_1 is clearly surplus since it is
not setting any bit.

MCI_CLK_QCOM_WIDEBUS_4 (2 << 10) ==
MCI_4BIT_BUS (1 <<11), this is the same thing just
expressed in two ways! No need to have some surplus
definition adding to the confusion. Just use MCI_4BIT_BUS

The only thing that is really different is
MCI_CLK_QCOM_WIDEBUS_8 which whould have
the name (following the sibling definitions for the ST block):

#define MCI_QCOM_8BIT_BUS (3 << 10)

> +#define MCI_CLK_QCOM_FLOWENA   (1 << 12)

Rename:
MCI_QCOM_CLK_HWFCEN (1<<13)

to match sibling definitions. It's clear that this enabled
hardware flow control and that name is more helpful to
understand what is going on.

> +#define MCI_CLK_QCOM_INVERTOUT (1 << 13)

Rename:
MCI_QCOM_CLK_INV
To match siblings.

> +/* select in latch data and command */
> +#define MCI_CLK_QCOM_SEL_IN_SHIFT      (14)
> +#define MCI_CLK_QCOM_SEL_MASK          (0x3)
> +#define MCI_CLK_QCOM_SEL_RISING_EDGE   (1)
> +#define MCI_CLK_QCOM_SEL_FEEDBACK_CLK  (2)
> +#define MCI_CLK_QCOM_SEL_DDR_MODE      (3)
> +
> +/* mclk selection */
> +#define MCI_CLK_SDC4_MCLK_SEL_SHIFT    (23)
> +#define MCI_CLK_SDC4_MCLK_SEL_MASK     (0x3)
> +#define MCI_CLK_SDC4_MCLK_SEL_FB_CLK   (1)
> +#define MCI_CLK_SDC4_MCLK_SEL_MCLK     (2)

These seem to have no related siblings but should still be
renamed MCL_QCOM_*

Yours,
Linus Walleij

Re: [PATCH v1 08/11] mmc: mmci: Qcom fix MCICLK register settings.

[Srinivas Kandagatla]

Thanks Linus W.

On 13/05/14 09:19, Linus Walleij wrote:
> Again follow the pattern of storing register templates in the vendor_data
> struct. I think you will quickly realize how this can be cut down with
> new fields like .clk_4bitmode etc.
>
>> >  /* Modified PL180 on Versatile Express platform */
>> >  #define MCI_ARM_HWFCEN         (1 << 12)
>> >
>> >+/* Modified on Qualcomm Integrations */
> First: follow the convention set for the ST-specific registers that
> look e.g. like this:
I agree, Will fix these in next version.
>
> MCI_ST_8BIT_BUS i.e. MCI_<vendor>_SPECIFIER
> So the below becomes MCI_QCOM_CLK_WIDEBUS... etc.
>

[PATCH v1 09/11] mmc: mmci: Add clock support for Qualcomm.

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

MCICLK going to card bus is directly driven by the clock controller, so the
driver has to set the required rates depending on the state of the card. This
bit of support is very much similar to bypass mode but there is no such thing
called bypass mode in MCICLK register of Qcom SD card controller. By default
the clock is directly driven by the clk controller.

This patch adds clock support for Qualcomm SDCC in the driver. This bit of
code is conditioned on hw designer.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/mmc/host/mmci.c |   21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 35aed38..da135c0 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -290,7 +290,10 @@ static void mmci_set_clkreg(struct mmci_host *host, unsigned int desired)
 	host->cclk = 0;
 
 	if (desired) {
-		if (desired >= host->mclk) {
+
+		if (host->hw_designer == AMBA_VENDOR_QCOM) {
+			host->cclk = host->mclk;
+		} else if (desired >= host->mclk) {
 			clk = MCI_CLK_BYPASS;
 			if (variant->st_clkdiv)
 				clk |= MCI_ST_UX500_NEG_EDGE;
@@ -1371,6 +1374,19 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	if (!ios->clock && variant->pwrreg_clkgate)
 		pwr &= ~MCI_PWR_ON;
 
+	if (ios->clock != host->mclk &&
+		host->hw_designer == AMBA_VENDOR_QCOM) {
+		/* Qcom MCLKCLK register does not define bypass bits */
+		int rc = clk_set_rate(host->clk, ios->clock);
+		if (rc < 0) {
+			dev_err(mmc_dev(host->mmc),
+				"Error setting clock rate (%d)\n", rc);
+		} else {
+			host->mclk = clk_get_rate(host->clk);
+			host->cclk = host->mclk;
+		}
+	}
+
 	spin_lock_irqsave(&host->lock, flags);
 
 	mmci_set_clkreg(host, ios->clock);
@@ -1611,7 +1627,8 @@ static int mmci_probe(struct amba_device *dev,
 	 * of course.
 	 */
 	if (plat->f_max)
-		mmc->f_max = min(host->mclk, plat->f_max);
+		mmc->f_max = (host->hw_designer == AMBA_VENDOR_QCOM) ?
+				plat->f_max : min(host->mclk, plat->f_max);
 	else
 		mmc->f_max = min(host->mclk, fmax);
 	dev_dbg(mmc_dev(mmc), "clocking block at %u Hz\n", mmc->f_max);
-- 
1.7.9.5

Re: [PATCH v1 09/11] mmc: mmci: Add clock support for Qualcomm.

[Linus Walleij]

On Tue, Apr 29, 2014 at 10:20 AM,  <srinivas.kandagatla@linaro.org> wrote:

> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> MCICLK going to card bus is directly driven by the clock controller, so the
> driver has to set the required rates depending on the state of the card. This
> bit of support is very much similar to bypass mode but there is no such thing
> called bypass mode in MCICLK register of Qcom SD card controller. By default
> the clock is directly driven by the clk controller.
>
> This patch adds clock support for Qualcomm SDCC in the driver. This bit of
> code is conditioned on hw designer.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
(...)
> +               if (host->hw_designer == AMBA_VENDOR_QCOM) {
> +                       host->cclk = host->mclk;
> +               } else if (desired >= host->mclk) {

Again refrain from hard-checking the vendor everywhere.

struct variant_data {
        bool            qcom_cclk_is_mclk;
(...)

As per example from st_clkdiv...

Then

if (host->vendor->qcom_cclk_is_mclk) {
  (...)
}

> +       if (ios->clock != host->mclk &&
> +               host->hw_designer == AMBA_VENDOR_QCOM) {
> +               /* Qcom MCLKCLK register does not define bypass bits */
> +               int rc = clk_set_rate(host->clk, ios->clock);
> +               if (rc < 0) {
> +                       dev_err(mmc_dev(host->mmc),
> +                               "Error setting clock rate (%d)\n", rc);
> +               } else {
> +                       host->mclk = clk_get_rate(host->clk);
> +                       host->cclk = host->mclk;
> +               }
> +       }

For this I would define a vendor data like:

struct variant_data {
        bool            explicit_mclk_control;
(...)

Or something. It explains what is actually going on.

>         if (plat->f_max)
> -               mmc->f_max = min(host->mclk, plat->f_max);
> +               mmc->f_max = (host->hw_designer == AMBA_VENDOR_QCOM) ?
> +                               plat->f_max : min(host->mclk, plat->f_max);

So rewrite like that:

if (host->vendor->explicit_mclk_control)
       mmc->f_max = plat->f_max;
else
      mmc->f_max = min(host->mclk, plat->f_max);

Yours,
Linus Walleij

Re: [PATCH v1 09/11] mmc: mmci: Add clock support for Qualcomm.

[Srinivas Kandagatla]

Thanks Linus W,

On 13/05/14 09:28, Linus Walleij wrote:
>> code is conditioned on hw designer.
>>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> (...)
>> +               if (host->hw_designer == AMBA_VENDOR_QCOM) {
>> +                       host->cclk = host->mclk;
>> +               } else if (desired >= host->mclk) {
>
> Again refrain from hard-checking the vendor everywhere.
>
> struct variant_data {
>          bool            qcom_cclk_is_mclk;
> (...)
>
Got it.. Will fix it in next version.
> As per example from st_clkdiv...
>
> Then
>
> if (host->vendor->qcom_cclk_is_mclk) {
>    (...)
> }
>
>> +       if (ios->clock != host->mclk &&
>> +               host->hw_designer == AMBA_VENDOR_QCOM) {
>> +               /* Qcom MCLKCLK register does not define bypass bits */
>> +               int rc = clk_set_rate(host->clk, ios->clock);
>> +               if (rc < 0) {
>> +                       dev_err(mmc_dev(host->mmc),
>> +                               "Error setting clock rate (%d)\n", rc);
>> +               } else {
>> +                       host->mclk = clk_get_rate(host->clk);
>> +                       host->cclk = host->mclk;
>> +               }
>> +       }
>
> For this I would define a vendor data like:
>
> struct variant_data {
>          bool            explicit_mclk_control;
> (...)
This looks good.
>
> Or something. It explains what is actually going on.
>
>>          if (plat->f_max)
>> -               mmc->f_max = min(host->mclk, plat->f_max);
>> +               mmc->f_max = (host->hw_designer == AMBA_VENDOR_QCOM) ?
>> +                               plat->f_max : min(host->mclk, plat->f_max);
>
> So rewrite like that:
>
> if (host->vendor->explicit_mclk_control)
>         mmc->f_max = plat->f_max;
> else
>        mmc->f_max = min(host->mclk, plat->f_max);
>

This looks much clean


> Yours,
> Linus Walleij
>

[PATCH v1 10/11] mmc: mmci: Add Qcom variations to MCICommand register.

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Some bits which control Command Path State Machine (CPSM) are new in Qcom
integration, so this patch adds support to those bits.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/mmc/host/mmci.c |    4 ++++
 drivers/mmc/host/mmci.h |    7 +++++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index da135c0..def1b19 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -901,6 +901,10 @@ mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c)
 	if (/*interrupt*/0)
 		c |= MCI_CPSM_INTERRUPT;
 
+	if (host->hw_designer == AMBA_VENDOR_QCOM &&
+	    mmc_cmd_type(cmd) == MMC_CMD_ADTC)
+		c |= MCI_CSPM_QCOM_DATCMD;
+
 	host->cmd = cmd;
 
 	mmci_writel(host, cmd->arg, MMCIARGUMENT);
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 0a6de1c..2ba0834 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -64,6 +64,13 @@
 #define MCI_ST_ENCMD_COMPL	(1 << 12)
 #define MCI_ST_NIEN		(1 << 13)
 #define MCI_ST_CE_ATACMD	(1 << 14)
+/* Modified on Qualcomm Integrations */
+#define MCI_CSPM_QCOM_DATCMD		(1 << 12)
+#define MCI_CSPM_QCOM_MCIABORT		(1 << 13)
+#define MCI_CSPM_QCOM_CCSENABLE		(1 << 14)
+#define MCI_CSPM_QCOM_CCSDISABLE	(1 << 15)
+#define MCI_CSPM_QCOM_AUTO_CMD19	(1 << 16)
+#define MCI_CSPM_QCOM_AUTO_CMD21	(1 << 21)
 
 #define MMCIRESPCMD		0x010
 #define MMCIRESPONSE0		0x014
-- 
1.7.9.5

Re: [PATCH v1 10/11] mmc: mmci: Add Qcom variations to MCICommand register.

[Linus Walleij]

On Tue, Apr 29, 2014 at 10:21 AM,  <srinivas.kandagatla@linaro.org> wrote:

> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> Some bits which control Command Path State Machine (CPSM) are new in Qcom
> integration, so this patch adds support to those bits.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
(...)

> +       if (host->hw_designer == AMBA_VENDOR_QCOM &&
> +           mmc_cmd_type(cmd) == MMC_CMD_ADTC)
> +               c |= MCI_CSPM_QCOM_DATCMD;
> +

You know the drill. Use vendor data.

> +/* Modified on Qualcomm Integrations */
> +#define MCI_CSPM_QCOM_DATCMD           (1 << 12)
> +#define MCI_CSPM_QCOM_MCIABORT         (1 << 13)
> +#define MCI_CSPM_QCOM_CCSENABLE                (1 << 14)
> +#define MCI_CSPM_QCOM_CCSDISABLE       (1 << 15)
> +#define MCI_CSPM_QCOM_AUTO_CMD19       (1 << 16)
> +#define MCI_CSPM_QCOM_AUTO_CMD21       (1 << 21)

You know how to rename these defines too :-)

Yours,
Linus Walleij

[PATCH v1 11/11] mmc: mmci: Add Qcom specific pio_read function.

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

MCIFIFOCNT register behaviour on Qcom chips is very different than the other
pl180 integrations. MCIFIFOCNT register contains the number of
words that are still waiting to be transferred through the FIFO. It keeps
decrementing once the host CPU reads the MCIFIFO. With the existing logic and
the MCIFIFOCNT behaviour, mmci_pio_read will loop forever, as the FIFOCNT
register will always return transfer size before reading the FIFO.

Also the data sheet states that "This register is only useful for debug
purposes and should not be used for normal operation since it does not reflect
data which may or may not be in the pipeline".

This patch implements qcom_pio_read function so as existing mmci_pio_read is
not suitable for Qcom SOCs.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/mmc/host/mmci.c |   31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index def1b19..45198b6 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -1038,6 +1038,29 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd,
 	}
 }
 
+static int mmci_qcom_pio_read(struct mmci_host *host, char *buffer,
+			 unsigned int remain)
+{
+	uint32_t	*ptr = (uint32_t *) buffer;
+	int		count = 0;
+	struct variant_data *variant = host->variant;
+	int		fifo_size = variant->fifosize;
+
+	if (remain % 4)
+		remain = ((remain >> 2) + 1) << 2;
+
+	while (readl(host->base + MMCISTATUS) & MCI_RXDATAAVLBL) {
+		*ptr = readl(host->base + MMCIFIFO + (count % fifo_size));
+		ptr++;
+		count += sizeof(uint32_t);
+
+		remain -=  sizeof(uint32_t);
+		if (remain == 0)
+			break;
+	}
+	return count;
+}
+
 static int mmci_pio_read(struct mmci_host *host, char *buffer, unsigned int remain)
 {
 	void __iomem *base = host->base;
@@ -1159,8 +1182,12 @@ static irqreturn_t mmci_pio_irq(int irq, void *dev_id)
 		remain = sg_miter->length;
 
 		len = 0;
-		if (status & MCI_RXACTIVE)
-			len = mmci_pio_read(host, buffer, remain);
+		if (status & MCI_RXACTIVE) {
+			if (host->hw_designer == AMBA_VENDOR_QCOM)
+				len = mmci_qcom_pio_read(host, buffer, remain);
+			else
+				len = mmci_pio_read(host, buffer, remain);
+		}
 		if (status & MCI_TXACTIVE)
 			len = mmci_pio_write(host, buffer, remain, status);
 
-- 
1.7.9.5

Re: [PATCH v1 11/11] mmc: mmci: Add Qcom specific pio_read function.

[Linus Walleij]

On Tue, Apr 29, 2014 at 10:21 AM,  <srinivas.kandagatla@linaro.org> wrote:

> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> MCIFIFOCNT register behaviour on Qcom chips is very different than the other
> pl180 integrations. MCIFIFOCNT register contains the number of
> words that are still waiting to be transferred through the FIFO. It keeps
> decrementing once the host CPU reads the MCIFIFO. With the existing logic and
> the MCIFIFOCNT behaviour, mmci_pio_read will loop forever, as the FIFOCNT
> register will always return transfer size before reading the FIFO.
>
> Also the data sheet states that "This register is only useful for debug
> purposes and should not be used for normal operation since it does not reflect
> data which may or may not be in the pipeline".
>
> This patch implements qcom_pio_read function so as existing mmci_pio_read is
> not suitable for Qcom SOCs.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
(...)

> +static int mmci_qcom_pio_read(struct mmci_host *host, char *buffer,
> +                        unsigned int remain)
> +{
> +       uint32_t        *ptr = (uint32_t *) buffer;

Just use u32 for this.

> +       int             count = 0;
> +       struct variant_data *variant = host->variant;
> +       int             fifo_size = variant->fifosize;
> +
> +       if (remain % 4)
> +               remain = ((remain >> 2) + 1) << 2;

Explain in a comment exactly what is happening here or noone will
understand the code.

> +       while (readl(host->base + MMCISTATUS) & MCI_RXDATAAVLBL) {
> +               *ptr = readl(host->base + MMCIFIFO + (count % fifo_size));
> +               ptr++;
> +               count += sizeof(uint32_t);
> +               remain -=  sizeof(uint32_t);


sizeof(u32) or just 4 works for these...

count += 4;
remain -= 4;

Is easier to parse and understand I think.

> +               if (remain == 0)
> +                       break;

if (!remain)
  break;

> +       }
> +       return count;
> +}

> -               if (status & MCI_RXACTIVE)
> -                       len = mmci_pio_read(host, buffer, remain);
> +               if (status & MCI_RXACTIVE) {
> +                       if (host->hw_designer == AMBA_VENDOR_QCOM)
> +                               len = mmci_qcom_pio_read(host, buffer, remain);
> +                       else
> +                               len = mmci_pio_read(host, buffer, remain);
> +               }

Use something like bool qcom_fifo; in vendor data instead.

Yours,
Linus Walleij

Re: [PATCH v1 11/11] mmc: mmci: Add Qcom specific pio_read function.

[Srinivas Kandagatla]

On 13/05/14 09:34, Linus Walleij wrote:
> On Tue, Apr 29, 2014 at 10:21 AM,  <srinivas.kandagatla@linaro.org> wrote:
>
>> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>
>> MCIFIFOCNT register behaviour on Qcom chips is very different than the other
>> pl180 integrations. MCIFIFOCNT register contains the number of
>> words that are still waiting to be transferred through the FIFO. It keeps
>> decrementing once the host CPU reads the MCIFIFO. With the existing logic and
>> the MCIFIFOCNT behaviour, mmci_pio_read will loop forever, as the FIFOCNT
>> register will always return transfer size before reading the FIFO.
>>
>> Also the data sheet states that "This register is only useful for debug
>> purposes and should not be used for normal operation since it does not reflect
>> data which may or may not be in the pipeline".
>>
>> This patch implements qcom_pio_read function so as existing mmci_pio_read is
>> not suitable for Qcom SOCs.
>>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> (...)
>
>> +static int mmci_qcom_pio_read(struct mmci_host *host, char *buffer,
>> +                        unsigned int remain)
>> +{
>> +       uint32_t        *ptr = (uint32_t *) buffer;
>
> Just use u32 for this.
>
>> +       int             count = 0;
>> +       struct variant_data *variant = host->variant;
>> +       int             fifo_size = variant->fifosize;
>> +
>> +       if (remain % 4)
>> +               remain = ((remain >> 2) + 1) << 2;
>
> Explain in a comment exactly what is happening here or noone will
> understand the code.

Ok, I will add more comments here.
>
>> +       while (readl(host->base + MMCISTATUS) & MCI_RXDATAAVLBL) {
>> +               *ptr = readl(host->base + MMCIFIFO + (count % fifo_size));
>> +               ptr++;
>> +               count += sizeof(uint32_t);
>> +               remain -=  sizeof(uint32_t);
>
>
> sizeof(u32) or just 4 works for these...
>
Yes, Will fix it in next version.
> count += 4;
> remain -= 4;
>
> Is easier to parse and understand I think.
>
>> +               if (remain == 0)
>> +                       break;
>
> if (!remain)
>    break;
>
yep.

>> +       }
>> +       return count;
>> +}
>
>> -               if (status & MCI_RXACTIVE)
>> -                       len = mmci_pio_read(host, buffer, remain);
>> +               if (status & MCI_RXACTIVE) {
>> +                       if (host->hw_designer == AMBA_VENDOR_QCOM)
>> +                               len = mmci_qcom_pio_read(host, buffer, remain);
>> +                       else
>> +                               len = mmci_pio_read(host, buffer, remain);
>> +               }
>
> Use something like bool qcom_fifo; in vendor data instead.
>
Ok, make sense, I will fix this in next version.

--srini

> Yours,
> Linus Walleij
>

Re: [PATCH v1 00/11] Add Qualcomm SD Card Controller support.

[Bjorn Andersson]

On Tue, Apr 29, 2014 at 1:18 AM,  <srinivas.kandagatla@linaro.org> wrote:
[...]
> This patch series adds Qualcomm SD Card Controller support in pl180 mmci
> driver. QCom SDCC is basically a pl180, but bit more customized, some of the
> register layouts and offsets are different to the ones mentioned in pl180
> datasheet. The plan is to totally remove the standalone SDCC driver
> drivers/mmc/host/msm_sdcc.* and start using generic mmci driver for all
> Qualcomm parts, as we get chance to test on other Qcom boards.

Hi Srinivas,

I think your series looks good, so I pulled it into my tree and
combined with some input from [1] I got this up and running on Sony
Xperia T (msm8960 based).

I do get below prints in dmesg during probe, mount and some initial
accesses; but the code seems to retry the operations and things work
out nicely.

[  193.556540] mmcblk0: response CRC error sending r/w cmd command,
card status 0xb00


PS. please send the patch enabling AMBA to Kumar as well.

[1] https://git.linaro.org/landing-teams/working/qualcomm/kernel.git/commitdiff/14ea2f1dc5bb719dbfb943e2c700ac166a8c125a

Regards,
Bjorn

Re: [PATCH v1 00/11] Add Qualcomm SD Card Controller support.

[Srinivas Kandagatla]

Thanks Bjorn for testing it on msm8960 SOC.

On 09/05/14 10:56, Bjorn Andersson wrote:
> On Tue, Apr 29, 2014 at 1:18 AM,  <srinivas.kandagatla@linaro.org> wrote:
> [...]
>> This patch series adds Qualcomm SD Card Controller support in pl180 mmci
>> driver. QCom SDCC is basically a pl180, but bit more customized, some of the
>> register layouts and offsets are different to the ones mentioned in pl180
>> datasheet. The plan is to totally remove the standalone SDCC driver
>> drivers/mmc/host/msm_sdcc.* and start using generic mmci driver for all
>> Qualcomm parts, as we get chance to test on other Qcom boards.
>
> Hi Srinivas,
>
> I think your series looks good, so I pulled it into my tree and
> combined with some input from [1] I got this up and running on Sony
> Xperia T (msm8960 based).
>
> I do get below prints in dmesg during probe, mount and some initial
> accesses; but the code seems to retry the operations and things work
> out nicely.

Great..

Could I add your Ack on these series?
>
> [  193.556540] mmcblk0: response CRC error sending r/w cmd command,
> card status 0xb00
>
I could not reproduce this issue with APQ8064 based IFC6410 board.
I will keep testing on other boards as on when they are available. Am 
hoping that we could get this support for v3.16

>
> PS. please send the patch enabling AMBA to Kumar as well.

Sure I will send this patch to Kumar.
>
> [1] https://git.linaro.org/landing-teams/working/qualcomm/kernel.git/commitdiff/14ea2f1dc5bb719dbfb943e2c700ac166a8c125a
>
> Regards,
> Bjorn
>

thanks,
srini

Re: [PATCH v1 00/11] Add Qualcomm SD Card Controller support.

[Ulf Hansson]

On 29 April 2014 10:18,  <srinivas.kandagatla@linaro.org> wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> Hi Russell,
>
> This patch series adds Qualcomm SD Card Controller support in pl180 mmci
> driver. QCom SDCC is basically a pl180, but bit more customized, some of the
> register layouts and offsets are different to the ones mentioned in pl180
> datasheet. The plan is to totally remove the standalone SDCC driver
> drivers/mmc/host/msm_sdcc.* and start using generic mmci driver for all
> Qualcomm parts, as we get chance to test on other Qcom boards.
>
> To start using the existing mmci driver, a fake amba id for Qualcomm is added
> in patches:
>  ARM: amba: Add Qualcomm vendor ID.
>  mmc: mmci: Add Qualcomm Id to amba id table.
>
> Second change is, adding a 3 clock cycle delay for register writes on QCOM
> SDCC
> registers, which is done in patches:
>   mmc: mmci: Add register read/write wrappers.
>   mmc: mmci: Qcomm: Add 3 clock cycle delay after each register write
>
> Third change was to accommodate DATCTRL and MMCICLK register layout changes in
> Qcom SDCC. Which is done in patches:
>   mmc: mmci: Add Qcom datactrl register variant
>   mmc: mmci: Add Qcom variations to MCICommand register.
>   mmc: mmci: Qcom fix MCICLK register settings.
>   mmc: mmci: Add clock support for Qualcomm.
>
> Fourth major change was to add qcom specfic pio read function, the need for
> this is because the way MCIFIFOCNT register behaved in QCOM SDCC is very
>  different to the one in pl180. This change is done in patch:
>   mmc: mmci: Add Qcom specific pio_read function.
>
> Last some Qcom unrelated changes to support Qcom are done in patches:
>   mmc: mmci: use NSEC_PER_SEC macro
>   mmc: mmci: move ST specific register extensions access under condition.
>
> This patches are tested  v3.15-rc3 in PIO mode on IFC6410 board with both eMMC
> and external SD card. I would appreciate any feedback on the patches.
> I would like to get this for v3.16.
>
> Thanks,
> srini
>

Hi Srinivas,

Thanks for working on this patchset, much appreciated! I intend to
review them as soon as I can.

For your information, currently there are one patch queued via Chris'
mmc tree for mmci and recently I have added my quite extensive
patchset, via mine mmc tree, all to be tested in Linux next.

I suppose the best approach to not hit conflicts would be if you base
your patches upon my tree. Potentially we could merge them this way,
unless of course Russell have other opinions.

git://git.linaro.org/people/ulf.hansson/mmc.git next

Kind regards
Ulf Hansson

Re: [PATCH v1 00/11] Add Qualcomm SD Card Controller support.

[Srinivas Kandagatla]

On 13/05/14 11:04, Ulf Hansson wrote:
> On 29 April 2014 10:18,  <srinivas.kandagatla@linaro.org> wrote:
>> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>
>> Hi Russell,
>>
>> This patch series adds Qualcomm SD Card Controller support in pl180 mmci
>> driver. QCom SDCC is basically a pl180, but bit more customized, some of the
>> register layouts and offsets are different to the ones mentioned in pl180
>> datasheet. The plan is to totally remove the standalone SDCC driver
>> drivers/mmc/host/msm_sdcc.* and start using generic mmci driver for all
>> Qualcomm parts, as we get chance to test on other Qcom boards.
>>
>> To start using the existing mmci driver, a fake amba id for Qualcomm is added
>> in patches:
>>   ARM: amba: Add Qualcomm vendor ID.
>>   mmc: mmci: Add Qualcomm Id to amba id table.
>>
>> Second change is, adding a 3 clock cycle delay for register writes on QCOM
>> SDCC
>> registers, which is done in patches:
>>    mmc: mmci: Add register read/write wrappers.
>>    mmc: mmci: Qcomm: Add 3 clock cycle delay after each register write
>>
>> Third change was to accommodate DATCTRL and MMCICLK register layout changes in
>> Qcom SDCC. Which is done in patches:
>>    mmc: mmci: Add Qcom datactrl register variant
>>    mmc: mmci: Add Qcom variations to MCICommand register.
>>    mmc: mmci: Qcom fix MCICLK register settings.
>>    mmc: mmci: Add clock support for Qualcomm.
>>
>> Fourth major change was to add qcom specfic pio read function, the need for
>> this is because the way MCIFIFOCNT register behaved in QCOM SDCC is very
>>   different to the one in pl180. This change is done in patch:
>>    mmc: mmci: Add Qcom specific pio_read function.
>>
>> Last some Qcom unrelated changes to support Qcom are done in patches:
>>    mmc: mmci: use NSEC_PER_SEC macro
>>    mmc: mmci: move ST specific register extensions access under condition.
>>
>> This patches are tested  v3.15-rc3 in PIO mode on IFC6410 board with both eMMC
>> and external SD card. I would appreciate any feedback on the patches.
>> I would like to get this for v3.16.
>>
>> Thanks,
>> srini
>>
>
> Hi Srinivas,
>
Hi Ulf,
> Thanks for working on this patchset, much appreciated! I intend to
> review them as soon as I can.

Thats Great.
>
> For your information, currently there are one patch queued via Chris'
> mmc tree for mmci and recently I have added my quite extensive
> patchset, via mine mmc tree, all to be tested in Linux next.
>
> I suppose the best approach to not hit conflicts would be if you base
> your patches upon my tree. Potentially we could merge them this way,
> unless of course Russell have other opinions.
>
> git://git.linaro.org/people/ulf.hansson/mmc.git next
>
Makes sense, I will rebase my next version of patches on top of your mmc 
git tree, if Russell has no issues with this approach.

Thanks,
srini
> Kind regards
> Ulf Hansson
>

[PATCH v2 00/14] Add Qualcomm SD Card Controller support

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Thankyou everyone for reviewing both RFC and v1 patches.

This patch series adds Qualcomm SD Card Controller support in pl180 mmci
driver. QCom SDCC is basically a pl180, but bit more customized, some of the
register layouts and offsets are different to the ones mentioned in pl180
datasheet. The plan is to totally remove the standalone SDCC driver
drivers/mmc/host/msm_sdcc.* and start using generic mmci driver for all
Qualcomm parts, as we get chance to test on other Qcom boards.

To start using the existing mmci driver, a fake amba id for Qualcomm is added
in patches:
 mmc: mmci: Add Qualcomm Id to amba id table.

Second change is, adding a 3 clock cycle delay for register writes on QCOM SDCC
registers, which is done in patches:
  mmc: mmci: Add register read/write wrappers.
  mmc: mmci: Qcomm: Add 3 clock cycle delay after register write

Third change is to accommodate CLK, DATCTRL and MMCICLK register layout changes
in Qcom SDCC and provide more flexibity in driver to specify these changes via
variant datastructure. Which are done in patches:
  mmc: mmci: Add Qcom datactrl register variant
  mmc: mmci: add ddrmode mask to variant data
  mmc: mmci: add 8bit bus support in variant data
  mmc: mmci: add edge support to data and command out in variant data.
  mmc: mmci: add Qcom specifics of clk and datactrl registers.
  mmc: mmci: Add support to data commands via variant structure.
  mmc: mmci: add support for fbclk to latch data and cmd.
  mmc: mmci: add qcom specific clk control

Fourth major change was to add qcom specfic pio read function, the need for
this is because the way MCIFIFOCNT register behaved in QCOM SDCC is very
 different to the one in pl180. This change is done in patch:
  mmc: mmci: Add Qcom specific pio_read function.

Last some Qcom unrelated changes/cleanup to driver are done in patches:
  mmc: mmci: use NSEC_PER_SEC macro
  mmc: mmci: convert register bits to use BIT() macro.

This patches are tested in PIO mode on IFC8064 board with both eMMC and
external SD card. I would like to get this support in v3.16.

Changes from v1:
	- moved most of the SOC specifics to variant parameters as suggested
	  by Linus W.
	- renamed registers as suggested by Linus W.
	- Added comments in the code as suggested by Linus W.
	- moved out AMBA ID addition patch from this series.
	- rebased the patches to 
		git://git.linaro.org/people/ulf.hansson/mmc.git next 
	  as suggested by Ulf H.

Changes from RFC:
	- moved out clk setup out of spinlock as pointed by Stephen B.
	
Thanks,
srini

Srinivas Kandagatla (14):
  mmc: mmci: use NSEC_PER_SEC macro
  mmc: mmci: convert register bits to use BIT() macro.
  mmc: mmci: Add Qualcomm Id to amba id table
  mmc: mmci: Add Qcom datactrl register variant
  mmc: mmci: Add register read/write wrappers.
  mmc: mmci: Qcomm: Add 3 clock cycle delay after register write
  mmc: mmci: add ddrmode mask to variant data
  mmc: mmci: add 8bit bus support in variant data
  mmc: mmci: add edge support to data and command out in variant data.
  mmc: mmci: add Qcom specifics of clk and datactrl registers.
  mmc: mmci: Add support to data commands via variant structure.
  mmc: mmci: add support for fbclk to latch data and cmd.
  mmc: mmci: add qcom specific clk control
  mmc: mmci: Add Qcom specific pio_read function.

 drivers/mmc/host/mmci.c | 245 ++++++++++++++++++++++++++++++++++++------------
 drivers/mmc/host/mmci.h | 232 +++++++++++++++++++++++++--------------------
 2 files changed, 311 insertions(+), 166 deletions(-)

-- 
1.9.1

[PATCH v2 01/14] mmc: mmci: use NSEC_PER_SEC macro

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

This patch replaces a constant used in calculating timeout with a proper
macro. This is make code more readable.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/mmc/host/mmci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index a084edd..a38e714 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -718,7 +718,7 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 	data->bytes_xfered = 0;
 
 	clks = (unsigned long long)data->timeout_ns * host->cclk;
-	do_div(clks, 1000000000UL);
+	do_div(clks, NSEC_PER_SEC);
 
 	timeout = data->timeout_clks + (unsigned int)clks;
 
-- 
1.9.1

[PATCH v2 02/14] mmc: mmci: convert register bits to use BIT() macro.

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

This patch converts the register bits in the header file to use BIT(()
macro, which looks much neater.

No functional changes done.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/mmc/host/mmci.h | 208 ++++++++++++++++++++++++------------------------
 1 file changed, 104 insertions(+), 104 deletions(-)

diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 347d942..cd83ca3 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -11,48 +11,48 @@
 #define MCI_PWR_OFF		0x00
 #define MCI_PWR_UP		0x02
 #define MCI_PWR_ON		0x03
-#define MCI_OD			(1 << 6)
-#define MCI_ROD			(1 << 7)
+#define MCI_OD			BIT(6)
+#define MCI_ROD			BIT(7)
 /*
  * The ST Micro version does not have ROD and reuse the voltage registers for
  * direction settings.
  */
-#define MCI_ST_DATA2DIREN	(1 << 2)
-#define MCI_ST_CMDDIREN		(1 << 3)
-#define MCI_ST_DATA0DIREN	(1 << 4)
-#define MCI_ST_DATA31DIREN	(1 << 5)
-#define MCI_ST_FBCLKEN		(1 << 7)
-#define MCI_ST_DATA74DIREN	(1 << 8)
+#define MCI_ST_DATA2DIREN	BIT(2)
+#define MCI_ST_CMDDIREN		BIT(3)
+#define MCI_ST_DATA0DIREN	BIT(4)
+#define MCI_ST_DATA31DIREN	BIT(5)
+#define MCI_ST_FBCLKEN		BIT(7)
+#define MCI_ST_DATA74DIREN	BIT(8)
 
 #define MMCICLOCK		0x004
-#define MCI_CLK_ENABLE		(1 << 8)
-#define MCI_CLK_PWRSAVE		(1 << 9)
-#define MCI_CLK_BYPASS		(1 << 10)
-#define MCI_4BIT_BUS		(1 << 11)
+#define MCI_CLK_ENABLE		BIT(8)
+#define MCI_CLK_PWRSAVE		BIT(9)
+#define MCI_CLK_BYPASS		BIT(10)
+#define MCI_4BIT_BUS		BIT(11)
 /*
  * 8bit wide buses, hardware flow contronl, negative edges and clock inversion
  * supported in ST Micro U300 and Ux500 versions
  */
-#define MCI_ST_8BIT_BUS		(1 << 12)
-#define MCI_ST_U300_HWFCEN	(1 << 13)
-#define MCI_ST_UX500_NEG_EDGE	(1 << 13)
-#define MCI_ST_UX500_HWFCEN	(1 << 14)
-#define MCI_ST_UX500_CLK_INV	(1 << 15)
+#define MCI_ST_8BIT_BUS		BIT(12)
+#define MCI_ST_U300_HWFCEN	BIT(13)
+#define MCI_ST_UX500_NEG_EDGE	BIT(13)
+#define MCI_ST_UX500_HWFCEN	BIT(14)
+#define MCI_ST_UX500_CLK_INV	BIT(15)
 /* Modified PL180 on Versatile Express platform */
-#define MCI_ARM_HWFCEN		(1 << 12)
+#define MCI_ARM_HWFCEN		BIT(12)
 
 #define MMCIARGUMENT		0x008
 #define MMCICOMMAND		0x00c
-#define MCI_CPSM_RESPONSE	(1 << 6)
-#define MCI_CPSM_LONGRSP	(1 << 7)
-#define MCI_CPSM_INTERRUPT	(1 << 8)
-#define MCI_CPSM_PENDING	(1 << 9)
-#define MCI_CPSM_ENABLE		(1 << 10)
+#define MCI_CPSM_RESPONSE	BIT(6)
+#define MCI_CPSM_LONGRSP	BIT(7)
+#define MCI_CPSM_INTERRUPT	BIT(8)
+#define MCI_CPSM_PENDING	BIT(9)
+#define MCI_CPSM_ENABLE		BIT(10)
 /* Argument flag extenstions in the ST Micro versions */
-#define MCI_ST_SDIO_SUSP	(1 << 11)
-#define MCI_ST_ENCMD_COMPL	(1 << 12)
-#define MCI_ST_NIEN		(1 << 13)
-#define MCI_ST_CE_ATACMD	(1 << 14)
+#define MCI_ST_SDIO_SUSP	BIT(11)
+#define MCI_ST_ENCMD_COMPL	BIT(12)
+#define MCI_ST_NIEN		BIT(13)
+#define MCI_ST_CE_ATACMD	BIT(14)
 
 #define MMCIRESPCMD		0x010
 #define MMCIRESPONSE0		0x014
@@ -62,95 +62,95 @@
 #define MMCIDATATIMER		0x024
 #define MMCIDATALENGTH		0x028
 #define MMCIDATACTRL		0x02c
-#define MCI_DPSM_ENABLE		(1 << 0)
-#define MCI_DPSM_DIRECTION	(1 << 1)
-#define MCI_DPSM_MODE		(1 << 2)
-#define MCI_DPSM_DMAENABLE	(1 << 3)
-#define MCI_DPSM_BLOCKSIZE	(1 << 4)
+#define MCI_DPSM_ENABLE		BIT(0)
+#define MCI_DPSM_DIRECTION	BIT(1)
+#define MCI_DPSM_MODE		BIT(2)
+#define MCI_DPSM_DMAENABLE	BIT(3)
+#define MCI_DPSM_BLOCKSIZE	BIT(4)
 /* Control register extensions in the ST Micro U300 and Ux500 versions */
-#define MCI_ST_DPSM_RWSTART	(1 << 8)
-#define MCI_ST_DPSM_RWSTOP	(1 << 9)
-#define MCI_ST_DPSM_RWMOD	(1 << 10)
-#define MCI_ST_DPSM_SDIOEN	(1 << 11)
+#define MCI_ST_DPSM_RWSTART	BIT(8)
+#define MCI_ST_DPSM_RWSTOP	BIT(9)
+#define MCI_ST_DPSM_RWMOD	BIT(10)
+#define MCI_ST_DPSM_SDIOEN	BIT(11)
 /* Control register extensions in the ST Micro Ux500 versions */
-#define MCI_ST_DPSM_DMAREQCTL	(1 << 12)
-#define MCI_ST_DPSM_DBOOTMODEEN	(1 << 13)
-#define MCI_ST_DPSM_BUSYMODE	(1 << 14)
-#define MCI_ST_DPSM_DDRMODE	(1 << 15)
+#define MCI_ST_DPSM_DMAREQCTL	BIT(12)
+#define MCI_ST_DPSM_DBOOTMODEEN	BIT(13)
+#define MCI_ST_DPSM_BUSYMODE	BIT(14)
+#define MCI_ST_DPSM_DDRMODE	BIT(15)
 
 #define MMCIDATACNT		0x030
 #define MMCISTATUS		0x034
-#define MCI_CMDCRCFAIL		(1 << 0)
-#define MCI_DATACRCFAIL		(1 << 1)
-#define MCI_CMDTIMEOUT		(1 << 2)
-#define MCI_DATATIMEOUT		(1 << 3)
-#define MCI_TXUNDERRUN		(1 << 4)
-#define MCI_RXOVERRUN		(1 << 5)
-#define MCI_CMDRESPEND		(1 << 6)
-#define MCI_CMDSENT		(1 << 7)
-#define MCI_DATAEND		(1 << 8)
-#define MCI_STARTBITERR		(1 << 9)
-#define MCI_DATABLOCKEND	(1 << 10)
-#define MCI_CMDACTIVE		(1 << 11)
-#define MCI_TXACTIVE		(1 << 12)
-#define MCI_RXACTIVE		(1 << 13)
-#define MCI_TXFIFOHALFEMPTY	(1 << 14)
-#define MCI_RXFIFOHALFFULL	(1 << 15)
-#define MCI_TXFIFOFULL		(1 << 16)
-#define MCI_RXFIFOFULL		(1 << 17)
-#define MCI_TXFIFOEMPTY		(1 << 18)
-#define MCI_RXFIFOEMPTY		(1 << 19)
-#define MCI_TXDATAAVLBL		(1 << 20)
-#define MCI_RXDATAAVLBL		(1 << 21)
+#define MCI_CMDCRCFAIL		BIT(0)
+#define MCI_DATACRCFAIL		BIT(1)
+#define MCI_CMDTIMEOUT		BIT(2)
+#define MCI_DATATIMEOUT		BIT(3)
+#define MCI_TXUNDERRUN		BIT(4)
+#define MCI_RXOVERRUN		BIT(5)
+#define MCI_CMDRESPEND		BIT(6)
+#define MCI_CMDSENT		BIT(7)
+#define MCI_DATAEND		BIT(8)
+#define MCI_STARTBITERR		BIT(9)
+#define MCI_DATABLOCKEND	BIT(10)
+#define MCI_CMDACTIVE		BIT(11)
+#define MCI_TXACTIVE		BIT(12)
+#define MCI_RXACTIVE		BIT(13)
+#define MCI_TXFIFOHALFEMPTY	BIT(14)
+#define MCI_RXFIFOHALFFULL	BIT(15)
+#define MCI_TXFIFOFULL		BIT(16)
+#define MCI_RXFIFOFULL		BIT(17)
+#define MCI_TXFIFOEMPTY		BIT(18)
+#define MCI_RXFIFOEMPTY		BIT(19)
+#define MCI_TXDATAAVLBL		BIT(20)
+#define MCI_RXDATAAVLBL		BIT(21)
 /* Extended status bits for the ST Micro variants */
-#define MCI_ST_SDIOIT		(1 << 22)
-#define MCI_ST_CEATAEND		(1 << 23)
-#define MCI_ST_CARDBUSY		(1 << 24)
+#define MCI_ST_SDIOIT		BIT(22)
+#define MCI_ST_CEATAEND		BIT(23)
+#define MCI_ST_CARDBUSY		BIT(24)
 
 #define MMCICLEAR		0x038
-#define MCI_CMDCRCFAILCLR	(1 << 0)
-#define MCI_DATACRCFAILCLR	(1 << 1)
-#define MCI_CMDTIMEOUTCLR	(1 << 2)
-#define MCI_DATATIMEOUTCLR	(1 << 3)
-#define MCI_TXUNDERRUNCLR	(1 << 4)
-#define MCI_RXOVERRUNCLR	(1 << 5)
-#define MCI_CMDRESPENDCLR	(1 << 6)
-#define MCI_CMDSENTCLR		(1 << 7)
-#define MCI_DATAENDCLR		(1 << 8)
-#define MCI_STARTBITERRCLR	(1 << 9)
-#define MCI_DATABLOCKENDCLR	(1 << 10)
+#define MCI_CMDCRCFAILCLR	BIT(0)
+#define MCI_DATACRCFAILCLR	BIT(1)
+#define MCI_CMDTIMEOUTCLR	BIT(2)
+#define MCI_DATATIMEOUTCLR	BIT(3)
+#define MCI_TXUNDERRUNCLR	BIT(4)
+#define MCI_RXOVERRUNCLR	BIT(5)
+#define MCI_CMDRESPENDCLR	BIT(6)
+#define MCI_CMDSENTCLR		BIT(7)
+#define MCI_DATAENDCLR		BIT(8)
+#define MCI_STARTBITERRCLR	BIT(9)
+#define MCI_DATABLOCKENDCLR	BIT(10)
 /* Extended status bits for the ST Micro variants */
-#define MCI_ST_SDIOITC		(1 << 22)
-#define MCI_ST_CEATAENDC	(1 << 23)
-#define MCI_ST_BUSYENDC		(1 << 24)
+#define MCI_ST_SDIOITC		BIT(22)
+#define MCI_ST_CEATAENDC	BIT(23)
+#define MCI_ST_BUSYENDC		BIT(24)
 
 #define MMCIMASK0		0x03c
-#define MCI_CMDCRCFAILMASK	(1 << 0)
-#define MCI_DATACRCFAILMASK	(1 << 1)
-#define MCI_CMDTIMEOUTMASK	(1 << 2)
-#define MCI_DATATIMEOUTMASK	(1 << 3)
-#define MCI_TXUNDERRUNMASK	(1 << 4)
-#define MCI_RXOVERRUNMASK	(1 << 5)
-#define MCI_CMDRESPENDMASK	(1 << 6)
-#define MCI_CMDSENTMASK		(1 << 7)
-#define MCI_DATAENDMASK		(1 << 8)
-#define MCI_STARTBITERRMASK	(1 << 9)
-#define MCI_DATABLOCKENDMASK	(1 << 10)
-#define MCI_CMDACTIVEMASK	(1 << 11)
-#define MCI_TXACTIVEMASK	(1 << 12)
-#define MCI_RXACTIVEMASK	(1 << 13)
-#define MCI_TXFIFOHALFEMPTYMASK	(1 << 14)
-#define MCI_RXFIFOHALFFULLMASK	(1 << 15)
-#define MCI_TXFIFOFULLMASK	(1 << 16)
-#define MCI_RXFIFOFULLMASK	(1 << 17)
-#define MCI_TXFIFOEMPTYMASK	(1 << 18)
-#define MCI_RXFIFOEMPTYMASK	(1 << 19)
-#define MCI_TXDATAAVLBLMASK	(1 << 20)
-#define MCI_RXDATAAVLBLMASK	(1 << 21)
+#define MCI_CMDCRCFAILMASK	BIT(0)
+#define MCI_DATACRCFAILMASK	BIT(1)
+#define MCI_CMDTIMEOUTMASK	BIT(2)
+#define MCI_DATATIMEOUTMASK	BIT(3)
+#define MCI_TXUNDERRUNMASK	BIT(4)
+#define MCI_RXOVERRUNMASK	BIT(5)
+#define MCI_CMDRESPENDMASK	BIT(6)
+#define MCI_CMDSENTMASK		BIT(7)
+#define MCI_DATAENDMASK		BIT(8)
+#define MCI_STARTBITERRMASK	BIT(9)
+#define MCI_DATABLOCKENDMASK	BIT(10)
+#define MCI_CMDACTIVEMASK	BIT(11)
+#define MCI_TXACTIVEMASK	BIT(12)
+#define MCI_RXACTIVEMASK	BIT(13)
+#define MCI_TXFIFOHALFEMPTYMASK	BIT(14)
+#define MCI_RXFIFOHALFFULLMASK	BIT(15)
+#define MCI_TXFIFOFULLMASK	BIT(16)
+#define MCI_RXFIFOFULLMASK	BIT(17)
+#define MCI_TXFIFOEMPTYMASK	BIT(18)
+#define MCI_RXFIFOEMPTYMASK	BIT(19)
+#define MCI_TXDATAAVLBLMASK	BIT(20)
+#define MCI_RXDATAAVLBLMASK	BIT(21)
 /* Extended status bits for the ST Micro variants */
-#define MCI_ST_SDIOITMASK	(1 << 22)
-#define MCI_ST_CEATAENDMASK	(1 << 23)
-#define MCI_ST_BUSYEND		(1 << 24)
+#define MCI_ST_SDIOITMASK	BIT(22)
+#define MCI_ST_CEATAENDMASK	BIT(23)
+#define MCI_ST_BUSYEND		BIT(24)
 
 #define MMCIMASK1		0x040
 #define MMCIFIFOCNT		0x048
-- 
1.9.1

[PATCH v2 03/14] mmc: mmci: Add Qualcomm Id to amba id table

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

This patch adds a fake Qualcomm ID 0x00051180 to the amba_ids, as Qualcomm
SDCC controller is pl180, but amba id registers read 0x0's.
The plan is to remove SDCC driver totally and use mmci as the main SD
controller driver for Qualcomm SOCs.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/mmc/host/mmci.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index a38e714..7bdf4d3 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -160,6 +160,15 @@ static struct variant_data variant_ux500v2 = {
 	.pwrreg_nopower		= true,
 };
 
+static struct variant_data variant_qcom = {
+	.fifosize		= 16 * 4,
+	.fifohalfsize		= 8 * 4,
+	.clkreg			= MCI_CLK_ENABLE,
+	.datalength_bits	= 24,
+	.blksz_datactrl4	= true,
+	.pwrreg_powerup		= MCI_PWR_UP,
+};
+
 static int mmci_card_busy(struct mmc_host *mmc)
 {
 	struct mmci_host *host = mmc_priv(mmc);
@@ -1750,6 +1759,12 @@ static struct amba_id mmci_ids[] = {
 		.mask   = 0xf0ffffff,
 		.data	= &variant_ux500v2,
 	},
+	/* Qualcomm variants */
+	{
+		.id     = 0x00051180,
+		.mask	= 0x000fffff,
+		.data	= &variant_qcom,
+	},
 	{ 0, 0 },
 };
 
-- 
1.9.1

[PATCH v2 04/14] mmc: mmci: Add Qcom datactrl register variant

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Instance of this IP on Qualcomm's SOCs has bit different layout for datactrl
register. Bit position datactrl[16:4] hold the true block size instead of power
of 2.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/mmc/host/mmci.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 7bdf4d3..324a886 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -60,6 +60,8 @@ static unsigned int fmax = 515633;
  * @sdio: variant supports SDIO
  * @st_clkdiv: true if using a ST-specific clock divider algorithm
  * @blksz_datactrl16: true if Block size is at b16..b30 position in datactrl register
+ * @blksz_datactrl4: true if Block size is at b4..b16 position in datactrl
+ *		     register
  * @pwrreg_powerup: power up value for MMCIPOWER register
  * @signal_direction: input/out direction of bus signals can be indicated
  * @pwrreg_clkgate: MMCIPOWER register must be used to gate the clock
@@ -75,6 +77,7 @@ struct variant_data {
 	bool			sdio;
 	bool			st_clkdiv;
 	bool			blksz_datactrl16;
+	bool			blksz_datactrl4;
 	u32			pwrreg_powerup;
 	bool			signal_direction;
 	bool			pwrreg_clkgate;
@@ -164,6 +167,7 @@ static struct variant_data variant_qcom = {
 	.fifosize		= 16 * 4,
 	.fifohalfsize		= 8 * 4,
 	.clkreg			= MCI_CLK_ENABLE,
+	.blksz_datactrl4	= true,
 	.datalength_bits	= 24,
 	.blksz_datactrl4	= true,
 	.pwrreg_powerup		= MCI_PWR_UP,
@@ -740,6 +744,8 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 
 	if (variant->blksz_datactrl16)
 		datactrl = MCI_DPSM_ENABLE | (data->blksz << 16);
+	else if (variant->blksz_datactrl4)
+		datactrl = MCI_DPSM_ENABLE | (data->blksz << 4);
 	else
 		datactrl = MCI_DPSM_ENABLE | blksz_bits << 4;
 
-- 
1.9.1

[PATCH v2 05/14] mmc: mmci: Add register read/write wrappers.

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

This patch adds wrappers for readl/writel functions used in the driver. The
reason for this wrappers is to accommodate SOCs like Qualcomm which has
requirement for delaying the write for few cycles when writing to its SD Card
Controller registers.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/mmc/host/mmci.c | 110 ++++++++++++++++++++++++++----------------------
 1 file changed, 59 insertions(+), 51 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 324a886..881bb24 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -173,6 +173,16 @@ static struct variant_data variant_qcom = {
 	.pwrreg_powerup		= MCI_PWR_UP,
 };
 
+static inline u32 mmci_readl(struct mmci_host *host, u32 off)
+{
+	return readl(host->base  + off);
+}
+
+static inline void mmci_writel(struct mmci_host *host, u32 data, u32 off)
+{
+	writel(data, host->base + off);
+}
+
 static int mmci_card_busy(struct mmc_host *mmc)
 {
 	struct mmci_host *host = mmc_priv(mmc);
@@ -182,7 +192,7 @@ static int mmci_card_busy(struct mmc_host *mmc)
 	pm_runtime_get_sync(mmc_dev(mmc));
 
 	spin_lock_irqsave(&host->lock, flags);
-	if (readl(host->base + MMCISTATUS) & MCI_ST_CARDBUSY)
+	if (mmci_readl(host, MMCISTATUS) & MCI_ST_CARDBUSY)
 		busy = 1;
 	spin_unlock_irqrestore(&host->lock, flags);
 
@@ -232,7 +242,7 @@ static void mmci_write_clkreg(struct mmci_host *host, u32 clk)
 {
 	if (host->clk_reg != clk) {
 		host->clk_reg = clk;
-		writel(clk, host->base + MMCICLOCK);
+		mmci_writel(host, clk, MMCICLOCK);
 	}
 }
 
@@ -243,7 +253,7 @@ static void mmci_write_pwrreg(struct mmci_host *host, u32 pwr)
 {
 	if (host->pwr_reg != pwr) {
 		host->pwr_reg = pwr;
-		writel(pwr, host->base + MMCIPOWER);
+		mmci_writel(host, pwr, MMCIPOWER);
 	}
 }
 
@@ -257,7 +267,7 @@ static void mmci_write_datactrlreg(struct mmci_host *host, u32 datactrl)
 
 	if (host->datactrl_reg != datactrl) {
 		host->datactrl_reg = datactrl;
-		writel(datactrl, host->base + MMCIDATACTRL);
+		mmci_writel(host, datactrl, MMCIDATACTRL);
 	}
 }
 
@@ -323,7 +333,7 @@ static void mmci_set_clkreg(struct mmci_host *host, unsigned int desired)
 static void
 mmci_request_end(struct mmci_host *host, struct mmc_request *mrq)
 {
-	writel(0, host->base + MMCICOMMAND);
+	mmci_writel(host, 0, MMCICOMMAND);
 
 	BUG_ON(host->data);
 
@@ -338,18 +348,16 @@ mmci_request_end(struct mmci_host *host, struct mmc_request *mrq)
 
 static void mmci_set_mask1(struct mmci_host *host, unsigned int mask)
 {
-	void __iomem *base = host->base;
-
 	if (host->singleirq) {
-		unsigned int mask0 = readl(base + MMCIMASK0);
+		unsigned int mask0 = mmci_readl(host, MMCIMASK0);
 
 		mask0 &= ~MCI_IRQ1MASK;
 		mask0 |= mask;
 
-		writel(mask0, base + MMCIMASK0);
+		mmci_writel(host, mask0, MMCIMASK0);
 	}
 
-	writel(mask, base + MMCIMASK1);
+	mmci_writel(host, mask, MMCIMASK1);
 }
 
 static void mmci_stop_data(struct mmci_host *host)
@@ -478,7 +486,7 @@ static void mmci_dma_finalize(struct mmci_host *host, struct mmc_data *data)
 
 	/* Wait up to 1ms for the DMA to complete */
 	for (i = 0; ; i++) {
-		status = readl(host->base + MMCISTATUS);
+		status = mmci_readl(host, MMCISTATUS);
 		if (!(status & MCI_RXDATAAVLBLMASK) || i >= 100)
 			break;
 		udelay(10);
@@ -617,8 +625,8 @@ static int mmci_dma_start_data(struct mmci_host *host, unsigned int datactrl)
 	 * to fire next DMA request. When that happens, MMCI will
 	 * call mmci_data_end()
 	 */
-	writel(readl(host->base + MMCIMASK0) | MCI_DATAENDMASK,
-	       host->base + MMCIMASK0);
+	mmci_writel(host, mmci_readl(host, MMCIMASK0) | MCI_DATAENDMASK,
+		    MMCIMASK0);
 	return 0;
 }
 
@@ -736,8 +744,8 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 	timeout = data->timeout_clks + (unsigned int)clks;
 
 	base = host->base;
-	writel(timeout, base + MMCIDATATIMER);
-	writel(host->size, base + MMCIDATALENGTH);
+	mmci_writel(host, timeout, MMCIDATATIMER);
+	mmci_writel(host, host->size, MMCIDATALENGTH);
 
 	blksz_bits = ffs(data->blksz) - 1;
 	BUG_ON(1 << blksz_bits != data->blksz);
@@ -811,20 +819,19 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 	}
 
 	mmci_write_datactrlreg(host, datactrl);
-	writel(readl(base + MMCIMASK0) & ~MCI_DATAENDMASK, base + MMCIMASK0);
+	mmci_writel(host, mmci_readl(host, MMCIMASK0) & ~MCI_DATAENDMASK,
+		    MMCIMASK0);
 	mmci_set_mask1(host, irqmask);
 }
 
 static void
 mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c)
 {
-	void __iomem *base = host->base;
-
 	dev_dbg(mmc_dev(host->mmc), "op %02x arg %08x flags %08x\n",
 	    cmd->opcode, cmd->arg, cmd->flags);
 
-	if (readl(base + MMCICOMMAND) & MCI_CPSM_ENABLE) {
-		writel(0, base + MMCICOMMAND);
+	if (mmci_readl(host, MMCICOMMAND) & MCI_CPSM_ENABLE) {
+		mmci_writel(host, 0, MMCICOMMAND);
 		udelay(1);
 	}
 
@@ -839,8 +846,8 @@ mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c)
 
 	host->cmd = cmd;
 
-	writel(cmd->arg, base + MMCIARGUMENT);
-	writel(c, base + MMCICOMMAND);
+	mmci_writel(host, cmd->arg, MMCIARGUMENT);
+	mmci_writel(host, c, MMCICOMMAND);
 }
 
 static void
@@ -865,7 +872,7 @@ mmci_data_irq(struct mmci_host *host, struct mmc_data *data,
 		 * can be as much as a FIFO-worth of data ahead.  This
 		 * matters for FIFO overruns only.
 		 */
-		remain = readl(host->base + MMCIDATACNT);
+		remain = mmci_readl(host, MMCIDATACNT);
 		success = data->blksz * data->blocks - remain;
 
 		dev_dbg(mmc_dev(host->mmc), "MCI ERROR IRQ, status 0x%08x at 0x%08x\n",
@@ -947,10 +954,10 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd,
 	} else if (status & MCI_CMDCRCFAIL && cmd->flags & MMC_RSP_CRC) {
 		cmd->error = -EILSEQ;
 	} else {
-		cmd->resp[0] = readl(base + MMCIRESPONSE0);
-		cmd->resp[1] = readl(base + MMCIRESPONSE1);
-		cmd->resp[2] = readl(base + MMCIRESPONSE2);
-		cmd->resp[3] = readl(base + MMCIRESPONSE3);
+		cmd->resp[0] = mmci_readl(host, MMCIRESPONSE0);
+		cmd->resp[1] = mmci_readl(host, MMCIRESPONSE1);
+		cmd->resp[2] = mmci_readl(host, MMCIRESPONSE2);
+		cmd->resp[3] = mmci_readl(host, MMCIRESPONSE3);
 	}
 
 	if ((!sbc && !cmd->data) || cmd->error) {
@@ -1061,11 +1068,10 @@ static irqreturn_t mmci_pio_irq(int irq, void *dev_id)
 	struct mmci_host *host = dev_id;
 	struct sg_mapping_iter *sg_miter = &host->sg_miter;
 	struct variant_data *variant = host->variant;
-	void __iomem *base = host->base;
 	unsigned long flags;
 	u32 status;
 
-	status = readl(base + MMCISTATUS);
+	status = mmci_readl(host, MMCISTATUS);
 
 	dev_dbg(mmc_dev(host->mmc), "irq1 (pio) %08x\n", status);
 
@@ -1105,7 +1111,7 @@ static irqreturn_t mmci_pio_irq(int irq, void *dev_id)
 		if (remain)
 			break;
 
-		status = readl(base + MMCISTATUS);
+		status = mmci_readl(host, MMCISTATUS);
 	} while (1);
 
 	sg_miter_stop(sg_miter);
@@ -1127,7 +1133,9 @@ static irqreturn_t mmci_pio_irq(int irq, void *dev_id)
 	 */
 	if (host->size == 0) {
 		mmci_set_mask1(host, 0);
-		writel(readl(base + MMCIMASK0) | MCI_DATAENDMASK, base + MMCIMASK0);
+		mmci_writel(host,
+			    mmci_readl(host, MMCIMASK0) | MCI_DATAENDMASK,
+			    MMCIMASK0);
 	}
 
 	return IRQ_HANDLED;
@@ -1148,10 +1156,10 @@ static irqreturn_t mmci_irq(int irq, void *dev_id)
 		struct mmc_command *cmd;
 		struct mmc_data *data;
 
-		status = readl(host->base + MMCISTATUS);
+		status = mmci_readl(host, MMCISTATUS);
 
 		if (host->singleirq) {
-			if (status & readl(host->base + MMCIMASK1))
+			if (status & mmci_readl(host, MMCIMASK1))
 				mmci_pio_irq(irq, dev_id);
 
 			status &= ~MCI_IRQ1MASK;
@@ -1162,8 +1170,8 @@ static irqreturn_t mmci_irq(int irq, void *dev_id)
 		 * enabled) since the HW seems to be triggering the IRQ on both
 		 * edges while monitoring DAT0 for busy completion.
 		 */
-		status &= readl(host->base + MMCIMASK0);
-		writel(status, host->base + MMCICLEAR);
+		status &= mmci_readl(host, MMCIMASK0);
+		mmci_writel(host, status, MMCICLEAR);
 
 		dev_dbg(mmc_dev(host->mmc), "irq0 (data+cmd) %08x\n", status);
 
@@ -1561,9 +1569,9 @@ static int mmci_probe(struct amba_device *dev,
 
 	spin_lock_init(&host->lock);
 
-	writel(0, host->base + MMCIMASK0);
-	writel(0, host->base + MMCIMASK1);
-	writel(0xfff, host->base + MMCICLEAR);
+	mmci_writel(host, 0, MMCIMASK0);
+	mmci_writel(host, 0, MMCIMASK1);
+	mmci_writel(host, 0xfff, MMCICLEAR);
 
 	/* If DT, cd/wp gpios must be supplied through it. */
 	if (!np && gpio_is_valid(plat->gpio_cd)) {
@@ -1591,7 +1599,7 @@ static int mmci_probe(struct amba_device *dev,
 			goto clk_disable;
 	}
 
-	writel(MCI_IRQENABLE, host->base + MMCIMASK0);
+	mmci_writel(host, MCI_IRQENABLE, MMCIMASK0);
 
 	amba_set_drvdata(dev, mmc);
 
@@ -1632,11 +1640,11 @@ static int mmci_remove(struct amba_device *dev)
 
 		mmc_remove_host(mmc);
 
-		writel(0, host->base + MMCIMASK0);
-		writel(0, host->base + MMCIMASK1);
+		mmci_writel(host, 0, MMCIMASK0);
+		mmci_writel(host, 0, MMCIMASK1);
 
-		writel(0, host->base + MMCICOMMAND);
-		writel(0, host->base + MMCIDATACTRL);
+		mmci_writel(host, 0, MMCICOMMAND);
+		mmci_writel(host, 0, MMCIDATACTRL);
 
 		mmci_dma_release(host);
 		clk_disable_unprepare(host->clk);
@@ -1653,11 +1661,11 @@ static void mmci_save(struct mmci_host *host)
 
 	spin_lock_irqsave(&host->lock, flags);
 
-	writel(0, host->base + MMCIMASK0);
+	mmci_writel(host, 0, MMCIMASK0);
 	if (host->variant->pwrreg_nopower) {
-		writel(0, host->base + MMCIDATACTRL);
-		writel(0, host->base + MMCIPOWER);
-		writel(0, host->base + MMCICLOCK);
+		mmci_writel(host, 0, MMCIDATACTRL);
+		mmci_writel(host, 0, MMCIPOWER);
+		mmci_writel(host, 0, MMCICLOCK);
 	}
 	mmci_reg_delay(host);
 
@@ -1671,11 +1679,11 @@ static void mmci_restore(struct mmci_host *host)
 	spin_lock_irqsave(&host->lock, flags);
 
 	if (host->variant->pwrreg_nopower) {
-		writel(host->clk_reg, host->base + MMCICLOCK);
-		writel(host->datactrl_reg, host->base + MMCIDATACTRL);
-		writel(host->pwr_reg, host->base + MMCIPOWER);
+		mmci_writel(host, host->clk_reg, MMCICLOCK);
+		mmci_writel(host, host->datactrl_reg, MMCIDATACTRL);
+		mmci_writel(host, host->pwr_reg, MMCIPOWER);
 	}
-	writel(MCI_IRQENABLE, host->base + MMCIMASK0);
+	mmci_writel(host, MCI_IRQENABLE, MMCIMASK0);
 	mmci_reg_delay(host);
 
 	spin_unlock_irqrestore(&host->lock, flags);
-- 
1.9.1

Re: [PATCH v2 05/14] mmc: mmci: Add register read/write wrappers.

[Linus Walleij]

On Thu, May 15, 2014 at 11:36 AM,  <srinivas.kandagatla@linaro.org> wrote:

> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> This patch adds wrappers for readl/writel functions used in the driver. The
> reason for this wrappers is to accommodate SOCs like Qualcomm which has
> requirement for delaying the write for few cycles when writing to its SD Card
> Controller registers.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH v2 06/14] mmc: mmci: Qcomm: Add 3 clock cycle delay after register write

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Most of the Qcomm SD card controller registers must be updated to the MCLK
domain so subsequent writes to registers will be ignored until 3 clock cycles
have passed.

This patch adds a 3 clock cycle delay required after writing to controller
registers on Qualcomm SOCs. Without this delay all the register writes are not
successful, resulting in not detecting cards. The write clock delay is
activated by setting up mclk_delayed_writes variable in variant data.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/mmc/host/mmci.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 881bb24..1385554 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -67,6 +67,8 @@ static unsigned int fmax = 515633;
  * @pwrreg_clkgate: MMCIPOWER register must be used to gate the clock
  * @busy_detect: true if busy detection on dat0 is supported
  * @pwrreg_nopower: bits in MMCIPOWER don't controls ext. power supply
+ * @mclk_delayed_writes: enable delayed writes to ensure, subsequent updates
+ *			 are not ignored.
  */
 struct variant_data {
 	unsigned int		clkreg;
@@ -83,6 +85,7 @@ struct variant_data {
 	bool			pwrreg_clkgate;
 	bool			busy_detect;
 	bool			pwrreg_nopower;
+	bool			mclk_delayed_writes;
 };
 
 static struct variant_data variant_arm = {
@@ -171,6 +174,12 @@ static struct variant_data variant_qcom = {
 	.datalength_bits	= 24,
 	.blksz_datactrl4	= true,
 	.pwrreg_powerup		= MCI_PWR_UP,
+	/*
+	 * On QCom SD card controller, registers must be updated to the
+	 * MCLK domain so subsequent writes to this register will be ignored
+	 * for 3 clk cycles.
+	 */
+	.mclk_delayed_writes	= true,
 };
 
 static inline u32 mmci_readl(struct mmci_host *host, u32 off)
@@ -181,6 +190,9 @@ static inline u32 mmci_readl(struct mmci_host *host, u32 off)
 static inline void mmci_writel(struct mmci_host *host, u32 data, u32 off)
 {
 	writel(data, host->base + off);
+
+	if (host->variant->mclk_delayed_writes)
+		udelay(DIV_ROUND_UP((3 * USEC_PER_SEC), host->mclk));
 }
 
 static int mmci_card_busy(struct mmc_host *mmc)
-- 
1.9.1

Re: [PATCH v2 06/14] mmc: mmci: Qcomm: Add 3 clock cycle delay after register write

[Linus Walleij]

On Thu, May 15, 2014 at 11:37 AM,  <srinivas.kandagatla@linaro.org> wrote:

> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> Most of the Qcomm SD card controller registers must be updated to the MCLK
> domain so subsequent writes to registers will be ignored until 3 clock cycles
> have passed.
>
> This patch adds a 3 clock cycle delay required after writing to controller
> registers on Qualcomm SOCs. Without this delay all the register writes are not
> successful, resulting in not detecting cards. The write clock delay is
> activated by setting up mclk_delayed_writes variable in variant data.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

OK this is looking good.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH v2 07/14] mmc: mmci: add ddrmode mask to variant data

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

This patch adds ddrmode mask to variant structure giving more flexibility
to the driver to support more SOCs which have different datactrl register
layout.

Without this patch datactrl register is updated with wrong ddrmode mask on non
ST SOCs, resulting in card detection failures.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/mmc/host/mmci.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 1385554..dec70d2 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -59,6 +59,7 @@ static unsigned int fmax = 515633;
  *		  is asserted (likewise for RX)
  * @sdio: variant supports SDIO
  * @st_clkdiv: true if using a ST-specific clock divider algorithm
+ * @datactrl_mask_ddrmode: ddr mode mask in datactrl register.
  * @blksz_datactrl16: true if Block size is at b16..b30 position in datactrl register
  * @blksz_datactrl4: true if Block size is at b4..b16 position in datactrl
  *		     register
@@ -76,6 +77,7 @@ struct variant_data {
 	unsigned int		datalength_bits;
 	unsigned int		fifosize;
 	unsigned int		fifohalfsize;
+	unsigned int		datactrl_mask_ddrmode;
 	bool			sdio;
 	bool			st_clkdiv;
 	bool			blksz_datactrl16;
@@ -114,6 +116,7 @@ static struct variant_data variant_u300 = {
 	.fifosize		= 16 * 4,
 	.fifohalfsize		= 8 * 4,
 	.clkreg_enable		= MCI_ST_U300_HWFCEN,
+	.datactrl_mask_ddrmode	= MCI_ST_DPSM_DDRMODE,
 	.datalength_bits	= 16,
 	.sdio			= true,
 	.pwrreg_powerup		= MCI_PWR_ON,
@@ -126,6 +129,7 @@ static struct variant_data variant_nomadik = {
 	.fifosize		= 16 * 4,
 	.fifohalfsize		= 8 * 4,
 	.clkreg			= MCI_CLK_ENABLE,
+	.datactrl_mask_ddrmode	= MCI_ST_DPSM_DDRMODE,
 	.datalength_bits	= 24,
 	.sdio			= true,
 	.st_clkdiv		= true,
@@ -140,6 +144,7 @@ static struct variant_data variant_ux500 = {
 	.fifohalfsize		= 8 * 4,
 	.clkreg			= MCI_CLK_ENABLE,
 	.clkreg_enable		= MCI_ST_UX500_HWFCEN,
+	.datactrl_mask_ddrmode	= MCI_ST_DPSM_DDRMODE,
 	.datalength_bits	= 24,
 	.sdio			= true,
 	.st_clkdiv		= true,
@@ -155,6 +160,7 @@ static struct variant_data variant_ux500v2 = {
 	.fifohalfsize		= 8 * 4,
 	.clkreg			= MCI_CLK_ENABLE,
 	.clkreg_enable		= MCI_ST_UX500_HWFCEN,
+	.datactrl_mask_ddrmode	= MCI_ST_DPSM_DDRMODE,
 	.datalength_bits	= 24,
 	.sdio			= true,
 	.st_clkdiv		= true,
@@ -800,7 +806,7 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 		}
 
 	if (host->mmc->ios.timing == MMC_TIMING_UHS_DDR50)
-		datactrl |= MCI_ST_DPSM_DDRMODE;
+		datactrl |= variant->datactrl_mask_ddrmode;
 
 	/*
 	 * Attempt to use DMA operation mode, if this
-- 
1.9.1

Re: [PATCH v2 07/14] mmc: mmci: add ddrmode mask to variant data

[Linus Walleij]

On Thu, May 15, 2014 at 11:37 AM,  <srinivas.kandagatla@linaro.org> wrote:

> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> This patch adds ddrmode mask to variant structure giving more flexibility
> to the driver to support more SOCs which have different datactrl register
> layout.
>
> Without this patch datactrl register is updated with wrong ddrmode mask on non
> ST SOCs, resulting in card detection failures.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH v2 08/14] mmc: mmci: add 8bit bus support in variant data

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

This patch adds 8bit bus enable to variant structure giving more flexibility
to the driver to support more SOCs which have different clock register layout.

Without this patch other new SOCs like Qcom will have to add more code
to special case them.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/mmc/host/mmci.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index dec70d2..a81f303 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -52,6 +52,7 @@ static unsigned int fmax = 515633;
  * struct variant_data - MMCI variant-specific quirks
  * @clkreg: default value for MCICLOCK register
  * @clkreg_enable: enable value for MMCICLOCK register
+ * @clkreg_8bit_bus_enable: enable value for 8 bit bus
  * @datalength_bits: number of bits in the MMCIDATALENGTH register
  * @fifosize: number of bytes that can be written when MMCI_TXFIFOEMPTY
  *	      is asserted (likewise for RX)
@@ -74,6 +75,7 @@ static unsigned int fmax = 515633;
 struct variant_data {
 	unsigned int		clkreg;
 	unsigned int		clkreg_enable;
+	unsigned int		clkreg_8bit_bus_enable;
 	unsigned int		datalength_bits;
 	unsigned int		fifosize;
 	unsigned int		fifohalfsize;
@@ -116,6 +118,7 @@ static struct variant_data variant_u300 = {
 	.fifosize		= 16 * 4,
 	.fifohalfsize		= 8 * 4,
 	.clkreg_enable		= MCI_ST_U300_HWFCEN,
+	.clkreg_8bit_bus_enable = MCI_ST_8BIT_BUS,
 	.datactrl_mask_ddrmode	= MCI_ST_DPSM_DDRMODE,
 	.datalength_bits	= 16,
 	.sdio			= true,
@@ -144,6 +147,7 @@ static struct variant_data variant_ux500 = {
 	.fifohalfsize		= 8 * 4,
 	.clkreg			= MCI_CLK_ENABLE,
 	.clkreg_enable		= MCI_ST_UX500_HWFCEN,
+	.clkreg_8bit_bus_enable = MCI_ST_8BIT_BUS,
 	.datactrl_mask_ddrmode	= MCI_ST_DPSM_DDRMODE,
 	.datalength_bits	= 24,
 	.sdio			= true,
@@ -160,6 +164,7 @@ static struct variant_data variant_ux500v2 = {
 	.fifohalfsize		= 8 * 4,
 	.clkreg			= MCI_CLK_ENABLE,
 	.clkreg_enable		= MCI_ST_UX500_HWFCEN,
+	.clkreg_8bit_bus_enable = MCI_ST_8BIT_BUS,
 	.datactrl_mask_ddrmode	= MCI_ST_DPSM_DDRMODE,
 	.datalength_bits	= 24,
 	.sdio			= true,
@@ -340,7 +345,7 @@ static void mmci_set_clkreg(struct mmci_host *host, unsigned int desired)
 	if (host->mmc->ios.bus_width == MMC_BUS_WIDTH_4)
 		clk |= MCI_4BIT_BUS;
 	if (host->mmc->ios.bus_width == MMC_BUS_WIDTH_8)
-		clk |= MCI_ST_8BIT_BUS;
+		clk |= variant->clkreg_8bit_bus_enable;
 
 	if (host->mmc->ios.timing == MMC_TIMING_UHS_DDR50)
 		clk |= MCI_ST_UX500_NEG_EDGE;
-- 
1.9.1

Re: [PATCH v2 08/14] mmc: mmci: add 8bit bus support in variant data

[Linus Walleij]

On Thu, May 15, 2014 at 11:37 AM,  <srinivas.kandagatla@linaro.org> wrote:

> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> This patch adds 8bit bus enable to variant structure giving more flexibility
> to the driver to support more SOCs which have different clock register layout.
>
> Without this patch other new SOCs like Qcom will have to add more code
> to special case them.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH v2 09/14] mmc: mmci: add edge support to data and command out in variant data.

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

This patch adds edge support for data and command out to variant structure
giving more flexibility to the driver to support more SOCs which have
different clock register layout.

Without this patch other new SOCs like Qcom will have to add more code to
special case them

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/mmc/host/mmci.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index a81f303..17e7f6a 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -53,6 +53,7 @@ static unsigned int fmax = 515633;
  * @clkreg: default value for MCICLOCK register
  * @clkreg_enable: enable value for MMCICLOCK register
  * @clkreg_8bit_bus_enable: enable value for 8 bit bus
+ * @clkreg_neg_edge_enable: enable value for inverted data/cmd output
  * @datalength_bits: number of bits in the MMCIDATALENGTH register
  * @fifosize: number of bytes that can be written when MMCI_TXFIFOEMPTY
  *	      is asserted (likewise for RX)
@@ -76,6 +77,7 @@ struct variant_data {
 	unsigned int		clkreg;
 	unsigned int		clkreg_enable;
 	unsigned int		clkreg_8bit_bus_enable;
+	unsigned int		clkreg_neg_edge_enable;
 	unsigned int		datalength_bits;
 	unsigned int		fifosize;
 	unsigned int		fifohalfsize;
@@ -148,6 +150,7 @@ static struct variant_data variant_ux500 = {
 	.clkreg			= MCI_CLK_ENABLE,
 	.clkreg_enable		= MCI_ST_UX500_HWFCEN,
 	.clkreg_8bit_bus_enable = MCI_ST_8BIT_BUS,
+	.clkreg_neg_edge_enable	= MCI_ST_UX500_NEG_EDGE,
 	.datactrl_mask_ddrmode	= MCI_ST_DPSM_DDRMODE,
 	.datalength_bits	= 24,
 	.sdio			= true,
@@ -165,6 +168,7 @@ static struct variant_data variant_ux500v2 = {
 	.clkreg			= MCI_CLK_ENABLE,
 	.clkreg_enable		= MCI_ST_UX500_HWFCEN,
 	.clkreg_8bit_bus_enable = MCI_ST_8BIT_BUS,
+	.clkreg_neg_edge_enable	= MCI_ST_UX500_NEG_EDGE,
 	.datactrl_mask_ddrmode	= MCI_ST_DPSM_DDRMODE,
 	.datalength_bits	= 24,
 	.sdio			= true,
@@ -348,7 +352,7 @@ static void mmci_set_clkreg(struct mmci_host *host, unsigned int desired)
 		clk |= variant->clkreg_8bit_bus_enable;
 
 	if (host->mmc->ios.timing == MMC_TIMING_UHS_DDR50)
-		clk |= MCI_ST_UX500_NEG_EDGE;
+		clk |= variant->clkreg_neg_edge_enable;
 
 	mmci_write_clkreg(host, clk);
 }
-- 
1.9.1

Re: [PATCH v2 09/14] mmc: mmci: add edge support to data and command out in variant data.

[Linus Walleij]

On Thu, May 15, 2014 at 11:37 AM,  <srinivas.kandagatla@linaro.org> wrote:

> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> This patch adds edge support for data and command out to variant structure
> giving more flexibility to the driver to support more SOCs which have
> different clock register layout.
>
> Without this patch other new SOCs like Qcom will have to add more code to
> special case them
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH v2 10/14] mmc: mmci: add Qcom specifics of clk and datactrl registers.

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

This patch adds specifics of clk and datactrl register on Qualcomm SD
Card controller. This patch also populates the Qcom variant data with
these new values specific to Qualcomm SD Card Controller.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/mmc/host/mmci.c |  3 +++
 drivers/mmc/host/mmci.h | 24 ++++++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 17e7f6a..0a0fc22 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -185,6 +185,9 @@ static struct variant_data variant_qcom = {
 	.fifosize		= 16 * 4,
 	.fifohalfsize		= 8 * 4,
 	.clkreg			= MCI_CLK_ENABLE,
+	.clkreg_enable		= MCI_QCOM_CLK_FLOWENA,
+	.clkreg_8bit_bus_enable = MCI_QCOM_CLK_WIDEBUS_8,
+	.datactrl_mask_ddrmode	= MCI_QCOM_CLK_DDR_MODE,
 	.blksz_datactrl4	= true,
 	.datalength_bits	= 24,
 	.blksz_datactrl4	= true,
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index cd83ca3..1b93ae7 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -41,6 +41,22 @@
 /* Modified PL180 on Versatile Express platform */
 #define MCI_ARM_HWFCEN		BIT(12)
 
+/* Modified on Qualcomm Integrations */
+#define MCI_QCOM_CLK_WIDEBUS_4	(2 << 10)
+#define MCI_QCOM_CLK_WIDEBUS_8	(3 << 10)
+#define MCI_QCOM_CLK_FLOWENA	BIT(12)
+#define MCI_QCOM_CLK_INVERTOUT	BIT(13)
+
+/* select in latch data and command */
+#define MCI_QCOM_CLK_SEL_IN_SHIFT	(14)
+#define MCI_QCOM_CLK_SEL_MASK		(0x3)
+#define MCI_QCOM_CLK_SEL_RISING_EDGE	(1)
+#define MCI_QCOM_CLK_FEEDBACK_CLK	(2 << 14)
+#define MCI_QCOM_CLK_DDR_MODE		(3 << 14)
+
+/* mclk selection */
+#define MCI_QCOM_CLK_SEL_MCLK		(2 << 23)
+
 #define MMCIARGUMENT		0x008
 #define MMCICOMMAND		0x00c
 #define MCI_CPSM_RESPONSE	BIT(6)
@@ -54,6 +70,14 @@
 #define MCI_ST_NIEN		BIT(13)
 #define MCI_ST_CE_ATACMD	BIT(14)
 
+/* Modified on Qualcomm Integrations */
+#define MCI_QCOM_CSPM_DATCMD		BIT(12)
+#define MCI_QCOM_CSPM_MCIABORT		BIT(13)
+#define MCI_QCOM_CSPM_CCSENABLE		BIT(14)
+#define MCI_QCOM_CSPM_CCSDISABLE	BIT(15)
+#define MCI_QCOM_CSPM_AUTO_CMD19	BIT(16)
+#define MCI_QCOM_CSPM_AUTO_CMD21	BIT(21)
+
 #define MMCIRESPCMD		0x010
 #define MMCIRESPONSE0		0x014
 #define MMCIRESPONSE1		0x018
-- 
1.9.1

Re: [PATCH v2 10/14] mmc: mmci: add Qcom specifics of clk and datactrl registers.

[Linus Walleij]

On Thu, May 15, 2014 at 11:37 AM,  <srinivas.kandagatla@linaro.org> wrote:

> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> This patch adds specifics of clk and datactrl register on Qualcomm SD
> Card controller. This patch also populates the Qcom variant data with
> these new values specific to Qualcomm SD Card Controller.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Now it looks real elegant.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH v2 11/14] mmc: mmci: Add support to data commands via variant structure.

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

On some SOCs like Qcom there are explicit bits in the command register
to specify if its a data transfer command or not. So this patch adds
support to such bits in variant data, giving more flexibility to the
driver.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/mmc/host/mmci.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 0a0fc22..05ae654 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -59,6 +59,7 @@ static unsigned int fmax = 515633;
  *	      is asserted (likewise for RX)
  * @fifohalfsize: number of bytes that can be written when MCI_TXFIFOHALFEMPTY
  *		  is asserted (likewise for RX)
+ * @data_cmd_enable: enable value for data commands.
  * @sdio: variant supports SDIO
  * @st_clkdiv: true if using a ST-specific clock divider algorithm
  * @datactrl_mask_ddrmode: ddr mode mask in datactrl register.
@@ -81,6 +82,7 @@ struct variant_data {
 	unsigned int		datalength_bits;
 	unsigned int		fifosize;
 	unsigned int		fifohalfsize;
+	unsigned int		data_cmd_enable;
 	unsigned int		datactrl_mask_ddrmode;
 	bool			sdio;
 	bool			st_clkdiv;
@@ -188,6 +190,7 @@ static struct variant_data variant_qcom = {
 	.clkreg_enable		= MCI_QCOM_CLK_FLOWENA,
 	.clkreg_8bit_bus_enable = MCI_QCOM_CLK_WIDEBUS_8,
 	.datactrl_mask_ddrmode	= MCI_QCOM_CLK_DDR_MODE,
+	.data_cmd_enable	= MCI_QCOM_CSPM_DATCMD,
 	.blksz_datactrl4	= true,
 	.datalength_bits	= 24,
 	.blksz_datactrl4	= true,
@@ -874,6 +877,9 @@ mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c)
 	if (/*interrupt*/0)
 		c |= MCI_CPSM_INTERRUPT;
 
+	if (mmc_cmd_type(cmd) == MMC_CMD_ADTC)
+		c |= host->variant->data_cmd_enable;
+
 	host->cmd = cmd;
 
 	mmci_writel(host, cmd->arg, MMCIARGUMENT);
-- 
1.9.1

Re: [PATCH v2 11/14] mmc: mmci: Add support to data commands via variant structure.

[Linus Walleij]

On Thu, May 15, 2014 at 11:37 AM,  <srinivas.kandagatla@linaro.org> wrote:

> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> On some SOCs like Qcom there are explicit bits in the command register
> to specify if its a data transfer command or not. So this patch adds
> support to such bits in variant data, giving more flexibility to the
> driver.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH v2 11/14] mmc: mmci: Add support to data commands via variant structure.

[Srinivas Kandagatla]

Thanks Linus W.

On 23/05/14 10:09, Linus Walleij wrote:
> On Thu, May 15, 2014 at 11:37 AM,  <srinivas.kandagatla@linaro.org> wrote:
>
>> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>
>> On some SOCs like Qcom there are explicit bits in the command register
>> to specify if its a data transfer command or not. So this patch adds
>> support to such bits in variant data, giving more flexibility to the
>> driver.
>>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>
> Yours,
> Linus Walleij
>

[PATCH v2 12/14] mmc: mmci: add support for fbclk to latch data and cmd.

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

This patch adds support to fbclk that is used to latch data and
cmd on some controllers like SD Card controller in Qcom SOC.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/mmc/host/mmci.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 05ae654..bc7b80d 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -54,6 +54,8 @@ static unsigned int fmax = 515633;
  * @clkreg_enable: enable value for MMCICLOCK register
  * @clkreg_8bit_bus_enable: enable value for 8 bit bus
  * @clkreg_neg_edge_enable: enable value for inverted data/cmd output
+ * @clkreg_fbclk_latch: enable value to select feedback clock to
+ *			latch data and command comming in.
  * @datalength_bits: number of bits in the MMCIDATALENGTH register
  * @fifosize: number of bytes that can be written when MMCI_TXFIFOEMPTY
  *	      is asserted (likewise for RX)
@@ -79,6 +81,7 @@ struct variant_data {
 	unsigned int		clkreg_enable;
 	unsigned int		clkreg_8bit_bus_enable;
 	unsigned int		clkreg_neg_edge_enable;
+	unsigned int		clkreg_fbclk_latch;
 	unsigned int		datalength_bits;
 	unsigned int		fifosize;
 	unsigned int		fifohalfsize;
@@ -189,6 +192,7 @@ static struct variant_data variant_qcom = {
 	.clkreg			= MCI_CLK_ENABLE,
 	.clkreg_enable		= MCI_QCOM_CLK_FLOWENA,
 	.clkreg_8bit_bus_enable = MCI_QCOM_CLK_WIDEBUS_8,
+	.clkreg_fbclk_latch	= MCI_QCOM_CLK_FEEDBACK_CLK,
 	.datactrl_mask_ddrmode	= MCI_QCOM_CLK_DDR_MODE,
 	.data_cmd_enable	= MCI_QCOM_CSPM_DATCMD,
 	.blksz_datactrl4	= true,
@@ -343,6 +347,7 @@ static void mmci_set_clkreg(struct mmci_host *host, unsigned int desired)
 			host->cclk = host->mclk / (2 * (clk + 1));
 		}
 
+		clk |= variant->clkreg_fbclk_latch;
 		clk |= variant->clkreg_enable;
 		clk |= MCI_CLK_ENABLE;
 		/* This hasn't proven to be worthwhile */
-- 
1.9.1

Re: [PATCH v2 12/14] mmc: mmci: add support for fbclk to latch data and cmd.

[Linus Walleij]

On Thu, May 15, 2014 at 11:37 AM,  <srinivas.kandagatla@linaro.org> wrote:

> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> This patch adds support to fbclk that is used to latch data and
> cmd on some controllers like SD Card controller in Qcom SOC.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

(...)

Isn't this overkill?

@@ -189,6 +192,7 @@ static struct variant_data variant_qcom = {
         .clkreg                 = MCI_CLK_ENABLE,
-        .clkreg_enable          = MCI_QCOM_CLK_FLOWENA,
+       .clkreg_enable          = MCI_QCOM_CLK_FLOWENA |
MCI_QCOM_CLK_FEEDBACK_CLK,
         .clkreg_8bit_bus_enable = MCI_QCOM_CLK_WIDEBUS_8,

Isn't this achieveing exactly the same thing without the extra fields?
You unconditionally do it at every enable anyway, don't you?

Yours,
Linus Walleij

Re: [PATCH v2 12/14] mmc: mmci: add support for fbclk to latch data and cmd.

[Srinivas Kandagatla]

On 23/05/14 10:12, Linus Walleij wrote:
> On Thu, May 15, 2014 at 11:37 AM,  <srinivas.kandagatla@linaro.org> wrote:
>
>> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>
>> This patch adds support to fbclk that is used to latch data and
>> cmd on some controllers like SD Card controller in Qcom SOC.
>>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> (...)
>
> Isn't this overkill?

I totally agree.

Initially I did do it the way you suggested, but wanted to be more 
explicit in what its actually doing and I was also not sure if its Ok to 
add more than one flag in clkreg_enable.

>
> @@ -189,6 +192,7 @@ static struct variant_data variant_qcom = {
>           .clkreg                 = MCI_CLK_ENABLE,
> -        .clkreg_enable          = MCI_QCOM_CLK_FLOWENA,
> +       .clkreg_enable          = MCI_QCOM_CLK_FLOWENA |
> MCI_QCOM_CLK_FEEDBACK_CLK,
>           .clkreg_8bit_bus_enable = MCI_QCOM_CLK_WIDEBUS_8,
>

> Isn't this achieveing exactly the same thing without the extra fields?
> You unconditionally do it at every enable anyway, don't you?
I will fix it and send a new version.

Thanks,
srini
>
> Yours,
> Linus Walleij
>

[PATCH v2 13/14] mmc: mmci: add qcom specific clk control

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

On Qcom SD card controller, cclk is mclk and mclk should be directly
controlled by the driver.
This patch adds support to control mclk directly in the driver, and also
adds explicit_mclk_control and cclk_is_mclk flags in variant structure giving
more flexibility to the driver.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/mmc/host/mmci.c | 30 +++++++++++++++++++++++++-----
 1 file changed, 25 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index bc7b80d..cf58fec1 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -75,6 +75,8 @@ static unsigned int fmax = 515633;
  * @pwrreg_nopower: bits in MMCIPOWER don't controls ext. power supply
  * @mclk_delayed_writes: enable delayed writes to ensure, subsequent updates
  *			 are not ignored.
+ * @explicit_mclk_control: enable explicit mclk control in driver.
+ * @qcom_cclk_is_mclk: enable iff card clock is multimedia card adapter clock.
  */
 struct variant_data {
 	unsigned int		clkreg;
@@ -97,6 +99,8 @@ struct variant_data {
 	bool			busy_detect;
 	bool			pwrreg_nopower;
 	bool			mclk_delayed_writes;
+	bool			explicit_mclk_control;
+	bool			qcom_cclk_is_mclk;
 };
 
 static struct variant_data variant_arm = {
@@ -205,6 +209,8 @@ static struct variant_data variant_qcom = {
 	 * for 3 clk cycles.
 	 */
 	.mclk_delayed_writes	= true,
+	.explicit_mclk_control	= true,
+	.qcom_cclk_is_mclk	= true,
 };
 
 static inline u32 mmci_readl(struct mmci_host *host, u32 off)
@@ -320,7 +326,9 @@ static void mmci_set_clkreg(struct mmci_host *host, unsigned int desired)
 	host->cclk = 0;
 
 	if (desired) {
-		if (desired >= host->mclk) {
+		if (variant->qcom_cclk_is_mclk) {
+			host->cclk = host->mclk;
+		} else if (desired >= host->mclk) {
 			clk = MCI_CLK_BYPASS;
 			if (variant->st_clkdiv)
 				clk |= MCI_ST_UX500_NEG_EDGE;
@@ -1358,6 +1366,16 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	if (!ios->clock && variant->pwrreg_clkgate)
 		pwr &= ~MCI_PWR_ON;
 
+	if (ios->clock != host->mclk && host->variant->explicit_mclk_control) {
+		int rc = clk_set_rate(host->clk, ios->clock);
+		if (rc < 0) {
+			dev_err(mmc_dev(host->mmc),
+				"Error setting clock rate (%d)\n", rc);
+		} else {
+			host->mclk = clk_get_rate(host->clk);
+		}
+	}
+
 	spin_lock_irqsave(&host->lock, flags);
 
 	mmci_set_clkreg(host, ios->clock);
@@ -1544,10 +1562,12 @@ static int mmci_probe(struct amba_device *dev,
 	 * is not specified. Either value must not exceed the clock rate into
 	 * the block, of course.
 	 */
-	if (mmc->f_max)
-		mmc->f_max = min(host->mclk, mmc->f_max);
-	else
-		mmc->f_max = min(host->mclk, fmax);
+	if (!host->variant->explicit_mclk_control) {
+		if (mmc->f_max)
+			mmc->f_max = min(host->mclk, mmc->f_max);
+		else
+			mmc->f_max = min(host->mclk, fmax);
+	}
 	dev_dbg(mmc_dev(mmc), "clocking block at %u Hz\n", mmc->f_max);
 
 	/* Get regulators and the supported OCR mask */
-- 
1.9.1

Re: [PATCH v2 13/14] mmc: mmci: add qcom specific clk control

[Linus Walleij]

On Thu, May 15, 2014 at 11:37 AM,  <srinivas.kandagatla@linaro.org> wrote:

> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> On Qcom SD card controller, cclk is mclk and mclk should be directly
> controlled by the driver.
> This patch adds support to control mclk directly in the driver, and also
> adds explicit_mclk_control and cclk_is_mclk flags in variant structure giving
> more flexibility to the driver.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
(...)
> +       bool                    qcom_cclk_is_mclk;

I have no idea why you prefix that with "qcom" since it's an
abstract concept.

But it's a very minor issue, so:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH v2 14/14] mmc: mmci: Add Qcom specific pio_read function.

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

MCIFIFOCNT register behaviour on Qcom chips is very different than the other
pl180 integrations. MCIFIFOCNT register contains the number of
words that are still waiting to be transferred through the FIFO. It keeps
decrementing once the host CPU reads the MCIFIFO. With the existing logic and
the MCIFIFOCNT behaviour, mmci_pio_read will loop forever, as the FIFOCNT
register will always return transfer size before reading the FIFO.

Also the data sheet states that "This register is only useful for debug
purposes and should not be used for normal operation since it does not reflect
data which may or may not be in the pipeline".

This patch implements qcom_pio_read function so as existing mmci_pio_read is
not suitable for Qcom SOCs. qcom_pio_read function is only selected
based on qcom_fifo flag in variant data structure.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/mmc/host/mmci.c | 35 +++++++++++++++++++++++++++++++++--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index cf58fec1..94b99d6 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -77,6 +77,7 @@ static unsigned int fmax = 515633;
  *			 are not ignored.
  * @explicit_mclk_control: enable explicit mclk control in driver.
  * @qcom_cclk_is_mclk: enable iff card clock is multimedia card adapter clock.
+ * @qcom_fifo: enables qcom specific fifo pio read function.
  */
 struct variant_data {
 	unsigned int		clkreg;
@@ -101,6 +102,7 @@ struct variant_data {
 	bool			mclk_delayed_writes;
 	bool			explicit_mclk_control;
 	bool			qcom_cclk_is_mclk;
+	bool			qcom_fifo;
 };
 
 static struct variant_data variant_arm = {
@@ -211,6 +213,7 @@ static struct variant_data variant_qcom = {
 	.mclk_delayed_writes	= true,
 	.explicit_mclk_control	= true,
 	.qcom_cclk_is_mclk	= true,
+	.qcom_fifo		= true,
 };
 
 static inline u32 mmci_readl(struct mmci_host *host, u32 off)
@@ -1026,6 +1029,29 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd,
 	}
 }
 
+static int mmci_qcom_pio_read(struct mmci_host *host, char *buffer,
+			 unsigned int remain)
+{
+	uint32_t	*ptr = (uint32_t *) buffer;
+	int		count = 0;
+	struct variant_data *variant = host->variant;
+	int		fifo_size = variant->fifosize;
+
+	if (remain % 4)
+		remain = ((remain >> 2) + 1) << 2;
+
+	while (readl(host->base + MMCISTATUS) & MCI_RXDATAAVLBL) {
+		*ptr = readl(host->base + MMCIFIFO + (count % fifo_size));
+		ptr++;
+		count += sizeof(uint32_t);
+
+		remain -=  sizeof(uint32_t);
+		if (remain == 0)
+			break;
+	}
+	return count;
+}
+
 static int mmci_pio_read(struct mmci_host *host, char *buffer, unsigned int remain)
 {
 	void __iomem *base = host->base;
@@ -1147,8 +1173,13 @@ static irqreturn_t mmci_pio_irq(int irq, void *dev_id)
 		remain = sg_miter->length;
 
 		len = 0;
-		if (status & MCI_RXACTIVE)
-			len = mmci_pio_read(host, buffer, remain);
+		if (status & MCI_RXACTIVE) {
+			if (variant->qcom_fifo)
+				len = mmci_qcom_pio_read(host, buffer, remain);
+			else
+				len = mmci_pio_read(host, buffer, remain);
+		}
+
 		if (status & MCI_TXACTIVE)
 			len = mmci_pio_write(host, buffer, remain, status);
 
-- 
1.9.1

Re: [PATCH v2 14/14] mmc: mmci: Add Qcom specific pio_read function.

[Linus Walleij]

On Thu, May 15, 2014 at 11:38 AM,  <srinivas.kandagatla@linaro.org> wrote:

> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> MCIFIFOCNT register behaviour on Qcom chips is very different than the other
> pl180 integrations. MCIFIFOCNT register contains the number of
> words that are still waiting to be transferred through the FIFO. It keeps
> decrementing once the host CPU reads the MCIFIFO. With the existing logic and
> the MCIFIFOCNT behaviour, mmci_pio_read will loop forever, as the FIFOCNT
> register will always return transfer size before reading the FIFO.
>
> Also the data sheet states that "This register is only useful for debug
> purposes and should not be used for normal operation since it does not reflect
> data which may or may not be in the pipeline".
>
> This patch implements qcom_pio_read function so as existing mmci_pio_read is
> not suitable for Qcom SOCs. qcom_pio_read function is only selected
> based on qcom_fifo flag in variant data structure.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
(...)

> +static int mmci_qcom_pio_read(struct mmci_host *host, char *buffer,
> +                        unsigned int remain)
> +{
> +       uint32_t        *ptr = (uint32_t *) buffer;

Just use u32 like the rest of the driver does.

> +       int             fifo_size = variant->fifosize;
> +
> +       if (remain % 4)
> +               remain = ((remain >> 2) + 1) << 2;

Insert a comment here so we know what is happening.
It seems you are rounding up to the nearest divisible
by four reads. It looks complicated. Does this accomplish
the same thing?

if (remain % 4) {
    remain &= ~0x03;
    remain += 4;
}

> +       while (readl(host->base + MMCISTATUS) & MCI_RXDATAAVLBL) {
> +               *ptr = readl(host->base + MMCIFIFO + (count % fifo_size));
> +               ptr++;
> +               count += sizeof(uint32_t);
> +
> +               remain -=  sizeof(uint32_t);

I already commented that this looks weird. It is a well known fact
that a u32 is 4 bytes.

count += 4;
remain -= 4;

works just fine!

> +               if (remain == 0)
> +                       break;
> +       }
> +       return count;
> +}

And another variant is to count the *number or words*
to read from the FIFO rather than the number of bytes! I would do it
like this:

static int mmci_qcom_pio_read(struct mmci_host *host, char *buffer,
                        unsigned int remain)
{
       u32 *ptr = (u32*) buffer;
       unsigned int count = 0;
       unsigned int words;
       unsigned int fifo_size = host->variant->fifosize;

       words = DIV_ROUND_UP(remain, 4);
       while (readl(host->base + MMCISTATUS) & MCI_RXDATAAVLBL) {
               *ptr = readl(host->base + MMCIFIFO + (count % fifo_size));
               ptr++;
               count += 4;
               remain--;
               if (!remain)
                       break;
       }
       return count;
}

I guess you will run into additional problems when you come to doing
SDIO. This function can return *more* bytes than asked for, as it rounds
up. It won't happen with MMC/SD transfers since these are always
divisible by 8, but it *will* happen on SDIO!

If you look carefully at the comments in mmci_pio_read() you will
see how this is handled. Are you sure this function cannot be
augmented to handle the qcom variant as well so you don't get this
problem further down the road?

Yours,
Linus Walleij

Re: [PATCH v2 14/14] mmc: mmci: Add Qcom specific pio_read function.

[Srinivas Kandagatla]

On 23/05/14 10:31, Linus Walleij wrote:
> On Thu, May 15, 2014 at 11:38 AM,  <srinivas.kandagatla@linaro.org> wrote:
>
>> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>
>> MCIFIFOCNT register behaviour on Qcom chips is very different than the other
>> pl180 integrations. MCIFIFOCNT register contains the number of
>> words that are still waiting to be transferred through the FIFO. It keeps
>> decrementing once the host CPU reads the MCIFIFO. With the existing logic and
>> the MCIFIFOCNT behaviour, mmci_pio_read will loop forever, as the FIFOCNT
>> register will always return transfer size before reading the FIFO.
>>
>> Also the data sheet states that "This register is only useful for debug
>> purposes and should not be used for normal operation since it does not reflect
>> data which may or may not be in the pipeline".
>>
>> This patch implements qcom_pio_read function so as existing mmci_pio_read is
>> not suitable for Qcom SOCs. qcom_pio_read function is only selected
>> based on qcom_fifo flag in variant data structure.
>>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> (...)
>
>> +static int mmci_qcom_pio_read(struct mmci_host *host, char *buffer,
>> +                        unsigned int remain)
>> +{
>> +       uint32_t        *ptr = (uint32_t *) buffer;
>
Oops..

Sorry Linus, I think this change-set got something different than what I 
did. I remember your comments in the last review, I did take care of 
them but some how It got missed... I will fix these and send a new version.

> Just use u32 like the rest of the driver does.
>
>> +       int             fifo_size = variant->fifosize;
>> +
>> +       if (remain % 4)

>
> And another variant is to count the *number or words*
> to read from the FIFO rather than the number of bytes! I would do it
> like this:
>
> static int mmci_qcom_pio_read(struct mmci_host *host, char *buffer,
>                          unsigned int remain)
> {
>         u32 *ptr = (u32*) buffer;
>         unsigned int count = 0;
>         unsigned int words;
>         unsigned int fifo_size = host->variant->fifosize;
>
>         words = DIV_ROUND_UP(remain, 4);
>         while (readl(host->base + MMCISTATUS) & MCI_RXDATAAVLBL) {
>                 *ptr = readl(host->base + MMCIFIFO + (count % fifo_size));
>                 ptr++;
>                 count += 4;
>                 remain--;
>                 if (!remain)
>                         break;
>         }
>         return count;
> }
>
> I guess you will run into additional problems when you come to doing
> SDIO. This function can return *more* bytes than asked for, as it rounds
> up. It won't happen with MMC/SD transfers since these are always
> divisible by 8, but it *will* happen on SDIO!
>
> If you look carefully at the comments in mmci_pio_read() you will
> see how this is handled. Are you sure this function cannot be
> augmented to handle the qcom variant as well so you don't get this
> problem further down the road?

I did try to customize the mmci_pio_read function but I failed all the 
time. The reason being the behaviour of MCI_FIFOCNT register which is 
totally different to the way the function is written. I will give a try 
again.

Thanks,
srini
>
> Yours,
> Linus Walleij
>

Re: [PATCH v2 14/14] mmc: mmci: Add Qcom specific pio_read function.

[Srinivas Kandagatla]

Hi Linus W,

On 23/05/14 10:31, Linus Walleij wrote:
> static int mmci_qcom_pio_read(struct mmci_host *host, char *buffer,
>                          unsigned int remain)
> {
>         u32 *ptr = (u32*) buffer;
>         unsigned int count = 0;
>         unsigned int words;
>         unsigned int fifo_size = host->variant->fifosize;
>
>         words = DIV_ROUND_UP(remain, 4);
>         while (readl(host->base + MMCISTATUS) & MCI_RXDATAAVLBL) {
>                 *ptr = readl(host->base + MMCIFIFO + (count % fifo_size));
>                 ptr++;
>                 count += 4;
>                 remain--;
>                 if (!remain)
>                         break;
>         }
>         return count;
> }
>
> I guess you will run into additional problems when you come to doing
> SDIO. This function can return*more*  bytes than asked for, as it rounds
> up. It won't happen with MMC/SD transfers since these are always
> divisible by 8, but it*will*  happen on SDIO!
That's a good point,

Qualcomm will need SDIO support in future, so I have slightly modified 
the  code to address this. Other thing I tried was to fit in this in 
mmci_pio_read, It became very ugly, as the FIFOCNT register behaviour is 
totally different and there is no way to tell how many bytes are ready 
to be consumed. So finally I think having a separate pio read for 
qualcomm looks much neater.

final mmci_qcom_pio_read looks like:


static int mmci_qcom_pio_read(struct mmci_host *host, char *buffer,
             unsigned int remain)
{
     u32 *ptr = (u32*) buffer;
     unsigned int count = 0;
     unsigned int words, bytes;
     unsigned int fifo_size = host->variant->fifosize;

     words = remain >> 2;
     bytes = remain % 4;
     /* read full words followed by leftover bytes */
     if (words) {
         while (readl(host->base + MMCISTATUS) & MCI_RXDATAAVLBL) {
             *ptr = readl(host->base + MMCIFIFO + (count % fifo_size));
             ptr++;
             count += 4;
             words--;
             if (!words)
                 break;
          }
     }

     /* read leftover bytes */
     if (unlikely(bytes)) {
         unsigned char buf[4];
         if (readl(host->base + MMCISTATUS) & MCI_RXDATAAVLBL) {
             *buf = readl(host->base + MMCIFIFO + (count % fifo_size));
             memcpy(ptr, buf, bytes);
              count += bytes;
         }
     }

     return count;
}

Thanks,
srini

Re: [PATCH v2 00/14] Add Qualcomm SD Card Controller support

[Srinivas Kandagatla]

Hi Ulf,
I like to get this patches for v3.16,  any chance of considering these 
patches to v3.16 ?

--srini

On 15/05/14 10:34, srinivas.kandagatla@linaro.org wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> Thankyou everyone for reviewing both RFC and v1 patches.
>
> This patch series adds Qualcomm SD Card Controller support in pl180 mmci
> driver. QCom SDCC is basically a pl180, but bit more customized, some of the
> register layouts and offsets are different to the ones mentioned in pl180
> datasheet. The plan is to totally remove the standalone SDCC driver
> drivers/mmc/host/msm_sdcc.* and start using generic mmci driver for all
> Qualcomm parts, as we get chance to test on other Qcom boards.
>
> To start using the existing mmci driver, a fake amba id for Qualcomm is added
> in patches:
>   mmc: mmci: Add Qualcomm Id to amba id table.
>
> Second change is, adding a 3 clock cycle delay for register writes on QCOM SDCC
> registers, which is done in patches:
>    mmc: mmci: Add register read/write wrappers.
>    mmc: mmci: Qcomm: Add 3 clock cycle delay after register write
>
> Third change is to accommodate CLK, DATCTRL and MMCICLK register layout changes
> in Qcom SDCC and provide more flexibity in driver to specify these changes via
> variant datastructure. Which are done in patches:
>    mmc: mmci: Add Qcom datactrl register variant
>    mmc: mmci: add ddrmode mask to variant data
>    mmc: mmci: add 8bit bus support in variant data
>    mmc: mmci: add edge support to data and command out in variant data.
>    mmc: mmci: add Qcom specifics of clk and datactrl registers.
>    mmc: mmci: Add support to data commands via variant structure.
>    mmc: mmci: add support for fbclk to latch data and cmd.
>    mmc: mmci: add qcom specific clk control
>
> Fourth major change was to add qcom specfic pio read function, the need for
> this is because the way MCIFIFOCNT register behaved in QCOM SDCC is very
>   different to the one in pl180. This change is done in patch:
>    mmc: mmci: Add Qcom specific pio_read function.
>
> Last some Qcom unrelated changes/cleanup to driver are done in patches:
>    mmc: mmci: use NSEC_PER_SEC macro
>    mmc: mmci: convert register bits to use BIT() macro.
>
> This patches are tested in PIO mode on IFC8064 board with both eMMC and
> external SD card. I would like to get this support in v3.16.
>
> Changes from v1:
> 	- moved most of the SOC specifics to variant parameters as suggested
> 	  by Linus W.
> 	- renamed registers as suggested by Linus W.
> 	- Added comments in the code as suggested by Linus W.
> 	- moved out AMBA ID addition patch from this series.
> 	- rebased the patches to
> 		git://git.linaro.org/people/ulf.hansson/mmc.git next
> 	  as suggested by Ulf H.
>
> Changes from RFC:
> 	- moved out clk setup out of spinlock as pointed by Stephen B.
> 	
> Thanks,
> srini
>
> Srinivas Kandagatla (14):
>    mmc: mmci: use NSEC_PER_SEC macro
>    mmc: mmci: convert register bits to use BIT() macro.
>    mmc: mmci: Add Qualcomm Id to amba id table
>    mmc: mmci: Add Qcom datactrl register variant
>    mmc: mmci: Add register read/write wrappers.
>    mmc: mmci: Qcomm: Add 3 clock cycle delay after register write
>    mmc: mmci: add ddrmode mask to variant data
>    mmc: mmci: add 8bit bus support in variant data
>    mmc: mmci: add edge support to data and command out in variant data.
>    mmc: mmci: add Qcom specifics of clk and datactrl registers.
>    mmc: mmci: Add support to data commands via variant structure.
>    mmc: mmci: add support for fbclk to latch data and cmd.
>    mmc: mmci: add qcom specific clk control
>    mmc: mmci: Add Qcom specific pio_read function.
>
>   drivers/mmc/host/mmci.c | 245 ++++++++++++++++++++++++++++++++++++------------
>   drivers/mmc/host/mmci.h | 232 +++++++++++++++++++++++++--------------------
>   2 files changed, 311 insertions(+), 166 deletions(-)
>

Re: [PATCH v2 00/14] Add Qualcomm SD Card Controller support

[Ulf Hansson]

On 23 May 2014 09:13, Srinivas Kandagatla
<srinivas.kandagatla@linaro.org> wrote:
> Hi Ulf,
> I like to get this patches for v3.16,  any chance of considering these
> patches to v3.16 ?

I promise to have them properly reviewed early next week, sorry for
taking so long. Let's see where this leads us.

It seems like you had some CRC issues during read/write? Did you
manage to resolve that issue?

Kind regards
Ulf Hansson

Re: [PATCH v2 00/14] Add Qualcomm SD Card Controller support

[Srinivas Kandagatla]

On 23/05/14 08:50, Ulf Hansson wrote:
> On 23 May 2014 09:13, Srinivas Kandagatla
> <srinivas.kandagatla@linaro.org> wrote:
>> Hi Ulf,
>> I like to get this patches for v3.16,  any chance of considering these
>> patches to v3.16 ?
>
> I promise to have them properly reviewed early next week, sorry for
> taking so long. Let's see where this leads us.
>
Thankyou.

> It seems like you had some CRC issues during read/write? Did you
> manage to resolve that issue?

Bjorn is using different SOC and board than the IFC6410 Am testing on, 
so its completely different setup.
on IFC6410 we did lot of stress testing and no issues seen. so 
suspecting the issues are very specific to that board or the eMMC Bjorn 
is using.

We are suspecting that the CRC issues are due to the fact that there is 
no code to manage regulators. My test setup uses dummy regulator, and 
the eMMC and external SD cards seems to be Ok with default voltages.

Am not sure if thats the same with Bjorn's board.

Thanks,
srini
>
> Kind regards
> Ulf Hansson
>
>

Re: [PATCH v2 00/14] Add Qualcomm SD Card Controller support

[Bjorn Andersson]

On Fri, May 23, 2014 at 1:11 AM, Srinivas Kandagatla
<srinivas.kandagatla@linaro.org> wrote:
> On 23/05/14 08:50, Ulf Hansson wrote:
[...]
>> It seems like you had some CRC issues during read/write? Did you
>> manage to resolve that issue?
>
>
> Bjorn is using different SOC and board than the IFC6410 Am testing on, so
> its completely different setup.
> on IFC6410 we did lot of stress testing and no issues seen. so suspecting
> the issues are very specific to that board or the eMMC Bjorn is using.
>
> We are suspecting that the CRC issues are due to the fact that there is no
> code to manage regulators. My test setup uses dummy regulator, and the eMMC
> and external SD cards seems to be Ok with default voltages.
>
> Am not sure if thats the same with Bjorn's board.

I started off by using the same setup as you; relying on the boot leaving
everything in a working condition and using fixed-regulator; with that setup
I saw the CRC issues.

I then introduced my rpm and regulator driver and it didn't change anything
related to the CRC error behaviour.

I have verified that I have the right pinconf and the clocks seems to match
what we have with working codeaurora based SW.
One thing that I did notice is that we run our devices with
MMC_CAP_UHS_DDR50, while Qualcomm doesn't, but I have not been
able to introduce a change in behaviour by modifying the caps.

I do see the same problem both on 8960 and on 8064; although on 8064
it's much worse.


I will do some more debugging to see if I can find any further differences
in the two setups.

Regards,
Bjorn

Re: [PATCH v2 00/14] Add Qualcomm SD Card Controller support

[Srinivas Kandagatla]

Hi Bjorn,

Thanks for the update.
On 23/05/14 16:20, Bjorn Andersson wrote:
> On Fri, May 23, 2014 at 1:11 AM, Srinivas Kandagatla
> <srinivas.kandagatla@linaro.org> wrote:
>> On 23/05/14 08:50, Ulf Hansson wrote:
> [...]
>>> It seems like you had some CRC issues during read/write? Did you
>>> manage to resolve that issue?
>>
>>
>> Bjorn is using different SOC and board than the IFC6410 Am testing on, so
>> its completely different setup.
>> on IFC6410 we did lot of stress testing and no issues seen. so suspecting
>> the issues are very specific to that board or the eMMC Bjorn is using.
>>
>> We are suspecting that the CRC issues are due to the fact that there is no
>> code to manage regulators. My test setup uses dummy regulator, and the eMMC
>> and external SD cards seems to be Ok with default voltages.
>>
>> Am not sure if thats the same with Bjorn's board.
>
> I started off by using the same setup as you; relying on the boot leaving
> everything in a working condition and using fixed-regulator; with that setup
> I saw the CRC issues.
>
> I then introduced my rpm and regulator driver and it didn't change anything
> related to the CRC error behaviour.
>
> I have verified that I have the right pinconf and the clocks seems to match
> what we have with working codeaurora based SW.
> One thing that I did notice is that we run our devices with
> MMC_CAP_UHS_DDR50, while Qualcomm doesn't, but I have not been
> able to introduce a change in behaviour by modifying the caps.
>
I think the driver needs some more additional configs in clkreg to 
support UHS_DDR50 mode.. I will try to create a patch for that.

Did you get a chance to test external SD/MMC card?
> I do see the same problem both on 8960 and on 8064; although on 8064
> it's much worse.
>
>
> I will do some more debugging to see if I can find any further differences
> in the two setups.
Yes, Please let me know if you need any kind of support in debugging.
We can sync up on IRC on this. Its important that we solve this issue.

Thanks,
srini
>
> Regards,
> Bjorn
>

Re: [PATCH v2 00/14] Add Qualcomm SD Card Controller support

[Bjorn Andersson]

On Fri, May 23, 2014 at 9:50 AM, Srinivas Kandagatla
<srinivas.kandagatla@linaro.org> wrote:
> Hi Bjorn,
>
> Thanks for the update.

Hi Srini,

After pulling in Ulfs next branch again and reapplying v3 my issues
are now gone. Not sure what I screwed up when I merged v2, this seems
to work just fine now. Sorry for the fuzz.

I haven't done any extensive testing, but my CRC issues are definitely
sorted out. I will go ahead and see if I can get external mmc working
now as well.

Regards,
Bjorn

Re: [PATCH v2 00/14] Add Qualcomm SD Card Controller support

[Srinivas Kandagatla]

Thanks Bjorn,


On 24/05/14 00:26, Bjorn Andersson wrote:
> On Fri, May 23, 2014 at 9:50 AM, Srinivas Kandagatla
> <srinivas.kandagatla@linaro.org> wrote:
>> Hi Bjorn,
>>
>> Thanks for the update.
>
> Hi Srini,
>
> After pulling in Ulfs next branch again and reapplying v3 my issues
> are now gone. Not sure what I screwed up when I merged v2, this seems
> to work just fine now. Sorry for the fuzz.
>
That's fantastic news and just in time.

Hopefully we could make these patches for 3.16?

thanks,
srini

> I haven't done any extensive testing, but my CRC issues are definitely
> sorted out. I will go ahead and see if I can get external mmc working
> now as well.
>
> Regards,
> Bjorn
>

Re: [PATCH RFC 00/12] Add Qualcomm SD Card Controller support.

[Bjorn Andersson]

On Mon, Apr 21, 2014 at 2:43 PM,  <srinivas.kandagatla@linaro.org> wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
[snip]
>
> This patches are tested in PIO mode on IFC8064 board with both eMMC and
> external SD card. I would appreciate any feedback/suggestions on the overall
> approach.

Hi Srini,

I finally got around to give this series a spin on our APQ8064 based
devices as well. I had a few reported CRC issues on our msm8960 based
devices, but thinks seems functionally "ok".

On APQ8064, the kernel finds the onboard eMMC and when mounting it I
get the following:

[    1.624118] mmc0: new high speed MMC card at address 0001
[    1.627521] mmcblk0: mmc0:0001 MAG2GA 14.5 GiB
[    1.632714] mmcblk0boot0: mmc0:0001 MAG2GA partition 1 2.00 MiB
[    1.636989] mmcblk0boot1: mmc0:0001 MAG2GA partition 2 2.00 MiB
[    1.637393] Freeing unused kernel memory: 332K (c09c6000 - c0a19000)
[    1.663379] mmcblk0: response CRC error sending r/w cmd command,
card status 0xb00
[    1.663659] mmcblk0: error -84 transferring data, sector 30535648,
nr 8, cmd response 0x900, card status 0xb00
[    1.669851] mmcblk0: retrying using single block read
[    1.681534] mmcblk0: response CRC error sending r/w cmd command,
card status 0xb00
[    1.686758]  mmcblk0: p1 p2 p3 p4 p5 p6 p7 p8 p9 p10 p11 p12 p13
p14 p15 p16 p17 p18 p19 p20 p21 p22 p23 p24 p25 p26
mkdir: can't create directory '/dev': File exists
[    1.712644]  mmcblk0boot1: unknown partition table
[    1.717419]  mmcblk0boot0: unknown partition table
[    2.255625] EXT3-fs (mmcblk0p24): error: couldn't mount because of
unsupported optional features (40)
[    2.256687] EXT2-fs (mmcblk0p24): error: couldn't mount because of
unsupported optional features (40)
[    2.277945] mmcblk0: error -84 transferring data, sector 160824, nr
232, cmd response 0x900, card status 0xb00
[    2.277985] mmcblk0: retrying using single block read
[    2.288518] mmcblk0: response CRC error sending r/w cmd command,
card status 0xb00
[    2.292914] mmcblk0: response CRC error sending r/w cmd command,
card status 0xb00
[    2.300985] mmcblk0: response CRC error sending r/w cmd command,
card status 0xb00
[    2.311787] mmcblk0: response CRC error sending r/w cmd command,
card status 0xb00
[    2.316151] mmcblk0: response CRC error sending r/w cmd command,
card status 0xb00
[    2.322986] mmcblk0: response CRC error sending r/w cmd command,
card status 0xb00
[    2.329640] end_request: I/O error, dev mmcblk0, sector 160853
[    2.337266] end_request: I/O error, dev mmcblk0, sector 160856
[    2.343093] end_request: I/O error, dev mmcblk0, sector 160864
[    2.348826] end_request: I/O error, dev mmcblk0, sector 160872
[    2.354720] end_request: I/O error, dev mmcblk0, sector 160880
[    2.360458] end_request: I/O error, dev mmcblk0, sector 160888
[    2.366346] end_request: I/O error, dev mmcblk0, sector 160896
[    2.372175] end_request: I/O error, dev mmcblk0, sector 160904
[    2.377906] end_request: I/O error, dev mmcblk0, sector 160912
[    2.383802] end_request: I/O error, dev mmcblk0, sector 160920
[    2.393204] EXT4-fs (mmcblk0p24): mounted filesystem with ordered
data mode. Opts: (null)
[    2.407231] EXT3-fs (mmcblk0p26): error: couldn't mount because of
unsupported optional features (40)
[    2.408196] EXT2-fs (mmcblk0p26): error: couldn't mount because of
unsupported optional features (44)
[    2.416686] mmcblk0: response CRC error sending r/w cmd command,
card status 0xb00
[    2.426058] mmcblk0: response CRC error sending r/w cmd command,
card status 0xb00
[    2.446754] mmcblk0: response CRC error sending r/w cmd command,
card status 0xb00
[    2.510905] mmcblk0: response CRC error sending r/w cmd command,
card status 0xb00
[    2.559272] mmcblk0: response CRC error sending r/w cmd command,
card status 0xb00
[    2.622231] mmcblk0: response CRC error sending r/w cmd command,
card status 0xb00
[    2.623297] mmcblk0: error -84 transferring data, sector 5492888,
nr 192, cmd response 0x900, card status 0xb00
[    2.628702] mmcblk0: retrying using single block read
[    2.640005] mmcblk0: response CRC error sending r/w cmd command,
card status 0xb00
[    2.645574] mmcblk0: response CRC error sending r/w cmd command,
card status 0xb00
[    2.652991] mmcblk0: response CRC error sending r/w cmd command,
card status 0xb00
[    2.659765] mmcblk0: response CRC error sending r/w cmd command,
card status 0xb00
[    2.668126] mmcblk0: response CRC error sending r/w cmd command,
card status 0xb00
[    2.674257] JBD2: Failed to read block at offset 283
[    2.681602] JBD2: recovery failed
[    2.686819] EXT4-fs (mmcblk0p26): error loading journal

Any suggestions of what I could do to help you debug this?

Regards,
Bjorn

Re: [PATCH RFC 00/12] Add Qualcomm SD Card Controller support.

[Srinivas Kandagatla]

Thanks Bjorn for testing this series.

On 19/05/14 23:08, Bjorn Andersson wrote:
> On Mon, Apr 21, 2014 at 2:43 PM,<srinivas.kandagatla@linaro.org>  wrote:
>> >From: Srinivas Kandagatla<srinivas.kandagatla@linaro.org>
> [snip]
>> >
>> >This patches are tested in PIO mode on IFC8064 board with both eMMC and
>> >external SD card. I would appreciate any feedback/suggestions on the overall
>> >approach.
> Hi Srini,
>
> I finally got around to give this series a spin on our APQ8064 based
> devices as well. I had a few reported CRC issues on our msm8960 based
> devices, but thinks seems functionally "ok".
>
> On APQ8064, the kernel finds the onboard eMMC and when mounting it I
> get the following:
>
> [    1.624118] mmc0: new high speed MMC card at address 0001
> [    1.627521] mmcblk0: mmc0:0001 MAG2GA 14.5 GiB
> [    1.632714] mmcblk0boot0: mmc0:0001 MAG2GA partition 1 2.00 MiB
> [    1.636989] mmcblk0boot1: mmc0:0001 MAG2GA partition 2 2.00 MiB
> [    1.637393] Freeing unused kernel memory: 332K (c09c6000 - c0a19000)
> [    1.663379] mmcblk0: response CRC error sending r/w cmd command,
>
> [    2.292914] mmcblk0: response CRC error sending r/w cmd command,
> card status 0xb00
> [    2.300985] mmcblk0: response CRC error sending r/w cmd command,
>
>
> Any suggestions of what I could do to help you debug this?
>

There might be couple of reasons why this might happen,

First is the regluators. currently the upstream kernel does not have 
support for regulators. So, does the eMMC on your board work perfectly 
fine with the default 2.7v?

secondly it could be eMMC vendor specific quirks.

I stress tested eMMC on IFC6410 and I did not hit any issues as you 
mentioned.

Are you aware of any such quirks specific to the eMMC on your board?

thanks,
srini

> Regards,
> Bjorn