[PATCH v2] mmc: dw_mmc: read all data in FIFO after Data transfer over interrupt in pio mode

[PATCH v2] mmc: dw_mmc: read all data in FIFO after Data transfer over interrupt in pio mode

[Kyoungil Kim]

In dwc manual, the below contents are described.
"During end of packet, interrupt is not generated if threshold
programming is larger than any remaining data. It is responsibility
of host to read remaining bytes on seeing Data Transfer Done
interrupt"
We also have seen the data cannot be read fully
when "sg_miter->length" is less than FIFO size.

Signed-off-by: Kyoungil Kim <ki0351.kim@samsung.com>
Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
---
 drivers/mmc/host/dw_mmc.c |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 323c502..064c010 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1430,7 +1430,7 @@ static void dw_mci_pull_data(struct dw_mci *host, void *buf, int cnt)
 	host->pull_data(host, buf, cnt);
 }
 
-static void dw_mci_read_data_pio(struct dw_mci *host)
+static void dw_mci_read_data_pio(struct dw_mci *host, bool int_data_over)
 {
 	struct sg_mapping_iter *sg_miter = &host->sg_miter;
 	void *buf;
@@ -1465,7 +1465,10 @@ static void dw_mci_read_data_pio(struct dw_mci *host)
 		sg_miter->consumed = offset;
 		status = mci_readl(host, MINTSTS);
 		mci_writel(host, RINTSTS, SDMMC_INT_RXDR);
-	} while (status & SDMMC_INT_RXDR); /*if the RXDR is ready read again*/
+	/* if the RXDR is ready read again */
+	} while ((status & SDMMC_INT_RXDR) ||
+			(int_data_over &&
+			 SDMMC_GET_FCNT(mci_readl(host, STATUS))));
 	data->bytes_xfered += nbytes;
 
 	if (!remain) {
@@ -1597,7 +1600,7 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
 			smp_wmb();
 			if (host->dir_status == DW_MCI_RECV_STATUS) {
 				if (host->sg != NULL)
-					dw_mci_read_data_pio(host);
+					dw_mci_read_data_pio(host, 1);
 			}
 			set_bit(EVENT_DATA_COMPLETE, &host->pending_events);
 			tasklet_schedule(&host->tasklet);
@@ -1606,7 +1609,7 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
 		if (pending & SDMMC_INT_RXDR) {
 			mci_writel(host, RINTSTS, SDMMC_INT_RXDR);
 			if (host->dir_status == DW_MCI_RECV_STATUS && host->sg)
-				dw_mci_read_data_pio(host);
+				dw_mci_read_data_pio(host, 0);
 		}
 
 		if (pending & SDMMC_INT_TXDR) {
-- 
1.7.4.1

Re: [PATCH v2] mmc: dw_mmc: read all data in FIFO after Data transfer over interrupt in pio mode

[Jaehoon Chung]

Hi, 

i didn't test with this patch. But it makes sense.
Just i have minor comment.

Acked-by: Jaehoon Chung <jh80.chung@samsung.com>

On 01/21/2013 09:28 PM, Kyoungil Kim wrote:
> In dwc manual, the below contents are described.
> "During end of packet, interrupt is not generated if threshold
> programming is larger than any remaining data. It is responsibility
> of host to read remaining bytes on seeing Data Transfer Done
> interrupt"
> We also have seen the data cannot be read fully
> when "sg_miter->length" is less than FIFO size.
> 
> Signed-off-by: Kyoungil Kim <ki0351.kim@samsung.com>
> Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
> ---
>  drivers/mmc/host/dw_mmc.c |   11 +++++++----
>  1 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 323c502..064c010 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1430,7 +1430,7 @@ static void dw_mci_pull_data(struct dw_mci *host, void *buf, int cnt)
>  	host->pull_data(host, buf, cnt);
>  }
>  
> -static void dw_mci_read_data_pio(struct dw_mci *host)
> +static void dw_mci_read_data_pio(struct dw_mci *host, bool int_data_over)
>  {
>  	struct sg_mapping_iter *sg_miter = &host->sg_miter;
>  	void *buf;
> @@ -1465,7 +1465,10 @@ static void dw_mci_read_data_pio(struct dw_mci *host)
>  		sg_miter->consumed = offset;
>  		status = mci_readl(host, MINTSTS);
>  		mci_writel(host, RINTSTS, SDMMC_INT_RXDR);
> -	} while (status & SDMMC_INT_RXDR); /*if the RXDR is ready read again*/
> +	/* if the RXDR is ready read again */
> +	} while ((status & SDMMC_INT_RXDR) ||
> +			(int_data_over &&
> +			 SDMMC_GET_FCNT(mci_readl(host, STATUS))));
If you can change int_data_over, we can use the one line.

Best Regards,
Jaehoon Chung
>  	data->bytes_xfered += nbytes;
>  
>  	if (!remain) {
> @@ -1597,7 +1600,7 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
>  			smp_wmb();
>  			if (host->dir_status == DW_MCI_RECV_STATUS) {
>  				if (host->sg != NULL)
> -					dw_mci_read_data_pio(host);
> +					dw_mci_read_data_pio(host, 1);
>  			}
>  			set_bit(EVENT_DATA_COMPLETE, &host->pending_events);
>  			tasklet_schedule(&host->tasklet);
> @@ -1606,7 +1609,7 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
>  		if (pending & SDMMC_INT_RXDR) {
>  			mci_writel(host, RINTSTS, SDMMC_INT_RXDR);
>  			if (host->dir_status == DW_MCI_RECV_STATUS && host->sg)
> -				dw_mci_read_data_pio(host);
> +				dw_mci_read_data_pio(host, 0);
>  		}
>  
>  		if (pending & SDMMC_INT_TXDR) {
> 

RE: [PATCH v2] mmc: dw_mmc: read all data in FIFO after Data transfer over interrupt in pio mode

[Seungwon Jeon]

Kyoungil,

Could you resend the following change considering Jaehoon's comment?
- int_data_over -> dto
- use boolean constants(true, false) for dw_mci_read_data_pio argument.

Thanks,
Seungwon Jeon

On Tuesday, January 22, 2013, Jaehoon Chung wrote:
> Hi,
> 
> i didn't test with this patch. But it makes sense.
> Just i have minor comment.
> 
> Acked-by: Jaehoon Chung <jh80.chung@samsung.com>
> 
> On 01/21/2013 09:28 PM, Kyoungil Kim wrote:
> > In dwc manual, the below contents are described.
> > "During end of packet, interrupt is not generated if threshold
> > programming is larger than any remaining data. It is responsibility
> > of host to read remaining bytes on seeing Data Transfer Done
> > interrupt"
> > We also have seen the data cannot be read fully
> > when "sg_miter->length" is less than FIFO size.
> >
> > Signed-off-by: Kyoungil Kim <ki0351.kim@samsung.com>
> > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
> > ---
> >  drivers/mmc/host/dw_mmc.c |   11 +++++++----
> >  1 files changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> > index 323c502..064c010 100644
> > --- a/drivers/mmc/host/dw_mmc.c
> > +++ b/drivers/mmc/host/dw_mmc.c
> > @@ -1430,7 +1430,7 @@ static void dw_mci_pull_data(struct dw_mci *host, void *buf, int cnt)
> >  	host->pull_data(host, buf, cnt);
> >  }
> >
> > -static void dw_mci_read_data_pio(struct dw_mci *host)
> > +static void dw_mci_read_data_pio(struct dw_mci *host, bool int_data_over)
> >  {
> >  	struct sg_mapping_iter *sg_miter = &host->sg_miter;
> >  	void *buf;
> > @@ -1465,7 +1465,10 @@ static void dw_mci_read_data_pio(struct dw_mci *host)
> >  		sg_miter->consumed = offset;
> >  		status = mci_readl(host, MINTSTS);
> >  		mci_writel(host, RINTSTS, SDMMC_INT_RXDR);
> > -	} while (status & SDMMC_INT_RXDR); /*if the RXDR is ready read again*/
> > +	/* if the RXDR is ready read again */
> > +	} while ((status & SDMMC_INT_RXDR) ||
> > +			(int_data_over &&
> > +			 SDMMC_GET_FCNT(mci_readl(host, STATUS))));
> If you can change int_data_over, we can use the one line.
> 
> Best Regards,
> Jaehoon Chung
> >  	data->bytes_xfered += nbytes;
> >
> >  	if (!remain) {
> > @@ -1597,7 +1600,7 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
> >  			smp_wmb();
> >  			if (host->dir_status == DW_MCI_RECV_STATUS) {
> >  				if (host->sg != NULL)
> > -					dw_mci_read_data_pio(host);
> > +					dw_mci_read_data_pio(host, 1);
> >  			}
> >  			set_bit(EVENT_DATA_COMPLETE, &host->pending_events);
> >  			tasklet_schedule(&host->tasklet);
> > @@ -1606,7 +1609,7 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
> >  		if (pending & SDMMC_INT_RXDR) {
> >  			mci_writel(host, RINTSTS, SDMMC_INT_RXDR);
> >  			if (host->dir_status == DW_MCI_RECV_STATUS && host->sg)
> > -				dw_mci_read_data_pio(host);
> > +				dw_mci_read_data_pio(host, 0);
> >  		}
> >
> >  		if (pending & SDMMC_INT_TXDR) {
> >
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v2] mmc: dw_mmc: read all data in FIFO after Data transfer over interrupt in pio mode

[Kyoungil Kim]

Thank you for review.
I'll apply.

On Tuesday, January 22, 2013 3:08 PM, Seungwon Jeon wrote:
> Kyoungil,
> 
> Could you resend the following change considering Jaehoon's comment?
> - int_data_over -> dto
> - use boolean constants(true, false) for dw_mci_read_data_pio argument.
> 
> Thanks,
> Seungwon Jeon
> 
> On Tuesday, January 22, 2013, Jaehoon Chung wrote:
> > Hi,
> >
> > i didn't test with this patch. But it makes sense.
> > Just i have minor comment.
> >
> > Acked-by: Jaehoon Chung <jh80.chung@samsung.com>
> >
> > On 01/21/2013 09:28 PM, Kyoungil Kim wrote:
> > > In dwc manual, the below contents are described.
> > > "During end of packet, interrupt is not generated if threshold
> > > programming is larger than any remaining data. It is responsibility
> > > of host to read remaining bytes on seeing Data Transfer Done
> > > interrupt"
> > > We also have seen the data cannot be read fully
> > > when "sg_miter->length" is less than FIFO size.
> > >
> > > Signed-off-by: Kyoungil Kim <ki0351.kim@samsung.com>
> > > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
> > > ---
> > >  drivers/mmc/host/dw_mmc.c |   11 +++++++----
> > >  1 files changed, 7 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> > > index 323c502..064c010 100644
> > > --- a/drivers/mmc/host/dw_mmc.c
> > > +++ b/drivers/mmc/host/dw_mmc.c
> > > @@ -1430,7 +1430,7 @@ static void dw_mci_pull_data(struct dw_mci *host, void *buf, int cnt)
> > >  	host->pull_data(host, buf, cnt);
> > >  }
> > >
> > > -static void dw_mci_read_data_pio(struct dw_mci *host)
> > > +static void dw_mci_read_data_pio(struct dw_mci *host, bool int_data_over)
> > >  {
> > >  	struct sg_mapping_iter *sg_miter = &host->sg_miter;
> > >  	void *buf;
> > > @@ -1465,7 +1465,10 @@ static void dw_mci_read_data_pio(struct dw_mci *host)
> > >  		sg_miter->consumed = offset;
> > >  		status = mci_readl(host, MINTSTS);
> > >  		mci_writel(host, RINTSTS, SDMMC_INT_RXDR);
> > > -	} while (status & SDMMC_INT_RXDR); /*if the RXDR is ready read again*/
> > > +	/* if the RXDR is ready read again */
> > > +	} while ((status & SDMMC_INT_RXDR) ||
> > > +			(int_data_over &&
> > > +			 SDMMC_GET_FCNT(mci_readl(host, STATUS))));
> > If you can change int_data_over, we can use the one line.
> >
> > Best Regards,
> > Jaehoon Chung