[PATCH v4 00/13] Add Qualcomm SD Card Controller support

[PATCH v4 00/13] Add Qualcomm SD Card Controller support

[srinivas.kandagatla]

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

Thankyou Linus W, Ulf H and everyone for reviewing RFC to v3 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 in between writes to
CLKCTRL/POWER/DATACTRL/COMMAND registers. Most of the delays are taken care with
the existing driver except delay for the COMMAND register was too small. 
This patch fixes it.
  mmc: mmci: Add enough delay between writes to CMD register.

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 f_max to variant structure
  mmc: mmci: add explicit 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.
Bjorn also confirmed that there are no more CRC errors seen on sony platform.

Changes from v3:
	- moved pio_read to a function pointer so as to reduce additional cycles
	in hot-path, suggested by Ulf.
	- simplify the flags used for explicit mclk control, suggested by Ulf.
	- fixed issues in cacluating f_max and f_min pointed and suggested by Ulf.
	- removed unessary DDR flags on un-supported STE variants.
	- used BIT macros as suggested by Ulf.
	- removed the read/write wrappers with delays, and used most optimal way
	to introduce the delays to the only registers that require delays.

Changes from v2:
	- merged fbclk latch patch with clkreg_enable patch as suggested by Linus W.
	- remove qcom prefix for explicit clk control pointed by Linus W.
	- cleaned up mmci_qcom_pio_read and consider SDIO as suggested by Linus W.

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.

If its not too late, Am hoping to get this for v3.16.

All these patches are tested on IF6410 board on both eMMC and external SD card.

Thanks,
srini

Srinivas Kandagatla (13):
  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 enough delay between writes to CMD register.
  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 f_max to variant structure
  mmc: mmci: add explicit clk control
  mmc: mmci: Add Qcom specific pio_read function.

 drivers/mmc/host/mmci.c | 203 +++++++++++++++++++++++++++++++++---------
 drivers/mmc/host/mmci.h | 228 ++++++++++++++++++++++++++----------------------
 2 files changed, 288 insertions(+), 143 deletions(-)

-- 
1.9.1

[PATCH v4 01/13] 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 v4 02/13] 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 v4 03/13] 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 | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index a38e714..86f25a9 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -160,6 +160,14 @@ 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,
+	.pwrreg_powerup		= MCI_PWR_UP,
+};
+
 static int mmci_card_busy(struct mmc_host *mmc)
 {
 	struct mmci_host *host = mmc_priv(mmc);
@@ -1750,6 +1758,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

Re: [PATCH v4 03/13] mmc: mmci: Add Qualcomm Id to amba id table

[Ulf Hansson]

On 28 May 2014 15:46,  <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>
> ---
>  drivers/mmc/host/mmci.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index a38e714..86f25a9 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -160,6 +160,14 @@ 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,
> +       .pwrreg_powerup         = MCI_PWR_UP,
> +};
> +
>  static int mmci_card_busy(struct mmc_host *mmc)
>  {
>         struct mmci_host *host = mmc_priv(mmc);
> @@ -1750,6 +1758,12 @@ static struct amba_id mmci_ids[] = {
>                 .mask   = 0xf0ffffff,
>                 .data   = &variant_ux500v2,
>         },
> +       /* Qualcomm variants */
> +       {
> +               .id     = 0x00051180,
> +               .mask   = 0x000fffff,
> +               .data   = &variant_qcom,
> +       },
>         { 0, 0 },
>  };

Shouldn't this patch be moved to very end of this patchset?

If we would apply this patch on it's own - the Qcom variant wouldn't
work, right?

Kind regards
Ulf Hansson

>
> --
> 1.9.1
>

Re: [PATCH v4 03/13] mmc: mmci: Add Qualcomm Id to amba id table

[Srinivas Kandagatla]

On 30/05/14 10:39, Ulf Hansson wrote:
> On 28 May 2014 15:46,  <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>
>> ---
>>   drivers/mmc/host/mmci.c | 14 ++++++++++++++
>>   1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
>> index a38e714..86f25a9 100644
>> --- a/drivers/mmc/host/mmci.c
>> +++ b/drivers/mmc/host/mmci.c
>> @@ -160,6 +160,14 @@ 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,
>> +       .pwrreg_powerup         = MCI_PWR_UP,
>> +};
>> +
>>   static int mmci_card_busy(struct mmc_host *mmc)
>>   {
>>          struct mmci_host *host = mmc_priv(mmc);
>> @@ -1750,6 +1758,12 @@ static struct amba_id mmci_ids[] = {
>>                  .mask   = 0xf0ffffff,
>>                  .data   = &variant_ux500v2,
>>          },
>> +       /* Qualcomm variants */
>> +       {
>> +               .id     = 0x00051180,
>> +               .mask   = 0x000fffff,
>> +               .data   = &variant_qcom,
>> +       },
>>          { 0, 0 },
>>   };
>
> Shouldn't this patch be moved to very end of this patchset?
>
> If we would apply this patch on it's own - the Qcom variant wouldn't
> work, right?
Yes, I would not work. I will move it to the end of the patchset.

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

[PATCH v4 04/13] mmc: mmci: Add enough delay between writes to CMD register.

[srinivas.kandagatla]

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

On Qcom SD Card controller POWER, CLKCTRL, DATACTRL and COMMAND registers
should be updated in MCLK domain, and writes to these registers must be
separated by three MCLK cycles. This resitriction is not applicable for
other registers. Any subsequent writes to these register will be ignored
until 3 MCLK have passed.

One usec delay between two CMD register writes is not sufficient in the
card identification phase where the CCLK is very low. This patch replaces
a static 1 usec delay to use mmci_reg_delay function which can provide
correct delay depending on the cclk frequency.

Without this patch the card is not detected.

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 86f25a9..aa2d381 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -818,7 +818,7 @@ mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c)
 
 	if (readl(base + MMCICOMMAND) & MCI_CPSM_ENABLE) {
 		writel(0, base + MMCICOMMAND);
-		udelay(1);
+		mmci_reg_delay(host);
 	}
 
 	c |= cmd->opcode | MCI_CPSM_ENABLE;
-- 
1.9.1

[PATCH v4 05/13] 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 aa2d381..23401b0 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,
 	.pwrreg_powerup		= MCI_PWR_UP,
 };
@@ -739,6 +743,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 v4 06/13] 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>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/mmc/host/mmci.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 23401b0..729105b 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
@@ -74,6 +75,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;
@@ -152,6 +154,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,
@@ -779,7 +782,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 v4 06/13] mmc: mmci: add ddrmode mask to variant data

[Ulf Hansson]

On 28 May 2014 15:46,  <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.

The above statement seems not correct. We don't have any issues
currently, right. :-)

Kind regards
Ulf Hansson

>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/mmc/host/mmci.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index 23401b0..729105b 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
> @@ -74,6 +75,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;
> @@ -152,6 +154,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,
> @@ -779,7 +782,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 v4 06/13] mmc: mmci: add ddrmode mask to variant data

[Srinivas Kandagatla]

On 30/05/14 10:35, Ulf Hansson wrote:
> On 28 May 2014 15:46,  <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.
>
> The above statement seems not correct. We don't have any issues
> currently, right. :-)
I should have said Qualcomm instead of generalizing it to non-ST SOCs.
Will fix this in next version.

Thanks,
srini
>
> Kind regards
> Ulf Hansson
>
>>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>> ---
>>   drivers/mmc/host/mmci.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
>> index 23401b0..729105b 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
>> @@ -74,6 +75,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;
>> @@ -152,6 +154,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,
>> @@ -779,7 +782,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
>>

[PATCH v4 07/13] 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>
Reviewed-by: Linus Walleij <linus.walleij@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 729105b..2f4cdf3 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)
@@ -72,6 +73,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;
@@ -113,6 +115,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,
 	.datalength_bits	= 16,
 	.sdio			= true,
 	.pwrreg_powerup		= MCI_PWR_ON,
@@ -139,6 +142,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,
 	.datalength_bits	= 24,
 	.sdio			= true,
 	.st_clkdiv		= true,
@@ -154,6 +158,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,
@@ -314,7 +319,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

[PATCH v4 08/13] 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>
Reviewed-by: Linus Walleij <linus.walleij@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 2f4cdf3..8deea4a 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)
@@ -74,6 +75,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;
@@ -143,6 +145,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,
 	.datalength_bits	= 24,
 	.sdio			= true,
 	.st_clkdiv		= true,
@@ -159,6 +162,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,
@@ -322,7 +326,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

[PATCH v4 09/13] 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>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/mmc/host/mmci.c |  4 ++++
 drivers/mmc/host/mmci.h | 17 +++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 8deea4a..dbcb952 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -179,6 +179,10 @@ static struct variant_data variant_qcom = {
 	.fifosize		= 16 * 4,
 	.fifohalfsize		= 8 * 4,
 	.clkreg			= MCI_CLK_ENABLE,
+	.clkreg_enable		= MCI_QCOM_CLK_FLOWENA |
+				  MCI_QCOM_CLK_SELECT_IN_FBCLK,
+	.clkreg_8bit_bus_enable = MCI_QCOM_CLK_WIDEBUS_8,
+	.datactrl_mask_ddrmode	= MCI_QCOM_CLK_SELECT_IN_DDR_MODE,
 	.blksz_datactrl4	= true,
 	.datalength_bits	= 24,
 	.pwrreg_powerup		= MCI_PWR_UP,
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index cd83ca3..706eb513 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -41,6 +41,15 @@
 /* Modified PL180 on Versatile Express platform */
 #define MCI_ARM_HWFCEN		BIT(12)
 
+/* Modified on Qualcomm Integrations */
+#define MCI_QCOM_CLK_WIDEBUS_8	(BIT(10) | BIT(11))
+#define MCI_QCOM_CLK_FLOWENA	BIT(12)
+#define MCI_QCOM_CLK_INVERTOUT	BIT(13)
+
+/* select in latch data and command in */
+#define MCI_QCOM_CLK_SELECT_IN_FBCLK	BIT(15)
+#define MCI_QCOM_CLK_SELECT_IN_DDR_MODE	(BIT(14) | BIT(15))
+
 #define MMCIARGUMENT		0x008
 #define MMCICOMMAND		0x00c
 #define MCI_CPSM_RESPONSE	BIT(6)
@@ -54,6 +63,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 v4 09/13] mmc: mmci: add Qcom specifics of clk and datactrl registers.

[Ulf Hansson]

On 28 May 2014 15:47,  <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>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/mmc/host/mmci.c |  4 ++++
>  drivers/mmc/host/mmci.h | 17 +++++++++++++++++
>  2 files changed, 21 insertions(+)
>
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index 8deea4a..dbcb952 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -179,6 +179,10 @@ static struct variant_data variant_qcom = {
>         .fifosize               = 16 * 4,
>         .fifohalfsize           = 8 * 4,
>         .clkreg                 = MCI_CLK_ENABLE,
> +       .clkreg_enable          = MCI_QCOM_CLK_FLOWENA |
> +                                 MCI_QCOM_CLK_SELECT_IN_FBCLK,
> +       .clkreg_8bit_bus_enable = MCI_QCOM_CLK_WIDEBUS_8,
> +       .datactrl_mask_ddrmode  = MCI_QCOM_CLK_SELECT_IN_DDR_MODE,

This stuff should go in patch3.

>         .blksz_datactrl4        = true,
>         .datalength_bits        = 24,
>         .pwrreg_powerup         = MCI_PWR_UP,
> diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
> index cd83ca3..706eb513 100644
> --- a/drivers/mmc/host/mmci.h
> +++ b/drivers/mmc/host/mmci.h
> @@ -41,6 +41,15 @@
>  /* Modified PL180 on Versatile Express platform */
>  #define MCI_ARM_HWFCEN         BIT(12)
>
> +/* Modified on Qualcomm Integrations */
> +#define MCI_QCOM_CLK_WIDEBUS_8 (BIT(10) | BIT(11))
> +#define MCI_QCOM_CLK_FLOWENA   BIT(12)
> +#define MCI_QCOM_CLK_INVERTOUT BIT(13)
> +
> +/* select in latch data and command in */
> +#define MCI_QCOM_CLK_SELECT_IN_FBCLK   BIT(15)
> +#define MCI_QCOM_CLK_SELECT_IN_DDR_MODE        (BIT(14) | BIT(15))
> +
>  #define MMCIARGUMENT           0x008
>  #define MMCICOMMAND            0x00c
>  #define MCI_CPSM_RESPONSE      BIT(6)
> @@ -54,6 +63,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)
> +

Maybe you should have one patch in the beginning of the patchset, that
adds all the new QCOM bits in the header file? Instead of splitting
them up?

>  #define MMCIRESPCMD            0x010
>  #define MMCIRESPONSE0          0x014
>  #define MMCIRESPONSE1          0x018
> --
> 1.9.1
>

Kind regards
Ulf Hansson

Re: [PATCH v4 09/13] mmc: mmci: add Qcom specifics of clk and datactrl registers.

[Srinivas Kandagatla]

On 30/05/14 10:55, Ulf Hansson wrote:
> On 28 May 2014 15:47,  <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>
>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>> ---
>>   drivers/mmc/host/mmci.c |  4 ++++
>>   drivers/mmc/host/mmci.h | 17 +++++++++++++++++
>>   2 files changed, 21 insertions(+)
>>
>> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
>> index 8deea4a..dbcb952 100644
>> --- a/drivers/mmc/host/mmci.c
>> +++ b/drivers/mmc/host/mmci.c
>> @@ -179,6 +179,10 @@ static struct variant_data variant_qcom = {
>>          .fifosize               = 16 * 4,
>>          .fifohalfsize           = 8 * 4,
>>          .clkreg                 = MCI_CLK_ENABLE,
>> +       .clkreg_enable          = MCI_QCOM_CLK_FLOWENA |
>> +                                 MCI_QCOM_CLK_SELECT_IN_FBCLK,
>> +       .clkreg_8bit_bus_enable = MCI_QCOM_CLK_WIDEBUS_8,
>> +       .datactrl_mask_ddrmode  = MCI_QCOM_CLK_SELECT_IN_DDR_MODE,
>
> This stuff should go in patch3.

Yes, I will merge patch3 and this change-set in next version.

>
>>          .blksz_datactrl4        = true,
>>          .datalength_bits        = 24,
>>          .pwrreg_powerup         = MCI_PWR_UP,
>> diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
>> index cd83ca3..706eb513 100644
>> --- a/drivers/mmc/host/mmci.h
>> +++ b/drivers/mmc/host/mmci.h
>> @@ -41,6 +41,15 @@
>>   /* Modified PL180 on Versatile Express platform */
>>   #define MCI_ARM_HWFCEN         BIT(12)
>>
>> +/* Modified on Qualcomm Integrations */
>> +#define MCI_QCOM_CLK_WIDEBUS_8 (BIT(10) | BIT(11))
>> +#define MCI_QCOM_CLK_FLOWENA   BIT(12)
>> +#define MCI_QCOM_CLK_INVERTOUT BIT(13)
>> +
>> +/* select in latch data and command in */
>> +#define MCI_QCOM_CLK_SELECT_IN_FBCLK   BIT(15)
>> +#define MCI_QCOM_CLK_SELECT_IN_DDR_MODE        (BIT(14) | BIT(15))
>> +
>>   #define MMCIARGUMENT           0x008
>>   #define MMCICOMMAND            0x00c
>>   #define MCI_CPSM_RESPONSE      BIT(6)
>> @@ -54,6 +63,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)
>> +
>
> Maybe you should have one patch in the beginning of the patchset, that
> adds all the new QCOM bits in the header file? Instead of splitting
> them up?
Ok, I will do that in next version.

Thanks,
srini
>
>>   #define MMCIRESPCMD            0x010
>>   #define MMCIRESPONSE0          0x014
>>   #define MMCIRESPONSE1          0x018
>> --
>> 1.9.1
>>
>
> Kind regards
> Ulf Hansson
>

[PATCH v4 10/13] 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>
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 dbcb952..fd40f9a 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.
@@ -79,6 +80,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;
@@ -183,6 +185,7 @@ static struct variant_data variant_qcom = {
 				  MCI_QCOM_CLK_SELECT_IN_FBCLK,
 	.clkreg_8bit_bus_enable = MCI_QCOM_CLK_WIDEBUS_8,
 	.datactrl_mask_ddrmode	= MCI_QCOM_CLK_SELECT_IN_DDR_MODE,
+	.data_cmd_enable	= MCI_QCOM_CSPM_DATCMD,
 	.blksz_datactrl4	= true,
 	.datalength_bits	= 24,
 	.pwrreg_powerup		= MCI_PWR_UP,
@@ -852,6 +855,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;
 
 	writel(cmd->arg, base + MMCIARGUMENT);
-- 
1.9.1

[PATCH v4 11/13] mmc: mmci: add f_max to variant structure

[srinivas.kandagatla]

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

Some of the controller have maximum supported frequency, This patch adds
support in variant data structure to specify such restrictions. This
gives more flexibility in calculating the f_max before passing it to
mmc-core.

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

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index fd40f9a..202f2d5 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -67,6 +67,7 @@ static unsigned int fmax = 515633;
  * @blksz_datactrl4: true if Block size is at b4..b16 position in datactrl
  *		     register
  * @pwrreg_powerup: power up value for MMCIPOWER register
+ * @f_max: maximum clk frequency supported by the controller.
  * @signal_direction: input/out direction of bus signals can be indicated
  * @pwrreg_clkgate: MMCIPOWER register must be used to gate the clock
  * @busy_detect: true if busy detection on dat0 is supported
@@ -87,6 +88,7 @@ struct variant_data {
 	bool			blksz_datactrl16;
 	bool			blksz_datactrl4;
 	u32			pwrreg_powerup;
+	u32			f_max;
 	bool			signal_direction;
 	bool			pwrreg_clkgate;
 	bool			busy_detect;
@@ -98,6 +100,7 @@ static struct variant_data variant_arm = {
 	.fifohalfsize		= 8 * 4,
 	.datalength_bits	= 16,
 	.pwrreg_powerup		= MCI_PWR_UP,
+	.f_max			= 100000000,
 };
 
 static struct variant_data variant_arm_extended_fifo = {
@@ -105,6 +108,7 @@ static struct variant_data variant_arm_extended_fifo = {
 	.fifohalfsize		= 64 * 4,
 	.datalength_bits	= 16,
 	.pwrreg_powerup		= MCI_PWR_UP,
+	.f_max			= 100000000,
 };
 
 static struct variant_data variant_arm_extended_fifo_hwfc = {
@@ -113,6 +117,7 @@ static struct variant_data variant_arm_extended_fifo_hwfc = {
 	.clkreg_enable		= MCI_ARM_HWFCEN,
 	.datalength_bits	= 16,
 	.pwrreg_powerup		= MCI_PWR_UP,
+	.f_max			= 100000000,
 };
 
 static struct variant_data variant_u300 = {
@@ -123,6 +128,7 @@ static struct variant_data variant_u300 = {
 	.datalength_bits	= 16,
 	.sdio			= true,
 	.pwrreg_powerup		= MCI_PWR_ON,
+	.f_max			= 100000000,
 	.signal_direction	= true,
 	.pwrreg_clkgate		= true,
 	.pwrreg_nopower		= true,
@@ -136,6 +142,7 @@ static struct variant_data variant_nomadik = {
 	.sdio			= true,
 	.st_clkdiv		= true,
 	.pwrreg_powerup		= MCI_PWR_ON,
+	.f_max			= 100000000,
 	.signal_direction	= true,
 	.pwrreg_clkgate		= true,
 	.pwrreg_nopower		= true,
@@ -152,6 +159,7 @@ static struct variant_data variant_ux500 = {
 	.sdio			= true,
 	.st_clkdiv		= true,
 	.pwrreg_powerup		= MCI_PWR_ON,
+	.f_max			= 100000000,
 	.signal_direction	= true,
 	.pwrreg_clkgate		= true,
 	.busy_detect		= true,
@@ -171,6 +179,7 @@ static struct variant_data variant_ux500v2 = {
 	.st_clkdiv		= true,
 	.blksz_datactrl16	= true,
 	.pwrreg_powerup		= MCI_PWR_ON,
+	.f_max			= 100000000,
 	.signal_direction	= true,
 	.pwrreg_clkgate		= true,
 	.busy_detect		= true,
@@ -189,6 +198,7 @@ static struct variant_data variant_qcom = {
 	.blksz_datactrl4	= true,
 	.datalength_bits	= 24,
 	.pwrreg_powerup		= MCI_PWR_UP,
+	.f_max			= 208000000,
 };
 
 static int mmci_card_busy(struct mmc_host *mmc)
@@ -1485,8 +1495,8 @@ static int mmci_probe(struct amba_device *dev,
 	 * so we try to adjust the clock down to this,
 	 * (if possible).
 	 */
-	if (host->mclk > 100000000) {
-		ret = clk_set_rate(host->clk, 100000000);
+	if (host->mclk > host->variant->f_max) {
+		ret = clk_set_rate(host->clk, host->variant->f_max);
 		if (ret < 0)
 			goto clk_disable;
 		host->mclk = clk_get_rate(host->clk);
-- 
1.9.1

Re: [PATCH v4 11/13] mmc: mmci: add f_max to variant structure

[Ulf Hansson]

On 28 May 2014 15:47,  <srinivas.kandagatla@linaro.org> wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> Some of the controller have maximum supported frequency, This patch adds
> support in variant data structure to specify such restrictions. This
> gives more flexibility in calculating the f_max before passing it to
> mmc-core.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>  drivers/mmc/host/mmci.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index fd40f9a..202f2d5 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -67,6 +67,7 @@ static unsigned int fmax = 515633;
>   * @blksz_datactrl4: true if Block size is at b4..b16 position in datactrl
>   *                  register
>   * @pwrreg_powerup: power up value for MMCIPOWER register
> + * @f_max: maximum clk frequency supported by the controller.
>   * @signal_direction: input/out direction of bus signals can be indicated
>   * @pwrreg_clkgate: MMCIPOWER register must be used to gate the clock
>   * @busy_detect: true if busy detection on dat0 is supported
> @@ -87,6 +88,7 @@ struct variant_data {
>         bool                    blksz_datactrl16;
>         bool                    blksz_datactrl4;
>         u32                     pwrreg_powerup;
> +       u32                     f_max;
>         bool                    signal_direction;
>         bool                    pwrreg_clkgate;
>         bool                    busy_detect;
> @@ -98,6 +100,7 @@ static struct variant_data variant_arm = {
>         .fifohalfsize           = 8 * 4,
>         .datalength_bits        = 16,
>         .pwrreg_powerup         = MCI_PWR_UP,
> +       .f_max                  = 100000000,
>  };
>
>  static struct variant_data variant_arm_extended_fifo = {
> @@ -105,6 +108,7 @@ static struct variant_data variant_arm_extended_fifo = {
>         .fifohalfsize           = 64 * 4,
>         .datalength_bits        = 16,
>         .pwrreg_powerup         = MCI_PWR_UP,
> +       .f_max                  = 100000000,
>  };
>
>  static struct variant_data variant_arm_extended_fifo_hwfc = {
> @@ -113,6 +117,7 @@ static struct variant_data variant_arm_extended_fifo_hwfc = {
>         .clkreg_enable          = MCI_ARM_HWFCEN,
>         .datalength_bits        = 16,
>         .pwrreg_powerup         = MCI_PWR_UP,
> +       .f_max                  = 100000000,
>  };
>
>  static struct variant_data variant_u300 = {
> @@ -123,6 +128,7 @@ static struct variant_data variant_u300 = {
>         .datalength_bits        = 16,
>         .sdio                   = true,
>         .pwrreg_powerup         = MCI_PWR_ON,
> +       .f_max                  = 100000000,
>         .signal_direction       = true,
>         .pwrreg_clkgate         = true,
>         .pwrreg_nopower         = true,
> @@ -136,6 +142,7 @@ static struct variant_data variant_nomadik = {
>         .sdio                   = true,
>         .st_clkdiv              = true,
>         .pwrreg_powerup         = MCI_PWR_ON,
> +       .f_max                  = 100000000,
>         .signal_direction       = true,
>         .pwrreg_clkgate         = true,
>         .pwrreg_nopower         = true,
> @@ -152,6 +159,7 @@ static struct variant_data variant_ux500 = {
>         .sdio                   = true,
>         .st_clkdiv              = true,
>         .pwrreg_powerup         = MCI_PWR_ON,
> +       .f_max                  = 100000000,
>         .signal_direction       = true,
>         .pwrreg_clkgate         = true,
>         .busy_detect            = true,
> @@ -171,6 +179,7 @@ static struct variant_data variant_ux500v2 = {
>         .st_clkdiv              = true,
>         .blksz_datactrl16       = true,
>         .pwrreg_powerup         = MCI_PWR_ON,
> +       .f_max                  = 100000000,
>         .signal_direction       = true,
>         .pwrreg_clkgate         = true,
>         .busy_detect            = true,
> @@ -189,6 +198,7 @@ static struct variant_data variant_qcom = {
>         .blksz_datactrl4        = true,
>         .datalength_bits        = 24,
>         .pwrreg_powerup         = MCI_PWR_UP,
> +       .f_max                  = 208000000,
>  };
>
>  static int mmci_card_busy(struct mmc_host *mmc)
> @@ -1485,8 +1495,8 @@ static int mmci_probe(struct amba_device *dev,
>          * so we try to adjust the clock down to this,
>          * (if possible).
>          */
> -       if (host->mclk > 100000000) {
> -               ret = clk_set_rate(host->clk, 100000000);
> +       if (host->mclk > host->variant->f_max) {

You can use the local variant pointer directly, instead of host->variant.

> +               ret = clk_set_rate(host->clk, host->variant->f_max);
>                 if (ret < 0)
>                         goto clk_disable;
>                 host->mclk = clk_get_rate(host->clk);
> --
> 1.9.1
>

Looks good otherwise!

Kind regards
Ulf Hansson

Re: [PATCH v4 11/13] mmc: mmci: add f_max to variant structure

[Srinivas Kandagatla]

Thanks Ulf,

On 30/05/14 11:28, Ulf Hansson wrote:
>>           */
>> >-       if (host->mclk > 100000000) {
>> >-               ret = clk_set_rate(host->clk, 100000000);
>> >+       if (host->mclk > host->variant->f_max) {
> You can use the local variant pointer directly, instead of host->variant.
>
yes, Will do that in next version.

Thanks,
srini
>> >+               ret = clk_set_rate(host->clk, host->variant->f_max);
>> >                 if (ret < 0)
>> >                         goto clk_disable;
>> >                 host->mclk = clk_get_rate(host->clk);
>> >--

[PATCH v4 12/13] mmc: mmci: add explicit clk control

[srinivas.kandagatla]

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

On Controllers like Qcom SD card controller where 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 flag in variant structure giving more flexibility
to the driver.

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

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 202f2d5..6eb0a29 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -72,6 +72,7 @@ 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
+ * @explicit_mclk_control: enable explicit mclk control in driver.
  */
 struct variant_data {
 	unsigned int		clkreg;
@@ -93,6 +94,7 @@ struct variant_data {
 	bool			pwrreg_clkgate;
 	bool			busy_detect;
 	bool			pwrreg_nopower;
+	bool			explicit_mclk_control;
 };
 
 static struct variant_data variant_arm = {
@@ -199,6 +201,7 @@ static struct variant_data variant_qcom = {
 	.datalength_bits	= 24,
 	.pwrreg_powerup		= MCI_PWR_UP,
 	.f_max			= 208000000,
+	.explicit_mclk_control	= true,
 };
 
 static int mmci_card_busy(struct mmc_host *mmc)
@@ -301,7 +304,9 @@ static void mmci_set_clkreg(struct mmci_host *host, unsigned int desired)
 	host->cclk = 0;
 
 	if (desired) {
-		if (desired >= host->mclk) {
+		if (variant->explicit_mclk_control) {
+			host->cclk = host->mclk;
+		} else if (desired >= host->mclk) {
 			clk = MCI_CLK_BYPASS;
 			if (variant->st_clkdiv)
 				clk |= MCI_ST_UX500_NEG_EDGE;
@@ -1340,6 +1345,18 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	if (!ios->clock && variant->pwrreg_clkgate)
 		pwr &= ~MCI_PWR_ON;
 
+	if ((host->variant->explicit_mclk_control) &&
+	    (ios->clock != host->mclk_req)) {
+		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->mclk_req = ios->clock;
+		}
+	}
+
 	spin_lock_irqsave(&host->lock, flags);
 
 	mmci_set_clkreg(host, ios->clock);
@@ -1490,19 +1507,6 @@ static int mmci_probe(struct amba_device *dev,
 	host->plat = plat;
 	host->variant = variant;
 	host->mclk = clk_get_rate(host->clk);
-	/*
-	 * According to the spec, mclk is max 100 MHz,
-	 * so we try to adjust the clock down to this,
-	 * (if possible).
-	 */
-	if (host->mclk > host->variant->f_max) {
-		ret = clk_set_rate(host->clk, host->variant->f_max);
-		if (ret < 0)
-			goto clk_disable;
-		host->mclk = clk_get_rate(host->clk);
-		dev_dbg(mmc_dev(mmc), "eventual mclk rate: %u Hz\n",
-			host->mclk);
-	}
 
 	host->phybase = dev->res.start;
 	host->base = devm_ioremap_resource(&dev->dev, &dev->res);
@@ -1511,25 +1515,51 @@ static int mmci_probe(struct amba_device *dev,
 		goto clk_disable;
 	}
 
-	/*
-	 * The ARM and ST versions of the block have slightly different
-	 * clock divider equations which means that the minimum divider
-	 * differs too.
-	 */
-	if (variant->st_clkdiv)
-		mmc->f_min = DIV_ROUND_UP(host->mclk, 257);
-	else
-		mmc->f_min = DIV_ROUND_UP(host->mclk, 512);
-	/*
-	 * If no maximum operating frequency is supplied, fall back to use
-	 * the module parameter, which has a (low) default value in case it
-	 * 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 (variant->explicit_mclk_control) {
+		/* get the nearest minimum clock to 100Khz */
+		mmc->f_min = clk_round_rate(host->clk, 100000);
+
+		if (mmc->f_max)
+			mmc->f_max = min(host->variant->f_max, mmc->f_max);
+		else
+			mmc->f_max = min(host->variant->f_max, fmax);
+
+	} else {
+		/*
+		 * According to the spec, mclk is max 100 MHz,
+		 * so we try to adjust the clock down to this,
+		 * (if possible).
+		 */
+		if (host->mclk > host->variant->f_max) {
+			ret = clk_set_rate(host->clk, host->variant->f_max);
+			if (ret < 0)
+				goto clk_disable;
+			host->mclk = clk_get_rate(host->clk);
+			dev_dbg(mmc_dev(mmc), "eventual mclk rate: %u Hz\n",
+				host->mclk);
+		}
+		/*
+		 * The ARM and ST versions of the block have slightly different
+		 * clock divider equations which means that the minimum divider
+		 * differs too.
+		 */
+		if (variant->st_clkdiv)
+			mmc->f_min = DIV_ROUND_UP(host->mclk, 257);
+		else
+			mmc->f_min = DIV_ROUND_UP(host->mclk, 512);
+		/*
+		 * If no maximum operating frequency is supplied, fall back to
+		 * use the module parameter, which has a (low) default value in
+		 * case it 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);
+
+	}
+
 	dev_dbg(mmc_dev(mmc), "clocking block at %u Hz\n", mmc->f_max);
 
 	/* Get regulators and the supported OCR mask */
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 706eb513..1882e20 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -208,6 +208,8 @@ struct mmci_host {
 	spinlock_t		lock;
 
 	unsigned int		mclk;
+	/* cached value of requested clk in set_ios */
+	unsigned int		mclk_req;
 	unsigned int		cclk;
 	u32			pwr_reg;
 	u32			pwr_reg_add;
-- 
1.9.1

Re: [PATCH v4 12/13] mmc: mmci: add explicit clk control

[Ulf Hansson]

On 28 May 2014 15:47,  <srinivas.kandagatla@linaro.org> wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> On Controllers like Qcom SD card controller where 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 flag in variant structure giving more flexibility
> to the driver.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>  drivers/mmc/host/mmci.c | 96 ++++++++++++++++++++++++++++++++-----------------
>  drivers/mmc/host/mmci.h |  2 ++
>  2 files changed, 65 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index 202f2d5..6eb0a29 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -72,6 +72,7 @@ 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
> + * @explicit_mclk_control: enable explicit mclk control in driver.
>   */
>  struct variant_data {
>         unsigned int            clkreg;
> @@ -93,6 +94,7 @@ struct variant_data {
>         bool                    pwrreg_clkgate;
>         bool                    busy_detect;
>         bool                    pwrreg_nopower;
> +       bool                    explicit_mclk_control;
>  };
>
>  static struct variant_data variant_arm = {
> @@ -199,6 +201,7 @@ static struct variant_data variant_qcom = {
>         .datalength_bits        = 24,
>         .pwrreg_powerup         = MCI_PWR_UP,
>         .f_max                  = 208000000,
> +       .explicit_mclk_control  = true,

This should go in patch3 instead.

>  };
>
>  static int mmci_card_busy(struct mmc_host *mmc)
> @@ -301,7 +304,9 @@ static void mmci_set_clkreg(struct mmci_host *host, unsigned int desired)
>         host->cclk = 0;
>
>         if (desired) {
> -               if (desired >= host->mclk) {
> +               if (variant->explicit_mclk_control) {
> +                       host->cclk = host->mclk;
> +               } else if (desired >= host->mclk) {
>                         clk = MCI_CLK_BYPASS;
>                         if (variant->st_clkdiv)
>                                 clk |= MCI_ST_UX500_NEG_EDGE;
> @@ -1340,6 +1345,18 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>         if (!ios->clock && variant->pwrreg_clkgate)
>                 pwr &= ~MCI_PWR_ON;
>
> +       if ((host->variant->explicit_mclk_control) &&
> +           (ios->clock != host->mclk_req)) {
> +               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->mclk_req = ios->clock;
> +               }
> +       }
> +
>         spin_lock_irqsave(&host->lock, flags);
>
>         mmci_set_clkreg(host, ios->clock);
> @@ -1490,19 +1507,6 @@ static int mmci_probe(struct amba_device *dev,
>         host->plat = plat;
>         host->variant = variant;
>         host->mclk = clk_get_rate(host->clk);
> -       /*
> -        * According to the spec, mclk is max 100 MHz,
> -        * so we try to adjust the clock down to this,
> -        * (if possible).
> -        */
> -       if (host->mclk > host->variant->f_max) {
> -               ret = clk_set_rate(host->clk, host->variant->f_max);
> -               if (ret < 0)
> -                       goto clk_disable;
> -               host->mclk = clk_get_rate(host->clk);
> -               dev_dbg(mmc_dev(mmc), "eventual mclk rate: %u Hz\n",
> -                       host->mclk);
> -       }

The above code is relevant for Qcom as well, I don't think you need
change/move it.

>
>         host->phybase = dev->res.start;
>         host->base = devm_ioremap_resource(&dev->dev, &dev->res);
> @@ -1511,25 +1515,51 @@ static int mmci_probe(struct amba_device *dev,
>                 goto clk_disable;
>         }
>
> -       /*
> -        * The ARM and ST versions of the block have slightly different
> -        * clock divider equations which means that the minimum divider
> -        * differs too.
> -        */
> -       if (variant->st_clkdiv)
> -               mmc->f_min = DIV_ROUND_UP(host->mclk, 257);

I think you could simplify the change of fixing with f_min and f_max.

Start by, just add another statement here "else if
(variant->explicit_mclk_control)" and do the clk_round_rate()

> -       else
> -               mmc->f_min = DIV_ROUND_UP(host->mclk, 512);
> -       /*
> -        * If no maximum operating frequency is supplied, fall back to use
> -        * the module parameter, which has a (low) default value in case it
> -        * 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);

Further replace the above with:
if (mmc->f_max)
     mmc->f_max = variant->explicit_mclk_control ? min(mmc->f_max,
variant->f_max) : min(host->mclk, mmc->f_max);
else
     mmc->f_max = variant->explicit_mclk_control ? fmax : min(host->mclk, fmax);

That should do it, right?

> +       if (variant->explicit_mclk_control) {
> +               /* get the nearest minimum clock to 100Khz */
> +               mmc->f_min = clk_round_rate(host->clk, 100000);
> +
> +               if (mmc->f_max)
> +                       mmc->f_max = min(host->variant->f_max, mmc->f_max);
> +               else
> +                       mmc->f_max = min(host->variant->f_max, fmax);
> +
> +       } else {
> +               /*
> +                * According to the spec, mclk is max 100 MHz,
> +                * so we try to adjust the clock down to this,
> +                * (if possible).
> +                */
> +               if (host->mclk > host->variant->f_max) {
> +                       ret = clk_set_rate(host->clk, host->variant->f_max);
> +                       if (ret < 0)
> +                               goto clk_disable;
> +                       host->mclk = clk_get_rate(host->clk);
> +                       dev_dbg(mmc_dev(mmc), "eventual mclk rate: %u Hz\n",
> +                               host->mclk);
> +               }
> +               /*
> +                * The ARM and ST versions of the block have slightly different
> +                * clock divider equations which means that the minimum divider
> +                * differs too.
> +                */
> +               if (variant->st_clkdiv)
> +                       mmc->f_min = DIV_ROUND_UP(host->mclk, 257);
> +               else
> +                       mmc->f_min = DIV_ROUND_UP(host->mclk, 512);
> +               /*
> +                * If no maximum operating frequency is supplied, fall back to
> +                * use the module parameter, which has a (low) default value in
> +                * case it 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);
> +
> +       }
> +
>         dev_dbg(mmc_dev(mmc), "clocking block at %u Hz\n", mmc->f_max);
>
>         /* Get regulators and the supported OCR mask */
> diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
> index 706eb513..1882e20 100644
> --- a/drivers/mmc/host/mmci.h
> +++ b/drivers/mmc/host/mmci.h
> @@ -208,6 +208,8 @@ struct mmci_host {
>         spinlock_t              lock;
>
>         unsigned int            mclk;
> +       /* cached value of requested clk in set_ios */
> +       unsigned int            mclk_req;

How about "clock_cache" instead?

>         unsigned int            cclk;
>         u32                     pwr_reg;
>         u32                     pwr_reg_add;
> --
> 1.9.1
>

Kind regards
Ulf Hansson

Re: [PATCH v4 12/13] mmc: mmci: add explicit clk control

[Srinivas Kandagatla]

Thanks Ulf for reviewing.

On 30/05/14 11:25, Ulf Hansson wrote:
> On 28 May 2014 15:47,  <srinivas.kandagatla@linaro.org> wrote:
>> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>
>> On Controllers like Qcom SD card controller where 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 flag in variant structure giving more flexibility
>> to the driver.
>>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> ---
>>   drivers/mmc/host/mmci.c | 96 ++++++++++++++++++++++++++++++++-----------------
>>   drivers/mmc/host/mmci.h |  2 ++
>>   2 files changed, 65 insertions(+), 33 deletions(-)
>>
>> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
>> index 202f2d5..6eb0a29 100644
>> --- a/drivers/mmc/host/mmci.c
>> +++ b/drivers/mmc/host/mmci.c
>> @@ -72,6 +72,7 @@ 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
>> + * @explicit_mclk_control: enable explicit mclk control in driver.
>>    */
>>   struct variant_data {
>>          unsigned int            clkreg;
>> @@ -93,6 +94,7 @@ struct variant_data {
>>          bool                    pwrreg_clkgate;
>>          bool                    busy_detect;
>>          bool                    pwrreg_nopower;
>> +       bool                    explicit_mclk_control;
>>   };
>>
>>   static struct variant_data variant_arm = {
>> @@ -199,6 +201,7 @@ static struct variant_data variant_qcom = {
>>          .datalength_bits        = 24,
>>          .pwrreg_powerup         = MCI_PWR_UP,
>>          .f_max                  = 208000000,
>> +       .explicit_mclk_control  = true,
>
> This should go in patch3 instead.

yes.. will get this in to the patch3 too.

>
>>   };
>>
>>   static int mmci_card_busy(struct mmc_host *mmc)
>> @@ -301,7 +304,9 @@ static void mmci_set_clkreg(struct mmci_host *host, unsigned int desired)
>>          host->cclk = 0;
>>
>>          if (desired) {
>> -               if (desired >= host->mclk) {
>> +               if (variant->explicit_mclk_control) {
>> +                       host->cclk = host->mclk;
>> +               } else if (desired >= host->mclk) {
>>                          clk = MCI_CLK_BYPASS;
>>                          if (variant->st_clkdiv)
>>                                  clk |= MCI_ST_UX500_NEG_EDGE;
>> @@ -1340,6 +1345,18 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>>          if (!ios->clock && variant->pwrreg_clkgate)
>>                  pwr &= ~MCI_PWR_ON;
>>
>> +       if ((host->variant->explicit_mclk_control) &&
>> +           (ios->clock != host->mclk_req)) {
>> +               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->mclk_req = ios->clock;
>> +               }
>> +       }
>> +
>>          spin_lock_irqsave(&host->lock, flags);
>>
>>          mmci_set_clkreg(host, ios->clock);
>> @@ -1490,19 +1507,6 @@ static int mmci_probe(struct amba_device *dev,
>>          host->plat = plat;
>>          host->variant = variant;
>>          host->mclk = clk_get_rate(host->clk);
>> -       /*
>> -        * According to the spec, mclk is max 100 MHz,
>> -        * so we try to adjust the clock down to this,
>> -        * (if possible).
>> -        */
>> -       if (host->mclk > host->variant->f_max) {
>> -               ret = clk_set_rate(host->clk, host->variant->f_max);
>> -               if (ret < 0)
>> -                       goto clk_disable;
>> -               host->mclk = clk_get_rate(host->clk);
>> -               dev_dbg(mmc_dev(mmc), "eventual mclk rate: %u Hz\n",
>> -                       host->mclk);
>> -       }
>
> The above code is relevant for Qcom as well, I don't think you need
> change/move it.
Yes, you are right. I will fix it in next version.
>
>>
>>          host->phybase = dev->res.start;
>>          host->base = devm_ioremap_resource(&dev->dev, &dev->res);
>> @@ -1511,25 +1515,51 @@ static int mmci_probe(struct amba_device *dev,
>>                  goto clk_disable;
>>          }
>>
>> -       /*
>> -        * The ARM and ST versions of the block have slightly different
>> -        * clock divider equations which means that the minimum divider
>> -        * differs too.
>> -        */
>> -       if (variant->st_clkdiv)
>> -               mmc->f_min = DIV_ROUND_UP(host->mclk, 257);
>
> I think you could simplify the change of fixing with f_min and f_max.
>
> Start by, just add another statement here "else if
> (variant->explicit_mclk_control)" and do the clk_round_rate()
>
>> -       else
>> -               mmc->f_min = DIV_ROUND_UP(host->mclk, 512);
>> -       /*
>> -        * If no maximum operating frequency is supplied, fall back to use
>> -        * the module parameter, which has a (low) default value in case it
>> -        * 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);
>
> Further replace the above with:
> if (mmc->f_max)
>       mmc->f_max = variant->explicit_mclk_control ? min(mmc->f_max,
> variant->f_max) : min(host->mclk, mmc->f_max);
> else
>       mmc->f_max = variant->explicit_mclk_control ? fmax : min(host->mclk, fmax);
>
> That should do it, right?
yes, that will work. It simplifies the logic too.

>
>> +       if (variant->explicit_mclk_control) {
>> +               /* get the nearest minimum clock to 100Khz */
>> +               mmc->f_min = clk_round_rate(host->clk, 100000);
>> +
>> +               if (mmc->f_max)
>> +                       mmc->f_max = min(host->variant->f_max, mmc->f_max);
>> +               else
>> +                       mmc->f_max = min(host->variant->f_max, fmax);
>> +
>> +       } else {
>> +               /*
>> +                * According to the spec, mclk is max 100 MHz,
>> +                * so we try to adjust the clock down to this,
>> +                * (if possible).
>> +                */
>> +               if (host->mclk > host->variant->f_max) {
>> +                       ret = clk_set_rate(host->clk, host->variant->f_max);
>> +                       if (ret < 0)
>> +                               goto clk_disable;
>> +                       host->mclk = clk_get_rate(host->clk);
>> +                       dev_dbg(mmc_dev(mmc), "eventual mclk rate: %u Hz\n",
>> +                               host->mclk);
>> +               }
>> +               /*
>> +                * The ARM and ST versions of the block have slightly different
>> +                * clock divider equations which means that the minimum divider
>> +                * differs too.
>> +                */
>> +               if (variant->st_clkdiv)
>> +                       mmc->f_min = DIV_ROUND_UP(host->mclk, 257);
>> +               else
>> +                       mmc->f_min = DIV_ROUND_UP(host->mclk, 512);
>> +               /*
>> +                * If no maximum operating frequency is supplied, fall back to
>> +                * use the module parameter, which has a (low) default value in
>> +                * case it 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);
>> +
>> +       }
>> +
>>          dev_dbg(mmc_dev(mmc), "clocking block at %u Hz\n", mmc->f_max);
>>
>>          /* Get regulators and the supported OCR mask */
>> diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
>> index 706eb513..1882e20 100644
>> --- a/drivers/mmc/host/mmci.h
>> +++ b/drivers/mmc/host/mmci.h
>> @@ -208,6 +208,8 @@ struct mmci_host {
>>          spinlock_t              lock;
>>
>>          unsigned int            mclk;
>> +       /* cached value of requested clk in set_ios */
>> +       unsigned int            mclk_req;
>
> How about "clock_cache" instead?
Sounds ok to me, I will change it in next version.

Thanks,
srini
>
>>          unsigned int            cclk;
>>          u32                     pwr_reg;
>>          u32                     pwr_reg_add;
>> --
>> 1.9.1
>>
>
> Kind regards
> Ulf Hansson
>

[PATCH v4 13/13] 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 | 45 ++++++++++++++++++++++++++++++++++++++++++++-
 drivers/mmc/host/mmci.h |  1 +
 2 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 6eb0a29..b68223a 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -73,6 +73,7 @@ static unsigned int fmax = 515633;
  * @busy_detect: true if busy detection on dat0 is supported
  * @pwrreg_nopower: bits in MMCIPOWER don't controls ext. power supply
  * @explicit_mclk_control: enable explicit mclk control in driver.
+ * @qcom_fifo: enables qcom specific fifo pio read function.
  */
 struct variant_data {
 	unsigned int		clkreg;
@@ -95,6 +96,7 @@ struct variant_data {
 	bool			busy_detect;
 	bool			pwrreg_nopower;
 	bool			explicit_mclk_control;
+	bool			qcom_fifo;
 };
 
 static struct variant_data variant_arm = {
@@ -202,6 +204,7 @@ static struct variant_data variant_qcom = {
 	.pwrreg_powerup		= MCI_PWR_UP,
 	.f_max			= 208000000,
 	.explicit_mclk_control	= true,
+	.qcom_fifo		= true,
 };
 
 static int mmci_card_busy(struct mmc_host *mmc)
@@ -1006,6 +1009,40 @@ 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)
+{
+	u32 *ptr = (u32 *) buffer;
+	unsigned int count = 0;
+	unsigned int words, bytes;
+	unsigned int fsize = 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 % fsize));
+			ptr++;
+			count += 4;
+			words--;
+			if (!words)
+				break;
+		}
+	}
+
+	if (bytes) {
+		unsigned char buf[4];
+		if (readl(host->base + MMCISTATUS) & MCI_RXDATAAVLBL) {
+			*buf = readl(host->base + MMCIFIFO + (count % fsize));
+			memcpy(ptr, buf, bytes);
+			count += bytes;
+		}
+	}
+
+	return count;
+}
+
 static int mmci_pio_read(struct mmci_host *host, char *buffer, unsigned int remain)
 {
 	void __iomem *base = host->base;
@@ -1129,7 +1166,8 @@ static irqreturn_t mmci_pio_irq(int irq, void *dev_id)
 
 		len = 0;
 		if (status & MCI_RXACTIVE)
-			len = mmci_pio_read(host, buffer, remain);
+			len = host->pio_read(host, buffer, remain);
+
 		if (status & MCI_TXACTIVE)
 			len = mmci_pio_write(host, buffer, remain, status);
 
@@ -1504,6 +1542,11 @@ static int mmci_probe(struct amba_device *dev,
 	if (ret)
 		goto host_free;
 
+	if (variant->qcom_fifo)
+		host->pio_read = mmci_qcom_pio_read;
+	else
+		host->pio_read = mmci_pio_read;
+
 	host->plat = plat;
 	host->variant = variant;
 	host->mclk = clk_get_rate(host->clk);
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 1882e20..dc9fe0a 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -229,6 +229,7 @@ struct mmci_host {
 	/* pio stuff */
 	struct sg_mapping_iter	sg_miter;
 	unsigned int		size;
+	int (*pio_read)(struct mmci_host *h, char *buf, unsigned int remain);
 
 #ifdef CONFIG_DMA_ENGINE
 	/* DMA stuff */
-- 
1.9.1

Re: [PATCH v4 13/13] mmc: mmci: Add Qcom specific pio_read function.

[Ulf Hansson]

On 28 May 2014 15:48,  <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>
> ---
>  drivers/mmc/host/mmci.c | 45 ++++++++++++++++++++++++++++++++++++++++++++-
>  drivers/mmc/host/mmci.h |  1 +
>  2 files changed, 45 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index 6eb0a29..b68223a 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -73,6 +73,7 @@ static unsigned int fmax = 515633;
>   * @busy_detect: true if busy detection on dat0 is supported
>   * @pwrreg_nopower: bits in MMCIPOWER don't controls ext. power supply
>   * @explicit_mclk_control: enable explicit mclk control in driver.
> + * @qcom_fifo: enables qcom specific fifo pio read function.
>   */
>  struct variant_data {
>         unsigned int            clkreg;
> @@ -95,6 +96,7 @@ struct variant_data {
>         bool                    busy_detect;
>         bool                    pwrreg_nopower;
>         bool                    explicit_mclk_control;
> +       bool                    qcom_fifo;
>  };
>
>  static struct variant_data variant_arm = {
> @@ -202,6 +204,7 @@ static struct variant_data variant_qcom = {
>         .pwrreg_powerup         = MCI_PWR_UP,
>         .f_max                  = 208000000,
>         .explicit_mclk_control  = true,
> +       .qcom_fifo              = true,
>  };
>
>  static int mmci_card_busy(struct mmc_host *mmc)
> @@ -1006,6 +1009,40 @@ 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)
> +{
> +       u32 *ptr = (u32 *) buffer;

Instead of u32 ptr above, I suggest to use a char *ptr. You need it
anyway further down below where you move in step of bytes instead of
words.

> +       unsigned int count = 0;
> +       unsigned int words, bytes;
> +       unsigned int fsize = 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) {

How about while (words && (readl(host->base + MMCISTATUS) & MCI_RXDATAAVLBL)

That would make it possible the remove the "if", both above and below.

> +                       *ptr = readl(host->base + MMCIFIFO + (count % fsize));

This looks strange. :-) Depending on the count you will read an offset
into the FIFO? Seems like a very awkward implementation of a FIFO in
the HW. :-)

BTW, what does "MCI_RXDATAAVLBL" actually mean for the Qcom variant?
How much data could you expect in the FIFO when this status is
triggered?

Are there no option of reading a number of words, depending on what
type FIFO IRQ that was raised?

> +                       ptr++;
> +                       count += 4;
> +                       words--;
> +                       if (!words)
> +                               break;
> +               }
> +       }
> +
> +       if (bytes) {
> +               unsigned char buf[4];
> +               if (readl(host->base + MMCISTATUS) & MCI_RXDATAAVLBL) {
> +                       *buf = readl(host->base + MMCIFIFO + (count % fsize));
> +                       memcpy(ptr, buf, bytes);
> +                       count += bytes;
> +               }
> +       }
> +
> +       return count;
> +}
> +
>  static int mmci_pio_read(struct mmci_host *host, char *buffer, unsigned int remain)
>  {
>         void __iomem *base = host->base;
> @@ -1129,7 +1166,8 @@ static irqreturn_t mmci_pio_irq(int irq, void *dev_id)
>
>                 len = 0;
>                 if (status & MCI_RXACTIVE)
> -                       len = mmci_pio_read(host, buffer, remain);
> +                       len = host->pio_read(host, buffer, remain);
> +
>                 if (status & MCI_TXACTIVE)
>                         len = mmci_pio_write(host, buffer, remain, status);
>
> @@ -1504,6 +1542,11 @@ static int mmci_probe(struct amba_device *dev,
>         if (ret)
>                 goto host_free;
>
> +       if (variant->qcom_fifo)
> +               host->pio_read = mmci_qcom_pio_read;
> +       else
> +               host->pio_read = mmci_pio_read;
> +
>         host->plat = plat;
>         host->variant = variant;
>         host->mclk = clk_get_rate(host->clk);
> diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
> index 1882e20..dc9fe0a 100644
> --- a/drivers/mmc/host/mmci.h
> +++ b/drivers/mmc/host/mmci.h
> @@ -229,6 +229,7 @@ struct mmci_host {
>         /* pio stuff */
>         struct sg_mapping_iter  sg_miter;
>         unsigned int            size;
> +       int (*pio_read)(struct mmci_host *h, char *buf, unsigned int remain);
>
>  #ifdef CONFIG_DMA_ENGINE
>         /* DMA stuff */
> --
> 1.9.1
>

Kind regards
Ulf Hansson

Re: [PATCH v4 13/13] mmc: mmci: Add Qcom specific pio_read function.

[Srinivas Kandagatla]

Hi Ulf,

On 30/05/14 12:27, Ulf Hansson wrote:
> On 28 May 2014 15:48,  <srinivas.kandagatla@linaro.org> wrote:
...

>>          .f_max                  = 208000000,
>>          .explicit_mclk_control  = true,
>> +       .qcom_fifo              = true,
>>   };
>>
>>   static int mmci_card_busy(struct mmc_host *mmc)
>> @@ -1006,6 +1009,40 @@ 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)
>> +{
>> +       u32 *ptr = (u32 *) buffer;
>
> Instead of u32 ptr above, I suggest to use a char *ptr. You need it
> anyway further down below where you move in step of bytes instead of
> words.
>
>> +       unsigned int count = 0;
>> +       unsigned int words, bytes;
>> +       unsigned int fsize = 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) {
>
> How about while (words && (readl(host->base + MMCISTATUS) & MCI_RXDATAAVLBL)
>
> That would make it possible the remove the "if", both above and below.

That sounds sensible.. I will try it.
>
>> +                       *ptr = readl(host->base + MMCIFIFO + (count % fsize));
>
> This looks strange. :-) Depending on the count you will read an offset
> into the FIFO? Seems like a very awkward implementation of a FIFO in
> the HW. :-)
>
I got into weird issues when I tried using the mmci_pio_read, the fifo 
implementation seems to be different. I dont have full details on the 
fifo behaviour.

Most of this logic is inherited from 3.4 qcom andriod kernel.

> BTW, what does "MCI_RXDATAAVLBL" actually mean for the Qcom variant?

It means, At least 1 word in the RX FIFO. SW can read 1 word only from 
the FIFO.

> How much data could you expect in the FIFO when this status is
> triggered?

> Are there no option of reading a number of words, depending on what
> type FIFO IRQ that was raised?
There are RXFIFO_HALF_FULL and RXFIFO_FULL bits in status register, but 
I never tried to use them. Might be worth a try before I send next 
version patches.


I will give a try and keep you posted.
Thanks,
srini
>
>> +                       ptr++;
>> +                       count += 4;
>> +                       words--;
>> +                       if (!words)
>> +                               break;
>> +               }
>> +       }
>> +
>> +       if (bytes) {
>> +               unsigned char buf[4];
>> +               if (readl(host->base + MMCISTATUS) & MCI_RXDATAAVLBL) {
>> +                       *buf = readl(host->base + MMCIFIFO + (count % fsize));
>> +                       memcpy(ptr, buf, bytes);
>> +                       count += bytes;
>> +               }
>> +       }
>> +
>> +       return count;
>> +}
>> +

>>
>
> Kind regards
> Ulf Hansson
>

Re: [PATCH v4 13/13] mmc: mmci: Add Qcom specific pio_read function.

[Srinivas Kandagatla]

Hi Ulf,

Managed to reuse the existing mmci_pio_read function with some minor 
modifications, Issue was with reading full fifo sizes which was creating 
the issue.

On 30/05/14 12:44, Srinivas Kandagatla wrote:
> That sounds sensible.. I will try it.
>>
>>> +                       *ptr = readl(host->base + MMCIFIFO + (count %
>>> fsize));
>>
>> This looks strange. :-) Depending on the count you will read an offset
>> into the FIFO? Seems like a very awkward implementation of a FIFO in
>> the HW. :-)
>>
> I got into weird issues when I tried using the mmci_pio_read, the fifo
> implementation seems to be different. I dont have full details on the
> fifo behaviour.
>
> Most of this logic is inherited from 3.4 qcom andriod kernel.
>
>> BTW, what does "MCI_RXDATAAVLBL" actually mean for the Qcom variant?
>
> It means, At least 1 word in the RX FIFO. SW can read 1 word only from
> the FIFO.
>
>> How much data could you expect in the FIFO when this status is
>> triggered?
>
>> Are there no option of reading a number of words, depending on what
>> type FIFO IRQ that was raised?
> There are RXFIFO_HALF_FULL and RXFIFO_FULL bits in status register, but
> I never tried to use them. Might be worth a try before I send next
> version patches.
>
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -73,6 +73,7 @@ static unsigned int fmax = 515633;
   * @busy_detect: true if busy detection on dat0 is supported
   * @pwrreg_nopower: bits in MMCIPOWER don't controls ext. power supply
   * @explicit_mclk_control: enable explicit mclk control in driver.
+ * @qcom_fifo: enables qcom specific fifo pio read logic.
   */
  struct variant_data {
         unsigned int            clkreg;
@@ -95,6 +96,7 @@ struct variant_data {
         bool                    busy_detect;
         bool                    pwrreg_nopower;
         bool                    explicit_mclk_control;
+       bool                    qcom_fifo;
  };

  static struct variant_data variant_arm = {
@@ -990,15 +992,33 @@ mmci_cmd_irq(struct mmci_host *host, struct 
mmc_command *cmd,
         }
  }

+static int mmci_get_rx_fifocnt(struct mmci_host *host, u32 status, int 
remain)
+{
+       return remain - (readl(host->base + MMCIFIFOCNT) << 2);
+}
+
+static int mmci_qcom_get_rx_fifocnt(struct mmci_host *host, u32 status, 
int r)
+{
+       /*
+        * on qcom SDCC4 only 8 words are used in each burst so only 8 
addresses
+        * from the fifo range should be used
+        */
+       if (status & MCI_RXFIFOHALFFULL)
+               return host->variant->fifohalfsize;
+       else
+               return 4;
+}
+
  static int mmci_pio_read(struct mmci_host *host, char *buffer, 
unsigned int remain)
  {
         void __iomem *base = host->base;
         char *ptr = buffer;
-       u32 status;
+       int hf = host->variant->fifohalfsize;
         int host_remain = host->size;
+       u32 status = readl(host->base + MMCISTATUS);

         do {
-               int count = host_remain - (readl(base + MMCIFIFOCNT) << 2);
+               int count = host->get_rx_fifocnt(host, status, host_remain);

                 if (count > remain)
                         count = remain;
@@ -1488,6 +1508,11 @@ static int mmci_probe(struct amba_device *dev,
         if (ret)
                 goto host_free;

+       if (variant->qcom_fifo)
+               host->get_rx_fifocnt = mmci_qcom_get_rx_fifocnt;
+       else
+               host->get_rx_fifocnt = mmci_get_rx_fifocnt;
+
         host->plat = plat;
         host->variant = variant;
         host->mclk = clk_get_rate(host->clk);
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index b5f0810..5f76670 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -229,6 +229,7 @@ struct mmci_host {
         /* pio stuff */
         struct sg_mapping_iter  sg_miter;
         unsigned int            size;
+       int (*get_rx_fifocnt)(struct mmci_host *h, u32 status, int remain);

  #ifdef CONFIG_DMA_ENGINE