From: Vincent Yang <hidden> Date: 2014-06-26 06:23:26
Hi,
We are adding support for a new Fujitsu sdhci IP.
These patches are against v3.16-rc1 mainline since nothing in
mmc-next at this moment.
These patches are tested on 3.16-rc1 integration tree.
We welcome any comment and advice about how to make any
improvements or better align them with upstream.
Changes from v1:
- Add sufficient description in DT binding ducument
- Remove one patch "mmc: sdhci: add quirk for broken 3.0V support" and use
voltage-ranges = <> in the device tree instead
Thanks a lot!
Best regards,
Vincent Yang
Vincent Yang (6):
mmc: sdhci: add quirk for voltage switch callback
mmc: sdhci: add quirk for tuning work around
mmc: sdhci: add quirk for single block transactions
mmc: sdhci: host: add new f_sdh30
mmc: core: hold SD Clock before CMD11 during Signal Voltage Switch
Procedure
mmc: core: add manual resume capability
.../devicetree/bindings/mmc/sdhci-fujitsu.txt | 35 ++
drivers/mmc/core/core.c | 8 +-
drivers/mmc/core/sd.c | 4 +
drivers/mmc/host/Kconfig | 7 +
drivers/mmc/host/Makefile | 1 +
drivers/mmc/host/sdhci.c | 15 +-
drivers/mmc/host/sdhci.h | 1 +
drivers/mmc/host/sdhci_f_sdh30.c | 435 +++++++++++++++++++++
drivers/mmc/host/sdhci_f_sdh30.h | 40 ++
include/linux/mmc/host.h | 14 +
include/linux/mmc/sdhci.h | 6 +
11 files changed, 561 insertions(+), 5 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mmc/sdhci-fujitsu.txt
create mode 100644 drivers/mmc/host/sdhci_f_sdh30.c
create mode 100644 drivers/mmc/host/sdhci_f_sdh30.h
--
1.9.0
From: Vincent Yang <hidden> Date: 2014-06-26 06:23:27
This patch defines a quirk to do a callback when
switching voltages so do controller-specific
actions.
It is a preparation and will be used by Fujitsu
SDHCI controller f_sdh30 driver.
Signed-off-by: Vincent Yang <redacted>
---
drivers/mmc/host/sdhci.c | 5 +++++
drivers/mmc/host/sdhci.h | 1 +
include/linux/mmc/sdhci.h | 2 ++
3 files changed, 8 insertions(+)
@@ -1763,6 +1763,11 @@ static int sdhci_do_start_signal_voltage_switch(struct sdhci_host *host,ctrl|=SDHCI_CTRL_VDD_180;sdhci_writew(host,ctrl,SDHCI_HOST_CONTROL2);+/* Some controller need to do more when switching */+if((host->quirks2&SDHCI_QUIRK2_VOLTAGE_SWITCH)&&+host->ops->voltage_switch)+host->ops->voltage_switch(host);+/* Wait for 5ms */usleep_range(5000,5500);
@@ -98,6 +98,8 @@ struct sdhci_host {#define SDHCI_QUIRK2_BROKEN_HS200 (1<<6)/* Controller does not support DDR50 */#define SDHCI_QUIRK2_BROKEN_DDR50 (1<<7)+/* Do a callback when switching voltages so do controller-specific actions */+#define SDHCI_QUIRK2_VOLTAGE_SWITCH (1<<8)intirq;/* Device IRQ */void__iomem*ioaddr;/* Mapped address */
From: Vincent Yang <hidden> Date: 2014-06-26 06:23:28
This patch defines a quirk for tuning work
around for some sdhci host controller. It sets
both SDHCI_CTRL_EXEC_TUNING and SDHCI_CTRL_TUNED_CLK
for tuning.
It is a preparation and will be used by Fujitsu
SDHCI controller f_sdh30 driver.
Signed-off-by: Vincent Yang <redacted>
---
drivers/mmc/host/sdhci.c | 2 ++
include/linux/mmc/sdhci.h | 2 ++
2 files changed, 4 insertions(+)
From: Vincent Yang <hidden> Date: 2014-06-26 06:23:29
This patch defines a quirk to disable the block count
for single block transactions.
It is a preparation and will be used by Fujitsu
SDHCI controller f_sdh30 driver.
Signed-off-by: Vincent Yang <redacted>
---
drivers/mmc/host/sdhci.c | 8 +++++---
include/linux/mmc/sdhci.h | 2 ++
2 files changed, 7 insertions(+), 3 deletions(-)
@@ -0,0 +1,35 @@+* Fujitsu SDHCI controller++This file documents differences between the core properties in mmc.txt+and the properties used by the sdhci_f_sdh30 driver.++Required properties:+- compatible: "fujitsu,f_sdh30"+- voltage-ranges : two cells are required, first cell specifies minimum+ slot voltage (mV), second cell specifies maximum slot voltage (mV).+ Several ranges could be specified.++Optional properties:+- gpios: This is one optional gpio for controlling a power mux which+ switches between two power supplies. 3.3V is selected when gpio is high,+ and 1.8V is selected when gpio is low. This voltage is used for signal+ level.+- clocks: Must contain an entry for each entry in clock-names. It is a+ list of phandles and clock-specifier pairs.+ See ../clocks/clock-bindings.txt for details.+- clock-names: Should contain the following two entries:+ "sd_sd4clk" - clock primarily used for tuning process+ "sd_bclk" - base clock for sdhci controller++Example:++ sdhci1: sdio@36600000 {+ compatible = "fujitsu,f_sdh30";+ reg = <0 0x36600000 0x1000>;+ interrupts = <0 172 0x4>,+ <0 173 0x4>;+ voltage-ranges = <1800 1800 3300 3300>;+ gpios = <&gpio0 7 0>;+ clocks = <&clk_hdmi_2_0>, <&clk_hdmi_3_0>;+ clock-names = "sd_sd4clk", "sd_bclk";+ };
From: Vincent Yang <hidden> Date: 2014-06-26 06:23:31
This patch is to fix an issue found on mb86s7x platforms.
[symptom]
There are some UHS-1 SD memory cards sometimes cannot be detected correctly,
e.g., Transcend 600x SDXC 64GB UHS-1 memory card.
During Signal Voltage Switch Procedure, failure to switch is indicated
by the card holding DAT[3:0] low.
[analysis]
According to SD Host Controller Simplified Specification Version 3.00
chapter 3.6.1, the Signal Voltage Switch Procedure should be:
(1) Check S18A; (2) Issue CMD11; (3) Check CMD 11 response;
(4) Stop providing SD clock; (5) Check DAT[3:0] should be 0000b;
(6) Set 1.8V Signal Enable; (7) Wait 5ms; (8) Check 1.8V Signal Enable;
(9) Provide SD Clock; (10) Wait 1ms; (11) Check DAT[3:0] should be 1111b;
(12) error handling
With CONFIG_MMC_CLKGATE=y, sometimes there is one more gating/un-gating
SD clock between (2) and (3). In this case, some UHS-1 SD cards will hold
DAT[3:0] 0000b at (11) and thus fails Signal Voltage Switch Procedure.
[solution]
By mmc_host_clk_hold() before CMD11, the additional gating/un-gating
SD clock between (2) and (3) can be prevented and thus no failure at (11).
It has been verified with many UHS-1 SD cards on mb86s7x platforms and
works correctly.
Signed-off-by: Vincent Yang <redacted>
---
drivers/mmc/core/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
@@ -1428,6 +1428,8 @@ int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, u32 ocr)pr_warning("%s: cannot verify signal voltage switch\n",mmc_hostname(host));+mmc_host_clk_hold(host);+cmd.opcode=SD_SWITCH_VOLTAGE;cmd.arg=0;cmd.flags=MMC_RSP_R1|MMC_CMD_AC;
@@ -1438,8 +1440,6 @@ int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, u32 ocr)if(!mmc_host_is_spi(host)&&(cmd.resp[0]&R1_ERROR))return-EIO;--mmc_host_clk_hold(host);/**Thecardshoulddrivecmdanddat[0:3]lowimmediately*aftertheresponseofcmd11,butwait1mstobesure
From: Vincent Yang <hidden> Date: 2014-06-26 06:23:32
This patch adds manual resume for some embedded platforms with rootfs
stored in SD card. It references CONFIG_MMC_BLOCK_DEFERRED_RESUME in
kernel 3.10. It lets host controller driver to manually handle resume
by itself.
[symptom]
This issue is found on mb86s7x platforms with rootfs stored in SD card.
It failed to resume form STR suspend mode because SD card cannot be ready
in time. It take longer time (e.g., 600ms) to be ready for access.
The error log looks like below:
root@localhost:~# echo mem > /sys/power/state
[ 30.441974] SUSPEND
SCB Firmware : Category 01 Version 02.03 Rev. 00_
Config : (no configuration)
root@localhost:~# [ 30.702976] Buffer I/O error on device mmcblk1p2, logical block 31349
[ 30.709678] Buffer I/O error on device mmcblk1p2, logical block 168073
[ 30.716220] Buffer I/O error on device mmcblk1p2, logical block 168074
[ 30.722759] Buffer I/O error on device mmcblk1p2, logical block 168075
[ 30.729456] Buffer I/O error on device mmcblk1p2, logical block 31349
[ 30.735916] Buffer I/O error on device mmcblk1p2, logical block 31350
[ 30.742370] Buffer I/O error on device mmcblk1p2, logical block 31351
[ 30.749025] Buffer I/O error on device mmcblk1p2, logical block 168075
[ 30.755657] Buffer I/O error on device mmcblk1p2, logical block 31351
[ 30.763130] Aborting journal on device mmcblk1p2-8.
[ 30.768060] JBD2: Error -5 detected when updating journal superblock for mmcblk1p2-8.
[ 30.776085] EXT4-fs error (device mmcblk1p2): ext4_journal_check_start:56: Detected aborted journal
[ 30.785259] EXT4-fs (mmcblk1p2): Remounting filesystem read-only
[ 31.370716] EXT4-fs error (device mmcblk1p2): ext4_find_entry:1309: inode #2490369: comm udevd: reading directory lblock 0
[ 31.382485] EXT4-fs error (device mmcblk1p2): ext4_find_entry:1309: inode #1048577: comm udevd: reading directory lblock 0
[analysis]
In system resume path, mmc_sd_resume() is failed with error code -123
because at that time SD card is still not ready on mb86s7x platforms.
[solution]
In order to not blocking system resume path, this patch just sets a flag
MMC_BUSRESUME_MANUAL_RESUME when this error happened, and then host controller
driver can understand it by this flag. Then host controller driver have to
resume SD card manually and asynchronously.
Signed-off-by: Vincent Yang <redacted>
---
drivers/mmc/core/core.c | 4 ++
drivers/mmc/core/sd.c | 4 ++
drivers/mmc/host/sdhci_f_sdh30.c | 89 ++++++++++++++++++++++++++++++++++++++++
include/linux/mmc/host.h | 14 +++++++
4 files changed, 111 insertions(+)
On 26 juni 2014 08:23:32 CEST, Vincent Yang [off-list ref] wrote:
This patch adds manual resume for some embedded platforms with rootfs
stored in SD card. It references CONFIG_MMC_BLOCK_DEFERRED_RESUME in
kernel 3.10. It lets host controller driver to manually handle resume
by itself.
[symptom]
This issue is found on mb86s7x platforms with rootfs stored in SD card.
It failed to resume form STR suspend mode because SD card cannot be
ready
in time. It take longer time (e.g., 600ms) to be ready for access.
The error log looks like below:
root@localhost:~# echo mem > /sys/power/state
[ 30.441974] SUSPEND
SCB Firmware : Category 01 Version 02.03 Rev. 00_
Config : (no configuration)
root@localhost:~# [ 30.702976] Buffer I/O error on device mmcblk1p2,
logical block 31349
[ 30.709678] Buffer I/O error on device mmcblk1p2, logical block
168073
[ 30.716220] Buffer I/O error on device mmcblk1p2, logical block
168074
[ 30.722759] Buffer I/O error on device mmcblk1p2, logical block
168075
[ 30.729456] Buffer I/O error on device mmcblk1p2, logical block
31349
[ 30.735916] Buffer I/O error on device mmcblk1p2, logical block
31350
[ 30.742370] Buffer I/O error on device mmcblk1p2, logical block
31351
[ 30.749025] Buffer I/O error on device mmcblk1p2, logical block
168075
[ 30.755657] Buffer I/O error on device mmcblk1p2, logical block
31351
[ 30.763130] Aborting journal on device mmcblk1p2-8.
[ 30.768060] JBD2: Error -5 detected when updating journal superblock
for mmcblk1p2-8.
[ 30.776085] EXT4-fs error (device mmcblk1p2):
ext4_journal_check_start:56: Detected aborted journal
[ 30.785259] EXT4-fs (mmcblk1p2): Remounting filesystem read-only
[ 31.370716] EXT4-fs error (device mmcblk1p2): ext4_find_entry:1309:
inode #2490369: comm udevd: reading directory lblock 0
[ 31.382485] EXT4-fs error (device mmcblk1p2): ext4_find_entry:1309:
inode #1048577: comm udevd: reading directory lblock 0
[analysis]
In system resume path, mmc_sd_resume() is failed with error code -123
because at that time SD card is still not ready on mb86s7x platforms.
So why does it fail? It shouldn't!
I get the impression that you are solving this in the wrong way.
Kind regards
Uffe
quoted hunk
[solution]
In order to not blocking system resume path, this patch just sets a
flag
MMC_BUSRESUME_MANUAL_RESUME when this error happened, and then host
controller
driver can understand it by this flag. Then host controller driver have
to
resume SD card manually and asynchronously.
Signed-off-by: Vincent Yang <redacted>
---
drivers/mmc/core/core.c | 4 ++
drivers/mmc/core/sd.c | 4 ++
drivers/mmc/host/sdhci_f_sdh30.c | 89
++++++++++++++++++++++++++++++++++++++++
include/linux/mmc/host.h | 14 +++++++
4 files changed, 111 insertions(+)
@@ -0,0 +1,35 @@+* Fujitsu SDHCI controller++This file documents differences between the core properties in mmc.txt+and the properties used by the sdhci_f_sdh30 driver.++Required properties:+- compatible: "fujitsu,f_sdh30"
Please use '-' rather than '_' in compatible strings.
This seems to be the name of the driver. What is the precise name of the
IP block?
+- voltage-ranges : two cells are required, first cell specifies minimum
+ slot voltage (mV), second cell specifies maximum slot voltage (mV).
+ Several ranges could be specified.
Describe this as a list of pairs, it's confusing otherwise.
I'm not sure I follow what having multiple pairs implies.
+Optional properties:
+- gpios: This is one optional gpio for controlling a power mux which
+ switches between two power supplies. 3.3V is selected when gpio is high,
+ and 1.8V is selected when gpio is low. This voltage is used for signal
+ level.
Give this a more descriptive name, like power-mux-gpios. That will match
up with the style of cd-gpios and wp-gpios.
+- clocks: Must contain an entry for each entry in clock-names. It is a
+ list of phandles and clock-specifier pairs.
+ See ../clocks/clock-bindings.txt for details.
+- clock-names: Should contain the following two entries:
+ "sd_sd4clk" - clock primarily used for tuning process
+ "sd_bclk" - base clock for sdhci controller
+
+Example:
+
+ sdhci1: sdio@36600000 {
+ compatible = "fujitsu,f_sdh30";
+ reg = <0 0x36600000 0x1000>;
+ interrupts = <0 172 0x4>,
+ <0 173 0x4>;
+ voltage-ranges = <1800 1800 3300 3300>;
Place brackets around each pair to make this clearer:
voltage-ranges = <1800 1800>, <3300 3300>;
[...]
On 26 juni 2014 08:23:32 CEST, Vincent Yang [off-list ref] wrote:
quoted
This patch adds manual resume for some embedded platforms with rootfs
stored in SD card. It references CONFIG_MMC_BLOCK_DEFERRED_RESUME in
kernel 3.10. It lets host controller driver to manually handle resume
by itself.
[symptom]
This issue is found on mb86s7x platforms with rootfs stored in SD card.
It failed to resume form STR suspend mode because SD card cannot be
ready
in time. It take longer time (e.g., 600ms) to be ready for access.
The error log looks like below:
root@localhost:~# echo mem > /sys/power/state
[ 30.441974] SUSPEND
SCB Firmware : Category 01 Version 02.03 Rev. 00_
Config : (no configuration)
root@localhost:~# [ 30.702976] Buffer I/O error on device mmcblk1p2,
logical block 31349
[ 30.709678] Buffer I/O error on device mmcblk1p2, logical block
168073
[ 30.716220] Buffer I/O error on device mmcblk1p2, logical block
168074
[ 30.722759] Buffer I/O error on device mmcblk1p2, logical block
168075
[ 30.729456] Buffer I/O error on device mmcblk1p2, logical block
31349
[ 30.735916] Buffer I/O error on device mmcblk1p2, logical block
31350
[ 30.742370] Buffer I/O error on device mmcblk1p2, logical block
31351
[ 30.749025] Buffer I/O error on device mmcblk1p2, logical block
168075
[ 30.755657] Buffer I/O error on device mmcblk1p2, logical block
31351
[ 30.763130] Aborting journal on device mmcblk1p2-8.
[ 30.768060] JBD2: Error -5 detected when updating journal superblock
for mmcblk1p2-8.
[ 30.776085] EXT4-fs error (device mmcblk1p2):
ext4_journal_check_start:56: Detected aborted journal
[ 30.785259] EXT4-fs (mmcblk1p2): Remounting filesystem read-only
[ 31.370716] EXT4-fs error (device mmcblk1p2): ext4_find_entry:1309:
inode #2490369: comm udevd: reading directory lblock 0
[ 31.382485] EXT4-fs error (device mmcblk1p2): ext4_find_entry:1309:
inode #1048577: comm udevd: reading directory lblock 0
[analysis]
In system resume path, mmc_sd_resume() is failed with error code -123
because at that time SD card is still not ready on mb86s7x platforms.
So why does it fail? It shouldn't!
I get the impression that you are solving this in the wrong way.
Hi Uffe,
On mb86s7x EVB, power for SD card is completely removed when entering
STR suspend mode (i.e., echo mem > /sys/power/state). When system
starts to resume, it turns on the power for SD card again. However, It take
longer time (e.g., 600ms) to get the power ready.
This is why mmc_sd_resume() failed with error code -123. At that time point,
power for SD card is not ready yet.
At first we fixed it by a simple method of using SDHCI_QUIRK_DELAY_AFTER_POWER:
*host, unsigned char mode,
* they can apply clock after applying power
*/
if (host->quirks & SDHCI_QUIRK_DELAY_AFTER_POWER)
- mdelay(10);
+ mdelay(600);
}
if (host->vmmc) {
However, we found it blocks the system resume path. It can slow down
system resume and also booting.
Therefore, we would like to resume SD card manually and asynchronously
by host controller driver itself. Thus system resume path is not blocked.
Thanks a lot for your review!
Best regards,
Vincent Yang
Kind regards
Uffe
quoted
[solution]
In order to not blocking system resume path, this patch just sets a
flag
MMC_BUSRESUME_MANUAL_RESUME when this error happened, and then host
controller
driver can understand it by this flag. Then host controller driver have
to
resume SD card manually and asynchronously.
Signed-off-by: Vincent Yang <redacted>
---
drivers/mmc/core/core.c | 4 ++
drivers/mmc/core/sd.c | 4 ++
drivers/mmc/host/sdhci_f_sdh30.c | 89
++++++++++++++++++++++++++++++++++++++++
include/linux/mmc/host.h | 14 +++++++
4 files changed, 111 insertions(+)
@@ -0,0 +1,35 @@+* Fujitsu SDHCI controller++This file documents differences between the core properties in mmc.txt+and the properties used by the sdhci_f_sdh30 driver.++Required properties:+- compatible: "fujitsu,f_sdh30"
Please use '-' rather than '_' in compatible strings.
Hi Mark,
Yes, I'll update it to '-' in next version.
This seems to be the name of the driver. What is the precise name of the
IP block?
The name of the IP block is "F_SDH30".
That's why it uses "fujitsu,f_sdh30"
quoted
+- voltage-ranges : two cells are required, first cell specifies minimum
+ slot voltage (mV), second cell specifies maximum slot voltage (mV).
+ Several ranges could be specified.
Describe this as a list of pairs, it's confusing otherwise.
I'm not sure I follow what having multiple pairs implies.
Yes, I'll update it in next version.
quoted
+Optional properties:
+- gpios: This is one optional gpio for controlling a power mux which
+ switches between two power supplies. 3.3V is selected when gpio is high,
+ and 1.8V is selected when gpio is low. This voltage is used for signal
+ level.
Give this a more descriptive name, like power-mux-gpios. That will match
up with the style of cd-gpios and wp-gpios.
Yes, I'll update it in next version.
quoted
+- clocks: Must contain an entry for each entry in clock-names. It is a
+ list of phandles and clock-specifier pairs.
+ See ../clocks/clock-bindings.txt for details.
+- clock-names: Should contain the following two entries:
+ "sd_sd4clk" - clock primarily used for tuning process
+ "sd_bclk" - base clock for sdhci controller
+
+Example:
+
+ sdhci1: sdio@36600000 {
+ compatible = "fujitsu,f_sdh30";
+ reg = <0 0x36600000 0x1000>;
+ interrupts = <0 172 0x4>,
+ <0 173 0x4>;
+ voltage-ranges = <1800 1800 3300 3300>;
Place brackets around each pair to make this clearer:
voltage-ranges = <1800 1800>, <3300 3300>;
This wasn't in the binding document, and a grep for "vendor-hs200" in a
v3.16-rc2 tree found me nothing.
Please document this.
Yes, it is a setting for a vendor specific register.
I'll update it in next version.
quoted
+
+ if (!of_property_read_u32(pdev->dev.of_node, "bus-width", &bus_width)) {
+ if (bus_width == 8) {
+ dev_info(dev, "Applying 8 bit bus width\n");
+ host->mmc->caps |= MMC_CAP_8_BIT_DATA;
+ }
+ }
What if bus-width is not 8, or is not present?
In both cases, it will not touch host->mmc->caps at all. Then sdhci_add_host()
will handle it and set MMC_CAP_4_BIT_DATA as default:
[...]
/*
* A controller may support 8-bit width, but the board itself
* might not have the pins brought out. Boards that support
* 8-bit width must set "mmc->caps |= MMC_CAP_8_BIT_DATA;" in
* their platform code before calling sdhci_add_host(), and we
* won't assume 8-bit width for hosts without that CAP.
*/
if (!(host->quirks & SDHCI_QUIRK_FORCE_1_BIT_DATA))
mmc->caps |= MMC_CAP_4_BIT_DATA;
[...]
quoted
+
+ ret = mmc_of_parse_voltage(pdev->dev.of_node, &host->ocr_mask);
+ if (ret) {
+ dev_err(dev, "%s: parse voltage error\n", __func__);
+ goto err_voltage;
+ }
+
+ host->hw_name = DRIVER_NAME;
+ host->ops = &sdhci_f_sdh30_ops;
+ host->irq = irq;
+
+ host->ioaddr = of_iomap(pdev->dev.of_node, 0);
+ if (!host->ioaddr) {
+ dev_err(dev, "%s: failed to remap registers\n", __func__);
+ ret = -ENXIO;
+ goto err_remap;
+ }
+
+ priv->clk_sd4 = of_clk_get(pdev->dev.of_node, 0);
+ if (!IS_ERR(priv->clk_sd4)) {
+ ret = clk_prepare_enable(priv->clk_sd4);
+ if (ret < 0) {
+ dev_err(dev, "Failed to enable sd4 clock: %d\n", ret);
+ goto err_clk1;
+ }
+ }
+ priv->clk_b = of_clk_get(pdev->dev.of_node, 1);
+ if (!IS_ERR(priv->clk_b)) {
+ ret = clk_prepare_enable(priv->clk_b);
+ if (ret < 0) {
+ dev_err(dev, "Failed to enable clk_b clock: %d\n", ret);
+ goto err_clk2;
+ }
+ }
Given you gave these names, get these by name rather than index. It's
less surprising and more flexible.
Yes, I'll update them as below in next version.
- priv->clk_sd4 = of_clk_get(pdev->dev.of_node, 0);
+ priv->clk_sd4 = clk_get(&pdev->dev, "sd_sd4clk");
- priv->clk_b = of_clk_get(pdev->dev.of_node, 1);
+ priv->clk_b = clk_get(&pdev->dev, "sd_bclk");
Thanks a lot for your review!
Best regards,
Vincent Yang
This patch adds manual resume for some embedded platforms with rootfs
stored in SD card. It references CONFIG_MMC_BLOCK_DEFERRED_RESUME in
kernel 3.10. It lets host controller driver to manually handle resume
by itself.
[symptom]
This issue is found on mb86s7x platforms with rootfs stored in SD
card.
quoted
quoted
It failed to resume form STR suspend mode because SD card cannot be
ready
in time. It take longer time (e.g., 600ms) to be ready for access.
The error log looks like below:
root@localhost:~# echo mem > /sys/power/state
[ 30.441974] SUSPEND
SCB Firmware : Category 01 Version 02.03 Rev. 00_
Config : (no configuration)
root@localhost:~# [ 30.702976] Buffer I/O error on device
inode #1048577: comm udevd: reading directory lblock 0
[analysis]
In system resume path, mmc_sd_resume() is failed with error code -123
because at that time SD card is still not ready on mb86s7x platforms.
So why does it fail? It shouldn't!
I get the impression that you are solving this in the wrong way.
Hi Uffe,
On mb86s7x EVB, power for SD card is completely removed when entering
STR suspend mode (i.e., echo mem > /sys/power/state). When system
starts to resume, it turns on the power for SD card again. However, It
take
longer time (e.g., 600ms) to get the power ready.
This is why mmc_sd_resume() failed with error code -123. At that time
point,
power for SD card is not ready yet.
At first we fixed it by a simple method of using
SDHCI_QUIRK_DELAY_AFTER_POWER:
*host, unsigned char mode,
* they can apply clock after applying power
*/
if (host->quirks & SDHCI_QUIRK_DELAY_AFTER_POWER)
- mdelay(10);
+ mdelay(600);
}
If you can't model the power to the card through a regulator, this is what you need to do.
if (host->vmmc) {
However, we found it blocks the system resume path. It can slow down
system resume and also booting.
Therefore, we would like to resume SD card manually and asynchronously
by host controller driver itself. Thus system resume path is not
blocked.
That is accomplished by using MMC_CAP_RUNTIME_RESUME.
Kind regards
Uffe
Thanks a lot for your review!
Best regards,
Vincent Yang
quoted
Kind regards
Uffe
quoted
[solution]
In order to not blocking system resume path, this patch just sets a
flag
MMC_BUSRESUME_MANUAL_RESUME when this error happened, and then host
controller
driver can understand it by this flag. Then host controller driver
have
quoted
quoted
to
resume SD card manually and asynchronously.
Signed-off-by: Vincent Yang <redacted>
---
drivers/mmc/core/core.c | 4 ++
drivers/mmc/core/sd.c | 4 ++
drivers/mmc/host/sdhci_f_sdh30.c | 89
++++++++++++++++++++++++++++++++++++++++
include/linux/mmc/host.h | 14 +++++++
4 files changed, 111 insertions(+)
@@ -0,0 +1,35 @@+* Fujitsu SDHCI controller++This file documents differences between the core properties in mmc.txt+and the properties used by the sdhci_f_sdh30 driver.++Required properties:+- compatible: "fujitsu,f_sdh30"
Please use '-' rather than '_' in compatible strings.
Hi Mark,
Yes, I'll update it to '-' in next version.
quoted
This seems to be the name of the driver. What is the precise name of the
IP block?
The name of the IP block is "F_SDH30".
That's why it uses "fujitsu,f_sdh30"
Hmm. I'd still be tempted to use "fujitsu,f-sdh30".
This wasn't in the binding document, and a grep for "vendor-hs200" in a
v3.16-rc2 tree found me nothing.
Please document this.
Yes, it is a setting for a vendor specific register.
I'll update it in next version.
It would be nice to know exactly what this is. We usually shy clear of
placing register values in dt. I can wait until the next posting if
you're goin to document that.
quoted
quoted
+ if (!of_property_read_u32(pdev->dev.of_node, "bus-width", &bus_width)) {
+ if (bus_width == 8) {
+ dev_info(dev, "Applying 8 bit bus width\n");
+ host->mmc->caps |= MMC_CAP_8_BIT_DATA;
+ }
+ }
What if bus-width is not 8, or is not present?
In both cases, it will not touch host->mmc->caps at all. Then sdhci_add_host()
will handle it and set MMC_CAP_4_BIT_DATA as default:
[...]
/*
* A controller may support 8-bit width, but the board itself
* might not have the pins brought out. Boards that support
* 8-bit width must set "mmc->caps |= MMC_CAP_8_BIT_DATA;" in
* their platform code before calling sdhci_add_host(), and we
* won't assume 8-bit width for hosts without that CAP.
*/
if (!(host->quirks & SDHCI_QUIRK_FORCE_1_BIT_DATA))
mmc->caps |= MMC_CAP_4_BIT_DATA;
Ok, but does it make sense for a dts to have:
bus-width = <1>;
If so, we should presumably do something.
If not, we should at least print a warning that the dtb doesn't make
sense.
Cheers,
Mark.
This patch adds manual resume for some embedded platforms with rootfs
stored in SD card. It references CONFIG_MMC_BLOCK_DEFERRED_RESUME in
kernel 3.10. It lets host controller driver to manually handle resume
by itself.
[symptom]
This issue is found on mb86s7x platforms with rootfs stored in SD
card.
quoted
quoted
It failed to resume form STR suspend mode because SD card cannot be
ready
in time. It take longer time (e.g., 600ms) to be ready for access.
The error log looks like below:
root@localhost:~# echo mem > /sys/power/state
[ 30.441974] SUSPEND
SCB Firmware : Category 01 Version 02.03 Rev. 00_
Config : (no configuration)
root@localhost:~# [ 30.702976] Buffer I/O error on device
inode #1048577: comm udevd: reading directory lblock 0
[analysis]
In system resume path, mmc_sd_resume() is failed with error code -123
because at that time SD card is still not ready on mb86s7x platforms.
So why does it fail? It shouldn't!
I get the impression that you are solving this in the wrong way.
Hi Uffe,
On mb86s7x EVB, power for SD card is completely removed when entering
STR suspend mode (i.e., echo mem > /sys/power/state). When system
starts to resume, it turns on the power for SD card again. However, It
take
longer time (e.g., 600ms) to get the power ready.
This is why mmc_sd_resume() failed with error code -123. At that time
point,
power for SD card is not ready yet.
At first we fixed it by a simple method of using
SDHCI_QUIRK_DELAY_AFTER_POWER:
*host, unsigned char mode,
* they can apply clock after applying power
*/
if (host->quirks & SDHCI_QUIRK_DELAY_AFTER_POWER)
- mdelay(10);
+ mdelay(600);
}
If you can't model the power to the card through a regulator, this is what you need to do.
Hi Uffe,
Yes, I got it.
quoted
if (host->vmmc) {
However, we found it blocks the system resume path. It can slow down
system resume and also booting.
Therefore, we would like to resume SD card manually and asynchronously
by host controller driver itself. Thus system resume path is not
blocked.
That is accomplished by using MMC_CAP_RUNTIME_RESUME.
Yes, I got it.
Thanks a lot for your review and help.
I will update it in next version.
Best regards,
Vincent Yang
Kind regards
Uffe
quoted
Thanks a lot for your review!
Best regards,
Vincent Yang
quoted
Kind regards
Uffe
quoted
[solution]
In order to not blocking system resume path, this patch just sets a
flag
MMC_BUSRESUME_MANUAL_RESUME when this error happened, and then host
controller
driver can understand it by this flag. Then host controller driver
have
quoted
quoted
to
resume SD card manually and asynchronously.
Signed-off-by: Vincent Yang <redacted>
---
drivers/mmc/core/core.c | 4 ++
drivers/mmc/core/sd.c | 4 ++
drivers/mmc/host/sdhci_f_sdh30.c | 89
++++++++++++++++++++++++++++++++++++++++
include/linux/mmc/host.h | 14 +++++++
4 files changed, 111 insertions(+)
@@ -0,0 +1,35 @@+* Fujitsu SDHCI controller++This file documents differences between the core properties in mmc.txt+and the properties used by the sdhci_f_sdh30 driver.++Required properties:+- compatible: "fujitsu,f_sdh30"
Please use '-' rather than '_' in compatible strings.
Hi Mark,
Yes, I'll update it to '-' in next version.
quoted
This seems to be the name of the driver. What is the precise name of the
IP block?
The name of the IP block is "F_SDH30".
That's why it uses "fujitsu,f_sdh30"
Hmm. I'd still be tempted to use "fujitsu,f-sdh30".
Hi Mark,
Sure, I'll update it to "fujitsu,f-sdh30" in next version.
This wasn't in the binding document, and a grep for "vendor-hs200" in a
v3.16-rc2 tree found me nothing.
Please document this.
Yes, it is a setting for a vendor specific register.
I'll update it in next version.
It would be nice to know exactly what this is. We usually shy clear of
placing register values in dt. I can wait until the next posting if
you're goin to document that.
I'm thinking about removing this register value in dt.
I'll update it in next version.
quoted
quoted
quoted
+ if (!of_property_read_u32(pdev->dev.of_node, "bus-width", &bus_width)) {
+ if (bus_width == 8) {
+ dev_info(dev, "Applying 8 bit bus width\n");
+ host->mmc->caps |= MMC_CAP_8_BIT_DATA;
+ }
+ }
What if bus-width is not 8, or is not present?
In both cases, it will not touch host->mmc->caps at all. Then sdhci_add_host()
will handle it and set MMC_CAP_4_BIT_DATA as default:
[...]
/*
* A controller may support 8-bit width, but the board itself
* might not have the pins brought out. Boards that support
* 8-bit width must set "mmc->caps |= MMC_CAP_8_BIT_DATA;" in
* their platform code before calling sdhci_add_host(), and we
* won't assume 8-bit width for hosts without that CAP.
*/
if (!(host->quirks & SDHCI_QUIRK_FORCE_1_BIT_DATA))
mmc->caps |= MMC_CAP_4_BIT_DATA;
Ok, but does it make sense for a dts to have:
bus-width = <1>;
If so, we should presumably do something.
If not, we should at least print a warning that the dtb doesn't make
sense.
I'll print a warning for invalid values in next version.
Thanks a lot for your review!
Best regards,
Vincent Yang