[PATCH 0/3] mmc:

[PATCH 0/3] mmc:

[Kuninori Morimoto]

Hi Chris

These patches fixup R-Car SDHI / TMIO driver.

1st/2nd patch adds new flag to avoid 2 block read HW bug
which Renesas SDHI has.

3rd patch cares Tx/Rx dma_offset.
Because Renesas SDHI
has non-normal register mapping

Kuninori Morimoto (3):
      mmc: block: add block number limitation flag for lultiple block read
      mmc: sdhi: tidyup mmc_data setting for R-Car SDHI
      mmc: tmio: care about DMA tx/rx addr offset

 drivers/mmc/card/block.c          |   19 +++++++++++++++++--
 drivers/mmc/host/sh_mobile_sdhi.c |    5 ++++-
 drivers/mmc/host/tmio_mmc_dma.c   |    2 +-
 include/linux/mfd/tmio.h          |    1 +
 include/linux/mmc/host.h          |    3 +++
 5 files changed, 26 insertions(+), 4 deletions(-)

[PATCH 1/3] mmc: block: add block number limitation flag for lultiple block read

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

In some controllers, when performing a multiple block read of
one or two blocks, depending on the timing with which the
response register is read, the response value may not
be read properly.
Use single block read for this HW bug

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 drivers/mmc/card/block.c |   19 +++++++++++++++++--
 include/linux/mmc/host.h |    3 +++
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 452782b..f3cbe37 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -1400,8 +1400,23 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
 
 		/* Some controllers can't do multiblock reads due to hw bugs */
 		if (card->host->caps2 & MMC_CAP2_NO_MULTI_READ &&
-		    rq_data_dir(req) = READ)
-			brq->data.blocks = 1;
+		    rq_data_dir(req) = READ) {
+
+			if (card->host->caps2 & MMC_CAP2_2BLKS_LIMIT) {
+				/*
+				 * In some controllers, when performing a
+				 * multiple block read of one or two blocks,
+				 * depending on the timing with which the
+				 * response register is read, the response
+				 * value may not be read properly.
+				 * Use single block read for this HW bug
+				 */
+				if (brq->data.blocks = 2)
+					brq->data.blocks = 1;
+			} else {
+				brq->data.blocks = 1;
+			}
+		}
 	}
 
 	if (brq->data.blocks > 1 || do_rel_wr) {
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index cb61ea4..5429cd7 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -266,6 +266,9 @@ struct mmc_host {
 #define MMC_CAP2_BOOTPART_NOACC	(1 << 0)	/* Boot partition no access */
 #define MMC_CAP2_FULL_PWR_CYCLE	(1 << 2)	/* Can do full power cycle */
 #define MMC_CAP2_NO_MULTI_READ	(1 << 3)	/* Multiblock reads don't work */
+#define MMC_CAP2_2BLKS_LIMIT	(1 << 4)	/* 2 blocks limit for multi read */
+#define MMC_CAP2_NO_2BLKS_READ	(MMC_CAP2_NO_MULTI_READ | \
+				 MMC_CAP2_2BLKS_LIMIT)
 #define MMC_CAP2_HS200_1_8V_SDR	(1 << 5)        /* can support */
 #define MMC_CAP2_HS200_1_2V_SDR	(1 << 6)        /* can support */
 #define MMC_CAP2_HS200		(MMC_CAP2_HS200_1_8V_SDR | \
-- 
1.7.9.5

[PATCH 2/3] mmc: sdhi: tidyup mmc_data setting for R-Car SDHI

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

R-Car Gen2 SDHI has 2 block read HW bugs.
Current setting is using MMC_CAP2_NO_MULTI_READ,
but it is overkill.
We can use MMC_CAP2_NO_2BLKS_READ instead of it.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 drivers/mmc/host/sh_mobile_sdhi.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 91058da..b3baa83 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -55,7 +55,7 @@ static const struct sh_mobile_sdhi_of_data of_rcar_gen1_compatible = {
 static const struct sh_mobile_sdhi_of_data of_rcar_gen2_compatible = {
 	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_WRPROTECT_DISABLE,
 	.capabilities	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ,
-	.capabilities2	= MMC_CAP2_NO_MULTI_READ,
+	.capabilities2	= MMC_CAP2_NO_2BLKS_READ,
 };
 
 static const struct of_device_id sh_mobile_sdhi_of_match[] = {
-- 
1.7.9.5

[PATCH 3/3] mmc: tmio: care about DMA tx/rx addr offset

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Basically, SD_BUF0 Tx/Rx addresses are same
in normal TMIO controller,
but, it is different on Renesas R-Car SDHI controller
if it uses DMAC
(Rx address needs to add 0x2000 to Tx address)

This patch adds new .dma_rx_offset and cares it

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 drivers/mmc/host/sh_mobile_sdhi.c |    3 +++
 drivers/mmc/host/tmio_mmc_dma.c   |    2 +-
 include/linux/mfd/tmio.h          |    1 +
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index b3baa83..91c6399 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -39,6 +39,7 @@ struct sh_mobile_sdhi_of_data {
 	unsigned long tmio_flags;
 	unsigned long capabilities;
 	unsigned long capabilities2;
+	dma_addr_t dma_rx_offset;
 };
 
 static const struct sh_mobile_sdhi_of_data sh_mobile_sdhi_of_cfg[] = {
@@ -56,6 +57,7 @@ static const struct sh_mobile_sdhi_of_data of_rcar_gen2_compatible = {
 	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_WRPROTECT_DISABLE,
 	.capabilities	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ,
 	.capabilities2	= MMC_CAP2_NO_2BLKS_READ,
+	.dma_rx_offset	= 0x2000,
 };
 
 static const struct of_device_id sh_mobile_sdhi_of_match[] = {
@@ -228,6 +230,7 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 		mmc_data->flags |= of_data->tmio_flags;
 		mmc_data->capabilities |= of_data->capabilities;
 		mmc_data->capabilities2 |= of_data->capabilities2;
+		dma_priv->dma_rx_offset = of_data->dma_rx_offset;
 	}
 
 	/* SD control register space size is 0x100, 0x200 for bus_shift=1 */
diff --git a/drivers/mmc/host/tmio_mmc_dma.c b/drivers/mmc/host/tmio_mmc_dma.c
index 03e7b28..1925170 100644
--- a/drivers/mmc/host/tmio_mmc_dma.c
+++ b/drivers/mmc/host/tmio_mmc_dma.c
@@ -311,7 +311,7 @@ void tmio_mmc_request_dma(struct tmio_mmc_host *host, struct tmio_mmc_data *pdat
 		if (pdata->dma->chan_priv_rx)
 			cfg.slave_id = pdata->dma->slave_id_rx;
 		cfg.direction = DMA_DEV_TO_MEM;
-		cfg.src_addr = cfg.dst_addr;
+		cfg.src_addr = cfg.dst_addr + pdata->dma->dma_rx_offset;
 		cfg.dst_addr = 0;
 		ret = dmaengine_slave_config(host->chan_rx, &cfg);
 		if (ret < 0)
diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
index 8f6f2e9..777e29b 100644
--- a/include/linux/mfd/tmio.h
+++ b/include/linux/mfd/tmio.h
@@ -96,6 +96,7 @@ struct tmio_mmc_dma {
 	int slave_id_tx;
 	int slave_id_rx;
 	int alignment_shift;
+	dma_addr_t dma_rx_offset;
 	bool (*filter)(struct dma_chan *chan, void *arg);
 };
 
-- 
1.7.9.5

Re: [PATCH 3/3] mmc: tmio: care about DMA tx/rx addr offset

[Ben Dooks]

On Fri, May 30, 2014 at 03:40:56AM -0700, Kuninori Morimoto wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> Basically, SD_BUF0 Tx/Rx addresses are same
> in normal TMIO controller,
> but, it is different on Renesas R-Car SDHI controller
> if it uses DMAC
> (Rx address needs to add 0x2000 to Tx address)
> 
> This patch adds new .dma_rx_offset and cares it
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>  drivers/mmc/host/sh_mobile_sdhi.c |    3 +++
>  drivers/mmc/host/tmio_mmc_dma.c   |    2 +-
>  include/linux/mfd/tmio.h          |    1 +
>  3 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
> index b3baa83..91c6399 100644
> --- a/drivers/mmc/host/sh_mobile_sdhi.c
> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> @@ -39,6 +39,7 @@ struct sh_mobile_sdhi_of_data {
>  	unsigned long tmio_flags;
>  	unsigned long capabilities;
>  	unsigned long capabilities2;
> +	dma_addr_t dma_rx_offset;
>  };
>  
>  static const struct sh_mobile_sdhi_of_data sh_mobile_sdhi_of_cfg[] = {
> @@ -56,6 +57,7 @@ static const struct sh_mobile_sdhi_of_data of_rcar_gen2_compatible = {
>  	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_WRPROTECT_DISABLE,
>  	.capabilities	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ,
>  	.capabilities2	= MMC_CAP2_NO_2BLKS_READ,
> +	.dma_rx_offset	= 0x2000,
>  };
>  
>  static const struct of_device_id sh_mobile_sdhi_of_match[] = {
> @@ -228,6 +230,7 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
>  		mmc_data->flags |= of_data->tmio_flags;
>  		mmc_data->capabilities |= of_data->capabilities;
>  		mmc_data->capabilities2 |= of_data->capabilities2;
> +		dma_priv->dma_rx_offset = of_data->dma_rx_offset;
>  	}
>  
>  	/* SD control register space size is 0x100, 0x200 for bus_shift=1 */
> diff --git a/drivers/mmc/host/tmio_mmc_dma.c b/drivers/mmc/host/tmio_mmc_dma.c
> index 03e7b28..1925170 100644
> --- a/drivers/mmc/host/tmio_mmc_dma.c
> +++ b/drivers/mmc/host/tmio_mmc_dma.c
> @@ -311,7 +311,7 @@ void tmio_mmc_request_dma(struct tmio_mmc_host *host, struct tmio_mmc_data *pdat
>  		if (pdata->dma->chan_priv_rx)
>  			cfg.slave_id = pdata->dma->slave_id_rx;
>  		cfg.direction = DMA_DEV_TO_MEM;
> -		cfg.src_addr = cfg.dst_addr;
> +		cfg.src_addr = cfg.dst_addr + pdata->dma->dma_rx_offset;
>  		cfg.dst_addr = 0;
>  		ret = dmaengine_slave_config(host->chan_rx, &cfg);
>  		if (ret < 0)

Ok, this is a useful fix. We did it by adding an option to specify
the DMA address in the DT.

Acked-by: Ben Dooks <ben-linux@fluff.org>


-- 
Ben Dooks, ben@fluff.org, http://www.fluff.org/ben/

Large Hadron Colada: A large Pina Colada that makes the universe disappear.

Re: [PATCH 1/3] mmc: block: add block number limitation flag for lultiple block read

[Jaehoon Chung]

Hi, Kuninori.

Subject has the typo. lultiple -> multiple?
I didn't know why you need this patch. Would you use the MMC_CAP2_NO_MULTI_READ?
And I think...it's not good that used the card's capability flags to fix your H/W bug.

Best Regards,
Jaehoon Chung

On 05/30/2014 07:40 PM, Kuninori Morimoto wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> In some controllers, when performing a multiple block read of
> one or two blocks, depending on the timing with which the
> response register is read, the response value may not
> be read properly.
> Use single block read for this HW bug
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>  drivers/mmc/card/block.c |   19 +++++++++++++++++--
>  include/linux/mmc/host.h |    3 +++
>  2 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index 452782b..f3cbe37 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -1400,8 +1400,23 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
>  
>  		/* Some controllers can't do multiblock reads due to hw bugs */
>  		if (card->host->caps2 & MMC_CAP2_NO_MULTI_READ &&
> -		    rq_data_dir(req) = READ)
> -			brq->data.blocks = 1;
> +		    rq_data_dir(req) = READ) {
> +
> +			if (card->host->caps2 & MMC_CAP2_2BLKS_LIMIT) {
> +				/*
> +				 * In some controllers, when performing a
> +				 * multiple block read of one or two blocks,
> +				 * depending on the timing with which the
> +				 * response register is read, the response
> +				 * value may not be read properly.
> +				 * Use single block read for this HW bug
> +				 */
> +				if (brq->data.blocks = 2)
> +					brq->data.blocks = 1;
> +			} else {
> +				brq->data.blocks = 1;
> +			}
> +		}
>  	}
>  
>  	if (brq->data.blocks > 1 || do_rel_wr) {
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index cb61ea4..5429cd7 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -266,6 +266,9 @@ struct mmc_host {
>  #define MMC_CAP2_BOOTPART_NOACC	(1 << 0)	/* Boot partition no access */
>  #define MMC_CAP2_FULL_PWR_CYCLE	(1 << 2)	/* Can do full power cycle */
>  #define MMC_CAP2_NO_MULTI_READ	(1 << 3)	/* Multiblock reads don't work */
> +#define MMC_CAP2_2BLKS_LIMIT	(1 << 4)	/* 2 blocks limit for multi read */
> +#define MMC_CAP2_NO_2BLKS_READ	(MMC_CAP2_NO_MULTI_READ | \
> +				 MMC_CAP2_2BLKS_LIMIT)
>  #define MMC_CAP2_HS200_1_8V_SDR	(1 << 5)        /* can support */
>  #define MMC_CAP2_HS200_1_2V_SDR	(1 << 6)        /* can support */
>  #define MMC_CAP2_HS200		(MMC_CAP2_HS200_1_8V_SDR | \
> 

Re: [PATCH 1/3] mmc: block: add block number limitation flag for lultiple block read

[Kuninori Morimoto]

Hi Jaehoon

> Subject has the typo. lultiple -> multiple?

grr..

> I didn't know why you need this patch. Would you use the MMC_CAP2_NO_MULTI_READ?
> And I think...it's not good that used the card's capability flags to fix your H/W bug.

Actually, current platform is using MMC_CAP2_NO_MULTI_READ at this point.
But, our HW bug happens when block size was 2.
Other block size is no problem.

I want to solve this issue somehow.
I'm not sure this is good method or not,
but, can I exchange block size under
mmc_host_ops :: request ?

Best regards
---
Kuninori Morimoto

Re: [PATCH 1/3] mmc: block: add block number limitation flag for lultiple block read

[Ben Dooks]

On Fri, May 30, 2014 at 08:00:17PM +0900, Jaehoon Chung wrote:
> Hi, Kuninori.
> 
> Subject has the typo. lultiple -> multiple?
> I didn't know why you need this patch. Would you use the MMC_CAP2_NO_MULTI_READ?
> And I think...it's not good that used the card's capability flags to fix your H/W bug.

If you do that, then there is around a 8-10x performance penalty.

-- 
Ben Dooks, ben@fluff.org, http://www.fluff.org/ben/

Large Hadron Colada: A large Pina Colada that makes the universe disappear.

Re: [PATCH 1/3] mmc: block: add block number limitation flag for lultiple block read

[Sergei Shtylyov]

On 05/30/2014 02:40 PM, Kuninori Morimoto wrote:

> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

> In some controllers, when performing a multiple block read of

    You've typoed in the subject: "lultiple".

> one or two blocks, depending on the timing with which the
> response register is read, the response value may not
> be read properly.
> Use single block read for this HW bug

> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>   drivers/mmc/card/block.c |   19 +++++++++++++++++--
>   include/linux/mmc/host.h |    3 +++
>   2 files changed, 20 insertions(+), 2 deletions(-)

> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index 452782b..f3cbe37 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -1400,8 +1400,23 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
>
>   		/* Some controllers can't do multiblock reads due to hw bugs */
>   		if (card->host->caps2 & MMC_CAP2_NO_MULTI_READ &&
> -		    rq_data_dir(req) = READ)
> -			brq->data.blocks = 1;
> +		    rq_data_dir(req) = READ) {
> +
> +			if (card->host->caps2 & MMC_CAP2_2BLKS_LIMIT) {
> +				/*
> +				 * In some controllers, when performing a
> +				 * multiple block read of one or two blocks,
> +				 * depending on the timing with which the
> +				 * response register is read, the response
> +				 * value may not be read properly.
> +				 * Use single block read for this HW bug
> +				 */
> +				if (brq->data.blocks = 2)
> +					brq->data.blocks = 1;

    I don't understand: previous code set 'brq->data.blocks' to 1 in any case 
without your extra flag. Why there's a need to set it specifically for 2 
blocks (and not set for a larger # of blocks)? This looks like an optimization 
of some sort, not a workaround?..

> +			} else {
> +				brq->data.blocks = 1;
> +			}
> +		}
>   	}
>
>   	if (brq->data.blocks > 1 || do_rel_wr) {

WBR, Sergei

Re: [PATCH 1/3] mmc: block: add block number limitation flag for lultiple block read

[Kuninori Morimoto]

Hi Sergei

> > +			if (card->host->caps2 & MMC_CAP2_2BLKS_LIMIT) {
> > +				/*
> > +				 * In some controllers, when performing a
> > +				 * multiple block read of one or two blocks,
> > +				 * depending on the timing with which the
> > +				 * response register is read, the response
> > +				 * value may not be read properly.
> > +				 * Use single block read for this HW bug
> > +				 */
> > +				if (brq->data.blocks = 2)
> > +					brq->data.blocks = 1;
> 
>     I don't understand: previous code set 'brq->data.blocks' to 1 in any case 
> without your extra flag. Why there's a need to set it specifically for 2 
> blocks (and not set for a larger # of blocks)? This looks like an optimization 
> of some sort, not a workaround?..

Basically, we want to use multi block read.
Otherwise, the speed will be super slow
if it always use single block.
(Previous code force to single block)

But we need to switch to single block mode
if brq->data.blocks = 2.
Please read the above comment for detail.