diff mbox

mmc: dw_mmc: fix card read threshold for PIO mode

Message ID 1416551950-8277-1-git-send-email-shawn.guo@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Shawn Guo Nov. 21, 2014, 6:39 a.m. UTC
Commit f1d2736c8156 ("mmc: dw_mmc: control card read threshold")
introduces a regression for use case where PIO mode is used, i.e.
CONFIG_MMC_DW_IDMAC is not disabled.  It stops kernel from booting
like below.

dw_mmc 9408000.dwmmc: fifo-depth property not found, using value of FIFOTH register as default
dw_mmc 9408000.dwmmc: Using PIO mode.
dw_mmc 9408000.dwmmc: Version ID is 210a
dw_mmc 9408000.dwmmc: DW MMC controller at irq 72, 32 bit host data width, 32 deep fifo
dw_mmc 9408000.dwmmc: 1 slots initialized
...
Waiting for root device /dev/mmcblk0p2...
mmc_host mmc0: Bus speed (slot 0) = 52000000Hz (slot req 50000000Hz, actual 26000000HZ div = 1)
mmc0: new high speed SDHC card at address b368
mmcblk0: mmc0:b368 AF UD 3.84 GiB

Rather than clearing read threshold bits, the best thing that function
dw_mci_ctrl_rd_thld() should do is leaving the bits untouched, in case
that card read threshold setup is not needed.

The patch fixes the regression by changing dw_mci_ctrl_rd_thld() a bit
to do nothing in case card read threshold bits setup is not needed.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
Fixes: f1d2736c8156 ("mmc: dw_mmc: control card read threshold")
Cc: <stable@vger.kernel.org> # 3.13+
---
 drivers/mmc/host/dw_mmc.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

Comments

Jaehoon Chung Nov. 21, 2014, 10:30 a.m. UTC | #1
Hi, Shawn.

On 11/21/2014 03:39 PM, Shawn Guo wrote:
> Commit f1d2736c8156 ("mmc: dw_mmc: control card read threshold")
> introduces a regression for use case where PIO mode is used, i.e.
> CONFIG_MMC_DW_IDMAC is not disabled.  It stops kernel from booting
> like below.
> 
> dw_mmc 9408000.dwmmc: fifo-depth property not found, using value of FIFOTH register as default
> dw_mmc 9408000.dwmmc: Using PIO mode.
> dw_mmc 9408000.dwmmc: Version ID is 210a
> dw_mmc 9408000.dwmmc: DW MMC controller at irq 72, 32 bit host data width, 32 deep fifo
> dw_mmc 9408000.dwmmc: 1 slots initialized
> ...
> Waiting for root device /dev/mmcblk0p2...
> mmc_host mmc0: Bus speed (slot 0) = 52000000Hz (slot req 50000000Hz, actual 26000000HZ div = 1)
> mmc0: new high speed SDHC card at address b368
> mmcblk0: mmc0:b368 AF UD 3.84 GiB

Stopping at here? I didn't know what's difference before applied your patch.
Clearing Card-read threshold value at every time would be caused that stop kernel?
Just wonder it.

> 
> Rather than clearing read threshold bits, the best thing that function
> dw_mci_ctrl_rd_thld() should do is leaving the bits untouched, in case
> that card read threshold setup is not needed.
> 
> The patch fixes the regression by changing dw_mci_ctrl_rd_thld() a bit
> to do nothing in case card read threshold bits setup is not needed.
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> Fixes: f1d2736c8156 ("mmc: dw_mmc: control card read threshold")
> Cc: <stable@vger.kernel.org> # 3.13+
> ---
>  drivers/mmc/host/dw_mmc.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 69f0cc68d5b2..52c04ba69970 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -628,13 +628,13 @@ static void dw_mci_ctrl_rd_thld(struct dw_mci *host, struct mmc_data *data)
>  
>  	if (host->timing != MMC_TIMING_MMC_HS200 &&
>  	    host->timing != MMC_TIMING_UHS_SDR104)
> -		goto disable;
> +		return;
>  
>  	blksz_depth = blksz / (1 << host->data_shift);
>  	fifo_depth = host->fifo_depth;
>  
>  	if (blksz_depth > fifo_depth)
> -		goto disable;
> +		return;
>  
>  	/*
>  	 * If (blksz_depth) >= (fifo_depth >> 1), should be 'thld_size <= blksz'
> @@ -644,9 +644,6 @@ static void dw_mci_ctrl_rd_thld(struct dw_mci *host, struct mmc_data *data)
>  	thld_size = blksz;
>  	mci_writel(host, CDTHRCTL, SDMMC_SET_RD_THLD(thld_size, 1));
>  	return;

Then "return" could be also removed.

Best Regards,
Jaehoon Chung

> -
> -disable:
> -	mci_writel(host, CDTHRCTL, SDMMC_SET_RD_THLD(0, 0));
>  }
>  
>  static int dw_mci_submit_data_dma(struct dw_mci *host, struct mmc_data *data)
> 

--
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
Shawn Guo Nov. 21, 2014, 11:48 a.m. UTC | #2
Hi Jaehoon,

On Fri, Nov 21, 2014 at 07:30:50PM +0900, Jaehoon Chung wrote:
> Hi, Shawn.
> 
> On 11/21/2014 03:39 PM, Shawn Guo wrote:
> > Commit f1d2736c8156 ("mmc: dw_mmc: control card read threshold")
> > introduces a regression for use case where PIO mode is used, i.e.
> > CONFIG_MMC_DW_IDMAC is not disabled.  It stops kernel from booting
> > like below.
> > 
> > dw_mmc 9408000.dwmmc: fifo-depth property not found, using value of FIFOTH register as default
> > dw_mmc 9408000.dwmmc: Using PIO mode.
> > dw_mmc 9408000.dwmmc: Version ID is 210a
> > dw_mmc 9408000.dwmmc: DW MMC controller at irq 72, 32 bit host data width, 32 deep fifo
> > dw_mmc 9408000.dwmmc: 1 slots initialized
> > ...
> > Waiting for root device /dev/mmcblk0p2...
> > mmc_host mmc0: Bus speed (slot 0) = 52000000Hz (slot req 50000000Hz, actual 26000000HZ div = 1)
> > mmc0: new high speed SDHC card at address b368
> > mmcblk0: mmc0:b368 AF UD 3.84 GiB
> 
> Stopping at here? I didn't know what's difference before applied your patch.
> Clearing Card-read threshold value at every time would be caused that stop kernel?
> Just wonder it.

With my patch, the kernel will continue to mount the rootfs on SD card
as below.

mmcblk0: mmc0:b368 AF UD 3.84 GiB 
 mmcblk0: p1 p2
EXT4-fs (mmcblk0p2): couldn't mount as ext3 due to feature incompatibilities
EXT4-fs (mmcblk0p2): couldn't mount as ext2 due to feature incompatibilities
EXT4-fs (mmcblk0p2): INFO: recovery required on readonly filesystem
EXT4-fs (mmcblk0p2): write access will be enabled during recovery
random: nonblocking pool is initialized
EXT4-fs (mmcblk0p2): recovery complete
EXT4-fs (mmcblk0p2): mounted filesystem with ordered data mode. Opts: (null)
VFS: Mounted root (ext4 filesystem) readonly on device 179:2.

> 
> > 
> > Rather than clearing read threshold bits, the best thing that function
> > dw_mci_ctrl_rd_thld() should do is leaving the bits untouched, in case
> > that card read threshold setup is not needed.
> > 
> > The patch fixes the regression by changing dw_mci_ctrl_rd_thld() a bit
> > to do nothing in case card read threshold bits setup is not needed.
> > 
> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > Fixes: f1d2736c8156 ("mmc: dw_mmc: control card read threshold")
> > Cc: <stable@vger.kernel.org> # 3.13+
> > ---
> >  drivers/mmc/host/dw_mmc.c | 7 ++-----
> >  1 file changed, 2 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> > index 69f0cc68d5b2..52c04ba69970 100644
> > --- a/drivers/mmc/host/dw_mmc.c
> > +++ b/drivers/mmc/host/dw_mmc.c
> > @@ -628,13 +628,13 @@ static void dw_mci_ctrl_rd_thld(struct dw_mci *host, struct mmc_data *data)
> >  
> >  	if (host->timing != MMC_TIMING_MMC_HS200 &&
> >  	    host->timing != MMC_TIMING_UHS_SDR104)
> > -		goto disable;
> > +		return;
> >  
> >  	blksz_depth = blksz / (1 << host->data_shift);
> >  	fifo_depth = host->fifo_depth;
> >  
> >  	if (blksz_depth > fifo_depth)
> > -		goto disable;
> > +		return;
> >  
> >  	/*
> >  	 * If (blksz_depth) >= (fifo_depth >> 1), should be 'thld_size <= blksz'
> > @@ -644,9 +644,6 @@ static void dw_mci_ctrl_rd_thld(struct dw_mci *host, struct mmc_data *data)
> >  	thld_size = blksz;
> >  	mci_writel(host, CDTHRCTL, SDMMC_SET_RD_THLD(thld_size, 1));
> >  	return;
> 
> Then "return" could be also removed.

You're right.  Will remove it in v2.

Shawn
--
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
Shawn Guo Nov. 21, 2014, 1:03 p.m. UTC | #3
On Fri, Nov 21, 2014 at 02:39:10PM +0800, Shawn Guo wrote:
> Commit f1d2736c8156 ("mmc: dw_mmc: control card read threshold")
> introduces a regression for use case where PIO mode is used, i.e.
> CONFIG_MMC_DW_IDMAC is not disabled.  It stops kernel from booting

s/disabled/enabled

> like below.

To make it clear, I will reword the sentence as below.

It makes kernel fail to detect the partitions on SD/MMC device where
rootfs is located.

Shawn

> 
> dw_mmc 9408000.dwmmc: fifo-depth property not found, using value of FIFOTH register as default
> dw_mmc 9408000.dwmmc: Using PIO mode.
> dw_mmc 9408000.dwmmc: Version ID is 210a
> dw_mmc 9408000.dwmmc: DW MMC controller at irq 72, 32 bit host data width, 32 deep fifo
> dw_mmc 9408000.dwmmc: 1 slots initialized
> ...
> Waiting for root device /dev/mmcblk0p2...
> mmc_host mmc0: Bus speed (slot 0) = 52000000Hz (slot req 50000000Hz, actual 26000000HZ div = 1)
> mmc0: new high speed SDHC card at address b368
> mmcblk0: mmc0:b368 AF UD 3.84 GiB
> 
> Rather than clearing read threshold bits, the best thing that function
> dw_mci_ctrl_rd_thld() should do is leaving the bits untouched, in case
> that card read threshold setup is not needed.
> 
> The patch fixes the regression by changing dw_mci_ctrl_rd_thld() a bit
> to do nothing in case card read threshold bits setup is not needed.
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> Fixes: f1d2736c8156 ("mmc: dw_mmc: control card read threshold")
> Cc: <stable@vger.kernel.org> # 3.13+
> ---
>  drivers/mmc/host/dw_mmc.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 69f0cc68d5b2..52c04ba69970 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -628,13 +628,13 @@ static void dw_mci_ctrl_rd_thld(struct dw_mci *host, struct mmc_data *data)
>  
>  	if (host->timing != MMC_TIMING_MMC_HS200 &&
>  	    host->timing != MMC_TIMING_UHS_SDR104)
> -		goto disable;
> +		return;
>  
>  	blksz_depth = blksz / (1 << host->data_shift);
>  	fifo_depth = host->fifo_depth;
>  
>  	if (blksz_depth > fifo_depth)
> -		goto disable;
> +		return;
>  
>  	/*
>  	 * If (blksz_depth) >= (fifo_depth >> 1), should be 'thld_size <= blksz'
> @@ -644,9 +644,6 @@ static void dw_mci_ctrl_rd_thld(struct dw_mci *host, struct mmc_data *data)
>  	thld_size = blksz;
>  	mci_writel(host, CDTHRCTL, SDMMC_SET_RD_THLD(thld_size, 1));
>  	return;
> -
> -disable:
> -	mci_writel(host, CDTHRCTL, SDMMC_SET_RD_THLD(0, 0));
>  }
>  
>  static int dw_mci_submit_data_dma(struct dw_mci *host, struct mmc_data *data)
> -- 
> 1.9.1
> 
--
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
Shawn Guo Nov. 21, 2014, 11:31 p.m. UTC | #4
Seungwon, Jaehoon,

On Fri, Nov 21, 2014 at 02:39:10PM +0800, Shawn Guo wrote:
> Commit f1d2736c8156 ("mmc: dw_mmc: control card read threshold")
> introduces a regression for use case where PIO mode is used, i.e.
> CONFIG_MMC_DW_IDMAC is not disabled.  It stops kernel from booting

s/not disabled/disabled

> like below.

For understanding the problem better, can you guys please to test the
dw_mmc driver on Samsung platforms with CONFIG_MMC_DW_IDMAC disabled.
I'm wondering this is a problem specific to my platform or a common one.

Shawn

> 
> dw_mmc 9408000.dwmmc: fifo-depth property not found, using value of FIFOTH register as default
> dw_mmc 9408000.dwmmc: Using PIO mode.
> dw_mmc 9408000.dwmmc: Version ID is 210a
> dw_mmc 9408000.dwmmc: DW MMC controller at irq 72, 32 bit host data width, 32 deep fifo
> dw_mmc 9408000.dwmmc: 1 slots initialized
> ...
> Waiting for root device /dev/mmcblk0p2...
> mmc_host mmc0: Bus speed (slot 0) = 52000000Hz (slot req 50000000Hz, actual 26000000HZ div = 1)
> mmc0: new high speed SDHC card at address b368
> mmcblk0: mmc0:b368 AF UD 3.84 GiB
> 
> Rather than clearing read threshold bits, the best thing that function
> dw_mci_ctrl_rd_thld() should do is leaving the bits untouched, in case
> that card read threshold setup is not needed.
> 
> The patch fixes the regression by changing dw_mci_ctrl_rd_thld() a bit
> to do nothing in case card read threshold bits setup is not needed.
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> Fixes: f1d2736c8156 ("mmc: dw_mmc: control card read threshold")
> Cc: <stable@vger.kernel.org> # 3.13+
> ---
>  drivers/mmc/host/dw_mmc.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 69f0cc68d5b2..52c04ba69970 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -628,13 +628,13 @@ static void dw_mci_ctrl_rd_thld(struct dw_mci *host, struct mmc_data *data)
>  
>  	if (host->timing != MMC_TIMING_MMC_HS200 &&
>  	    host->timing != MMC_TIMING_UHS_SDR104)
> -		goto disable;
> +		return;
>  
>  	blksz_depth = blksz / (1 << host->data_shift);
>  	fifo_depth = host->fifo_depth;
>  
>  	if (blksz_depth > fifo_depth)
> -		goto disable;
> +		return;
>  
>  	/*
>  	 * If (blksz_depth) >= (fifo_depth >> 1), should be 'thld_size <= blksz'
> @@ -644,9 +644,6 @@ static void dw_mci_ctrl_rd_thld(struct dw_mci *host, struct mmc_data *data)
>  	thld_size = blksz;
>  	mci_writel(host, CDTHRCTL, SDMMC_SET_RD_THLD(thld_size, 1));
>  	return;
> -
> -disable:
> -	mci_writel(host, CDTHRCTL, SDMMC_SET_RD_THLD(0, 0));
>  }
>  
>  static int dw_mci_submit_data_dma(struct dw_mci *host, struct mmc_data *data)
> -- 
> 1.9.1
> 
--
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
Alim Akhtar Nov. 22, 2014, 1:26 p.m. UTC | #5
Hi Shawn,

On Sat, Nov 22, 2014 at 5:01 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> Seungwon, Jaehoon,
>
> On Fri, Nov 21, 2014 at 02:39:10PM +0800, Shawn Guo wrote:
>> Commit f1d2736c8156 ("mmc: dw_mmc: control card read threshold")
>> introduces a regression for use case where PIO mode is used, i.e.
>> CONFIG_MMC_DW_IDMAC is not disabled.  It stops kernel from booting
>
> s/not disabled/disabled
>
>> like below.
>
> For understanding the problem better, can you guys please to test the
> dw_mmc driver on Samsung platforms with CONFIG_MMC_DW_IDMAC disabled.
> I'm wondering this is a problem specific to my platform or a common one.
>
I tested PIO mode on exynos5800-peach-pi board with Ulf's -next
branch, and it works fine.
To confirm PIO mode works, I ran iozone and it gave  low performance
number (as expected) then IDMA mode.

One thing to note, is your controller version is 210a, so not sure if
something more in missing from driver.
Also from log its looks like you card clock is 26Mhz, did you tried
running at 52Mhz?

> Shawn
>
>>
>> dw_mmc 9408000.dwmmc: fifo-depth property not found, using value of FIFOTH register as default
>> dw_mmc 9408000.dwmmc: Using PIO mode.
>> dw_mmc 9408000.dwmmc: Version ID is 210a
>> dw_mmc 9408000.dwmmc: DW MMC controller at irq 72, 32 bit host data width, 32 deep fifo
>> dw_mmc 9408000.dwmmc: 1 slots initialized
>> ...
>> Waiting for root device /dev/mmcblk0p2...
>> mmc_host mmc0: Bus speed (slot 0) = 52000000Hz (slot req 50000000Hz, actual 26000000HZ div = 1)
>> mmc0: new high speed SDHC card at address b368
>> mmcblk0: mmc0:b368 AF UD 3.84 GiB
>>
>> Rather than clearing read threshold bits, the best thing that function
>> dw_mci_ctrl_rd_thld() should do is leaving the bits untouched, in case
>> that card read threshold setup is not needed.
>>
>> The patch fixes the regression by changing dw_mci_ctrl_rd_thld() a bit
>> to do nothing in case card read threshold bits setup is not needed.
>>
>> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
>> Fixes: f1d2736c8156 ("mmc: dw_mmc: control card read threshold")
>> Cc: <stable@vger.kernel.org> # 3.13+
>> ---
>>  drivers/mmc/host/dw_mmc.c | 7 ++-----
>>  1 file changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 69f0cc68d5b2..52c04ba69970 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -628,13 +628,13 @@ static void dw_mci_ctrl_rd_thld(struct dw_mci *host, struct mmc_data *data)
>>
>>       if (host->timing != MMC_TIMING_MMC_HS200 &&
>>           host->timing != MMC_TIMING_UHS_SDR104)
>> -             goto disable;
>> +             return;
>>
>>       blksz_depth = blksz / (1 << host->data_shift);
>>       fifo_depth = host->fifo_depth;
>>
>>       if (blksz_depth > fifo_depth)
>> -             goto disable;
>> +             return;
>>
>>       /*
>>        * If (blksz_depth) >= (fifo_depth >> 1), should be 'thld_size <= blksz'
>> @@ -644,9 +644,6 @@ static void dw_mci_ctrl_rd_thld(struct dw_mci *host, struct mmc_data *data)
>>       thld_size = blksz;
>>       mci_writel(host, CDTHRCTL, SDMMC_SET_RD_THLD(thld_size, 1));
>>       return;
>> -
>> -disable:
>> -     mci_writel(host, CDTHRCTL, SDMMC_SET_RD_THLD(0, 0));
>>  }
>>
>>  static int dw_mci_submit_data_dma(struct dw_mci *host, struct mmc_data *data)
>> --
>> 1.9.1
>>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Shawn Guo Nov. 23, 2014, 3:30 p.m. UTC | #6
Hi Alim,

On Sat, Nov 22, 2014 at 06:56:46PM +0530, Alim Akhtar wrote:
> Hi Shawn,
> 
> On Sat, Nov 22, 2014 at 5:01 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> > Seungwon, Jaehoon,
> >
> > On Fri, Nov 21, 2014 at 02:39:10PM +0800, Shawn Guo wrote:
> >> Commit f1d2736c8156 ("mmc: dw_mmc: control card read threshold")
> >> introduces a regression for use case where PIO mode is used, i.e.
> >> CONFIG_MMC_DW_IDMAC is not disabled.  It stops kernel from booting
> >
> > s/not disabled/disabled
> >
> >> like below.
> >
> > For understanding the problem better, can you guys please to test the
> > dw_mmc driver on Samsung platforms with CONFIG_MMC_DW_IDMAC disabled.
> > I'm wondering this is a problem specific to my platform or a common one.
> >
> I tested PIO mode on exynos5800-peach-pi board with Ulf's -next
> branch, and it works fine.
> To confirm PIO mode works, I ran iozone and it gave  low performance
> number (as expected) then IDMA mode.

Thanks a lot for testing and confirming that it works on exynos.

> 
> One thing to note, is your controller version is 210a, so not sure if
> something more in missing from driver.

Unfortunately, I do not have the reference manual for the hardware I'm
running so far.  But from the kernel log, the controller version is
210a.  What do you mean by "something more in missing from driver"?
The driver has any limitation for any particular controller versions?

One thing I need to check with hardware vendor is whether register
CDTHRCTL is available on my dwmmc controller.  From my testing, even
reading this register every time dw_mci_submit_data() is called causes
problem for me.

> Also from log its looks like you card clock is 26Mhz, did you tried
> running at 52Mhz?

I did not.  But it's unlikely the problem of clock frequency from what
I've seen.

Shawn
--
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
Alim Akhtar Nov. 23, 2014, 10:06 p.m. UTC | #7
Hi Shawn,

On Sun, Nov 23, 2014 at 9:00 PM, Shawn Guo <shawn.guo@linaro.org> wrote:
> Hi Alim,
>
> On Sat, Nov 22, 2014 at 06:56:46PM +0530, Alim Akhtar wrote:
>> Hi Shawn,
>>
>> On Sat, Nov 22, 2014 at 5:01 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
>> > Seungwon, Jaehoon,
>> >
>> > On Fri, Nov 21, 2014 at 02:39:10PM +0800, Shawn Guo wrote:
>> >> Commit f1d2736c8156 ("mmc: dw_mmc: control card read threshold")
>> >> introduces a regression for use case where PIO mode is used, i.e.
>> >> CONFIG_MMC_DW_IDMAC is not disabled.  It stops kernel from booting
>> >
>> > s/not disabled/disabled
>> >
>> >> like below.
>> >
>> > For understanding the problem better, can you guys please to test the
>> > dw_mmc driver on Samsung platforms with CONFIG_MMC_DW_IDMAC disabled.
>> > I'm wondering this is a problem specific to my platform or a common one.
>> >
>> I tested PIO mode on exynos5800-peach-pi board with Ulf's -next
>> branch, and it works fine.
>> To confirm PIO mode works, I ran iozone and it gave  low performance
>> number (as expected) then IDMA mode.
>
> Thanks a lot for testing and confirming that it works on exynos.
>
>>
>> One thing to note, is your controller version is 210a, so not sure if
>> something more in missing from driver.
>
> Unfortunately, I do not have the reference manual for the hardware I'm
> running so far.  But from the kernel log, the controller version is
> 210a.  What do you mean by "something more in missing from driver"?
> The driver has any limitation for any particular controller versions?
>
For that you need to check dw_mmc data book for 210a version. Sorry I
don't have it.

> One thing I need to check with hardware vendor is whether register
> CDTHRCTL is available on my dwmmc controller.  From my testing, even
> reading this register every time dw_mci_submit_data() is called causes
> problem for me.
>
yes, your guess is right, did you tested with ulf's -next branch?
Recently patch [1] is merged, that most likely to solve your platform
issue. Have a try.

[1] http://www.spinics.net/lists/stable/msg69884.html

>> Also from log its looks like you card clock is 26Mhz, did you tried
>> running at 52Mhz?
>
> I did not.  But it's unlikely the problem of clock frequency from what
> I've seen.
>
> Shawn
Shawn Guo Nov. 24, 2014, 12:25 a.m. UTC | #8
On Mon, Nov 24, 2014 at 03:36:13AM +0530, Alim Akhtar wrote:
> Hi Shawn,
> 
> On Sun, Nov 23, 2014 at 9:00 PM, Shawn Guo <shawn.guo@linaro.org> wrote:
> > One thing I need to check with hardware vendor is whether register
> > CDTHRCTL is available on my dwmmc controller.  From my testing, even
> > reading this register every time dw_mci_submit_data() is called causes
> > problem for me.
> >
> yes, your guess is right, did you tested with ulf's -next branch?
> Recently patch [1] is merged, that most likely to solve your platform
> issue. Have a try.
> 
> [1] http://www.spinics.net/lists/stable/msg69884.html

Yeah, the patch fixes my problem.  Thanks a lot for the pointer, Alim.

Shawn
--
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
diff mbox

Patch

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 69f0cc68d5b2..52c04ba69970 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -628,13 +628,13 @@  static void dw_mci_ctrl_rd_thld(struct dw_mci *host, struct mmc_data *data)
 
 	if (host->timing != MMC_TIMING_MMC_HS200 &&
 	    host->timing != MMC_TIMING_UHS_SDR104)
-		goto disable;
+		return;
 
 	blksz_depth = blksz / (1 << host->data_shift);
 	fifo_depth = host->fifo_depth;
 
 	if (blksz_depth > fifo_depth)
-		goto disable;
+		return;
 
 	/*
 	 * If (blksz_depth) >= (fifo_depth >> 1), should be 'thld_size <= blksz'
@@ -644,9 +644,6 @@  static void dw_mci_ctrl_rd_thld(struct dw_mci *host, struct mmc_data *data)
 	thld_size = blksz;
 	mci_writel(host, CDTHRCTL, SDMMC_SET_RD_THLD(thld_size, 1));
 	return;
-
-disable:
-	mci_writel(host, CDTHRCTL, SDMMC_SET_RD_THLD(0, 0));
 }
 
 static int dw_mci_submit_data_dma(struct dw_mci *host, struct mmc_data *data)