[PATCH RFC 0/5] mmc: mmci: sdio related fixes

[PATCH RFC 0/5] mmc: mmci: sdio related fixes

[Srinivas Kandagatla]

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

This patchset fixes few sdio related issues encountered while testing
WLAN ath6kl via SDIO on IFC6410 board with Qualcomm APQ8064 SOC.

First patch "mmc: mmci: Enable SDIO support for Qualcomm variant data"
enables sdio support for Qualcom SOCs. I can move this out of this series
if it make sense to do.

Patches "mmc: mmci: move block size validation under relevant code" and
"mmc: mmci: relax blksz check for SDIO"
relaxes error checks for blocksizes which are not exactly power of 2.
The issue was ath6kl driver was issuing 12 bytes and 24 bytes reads
which are caught as part of the error handing in the driver and
resulting in failures.

Patch "mmc: mmci: Add sdio enable mask in variant data" adds extra
variant parameter to enable sdio. This makes mmci driver more flexible.

Patch "mmc: mmci: move ST specific sdio setup under a condition"
move ST specific sdio setup under a vendor id check condition so that
other variants dont use it. Am happy to change this if there is any
other better way.

All these patches are tested on IFC6410 board with ath6kl WLAN via SDIO.

Thanks,
srini

Srinivas Kandagatla (5):
  mmc: mmci: Enable SDIO support for Qualcomm variant data
  mmc: mmci: move block size validation under relevant code
  mmc: mmci: relax blksz check for SDIO
  mmc: mmci: Add sdio enable mask in variant data
  mmc: mmci: move ST specific sdio setup under a condition

 drivers/mmc/host/mmci.c | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

-- 
1.9.1

[PATCH RFC 1/5] mmc: mmci: Enable SDIO support for Qualcomm variant data

[Srinivas Kandagatla]

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

This patch sets sdio flag in qualcomm variant data to enable WLAN
connected via sdio on IFC6410 board.

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

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 9059570..8641572 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -203,6 +203,7 @@ static struct variant_data variant_qcom = {
 	.datactrl_mask_ddrmode	= MCI_QCOM_CLK_SELECT_IN_DDR_MODE,
 	.data_cmd_enable	= MCI_QCOM_CSPM_DATCMD,
 	.blksz_datactrl4	= true,
+	.sdio			= true,
 	.datalength_bits	= 24,
 	.pwrreg_powerup		= MCI_PWR_UP,
 	.f_max			= 208000000,
-- 
1.9.1

[PATCH RFC 2/5] mmc: mmci: move block size validation under relevant code

[Srinivas Kandagatla]

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

This code moves a BUG_ON condition to relevant if block, this check is
not necessary for IPs which can set any arbitrary block size in a given
range.
This patch is necessary for SDIO which sets block sizes that are exactly
not power of 2.

Original issue detected while testing WLAN ath6kl on Qualcomm APQ8064 SOC.

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

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 8641572..99fe80a 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -793,15 +793,16 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 	writel(timeout, base + MMCIDATATIMER);
 	writel(host->size, base + MMCIDATALENGTH);
 
-	blksz_bits = ffs(data->blksz) - 1;
-	BUG_ON(1 << blksz_bits != data->blksz);
 
-	if (variant->blksz_datactrl16)
+	if (variant->blksz_datactrl16) {
 		datactrl = MCI_DPSM_ENABLE | (data->blksz << 16);
-	else if (variant->blksz_datactrl4)
+	} else if (variant->blksz_datactrl4) {
 		datactrl = MCI_DPSM_ENABLE | (data->blksz << 4);
-	else
+	} else {
+		blksz_bits = ffs(data->blksz) - 1;
+		BUG_ON(1 << blksz_bits != data->blksz);
 		datactrl = MCI_DPSM_ENABLE | blksz_bits << 4;
+	}
 
 	if (data->flags & MMC_DATA_READ)
 		datactrl |= MCI_DPSM_DIRECTION;
-- 
1.9.1

[PATCH RFC 3/5] mmc: mmci: relax blksz check for SDIO

[Srinivas Kandagatla]

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

This patch adds condition in mmci_validate_data to skip checking
blocksize for SDIO card types. SDIO card type can issue blocksizes
which are not exactly power of 2 so this check always fails, resulting
in SDIO failures.

Relaxing this check for SDIO in mmci_validate_data solves the issue
encountered during WLAN ath6kl testing with Qualcomm APQ8064 SOC on
IFC6410 board.

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

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 99fe80a..747aba0 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -240,6 +240,10 @@ static int mmci_validate_data(struct mmci_host *host,
 	if (!data)
 		return 0;
 
+	/* sdio could have sizes which are not power of 2 */
+	if (host->mmc->card && mmc_card_sdio(host->mmc->card))
+		return 0;
+
 	if (!is_power_of_2(data->blksz)) {
 		dev_err(mmc_dev(host->mmc),
 			"unsupported block size (%d bytes)\n", data->blksz);
-- 
1.9.1

[PATCH RFC 4/5] mmc: mmci: Add sdio enable mask in variant data

[Srinivas Kandagatla]

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

This patch adds sdio enable mask in variant data, SOCs like ST have
special bits in datactrl register to enable sdio. Unconditionally setting
this bit in this driver breaks other SOCs like Qualcomm which maps this
bits to something else, so making this enable bit to come from variant
data solves the issue.

Originally the issue is detected while testing WLAN ath6kl on Qualcomm
APQ8064.

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

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 747aba0..848e2bb 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -67,6 +67,7 @@ static unsigned int fmax = 515633;
  * @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
+ * @datactrl_mask_sdio: SDIO enable mask 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
@@ -87,6 +88,7 @@ struct variant_data {
 	unsigned int		fifohalfsize;
 	unsigned int		data_cmd_enable;
 	unsigned int		datactrl_mask_ddrmode;
+	unsigned int		datactrl_mask_sdio;
 	bool			sdio;
 	bool			st_clkdiv;
 	bool			blksz_datactrl16;
@@ -133,6 +135,7 @@ static struct variant_data variant_u300 = {
 	.clkreg_enable		= MCI_ST_U300_HWFCEN,
 	.clkreg_8bit_bus_enable = MCI_ST_8BIT_BUS,
 	.datalength_bits	= 16,
+	.datactrl_mask_sdio	= MCI_ST_DPSM_SDIOEN,
 	.sdio			= true,
 	.pwrreg_powerup		= MCI_PWR_ON,
 	.f_max			= 100000000,
@@ -146,6 +149,7 @@ static struct variant_data variant_nomadik = {
 	.fifohalfsize		= 8 * 4,
 	.clkreg			= MCI_CLK_ENABLE,
 	.datalength_bits	= 24,
+	.datactrl_mask_sdio	= MCI_ST_DPSM_SDIOEN,
 	.sdio			= true,
 	.st_clkdiv		= true,
 	.pwrreg_powerup		= MCI_PWR_ON,
@@ -163,6 +167,7 @@ static struct variant_data variant_ux500 = {
 	.clkreg_8bit_bus_enable = MCI_ST_8BIT_BUS,
 	.clkreg_neg_edge_enable	= MCI_ST_UX500_NEG_EDGE,
 	.datalength_bits	= 24,
+	.datactrl_mask_sdio	= MCI_ST_DPSM_SDIOEN,
 	.sdio			= true,
 	.st_clkdiv		= true,
 	.pwrreg_powerup		= MCI_PWR_ON,
@@ -182,6 +187,7 @@ static struct variant_data variant_ux500v2 = {
 	.clkreg_neg_edge_enable	= MCI_ST_UX500_NEG_EDGE,
 	.datactrl_mask_ddrmode	= MCI_ST_DPSM_DDRMODE,
 	.datalength_bits	= 24,
+	.datactrl_mask_sdio	= MCI_ST_DPSM_SDIOEN,
 	.sdio			= true,
 	.st_clkdiv		= true,
 	.blksz_datactrl16	= true,
@@ -811,16 +817,10 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 	if (data->flags & MMC_DATA_READ)
 		datactrl |= MCI_DPSM_DIRECTION;
 
-	/* The ST Micro variants has a special bit to enable SDIO */
 	if (variant->sdio && host->mmc->card)
 		if (mmc_card_sdio(host->mmc->card)) {
-			/*
-			 * The ST Micro variants has a special bit
-			 * to enable SDIO.
-			 */
 			u32 clk;
-
-			datactrl |= MCI_ST_DPSM_SDIOEN;
+			datactrl |= variant->datactrl_mask_sdio;
 
 			/*
 			 * The ST Micro variant for SDIO small write transfers
-- 
1.9.1

[PATCH RFC 5/5] mmc: mmci: move ST specific sdio setup under a condition

[Srinivas Kandagatla]

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

This patch moves ST specific sdio setup under a vendor id condition,
this will ensure that the ST specfic setup is not done on other vendor
like Qualcomm.

Originally the issue was detected while testing WLAN ath6kl on IFC6410
board with APQ8064 SOC.

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

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 848e2bb..1783be7 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -828,7 +828,8 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 			 * otherwise the transfer will not start. The threshold
 			 * depends on the rate of MCLK.
 			 */
-			if (data->flags & MMC_DATA_WRITE &&
+			if (host->hw_designer == AMBA_VENDOR_ST &&
+			    data->flags & MMC_DATA_WRITE &&
 			    (host->size < 8 ||
 			     (host->size <= 8 && host->mclk > 50000000)))
 				clk = host->clk_reg & ~variant->clkreg_enable;
-- 
1.9.1

Re: [PATCH RFC 3/5] mmc: mmci: relax blksz check for SDIO

[Russell King - ARM Linux]

On Tue, Aug 12, 2014 at 01:04:40PM +0100, Srinivas Kandagatla wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> 
> This patch adds condition in mmci_validate_data to skip checking
> blocksize for SDIO card types. SDIO card type can issue blocksizes
> which are not exactly power of 2 so this check always fails, resulting
> in SDIO failures.

Do you understand why this check is there?

Do you realise that the standard MMCI from ARM Ltd can /only/ perform
power-of-2 block transfers?

Hence, NAK on this change as it stands.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH RFC 3/5] mmc: mmci: relax blksz check for SDIO

[Srinivas Kandagatla]

Thankyou for the feedback.

On 12/08/14 15:12, Russell King - ARM Linux wrote:
> On Tue, Aug 12, 2014 at 01:04:40PM +0100, Srinivas Kandagatla wrote:
>> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>
>> This patch adds condition in mmci_validate_data to skip checking
>> blocksize for SDIO card types. SDIO card type can issue blocksizes
>> which are not exactly power of 2 so this check always fails, resulting
>> in SDIO failures.
>
> Do you understand why this check is there?
>
> Do you realise that the standard MMCI from ARM Ltd can /only/ perform
> power-of-2 block transfers?

Yes, I noticed that in pl180 TRM. I agree this patch will break standard 
MMCI.

In the past Ulf Hansson and Stefan Nilsson have submitted a patch to fix 
this issue for ux500v2. This patch adds new flag non_power_of_2_blksize 
/ any_blksize to variant_data.

http://www.spinics.net/lists/linux-mmc/msg12160.html
http://marc.info/?l=linux-mmc&m=131710963908853&w=2

This patch will fix the issue for Qcom too. If you are Ok with adding 
new flag non_power_of_2_blksize/any_blksize in the variant_data, I can 
rebase and include Ulf's patch in next version.


thanks,
--srini

>
> Hence, NAK on this change as it stands.
>

Re: [PATCH RFC 3/5] mmc: mmci: relax blksz check for SDIO

[Linus Walleij]

On Wed, Aug 13, 2014 at 7:14 AM, Srinivas Kandagatla
<srinivas.kandagatla@linaro.org> wrote:

> In the past Ulf Hansson and Stefan Nilsson have submitted a patch to fix
> this issue for ux500v2. This patch adds new flag non_power_of_2_blksize /
> any_blksize to variant_data.

That's the right approach. It makes non-2^n support the exception rather
than the rule and applies only to the variants relevant so the logic is right.

Yours,
Linus Walleij

Re: [PATCH RFC 4/5] mmc: mmci: Add sdio enable mask in variant data

[Linus Walleij]

On Tue, Aug 12, 2014 at 2:05 PM, Srinivas Kandagatla
<srinivas.kandagatla@linaro.org> wrote:

> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> This patch adds sdio enable mask in variant data, SOCs like ST have
> special bits in datactrl register to enable sdio. Unconditionally setting
> this bit in this driver breaks other SOCs like Qualcomm which maps this
> bits to something else, so making this enable bit to come from variant
> data solves the issue.
>
> Originally the issue is detected while testing WLAN ath6kl on Qualcomm
> APQ8064.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

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

Yours,
Linus Walleij

Re: [PATCH RFC 5/5] mmc: mmci: move ST specific sdio setup under a condition

[Linus Walleij]

On Tue, Aug 12, 2014 at 2:05 PM, Srinivas Kandagatla
<srinivas.kandagatla@linaro.org> wrote:

> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> This patch moves ST specific sdio setup under a vendor id condition,
> this will ensure that the ST specfic setup is not done on other vendor
> like Qualcomm.
>
> Originally the issue was detected while testing WLAN ath6kl on IFC6410
> board with APQ8064 SOC.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

So instead of again checking hw_designer (I think that's a bit
messy) and instead of having the flag bool sdio from patch
1, why not introduce 2 flags in vendor_data:

bool st_sdio;
bool qcom_sdio;

> +++ b/drivers/mmc/host/mmci.c
> @@ -828,7 +828,8 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
>                          * otherwise the transfer will not start. The threshold
>                          * depends on the rate of MCLK.
>                          */
> -                       if (data->flags & MMC_DATA_WRITE &&
> +                       if (host->hw_designer == AMBA_VENDOR_ST &&
> +                           data->flags & MMC_DATA_WRITE &&

And this becomes just

if (variant->st_sdio)...

>                             (host->size < 8 ||
>                              (host->size <= 8 && host->mclk > 50000000)))
>                                 clk = host->clk_reg & ~variant->clkreg_enable;

Yours,
Linus Walleij

Re: [PATCH RFC 5/5] mmc: mmci: move ST specific sdio setup under a condition

[Srinivas Kandagatla]

Thankyou Linus for the feedback.

On 13/08/14 09:58, Linus Walleij wrote:
> On Tue, Aug 12, 2014 at 2:05 PM, Srinivas Kandagatla
> <srinivas.kandagatla@linaro.org> wrote:
>
>> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>
>> This patch moves ST specific sdio setup under a vendor id condition,
>> this will ensure that the ST specfic setup is not done on other vendor
>> like Qualcomm.
>>
>> Originally the issue was detected while testing WLAN ath6kl on IFC6410
>> board with APQ8064 SOC.
>>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> So instead of again checking hw_designer (I think that's a bit
> messy) and instead of having the flag bool sdio from patch
> 1, why not introduce 2 flags in vendor_data:
>
> bool st_sdio;
> bool qcom_sdio;

Thats good idea,

If we rename the existing sdio flag to st_sdio, then we can get rid of 
the first patch too. As there is no specific sdio logic required for qcom.

I will send the change in next version.

thanks
srini
>
>> +++ b/drivers/mmc/host/mmci.c
>> @@ -828,7 +828,8 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
>>                           * otherwise the transfer will not start. The threshold
>>                           * depends on the rate of MCLK.
>>                           */
>> -                       if (data->flags & MMC_DATA_WRITE &&
>> +                       if (host->hw_designer == AMBA_VENDOR_ST &&
>> +                           data->flags & MMC_DATA_WRITE &&
>
> And this becomes just
>
> if (variant->st_sdio)...
>
>>                              (host->size < 8 ||
>>                               (host->size <= 8 && host->mclk > 50000000)))
>>                                  clk = host->clk_reg & ~variant->clkreg_enable;
>
> Yours,
> Linus Walleij
>

[PATCH v2 0/4] mmc: mmci: sdio related fixes

[Srinivas Kandagatla]

This patchset fixes few sdio related issues encountered while testing
WLAN ath6kl via SDIO on IFC6410 board with Qualcomm APQ8064 SOC.

Patch: "mmc: mmci: Support any block sizes for ux500v2 and qcom variant" is
a very old patch by Ulf to support IPs which support any size of block sizes.
http://www.spinics.net/lists/linux-mmc/msg12160.html I modified the subject
line to include qcom.

Patches "mmc: mmci: move block size validation under relevant code" 
Moves a BUG check to relevant code path.

Both of the patches fixes below issues reported while testing sdio.
The issue was ath6kl driver was issuing 12 bytes and 24 bytes reads
which are caught as part of the error handing in the driver and
resulting in failures. 

Patch "mmc: mmci: Add sdio enable mask in variant data" adds extra
variant parameter to enable sdio. This makes mmci driver more flexible.

Patch "mmc: mmci: rename sdio flag in vendor data to st_sdio" renames sdio
flag in vendor data to st_sdio, as this flag is only used to setup st
specific sdio logic.

All these patches are tested on IFC6410 board with ath6kl WLAN via SDIO.

Thanks to Linus W and Russell for comments on RFC.

Changes since RFC:
	- moved sdio flag to st_sdio to simplify the checks.
	- use Ulf's patch to address IP's which support anysize blocks.

Thanks,
srini

Srinivas Kandagatla (3):
  mmc: mmci: move block size validation under relevant code
  mmc: mmci: Add sdio enable mask in variant data
  mmc: mmci: rename sdio flag in vendor data to st_sdio

Ulf Hansson (1):
  mmc: mmci: Support any block sizes for ux500v2 and qcom variant

 drivers/mmc/host/mmci.c | 78 ++++++++++++++++++++++++++-----------------------
 1 file changed, 42 insertions(+), 36 deletions(-)

-- 
1.9.1

[PATCH v2 1/4] mmc: mmci: Support any block sizes for ux500v2 and qcom variant

[Srinivas Kandagatla]

From: Ulf Hansson <ulf.hansson@linaro.org>

For the ux500v2 variant of the PL18x block, any block sizes are
supported. This will make it possible to decrease data overhead
for SDIO transfers.

This patch is based on Ulf Hansson patch
http://www.spinics.net/lists/linux-mmc/msg12160.html

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
	enabled this support on qcom variant.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/mmc/host/mmci.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index c11cb05..3089fba 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -77,6 +77,7 @@ static unsigned int fmax = 515633;
  * @qcom_fifo: enables qcom specific fifo pio read logic.
  * @reversed_irq_handling: handle data irq before cmd irq.
  * @qcom_dml: enables qcom specific dma glue for dma transfers.
+ * @any_blksize: true if block any sizes are supported
  */
 struct variant_data {
 	unsigned int		clkreg;
@@ -102,6 +103,7 @@ struct variant_data {
 	bool			qcom_fifo;
 	bool			reversed_irq_handling;
 	bool			qcom_dml;
+	bool			any_blksize;
 };
 
 static struct variant_data variant_arm = {
@@ -194,6 +196,7 @@ static struct variant_data variant_ux500v2 = {
 	.pwrreg_clkgate		= true,
 	.busy_detect		= true,
 	.pwrreg_nopower		= true,
+	.any_blksize		= true,
 };
 
 static struct variant_data variant_qcom = {
@@ -212,6 +215,7 @@ static struct variant_data variant_qcom = {
 	.explicit_mclk_control	= true,
 	.qcom_fifo		= true,
 	.qcom_dml		= true,
+	.any_blksize		= true,
 };
 
 static int mmci_card_busy(struct mmc_host *mmc)
@@ -239,10 +243,11 @@ static int mmci_card_busy(struct mmc_host *mmc)
 static int mmci_validate_data(struct mmci_host *host,
 			      struct mmc_data *data)
 {
+	struct variant_data *variant = host->variant;
+
 	if (!data)
 		return 0;
-
-	if (!is_power_of_2(data->blksz)) {
+	if (!is_power_of_2(data->blksz) && !variant->any_blksize) {
 		dev_err(mmc_dev(host->mmc),
 			"unsupported block size (%d bytes)\n", data->blksz);
 		return -EINVAL;
-- 
1.9.1

[PATCH v2 2/4] mmc: mmci: move block size validation under relevant code

[Srinivas Kandagatla]

This code moves a BUG_ON condition to relevant if block, this check is
not necessary for IPs which can set any arbitrary block size in a given
range.
This patch is necessary for SDIO which sets block sizes that are exactly
not power of 2.

Original issue detected while testing WLAN ath6kl on Qualcomm APQ8064 SOC.

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

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 3089fba..1c99195 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -800,15 +800,16 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 	writel(timeout, base + MMCIDATATIMER);
 	writel(host->size, base + MMCIDATALENGTH);
 
-	blksz_bits = ffs(data->blksz) - 1;
-	BUG_ON(1 << blksz_bits != data->blksz);
 
-	if (variant->blksz_datactrl16)
+	if (variant->blksz_datactrl16) {
 		datactrl = MCI_DPSM_ENABLE | (data->blksz << 16);
-	else if (variant->blksz_datactrl4)
+	} else if (variant->blksz_datactrl4) {
 		datactrl = MCI_DPSM_ENABLE | (data->blksz << 4);
-	else
+	} else {
+		blksz_bits = ffs(data->blksz) - 1;
+		BUG_ON(1 << blksz_bits != data->blksz);
 		datactrl = MCI_DPSM_ENABLE | blksz_bits << 4;
+	}
 
 	if (data->flags & MMC_DATA_READ)
 		datactrl |= MCI_DPSM_DIRECTION;
-- 
1.9.1

[PATCH v2 3/4] mmc: mmci: Add sdio enable mask in variant data

[Srinivas Kandagatla]

This patch adds sdio enable mask in variant data, SOCs like ST have
special bits in datactrl register to enable sdio. Unconditionally setting
this bit in this driver breaks other SOCs like Qualcomm which maps this
bits to something else, so making this enable bit to come from variant
data solves the issue.

Originally the issue is detected while testing WLAN ath6kl on Qualcomm
APQ8064.

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

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 1c99195..fc08203 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -67,6 +67,7 @@ static unsigned int fmax = 515633;
  * @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
+ * @datactrl_mask_sdio: SDIO enable mask 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
@@ -89,6 +90,7 @@ struct variant_data {
 	unsigned int		fifohalfsize;
 	unsigned int		data_cmd_enable;
 	unsigned int		datactrl_mask_ddrmode;
+	unsigned int		datactrl_mask_sdio;
 	bool			sdio;
 	bool			st_clkdiv;
 	bool			blksz_datactrl16;
@@ -138,6 +140,7 @@ static struct variant_data variant_u300 = {
 	.clkreg_enable		= MCI_ST_U300_HWFCEN,
 	.clkreg_8bit_bus_enable = MCI_ST_8BIT_BUS,
 	.datalength_bits	= 16,
+	.datactrl_mask_sdio	= MCI_ST_DPSM_SDIOEN,
 	.sdio			= true,
 	.pwrreg_powerup		= MCI_PWR_ON,
 	.f_max			= 100000000,
@@ -151,6 +154,7 @@ static struct variant_data variant_nomadik = {
 	.fifohalfsize		= 8 * 4,
 	.clkreg			= MCI_CLK_ENABLE,
 	.datalength_bits	= 24,
+	.datactrl_mask_sdio	= MCI_ST_DPSM_SDIOEN,
 	.sdio			= true,
 	.st_clkdiv		= true,
 	.pwrreg_powerup		= MCI_PWR_ON,
@@ -168,6 +172,7 @@ static struct variant_data variant_ux500 = {
 	.clkreg_8bit_bus_enable = MCI_ST_8BIT_BUS,
 	.clkreg_neg_edge_enable	= MCI_ST_UX500_NEG_EDGE,
 	.datalength_bits	= 24,
+	.datactrl_mask_sdio	= MCI_ST_DPSM_SDIOEN,
 	.sdio			= true,
 	.st_clkdiv		= true,
 	.pwrreg_powerup		= MCI_PWR_ON,
@@ -187,6 +192,7 @@ static struct variant_data variant_ux500v2 = {
 	.clkreg_neg_edge_enable	= MCI_ST_UX500_NEG_EDGE,
 	.datactrl_mask_ddrmode	= MCI_ST_DPSM_DDRMODE,
 	.datalength_bits	= 24,
+	.datactrl_mask_sdio	= MCI_ST_DPSM_SDIOEN,
 	.sdio			= true,
 	.st_clkdiv		= true,
 	.blksz_datactrl16	= true,
@@ -814,16 +820,10 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 	if (data->flags & MMC_DATA_READ)
 		datactrl |= MCI_DPSM_DIRECTION;
 
-	/* The ST Micro variants has a special bit to enable SDIO */
 	if (variant->sdio && host->mmc->card)
 		if (mmc_card_sdio(host->mmc->card)) {
-			/*
-			 * The ST Micro variants has a special bit
-			 * to enable SDIO.
-			 */
 			u32 clk;
-
-			datactrl |= MCI_ST_DPSM_SDIOEN;
+			datactrl |= variant->datactrl_mask_sdio;
 
 			/*
 			 * The ST Micro variant for SDIO small write transfers
-- 
1.9.1

[PATCH v2 4/4] mmc: mmci: rename sdio flag in vendor data to st_sdio

[Srinivas Kandagatla]

This patch renames sdio flag in vendor data to st_sdio, as this flag is
only used to enable ST specific sdio setup. This will also ensure that
the ST specfic setup is not done on other vendor like Qualcomm.

Originally the issue was detected while testing WLAN ath6kl on IFC6410
board with APQ8064 SOC.

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

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index fc08203..26aa443 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -61,7 +61,7 @@ static unsigned int fmax = 515633;
  * @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_sdio: enable ST specific SDIO logic
  * @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
@@ -91,7 +91,7 @@ struct variant_data {
 	unsigned int		data_cmd_enable;
 	unsigned int		datactrl_mask_ddrmode;
 	unsigned int		datactrl_mask_sdio;
-	bool			sdio;
+	bool			st_sdio;
 	bool			st_clkdiv;
 	bool			blksz_datactrl16;
 	bool			blksz_datactrl4;
@@ -141,7 +141,7 @@ static struct variant_data variant_u300 = {
 	.clkreg_8bit_bus_enable = MCI_ST_8BIT_BUS,
 	.datalength_bits	= 16,
 	.datactrl_mask_sdio	= MCI_ST_DPSM_SDIOEN,
-	.sdio			= true,
+	.st_sdio			= true,
 	.pwrreg_powerup		= MCI_PWR_ON,
 	.f_max			= 100000000,
 	.signal_direction	= true,
@@ -155,7 +155,7 @@ static struct variant_data variant_nomadik = {
 	.clkreg			= MCI_CLK_ENABLE,
 	.datalength_bits	= 24,
 	.datactrl_mask_sdio	= MCI_ST_DPSM_SDIOEN,
-	.sdio			= true,
+	.st_sdio		= true,
 	.st_clkdiv		= true,
 	.pwrreg_powerup		= MCI_PWR_ON,
 	.f_max			= 100000000,
@@ -173,7 +173,7 @@ static struct variant_data variant_ux500 = {
 	.clkreg_neg_edge_enable	= MCI_ST_UX500_NEG_EDGE,
 	.datalength_bits	= 24,
 	.datactrl_mask_sdio	= MCI_ST_DPSM_SDIOEN,
-	.sdio			= true,
+	.st_sdio		= true,
 	.st_clkdiv		= true,
 	.pwrreg_powerup		= MCI_PWR_ON,
 	.f_max			= 100000000,
@@ -193,7 +193,7 @@ static struct variant_data variant_ux500v2 = {
 	.datactrl_mask_ddrmode	= MCI_ST_DPSM_DDRMODE,
 	.datalength_bits	= 24,
 	.datactrl_mask_sdio	= MCI_ST_DPSM_SDIOEN,
-	.sdio			= true,
+	.st_sdio		= true,
 	.st_clkdiv		= true,
 	.blksz_datactrl16	= true,
 	.pwrreg_powerup		= MCI_PWR_ON,
@@ -820,26 +820,26 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 	if (data->flags & MMC_DATA_READ)
 		datactrl |= MCI_DPSM_DIRECTION;
 
-	if (variant->sdio && host->mmc->card)
-		if (mmc_card_sdio(host->mmc->card)) {
-			u32 clk;
-			datactrl |= variant->datactrl_mask_sdio;
+	if (host->mmc->card && mmc_card_sdio(host->mmc->card)) {
+		u32 clk;
 
-			/*
-			 * The ST Micro variant for SDIO small write transfers
-			 * needs to have clock H/W flow control disabled,
-			 * otherwise the transfer will not start. The threshold
-			 * depends on the rate of MCLK.
-			 */
-			if (data->flags & MMC_DATA_WRITE &&
-			    (host->size < 8 ||
-			     (host->size <= 8 && host->mclk > 50000000)))
-				clk = host->clk_reg & ~variant->clkreg_enable;
-			else
-				clk = host->clk_reg | variant->clkreg_enable;
+		datactrl |= variant->datactrl_mask_sdio;
 
-			mmci_write_clkreg(host, clk);
-		}
+		/*
+		 * The ST Micro variant for SDIO small write transfers
+		 * needs to have clock H/W flow control disabled,
+		 * otherwise the transfer will not start. The threshold
+		 * depends on the rate of MCLK.
+		 */
+		if (variant->st_sdio && data->flags & MMC_DATA_WRITE &&
+		    (host->size < 8 ||
+		     (host->size <= 8 && host->mclk > 50000000)))
+			clk = host->clk_reg & ~variant->clkreg_enable;
+		else
+			clk = host->clk_reg | variant->clkreg_enable;
+
+		mmci_write_clkreg(host, clk);
+	}
 
 	if (host->mmc->ios.timing == MMC_TIMING_UHS_DDR50 ||
 	    host->mmc->ios.timing == MMC_TIMING_MMC_DDR52)
-- 
1.9.1

Re: [PATCH v2 2/4] mmc: mmci: move block size validation under relevant code

[Ulf Hansson]

On 19 August 2014 13:14, Srinivas Kandagatla
<srinivas.kandagatla@linaro.org> wrote:
> This code moves a BUG_ON condition to relevant if block, this check is
> not necessary for IPs which can set any arbitrary block size in a given
> range.
> This patch is necessary for SDIO which sets block sizes that are exactly
> not power of 2.
>
> Original issue detected while testing WLAN ath6kl on Qualcomm APQ8064 SOC.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>  drivers/mmc/host/mmci.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index 3089fba..1c99195 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -800,15 +800,16 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
>         writel(timeout, base + MMCIDATATIMER);
>         writel(host->size, base + MMCIDATALENGTH);
>
> -       blksz_bits = ffs(data->blksz) - 1;
> -       BUG_ON(1 << blksz_bits != data->blksz);

I don't like this BUG_ON at all, I would prefer if we remove it. The
original patch "mmc: mmci: Support any block sizes for ux500v2", did
so as well.

Now, if we still think removing it is fragile, let additional tests in
mmci_validate_data() and return and error code from there.

Kind regards
Uffe

>
> -       if (variant->blksz_datactrl16)
> +       if (variant->blksz_datactrl16) {
>                 datactrl = MCI_DPSM_ENABLE | (data->blksz << 16);
> -       else if (variant->blksz_datactrl4)
> +       } else if (variant->blksz_datactrl4) {
>                 datactrl = MCI_DPSM_ENABLE | (data->blksz << 4);
> -       else
> +       } else {
> +               blksz_bits = ffs(data->blksz) - 1;
> +               BUG_ON(1 << blksz_bits != data->blksz);
>                 datactrl = MCI_DPSM_ENABLE | blksz_bits << 4;
> +       }
>
>         if (data->flags & MMC_DATA_READ)
>                 datactrl |= MCI_DPSM_DIRECTION;
> --
> 1.9.1
>

Re: [PATCH v2 2/4] mmc: mmci: move block size validation under relevant code

[Srinivas Kandagatla]

On 19/08/14 12:55, Ulf Hansson wrote:
>>          writel(host->size, base + MMCIDATALENGTH);
>> >
>> >-       blksz_bits = ffs(data->blksz) - 1;
>> >-       BUG_ON(1 << blksz_bits != data->blksz);
> I don't like this BUG_ON at all, I would prefer if we remove it. The
> original patch "mmc: mmci: Support any block sizes for ux500v2", did
> so as well.
>
Yes, you are right this is redundant check, mmci_validate_data in 
mmci_request should catch this error anyway. we can remove this check 
totally and use your original patch.


--srini
> Now, if we still think removing it is fragile, let additional tests in
> mmci_validate_data() and return and error code from there.

Re: [PATCH v2 1/4] mmc: mmci: Support any block sizes for ux500v2 and qcom variant

[Ulf Hansson]

On 19 August 2014 13:14, Srinivas Kandagatla
<srinivas.kandagatla@linaro.org> wrote:
> From: Ulf Hansson <ulf.hansson@linaro.org>
>
> For the ux500v2 variant of the PL18x block, any block sizes are
> supported. This will make it possible to decrease data overhead
> for SDIO transfers.
>
> This patch is based on Ulf Hansson patch
> http://www.spinics.net/lists/linux-mmc/msg12160.html
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>         enabled this support on qcom variant.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/mmc/host/mmci.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index c11cb05..3089fba 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -77,6 +77,7 @@ static unsigned int fmax = 515633;
>   * @qcom_fifo: enables qcom specific fifo pio read logic.
>   * @reversed_irq_handling: handle data irq before cmd irq.
>   * @qcom_dml: enables qcom specific dma glue for dma transfers.
> + * @any_blksize: true if block any sizes are supported
>   */
>  struct variant_data {
>         unsigned int            clkreg;
> @@ -102,6 +103,7 @@ struct variant_data {
>         bool                    qcom_fifo;
>         bool                    reversed_irq_handling;
>         bool                    qcom_dml;
> +       bool                    any_blksize;
>  };
>
>  static struct variant_data variant_arm = {
> @@ -194,6 +196,7 @@ static struct variant_data variant_ux500v2 = {
>         .pwrreg_clkgate         = true,
>         .busy_detect            = true,
>         .pwrreg_nopower         = true,
> +       .any_blksize            = true,
>  };

There are some prerequisites of the data buffers to supports any block
size, at least for ux500.
Try read up on this discussion:
http://marc.info/?t=135005062400002&r=2&w=2

The conclusion from the above is that we need to adopt
mmci_pio_write() to handle corner cases.

Now, I suppose, unless someone objects, we could go ahead and merge
this patch. Then you will have to fixup mmci_pio_write() later on.

Kind regards
Uffe

[PATCH v3 0/3] mmc: mmci: sdio related fixes

[Srinivas Kandagatla]

This patchset fixes few sdio related issues encountered while testing
WLAN ath6kl via SDIO on IFC6410 board with Qualcomm APQ8064 SOC.

Patch: "mmc: mmci: Support any block sizes for ux500v2 and qcom variant" is
a very old patch by Ulf to support IPs which support any size of block sizes.
http://www.spinics.net/lists/linux-mmc/msg12160.html I modified the subject
line to include qcom.
Patch fixes below issues reported while testing sdio.
The issue was ath6kl driver was issuing 12 bytes and 24 bytes reads
which are caught as part of the error handing in the driver and
resulting in failures. 

Patch "mmc: mmci: Add sdio enable mask in variant data" adds extra
variant parameter to enable sdio. This makes mmci driver more flexible.

Patch "mmc: mmci: rename sdio flag in vendor data to st_sdio" renames sdio
flag in vendor data to st_sdio, as this flag is only used to setup st
specific sdio logic.

All these patches are tested on IFC6410 board with ath6kl WLAN via SDIO.

Thanks to Linus W, Ulf annd Russell for comments since RFC.

Changes since v2:
	- removed "mmc: mmci: move block size validation under relevant code" patch
	as this is already fixed by original patch from Ulf.

Changes since RFC:
	- moved sdio flag to st_sdio to simplify the checks.
	- use Ulf's patch to address IP's which support anysize blocks.

Thanks,
srini

Srinivas Kandagatla (2):
  mmc: mmci: Add sdio enable mask in variant data
  mmc: mmci: rename sdio flag in vendor data to st_sdio

Ulf Hansson (1):
  mmc: mmci: Support any block sizes for ux500v2 and qcom variant

 drivers/mmc/host/mmci.c | 68 ++++++++++++++++++++++++++-----------------------
 1 file changed, 36 insertions(+), 32 deletions(-)

-- 
1.9.1

[PATCH v3 1/3] mmc: mmci: Support any block sizes for ux500v2 and qcom variant

[Srinivas Kandagatla]

From: Ulf Hansson <ulf.hansson@linaro.org>

For the ux500v2 variant of the PL18x block, any block sizes are
supported. This will make it possible to decrease data overhead
for SDIO transfers.

This patch is based on Ulf Hansson patch
http://www.spinics.net/lists/linux-mmc/msg12160.html

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
	enabled this support on qcom variant.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/mmc/host/mmci.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index c11cb05..533ad2b 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -77,6 +77,7 @@ static unsigned int fmax = 515633;
  * @qcom_fifo: enables qcom specific fifo pio read logic.
  * @reversed_irq_handling: handle data irq before cmd irq.
  * @qcom_dml: enables qcom specific dma glue for dma transfers.
+ * @any_blksize: true if block any sizes are supported
  */
 struct variant_data {
 	unsigned int		clkreg;
@@ -102,6 +103,7 @@ struct variant_data {
 	bool			qcom_fifo;
 	bool			reversed_irq_handling;
 	bool			qcom_dml;
+	bool			any_blksize;
 };
 
 static struct variant_data variant_arm = {
@@ -194,6 +196,7 @@ static struct variant_data variant_ux500v2 = {
 	.pwrreg_clkgate		= true,
 	.busy_detect		= true,
 	.pwrreg_nopower		= true,
+	.any_blksize		= true,
 };
 
 static struct variant_data variant_qcom = {
@@ -212,6 +215,7 @@ static struct variant_data variant_qcom = {
 	.explicit_mclk_control	= true,
 	.qcom_fifo		= true,
 	.qcom_dml		= true,
+	.any_blksize		= true,
 };
 
 static int mmci_card_busy(struct mmc_host *mmc)
@@ -239,10 +243,11 @@ static int mmci_card_busy(struct mmc_host *mmc)
 static int mmci_validate_data(struct mmci_host *host,
 			      struct mmc_data *data)
 {
+	struct variant_data *variant = host->variant;
+
 	if (!data)
 		return 0;
-
-	if (!is_power_of_2(data->blksz)) {
+	if (!is_power_of_2(data->blksz) && !variant->any_blksize) {
 		dev_err(mmc_dev(host->mmc),
 			"unsupported block size (%d bytes)\n", data->blksz);
 		return -EINVAL;
@@ -796,7 +801,6 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 	writel(host->size, base + MMCIDATALENGTH);
 
 	blksz_bits = ffs(data->blksz) - 1;
-	BUG_ON(1 << blksz_bits != data->blksz);
 
 	if (variant->blksz_datactrl16)
 		datactrl = MCI_DPSM_ENABLE | (data->blksz << 16);
-- 
1.9.1

[PATCH v3 2/3] mmc: mmci: Add sdio enable mask in variant data

[Srinivas Kandagatla]

This patch adds sdio enable mask in variant data, SOCs like ST have
special bits in datactrl register to enable sdio. Unconditionally setting
this bit in this driver breaks other SOCs like Qualcomm which maps this
bits to something else, so making this enable bit to come from variant
data solves the issue.

Originally the issue is detected while testing WLAN ath6kl on Qualcomm
APQ8064.

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

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 533ad2b..a25759e 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -67,6 +67,7 @@ static unsigned int fmax = 515633;
  * @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
+ * @datactrl_mask_sdio: SDIO enable mask 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
@@ -89,6 +90,7 @@ struct variant_data {
 	unsigned int		fifohalfsize;
 	unsigned int		data_cmd_enable;
 	unsigned int		datactrl_mask_ddrmode;
+	unsigned int		datactrl_mask_sdio;
 	bool			sdio;
 	bool			st_clkdiv;
 	bool			blksz_datactrl16;
@@ -138,6 +140,7 @@ static struct variant_data variant_u300 = {
 	.clkreg_enable		= MCI_ST_U300_HWFCEN,
 	.clkreg_8bit_bus_enable = MCI_ST_8BIT_BUS,
 	.datalength_bits	= 16,
+	.datactrl_mask_sdio	= MCI_ST_DPSM_SDIOEN,
 	.sdio			= true,
 	.pwrreg_powerup		= MCI_PWR_ON,
 	.f_max			= 100000000,
@@ -151,6 +154,7 @@ static struct variant_data variant_nomadik = {
 	.fifohalfsize		= 8 * 4,
 	.clkreg			= MCI_CLK_ENABLE,
 	.datalength_bits	= 24,
+	.datactrl_mask_sdio	= MCI_ST_DPSM_SDIOEN,
 	.sdio			= true,
 	.st_clkdiv		= true,
 	.pwrreg_powerup		= MCI_PWR_ON,
@@ -168,6 +172,7 @@ static struct variant_data variant_ux500 = {
 	.clkreg_8bit_bus_enable = MCI_ST_8BIT_BUS,
 	.clkreg_neg_edge_enable	= MCI_ST_UX500_NEG_EDGE,
 	.datalength_bits	= 24,
+	.datactrl_mask_sdio	= MCI_ST_DPSM_SDIOEN,
 	.sdio			= true,
 	.st_clkdiv		= true,
 	.pwrreg_powerup		= MCI_PWR_ON,
@@ -187,6 +192,7 @@ static struct variant_data variant_ux500v2 = {
 	.clkreg_neg_edge_enable	= MCI_ST_UX500_NEG_EDGE,
 	.datactrl_mask_ddrmode	= MCI_ST_DPSM_DDRMODE,
 	.datalength_bits	= 24,
+	.datactrl_mask_sdio	= MCI_ST_DPSM_SDIOEN,
 	.sdio			= true,
 	.st_clkdiv		= true,
 	.blksz_datactrl16	= true,
@@ -812,16 +818,10 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 	if (data->flags & MMC_DATA_READ)
 		datactrl |= MCI_DPSM_DIRECTION;
 
-	/* The ST Micro variants has a special bit to enable SDIO */
 	if (variant->sdio && host->mmc->card)
 		if (mmc_card_sdio(host->mmc->card)) {
-			/*
-			 * The ST Micro variants has a special bit
-			 * to enable SDIO.
-			 */
 			u32 clk;
-
-			datactrl |= MCI_ST_DPSM_SDIOEN;
+			datactrl |= variant->datactrl_mask_sdio;
 
 			/*
 			 * The ST Micro variant for SDIO small write transfers
-- 
1.9.1

[PATCH v3 3/3] mmc: mmci: rename sdio flag in vendor data to st_sdio

[Srinivas Kandagatla]

This patch renames sdio flag in vendor data to st_sdio, as this flag is
only used to enable ST specific sdio setup. This will also ensure that
the ST specfic setup is not done on other vendor like Qualcomm.

Originally the issue was detected while testing WLAN ath6kl on IFC6410
board with APQ8064 SOC.

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

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index a25759e..264c947 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -61,7 +61,7 @@ static unsigned int fmax = 515633;
  * @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_sdio: enable ST specific SDIO logic
  * @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
@@ -91,7 +91,7 @@ struct variant_data {
 	unsigned int		data_cmd_enable;
 	unsigned int		datactrl_mask_ddrmode;
 	unsigned int		datactrl_mask_sdio;
-	bool			sdio;
+	bool			st_sdio;
 	bool			st_clkdiv;
 	bool			blksz_datactrl16;
 	bool			blksz_datactrl4;
@@ -141,7 +141,7 @@ static struct variant_data variant_u300 = {
 	.clkreg_8bit_bus_enable = MCI_ST_8BIT_BUS,
 	.datalength_bits	= 16,
 	.datactrl_mask_sdio	= MCI_ST_DPSM_SDIOEN,
-	.sdio			= true,
+	.st_sdio			= true,
 	.pwrreg_powerup		= MCI_PWR_ON,
 	.f_max			= 100000000,
 	.signal_direction	= true,
@@ -155,7 +155,7 @@ static struct variant_data variant_nomadik = {
 	.clkreg			= MCI_CLK_ENABLE,
 	.datalength_bits	= 24,
 	.datactrl_mask_sdio	= MCI_ST_DPSM_SDIOEN,
-	.sdio			= true,
+	.st_sdio		= true,
 	.st_clkdiv		= true,
 	.pwrreg_powerup		= MCI_PWR_ON,
 	.f_max			= 100000000,
@@ -173,7 +173,7 @@ static struct variant_data variant_ux500 = {
 	.clkreg_neg_edge_enable	= MCI_ST_UX500_NEG_EDGE,
 	.datalength_bits	= 24,
 	.datactrl_mask_sdio	= MCI_ST_DPSM_SDIOEN,
-	.sdio			= true,
+	.st_sdio		= true,
 	.st_clkdiv		= true,
 	.pwrreg_powerup		= MCI_PWR_ON,
 	.f_max			= 100000000,
@@ -193,7 +193,7 @@ static struct variant_data variant_ux500v2 = {
 	.datactrl_mask_ddrmode	= MCI_ST_DPSM_DDRMODE,
 	.datalength_bits	= 24,
 	.datactrl_mask_sdio	= MCI_ST_DPSM_SDIOEN,
-	.sdio			= true,
+	.st_sdio		= true,
 	.st_clkdiv		= true,
 	.blksz_datactrl16	= true,
 	.pwrreg_powerup		= MCI_PWR_ON,
@@ -818,26 +818,26 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 	if (data->flags & MMC_DATA_READ)
 		datactrl |= MCI_DPSM_DIRECTION;
 
-	if (variant->sdio && host->mmc->card)
-		if (mmc_card_sdio(host->mmc->card)) {
-			u32 clk;
-			datactrl |= variant->datactrl_mask_sdio;
+	if (host->mmc->card && mmc_card_sdio(host->mmc->card)) {
+		u32 clk;
 
-			/*
-			 * The ST Micro variant for SDIO small write transfers
-			 * needs to have clock H/W flow control disabled,
-			 * otherwise the transfer will not start. The threshold
-			 * depends on the rate of MCLK.
-			 */
-			if (data->flags & MMC_DATA_WRITE &&
-			    (host->size < 8 ||
-			     (host->size <= 8 && host->mclk > 50000000)))
-				clk = host->clk_reg & ~variant->clkreg_enable;
-			else
-				clk = host->clk_reg | variant->clkreg_enable;
+		datactrl |= variant->datactrl_mask_sdio;
 
-			mmci_write_clkreg(host, clk);
-		}
+		/*
+		 * The ST Micro variant for SDIO small write transfers
+		 * needs to have clock H/W flow control disabled,
+		 * otherwise the transfer will not start. The threshold
+		 * depends on the rate of MCLK.
+		 */
+		if (variant->st_sdio && data->flags & MMC_DATA_WRITE &&
+		    (host->size < 8 ||
+		     (host->size <= 8 && host->mclk > 50000000)))
+			clk = host->clk_reg & ~variant->clkreg_enable;
+		else
+			clk = host->clk_reg | variant->clkreg_enable;
+
+		mmci_write_clkreg(host, clk);
+	}
 
 	if (host->mmc->ios.timing == MMC_TIMING_UHS_DDR50 ||
 	    host->mmc->ios.timing == MMC_TIMING_MMC_DDR52)
-- 
1.9.1

[PATCH] mmc: mmci_qcom_dml: fix is never less than zero warning.

[Srinivas Kandagatla]

This patch fixes a typo which resulted in 'is never less
than zero warning' reported by static checker.

	drivers/mmc/host/mmci_qcom_dml.c:131 dml_hw_init()
	warn: unsigned 'producer_id' is never less than zero.

drivers/mmc/host/mmci_qcom_dml.c
   121  /* Initialize the dml hardware connected to SD Card controller */
   122  int dml_hw_init(struct mmci_host *host, struct device_node *np)
   123  {
   124          u32 config;
   125          void __iomem *base;
   126          u32 consumer_id, producer_id;
   127
   128          consumer_id = of_get_dml_pipe_index(np, "tx");
   129          producer_id = of_get_dml_pipe_index(np, "rx");
   130
   131          if (producer_id < 0 || consumer_id < 0)
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   132

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
Hi Ulf, 

This patch is on top of https://git.linaro.org/people/ulf.hansson/mmc.git next branch
to fix up a warning reported by static checker tool.

Are you ok to take this patch as it is or do you think I should resend the 
original DML patch with this fix?

thanks,
srini

 drivers/mmc/host/mmci_qcom_dml.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/mmci_qcom_dml.c b/drivers/mmc/host/mmci_qcom_dml.c
index 300ef50..2b7fc37 100644
--- a/drivers/mmc/host/mmci_qcom_dml.c
+++ b/drivers/mmc/host/mmci_qcom_dml.c
@@ -123,7 +123,7 @@ int dml_hw_init(struct mmci_host *host, struct device_node *np)
 {
 	u32 config;
 	void __iomem *base;
-	u32 consumer_id, producer_id;
+	int consumer_id, producer_id;
 
 	consumer_id = of_get_dml_pipe_index(np, "tx");
 	producer_id = of_get_dml_pipe_index(np, "rx");
-- 
1.9.1

Re: [PATCH] mmc: mmci_qcom_dml: fix is never less than zero warning.

[Ulf Hansson]

On 9 September 2014 10:34, Srinivas Kandagatla
<srinivas.kandagatla@linaro.org> wrote:
> This patch fixes a typo which resulted in 'is never less
> than zero warning' reported by static checker.
>
>         drivers/mmc/host/mmci_qcom_dml.c:131 dml_hw_init()
>         warn: unsigned 'producer_id' is never less than zero.
>
> drivers/mmc/host/mmci_qcom_dml.c
>    121  /* Initialize the dml hardware connected to SD Card controller */
>    122  int dml_hw_init(struct mmci_host *host, struct device_node *np)
>    123  {
>    124          u32 config;
>    125          void __iomem *base;
>    126          u32 consumer_id, producer_id;
>    127
>    128          consumer_id = of_get_dml_pipe_index(np, "tx");
>    129          producer_id = of_get_dml_pipe_index(np, "rx");
>    130
>    131          if (producer_id < 0 || consumer_id < 0)
>                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>    132
>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
> Hi Ulf,
>
> This patch is on top of https://git.linaro.org/people/ulf.hansson/mmc.git next branch
> to fix up a warning reported by static checker tool.
>
> Are you ok to take this patch as it is or do you think I should resend the
> original DML patch with this fix?
>

I am fine with this, I will fold it into the original patch. Applied for next.

Kind regards
Uffe

> thanks,
> srini
>
>  drivers/mmc/host/mmci_qcom_dml.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/mmci_qcom_dml.c b/drivers/mmc/host/mmci_qcom_dml.c
> index 300ef50..2b7fc37 100644
> --- a/drivers/mmc/host/mmci_qcom_dml.c
> +++ b/drivers/mmc/host/mmci_qcom_dml.c
> @@ -123,7 +123,7 @@ int dml_hw_init(struct mmci_host *host, struct device_node *np)
>  {
>         u32 config;
>         void __iomem *base;
> -       u32 consumer_id, producer_id;
> +       int consumer_id, producer_id;
>
>         consumer_id = of_get_dml_pipe_index(np, "tx");
>         producer_id = of_get_dml_pipe_index(np, "rx");
> --
> 1.9.1
>

Re: [PATCH v3 2/3] mmc: mmci: Add sdio enable mask in variant data

[Ulf Hansson]

On 22 August 2014 06:54, Srinivas Kandagatla
<srinivas.kandagatla@linaro.org> wrote:
> This patch adds sdio enable mask in variant data, SOCs like ST have
> special bits in datactrl register to enable sdio. Unconditionally setting
> this bit in this driver breaks other SOCs like Qualcomm which maps this
> bits to something else, so making this enable bit to come from variant
> data solves the issue.
>
> Originally the issue is detected while testing WLAN ath6kl on Qualcomm
> APQ8064.
>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Thanks! Applied for next!

Kind regards
Uffe

> ---
>  drivers/mmc/host/mmci.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index 533ad2b..a25759e 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -67,6 +67,7 @@ static unsigned int fmax = 515633;
>   * @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
> + * @datactrl_mask_sdio: SDIO enable mask 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
> @@ -89,6 +90,7 @@ struct variant_data {
>         unsigned int            fifohalfsize;
>         unsigned int            data_cmd_enable;
>         unsigned int            datactrl_mask_ddrmode;
> +       unsigned int            datactrl_mask_sdio;
>         bool                    sdio;
>         bool                    st_clkdiv;
>         bool                    blksz_datactrl16;
> @@ -138,6 +140,7 @@ static struct variant_data variant_u300 = {
>         .clkreg_enable          = MCI_ST_U300_HWFCEN,
>         .clkreg_8bit_bus_enable = MCI_ST_8BIT_BUS,
>         .datalength_bits        = 16,
> +       .datactrl_mask_sdio     = MCI_ST_DPSM_SDIOEN,
>         .sdio                   = true,
>         .pwrreg_powerup         = MCI_PWR_ON,
>         .f_max                  = 100000000,
> @@ -151,6 +154,7 @@ static struct variant_data variant_nomadik = {
>         .fifohalfsize           = 8 * 4,
>         .clkreg                 = MCI_CLK_ENABLE,
>         .datalength_bits        = 24,
> +       .datactrl_mask_sdio     = MCI_ST_DPSM_SDIOEN,
>         .sdio                   = true,
>         .st_clkdiv              = true,
>         .pwrreg_powerup         = MCI_PWR_ON,
> @@ -168,6 +172,7 @@ static struct variant_data variant_ux500 = {
>         .clkreg_8bit_bus_enable = MCI_ST_8BIT_BUS,
>         .clkreg_neg_edge_enable = MCI_ST_UX500_NEG_EDGE,
>         .datalength_bits        = 24,
> +       .datactrl_mask_sdio     = MCI_ST_DPSM_SDIOEN,
>         .sdio                   = true,
>         .st_clkdiv              = true,
>         .pwrreg_powerup         = MCI_PWR_ON,
> @@ -187,6 +192,7 @@ static struct variant_data variant_ux500v2 = {
>         .clkreg_neg_edge_enable = MCI_ST_UX500_NEG_EDGE,
>         .datactrl_mask_ddrmode  = MCI_ST_DPSM_DDRMODE,
>         .datalength_bits        = 24,
> +       .datactrl_mask_sdio     = MCI_ST_DPSM_SDIOEN,
>         .sdio                   = true,
>         .st_clkdiv              = true,
>         .blksz_datactrl16       = true,
> @@ -812,16 +818,10 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
>         if (data->flags & MMC_DATA_READ)
>                 datactrl |= MCI_DPSM_DIRECTION;
>
> -       /* The ST Micro variants has a special bit to enable SDIO */
>         if (variant->sdio && host->mmc->card)
>                 if (mmc_card_sdio(host->mmc->card)) {
> -                       /*
> -                        * The ST Micro variants has a special bit
> -                        * to enable SDIO.
> -                        */
>                         u32 clk;
> -
> -                       datactrl |= MCI_ST_DPSM_SDIOEN;
> +                       datactrl |= variant->datactrl_mask_sdio;
>
>                         /*
>                          * The ST Micro variant for SDIO small write transfers
> --
> 1.9.1
>

Re: [PATCH v3 3/3] mmc: mmci: rename sdio flag in vendor data to st_sdio

[Ulf Hansson]

On 22 August 2014 06:55, Srinivas Kandagatla
<srinivas.kandagatla@linaro.org> wrote:
> This patch renames sdio flag in vendor data to st_sdio, as this flag is
> only used to enable ST specific sdio setup. This will also ensure that
> the ST specfic setup is not done on other vendor like Qualcomm.
>
> Originally the issue was detected while testing WLAN ath6kl on IFC6410
> board with APQ8064 SOC.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Thanks! Applied for next!

Kind regards
Uffe

> ---
>  drivers/mmc/host/mmci.c | 48 ++++++++++++++++++++++++------------------------
>  1 file changed, 24 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index a25759e..264c947 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -61,7 +61,7 @@ static unsigned int fmax = 515633;
>   * @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_sdio: enable ST specific SDIO logic
>   * @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
> @@ -91,7 +91,7 @@ struct variant_data {
>         unsigned int            data_cmd_enable;
>         unsigned int            datactrl_mask_ddrmode;
>         unsigned int            datactrl_mask_sdio;
> -       bool                    sdio;
> +       bool                    st_sdio;
>         bool                    st_clkdiv;
>         bool                    blksz_datactrl16;
>         bool                    blksz_datactrl4;
> @@ -141,7 +141,7 @@ static struct variant_data variant_u300 = {
>         .clkreg_8bit_bus_enable = MCI_ST_8BIT_BUS,
>         .datalength_bits        = 16,
>         .datactrl_mask_sdio     = MCI_ST_DPSM_SDIOEN,
> -       .sdio                   = true,
> +       .st_sdio                        = true,
>         .pwrreg_powerup         = MCI_PWR_ON,
>         .f_max                  = 100000000,
>         .signal_direction       = true,
> @@ -155,7 +155,7 @@ static struct variant_data variant_nomadik = {
>         .clkreg                 = MCI_CLK_ENABLE,
>         .datalength_bits        = 24,
>         .datactrl_mask_sdio     = MCI_ST_DPSM_SDIOEN,
> -       .sdio                   = true,
> +       .st_sdio                = true,
>         .st_clkdiv              = true,
>         .pwrreg_powerup         = MCI_PWR_ON,
>         .f_max                  = 100000000,
> @@ -173,7 +173,7 @@ static struct variant_data variant_ux500 = {
>         .clkreg_neg_edge_enable = MCI_ST_UX500_NEG_EDGE,
>         .datalength_bits        = 24,
>         .datactrl_mask_sdio     = MCI_ST_DPSM_SDIOEN,
> -       .sdio                   = true,
> +       .st_sdio                = true,
>         .st_clkdiv              = true,
>         .pwrreg_powerup         = MCI_PWR_ON,
>         .f_max                  = 100000000,
> @@ -193,7 +193,7 @@ static struct variant_data variant_ux500v2 = {
>         .datactrl_mask_ddrmode  = MCI_ST_DPSM_DDRMODE,
>         .datalength_bits        = 24,
>         .datactrl_mask_sdio     = MCI_ST_DPSM_SDIOEN,
> -       .sdio                   = true,
> +       .st_sdio                = true,
>         .st_clkdiv              = true,
>         .blksz_datactrl16       = true,
>         .pwrreg_powerup         = MCI_PWR_ON,
> @@ -818,26 +818,26 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
>         if (data->flags & MMC_DATA_READ)
>                 datactrl |= MCI_DPSM_DIRECTION;
>
> -       if (variant->sdio && host->mmc->card)
> -               if (mmc_card_sdio(host->mmc->card)) {
> -                       u32 clk;
> -                       datactrl |= variant->datactrl_mask_sdio;
> +       if (host->mmc->card && mmc_card_sdio(host->mmc->card)) {
> +               u32 clk;
>
> -                       /*
> -                        * The ST Micro variant for SDIO small write transfers
> -                        * needs to have clock H/W flow control disabled,
> -                        * otherwise the transfer will not start. The threshold
> -                        * depends on the rate of MCLK.
> -                        */
> -                       if (data->flags & MMC_DATA_WRITE &&
> -                           (host->size < 8 ||
> -                            (host->size <= 8 && host->mclk > 50000000)))
> -                               clk = host->clk_reg & ~variant->clkreg_enable;
> -                       else
> -                               clk = host->clk_reg | variant->clkreg_enable;
> +               datactrl |= variant->datactrl_mask_sdio;
>
> -                       mmci_write_clkreg(host, clk);
> -               }
> +               /*
> +                * The ST Micro variant for SDIO small write transfers
> +                * needs to have clock H/W flow control disabled,
> +                * otherwise the transfer will not start. The threshold
> +                * depends on the rate of MCLK.
> +                */
> +               if (variant->st_sdio && data->flags & MMC_DATA_WRITE &&
> +                   (host->size < 8 ||
> +                    (host->size <= 8 && host->mclk > 50000000)))
> +                       clk = host->clk_reg & ~variant->clkreg_enable;
> +               else
> +                       clk = host->clk_reg | variant->clkreg_enable;
> +
> +               mmci_write_clkreg(host, clk);
> +       }
>
>         if (host->mmc->ios.timing == MMC_TIMING_UHS_DDR50 ||
>             host->mmc->ios.timing == MMC_TIMING_MMC_DDR52)
> --
> 1.9.1
>

Re: [PATCH v3 1/3] mmc: mmci: Support any block sizes for ux500v2 and qcom variant

[Ulf Hansson]

On 22 August 2014 06:54, Srinivas Kandagatla
<srinivas.kandagatla@linaro.org> wrote:
> From: Ulf Hansson <ulf.hansson@linaro.org>
>
> For the ux500v2 variant of the PL18x block, any block sizes are
> supported. This will make it possible to decrease data overhead
> for SDIO transfers.
>
> This patch is based on Ulf Hansson patch
> http://www.spinics.net/lists/linux-mmc/msg12160.html
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>         enabled this support on qcom variant.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

I am not sure how to handle this patch.

It will as you say in the cover letter for this patchset, improve
situations for the ath6kl driver when it's issuing 12 bytes and 24
bytes reads and solve those issues.

On the other hand, as stated earlier mmci_pio_write need to be fixed
to have full support for any block size. That applies to the qcom
variant as well.

For ux500, I am sure this won't cause any regressions since the cw1200
isn't probed. Also, I am not sure the "any block size" support is even
enabled for that driver.

How about, that we add a comment in the pio_write function describing
that we need to fix it for "SDIO any block size" support? And leave
that as a future improvement?

Kind regards
Uffe

> ---
>  drivers/mmc/host/mmci.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index c11cb05..533ad2b 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -77,6 +77,7 @@ static unsigned int fmax = 515633;
>   * @qcom_fifo: enables qcom specific fifo pio read logic.
>   * @reversed_irq_handling: handle data irq before cmd irq.
>   * @qcom_dml: enables qcom specific dma glue for dma transfers.
> + * @any_blksize: true if block any sizes are supported
>   */
>  struct variant_data {
>         unsigned int            clkreg;
> @@ -102,6 +103,7 @@ struct variant_data {
>         bool                    qcom_fifo;
>         bool                    reversed_irq_handling;
>         bool                    qcom_dml;
> +       bool                    any_blksize;
>  };
>
>  static struct variant_data variant_arm = {
> @@ -194,6 +196,7 @@ static struct variant_data variant_ux500v2 = {
>         .pwrreg_clkgate         = true,
>         .busy_detect            = true,
>         .pwrreg_nopower         = true,
> +       .any_blksize            = true,
>  };
>
>  static struct variant_data variant_qcom = {
> @@ -212,6 +215,7 @@ static struct variant_data variant_qcom = {
>         .explicit_mclk_control  = true,
>         .qcom_fifo              = true,
>         .qcom_dml               = true,
> +       .any_blksize            = true,
>  };
>
>  static int mmci_card_busy(struct mmc_host *mmc)
> @@ -239,10 +243,11 @@ static int mmci_card_busy(struct mmc_host *mmc)
>  static int mmci_validate_data(struct mmci_host *host,
>                               struct mmc_data *data)
>  {
> +       struct variant_data *variant = host->variant;
> +
>         if (!data)
>                 return 0;
> -
> -       if (!is_power_of_2(data->blksz)) {
> +       if (!is_power_of_2(data->blksz) && !variant->any_blksize) {
>                 dev_err(mmc_dev(host->mmc),
>                         "unsupported block size (%d bytes)\n", data->blksz);
>                 return -EINVAL;
> @@ -796,7 +801,6 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
>         writel(host->size, base + MMCIDATALENGTH);
>
>         blksz_bits = ffs(data->blksz) - 1;
> -       BUG_ON(1 << blksz_bits != data->blksz);
>
>         if (variant->blksz_datactrl16)
>                 datactrl = MCI_DPSM_ENABLE | (data->blksz << 16);
> --
> 1.9.1
>

Re: [PATCH v3 1/3] mmc: mmci: Support any block sizes for ux500v2 and qcom variant

[Srinivas Kandagatla]

Hi Ulf,

On 10/09/14 08:58, Ulf Hansson wrote:
> On 22 August 2014 06:54, Srinivas Kandagatla
> <srinivas.kandagatla@linaro.org> wrote:
>> From: Ulf Hansson <ulf.hansson@linaro.org>
>>
>> For the ux500v2 variant of the PL18x block, any block sizes are
>> supported. This will make it possible to decrease data overhead
>> for SDIO transfers.
>>
>> This patch is based on Ulf Hansson patch
>> http://www.spinics.net/lists/linux-mmc/msg12160.html
>>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>          enabled this support on qcom variant.
>>
>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>
> I am not sure how to handle this patch.
>
> It will as you say in the cover letter for this patchset, improve
> situations for the ath6kl driver when it's issuing 12 bytes and 24
> bytes reads and solve those issues.
>
> On the other hand, as stated earlier mmci_pio_write need to be fixed
> to have full support for any block size. That applies to the qcom
> variant as well.
looking at current mmci_pio_write, I see it can support any block sizes 
as it is. Unless Am missing something obvious.

block size aligned to 4 is taken care in the code and is straight forward.
block sizes not aligned to 4 are also partly taken care in pio_write and 
partly by programming blksz in datactrl register. However Am not sure if 
it was safe to handle the buffer pointer out of its boundary.
>
> For ux500, I am sure this won't cause any regressions since the cw1200
> isn't probed. Also, I am not sure the "any block size" support is even
> enabled for that driver.
>
> How about, that we add a comment in the pio_write function describing
> that we need to fix it for "SDIO any block size" support? And leave
> that as a future improvement?

Is below patch any good?
With the below patch It should be possible to address the case where 
buffer passed the length is handled safely. If you ok with the approach 
I can send a patch as RFC.


----cut here----
diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 264c947..8480c02 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -1134,7 +1134,20 @@ static int mmci_pio_write(struct mmci_host *host, 
char *buffer, unsigned int rem
                  * byte become a 32bit write, 7 bytes will be two
                  * 32bit writes etc.
                  */
-               iowrite32_rep(base + MMCIFIFO, ptr, (count + 3) >> 2);
+               if (unlikely(count & 0x3)) {
+                       unsigned char buf[4] = {0, };
+
+                       if (count < 4) {
+                               memcpy(buf, ptr, count);
+                               iowrite32_rep(base + MMCIFIFO, buf, 1);
+                       } else {
+                               iowrite32_rep(base + MMCIFIFO, ptr, 
count >> 2);
+                               memcpy(buf, ptr + (count & ~0x3), count 
& 0x3);
+                               iowrite32_rep(base + MMCIFIFO, buf, 1);
+                       }
+               } else {
+                       iowrite32_rep(base + MMCIFIFO, ptr, count >> 2);
+               }

                 ptr += count;
                 remain -= count;

----cut here----

thanks,
srini
>
> Kind regards
> Uffe
>
>> ---
>>   drivers/mmc/host/mmci.c | 10 +++++++---
>>   1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
>> index c11cb05..533ad2b 100644
>> --- a/drivers/mmc/host/mmci.c
>> +++ b/drivers/mmc/host/mmci.c
>> @@ -77,6 +77,7 @@ static unsigned int fmax = 515633;
>>    * @qcom_fifo: enables qcom specific fifo pio read logic.
>>    * @reversed_irq_handling: handle data irq before cmd irq.
>>    * @qcom_dml: enables qcom specific dma glue for dma transfers.
>> + * @any_blksize: true if block any sizes are supported
>>    */
>>   struct variant_data {
>>          unsigned int            clkreg;
>> @@ -102,6 +103,7 @@ struct variant_data {
>>          bool                    qcom_fifo;
>>          bool                    reversed_irq_handling;
>>          bool                    qcom_dml;
>> +       bool                    any_blksize;
>>   };
>>
>>   static struct variant_data variant_arm = {
>> @@ -194,6 +196,7 @@ static struct variant_data variant_ux500v2 = {
>>          .pwrreg_clkgate         = true,
>>          .busy_detect            = true,
>>          .pwrreg_nopower         = true,
>> +       .any_blksize            = true,
>>   };
>>
>>   static struct variant_data variant_qcom = {
>> @@ -212,6 +215,7 @@ static struct variant_data variant_qcom = {
>>          .explicit_mclk_control  = true,
>>          .qcom_fifo              = true,
>>          .qcom_dml               = true,
>> +       .any_blksize            = true,
>>   };
>>
>>   static int mmci_card_busy(struct mmc_host *mmc)
>> @@ -239,10 +243,11 @@ static int mmci_card_busy(struct mmc_host *mmc)
>>   static int mmci_validate_data(struct mmci_host *host,
>>                                struct mmc_data *data)
>>   {
>> +       struct variant_data *variant = host->variant;
>> +
>>          if (!data)
>>                  return 0;
>> -
>> -       if (!is_power_of_2(data->blksz)) {
>> +       if (!is_power_of_2(data->blksz) && !variant->any_blksize) {
>>                  dev_err(mmc_dev(host->mmc),
>>                          "unsupported block size (%d bytes)\n", data->blksz);
>>                  return -EINVAL;
>> @@ -796,7 +801,6 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
>>          writel(host->size, base + MMCIDATALENGTH);
>>
>>          blksz_bits = ffs(data->blksz) - 1;
>> -       BUG_ON(1 << blksz_bits != data->blksz);
>>
>>          if (variant->blksz_datactrl16)
>>                  datactrl = MCI_DPSM_ENABLE | (data->blksz << 16);
>> --
>> 1.9.1
>>

Re: [PATCH v3 1/3] mmc: mmci: Support any block sizes for ux500v2 and qcom variant

[Ulf Hansson]

On 10 September 2014 11:07, Srinivas Kandagatla
<srinivas.kandagatla@linaro.org> wrote:
> Hi Ulf,
>
> On 10/09/14 08:58, Ulf Hansson wrote:
>>
>> On 22 August 2014 06:54, Srinivas Kandagatla
>> <srinivas.kandagatla@linaro.org> wrote:
>>>
>>> From: Ulf Hansson <ulf.hansson@linaro.org>
>>>
>>> For the ux500v2 variant of the PL18x block, any block sizes are
>>> supported. This will make it possible to decrease data overhead
>>> for SDIO transfers.
>>>
>>> This patch is based on Ulf Hansson patch
>>> http://www.spinics.net/lists/linux-mmc/msg12160.html
>>>
>>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>>          enabled this support on qcom variant.
>>>
>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>>
>>
>> I am not sure how to handle this patch.
>>
>> It will as you say in the cover letter for this patchset, improve
>> situations for the ath6kl driver when it's issuing 12 bytes and 24
>> bytes reads and solve those issues.
>>
>> On the other hand, as stated earlier mmci_pio_write need to be fixed
>> to have full support for any block size. That applies to the qcom
>> variant as well.
>
> looking at current mmci_pio_write, I see it can support any block sizes as
> it is. Unless Am missing something obvious.
>
> block size aligned to 4 is taken care in the code and is straight forward.
> block sizes not aligned to 4 are also partly taken care in pio_write and
> partly by programming blksz in datactrl register. However Am not sure if it
> was safe to handle the buffer pointer out of its boundary.

There are some prerequisites of the data buffers to supports any block
size. Make sure you read up on this discussion to really understand the problem.

http://marc.info/?t=135005062400002&r=2&w=2

>>
>>
>> For ux500, I am sure this won't cause any regressions since the cw1200
>> isn't probed. Also, I am not sure the "any block size" support is even
>> enabled for that driver.
>>
>> How about, that we add a comment in the pio_write function describing
>> that we need to fix it for "SDIO any block size" support? And leave
>> that as a future improvement?
>
>
> Is below patch any good?
> With the below patch It should be possible to address the case where buffer
> passed the length is handled safely. If you ok with the approach I can send
> a patch as RFC.

Please go ahead an send a patch :-) This has been on my todo list
forever, I would happily like to remove it from there.

Kind regards
Uffe

Re: [PATCH v3 1/3] mmc: mmci: Support any block sizes for ux500v2 and qcom variant

[Bjorn Andersson]

On Thu, Aug 21, 2014 at 9:54 PM, Srinivas Kandagatla
<srinivas.kandagatla@linaro.org> wrote:
> From: Ulf Hansson <ulf.hansson@linaro.org>
>
> For the ux500v2 variant of the PL18x block, any block sizes are
> supported. This will make it possible to decrease data overhead
> for SDIO transfers.
>
> This patch is based on Ulf Hansson patch
> http://www.spinics.net/lists/linux-mmc/msg12160.html
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>         enabled this support on qcom variant.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Srinivas,

Pulled this in to my tree today and got the bcm4339 in the Sony Xperia
Z3 to join the internet :)
So, did you pursue this further?

Regards,
Bjorn

Re: [PATCH v3 1/3] mmc: mmci: Support any block sizes for ux500v2 and qcom variant

[Linus Walleij]

Sorry for top-posting, patch included for context.

I saw today that Brian has the Nexus 5 working with Broadcom/Cypress
4339 over SDIO on the MMCI.

Brian are you managing to use the WLAN without this patch or is this
something you stacked up to make it work? (If you want context I
can forward the rest of the conversation where we discuss how to
fix it properly.)

I discussed this patch several times with Ulf and I think Niklas looked
at it too, but we never got around to fix it up properly.

Yours,
Linus Walleij


On Fri, Aug 22, 2014 at 6:54 AM Srinivas Kandagatla
<srinivas.kandagatla@linaro.org> wrote:

> From: Ulf Hansson <ulf.hansson@linaro.org>
>
> For the ux500v2 variant of the PL18x block, any block sizes are
> supported. This will make it possible to decrease data overhead
> for SDIO transfers.
>
> This patch is based on Ulf Hansson patch
> http://www.spinics.net/lists/linux-mmc/msg12160.html
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>         enabled this support on qcom variant.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/mmc/host/mmci.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index c11cb05..533ad2b 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -77,6 +77,7 @@ static unsigned int fmax = 515633;
>   * @qcom_fifo: enables qcom specific fifo pio read logic.
>   * @reversed_irq_handling: handle data irq before cmd irq.
>   * @qcom_dml: enables qcom specific dma glue for dma transfers.
> + * @any_blksize: true if block any sizes are supported
>   */
>  struct variant_data {
>         unsigned int            clkreg;
> @@ -102,6 +103,7 @@ struct variant_data {
>         bool                    qcom_fifo;
>         bool                    reversed_irq_handling;
>         bool                    qcom_dml;
> +       bool                    any_blksize;
>  };
>
>  static struct variant_data variant_arm = {
> @@ -194,6 +196,7 @@ static struct variant_data variant_ux500v2 = {
>         .pwrreg_clkgate         = true,
>         .busy_detect            = true,
>         .pwrreg_nopower         = true,
> +       .any_blksize            = true,
>  };
>
>  static struct variant_data variant_qcom = {
> @@ -212,6 +215,7 @@ static struct variant_data variant_qcom = {
>         .explicit_mclk_control  = true,
>         .qcom_fifo              = true,
>         .qcom_dml               = true,
> +       .any_blksize            = true,
>  };
>
>  static int mmci_card_busy(struct mmc_host *mmc)
> @@ -239,10 +243,11 @@ static int mmci_card_busy(struct mmc_host *mmc)
>  static int mmci_validate_data(struct mmci_host *host,
>                               struct mmc_data *data)
>  {
> +       struct variant_data *variant = host->variant;
> +
>         if (!data)
>                 return 0;
> -
> -       if (!is_power_of_2(data->blksz)) {
> +       if (!is_power_of_2(data->blksz) && !variant->any_blksize) {
>                 dev_err(mmc_dev(host->mmc),
>                         "unsupported block size (%d bytes)\n", data->blksz);
>                 return -EINVAL;
> @@ -796,7 +801,6 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
>         writel(host->size, base + MMCIDATALENGTH);
>
>         blksz_bits = ffs(data->blksz) - 1;
> -       BUG_ON(1 << blksz_bits != data->blksz);
>
>         if (variant->blksz_datactrl16)
>                 datactrl = MCI_DPSM_ENABLE | (data->blksz << 16);
> --
> 1.9.1
>

Re: [PATCH v3 1/3] mmc: mmci: Support any block sizes for ux500v2 and qcom variant

[Bjorn Andersson]

On Tue 05 Mar 14:25 PST 2019, Linus Walleij wrote:

> Sorry for top-posting, patch included for context.
> 
> I saw today that Brian has the Nexus 5 working with Broadcom/Cypress
> 4339 over SDIO on the MMCI.
> 
> Brian are you managing to use the WLAN without this patch or is this
> something you stacked up to make it work? (If you want context I
> can forward the rest of the conversation where we discuss how to
> fix it properly.)
> 

Brian's Nexus 5 is on 8974 where the MMCI block has become the sdhci-msm
block, so while being a superset (afaict) of the old MMCI block we use
the sdhci driver instead.

> I discussed this patch several times with Ulf and I think Niklas looked
> at it too, but we never got around to fix it up properly.
> 

This is still needed for at least 8064 to have working sdio wifi (and
perhaps your 8060?).

Regards,
Bjorn

> Yours,
> Linus Walleij
> 
> 
> On Fri, Aug 22, 2014 at 6:54 AM Srinivas Kandagatla
> <srinivas.kandagatla@linaro.org> wrote:
> 
> > From: Ulf Hansson <ulf.hansson@linaro.org>
> >
> > For the ux500v2 variant of the PL18x block, any block sizes are
> > supported. This will make it possible to decrease data overhead
> > for SDIO transfers.
> >
> > This patch is based on Ulf Hansson patch
> > http://www.spinics.net/lists/linux-mmc/msg12160.html
> >
> > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> >         enabled this support on qcom variant.
> >
> > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> > ---
> >  drivers/mmc/host/mmci.c | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> > index c11cb05..533ad2b 100644
> > --- a/drivers/mmc/host/mmci.c
> > +++ b/drivers/mmc/host/mmci.c
> > @@ -77,6 +77,7 @@ static unsigned int fmax = 515633;
> >   * @qcom_fifo: enables qcom specific fifo pio read logic.
> >   * @reversed_irq_handling: handle data irq before cmd irq.
> >   * @qcom_dml: enables qcom specific dma glue for dma transfers.
> > + * @any_blksize: true if block any sizes are supported
> >   */
> >  struct variant_data {
> >         unsigned int            clkreg;
> > @@ -102,6 +103,7 @@ struct variant_data {
> >         bool                    qcom_fifo;
> >         bool                    reversed_irq_handling;
> >         bool                    qcom_dml;
> > +       bool                    any_blksize;
> >  };
> >
> >  static struct variant_data variant_arm = {
> > @@ -194,6 +196,7 @@ static struct variant_data variant_ux500v2 = {
> >         .pwrreg_clkgate         = true,
> >         .busy_detect            = true,
> >         .pwrreg_nopower         = true,
> > +       .any_blksize            = true,
> >  };
> >
> >  static struct variant_data variant_qcom = {
> > @@ -212,6 +215,7 @@ static struct variant_data variant_qcom = {
> >         .explicit_mclk_control  = true,
> >         .qcom_fifo              = true,
> >         .qcom_dml               = true,
> > +       .any_blksize            = true,
> >  };
> >
> >  static int mmci_card_busy(struct mmc_host *mmc)
> > @@ -239,10 +243,11 @@ static int mmci_card_busy(struct mmc_host *mmc)
> >  static int mmci_validate_data(struct mmci_host *host,
> >                               struct mmc_data *data)
> >  {
> > +       struct variant_data *variant = host->variant;
> > +
> >         if (!data)
> >                 return 0;
> > -
> > -       if (!is_power_of_2(data->blksz)) {
> > +       if (!is_power_of_2(data->blksz) && !variant->any_blksize) {
> >                 dev_err(mmc_dev(host->mmc),
> >                         "unsupported block size (%d bytes)\n", data->blksz);
> >                 return -EINVAL;
> > @@ -796,7 +801,6 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
> >         writel(host->size, base + MMCIDATALENGTH);
> >
> >         blksz_bits = ffs(data->blksz) - 1;
> > -       BUG_ON(1 << blksz_bits != data->blksz);
> >
> >         if (variant->blksz_datactrl16)
> >                 datactrl = MCI_DPSM_ENABLE | (data->blksz << 16);
> > --
> > 1.9.1
> >