mmc: dw_mmc: fix card read threshold for PIO mode
Subsystems (2)
PATCH 1/1 mmc: dw_mmc: fix card read threshold for PIO mode
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 210adw_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.
--- drivers/mmc/host/dw_mmc.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
drivers/mmc/host/dw_mmc.c +2 -5
@@ -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)
Signature · 1 line
1.9.1
Jaehoon Chung
maintainerPATCH 1/1 mmc: dw_mmc: fix card read threshold for PIO mode
Hi, Shawn.
Quoted 5 lines · Shawn Guo
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_IDMACis not disabled. It stops kernel from booting like below.
Quoted 10 lines
dw_mmc9408000.dwmmc: fifo-depth property not found, using value of FIFOTH register as defaultdw_mmc9408000.dwmmc: Using PIO mode.dw_mmc9408000.dwmmc: Version ID is 210adw_mmc9408000.dwmmc: DW MMC controller at irq 72, 32 bit host data width, 32 deep fifodw_mmc9408000.dwmmc: 1 slots initialized
...
Waiting for root device/dev/mmcblk0p2...mmc_hostmmc0: 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.
Quoted 36 lines
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 <[email protected]>Fixes f1d2736c8156 ("mmc: dw_mmc: control card read threshold")Cc <[email protected]> # 3.13+--- drivers/mmc/host/dw_mmc.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)drivers/mmc/host/dw_mmc.c +2
-2@@ -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'
drivers/mmc/host/dw_mmc.c static void dw_mci_ctrl_rd_thld(struct dw_mci *host, struct mmc_data *data) · line 646
Quoted 3 lines
drivers/mmc/host/dw_mmc.c +0
-0@@ -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
Quoted 7 lines
drivers/mmc/host/dw_mmc.c +0
-3- -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)
PATCH 1/1 mmc: dw_mmc: fix card read threshold for PIO mode
Hi Jaehoon,
Quoted 19 lines · Jaehoon Chung
Hi, Shawn.
Quoted 5 lines · Shawn Guo
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_IDMACis not disabled. It stops kernel from booting like below.Quoted 10 lines
dw_mmc9408000.dwmmc: fifo-depth property not found, using value of FIFOTH register as defaultdw_mmc9408000.dwmmc: Using PIO mode.dw_mmc9408000.dwmmc: Version ID is 210adw_mmc9408000.dwmmc: DW MMC controller at irq 72, 32 bit host data width, 32 deep fifodw_mmc9408000.dwmmc: 1 slots initialized
...
Waiting for root device/dev/mmcblk0p2...mmc_hostmmc0: 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
Quoted 3 lines
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.
Quoted 34 lines
Quoted 34 lines
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 <[email protected]>Fixes f1d2736c8156 ("mmc: dw_mmc: control card read threshold")Cc <[email protected]> # 3.13+--- drivers/mmc/host/dw_mmc.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)drivers/mmc/host/dw_mmc.c +2
-2@@ -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 ;
Quoted 8 lines
drivers/mmc/host/dw_mmc.c static void dw_mci_ctrl_rd_thld(struct dw_mci *host, struct mmc_data *data) · line 646
Quoted 6 lines
drivers/mmc/host/dw_mmc.c +0
-0/* * 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
PATCH 1/1 mmc: dw_mmc: fix card read threshold for PIO mode
In reply to Shawn GuoQuoted 3 lines · Shawn Guo
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_IDMACis not disabled. It stops kernel from booting
s/disabled/enabled
Quoted 1 line
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
Quoted 59 lines
dw_mmc9408000.dwmmc: fifo-depth property not found, using value of FIFOTH register as defaultdw_mmc9408000.dwmmc: Using PIO mode.dw_mmc9408000.dwmmc: Version ID is 210adw_mmc9408000.dwmmc: DW MMC controller at irq 72, 32 bit host data width, 32 deep fifodw_mmc9408000.dwmmc: 1 slots initialized
...
Waiting for root device/dev/mmcblk0p2...mmc_hostmmc0: 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 GiBRather 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 <[email protected]>Fixes f1d2736c8156 ("mmc: dw_mmc: control card read threshold")Cc <[email protected]> # 3.13+--- drivers/mmc/host/dw_mmc.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)drivers/mmc/host/dw_mmc.c +2
-5@@ -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)Signature · 1 line
1.9.1
PATCH 1/1 mmc: dw_mmc: fix card read threshold for PIO mode
In reply to Shawn GuoSeungwon, Jaehoon,
Quoted 3 lines · Shawn Guo
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_IDMACis not disabled. It stops kernel from booting
s/not disabled/disabled
Quoted 1 line
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
Quoted 59 lines
dw_mmc9408000.dwmmc: fifo-depth property not found, using value of FIFOTH register as defaultdw_mmc9408000.dwmmc: Using PIO mode.dw_mmc9408000.dwmmc: Version ID is 210adw_mmc9408000.dwmmc: DW MMC controller at irq 72, 32 bit host data width, 32 deep fifodw_mmc9408000.dwmmc: 1 slots initialized
...
Waiting for root device/dev/mmcblk0p2...mmc_hostmmc0: 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 GiBRather 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 <[email protected]>Fixes f1d2736c8156 ("mmc: dw_mmc: control card read threshold")Cc <[email protected]> # 3.13+--- drivers/mmc/host/dw_mmc.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)drivers/mmc/host/dw_mmc.c +2
-5@@ -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)Signature · 1 line
1.9.1
PATCH 1/1 mmc: dw_mmc: fix card read threshold for PIO mode
Hi Shawn,
Quoted 7 lines · Shawn Guo
Seungwon, Jaehoon,
Quoted 3 lines · Shawn Guo
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_IDMACis not disabled. It stops kernel from booting
Quoted 8 lines
s/not disabled/disabled
Quoted 1 line
like below.
For understanding the problem better, can you guys please to test the
dw_mmcdriver on Samsung platforms withCONFIG_MMC_DW_IDMACdisabled. 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?
Quoted 66 lines
Shawn
Quoted 57 lines
dw_mmc9408000.dwmmc: fifo-depth property not found, using value of FIFOTH register as defaultdw_mmc9408000.dwmmc: Using PIO mode.dw_mmc9408000.dwmmc: Version ID is 210adw_mmc9408000.dwmmc: DW MMC controller at irq 72, 32 bit host data width, 32 deep fifodw_mmc9408000.dwmmc: 1 slots initialized
...
Waiting for root device/dev/mmcblk0p2...mmc_hostmmc0: 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 GiBRather 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 <[email protected]>Fixes f1d2736c8156 ("mmc: dw_mmc: control card read threshold")Cc <[email protected]> # 3.13+--- drivers/mmc/host/dw_mmc.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)drivers/mmc/host/dw_mmc.c +2
-6@@ -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) --Quoted 2 lines
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Signature · 2 lines
Regards, Alim
PATCH 1/1 mmc: dw_mmc: fix card read threshold for PIO mode
Hi Alim,
Quoted 14 lines · Alim Akhtar
Hi Shawn,
Quoted 11 lines · Shawn Guo
Seungwon, Jaehoon,
Quoted 3 lines · Shawn Guo
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_IDMACis not disabled. It stops kernel from bootings/not disabled/disabled
Quoted 1 line
like below.
Quoted 8 lines
Quoted 4 lines
For understanding the problem better, can you guys please to test the
dw_mmcdriver on Samsung platforms withCONFIG_MMC_DW_IDMACdisabled. 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.
Quoted 3 lines
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.
Quoted 2 lines
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
PATCH 1/1 mmc: dw_mmc: fix card read threshold for PIO mode
Hi Shawn,
Quoted 29 lines · Shawn Guo
Hi Alim,
Quoted 14 lines · Alim Akhtar
Hi Shawn,
Quoted 11 lines · Shawn Guo
Seungwon, Jaehoon,
Quoted 3 lines · Shawn Guo
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_IDMACis not disabled. It stops kernel from bootings/not disabled/disabled
Quoted 1 line
like below.
Quoted 8 lines
Quoted 4 lines
For understanding the problem better, can you guys please to test the
dw_mmcdriver on Samsung platforms withCONFIG_MMC_DW_IDMACdisabled. 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.
Quoted 1 line
Quoted 8 lines
Quoted 2 lines
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.
Quoted 5 lines
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
Quoted 7 lines
Quoted 2 lines
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
Signature · 2 lines
Regards, Alim
PATCH 1/1 mmc: dw_mmc: fix card read threshold for PIO mode
Quoted 8 lines · Alim Akhtar
Hi Shawn,
Quoted 5 lines · Shawn Guo
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.
Quoted 5 lines
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.
Yeah, the patch fixes my problem. Thanks a lot for the pointer, Alim.
Shawn