* [RFC PATCH v2 0/6] mmc: sdhci: adding support for a new Fujitsu sdhci IP
@ 2014-06-26 6:23 ` Vincent Yang
0 siblings, 0 replies; 30+ messages in thread
From: Vincent Yang @ 2014-06-26 6:23 UTC (permalink / raw)
To: chris, linux-mmc
Cc: devicetree, andy.green, patches, anton, Vincent.Yang,
jaswinder.singh, linuxppc-dev
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
^ permalink raw reply [flat|nested] 30+ messages in thread* [RFC PATCH v2 1/6] mmc: sdhci: add quirk for voltage switch callback
2014-06-26 6:23 ` Vincent Yang
@ 2014-06-26 6:23 ` Vincent Yang
-1 siblings, 0 replies; 30+ messages in thread
From: Vincent Yang @ 2014-06-26 6:23 UTC (permalink / raw)
To: chris, linux-mmc
Cc: devicetree, anton, linuxppc-dev, patches, andy.green,
jaswinder.singh, Vincent.Yang
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 <Vincent.Yang@tw.fujitsu.com>
---
drivers/mmc/host/sdhci.c | 5 +++++
drivers/mmc/host/sdhci.h | 1 +
include/linux/mmc/sdhci.h | 2 ++
3 files changed, 8 insertions(+)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 47055f3..d62262b 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -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);
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 4a5cd5e..63c7a46 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -292,6 +292,7 @@ struct sdhci_ops {
void (*adma_workaround)(struct sdhci_host *host, u32 intmask);
void (*platform_init)(struct sdhci_host *host);
void (*card_event)(struct sdhci_host *host);
+ void (*voltage_switch)(struct sdhci_host *host);
};
#ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index 08abe99..5433f04 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -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)
int irq; /* Device IRQ */
void __iomem *ioaddr; /* Mapped address */
--
1.9.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [RFC PATCH v2 1/6] mmc: sdhci: add quirk for voltage switch callback
@ 2014-06-26 6:23 ` Vincent Yang
0 siblings, 0 replies; 30+ messages in thread
From: Vincent Yang @ 2014-06-26 6:23 UTC (permalink / raw)
To: chris, linux-mmc
Cc: devicetree, andy.green, patches, anton, Vincent.Yang,
jaswinder.singh, linuxppc-dev
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 <Vincent.Yang@tw.fujitsu.com>
---
drivers/mmc/host/sdhci.c | 5 +++++
drivers/mmc/host/sdhci.h | 1 +
include/linux/mmc/sdhci.h | 2 ++
3 files changed, 8 insertions(+)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 47055f3..d62262b 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -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);
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 4a5cd5e..63c7a46 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -292,6 +292,7 @@ struct sdhci_ops {
void (*adma_workaround)(struct sdhci_host *host, u32 intmask);
void (*platform_init)(struct sdhci_host *host);
void (*card_event)(struct sdhci_host *host);
+ void (*voltage_switch)(struct sdhci_host *host);
};
#ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index 08abe99..5433f04 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -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)
int irq; /* Device IRQ */
void __iomem *ioaddr; /* Mapped address */
--
1.9.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [RFC PATCH v2 2/6] mmc: sdhci: add quirk for tuning work around
2014-06-26 6:23 ` Vincent Yang
@ 2014-06-26 6:23 ` Vincent Yang
-1 siblings, 0 replies; 30+ messages in thread
From: Vincent Yang @ 2014-06-26 6:23 UTC (permalink / raw)
To: chris, linux-mmc
Cc: devicetree, anton, linuxppc-dev, patches, andy.green,
jaswinder.singh, Vincent.Yang
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 <Vincent.Yang@tw.fujitsu.com>
---
drivers/mmc/host/sdhci.c | 2 ++
include/linux/mmc/sdhci.h | 2 ++
2 files changed, 4 insertions(+)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index d62262b..900b4e4 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1867,6 +1867,8 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
ctrl |= SDHCI_CTRL_EXEC_TUNING;
+ if (host->quirks2 & SDHCI_QUIRK2_TUNING_WORK_AROUND)
+ ctrl |= SDHCI_CTRL_TUNED_CLK;
sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
/*
diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index 5433f04..bcbad45 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -100,6 +100,8 @@ struct sdhci_host {
#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)
+/* forced tuned clock */
+#define SDHCI_QUIRK2_TUNING_WORK_AROUND (1<<9)
int irq; /* Device IRQ */
void __iomem *ioaddr; /* Mapped address */
--
1.9.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [RFC PATCH v2 2/6] mmc: sdhci: add quirk for tuning work around
@ 2014-06-26 6:23 ` Vincent Yang
0 siblings, 0 replies; 30+ messages in thread
From: Vincent Yang @ 2014-06-26 6:23 UTC (permalink / raw)
To: chris, linux-mmc
Cc: devicetree, andy.green, patches, anton, Vincent.Yang,
jaswinder.singh, linuxppc-dev
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 <Vincent.Yang@tw.fujitsu.com>
---
drivers/mmc/host/sdhci.c | 2 ++
include/linux/mmc/sdhci.h | 2 ++
2 files changed, 4 insertions(+)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index d62262b..900b4e4 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1867,6 +1867,8 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
ctrl |= SDHCI_CTRL_EXEC_TUNING;
+ if (host->quirks2 & SDHCI_QUIRK2_TUNING_WORK_AROUND)
+ ctrl |= SDHCI_CTRL_TUNED_CLK;
sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
/*
diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index 5433f04..bcbad45 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -100,6 +100,8 @@ struct sdhci_host {
#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)
+/* forced tuned clock */
+#define SDHCI_QUIRK2_TUNING_WORK_AROUND (1<<9)
int irq; /* Device IRQ */
void __iomem *ioaddr; /* Mapped address */
--
1.9.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [RFC PATCH v2 3/6] mmc: sdhci: add quirk for single block transactions
2014-06-26 6:23 ` Vincent Yang
@ 2014-06-26 6:23 ` Vincent Yang
-1 siblings, 0 replies; 30+ messages in thread
From: Vincent Yang @ 2014-06-26 6:23 UTC (permalink / raw)
To: chris, linux-mmc
Cc: devicetree, anton, linuxppc-dev, patches, andy.green,
jaswinder.singh, Vincent.Yang
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 <Vincent.Yang@tw.fujitsu.com>
---
drivers/mmc/host/sdhci.c | 8 +++++---
include/linux/mmc/sdhci.h | 2 ++
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 900b4e4..169e17d 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -876,7 +876,7 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd)
static void sdhci_set_transfer_mode(struct sdhci_host *host,
struct mmc_command *cmd)
{
- u16 mode;
+ u16 mode = 0;
struct mmc_data *data = cmd->data;
if (data == NULL) {
@@ -889,9 +889,11 @@ static void sdhci_set_transfer_mode(struct sdhci_host *host,
WARN_ON(!host->data);
- mode = SDHCI_TRNS_BLK_CNT_EN;
+ if (!(host->quirks2 & SDHCI_QUIRK2_SUPPORT_SINGLE))
+ mode = SDHCI_TRNS_BLK_CNT_EN;
+
if (mmc_op_multi(cmd->opcode) || data->blocks > 1) {
- mode |= SDHCI_TRNS_MULTI;
+ mode = SDHCI_TRNS_BLK_CNT_EN | SDHCI_TRNS_MULTI;
/*
* If we are sending CMD23, CMD12 never gets sent
* on successful completion (so no Auto-CMD12).
diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index bcbad45..72072d1 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -102,6 +102,8 @@ struct sdhci_host {
#define SDHCI_QUIRK2_VOLTAGE_SWITCH (1<<8)
/* forced tuned clock */
#define SDHCI_QUIRK2_TUNING_WORK_AROUND (1<<9)
+/* disable the block count for single block transactions */
+#define SDHCI_QUIRK2_SUPPORT_SINGLE (1<<10)
int irq; /* Device IRQ */
void __iomem *ioaddr; /* Mapped address */
--
1.9.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [RFC PATCH v2 3/6] mmc: sdhci: add quirk for single block transactions
@ 2014-06-26 6:23 ` Vincent Yang
0 siblings, 0 replies; 30+ messages in thread
From: Vincent Yang @ 2014-06-26 6:23 UTC (permalink / raw)
To: chris, linux-mmc
Cc: devicetree, andy.green, patches, anton, Vincent.Yang,
jaswinder.singh, linuxppc-dev
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 <Vincent.Yang@tw.fujitsu.com>
---
drivers/mmc/host/sdhci.c | 8 +++++---
include/linux/mmc/sdhci.h | 2 ++
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 900b4e4..169e17d 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -876,7 +876,7 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd)
static void sdhci_set_transfer_mode(struct sdhci_host *host,
struct mmc_command *cmd)
{
- u16 mode;
+ u16 mode = 0;
struct mmc_data *data = cmd->data;
if (data == NULL) {
@@ -889,9 +889,11 @@ static void sdhci_set_transfer_mode(struct sdhci_host *host,
WARN_ON(!host->data);
- mode = SDHCI_TRNS_BLK_CNT_EN;
+ if (!(host->quirks2 & SDHCI_QUIRK2_SUPPORT_SINGLE))
+ mode = SDHCI_TRNS_BLK_CNT_EN;
+
if (mmc_op_multi(cmd->opcode) || data->blocks > 1) {
- mode |= SDHCI_TRNS_MULTI;
+ mode = SDHCI_TRNS_BLK_CNT_EN | SDHCI_TRNS_MULTI;
/*
* If we are sending CMD23, CMD12 never gets sent
* on successful completion (so no Auto-CMD12).
diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index bcbad45..72072d1 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -102,6 +102,8 @@ struct sdhci_host {
#define SDHCI_QUIRK2_VOLTAGE_SWITCH (1<<8)
/* forced tuned clock */
#define SDHCI_QUIRK2_TUNING_WORK_AROUND (1<<9)
+/* disable the block count for single block transactions */
+#define SDHCI_QUIRK2_SUPPORT_SINGLE (1<<10)
int irq; /* Device IRQ */
void __iomem *ioaddr; /* Mapped address */
--
1.9.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [RFC PATCH v2 4/6] mmc: sdhci: host: add new f_sdh30
2014-06-26 6:23 ` Vincent Yang
@ 2014-06-26 6:23 ` Vincent Yang
-1 siblings, 0 replies; 30+ messages in thread
From: Vincent Yang @ 2014-06-26 6:23 UTC (permalink / raw)
To: chris, linux-mmc
Cc: devicetree, anton, linuxppc-dev, patches, andy.green,
jaswinder.singh, Vincent.Yang
This patch adds new host controller driver for
Fujitsu SDHCI controller f_sdh30.
Signed-off-by: Vincent Yang <Vincent.Yang@tw.fujitsu.com>
---
.../devicetree/bindings/mmc/sdhci-fujitsu.txt | 35 +++
drivers/mmc/host/Kconfig | 7 +
drivers/mmc/host/Makefile | 1 +
drivers/mmc/host/sdhci_f_sdh30.c | 346 +++++++++++++++++++++
drivers/mmc/host/sdhci_f_sdh30.h | 40 +++
5 files changed, 429 insertions(+)
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
diff --git a/Documentation/devicetree/bindings/mmc/sdhci-fujitsu.txt b/Documentation/devicetree/bindings/mmc/sdhci-fujitsu.txt
new file mode 100644
index 0000000..40add438
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/sdhci-fujitsu.txt
@@ -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";
+ };
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 7fee224..a1f3207 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -281,6 +281,13 @@ config MMC_SDHCI_BCM2835
This selects the BCM2835 SD/MMC controller. If you have a BCM2835
platform with SD or MMC devices, say Y or M here.
+config MMC_SDHCI_F_SDH30
+ tristate "SDHCI support for Fujitsu Semiconductor F_SDH30"
+ depends on MMC_SDHCI && (ARCH_MB8AC0300 || ARCH_MB86S70)
+ help
+ This selects the Secure Digital Host Controller Interface (SDHCI)
+ Needed by some Fujitsu SoC for MMC / SD / SDIO support.
+ If you have a controller with this interface, say Y or M here.
If unsure, say N.
config MMC_MOXART
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index 7f81ddf..a4c89e5 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_MMC_SDHCI_PXAV3) += sdhci-pxav3.o
obj-$(CONFIG_MMC_SDHCI_PXAV2) += sdhci-pxav2.o
obj-$(CONFIG_MMC_SDHCI_S3C) += sdhci-s3c.o
obj-$(CONFIG_MMC_SDHCI_SIRF) += sdhci-sirf.o
+obj-$(CONFIG_MMC_SDHCI_F_SDH30) += sdhci_f_sdh30.o
obj-$(CONFIG_MMC_SDHCI_SPEAR) += sdhci-spear.o
obj-$(CONFIG_MMC_WBSD) += wbsd.o
obj-$(CONFIG_MMC_AU1X) += au1xmmc.o
diff --git a/drivers/mmc/host/sdhci_f_sdh30.c b/drivers/mmc/host/sdhci_f_sdh30.c
new file mode 100644
index 0000000..6fae509
--- /dev/null
+++ b/drivers/mmc/host/sdhci_f_sdh30.c
@@ -0,0 +1,346 @@
+/*
+ * linux/drivers/mmc/host/sdhci_f_sdh30.c
+ *
+ * Copyright (C) 2013 - 2014 Fujitsu Semiconductor, Ltd
+ * Vincent Yang <vincent.yang@tw.fujitsu.com>
+ * Copyright (C) 2014 Linaro Ltd Andy Green <andy.green@linaro.org>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, version 2 of the License.
+ */
+
+#include <linux/err.h>
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/mmc/sd.h>
+#include <linux/mmc/host.h>
+#include <linux/mmc/card.h>
+#include <linux/gpio.h>
+#include <linux/of_gpio.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/pm.h>
+#include <linux/pm_runtime.h>
+#include <linux/suspend.h>
+
+#include "sdhci.h"
+#include "sdhci-pltfm.h"
+#include "sdhci_f_sdh30.h"
+#include "../core/core.h"
+
+#define DRIVER_NAME "f_sdh30"
+
+
+struct f_sdhost_priv {
+ struct clk *clk_sd4;
+ struct clk *clk_b;
+ int gpio_select_1v8;
+ u32 vendor_hs200;
+ struct device *dev;
+};
+
+void sdhci_f_sdh30_soft_voltage_switch(struct sdhci_host *host)
+{
+ struct f_sdhost_priv *priv = sdhci_priv(host);
+ u32 ctrl = 0;
+
+ usleep_range(2500, 3000);
+ ctrl = sdhci_readl(host, F_SDH30_IO_CONTROL2);
+ ctrl |= F_SDH30_CRES_O_DN;
+ sdhci_writel(host, ctrl, F_SDH30_IO_CONTROL2);
+ ctrl |= F_SDH30_MSEL_O_1_8;
+ sdhci_writel(host, ctrl, F_SDH30_IO_CONTROL2);
+
+ if (gpio_is_valid(priv->gpio_select_1v8)) {
+ dev_info(priv->dev, "%s: setting gpio\n", __func__);
+ gpio_direction_output(priv->gpio_select_1v8, 0);
+ }
+
+ ctrl &= ~F_SDH30_CRES_O_DN;
+ sdhci_writel(host, ctrl, F_SDH30_IO_CONTROL2);
+ usleep_range(2500, 3000);
+
+ if (priv->vendor_hs200) {
+ dev_info(priv->dev, "%s: setting hs200\n", __func__);
+ ctrl = sdhci_readl(host, F_SDH30_ESD_CONTROL);
+ ctrl |= priv->vendor_hs200;
+ sdhci_writel(host, ctrl, F_SDH30_ESD_CONTROL);
+ }
+
+ ctrl = sdhci_readl(host, F_SDH30_TUNING_SETTING);
+ ctrl |= F_SDH30_CMD_CHK_DIS;
+ sdhci_writel(host, ctrl, F_SDH30_TUNING_SETTING);
+}
+
+unsigned int sdhci_f_sdh30_get_min_clock(struct sdhci_host *host)
+{
+ return F_SDH30_MIN_CLOCK;
+}
+
+void sdhci_f_sdh30_reset(struct sdhci_host *host, u8 mask)
+{
+ struct f_sdhost_priv *priv = sdhci_priv(host);
+
+ if (gpio_is_valid(priv->gpio_select_1v8))
+ gpio_direction_output(priv->gpio_select_1v8, 1);
+
+ if (sdhci_readw(host, SDHCI_CLOCK_CONTROL) == 0) {
+ sdhci_writew(host, 0xBC01, SDHCI_CLOCK_CONTROL);
+ mmiowb();
+ }
+
+ sdhci_reset(host, mask);
+}
+
+static const struct sdhci_ops sdhci_f_sdh30_ops = {
+ .voltage_switch = sdhci_f_sdh30_soft_voltage_switch,
+ .get_min_clock = sdhci_f_sdh30_get_min_clock,
+ .reset = sdhci_f_sdh30_reset,
+ .set_clock = sdhci_set_clock,
+ .set_bus_width = sdhci_set_bus_width,
+ .set_uhs_signaling = sdhci_set_uhs_signaling,
+};
+
+static int sdhci_f_sdh30_probe(struct platform_device *pdev)
+{
+ struct sdhci_host *host;
+ struct device *dev = &pdev->dev;
+ int irq, ctrl = 0, ret = 0;
+ struct f_sdhost_priv *priv;
+ u32 reg = 0, bus_width;
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0) {
+ dev_err(dev, "%s: no irq specified\n", __func__);
+ ret = irq;
+ goto err;
+ }
+
+ host = sdhci_alloc_host(dev, sizeof(struct sdhci_host) +
+ sizeof(struct f_sdhost_priv));
+ if (IS_ERR(host)) {
+ dev_err(dev, "%s: host allocate error\n", __func__);
+ ret = -ENOMEM;
+ goto err;
+ }
+ priv = sdhci_priv(host);
+ priv->dev = dev;
+
+ host->quirks = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC |
+ SDHCI_QUIRK_INVERTED_WRITE_PROTECT;
+ host->quirks2 = SDHCI_QUIRK2_SUPPORT_SINGLE |
+ SDHCI_QUIRK2_VOLTAGE_SWITCH |
+ SDHCI_QUIRK2_TUNING_WORK_AROUND;
+
+ if (!of_property_read_u32(pdev->dev.of_node, "vendor-hs200",
+ &priv->vendor_hs200))
+ dev_info(dev, "Applying vendor-hs200 setting\n");
+ else
+ priv->vendor_hs200 = 0;
+
+ 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;
+ }
+ }
+
+ 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;
+ }
+ }
+
+ /* optional */
+ priv->gpio_select_1v8 = of_get_gpio(pdev->dev.of_node, 0);
+ if (gpio_is_valid(priv->gpio_select_1v8)) {
+ ret = gpio_request(priv->gpio_select_1v8, "sdhci");
+ if (unlikely(ret)) {
+ dev_err(dev, " gpio %d request failed ",
+ priv->gpio_select_1v8);
+ goto err_gpio;
+ }
+ /* initially 3.3V */
+ ret = gpio_direction_output(priv->gpio_select_1v8, 1);
+ if (unlikely(ret)) {
+ dev_err(dev, "failed to set phy_enable gpio\n");
+ goto err_gpio;
+ }
+ }
+
+ ret = sdhci_add_host(host);
+ if (ret) {
+ dev_err(dev, "%s: host add error\n", __func__);
+ goto err_add_host;
+ }
+
+ platform_set_drvdata(pdev, host);
+
+#ifdef CONFIG_PM_RUNTIME
+ pm_runtime_enable(&pdev->dev);
+ ret = pm_runtime_get_sync(&pdev->dev);
+ if (ret < 0)
+ dev_err(dev, "Failed to pm_runtime_get_sync: %d\n", ret);
+#endif
+
+ /* init vendor specific regs */
+ ctrl = sdhci_readw(host, F_SDH30_AHB_CONFIG);
+ ctrl |= F_SDH30_SIN | F_SDH30_AHB_INCR_16 | F_SDH30_AHB_INCR_8 |
+ F_SDH30_AHB_INCR_4;
+ ctrl &= ~(F_SDH30_AHB_BIGED | F_SDH30_BUSLOCK_EN);
+ sdhci_writew(host, ctrl, F_SDH30_AHB_CONFIG);
+ mmiowb();
+
+ reg = sdhci_readl(host, F_SDH30_ESD_CONTROL);
+ sdhci_writel(host, reg & ~F_SDH30_EMMC_RST, F_SDH30_ESD_CONTROL);
+ msleep(20);
+ sdhci_writel(host, reg | F_SDH30_EMMC_RST, F_SDH30_ESD_CONTROL);
+ mmiowb();
+
+ return 0;
+
+err_add_host:
+ if (gpio_is_valid(priv->gpio_select_1v8)) {
+ gpio_direction_output(priv->gpio_select_1v8, 1);
+ gpio_free(priv->gpio_select_1v8);
+ }
+err_gpio:
+ clk_put(priv->clk_b);
+err_clk2:
+ clk_put(priv->clk_sd4);
+err_clk1:
+ iounmap(host->ioaddr);
+err_remap:
+err_voltage:
+ sdhci_free_host(host);
+err:
+ return ret;
+}
+
+static int sdhci_f_sdh30_remove(struct platform_device *pdev)
+{
+ struct sdhci_host *host = platform_get_drvdata(pdev);
+ struct f_sdhost_priv *priv = sdhci_priv(host);
+ struct resource *iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+ sdhci_remove_host(host, readl(host->ioaddr + SDHCI_INT_STATUS) ==
+ 0xffffffff);
+ iounmap(host->ioaddr);
+ release_mem_region(iomem->start, resource_size(iomem));
+
+ clk_disable_unprepare(priv->clk_sd4);
+ clk_disable_unprepare(priv->clk_b);
+
+ clk_put(priv->clk_b);
+ clk_put(priv->clk_sd4);
+
+ if (gpio_is_valid(priv->gpio_select_1v8)) {
+ gpio_direction_output(priv->gpio_select_1v8, 1);
+ gpio_free(priv->gpio_select_1v8);
+ }
+
+ sdhci_free_host(host);
+ platform_set_drvdata(pdev, NULL);
+
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int sdhci_f_sdh30_suspend(struct device *dev)
+{
+ struct sdhci_host *host = dev_get_drvdata(dev);
+
+ return sdhci_suspend_host(host);
+}
+
+static int sdhci_f_sdh30_resume(struct device *dev)
+{
+ struct sdhci_host *host = dev_get_drvdata(dev);
+
+ return sdhci_resume_host(host);
+}
+#endif
+
+#ifdef CONFIG_PM_RUNTIME
+static int sdhci_f_sdh30_runtime_suspend(struct device *dev)
+{
+ struct sdhci_host *host = dev_get_drvdata(dev);
+
+ return sdhci_runtime_suspend_host(host);
+}
+
+static int sdhci_f_sdh30_runtime_resume(struct device *dev)
+{
+ struct sdhci_host *host = dev_get_drvdata(dev);
+
+ return sdhci_runtime_resume_host(host);
+}
+#endif
+
+#ifdef CONFIG_PM
+static const struct dev_pm_ops sdhci_f_sdh30_pmops = {
+ SET_SYSTEM_SLEEP_PM_OPS(sdhci_f_sdh30_suspend, sdhci_f_sdh30_resume)
+ SET_RUNTIME_PM_OPS(sdhci_f_sdh30_runtime_suspend,
+ sdhci_f_sdh30_runtime_resume, NULL)
+};
+
+#define SDHCI_F_SDH30_PMOPS (&sdhci_f_sdh30_pmops)
+
+#else
+#define SDHCI_F_SDH30_PMOPS NULL
+#endif
+
+static const struct of_device_id f_sdh30_dt_ids[] = {
+ { .compatible = "fujitsu,f_sdh30" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, f_sdh30_dt_ids);
+
+static struct platform_driver sdhci_f_sdh30_driver = {
+ .driver = {
+ .name = DRIVER_NAME,
+ .owner = THIS_MODULE,
+ .of_match_table = f_sdh30_dt_ids,
+#ifdef CONFIG_PM_SLEEP
+ .pm = SDHCI_F_SDH30_PMOPS,
+#endif
+ },
+ .probe = sdhci_f_sdh30_probe,
+ .remove = sdhci_f_sdh30_remove,
+};
+
+module_platform_driver(sdhci_f_sdh30_driver);
+
+MODULE_DESCRIPTION("F_SDH30 SD Card Controller driver");
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("FUJITSU SEMICONDUCTOR LTD.");
+MODULE_ALIAS("platform: " DRIVER_NAME);
diff --git a/drivers/mmc/host/sdhci_f_sdh30.h b/drivers/mmc/host/sdhci_f_sdh30.h
new file mode 100644
index 0000000..50c51c9
--- /dev/null
+++ b/drivers/mmc/host/sdhci_f_sdh30.h
@@ -0,0 +1,40 @@
+/*
+ * linux/drivers/mmc/host/sdhci_f_sdh30.h
+ *
+ * Copyright (C) 2013 - 2014 Fujitsu Semiconductor, Ltd
+ * Vincent Yang <vincent.yang@tw.fujitsu.com>
+ * Copyright (C) 2014 Linaro Ltd Andy Green <andy.green@linaro.org>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, version 2 of the License.
+ */
+
+#ifndef F_SDH30_H
+#define F_SDH30_H
+
+/* F_SDH30 extended Controller registers */
+#define F_SDH30_AHB_CONFIG 0x100
+#define F_SDH30_AHB_BIGED 0x00000040
+#define F_SDH30_BUSLOCK_DMA 0x00000020
+#define F_SDH30_BUSLOCK_EN 0x00000010
+#define F_SDH30_SIN 0x00000008
+#define F_SDH30_AHB_INCR_16 0x00000004
+#define F_SDH30_AHB_INCR_8 0x00000002
+#define F_SDH30_AHB_INCR_4 0x00000001
+
+#define F_SDH30_TUNING_SETTING 0x108
+#define F_SDH30_CMD_CHK_DIS 0x00010000
+
+#define F_SDH30_IO_CONTROL2 0x114
+#define F_SDH30_CRES_O_DN 0x00080000
+#define F_SDH30_MSEL_O_1_8 0x00040000
+
+#define F_SDH30_ESD_CONTROL 0x124
+#define F_SDH30_EMMC_RST 0x00000002
+
+#define F_SDH30_CMD_DAT_DELAY 0x200
+
+#define F_SDH30_MIN_CLOCK 400000
+
+#endif
--
1.9.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [RFC PATCH v2 4/6] mmc: sdhci: host: add new f_sdh30
@ 2014-06-26 6:23 ` Vincent Yang
0 siblings, 0 replies; 30+ messages in thread
From: Vincent Yang @ 2014-06-26 6:23 UTC (permalink / raw)
To: chris, linux-mmc
Cc: devicetree, andy.green, patches, anton, Vincent.Yang,
jaswinder.singh, linuxppc-dev
This patch adds new host controller driver for
Fujitsu SDHCI controller f_sdh30.
Signed-off-by: Vincent Yang <Vincent.Yang@tw.fujitsu.com>
---
.../devicetree/bindings/mmc/sdhci-fujitsu.txt | 35 +++
drivers/mmc/host/Kconfig | 7 +
drivers/mmc/host/Makefile | 1 +
drivers/mmc/host/sdhci_f_sdh30.c | 346 +++++++++++++++++++++
drivers/mmc/host/sdhci_f_sdh30.h | 40 +++
5 files changed, 429 insertions(+)
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
diff --git a/Documentation/devicetree/bindings/mmc/sdhci-fujitsu.txt b/Documentation/devicetree/bindings/mmc/sdhci-fujitsu.txt
new file mode 100644
index 0000000..40add438
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/sdhci-fujitsu.txt
@@ -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";
+ };
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 7fee224..a1f3207 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -281,6 +281,13 @@ config MMC_SDHCI_BCM2835
This selects the BCM2835 SD/MMC controller. If you have a BCM2835
platform with SD or MMC devices, say Y or M here.
+config MMC_SDHCI_F_SDH30
+ tristate "SDHCI support for Fujitsu Semiconductor F_SDH30"
+ depends on MMC_SDHCI && (ARCH_MB8AC0300 || ARCH_MB86S70)
+ help
+ This selects the Secure Digital Host Controller Interface (SDHCI)
+ Needed by some Fujitsu SoC for MMC / SD / SDIO support.
+ If you have a controller with this interface, say Y or M here.
If unsure, say N.
config MMC_MOXART
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index 7f81ddf..a4c89e5 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_MMC_SDHCI_PXAV3) += sdhci-pxav3.o
obj-$(CONFIG_MMC_SDHCI_PXAV2) += sdhci-pxav2.o
obj-$(CONFIG_MMC_SDHCI_S3C) += sdhci-s3c.o
obj-$(CONFIG_MMC_SDHCI_SIRF) += sdhci-sirf.o
+obj-$(CONFIG_MMC_SDHCI_F_SDH30) += sdhci_f_sdh30.o
obj-$(CONFIG_MMC_SDHCI_SPEAR) += sdhci-spear.o
obj-$(CONFIG_MMC_WBSD) += wbsd.o
obj-$(CONFIG_MMC_AU1X) += au1xmmc.o
diff --git a/drivers/mmc/host/sdhci_f_sdh30.c b/drivers/mmc/host/sdhci_f_sdh30.c
new file mode 100644
index 0000000..6fae509
--- /dev/null
+++ b/drivers/mmc/host/sdhci_f_sdh30.c
@@ -0,0 +1,346 @@
+/*
+ * linux/drivers/mmc/host/sdhci_f_sdh30.c
+ *
+ * Copyright (C) 2013 - 2014 Fujitsu Semiconductor, Ltd
+ * Vincent Yang <vincent.yang@tw.fujitsu.com>
+ * Copyright (C) 2014 Linaro Ltd Andy Green <andy.green@linaro.org>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, version 2 of the License.
+ */
+
+#include <linux/err.h>
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/mmc/sd.h>
+#include <linux/mmc/host.h>
+#include <linux/mmc/card.h>
+#include <linux/gpio.h>
+#include <linux/of_gpio.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/pm.h>
+#include <linux/pm_runtime.h>
+#include <linux/suspend.h>
+
+#include "sdhci.h"
+#include "sdhci-pltfm.h"
+#include "sdhci_f_sdh30.h"
+#include "../core/core.h"
+
+#define DRIVER_NAME "f_sdh30"
+
+
+struct f_sdhost_priv {
+ struct clk *clk_sd4;
+ struct clk *clk_b;
+ int gpio_select_1v8;
+ u32 vendor_hs200;
+ struct device *dev;
+};
+
+void sdhci_f_sdh30_soft_voltage_switch(struct sdhci_host *host)
+{
+ struct f_sdhost_priv *priv = sdhci_priv(host);
+ u32 ctrl = 0;
+
+ usleep_range(2500, 3000);
+ ctrl = sdhci_readl(host, F_SDH30_IO_CONTROL2);
+ ctrl |= F_SDH30_CRES_O_DN;
+ sdhci_writel(host, ctrl, F_SDH30_IO_CONTROL2);
+ ctrl |= F_SDH30_MSEL_O_1_8;
+ sdhci_writel(host, ctrl, F_SDH30_IO_CONTROL2);
+
+ if (gpio_is_valid(priv->gpio_select_1v8)) {
+ dev_info(priv->dev, "%s: setting gpio\n", __func__);
+ gpio_direction_output(priv->gpio_select_1v8, 0);
+ }
+
+ ctrl &= ~F_SDH30_CRES_O_DN;
+ sdhci_writel(host, ctrl, F_SDH30_IO_CONTROL2);
+ usleep_range(2500, 3000);
+
+ if (priv->vendor_hs200) {
+ dev_info(priv->dev, "%s: setting hs200\n", __func__);
+ ctrl = sdhci_readl(host, F_SDH30_ESD_CONTROL);
+ ctrl |= priv->vendor_hs200;
+ sdhci_writel(host, ctrl, F_SDH30_ESD_CONTROL);
+ }
+
+ ctrl = sdhci_readl(host, F_SDH30_TUNING_SETTING);
+ ctrl |= F_SDH30_CMD_CHK_DIS;
+ sdhci_writel(host, ctrl, F_SDH30_TUNING_SETTING);
+}
+
+unsigned int sdhci_f_sdh30_get_min_clock(struct sdhci_host *host)
+{
+ return F_SDH30_MIN_CLOCK;
+}
+
+void sdhci_f_sdh30_reset(struct sdhci_host *host, u8 mask)
+{
+ struct f_sdhost_priv *priv = sdhci_priv(host);
+
+ if (gpio_is_valid(priv->gpio_select_1v8))
+ gpio_direction_output(priv->gpio_select_1v8, 1);
+
+ if (sdhci_readw(host, SDHCI_CLOCK_CONTROL) == 0) {
+ sdhci_writew(host, 0xBC01, SDHCI_CLOCK_CONTROL);
+ mmiowb();
+ }
+
+ sdhci_reset(host, mask);
+}
+
+static const struct sdhci_ops sdhci_f_sdh30_ops = {
+ .voltage_switch = sdhci_f_sdh30_soft_voltage_switch,
+ .get_min_clock = sdhci_f_sdh30_get_min_clock,
+ .reset = sdhci_f_sdh30_reset,
+ .set_clock = sdhci_set_clock,
+ .set_bus_width = sdhci_set_bus_width,
+ .set_uhs_signaling = sdhci_set_uhs_signaling,
+};
+
+static int sdhci_f_sdh30_probe(struct platform_device *pdev)
+{
+ struct sdhci_host *host;
+ struct device *dev = &pdev->dev;
+ int irq, ctrl = 0, ret = 0;
+ struct f_sdhost_priv *priv;
+ u32 reg = 0, bus_width;
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0) {
+ dev_err(dev, "%s: no irq specified\n", __func__);
+ ret = irq;
+ goto err;
+ }
+
+ host = sdhci_alloc_host(dev, sizeof(struct sdhci_host) +
+ sizeof(struct f_sdhost_priv));
+ if (IS_ERR(host)) {
+ dev_err(dev, "%s: host allocate error\n", __func__);
+ ret = -ENOMEM;
+ goto err;
+ }
+ priv = sdhci_priv(host);
+ priv->dev = dev;
+
+ host->quirks = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC |
+ SDHCI_QUIRK_INVERTED_WRITE_PROTECT;
+ host->quirks2 = SDHCI_QUIRK2_SUPPORT_SINGLE |
+ SDHCI_QUIRK2_VOLTAGE_SWITCH |
+ SDHCI_QUIRK2_TUNING_WORK_AROUND;
+
+ if (!of_property_read_u32(pdev->dev.of_node, "vendor-hs200",
+ &priv->vendor_hs200))
+ dev_info(dev, "Applying vendor-hs200 setting\n");
+ else
+ priv->vendor_hs200 = 0;
+
+ 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;
+ }
+ }
+
+ 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;
+ }
+ }
+
+ /* optional */
+ priv->gpio_select_1v8 = of_get_gpio(pdev->dev.of_node, 0);
+ if (gpio_is_valid(priv->gpio_select_1v8)) {
+ ret = gpio_request(priv->gpio_select_1v8, "sdhci");
+ if (unlikely(ret)) {
+ dev_err(dev, " gpio %d request failed ",
+ priv->gpio_select_1v8);
+ goto err_gpio;
+ }
+ /* initially 3.3V */
+ ret = gpio_direction_output(priv->gpio_select_1v8, 1);
+ if (unlikely(ret)) {
+ dev_err(dev, "failed to set phy_enable gpio\n");
+ goto err_gpio;
+ }
+ }
+
+ ret = sdhci_add_host(host);
+ if (ret) {
+ dev_err(dev, "%s: host add error\n", __func__);
+ goto err_add_host;
+ }
+
+ platform_set_drvdata(pdev, host);
+
+#ifdef CONFIG_PM_RUNTIME
+ pm_runtime_enable(&pdev->dev);
+ ret = pm_runtime_get_sync(&pdev->dev);
+ if (ret < 0)
+ dev_err(dev, "Failed to pm_runtime_get_sync: %d\n", ret);
+#endif
+
+ /* init vendor specific regs */
+ ctrl = sdhci_readw(host, F_SDH30_AHB_CONFIG);
+ ctrl |= F_SDH30_SIN | F_SDH30_AHB_INCR_16 | F_SDH30_AHB_INCR_8 |
+ F_SDH30_AHB_INCR_4;
+ ctrl &= ~(F_SDH30_AHB_BIGED | F_SDH30_BUSLOCK_EN);
+ sdhci_writew(host, ctrl, F_SDH30_AHB_CONFIG);
+ mmiowb();
+
+ reg = sdhci_readl(host, F_SDH30_ESD_CONTROL);
+ sdhci_writel(host, reg & ~F_SDH30_EMMC_RST, F_SDH30_ESD_CONTROL);
+ msleep(20);
+ sdhci_writel(host, reg | F_SDH30_EMMC_RST, F_SDH30_ESD_CONTROL);
+ mmiowb();
+
+ return 0;
+
+err_add_host:
+ if (gpio_is_valid(priv->gpio_select_1v8)) {
+ gpio_direction_output(priv->gpio_select_1v8, 1);
+ gpio_free(priv->gpio_select_1v8);
+ }
+err_gpio:
+ clk_put(priv->clk_b);
+err_clk2:
+ clk_put(priv->clk_sd4);
+err_clk1:
+ iounmap(host->ioaddr);
+err_remap:
+err_voltage:
+ sdhci_free_host(host);
+err:
+ return ret;
+}
+
+static int sdhci_f_sdh30_remove(struct platform_device *pdev)
+{
+ struct sdhci_host *host = platform_get_drvdata(pdev);
+ struct f_sdhost_priv *priv = sdhci_priv(host);
+ struct resource *iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+ sdhci_remove_host(host, readl(host->ioaddr + SDHCI_INT_STATUS) ==
+ 0xffffffff);
+ iounmap(host->ioaddr);
+ release_mem_region(iomem->start, resource_size(iomem));
+
+ clk_disable_unprepare(priv->clk_sd4);
+ clk_disable_unprepare(priv->clk_b);
+
+ clk_put(priv->clk_b);
+ clk_put(priv->clk_sd4);
+
+ if (gpio_is_valid(priv->gpio_select_1v8)) {
+ gpio_direction_output(priv->gpio_select_1v8, 1);
+ gpio_free(priv->gpio_select_1v8);
+ }
+
+ sdhci_free_host(host);
+ platform_set_drvdata(pdev, NULL);
+
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int sdhci_f_sdh30_suspend(struct device *dev)
+{
+ struct sdhci_host *host = dev_get_drvdata(dev);
+
+ return sdhci_suspend_host(host);
+}
+
+static int sdhci_f_sdh30_resume(struct device *dev)
+{
+ struct sdhci_host *host = dev_get_drvdata(dev);
+
+ return sdhci_resume_host(host);
+}
+#endif
+
+#ifdef CONFIG_PM_RUNTIME
+static int sdhci_f_sdh30_runtime_suspend(struct device *dev)
+{
+ struct sdhci_host *host = dev_get_drvdata(dev);
+
+ return sdhci_runtime_suspend_host(host);
+}
+
+static int sdhci_f_sdh30_runtime_resume(struct device *dev)
+{
+ struct sdhci_host *host = dev_get_drvdata(dev);
+
+ return sdhci_runtime_resume_host(host);
+}
+#endif
+
+#ifdef CONFIG_PM
+static const struct dev_pm_ops sdhci_f_sdh30_pmops = {
+ SET_SYSTEM_SLEEP_PM_OPS(sdhci_f_sdh30_suspend, sdhci_f_sdh30_resume)
+ SET_RUNTIME_PM_OPS(sdhci_f_sdh30_runtime_suspend,
+ sdhci_f_sdh30_runtime_resume, NULL)
+};
+
+#define SDHCI_F_SDH30_PMOPS (&sdhci_f_sdh30_pmops)
+
+#else
+#define SDHCI_F_SDH30_PMOPS NULL
+#endif
+
+static const struct of_device_id f_sdh30_dt_ids[] = {
+ { .compatible = "fujitsu,f_sdh30" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, f_sdh30_dt_ids);
+
+static struct platform_driver sdhci_f_sdh30_driver = {
+ .driver = {
+ .name = DRIVER_NAME,
+ .owner = THIS_MODULE,
+ .of_match_table = f_sdh30_dt_ids,
+#ifdef CONFIG_PM_SLEEP
+ .pm = SDHCI_F_SDH30_PMOPS,
+#endif
+ },
+ .probe = sdhci_f_sdh30_probe,
+ .remove = sdhci_f_sdh30_remove,
+};
+
+module_platform_driver(sdhci_f_sdh30_driver);
+
+MODULE_DESCRIPTION("F_SDH30 SD Card Controller driver");
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("FUJITSU SEMICONDUCTOR LTD.");
+MODULE_ALIAS("platform: " DRIVER_NAME);
diff --git a/drivers/mmc/host/sdhci_f_sdh30.h b/drivers/mmc/host/sdhci_f_sdh30.h
new file mode 100644
index 0000000..50c51c9
--- /dev/null
+++ b/drivers/mmc/host/sdhci_f_sdh30.h
@@ -0,0 +1,40 @@
+/*
+ * linux/drivers/mmc/host/sdhci_f_sdh30.h
+ *
+ * Copyright (C) 2013 - 2014 Fujitsu Semiconductor, Ltd
+ * Vincent Yang <vincent.yang@tw.fujitsu.com>
+ * Copyright (C) 2014 Linaro Ltd Andy Green <andy.green@linaro.org>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, version 2 of the License.
+ */
+
+#ifndef F_SDH30_H
+#define F_SDH30_H
+
+/* F_SDH30 extended Controller registers */
+#define F_SDH30_AHB_CONFIG 0x100
+#define F_SDH30_AHB_BIGED 0x00000040
+#define F_SDH30_BUSLOCK_DMA 0x00000020
+#define F_SDH30_BUSLOCK_EN 0x00000010
+#define F_SDH30_SIN 0x00000008
+#define F_SDH30_AHB_INCR_16 0x00000004
+#define F_SDH30_AHB_INCR_8 0x00000002
+#define F_SDH30_AHB_INCR_4 0x00000001
+
+#define F_SDH30_TUNING_SETTING 0x108
+#define F_SDH30_CMD_CHK_DIS 0x00010000
+
+#define F_SDH30_IO_CONTROL2 0x114
+#define F_SDH30_CRES_O_DN 0x00080000
+#define F_SDH30_MSEL_O_1_8 0x00040000
+
+#define F_SDH30_ESD_CONTROL 0x124
+#define F_SDH30_EMMC_RST 0x00000002
+
+#define F_SDH30_CMD_DAT_DELAY 0x200
+
+#define F_SDH30_MIN_CLOCK 400000
+
+#endif
--
1.9.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [RFC PATCH v2 4/6] mmc: sdhci: host: add new f_sdh30
2014-06-26 6:23 ` Vincent Yang
@ 2014-06-26 11:03 ` Mark Rutland
-1 siblings, 0 replies; 30+ messages in thread
From: Mark Rutland @ 2014-06-26 11:03 UTC (permalink / raw)
To: Vincent Yang
Cc: chris, linux-mmc, devicetree, anton, linuxppc-dev, patches,
andy.green, jaswinder.singh, Vincent.Yang
On Thu, Jun 26, 2014 at 07:23:30AM +0100, Vincent Yang wrote:
> This patch adds new host controller driver for
> Fujitsu SDHCI controller f_sdh30.
>
> Signed-off-by: Vincent Yang <Vincent.Yang@tw.fujitsu.com>
> ---
> .../devicetree/bindings/mmc/sdhci-fujitsu.txt | 35 +++
> drivers/mmc/host/Kconfig | 7 +
> drivers/mmc/host/Makefile | 1 +
> drivers/mmc/host/sdhci_f_sdh30.c | 346 +++++++++++++++++++++
> drivers/mmc/host/sdhci_f_sdh30.h | 40 +++
> 5 files changed, 429 insertions(+)
> 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
>
> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-fujitsu.txt b/Documentation/devicetree/bindings/mmc/sdhci-fujitsu.txt
> new file mode 100644
> index 0000000..40add438
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/sdhci-fujitsu.txt
> @@ -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>;
[...]
> + if (!of_property_read_u32(pdev->dev.of_node, "vendor-hs200",
> + &priv->vendor_hs200))
> + dev_info(dev, "Applying vendor-hs200 setting\n");
> + else
> + priv->vendor_hs200 = 0;
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.
> +
> + 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?
> +
> + 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.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [RFC PATCH v2 4/6] mmc: sdhci: host: add new f_sdh30
@ 2014-06-26 11:03 ` Mark Rutland
0 siblings, 0 replies; 30+ messages in thread
From: Mark Rutland @ 2014-06-26 11:03 UTC (permalink / raw)
To: Vincent Yang
Cc: devicetree, andy.green, patches, anton, linux-mmc, chris,
Vincent.Yang, jaswinder.singh, linuxppc-dev
On Thu, Jun 26, 2014 at 07:23:30AM +0100, Vincent Yang wrote:
> This patch adds new host controller driver for
> Fujitsu SDHCI controller f_sdh30.
>
> Signed-off-by: Vincent Yang <Vincent.Yang@tw.fujitsu.com>
> ---
> .../devicetree/bindings/mmc/sdhci-fujitsu.txt | 35 +++
> drivers/mmc/host/Kconfig | 7 +
> drivers/mmc/host/Makefile | 1 +
> drivers/mmc/host/sdhci_f_sdh30.c | 346 +++++++++++++++++++++
> drivers/mmc/host/sdhci_f_sdh30.h | 40 +++
> 5 files changed, 429 insertions(+)
> 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
>
> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-fujitsu.txt b/Documentation/devicetree/bindings/mmc/sdhci-fujitsu.txt
> new file mode 100644
> index 0000000..40add438
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/sdhci-fujitsu.txt
> @@ -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>;
[...]
> + if (!of_property_read_u32(pdev->dev.of_node, "vendor-hs200",
> + &priv->vendor_hs200))
> + dev_info(dev, "Applying vendor-hs200 setting\n");
> + else
> + priv->vendor_hs200 = 0;
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.
> +
> + 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?
> +
> + 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.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [RFC PATCH v2 4/6] mmc: sdhci: host: add new f_sdh30
2014-06-26 11:03 ` Mark Rutland
@ 2014-06-27 3:32 ` Vincent Yang
-1 siblings, 0 replies; 30+ messages in thread
From: Vincent Yang @ 2014-06-27 3:32 UTC (permalink / raw)
To: Mark Rutland
Cc: chris, linux-mmc, devicetree, anton, linuxppc-dev, patches,
andy.green, jaswinder.singh, Vincent.Yang
2014-06-26 19:03 GMT+08:00 Mark Rutland <mark.rutland@arm.com>:
> On Thu, Jun 26, 2014 at 07:23:30AM +0100, Vincent Yang wrote:
>> This patch adds new host controller driver for
>> Fujitsu SDHCI controller f_sdh30.
>>
>> Signed-off-by: Vincent Yang <Vincent.Yang@tw.fujitsu.com>
>> ---
>> .../devicetree/bindings/mmc/sdhci-fujitsu.txt | 35 +++
>> drivers/mmc/host/Kconfig | 7 +
>> drivers/mmc/host/Makefile | 1 +
>> drivers/mmc/host/sdhci_f_sdh30.c | 346 +++++++++++++++++++++
>> drivers/mmc/host/sdhci_f_sdh30.h | 40 +++
>> 5 files changed, 429 insertions(+)
>> 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
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-fujitsu.txt b/Documentation/devicetree/bindings/mmc/sdhci-fujitsu.txt
>> new file mode 100644
>> index 0000000..40add438
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-fujitsu.txt
>> @@ -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"
>
>> +- 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.
>
>> +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.
>
>> +- 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>;
Yes, I'll update it in next version.
>
> [...]
>
>> + if (!of_property_read_u32(pdev->dev.of_node, "vendor-hs200",
>> + &priv->vendor_hs200))
>> + dev_info(dev, "Applying vendor-hs200 setting\n");
>> + else
>> + priv->vendor_hs200 = 0;
>
> 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.
>
>> +
>> + 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;
[...]
>
>> +
>> + 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
>
> Thanks,
> Mark.
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [RFC PATCH v2 4/6] mmc: sdhci: host: add new f_sdh30
@ 2014-06-27 3:32 ` Vincent Yang
0 siblings, 0 replies; 30+ messages in thread
From: Vincent Yang @ 2014-06-27 3:32 UTC (permalink / raw)
To: Mark Rutland
Cc: devicetree, andy.green, patches, anton, linux-mmc, chris,
Vincent.Yang, jaswinder.singh, linuxppc-dev
2014-06-26 19:03 GMT+08:00 Mark Rutland <mark.rutland@arm.com>:
> On Thu, Jun 26, 2014 at 07:23:30AM +0100, Vincent Yang wrote:
>> This patch adds new host controller driver for
>> Fujitsu SDHCI controller f_sdh30.
>>
>> Signed-off-by: Vincent Yang <Vincent.Yang@tw.fujitsu.com>
>> ---
>> .../devicetree/bindings/mmc/sdhci-fujitsu.txt | 35 +++
>> drivers/mmc/host/Kconfig | 7 +
>> drivers/mmc/host/Makefile | 1 +
>> drivers/mmc/host/sdhci_f_sdh30.c | 346 +++++++++++++++++++++
>> drivers/mmc/host/sdhci_f_sdh30.h | 40 +++
>> 5 files changed, 429 insertions(+)
>> 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
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-fujitsu.txt b/Documentation/devicetree/bindings/mmc/sdhci-fujitsu.txt
>> new file mode 100644
>> index 0000000..40add438
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-fujitsu.txt
>> @@ -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"
>
>> +- 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.
>
>> +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.
>
>> +- 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>;
Yes, I'll update it in next version.
>
> [...]
>
>> + if (!of_property_read_u32(pdev->dev.of_node, "vendor-hs200",
>> + &priv->vendor_hs200))
>> + dev_info(dev, "Applying vendor-hs200 setting\n");
>> + else
>> + priv->vendor_hs200 = 0;
>
> 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.
>
>> +
>> + 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;
[...]
>
>> +
>> + 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
>
> Thanks,
> Mark.
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [RFC PATCH v2 4/6] mmc: sdhci: host: add new f_sdh30
2014-06-27 3:32 ` Vincent Yang
@ 2014-06-27 10:12 ` Mark Rutland
-1 siblings, 0 replies; 30+ messages in thread
From: Mark Rutland @ 2014-06-27 10:12 UTC (permalink / raw)
To: Vincent Yang
Cc: chris, linux-mmc, devicetree, anton, linuxppc-dev, patches,
andy.green, jaswinder.singh, Vincent.Yang
On Fri, Jun 27, 2014 at 04:32:21AM +0100, Vincent Yang wrote:
> 2014-06-26 19:03 GMT+08:00 Mark Rutland <mark.rutland@arm.com>:
> > On Thu, Jun 26, 2014 at 07:23:30AM +0100, Vincent Yang wrote:
> >> This patch adds new host controller driver for
> >> Fujitsu SDHCI controller f_sdh30.
> >>
> >> Signed-off-by: Vincent Yang <Vincent.Yang@tw.fujitsu.com>
> >> ---
> >> .../devicetree/bindings/mmc/sdhci-fujitsu.txt | 35 +++
> >> drivers/mmc/host/Kconfig | 7 +
> >> drivers/mmc/host/Makefile | 1 +
> >> drivers/mmc/host/sdhci_f_sdh30.c | 346 +++++++++++++++++++++
> >> drivers/mmc/host/sdhci_f_sdh30.h | 40 +++
> >> 5 files changed, 429 insertions(+)
> >> 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
> >>
> >> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-fujitsu.txt b/Documentation/devicetree/bindings/mmc/sdhci-fujitsu.txt
> >> new file mode 100644
> >> index 0000000..40add438
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/mmc/sdhci-fujitsu.txt
> >> @@ -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"
Hmm. I'd still be tempted to use "fujitsu,f-sdh30".
> >
> > [...]
> >
> >> + if (!of_property_read_u32(pdev->dev.of_node, "vendor-hs200",
> >> + &priv->vendor_hs200))
> >> + dev_info(dev, "Applying vendor-hs200 setting\n");
> >> + else
> >> + priv->vendor_hs200 = 0;
> >
> > 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.
> >> + 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.
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [RFC PATCH v2 4/6] mmc: sdhci: host: add new f_sdh30
@ 2014-06-27 10:12 ` Mark Rutland
0 siblings, 0 replies; 30+ messages in thread
From: Mark Rutland @ 2014-06-27 10:12 UTC (permalink / raw)
To: Vincent Yang
Cc: devicetree, andy.green, patches, anton, linux-mmc, chris,
Vincent.Yang, jaswinder.singh, linuxppc-dev
On Fri, Jun 27, 2014 at 04:32:21AM +0100, Vincent Yang wrote:
> 2014-06-26 19:03 GMT+08:00 Mark Rutland <mark.rutland@arm.com>:
> > On Thu, Jun 26, 2014 at 07:23:30AM +0100, Vincent Yang wrote:
> >> This patch adds new host controller driver for
> >> Fujitsu SDHCI controller f_sdh30.
> >>
> >> Signed-off-by: Vincent Yang <Vincent.Yang@tw.fujitsu.com>
> >> ---
> >> .../devicetree/bindings/mmc/sdhci-fujitsu.txt | 35 +++
> >> drivers/mmc/host/Kconfig | 7 +
> >> drivers/mmc/host/Makefile | 1 +
> >> drivers/mmc/host/sdhci_f_sdh30.c | 346 +++++++++++++++++++++
> >> drivers/mmc/host/sdhci_f_sdh30.h | 40 +++
> >> 5 files changed, 429 insertions(+)
> >> 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
> >>
> >> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-fujitsu.txt b/Documentation/devicetree/bindings/mmc/sdhci-fujitsu.txt
> >> new file mode 100644
> >> index 0000000..40add438
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/mmc/sdhci-fujitsu.txt
> >> @@ -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"
Hmm. I'd still be tempted to use "fujitsu,f-sdh30".
> >
> > [...]
> >
> >> + if (!of_property_read_u32(pdev->dev.of_node, "vendor-hs200",
> >> + &priv->vendor_hs200))
> >> + dev_info(dev, "Applying vendor-hs200 setting\n");
> >> + else
> >> + priv->vendor_hs200 = 0;
> >
> > 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.
> >> + 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.
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [RFC PATCH v2 4/6] mmc: sdhci: host: add new f_sdh30
2014-06-27 10:12 ` Mark Rutland
@ 2014-06-30 2:10 ` Vincent Yang
-1 siblings, 0 replies; 30+ messages in thread
From: Vincent Yang @ 2014-06-30 2:10 UTC (permalink / raw)
To: Mark Rutland
Cc: chris, linux-mmc, devicetree, anton, linuxppc-dev, patches,
andy.green, jaswinder.singh
2014-06-27 18:12 GMT+08:00 Mark Rutland <mark.rutland@arm.com>:
> On Fri, Jun 27, 2014 at 04:32:21AM +0100, Vincent Yang wrote:
>> 2014-06-26 19:03 GMT+08:00 Mark Rutland <mark.rutland@arm.com>:
>> > On Thu, Jun 26, 2014 at 07:23:30AM +0100, Vincent Yang wrote:
>> >> This patch adds new host controller driver for
>> >> Fujitsu SDHCI controller f_sdh30.
>> >>
>> >> Signed-off-by: Vincent Yang <Vincent.Yang@tw.fujitsu.com>
>> >> ---
>> >> .../devicetree/bindings/mmc/sdhci-fujitsu.txt | 35 +++
>> >> drivers/mmc/host/Kconfig | 7 +
>> >> drivers/mmc/host/Makefile | 1 +
>> >> drivers/mmc/host/sdhci_f_sdh30.c | 346 +++++++++++++++++++++
>> >> drivers/mmc/host/sdhci_f_sdh30.h | 40 +++
>> >> 5 files changed, 429 insertions(+)
>> >> 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
>> >>
>> >> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-fujitsu.txt b/Documentation/devicetree/bindings/mmc/sdhci-fujitsu.txt
>> >> new file mode 100644
>> >> index 0000000..40add438
>> >> --- /dev/null
>> >> +++ b/Documentation/devicetree/bindings/mmc/sdhci-fujitsu.txt
>> >> @@ -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"
>
> 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.
>
>> >
>> > [...]
>> >
>> >> + if (!of_property_read_u32(pdev->dev.of_node, "vendor-hs200",
>> >> + &priv->vendor_hs200))
>> >> + dev_info(dev, "Applying vendor-hs200 setting\n");
>> >> + else
>> >> + priv->vendor_hs200 = 0;
>> >
>> > 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.
>
>> >> + 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
>
> Cheers,
> Mark.
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [RFC PATCH v2 4/6] mmc: sdhci: host: add new f_sdh30
@ 2014-06-30 2:10 ` Vincent Yang
0 siblings, 0 replies; 30+ messages in thread
From: Vincent Yang @ 2014-06-30 2:10 UTC (permalink / raw)
To: Mark Rutland
Cc: devicetree, andy.green, patches, anton, linux-mmc, chris,
jaswinder.singh, linuxppc-dev
2014-06-27 18:12 GMT+08:00 Mark Rutland <mark.rutland@arm.com>:
> On Fri, Jun 27, 2014 at 04:32:21AM +0100, Vincent Yang wrote:
>> 2014-06-26 19:03 GMT+08:00 Mark Rutland <mark.rutland@arm.com>:
>> > On Thu, Jun 26, 2014 at 07:23:30AM +0100, Vincent Yang wrote:
>> >> This patch adds new host controller driver for
>> >> Fujitsu SDHCI controller f_sdh30.
>> >>
>> >> Signed-off-by: Vincent Yang <Vincent.Yang@tw.fujitsu.com>
>> >> ---
>> >> .../devicetree/bindings/mmc/sdhci-fujitsu.txt | 35 +++
>> >> drivers/mmc/host/Kconfig | 7 +
>> >> drivers/mmc/host/Makefile | 1 +
>> >> drivers/mmc/host/sdhci_f_sdh30.c | 346 +++++++++++++++++++++
>> >> drivers/mmc/host/sdhci_f_sdh30.h | 40 +++
>> >> 5 files changed, 429 insertions(+)
>> >> 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
>> >>
>> >> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-fujitsu.txt b/Documentation/devicetree/bindings/mmc/sdhci-fujitsu.txt
>> >> new file mode 100644
>> >> index 0000000..40add438
>> >> --- /dev/null
>> >> +++ b/Documentation/devicetree/bindings/mmc/sdhci-fujitsu.txt
>> >> @@ -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"
>
> 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.
>
>> >
>> > [...]
>> >
>> >> + if (!of_property_read_u32(pdev->dev.of_node, "vendor-hs200",
>> >> + &priv->vendor_hs200))
>> >> + dev_info(dev, "Applying vendor-hs200 setting\n");
>> >> + else
>> >> + priv->vendor_hs200 = 0;
>> >
>> > 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.
>
>> >> + 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
>
> Cheers,
> Mark.
^ permalink raw reply [flat|nested] 30+ messages in thread
* [RFC PATCH v2 5/6] mmc: core: hold SD Clock before CMD11 during Signal Voltage Switch Procedure
2014-06-26 6:23 ` Vincent Yang
@ 2014-06-26 6:23 ` Vincent Yang
-1 siblings, 0 replies; 30+ messages in thread
From: Vincent Yang @ 2014-06-26 6:23 UTC (permalink / raw)
To: chris, linux-mmc
Cc: devicetree, anton, linuxppc-dev, patches, andy.green,
jaswinder.singh, Vincent.Yang
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 <Vincent.Yang@tw.fujitsu.com>
---
drivers/mmc/core/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 7dc0c85..764af63 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -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);
/*
* The card should drive cmd and dat[0:3] low immediately
* after the response of cmd11, but wait 1 ms to be sure
--
1.9.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [RFC PATCH v2 5/6] mmc: core: hold SD Clock before CMD11 during Signal Voltage Switch Procedure
@ 2014-06-26 6:23 ` Vincent Yang
0 siblings, 0 replies; 30+ messages in thread
From: Vincent Yang @ 2014-06-26 6:23 UTC (permalink / raw)
To: chris, linux-mmc
Cc: devicetree, andy.green, patches, anton, Vincent.Yang,
jaswinder.singh, linuxppc-dev
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 <Vincent.Yang@tw.fujitsu.com>
---
drivers/mmc/core/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 7dc0c85..764af63 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -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);
/*
* The card should drive cmd and dat[0:3] low immediately
* after the response of cmd11, but wait 1 ms to be sure
--
1.9.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [RFC PATCH v2 6/6] mmc: core: add manual resume capability
2014-06-26 6:23 ` Vincent Yang
@ 2014-06-26 6:23 ` Vincent Yang
-1 siblings, 0 replies; 30+ messages in thread
From: Vincent Yang @ 2014-06-26 6:23 UTC (permalink / raw)
To: chris, linux-mmc
Cc: devicetree, anton, linuxppc-dev, patches, andy.green,
jaswinder.singh, 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.
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 <Vincent.Yang@tw.fujitsu.com>
---
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(+)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 764af63..51fce49 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2648,6 +2648,10 @@ int mmc_pm_notify(struct notifier_block *notify_block,
case PM_POST_RESTORE:
spin_lock_irqsave(&host->lock, flags);
+ if (mmc_bus_manual_resume(host)) {
+ spin_unlock_irqrestore(&host->lock, flags);
+ break;
+ }
host->rescan_disable = 0;
spin_unlock_irqrestore(&host->lock, flags);
_mmc_detect_change(host, 0, false);
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index 0c44510..859390d 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -1133,6 +1133,10 @@ static int mmc_sd_resume(struct mmc_host *host)
if (!(host->caps & MMC_CAP_RUNTIME_RESUME)) {
err = _mmc_sd_resume(host);
+ if ((host->caps2 & MMC_CAP2_MANUAL_RESUME) && err)
+ mmc_set_bus_resume_policy(host, 1);
+ else
+ mmc_set_bus_resume_policy(host, 0);
pm_runtime_set_active(&host->card->dev);
pm_runtime_mark_last_busy(&host->card->dev);
}
diff --git a/drivers/mmc/host/sdhci_f_sdh30.c b/drivers/mmc/host/sdhci_f_sdh30.c
index 6fae509..67bcff2 100644
--- a/drivers/mmc/host/sdhci_f_sdh30.c
+++ b/drivers/mmc/host/sdhci_f_sdh30.c
@@ -30,6 +30,12 @@
#include "../core/core.h"
#define DRIVER_NAME "f_sdh30"
+#define RESUME_WAIT_COUNT 100
+#define RESUME_WAIT_TIME 50
+#define RESUME_WAIT_JIFFIES msecs_to_jiffies(RESUME_DETECT_TIME)
+#define RESUME_DETECT_COUNT 16
+#define RESUME_DETECT_TIME 50
+#define RESUME_DETECT_JIFFIES msecs_to_jiffies(RESUME_DETECT_TIME)
struct f_sdhost_priv {
@@ -38,8 +44,59 @@ struct f_sdhost_priv {
int gpio_select_1v8;
u32 vendor_hs200;
struct device *dev;
+ unsigned int quirks; /* Deviations from spec. */
+
+/* retry to detect mmc device when resume */
+#define F_SDH30_QUIRK_RESUME_DETECT_RETRY (1<<0)
+
+ struct workqueue_struct *resume_detect_wq;
+ struct delayed_work resume_detect_work;
+ unsigned int resume_detect_count;
+ unsigned int resume_wait_count;
};
+static void sdhci_f_sdh30_resume_detect_work_func(struct work_struct *work)
+{
+ struct f_sdhost_priv *priv = container_of(work, struct f_sdhost_priv,
+ resume_detect_work.work);
+ struct sdhci_host *host = dev_get_drvdata(priv->dev);
+ int err = 0;
+
+ if (mmc_bus_manual_resume(host->mmc)) {
+ pm_runtime_disable(&host->mmc->card->dev);
+ mmc_card_set_suspended(host->mmc->card);
+ err = host->mmc->bus_ops->resume(host->mmc);
+ if (priv->resume_detect_count-- && err)
+ queue_delayed_work(priv->resume_detect_wq,
+ &priv->resume_detect_work,
+ RESUME_DETECT_JIFFIES);
+ else
+ pr_debug("%s: resume detection done (count:%d, wait:%d, err:%d)\n",
+ mmc_hostname(host->mmc),
+ priv->resume_detect_count,
+ priv->resume_wait_count, err);
+ } else {
+ if (priv->resume_wait_count--)
+ queue_delayed_work(priv->resume_detect_wq,
+ &priv->resume_detect_work,
+ RESUME_WAIT_JIFFIES);
+ else
+ pr_debug("%s: resume done\n", mmc_hostname(host->mmc));
+ }
+}
+
+static void sdhci_f_sdh30_resume_detect(struct mmc_host *mmc,
+ int detect, int wait)
+{
+ struct sdhci_host *host = mmc_priv(mmc);
+ struct f_sdhost_priv *priv = sdhci_priv(host);
+
+ priv->resume_detect_count = detect;
+ priv->resume_wait_count = wait;
+ queue_delayed_work(priv->resume_detect_wq,
+ &priv->resume_detect_work, 0);
+}
+
void sdhci_f_sdh30_soft_voltage_switch(struct sdhci_host *host)
{
struct f_sdhost_priv *priv = sdhci_priv(host);
@@ -146,6 +203,12 @@ static int sdhci_f_sdh30_probe(struct platform_device *pdev)
}
}
+ if (of_find_property(pdev->dev.of_node, "resume-detect-retry", NULL)) {
+ dev_info(dev, "Applying resume detect retry quirk\n");
+ priv->quirks |= F_SDH30_QUIRK_RESUME_DETECT_RETRY;
+ host->mmc->caps2 |= MMC_CAP2_MANUAL_RESUME;
+ }
+
ret = mmc_of_parse_voltage(pdev->dev.of_node, &host->ocr_mask);
if (ret) {
dev_err(dev, "%s: parse voltage error\n", __func__);
@@ -197,6 +260,18 @@ static int sdhci_f_sdh30_probe(struct platform_device *pdev)
}
}
+ /* Init workqueue */
+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY) {
+ priv->resume_detect_wq = create_workqueue("sdhci_f_sdh30");
+ if (priv->resume_detect_wq == NULL) {
+ ret = -ENOMEM;
+ dev_err(dev, "Failed to create resume detection workqueue\n");
+ goto err_init_wq;
+ }
+ INIT_DELAYED_WORK(&priv->resume_detect_work,
+ sdhci_f_sdh30_resume_detect_work_func);
+ }
+
ret = sdhci_add_host(host);
if (ret) {
dev_err(dev, "%s: host add error\n", __func__);
@@ -229,6 +304,9 @@ static int sdhci_f_sdh30_probe(struct platform_device *pdev)
return 0;
err_add_host:
+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY)
+ destroy_workqueue(priv->resume_detect_wq);
+err_init_wq:
if (gpio_is_valid(priv->gpio_select_1v8)) {
gpio_direction_output(priv->gpio_select_1v8, 1);
gpio_free(priv->gpio_select_1v8);
@@ -268,6 +346,9 @@ static int sdhci_f_sdh30_remove(struct platform_device *pdev)
gpio_free(priv->gpio_select_1v8);
}
+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY)
+ destroy_workqueue(priv->resume_detect_wq);
+
sdhci_free_host(host);
platform_set_drvdata(pdev, NULL);
@@ -285,7 +366,15 @@ static int sdhci_f_sdh30_suspend(struct device *dev)
static int sdhci_f_sdh30_resume(struct device *dev)
{
struct sdhci_host *host = dev_get_drvdata(dev);
+ struct f_sdhost_priv *priv = sdhci_priv(host);
+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY) {
+ pr_debug("%s: start resume detect\n",
+ mmc_hostname(host->mmc));
+ sdhci_f_sdh30_resume_detect(host->mmc,
+ RESUME_DETECT_COUNT,
+ RESUME_WAIT_COUNT);
+ }
return sdhci_resume_host(host);
}
#endif
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 7960424..55221dd 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -283,6 +283,7 @@ struct mmc_host {
#define MMC_CAP2_HS400 (MMC_CAP2_HS400_1_8V | \
MMC_CAP2_HS400_1_2V)
#define MMC_CAP2_SDIO_IRQ_NOTHREAD (1 << 17)
+#define MMC_CAP2_MANUAL_RESUME (1 << 18) /* Resume manually when error */
mmc_pm_flag_t pm_caps; /* supported pm features */
@@ -338,6 +339,9 @@ struct mmc_host {
const struct mmc_bus_ops *bus_ops; /* current bus driver */
unsigned int bus_refs; /* reference counter */
+ unsigned int bus_resume_flags;
+#define MMC_BUSRESUME_MANUAL_RESUME (1 << 0)
+
unsigned int sdio_irqs;
struct task_struct *sdio_irq_thread;
bool sdio_irq_pending;
@@ -384,6 +388,16 @@ static inline void *mmc_priv(struct mmc_host *host)
#define mmc_dev(x) ((x)->parent)
#define mmc_classdev(x) (&(x)->class_dev)
#define mmc_hostname(x) (dev_name(&(x)->class_dev))
+#define mmc_bus_manual_resume(host) ((host)->bus_resume_flags & \
+ MMC_BUSRESUME_MANUAL_RESUME)
+
+static inline void mmc_set_bus_resume_policy(struct mmc_host *host, int manual)
+{
+ if (manual)
+ host->bus_resume_flags |= MMC_BUSRESUME_MANUAL_RESUME;
+ else
+ host->bus_resume_flags &= ~MMC_BUSRESUME_MANUAL_RESUME;
+}
int mmc_power_save_host(struct mmc_host *host);
int mmc_power_restore_host(struct mmc_host *host);
--
1.9.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [RFC PATCH v2 6/6] mmc: core: add manual resume capability
@ 2014-06-26 6:23 ` Vincent Yang
0 siblings, 0 replies; 30+ messages in thread
From: Vincent Yang @ 2014-06-26 6:23 UTC (permalink / raw)
To: chris, linux-mmc
Cc: devicetree, andy.green, patches, anton, Vincent.Yang,
jaswinder.singh, linuxppc-dev
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 <Vincent.Yang@tw.fujitsu.com>
---
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(+)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 764af63..51fce49 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2648,6 +2648,10 @@ int mmc_pm_notify(struct notifier_block *notify_block,
case PM_POST_RESTORE:
spin_lock_irqsave(&host->lock, flags);
+ if (mmc_bus_manual_resume(host)) {
+ spin_unlock_irqrestore(&host->lock, flags);
+ break;
+ }
host->rescan_disable = 0;
spin_unlock_irqrestore(&host->lock, flags);
_mmc_detect_change(host, 0, false);
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index 0c44510..859390d 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -1133,6 +1133,10 @@ static int mmc_sd_resume(struct mmc_host *host)
if (!(host->caps & MMC_CAP_RUNTIME_RESUME)) {
err = _mmc_sd_resume(host);
+ if ((host->caps2 & MMC_CAP2_MANUAL_RESUME) && err)
+ mmc_set_bus_resume_policy(host, 1);
+ else
+ mmc_set_bus_resume_policy(host, 0);
pm_runtime_set_active(&host->card->dev);
pm_runtime_mark_last_busy(&host->card->dev);
}
diff --git a/drivers/mmc/host/sdhci_f_sdh30.c b/drivers/mmc/host/sdhci_f_sdh30.c
index 6fae509..67bcff2 100644
--- a/drivers/mmc/host/sdhci_f_sdh30.c
+++ b/drivers/mmc/host/sdhci_f_sdh30.c
@@ -30,6 +30,12 @@
#include "../core/core.h"
#define DRIVER_NAME "f_sdh30"
+#define RESUME_WAIT_COUNT 100
+#define RESUME_WAIT_TIME 50
+#define RESUME_WAIT_JIFFIES msecs_to_jiffies(RESUME_DETECT_TIME)
+#define RESUME_DETECT_COUNT 16
+#define RESUME_DETECT_TIME 50
+#define RESUME_DETECT_JIFFIES msecs_to_jiffies(RESUME_DETECT_TIME)
struct f_sdhost_priv {
@@ -38,8 +44,59 @@ struct f_sdhost_priv {
int gpio_select_1v8;
u32 vendor_hs200;
struct device *dev;
+ unsigned int quirks; /* Deviations from spec. */
+
+/* retry to detect mmc device when resume */
+#define F_SDH30_QUIRK_RESUME_DETECT_RETRY (1<<0)
+
+ struct workqueue_struct *resume_detect_wq;
+ struct delayed_work resume_detect_work;
+ unsigned int resume_detect_count;
+ unsigned int resume_wait_count;
};
+static void sdhci_f_sdh30_resume_detect_work_func(struct work_struct *work)
+{
+ struct f_sdhost_priv *priv = container_of(work, struct f_sdhost_priv,
+ resume_detect_work.work);
+ struct sdhci_host *host = dev_get_drvdata(priv->dev);
+ int err = 0;
+
+ if (mmc_bus_manual_resume(host->mmc)) {
+ pm_runtime_disable(&host->mmc->card->dev);
+ mmc_card_set_suspended(host->mmc->card);
+ err = host->mmc->bus_ops->resume(host->mmc);
+ if (priv->resume_detect_count-- && err)
+ queue_delayed_work(priv->resume_detect_wq,
+ &priv->resume_detect_work,
+ RESUME_DETECT_JIFFIES);
+ else
+ pr_debug("%s: resume detection done (count:%d, wait:%d, err:%d)\n",
+ mmc_hostname(host->mmc),
+ priv->resume_detect_count,
+ priv->resume_wait_count, err);
+ } else {
+ if (priv->resume_wait_count--)
+ queue_delayed_work(priv->resume_detect_wq,
+ &priv->resume_detect_work,
+ RESUME_WAIT_JIFFIES);
+ else
+ pr_debug("%s: resume done\n", mmc_hostname(host->mmc));
+ }
+}
+
+static void sdhci_f_sdh30_resume_detect(struct mmc_host *mmc,
+ int detect, int wait)
+{
+ struct sdhci_host *host = mmc_priv(mmc);
+ struct f_sdhost_priv *priv = sdhci_priv(host);
+
+ priv->resume_detect_count = detect;
+ priv->resume_wait_count = wait;
+ queue_delayed_work(priv->resume_detect_wq,
+ &priv->resume_detect_work, 0);
+}
+
void sdhci_f_sdh30_soft_voltage_switch(struct sdhci_host *host)
{
struct f_sdhost_priv *priv = sdhci_priv(host);
@@ -146,6 +203,12 @@ static int sdhci_f_sdh30_probe(struct platform_device *pdev)
}
}
+ if (of_find_property(pdev->dev.of_node, "resume-detect-retry", NULL)) {
+ dev_info(dev, "Applying resume detect retry quirk\n");
+ priv->quirks |= F_SDH30_QUIRK_RESUME_DETECT_RETRY;
+ host->mmc->caps2 |= MMC_CAP2_MANUAL_RESUME;
+ }
+
ret = mmc_of_parse_voltage(pdev->dev.of_node, &host->ocr_mask);
if (ret) {
dev_err(dev, "%s: parse voltage error\n", __func__);
@@ -197,6 +260,18 @@ static int sdhci_f_sdh30_probe(struct platform_device *pdev)
}
}
+ /* Init workqueue */
+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY) {
+ priv->resume_detect_wq = create_workqueue("sdhci_f_sdh30");
+ if (priv->resume_detect_wq == NULL) {
+ ret = -ENOMEM;
+ dev_err(dev, "Failed to create resume detection workqueue\n");
+ goto err_init_wq;
+ }
+ INIT_DELAYED_WORK(&priv->resume_detect_work,
+ sdhci_f_sdh30_resume_detect_work_func);
+ }
+
ret = sdhci_add_host(host);
if (ret) {
dev_err(dev, "%s: host add error\n", __func__);
@@ -229,6 +304,9 @@ static int sdhci_f_sdh30_probe(struct platform_device *pdev)
return 0;
err_add_host:
+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY)
+ destroy_workqueue(priv->resume_detect_wq);
+err_init_wq:
if (gpio_is_valid(priv->gpio_select_1v8)) {
gpio_direction_output(priv->gpio_select_1v8, 1);
gpio_free(priv->gpio_select_1v8);
@@ -268,6 +346,9 @@ static int sdhci_f_sdh30_remove(struct platform_device *pdev)
gpio_free(priv->gpio_select_1v8);
}
+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY)
+ destroy_workqueue(priv->resume_detect_wq);
+
sdhci_free_host(host);
platform_set_drvdata(pdev, NULL);
@@ -285,7 +366,15 @@ static int sdhci_f_sdh30_suspend(struct device *dev)
static int sdhci_f_sdh30_resume(struct device *dev)
{
struct sdhci_host *host = dev_get_drvdata(dev);
+ struct f_sdhost_priv *priv = sdhci_priv(host);
+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY) {
+ pr_debug("%s: start resume detect\n",
+ mmc_hostname(host->mmc));
+ sdhci_f_sdh30_resume_detect(host->mmc,
+ RESUME_DETECT_COUNT,
+ RESUME_WAIT_COUNT);
+ }
return sdhci_resume_host(host);
}
#endif
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 7960424..55221dd 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -283,6 +283,7 @@ struct mmc_host {
#define MMC_CAP2_HS400 (MMC_CAP2_HS400_1_8V | \
MMC_CAP2_HS400_1_2V)
#define MMC_CAP2_SDIO_IRQ_NOTHREAD (1 << 17)
+#define MMC_CAP2_MANUAL_RESUME (1 << 18) /* Resume manually when error */
mmc_pm_flag_t pm_caps; /* supported pm features */
@@ -338,6 +339,9 @@ struct mmc_host {
const struct mmc_bus_ops *bus_ops; /* current bus driver */
unsigned int bus_refs; /* reference counter */
+ unsigned int bus_resume_flags;
+#define MMC_BUSRESUME_MANUAL_RESUME (1 << 0)
+
unsigned int sdio_irqs;
struct task_struct *sdio_irq_thread;
bool sdio_irq_pending;
@@ -384,6 +388,16 @@ static inline void *mmc_priv(struct mmc_host *host)
#define mmc_dev(x) ((x)->parent)
#define mmc_classdev(x) (&(x)->class_dev)
#define mmc_hostname(x) (dev_name(&(x)->class_dev))
+#define mmc_bus_manual_resume(host) ((host)->bus_resume_flags & \
+ MMC_BUSRESUME_MANUAL_RESUME)
+
+static inline void mmc_set_bus_resume_policy(struct mmc_host *host, int manual)
+{
+ if (manual)
+ host->bus_resume_flags |= MMC_BUSRESUME_MANUAL_RESUME;
+ else
+ host->bus_resume_flags &= ~MMC_BUSRESUME_MANUAL_RESUME;
+}
int mmc_power_save_host(struct mmc_host *host);
int mmc_power_restore_host(struct mmc_host *host);
--
1.9.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [RFC PATCH v2 6/6] mmc: core: add manual resume capability
2014-06-26 6:23 ` Vincent Yang
@ 2014-06-26 9:42 ` Ulf Hansson
-1 siblings, 0 replies; 30+ messages in thread
From: Ulf Hansson @ 2014-06-26 9:42 UTC (permalink / raw)
To: Vincent Yang, chris, linux-mmc
Cc: devicetree, anton, linuxppc-dev, patches, andy.green,
jaswinder.singh, Vincent.Yang
On 26 juni 2014 08:23:32 CEST, Vincent Yang <vincent.yang.fujitsu@gmail.com> 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
>
>[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 <Vincent.Yang@tw.fujitsu.com>
>---
> 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(+)
>
>diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>index 764af63..51fce49 100644
>--- a/drivers/mmc/core/core.c
>+++ b/drivers/mmc/core/core.c
>@@ -2648,6 +2648,10 @@ int mmc_pm_notify(struct notifier_block
>*notify_block,
> case PM_POST_RESTORE:
>
> spin_lock_irqsave(&host->lock, flags);
>+ if (mmc_bus_manual_resume(host)) {
>+ spin_unlock_irqrestore(&host->lock, flags);
>+ break;
>+ }
> host->rescan_disable = 0;
> spin_unlock_irqrestore(&host->lock, flags);
> _mmc_detect_change(host, 0, false);
>diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
>index 0c44510..859390d 100644
>--- a/drivers/mmc/core/sd.c
>+++ b/drivers/mmc/core/sd.c
>@@ -1133,6 +1133,10 @@ static int mmc_sd_resume(struct mmc_host *host)
>
> if (!(host->caps & MMC_CAP_RUNTIME_RESUME)) {
> err = _mmc_sd_resume(host);
>+ if ((host->caps2 & MMC_CAP2_MANUAL_RESUME) && err)
>+ mmc_set_bus_resume_policy(host, 1);
>+ else
>+ mmc_set_bus_resume_policy(host, 0);
> pm_runtime_set_active(&host->card->dev);
> pm_runtime_mark_last_busy(&host->card->dev);
> }
>diff --git a/drivers/mmc/host/sdhci_f_sdh30.c
>b/drivers/mmc/host/sdhci_f_sdh30.c
>index 6fae509..67bcff2 100644
>--- a/drivers/mmc/host/sdhci_f_sdh30.c
>+++ b/drivers/mmc/host/sdhci_f_sdh30.c
>@@ -30,6 +30,12 @@
> #include "../core/core.h"
>
> #define DRIVER_NAME "f_sdh30"
>+#define RESUME_WAIT_COUNT 100
>+#define RESUME_WAIT_TIME 50
>+#define RESUME_WAIT_JIFFIES msecs_to_jiffies(RESUME_DETECT_TIME)
>+#define RESUME_DETECT_COUNT 16
>+#define RESUME_DETECT_TIME 50
>+#define RESUME_DETECT_JIFFIES msecs_to_jiffies(RESUME_DETECT_TIME)
>
>
> struct f_sdhost_priv {
>@@ -38,8 +44,59 @@ struct f_sdhost_priv {
> int gpio_select_1v8;
> u32 vendor_hs200;
> struct device *dev;
>+ unsigned int quirks; /* Deviations from spec. */
>+
>+/* retry to detect mmc device when resume */
>+#define F_SDH30_QUIRK_RESUME_DETECT_RETRY (1<<0)
>+
>+ struct workqueue_struct *resume_detect_wq;
>+ struct delayed_work resume_detect_work;
>+ unsigned int resume_detect_count;
>+ unsigned int resume_wait_count;
> };
>
>+static void sdhci_f_sdh30_resume_detect_work_func(struct work_struct
>*work)
>+{
>+ struct f_sdhost_priv *priv = container_of(work, struct f_sdhost_priv,
>+ resume_detect_work.work);
>+ struct sdhci_host *host = dev_get_drvdata(priv->dev);
>+ int err = 0;
>+
>+ if (mmc_bus_manual_resume(host->mmc)) {
>+ pm_runtime_disable(&host->mmc->card->dev);
>+ mmc_card_set_suspended(host->mmc->card);
>+ err = host->mmc->bus_ops->resume(host->mmc);
>+ if (priv->resume_detect_count-- && err)
>+ queue_delayed_work(priv->resume_detect_wq,
>+ &priv->resume_detect_work,
>+ RESUME_DETECT_JIFFIES);
>+ else
>+ pr_debug("%s: resume detection done (count:%d, wait:%d, err:%d)\n",
>+ mmc_hostname(host->mmc),
>+ priv->resume_detect_count,
>+ priv->resume_wait_count, err);
>+ } else {
>+ if (priv->resume_wait_count--)
>+ queue_delayed_work(priv->resume_detect_wq,
>+ &priv->resume_detect_work,
>+ RESUME_WAIT_JIFFIES);
>+ else
>+ pr_debug("%s: resume done\n", mmc_hostname(host->mmc));
>+ }
>+}
>+
>+static void sdhci_f_sdh30_resume_detect(struct mmc_host *mmc,
>+ int detect, int wait)
>+{
>+ struct sdhci_host *host = mmc_priv(mmc);
>+ struct f_sdhost_priv *priv = sdhci_priv(host);
>+
>+ priv->resume_detect_count = detect;
>+ priv->resume_wait_count = wait;
>+ queue_delayed_work(priv->resume_detect_wq,
>+ &priv->resume_detect_work, 0);
>+}
>+
> void sdhci_f_sdh30_soft_voltage_switch(struct sdhci_host *host)
> {
> struct f_sdhost_priv *priv = sdhci_priv(host);
>@@ -146,6 +203,12 @@ static int sdhci_f_sdh30_probe(struct
>platform_device *pdev)
> }
> }
>
>+ if (of_find_property(pdev->dev.of_node, "resume-detect-retry", NULL))
>{
>+ dev_info(dev, "Applying resume detect retry quirk\n");
>+ priv->quirks |= F_SDH30_QUIRK_RESUME_DETECT_RETRY;
>+ host->mmc->caps2 |= MMC_CAP2_MANUAL_RESUME;
>+ }
>+
> ret = mmc_of_parse_voltage(pdev->dev.of_node, &host->ocr_mask);
> if (ret) {
> dev_err(dev, "%s: parse voltage error\n", __func__);
>@@ -197,6 +260,18 @@ static int sdhci_f_sdh30_probe(struct
>platform_device *pdev)
> }
> }
>
>+ /* Init workqueue */
>+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY) {
>+ priv->resume_detect_wq = create_workqueue("sdhci_f_sdh30");
>+ if (priv->resume_detect_wq == NULL) {
>+ ret = -ENOMEM;
>+ dev_err(dev, "Failed to create resume detection workqueue\n");
>+ goto err_init_wq;
>+ }
>+ INIT_DELAYED_WORK(&priv->resume_detect_work,
>+ sdhci_f_sdh30_resume_detect_work_func);
>+ }
>+
> ret = sdhci_add_host(host);
> if (ret) {
> dev_err(dev, "%s: host add error\n", __func__);
>@@ -229,6 +304,9 @@ static int sdhci_f_sdh30_probe(struct
>platform_device *pdev)
> return 0;
>
> err_add_host:
>+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY)
>+ destroy_workqueue(priv->resume_detect_wq);
>+err_init_wq:
> if (gpio_is_valid(priv->gpio_select_1v8)) {
> gpio_direction_output(priv->gpio_select_1v8, 1);
> gpio_free(priv->gpio_select_1v8);
>@@ -268,6 +346,9 @@ static int sdhci_f_sdh30_remove(struct
>platform_device *pdev)
> gpio_free(priv->gpio_select_1v8);
> }
>
>+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY)
>+ destroy_workqueue(priv->resume_detect_wq);
>+
> sdhci_free_host(host);
> platform_set_drvdata(pdev, NULL);
>
>@@ -285,7 +366,15 @@ static int sdhci_f_sdh30_suspend(struct device
>*dev)
> static int sdhci_f_sdh30_resume(struct device *dev)
> {
> struct sdhci_host *host = dev_get_drvdata(dev);
>+ struct f_sdhost_priv *priv = sdhci_priv(host);
>
>+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY) {
>+ pr_debug("%s: start resume detect\n",
>+ mmc_hostname(host->mmc));
>+ sdhci_f_sdh30_resume_detect(host->mmc,
>+ RESUME_DETECT_COUNT,
>+ RESUME_WAIT_COUNT);
>+ }
> return sdhci_resume_host(host);
> }
> #endif
>diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>index 7960424..55221dd 100644
>--- a/include/linux/mmc/host.h
>+++ b/include/linux/mmc/host.h
>@@ -283,6 +283,7 @@ struct mmc_host {
> #define MMC_CAP2_HS400 (MMC_CAP2_HS400_1_8V | \
> MMC_CAP2_HS400_1_2V)
> #define MMC_CAP2_SDIO_IRQ_NOTHREAD (1 << 17)
>+#define MMC_CAP2_MANUAL_RESUME (1 << 18) /* Resume manually when
>error */
>
> mmc_pm_flag_t pm_caps; /* supported pm features */
>
>@@ -338,6 +339,9 @@ struct mmc_host {
> const struct mmc_bus_ops *bus_ops; /* current bus driver */
> unsigned int bus_refs; /* reference counter */
>
>+ unsigned int bus_resume_flags;
>+#define MMC_BUSRESUME_MANUAL_RESUME (1 << 0)
>+
> unsigned int sdio_irqs;
> struct task_struct *sdio_irq_thread;
> bool sdio_irq_pending;
>@@ -384,6 +388,16 @@ static inline void *mmc_priv(struct mmc_host
>*host)
> #define mmc_dev(x) ((x)->parent)
> #define mmc_classdev(x) (&(x)->class_dev)
> #define mmc_hostname(x) (dev_name(&(x)->class_dev))
>+#define mmc_bus_manual_resume(host) ((host)->bus_resume_flags & \
>+ MMC_BUSRESUME_MANUAL_RESUME)
>+
>+static inline void mmc_set_bus_resume_policy(struct mmc_host *host,
>int manual)
>+{
>+ if (manual)
>+ host->bus_resume_flags |= MMC_BUSRESUME_MANUAL_RESUME;
>+ else
>+ host->bus_resume_flags &= ~MMC_BUSRESUME_MANUAL_RESUME;
>+}
>
> int mmc_power_save_host(struct mmc_host *host);
> int mmc_power_restore_host(struct mmc_host *host);
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [RFC PATCH v2 6/6] mmc: core: add manual resume capability
@ 2014-06-26 9:42 ` Ulf Hansson
0 siblings, 0 replies; 30+ messages in thread
From: Ulf Hansson @ 2014-06-26 9:42 UTC (permalink / raw)
To: Vincent Yang, chris, linux-mmc
Cc: devicetree, andy.green, patches, anton, Vincent.Yang,
jaswinder.singh, linuxppc-dev
On 26 juni 2014 08:23:32 CEST, Vincent Yang <vincent.yang.fujitsu@gmail.com> 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
>
>[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 <Vincent.Yang@tw.fujitsu.com>
>---
> 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(+)
>
>diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>index 764af63..51fce49 100644
>--- a/drivers/mmc/core/core.c
>+++ b/drivers/mmc/core/core.c
>@@ -2648,6 +2648,10 @@ int mmc_pm_notify(struct notifier_block
>*notify_block,
> case PM_POST_RESTORE:
>
> spin_lock_irqsave(&host->lock, flags);
>+ if (mmc_bus_manual_resume(host)) {
>+ spin_unlock_irqrestore(&host->lock, flags);
>+ break;
>+ }
> host->rescan_disable = 0;
> spin_unlock_irqrestore(&host->lock, flags);
> _mmc_detect_change(host, 0, false);
>diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
>index 0c44510..859390d 100644
>--- a/drivers/mmc/core/sd.c
>+++ b/drivers/mmc/core/sd.c
>@@ -1133,6 +1133,10 @@ static int mmc_sd_resume(struct mmc_host *host)
>
> if (!(host->caps & MMC_CAP_RUNTIME_RESUME)) {
> err = _mmc_sd_resume(host);
>+ if ((host->caps2 & MMC_CAP2_MANUAL_RESUME) && err)
>+ mmc_set_bus_resume_policy(host, 1);
>+ else
>+ mmc_set_bus_resume_policy(host, 0);
> pm_runtime_set_active(&host->card->dev);
> pm_runtime_mark_last_busy(&host->card->dev);
> }
>diff --git a/drivers/mmc/host/sdhci_f_sdh30.c
>b/drivers/mmc/host/sdhci_f_sdh30.c
>index 6fae509..67bcff2 100644
>--- a/drivers/mmc/host/sdhci_f_sdh30.c
>+++ b/drivers/mmc/host/sdhci_f_sdh30.c
>@@ -30,6 +30,12 @@
> #include "../core/core.h"
>
> #define DRIVER_NAME "f_sdh30"
>+#define RESUME_WAIT_COUNT 100
>+#define RESUME_WAIT_TIME 50
>+#define RESUME_WAIT_JIFFIES msecs_to_jiffies(RESUME_DETECT_TIME)
>+#define RESUME_DETECT_COUNT 16
>+#define RESUME_DETECT_TIME 50
>+#define RESUME_DETECT_JIFFIES msecs_to_jiffies(RESUME_DETECT_TIME)
>
>
> struct f_sdhost_priv {
>@@ -38,8 +44,59 @@ struct f_sdhost_priv {
> int gpio_select_1v8;
> u32 vendor_hs200;
> struct device *dev;
>+ unsigned int quirks; /* Deviations from spec. */
>+
>+/* retry to detect mmc device when resume */
>+#define F_SDH30_QUIRK_RESUME_DETECT_RETRY (1<<0)
>+
>+ struct workqueue_struct *resume_detect_wq;
>+ struct delayed_work resume_detect_work;
>+ unsigned int resume_detect_count;
>+ unsigned int resume_wait_count;
> };
>
>+static void sdhci_f_sdh30_resume_detect_work_func(struct work_struct
>*work)
>+{
>+ struct f_sdhost_priv *priv = container_of(work, struct f_sdhost_priv,
>+ resume_detect_work.work);
>+ struct sdhci_host *host = dev_get_drvdata(priv->dev);
>+ int err = 0;
>+
>+ if (mmc_bus_manual_resume(host->mmc)) {
>+ pm_runtime_disable(&host->mmc->card->dev);
>+ mmc_card_set_suspended(host->mmc->card);
>+ err = host->mmc->bus_ops->resume(host->mmc);
>+ if (priv->resume_detect_count-- && err)
>+ queue_delayed_work(priv->resume_detect_wq,
>+ &priv->resume_detect_work,
>+ RESUME_DETECT_JIFFIES);
>+ else
>+ pr_debug("%s: resume detection done (count:%d, wait:%d, err:%d)\n",
>+ mmc_hostname(host->mmc),
>+ priv->resume_detect_count,
>+ priv->resume_wait_count, err);
>+ } else {
>+ if (priv->resume_wait_count--)
>+ queue_delayed_work(priv->resume_detect_wq,
>+ &priv->resume_detect_work,
>+ RESUME_WAIT_JIFFIES);
>+ else
>+ pr_debug("%s: resume done\n", mmc_hostname(host->mmc));
>+ }
>+}
>+
>+static void sdhci_f_sdh30_resume_detect(struct mmc_host *mmc,
>+ int detect, int wait)
>+{
>+ struct sdhci_host *host = mmc_priv(mmc);
>+ struct f_sdhost_priv *priv = sdhci_priv(host);
>+
>+ priv->resume_detect_count = detect;
>+ priv->resume_wait_count = wait;
>+ queue_delayed_work(priv->resume_detect_wq,
>+ &priv->resume_detect_work, 0);
>+}
>+
> void sdhci_f_sdh30_soft_voltage_switch(struct sdhci_host *host)
> {
> struct f_sdhost_priv *priv = sdhci_priv(host);
>@@ -146,6 +203,12 @@ static int sdhci_f_sdh30_probe(struct
>platform_device *pdev)
> }
> }
>
>+ if (of_find_property(pdev->dev.of_node, "resume-detect-retry", NULL))
>{
>+ dev_info(dev, "Applying resume detect retry quirk\n");
>+ priv->quirks |= F_SDH30_QUIRK_RESUME_DETECT_RETRY;
>+ host->mmc->caps2 |= MMC_CAP2_MANUAL_RESUME;
>+ }
>+
> ret = mmc_of_parse_voltage(pdev->dev.of_node, &host->ocr_mask);
> if (ret) {
> dev_err(dev, "%s: parse voltage error\n", __func__);
>@@ -197,6 +260,18 @@ static int sdhci_f_sdh30_probe(struct
>platform_device *pdev)
> }
> }
>
>+ /* Init workqueue */
>+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY) {
>+ priv->resume_detect_wq = create_workqueue("sdhci_f_sdh30");
>+ if (priv->resume_detect_wq == NULL) {
>+ ret = -ENOMEM;
>+ dev_err(dev, "Failed to create resume detection workqueue\n");
>+ goto err_init_wq;
>+ }
>+ INIT_DELAYED_WORK(&priv->resume_detect_work,
>+ sdhci_f_sdh30_resume_detect_work_func);
>+ }
>+
> ret = sdhci_add_host(host);
> if (ret) {
> dev_err(dev, "%s: host add error\n", __func__);
>@@ -229,6 +304,9 @@ static int sdhci_f_sdh30_probe(struct
>platform_device *pdev)
> return 0;
>
> err_add_host:
>+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY)
>+ destroy_workqueue(priv->resume_detect_wq);
>+err_init_wq:
> if (gpio_is_valid(priv->gpio_select_1v8)) {
> gpio_direction_output(priv->gpio_select_1v8, 1);
> gpio_free(priv->gpio_select_1v8);
>@@ -268,6 +346,9 @@ static int sdhci_f_sdh30_remove(struct
>platform_device *pdev)
> gpio_free(priv->gpio_select_1v8);
> }
>
>+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY)
>+ destroy_workqueue(priv->resume_detect_wq);
>+
> sdhci_free_host(host);
> platform_set_drvdata(pdev, NULL);
>
>@@ -285,7 +366,15 @@ static int sdhci_f_sdh30_suspend(struct device
>*dev)
> static int sdhci_f_sdh30_resume(struct device *dev)
> {
> struct sdhci_host *host = dev_get_drvdata(dev);
>+ struct f_sdhost_priv *priv = sdhci_priv(host);
>
>+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY) {
>+ pr_debug("%s: start resume detect\n",
>+ mmc_hostname(host->mmc));
>+ sdhci_f_sdh30_resume_detect(host->mmc,
>+ RESUME_DETECT_COUNT,
>+ RESUME_WAIT_COUNT);
>+ }
> return sdhci_resume_host(host);
> }
> #endif
>diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>index 7960424..55221dd 100644
>--- a/include/linux/mmc/host.h
>+++ b/include/linux/mmc/host.h
>@@ -283,6 +283,7 @@ struct mmc_host {
> #define MMC_CAP2_HS400 (MMC_CAP2_HS400_1_8V | \
> MMC_CAP2_HS400_1_2V)
> #define MMC_CAP2_SDIO_IRQ_NOTHREAD (1 << 17)
>+#define MMC_CAP2_MANUAL_RESUME (1 << 18) /* Resume manually when
>error */
>
> mmc_pm_flag_t pm_caps; /* supported pm features */
>
>@@ -338,6 +339,9 @@ struct mmc_host {
> const struct mmc_bus_ops *bus_ops; /* current bus driver */
> unsigned int bus_refs; /* reference counter */
>
>+ unsigned int bus_resume_flags;
>+#define MMC_BUSRESUME_MANUAL_RESUME (1 << 0)
>+
> unsigned int sdio_irqs;
> struct task_struct *sdio_irq_thread;
> bool sdio_irq_pending;
>@@ -384,6 +388,16 @@ static inline void *mmc_priv(struct mmc_host
>*host)
> #define mmc_dev(x) ((x)->parent)
> #define mmc_classdev(x) (&(x)->class_dev)
> #define mmc_hostname(x) (dev_name(&(x)->class_dev))
>+#define mmc_bus_manual_resume(host) ((host)->bus_resume_flags & \
>+ MMC_BUSRESUME_MANUAL_RESUME)
>+
>+static inline void mmc_set_bus_resume_policy(struct mmc_host *host,
>int manual)
>+{
>+ if (manual)
>+ host->bus_resume_flags |= MMC_BUSRESUME_MANUAL_RESUME;
>+ else
>+ host->bus_resume_flags &= ~MMC_BUSRESUME_MANUAL_RESUME;
>+}
>
> int mmc_power_save_host(struct mmc_host *host);
> int mmc_power_restore_host(struct mmc_host *host);
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [RFC PATCH v2 6/6] mmc: core: add manual resume capability
2014-06-26 9:42 ` Ulf Hansson
@ 2014-06-27 2:23 ` Vincent Yang
-1 siblings, 0 replies; 30+ messages in thread
From: Vincent Yang @ 2014-06-27 2:23 UTC (permalink / raw)
To: Ulf Hansson
Cc: chris, linux-mmc, devicetree, Anton Vorontsov, linuxppc-dev,
patches, Andy Green, Jaswinder Singh, Vincent Yang
2014-06-26 17:42 GMT+08:00 Ulf Hansson <ulf.hansson@linaro.org>:
>
>
> On 26 juni 2014 08:23:32 CEST, Vincent Yang <vincent.yang.fujitsu@gmail.com> 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.
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:
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 169e17d..ed28896 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1283,7 +1283,7 @@ static void sdhci_set_power(struct sdhci_host
*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
>
>>
>>[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 <Vincent.Yang@tw.fujitsu.com>
>>---
>> 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(+)
>>
>>diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>index 764af63..51fce49 100644
>>--- a/drivers/mmc/core/core.c
>>+++ b/drivers/mmc/core/core.c
>>@@ -2648,6 +2648,10 @@ int mmc_pm_notify(struct notifier_block
>>*notify_block,
>> case PM_POST_RESTORE:
>>
>> spin_lock_irqsave(&host->lock, flags);
>>+ if (mmc_bus_manual_resume(host)) {
>>+ spin_unlock_irqrestore(&host->lock, flags);
>>+ break;
>>+ }
>> host->rescan_disable = 0;
>> spin_unlock_irqrestore(&host->lock, flags);
>> _mmc_detect_change(host, 0, false);
>>diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
>>index 0c44510..859390d 100644
>>--- a/drivers/mmc/core/sd.c
>>+++ b/drivers/mmc/core/sd.c
>>@@ -1133,6 +1133,10 @@ static int mmc_sd_resume(struct mmc_host *host)
>>
>> if (!(host->caps & MMC_CAP_RUNTIME_RESUME)) {
>> err = _mmc_sd_resume(host);
>>+ if ((host->caps2 & MMC_CAP2_MANUAL_RESUME) && err)
>>+ mmc_set_bus_resume_policy(host, 1);
>>+ else
>>+ mmc_set_bus_resume_policy(host, 0);
>> pm_runtime_set_active(&host->card->dev);
>> pm_runtime_mark_last_busy(&host->card->dev);
>> }
>>diff --git a/drivers/mmc/host/sdhci_f_sdh30.c
>>b/drivers/mmc/host/sdhci_f_sdh30.c
>>index 6fae509..67bcff2 100644
>>--- a/drivers/mmc/host/sdhci_f_sdh30.c
>>+++ b/drivers/mmc/host/sdhci_f_sdh30.c
>>@@ -30,6 +30,12 @@
>> #include "../core/core.h"
>>
>> #define DRIVER_NAME "f_sdh30"
>>+#define RESUME_WAIT_COUNT 100
>>+#define RESUME_WAIT_TIME 50
>>+#define RESUME_WAIT_JIFFIES msecs_to_jiffies(RESUME_DETECT_TIME)
>>+#define RESUME_DETECT_COUNT 16
>>+#define RESUME_DETECT_TIME 50
>>+#define RESUME_DETECT_JIFFIES msecs_to_jiffies(RESUME_DETECT_TIME)
>>
>>
>> struct f_sdhost_priv {
>>@@ -38,8 +44,59 @@ struct f_sdhost_priv {
>> int gpio_select_1v8;
>> u32 vendor_hs200;
>> struct device *dev;
>>+ unsigned int quirks; /* Deviations from spec. */
>>+
>>+/* retry to detect mmc device when resume */
>>+#define F_SDH30_QUIRK_RESUME_DETECT_RETRY (1<<0)
>>+
>>+ struct workqueue_struct *resume_detect_wq;
>>+ struct delayed_work resume_detect_work;
>>+ unsigned int resume_detect_count;
>>+ unsigned int resume_wait_count;
>> };
>>
>>+static void sdhci_f_sdh30_resume_detect_work_func(struct work_struct
>>*work)
>>+{
>>+ struct f_sdhost_priv *priv = container_of(work, struct f_sdhost_priv,
>>+ resume_detect_work.work);
>>+ struct sdhci_host *host = dev_get_drvdata(priv->dev);
>>+ int err = 0;
>>+
>>+ if (mmc_bus_manual_resume(host->mmc)) {
>>+ pm_runtime_disable(&host->mmc->card->dev);
>>+ mmc_card_set_suspended(host->mmc->card);
>>+ err = host->mmc->bus_ops->resume(host->mmc);
>>+ if (priv->resume_detect_count-- && err)
>>+ queue_delayed_work(priv->resume_detect_wq,
>>+ &priv->resume_detect_work,
>>+ RESUME_DETECT_JIFFIES);
>>+ else
>>+ pr_debug("%s: resume detection done (count:%d, wait:%d, err:%d)\n",
>>+ mmc_hostname(host->mmc),
>>+ priv->resume_detect_count,
>>+ priv->resume_wait_count, err);
>>+ } else {
>>+ if (priv->resume_wait_count--)
>>+ queue_delayed_work(priv->resume_detect_wq,
>>+ &priv->resume_detect_work,
>>+ RESUME_WAIT_JIFFIES);
>>+ else
>>+ pr_debug("%s: resume done\n", mmc_hostname(host->mmc));
>>+ }
>>+}
>>+
>>+static void sdhci_f_sdh30_resume_detect(struct mmc_host *mmc,
>>+ int detect, int wait)
>>+{
>>+ struct sdhci_host *host = mmc_priv(mmc);
>>+ struct f_sdhost_priv *priv = sdhci_priv(host);
>>+
>>+ priv->resume_detect_count = detect;
>>+ priv->resume_wait_count = wait;
>>+ queue_delayed_work(priv->resume_detect_wq,
>>+ &priv->resume_detect_work, 0);
>>+}
>>+
>> void sdhci_f_sdh30_soft_voltage_switch(struct sdhci_host *host)
>> {
>> struct f_sdhost_priv *priv = sdhci_priv(host);
>>@@ -146,6 +203,12 @@ static int sdhci_f_sdh30_probe(struct
>>platform_device *pdev)
>> }
>> }
>>
>>+ if (of_find_property(pdev->dev.of_node, "resume-detect-retry", NULL))
>>{
>>+ dev_info(dev, "Applying resume detect retry quirk\n");
>>+ priv->quirks |= F_SDH30_QUIRK_RESUME_DETECT_RETRY;
>>+ host->mmc->caps2 |= MMC_CAP2_MANUAL_RESUME;
>>+ }
>>+
>> ret = mmc_of_parse_voltage(pdev->dev.of_node, &host->ocr_mask);
>> if (ret) {
>> dev_err(dev, "%s: parse voltage error\n", __func__);
>>@@ -197,6 +260,18 @@ static int sdhci_f_sdh30_probe(struct
>>platform_device *pdev)
>> }
>> }
>>
>>+ /* Init workqueue */
>>+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY) {
>>+ priv->resume_detect_wq = create_workqueue("sdhci_f_sdh30");
>>+ if (priv->resume_detect_wq == NULL) {
>>+ ret = -ENOMEM;
>>+ dev_err(dev, "Failed to create resume detection workqueue\n");
>>+ goto err_init_wq;
>>+ }
>>+ INIT_DELAYED_WORK(&priv->resume_detect_work,
>>+ sdhci_f_sdh30_resume_detect_work_func);
>>+ }
>>+
>> ret = sdhci_add_host(host);
>> if (ret) {
>> dev_err(dev, "%s: host add error\n", __func__);
>>@@ -229,6 +304,9 @@ static int sdhci_f_sdh30_probe(struct
>>platform_device *pdev)
>> return 0;
>>
>> err_add_host:
>>+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY)
>>+ destroy_workqueue(priv->resume_detect_wq);
>>+err_init_wq:
>> if (gpio_is_valid(priv->gpio_select_1v8)) {
>> gpio_direction_output(priv->gpio_select_1v8, 1);
>> gpio_free(priv->gpio_select_1v8);
>>@@ -268,6 +346,9 @@ static int sdhci_f_sdh30_remove(struct
>>platform_device *pdev)
>> gpio_free(priv->gpio_select_1v8);
>> }
>>
>>+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY)
>>+ destroy_workqueue(priv->resume_detect_wq);
>>+
>> sdhci_free_host(host);
>> platform_set_drvdata(pdev, NULL);
>>
>>@@ -285,7 +366,15 @@ static int sdhci_f_sdh30_suspend(struct device
>>*dev)
>> static int sdhci_f_sdh30_resume(struct device *dev)
>> {
>> struct sdhci_host *host = dev_get_drvdata(dev);
>>+ struct f_sdhost_priv *priv = sdhci_priv(host);
>>
>>+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY) {
>>+ pr_debug("%s: start resume detect\n",
>>+ mmc_hostname(host->mmc));
>>+ sdhci_f_sdh30_resume_detect(host->mmc,
>>+ RESUME_DETECT_COUNT,
>>+ RESUME_WAIT_COUNT);
>>+ }
>> return sdhci_resume_host(host);
>> }
>> #endif
>>diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>>index 7960424..55221dd 100644
>>--- a/include/linux/mmc/host.h
>>+++ b/include/linux/mmc/host.h
>>@@ -283,6 +283,7 @@ struct mmc_host {
>> #define MMC_CAP2_HS400 (MMC_CAP2_HS400_1_8V | \
>> MMC_CAP2_HS400_1_2V)
>> #define MMC_CAP2_SDIO_IRQ_NOTHREAD (1 << 17)
>>+#define MMC_CAP2_MANUAL_RESUME (1 << 18) /* Resume manually when
>>error */
>>
>> mmc_pm_flag_t pm_caps; /* supported pm features */
>>
>>@@ -338,6 +339,9 @@ struct mmc_host {
>> const struct mmc_bus_ops *bus_ops; /* current bus driver */
>> unsigned int bus_refs; /* reference counter */
>>
>>+ unsigned int bus_resume_flags;
>>+#define MMC_BUSRESUME_MANUAL_RESUME (1 << 0)
>>+
>> unsigned int sdio_irqs;
>> struct task_struct *sdio_irq_thread;
>> bool sdio_irq_pending;
>>@@ -384,6 +388,16 @@ static inline void *mmc_priv(struct mmc_host
>>*host)
>> #define mmc_dev(x) ((x)->parent)
>> #define mmc_classdev(x) (&(x)->class_dev)
>> #define mmc_hostname(x) (dev_name(&(x)->class_dev))
>>+#define mmc_bus_manual_resume(host) ((host)->bus_resume_flags & \
>>+ MMC_BUSRESUME_MANUAL_RESUME)
>>+
>>+static inline void mmc_set_bus_resume_policy(struct mmc_host *host,
>>int manual)
>>+{
>>+ if (manual)
>>+ host->bus_resume_flags |= MMC_BUSRESUME_MANUAL_RESUME;
>>+ else
>>+ host->bus_resume_flags &= ~MMC_BUSRESUME_MANUAL_RESUME;
>>+}
>>
>> int mmc_power_save_host(struct mmc_host *host);
>> int mmc_power_restore_host(struct mmc_host *host);
>
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [RFC PATCH v2 6/6] mmc: core: add manual resume capability
@ 2014-06-27 2:23 ` Vincent Yang
0 siblings, 0 replies; 30+ messages in thread
From: Vincent Yang @ 2014-06-27 2:23 UTC (permalink / raw)
To: Ulf Hansson
Cc: devicetree, Andy Green, patches, Anton Vorontsov, linux-mmc,
chris, Vincent Yang, Jaswinder Singh, linuxppc-dev
2014-06-26 17:42 GMT+08:00 Ulf Hansson <ulf.hansson@linaro.org>:
>
>
> On 26 juni 2014 08:23:32 CEST, Vincent Yang <vincent.yang.fujitsu@gmail.com> 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.
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:
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 169e17d..ed28896 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1283,7 +1283,7 @@ static void sdhci_set_power(struct sdhci_host
*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
>
>>
>>[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 <Vincent.Yang@tw.fujitsu.com>
>>---
>> 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(+)
>>
>>diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>index 764af63..51fce49 100644
>>--- a/drivers/mmc/core/core.c
>>+++ b/drivers/mmc/core/core.c
>>@@ -2648,6 +2648,10 @@ int mmc_pm_notify(struct notifier_block
>>*notify_block,
>> case PM_POST_RESTORE:
>>
>> spin_lock_irqsave(&host->lock, flags);
>>+ if (mmc_bus_manual_resume(host)) {
>>+ spin_unlock_irqrestore(&host->lock, flags);
>>+ break;
>>+ }
>> host->rescan_disable = 0;
>> spin_unlock_irqrestore(&host->lock, flags);
>> _mmc_detect_change(host, 0, false);
>>diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
>>index 0c44510..859390d 100644
>>--- a/drivers/mmc/core/sd.c
>>+++ b/drivers/mmc/core/sd.c
>>@@ -1133,6 +1133,10 @@ static int mmc_sd_resume(struct mmc_host *host)
>>
>> if (!(host->caps & MMC_CAP_RUNTIME_RESUME)) {
>> err = _mmc_sd_resume(host);
>>+ if ((host->caps2 & MMC_CAP2_MANUAL_RESUME) && err)
>>+ mmc_set_bus_resume_policy(host, 1);
>>+ else
>>+ mmc_set_bus_resume_policy(host, 0);
>> pm_runtime_set_active(&host->card->dev);
>> pm_runtime_mark_last_busy(&host->card->dev);
>> }
>>diff --git a/drivers/mmc/host/sdhci_f_sdh30.c
>>b/drivers/mmc/host/sdhci_f_sdh30.c
>>index 6fae509..67bcff2 100644
>>--- a/drivers/mmc/host/sdhci_f_sdh30.c
>>+++ b/drivers/mmc/host/sdhci_f_sdh30.c
>>@@ -30,6 +30,12 @@
>> #include "../core/core.h"
>>
>> #define DRIVER_NAME "f_sdh30"
>>+#define RESUME_WAIT_COUNT 100
>>+#define RESUME_WAIT_TIME 50
>>+#define RESUME_WAIT_JIFFIES msecs_to_jiffies(RESUME_DETECT_TIME)
>>+#define RESUME_DETECT_COUNT 16
>>+#define RESUME_DETECT_TIME 50
>>+#define RESUME_DETECT_JIFFIES msecs_to_jiffies(RESUME_DETECT_TIME)
>>
>>
>> struct f_sdhost_priv {
>>@@ -38,8 +44,59 @@ struct f_sdhost_priv {
>> int gpio_select_1v8;
>> u32 vendor_hs200;
>> struct device *dev;
>>+ unsigned int quirks; /* Deviations from spec. */
>>+
>>+/* retry to detect mmc device when resume */
>>+#define F_SDH30_QUIRK_RESUME_DETECT_RETRY (1<<0)
>>+
>>+ struct workqueue_struct *resume_detect_wq;
>>+ struct delayed_work resume_detect_work;
>>+ unsigned int resume_detect_count;
>>+ unsigned int resume_wait_count;
>> };
>>
>>+static void sdhci_f_sdh30_resume_detect_work_func(struct work_struct
>>*work)
>>+{
>>+ struct f_sdhost_priv *priv = container_of(work, struct f_sdhost_priv,
>>+ resume_detect_work.work);
>>+ struct sdhci_host *host = dev_get_drvdata(priv->dev);
>>+ int err = 0;
>>+
>>+ if (mmc_bus_manual_resume(host->mmc)) {
>>+ pm_runtime_disable(&host->mmc->card->dev);
>>+ mmc_card_set_suspended(host->mmc->card);
>>+ err = host->mmc->bus_ops->resume(host->mmc);
>>+ if (priv->resume_detect_count-- && err)
>>+ queue_delayed_work(priv->resume_detect_wq,
>>+ &priv->resume_detect_work,
>>+ RESUME_DETECT_JIFFIES);
>>+ else
>>+ pr_debug("%s: resume detection done (count:%d, wait:%d, err:%d)\n",
>>+ mmc_hostname(host->mmc),
>>+ priv->resume_detect_count,
>>+ priv->resume_wait_count, err);
>>+ } else {
>>+ if (priv->resume_wait_count--)
>>+ queue_delayed_work(priv->resume_detect_wq,
>>+ &priv->resume_detect_work,
>>+ RESUME_WAIT_JIFFIES);
>>+ else
>>+ pr_debug("%s: resume done\n", mmc_hostname(host->mmc));
>>+ }
>>+}
>>+
>>+static void sdhci_f_sdh30_resume_detect(struct mmc_host *mmc,
>>+ int detect, int wait)
>>+{
>>+ struct sdhci_host *host = mmc_priv(mmc);
>>+ struct f_sdhost_priv *priv = sdhci_priv(host);
>>+
>>+ priv->resume_detect_count = detect;
>>+ priv->resume_wait_count = wait;
>>+ queue_delayed_work(priv->resume_detect_wq,
>>+ &priv->resume_detect_work, 0);
>>+}
>>+
>> void sdhci_f_sdh30_soft_voltage_switch(struct sdhci_host *host)
>> {
>> struct f_sdhost_priv *priv = sdhci_priv(host);
>>@@ -146,6 +203,12 @@ static int sdhci_f_sdh30_probe(struct
>>platform_device *pdev)
>> }
>> }
>>
>>+ if (of_find_property(pdev->dev.of_node, "resume-detect-retry", NULL))
>>{
>>+ dev_info(dev, "Applying resume detect retry quirk\n");
>>+ priv->quirks |= F_SDH30_QUIRK_RESUME_DETECT_RETRY;
>>+ host->mmc->caps2 |= MMC_CAP2_MANUAL_RESUME;
>>+ }
>>+
>> ret = mmc_of_parse_voltage(pdev->dev.of_node, &host->ocr_mask);
>> if (ret) {
>> dev_err(dev, "%s: parse voltage error\n", __func__);
>>@@ -197,6 +260,18 @@ static int sdhci_f_sdh30_probe(struct
>>platform_device *pdev)
>> }
>> }
>>
>>+ /* Init workqueue */
>>+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY) {
>>+ priv->resume_detect_wq = create_workqueue("sdhci_f_sdh30");
>>+ if (priv->resume_detect_wq == NULL) {
>>+ ret = -ENOMEM;
>>+ dev_err(dev, "Failed to create resume detection workqueue\n");
>>+ goto err_init_wq;
>>+ }
>>+ INIT_DELAYED_WORK(&priv->resume_detect_work,
>>+ sdhci_f_sdh30_resume_detect_work_func);
>>+ }
>>+
>> ret = sdhci_add_host(host);
>> if (ret) {
>> dev_err(dev, "%s: host add error\n", __func__);
>>@@ -229,6 +304,9 @@ static int sdhci_f_sdh30_probe(struct
>>platform_device *pdev)
>> return 0;
>>
>> err_add_host:
>>+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY)
>>+ destroy_workqueue(priv->resume_detect_wq);
>>+err_init_wq:
>> if (gpio_is_valid(priv->gpio_select_1v8)) {
>> gpio_direction_output(priv->gpio_select_1v8, 1);
>> gpio_free(priv->gpio_select_1v8);
>>@@ -268,6 +346,9 @@ static int sdhci_f_sdh30_remove(struct
>>platform_device *pdev)
>> gpio_free(priv->gpio_select_1v8);
>> }
>>
>>+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY)
>>+ destroy_workqueue(priv->resume_detect_wq);
>>+
>> sdhci_free_host(host);
>> platform_set_drvdata(pdev, NULL);
>>
>>@@ -285,7 +366,15 @@ static int sdhci_f_sdh30_suspend(struct device
>>*dev)
>> static int sdhci_f_sdh30_resume(struct device *dev)
>> {
>> struct sdhci_host *host = dev_get_drvdata(dev);
>>+ struct f_sdhost_priv *priv = sdhci_priv(host);
>>
>>+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY) {
>>+ pr_debug("%s: start resume detect\n",
>>+ mmc_hostname(host->mmc));
>>+ sdhci_f_sdh30_resume_detect(host->mmc,
>>+ RESUME_DETECT_COUNT,
>>+ RESUME_WAIT_COUNT);
>>+ }
>> return sdhci_resume_host(host);
>> }
>> #endif
>>diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>>index 7960424..55221dd 100644
>>--- a/include/linux/mmc/host.h
>>+++ b/include/linux/mmc/host.h
>>@@ -283,6 +283,7 @@ struct mmc_host {
>> #define MMC_CAP2_HS400 (MMC_CAP2_HS400_1_8V | \
>> MMC_CAP2_HS400_1_2V)
>> #define MMC_CAP2_SDIO_IRQ_NOTHREAD (1 << 17)
>>+#define MMC_CAP2_MANUAL_RESUME (1 << 18) /* Resume manually when
>>error */
>>
>> mmc_pm_flag_t pm_caps; /* supported pm features */
>>
>>@@ -338,6 +339,9 @@ struct mmc_host {
>> const struct mmc_bus_ops *bus_ops; /* current bus driver */
>> unsigned int bus_refs; /* reference counter */
>>
>>+ unsigned int bus_resume_flags;
>>+#define MMC_BUSRESUME_MANUAL_RESUME (1 << 0)
>>+
>> unsigned int sdio_irqs;
>> struct task_struct *sdio_irq_thread;
>> bool sdio_irq_pending;
>>@@ -384,6 +388,16 @@ static inline void *mmc_priv(struct mmc_host
>>*host)
>> #define mmc_dev(x) ((x)->parent)
>> #define mmc_classdev(x) (&(x)->class_dev)
>> #define mmc_hostname(x) (dev_name(&(x)->class_dev))
>>+#define mmc_bus_manual_resume(host) ((host)->bus_resume_flags & \
>>+ MMC_BUSRESUME_MANUAL_RESUME)
>>+
>>+static inline void mmc_set_bus_resume_policy(struct mmc_host *host,
>>int manual)
>>+{
>>+ if (manual)
>>+ host->bus_resume_flags |= MMC_BUSRESUME_MANUAL_RESUME;
>>+ else
>>+ host->bus_resume_flags &= ~MMC_BUSRESUME_MANUAL_RESUME;
>>+}
>>
>> int mmc_power_save_host(struct mmc_host *host);
>> int mmc_power_restore_host(struct mmc_host *host);
>
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [RFC PATCH v2 6/6] mmc: core: add manual resume capability
2014-06-27 2:23 ` Vincent Yang
@ 2014-06-27 9:40 ` Ulf Hansson
-1 siblings, 0 replies; 30+ messages in thread
From: Ulf Hansson @ 2014-06-27 9:40 UTC (permalink / raw)
To: Vincent Yang
Cc: chris, linux-mmc, devicetree, Anton Vorontsov, linuxppc-dev,
patches, Andy Green, Jaswinder Singh, Vincent Yang
On 27 juni 2014 04:23:42 CEST, Vincent Yang <vincent.yang.fujitsu@gmail.com> wrote:
>2014-06-26 17:42 GMT+08:00 Ulf Hansson <ulf.hansson@linaro.org>:
>>
>>
>> On 26 juni 2014 08:23:32 CEST, Vincent Yang
><vincent.yang.fujitsu@gmail.com> 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.
>
>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:
>
>diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>index 169e17d..ed28896 100644
>--- a/drivers/mmc/host/sdhci.c
>+++ b/drivers/mmc/host/sdhci.c
>@@ -1283,7 +1283,7 @@ static void sdhci_set_power(struct sdhci_host
>*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
>
>>
>> Kind regards
>> Uffe
>>
>>>
>>>[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 <Vincent.Yang@tw.fujitsu.com>
>>>---
>>> 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(+)
>>>
>>>diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>>index 764af63..51fce49 100644
>>>--- a/drivers/mmc/core/core.c
>>>+++ b/drivers/mmc/core/core.c
>>>@@ -2648,6 +2648,10 @@ int mmc_pm_notify(struct notifier_block
>>>*notify_block,
>>> case PM_POST_RESTORE:
>>>
>>> spin_lock_irqsave(&host->lock, flags);
>>>+ if (mmc_bus_manual_resume(host)) {
>>>+ spin_unlock_irqrestore(&host->lock, flags);
>>>+ break;
>>>+ }
>>> host->rescan_disable = 0;
>>> spin_unlock_irqrestore(&host->lock, flags);
>>> _mmc_detect_change(host, 0, false);
>>>diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
>>>index 0c44510..859390d 100644
>>>--- a/drivers/mmc/core/sd.c
>>>+++ b/drivers/mmc/core/sd.c
>>>@@ -1133,6 +1133,10 @@ static int mmc_sd_resume(struct mmc_host
>*host)
>>>
>>> if (!(host->caps & MMC_CAP_RUNTIME_RESUME)) {
>>> err = _mmc_sd_resume(host);
>>>+ if ((host->caps2 & MMC_CAP2_MANUAL_RESUME) && err)
>>>+ mmc_set_bus_resume_policy(host, 1);
>>>+ else
>>>+ mmc_set_bus_resume_policy(host, 0);
>>> pm_runtime_set_active(&host->card->dev);
>>> pm_runtime_mark_last_busy(&host->card->dev);
>>> }
>>>diff --git a/drivers/mmc/host/sdhci_f_sdh30.c
>>>b/drivers/mmc/host/sdhci_f_sdh30.c
>>>index 6fae509..67bcff2 100644
>>>--- a/drivers/mmc/host/sdhci_f_sdh30.c
>>>+++ b/drivers/mmc/host/sdhci_f_sdh30.c
>>>@@ -30,6 +30,12 @@
>>> #include "../core/core.h"
>>>
>>> #define DRIVER_NAME "f_sdh30"
>>>+#define RESUME_WAIT_COUNT 100
>>>+#define RESUME_WAIT_TIME 50
>>>+#define RESUME_WAIT_JIFFIES msecs_to_jiffies(RESUME_DETECT_TIME)
>>>+#define RESUME_DETECT_COUNT 16
>>>+#define RESUME_DETECT_TIME 50
>>>+#define RESUME_DETECT_JIFFIES msecs_to_jiffies(RESUME_DETECT_TIME)
>>>
>>>
>>> struct f_sdhost_priv {
>>>@@ -38,8 +44,59 @@ struct f_sdhost_priv {
>>> int gpio_select_1v8;
>>> u32 vendor_hs200;
>>> struct device *dev;
>>>+ unsigned int quirks; /* Deviations from spec. */
>>>+
>>>+/* retry to detect mmc device when resume */
>>>+#define F_SDH30_QUIRK_RESUME_DETECT_RETRY (1<<0)
>>>+
>>>+ struct workqueue_struct *resume_detect_wq;
>>>+ struct delayed_work resume_detect_work;
>>>+ unsigned int resume_detect_count;
>>>+ unsigned int resume_wait_count;
>>> };
>>>
>>>+static void sdhci_f_sdh30_resume_detect_work_func(struct work_struct
>>>*work)
>>>+{
>>>+ struct f_sdhost_priv *priv = container_of(work, struct
>f_sdhost_priv,
>>>+ resume_detect_work.work);
>>>+ struct sdhci_host *host = dev_get_drvdata(priv->dev);
>>>+ int err = 0;
>>>+
>>>+ if (mmc_bus_manual_resume(host->mmc)) {
>>>+ pm_runtime_disable(&host->mmc->card->dev);
>>>+ mmc_card_set_suspended(host->mmc->card);
>>>+ err = host->mmc->bus_ops->resume(host->mmc);
>>>+ if (priv->resume_detect_count-- && err)
>>>+ queue_delayed_work(priv->resume_detect_wq,
>>>+ &priv->resume_detect_work,
>>>+ RESUME_DETECT_JIFFIES);
>>>+ else
>>>+ pr_debug("%s: resume detection done (count:%d,
>wait:%d, err:%d)\n",
>>>+ mmc_hostname(host->mmc),
>>>+ priv->resume_detect_count,
>>>+ priv->resume_wait_count, err);
>>>+ } else {
>>>+ if (priv->resume_wait_count--)
>>>+ queue_delayed_work(priv->resume_detect_wq,
>>>+ &priv->resume_detect_work,
>>>+ RESUME_WAIT_JIFFIES);
>>>+ else
>>>+ pr_debug("%s: resume done\n",
>mmc_hostname(host->mmc));
>>>+ }
>>>+}
>>>+
>>>+static void sdhci_f_sdh30_resume_detect(struct mmc_host *mmc,
>>>+ int detect, int wait)
>>>+{
>>>+ struct sdhci_host *host = mmc_priv(mmc);
>>>+ struct f_sdhost_priv *priv = sdhci_priv(host);
>>>+
>>>+ priv->resume_detect_count = detect;
>>>+ priv->resume_wait_count = wait;
>>>+ queue_delayed_work(priv->resume_detect_wq,
>>>+ &priv->resume_detect_work, 0);
>>>+}
>>>+
>>> void sdhci_f_sdh30_soft_voltage_switch(struct sdhci_host *host)
>>> {
>>> struct f_sdhost_priv *priv = sdhci_priv(host);
>>>@@ -146,6 +203,12 @@ static int sdhci_f_sdh30_probe(struct
>>>platform_device *pdev)
>>> }
>>> }
>>>
>>>+ if (of_find_property(pdev->dev.of_node, "resume-detect-retry",
>NULL))
>>>{
>>>+ dev_info(dev, "Applying resume detect retry quirk\n");
>>>+ priv->quirks |= F_SDH30_QUIRK_RESUME_DETECT_RETRY;
>>>+ host->mmc->caps2 |= MMC_CAP2_MANUAL_RESUME;
>>>+ }
>>>+
>>> ret = mmc_of_parse_voltage(pdev->dev.of_node,
>&host->ocr_mask);
>>> if (ret) {
>>> dev_err(dev, "%s: parse voltage error\n", __func__);
>>>@@ -197,6 +260,18 @@ static int sdhci_f_sdh30_probe(struct
>>>platform_device *pdev)
>>> }
>>> }
>>>
>>>+ /* Init workqueue */
>>>+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY) {
>>>+ priv->resume_detect_wq =
>create_workqueue("sdhci_f_sdh30");
>>>+ if (priv->resume_detect_wq == NULL) {
>>>+ ret = -ENOMEM;
>>>+ dev_err(dev, "Failed to create resume
>detection workqueue\n");
>>>+ goto err_init_wq;
>>>+ }
>>>+ INIT_DELAYED_WORK(&priv->resume_detect_work,
>>>+
>sdhci_f_sdh30_resume_detect_work_func);
>>>+ }
>>>+
>>> ret = sdhci_add_host(host);
>>> if (ret) {
>>> dev_err(dev, "%s: host add error\n", __func__);
>>>@@ -229,6 +304,9 @@ static int sdhci_f_sdh30_probe(struct
>>>platform_device *pdev)
>>> return 0;
>>>
>>> err_add_host:
>>>+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY)
>>>+ destroy_workqueue(priv->resume_detect_wq);
>>>+err_init_wq:
>>> if (gpio_is_valid(priv->gpio_select_1v8)) {
>>> gpio_direction_output(priv->gpio_select_1v8, 1);
>>> gpio_free(priv->gpio_select_1v8);
>>>@@ -268,6 +346,9 @@ static int sdhci_f_sdh30_remove(struct
>>>platform_device *pdev)
>>> gpio_free(priv->gpio_select_1v8);
>>> }
>>>
>>>+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY)
>>>+ destroy_workqueue(priv->resume_detect_wq);
>>>+
>>> sdhci_free_host(host);
>>> platform_set_drvdata(pdev, NULL);
>>>
>>>@@ -285,7 +366,15 @@ static int sdhci_f_sdh30_suspend(struct device
>>>*dev)
>>> static int sdhci_f_sdh30_resume(struct device *dev)
>>> {
>>> struct sdhci_host *host = dev_get_drvdata(dev);
>>>+ struct f_sdhost_priv *priv = sdhci_priv(host);
>>>
>>>+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY) {
>>>+ pr_debug("%s: start resume detect\n",
>>>+ mmc_hostname(host->mmc));
>>>+ sdhci_f_sdh30_resume_detect(host->mmc,
>>>+ RESUME_DETECT_COUNT,
>>>+ RESUME_WAIT_COUNT);
>>>+ }
>>> return sdhci_resume_host(host);
>>> }
>>> #endif
>>>diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>>>index 7960424..55221dd 100644
>>>--- a/include/linux/mmc/host.h
>>>+++ b/include/linux/mmc/host.h
>>>@@ -283,6 +283,7 @@ struct mmc_host {
>>> #define MMC_CAP2_HS400 (MMC_CAP2_HS400_1_8V | \
>>> MMC_CAP2_HS400_1_2V)
>>> #define MMC_CAP2_SDIO_IRQ_NOTHREAD (1 << 17)
>>>+#define MMC_CAP2_MANUAL_RESUME (1 << 18) /* Resume manually
>when
>>>error */
>>>
>>> mmc_pm_flag_t pm_caps; /* supported pm
>features */
>>>
>>>@@ -338,6 +339,9 @@ struct mmc_host {
>>> const struct mmc_bus_ops *bus_ops; /* current bus driver
>*/
>>> unsigned int bus_refs; /* reference counter
>*/
>>>
>>>+ unsigned int bus_resume_flags;
>>>+#define MMC_BUSRESUME_MANUAL_RESUME (1 << 0)
>>>+
>>> unsigned int sdio_irqs;
>>> struct task_struct *sdio_irq_thread;
>>> bool sdio_irq_pending;
>>>@@ -384,6 +388,16 @@ static inline void *mmc_priv(struct mmc_host
>>>*host)
>>> #define mmc_dev(x) ((x)->parent)
>>> #define mmc_classdev(x) (&(x)->class_dev)
>>> #define mmc_hostname(x) (dev_name(&(x)->class_dev))
>>>+#define mmc_bus_manual_resume(host) ((host)->bus_resume_flags &
> \
>>>+ MMC_BUSRESUME_MANUAL_RESUME)
>>>+
>>>+static inline void mmc_set_bus_resume_policy(struct mmc_host *host,
>>>int manual)
>>>+{
>>>+ if (manual)
>>>+ host->bus_resume_flags |= MMC_BUSRESUME_MANUAL_RESUME;
>>>+ else
>>>+ host->bus_resume_flags &=
>~MMC_BUSRESUME_MANUAL_RESUME;
>>>+}
>>>
>>> int mmc_power_save_host(struct mmc_host *host);
>>> int mmc_power_restore_host(struct mmc_host *host);
>>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [RFC PATCH v2 6/6] mmc: core: add manual resume capability
@ 2014-06-27 9:40 ` Ulf Hansson
0 siblings, 0 replies; 30+ messages in thread
From: Ulf Hansson @ 2014-06-27 9:40 UTC (permalink / raw)
To: Vincent Yang
Cc: devicetree, Andy Green, patches, Anton Vorontsov, linux-mmc,
chris, Vincent Yang, Jaswinder Singh, linuxppc-dev
On 27 juni 2014 04:23:42 CEST, Vincent Yang <vincent.yang.fujitsu@gmail.com> wrote:
>2014-06-26 17:42 GMT+08:00 Ulf Hansson <ulf.hansson@linaro.org>:
>>
>>
>> On 26 juni 2014 08:23:32 CEST, Vincent Yang
><vincent.yang.fujitsu@gmail.com> 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.
>
>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:
>
>diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>index 169e17d..ed28896 100644
>--- a/drivers/mmc/host/sdhci.c
>+++ b/drivers/mmc/host/sdhci.c
>@@ -1283,7 +1283,7 @@ static void sdhci_set_power(struct sdhci_host
>*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
>
>>
>> Kind regards
>> Uffe
>>
>>>
>>>[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 <Vincent.Yang@tw.fujitsu.com>
>>>---
>>> 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(+)
>>>
>>>diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>>index 764af63..51fce49 100644
>>>--- a/drivers/mmc/core/core.c
>>>+++ b/drivers/mmc/core/core.c
>>>@@ -2648,6 +2648,10 @@ int mmc_pm_notify(struct notifier_block
>>>*notify_block,
>>> case PM_POST_RESTORE:
>>>
>>> spin_lock_irqsave(&host->lock, flags);
>>>+ if (mmc_bus_manual_resume(host)) {
>>>+ spin_unlock_irqrestore(&host->lock, flags);
>>>+ break;
>>>+ }
>>> host->rescan_disable = 0;
>>> spin_unlock_irqrestore(&host->lock, flags);
>>> _mmc_detect_change(host, 0, false);
>>>diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
>>>index 0c44510..859390d 100644
>>>--- a/drivers/mmc/core/sd.c
>>>+++ b/drivers/mmc/core/sd.c
>>>@@ -1133,6 +1133,10 @@ static int mmc_sd_resume(struct mmc_host
>*host)
>>>
>>> if (!(host->caps & MMC_CAP_RUNTIME_RESUME)) {
>>> err = _mmc_sd_resume(host);
>>>+ if ((host->caps2 & MMC_CAP2_MANUAL_RESUME) && err)
>>>+ mmc_set_bus_resume_policy(host, 1);
>>>+ else
>>>+ mmc_set_bus_resume_policy(host, 0);
>>> pm_runtime_set_active(&host->card->dev);
>>> pm_runtime_mark_last_busy(&host->card->dev);
>>> }
>>>diff --git a/drivers/mmc/host/sdhci_f_sdh30.c
>>>b/drivers/mmc/host/sdhci_f_sdh30.c
>>>index 6fae509..67bcff2 100644
>>>--- a/drivers/mmc/host/sdhci_f_sdh30.c
>>>+++ b/drivers/mmc/host/sdhci_f_sdh30.c
>>>@@ -30,6 +30,12 @@
>>> #include "../core/core.h"
>>>
>>> #define DRIVER_NAME "f_sdh30"
>>>+#define RESUME_WAIT_COUNT 100
>>>+#define RESUME_WAIT_TIME 50
>>>+#define RESUME_WAIT_JIFFIES msecs_to_jiffies(RESUME_DETECT_TIME)
>>>+#define RESUME_DETECT_COUNT 16
>>>+#define RESUME_DETECT_TIME 50
>>>+#define RESUME_DETECT_JIFFIES msecs_to_jiffies(RESUME_DETECT_TIME)
>>>
>>>
>>> struct f_sdhost_priv {
>>>@@ -38,8 +44,59 @@ struct f_sdhost_priv {
>>> int gpio_select_1v8;
>>> u32 vendor_hs200;
>>> struct device *dev;
>>>+ unsigned int quirks; /* Deviations from spec. */
>>>+
>>>+/* retry to detect mmc device when resume */
>>>+#define F_SDH30_QUIRK_RESUME_DETECT_RETRY (1<<0)
>>>+
>>>+ struct workqueue_struct *resume_detect_wq;
>>>+ struct delayed_work resume_detect_work;
>>>+ unsigned int resume_detect_count;
>>>+ unsigned int resume_wait_count;
>>> };
>>>
>>>+static void sdhci_f_sdh30_resume_detect_work_func(struct work_struct
>>>*work)
>>>+{
>>>+ struct f_sdhost_priv *priv = container_of(work, struct
>f_sdhost_priv,
>>>+ resume_detect_work.work);
>>>+ struct sdhci_host *host = dev_get_drvdata(priv->dev);
>>>+ int err = 0;
>>>+
>>>+ if (mmc_bus_manual_resume(host->mmc)) {
>>>+ pm_runtime_disable(&host->mmc->card->dev);
>>>+ mmc_card_set_suspended(host->mmc->card);
>>>+ err = host->mmc->bus_ops->resume(host->mmc);
>>>+ if (priv->resume_detect_count-- && err)
>>>+ queue_delayed_work(priv->resume_detect_wq,
>>>+ &priv->resume_detect_work,
>>>+ RESUME_DETECT_JIFFIES);
>>>+ else
>>>+ pr_debug("%s: resume detection done (count:%d,
>wait:%d, err:%d)\n",
>>>+ mmc_hostname(host->mmc),
>>>+ priv->resume_detect_count,
>>>+ priv->resume_wait_count, err);
>>>+ } else {
>>>+ if (priv->resume_wait_count--)
>>>+ queue_delayed_work(priv->resume_detect_wq,
>>>+ &priv->resume_detect_work,
>>>+ RESUME_WAIT_JIFFIES);
>>>+ else
>>>+ pr_debug("%s: resume done\n",
>mmc_hostname(host->mmc));
>>>+ }
>>>+}
>>>+
>>>+static void sdhci_f_sdh30_resume_detect(struct mmc_host *mmc,
>>>+ int detect, int wait)
>>>+{
>>>+ struct sdhci_host *host = mmc_priv(mmc);
>>>+ struct f_sdhost_priv *priv = sdhci_priv(host);
>>>+
>>>+ priv->resume_detect_count = detect;
>>>+ priv->resume_wait_count = wait;
>>>+ queue_delayed_work(priv->resume_detect_wq,
>>>+ &priv->resume_detect_work, 0);
>>>+}
>>>+
>>> void sdhci_f_sdh30_soft_voltage_switch(struct sdhci_host *host)
>>> {
>>> struct f_sdhost_priv *priv = sdhci_priv(host);
>>>@@ -146,6 +203,12 @@ static int sdhci_f_sdh30_probe(struct
>>>platform_device *pdev)
>>> }
>>> }
>>>
>>>+ if (of_find_property(pdev->dev.of_node, "resume-detect-retry",
>NULL))
>>>{
>>>+ dev_info(dev, "Applying resume detect retry quirk\n");
>>>+ priv->quirks |= F_SDH30_QUIRK_RESUME_DETECT_RETRY;
>>>+ host->mmc->caps2 |= MMC_CAP2_MANUAL_RESUME;
>>>+ }
>>>+
>>> ret = mmc_of_parse_voltage(pdev->dev.of_node,
>&host->ocr_mask);
>>> if (ret) {
>>> dev_err(dev, "%s: parse voltage error\n", __func__);
>>>@@ -197,6 +260,18 @@ static int sdhci_f_sdh30_probe(struct
>>>platform_device *pdev)
>>> }
>>> }
>>>
>>>+ /* Init workqueue */
>>>+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY) {
>>>+ priv->resume_detect_wq =
>create_workqueue("sdhci_f_sdh30");
>>>+ if (priv->resume_detect_wq == NULL) {
>>>+ ret = -ENOMEM;
>>>+ dev_err(dev, "Failed to create resume
>detection workqueue\n");
>>>+ goto err_init_wq;
>>>+ }
>>>+ INIT_DELAYED_WORK(&priv->resume_detect_work,
>>>+
>sdhci_f_sdh30_resume_detect_work_func);
>>>+ }
>>>+
>>> ret = sdhci_add_host(host);
>>> if (ret) {
>>> dev_err(dev, "%s: host add error\n", __func__);
>>>@@ -229,6 +304,9 @@ static int sdhci_f_sdh30_probe(struct
>>>platform_device *pdev)
>>> return 0;
>>>
>>> err_add_host:
>>>+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY)
>>>+ destroy_workqueue(priv->resume_detect_wq);
>>>+err_init_wq:
>>> if (gpio_is_valid(priv->gpio_select_1v8)) {
>>> gpio_direction_output(priv->gpio_select_1v8, 1);
>>> gpio_free(priv->gpio_select_1v8);
>>>@@ -268,6 +346,9 @@ static int sdhci_f_sdh30_remove(struct
>>>platform_device *pdev)
>>> gpio_free(priv->gpio_select_1v8);
>>> }
>>>
>>>+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY)
>>>+ destroy_workqueue(priv->resume_detect_wq);
>>>+
>>> sdhci_free_host(host);
>>> platform_set_drvdata(pdev, NULL);
>>>
>>>@@ -285,7 +366,15 @@ static int sdhci_f_sdh30_suspend(struct device
>>>*dev)
>>> static int sdhci_f_sdh30_resume(struct device *dev)
>>> {
>>> struct sdhci_host *host = dev_get_drvdata(dev);
>>>+ struct f_sdhost_priv *priv = sdhci_priv(host);
>>>
>>>+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY) {
>>>+ pr_debug("%s: start resume detect\n",
>>>+ mmc_hostname(host->mmc));
>>>+ sdhci_f_sdh30_resume_detect(host->mmc,
>>>+ RESUME_DETECT_COUNT,
>>>+ RESUME_WAIT_COUNT);
>>>+ }
>>> return sdhci_resume_host(host);
>>> }
>>> #endif
>>>diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>>>index 7960424..55221dd 100644
>>>--- a/include/linux/mmc/host.h
>>>+++ b/include/linux/mmc/host.h
>>>@@ -283,6 +283,7 @@ struct mmc_host {
>>> #define MMC_CAP2_HS400 (MMC_CAP2_HS400_1_8V | \
>>> MMC_CAP2_HS400_1_2V)
>>> #define MMC_CAP2_SDIO_IRQ_NOTHREAD (1 << 17)
>>>+#define MMC_CAP2_MANUAL_RESUME (1 << 18) /* Resume manually
>when
>>>error */
>>>
>>> mmc_pm_flag_t pm_caps; /* supported pm
>features */
>>>
>>>@@ -338,6 +339,9 @@ struct mmc_host {
>>> const struct mmc_bus_ops *bus_ops; /* current bus driver
>*/
>>> unsigned int bus_refs; /* reference counter
>*/
>>>
>>>+ unsigned int bus_resume_flags;
>>>+#define MMC_BUSRESUME_MANUAL_RESUME (1 << 0)
>>>+
>>> unsigned int sdio_irqs;
>>> struct task_struct *sdio_irq_thread;
>>> bool sdio_irq_pending;
>>>@@ -384,6 +388,16 @@ static inline void *mmc_priv(struct mmc_host
>>>*host)
>>> #define mmc_dev(x) ((x)->parent)
>>> #define mmc_classdev(x) (&(x)->class_dev)
>>> #define mmc_hostname(x) (dev_name(&(x)->class_dev))
>>>+#define mmc_bus_manual_resume(host) ((host)->bus_resume_flags &
> \
>>>+ MMC_BUSRESUME_MANUAL_RESUME)
>>>+
>>>+static inline void mmc_set_bus_resume_policy(struct mmc_host *host,
>>>int manual)
>>>+{
>>>+ if (manual)
>>>+ host->bus_resume_flags |= MMC_BUSRESUME_MANUAL_RESUME;
>>>+ else
>>>+ host->bus_resume_flags &=
>~MMC_BUSRESUME_MANUAL_RESUME;
>>>+}
>>>
>>> int mmc_power_save_host(struct mmc_host *host);
>>> int mmc_power_restore_host(struct mmc_host *host);
>>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [RFC PATCH v2 6/6] mmc: core: add manual resume capability
2014-06-27 9:40 ` Ulf Hansson
@ 2014-06-27 11:00 ` Vincent Yang
-1 siblings, 0 replies; 30+ messages in thread
From: Vincent Yang @ 2014-06-27 11:00 UTC (permalink / raw)
To: Ulf Hansson
Cc: chris, linux-mmc, devicetree, Anton Vorontsov, linuxppc-dev,
patches, Andy Green, Jaswinder Singh
2014-06-27 17:40 GMT+08:00 Ulf Hansson <ulf.hansson@linaro.org>:
>
>
> On 27 juni 2014 04:23:42 CEST, Vincent Yang <vincent.yang.fujitsu@gmail.com> wrote:
>>2014-06-26 17:42 GMT+08:00 Ulf Hansson <ulf.hansson@linaro.org>:
>>>
>>>
>>> On 26 juni 2014 08:23:32 CEST, Vincent Yang
>><vincent.yang.fujitsu@gmail.com> 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.
>>
>>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:
>>
>>diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>index 169e17d..ed28896 100644
>>--- a/drivers/mmc/host/sdhci.c
>>+++ b/drivers/mmc/host/sdhci.c
>>@@ -1283,7 +1283,7 @@ static void sdhci_set_power(struct sdhci_host
>>*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.
>
>>
>> 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
>
>
>>Thanks a lot for your review!
>>
>>
>>Best regards,
>>Vincent Yang
>>
>>>
>>> Kind regards
>>> Uffe
>>>
>>>>
>>>>[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 <Vincent.Yang@tw.fujitsu.com>
>>>>---
>>>> 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(+)
>>>>
>>>>diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>>>index 764af63..51fce49 100644
>>>>--- a/drivers/mmc/core/core.c
>>>>+++ b/drivers/mmc/core/core.c
>>>>@@ -2648,6 +2648,10 @@ int mmc_pm_notify(struct notifier_block
>>>>*notify_block,
>>>> case PM_POST_RESTORE:
>>>>
>>>> spin_lock_irqsave(&host->lock, flags);
>>>>+ if (mmc_bus_manual_resume(host)) {
>>>>+ spin_unlock_irqrestore(&host->lock, flags);
>>>>+ break;
>>>>+ }
>>>> host->rescan_disable = 0;
>>>> spin_unlock_irqrestore(&host->lock, flags);
>>>> _mmc_detect_change(host, 0, false);
>>>>diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
>>>>index 0c44510..859390d 100644
>>>>--- a/drivers/mmc/core/sd.c
>>>>+++ b/drivers/mmc/core/sd.c
>>>>@@ -1133,6 +1133,10 @@ static int mmc_sd_resume(struct mmc_host
>>*host)
>>>>
>>>> if (!(host->caps & MMC_CAP_RUNTIME_RESUME)) {
>>>> err = _mmc_sd_resume(host);
>>>>+ if ((host->caps2 & MMC_CAP2_MANUAL_RESUME) && err)
>>>>+ mmc_set_bus_resume_policy(host, 1);
>>>>+ else
>>>>+ mmc_set_bus_resume_policy(host, 0);
>>>> pm_runtime_set_active(&host->card->dev);
>>>> pm_runtime_mark_last_busy(&host->card->dev);
>>>> }
>>>>diff --git a/drivers/mmc/host/sdhci_f_sdh30.c
>>>>b/drivers/mmc/host/sdhci_f_sdh30.c
>>>>index 6fae509..67bcff2 100644
>>>>--- a/drivers/mmc/host/sdhci_f_sdh30.c
>>>>+++ b/drivers/mmc/host/sdhci_f_sdh30.c
>>>>@@ -30,6 +30,12 @@
>>>> #include "../core/core.h"
>>>>
>>>> #define DRIVER_NAME "f_sdh30"
>>>>+#define RESUME_WAIT_COUNT 100
>>>>+#define RESUME_WAIT_TIME 50
>>>>+#define RESUME_WAIT_JIFFIES msecs_to_jiffies(RESUME_DETECT_TIME)
>>>>+#define RESUME_DETECT_COUNT 16
>>>>+#define RESUME_DETECT_TIME 50
>>>>+#define RESUME_DETECT_JIFFIES msecs_to_jiffies(RESUME_DETECT_TIME)
>>>>
>>>>
>>>> struct f_sdhost_priv {
>>>>@@ -38,8 +44,59 @@ struct f_sdhost_priv {
>>>> int gpio_select_1v8;
>>>> u32 vendor_hs200;
>>>> struct device *dev;
>>>>+ unsigned int quirks; /* Deviations from spec. */
>>>>+
>>>>+/* retry to detect mmc device when resume */
>>>>+#define F_SDH30_QUIRK_RESUME_DETECT_RETRY (1<<0)
>>>>+
>>>>+ struct workqueue_struct *resume_detect_wq;
>>>>+ struct delayed_work resume_detect_work;
>>>>+ unsigned int resume_detect_count;
>>>>+ unsigned int resume_wait_count;
>>>> };
>>>>
>>>>+static void sdhci_f_sdh30_resume_detect_work_func(struct work_struct
>>>>*work)
>>>>+{
>>>>+ struct f_sdhost_priv *priv = container_of(work, struct
>>f_sdhost_priv,
>>>>+ resume_detect_work.work);
>>>>+ struct sdhci_host *host = dev_get_drvdata(priv->dev);
>>>>+ int err = 0;
>>>>+
>>>>+ if (mmc_bus_manual_resume(host->mmc)) {
>>>>+ pm_runtime_disable(&host->mmc->card->dev);
>>>>+ mmc_card_set_suspended(host->mmc->card);
>>>>+ err = host->mmc->bus_ops->resume(host->mmc);
>>>>+ if (priv->resume_detect_count-- && err)
>>>>+ queue_delayed_work(priv->resume_detect_wq,
>>>>+ &priv->resume_detect_work,
>>>>+ RESUME_DETECT_JIFFIES);
>>>>+ else
>>>>+ pr_debug("%s: resume detection done (count:%d,
>>wait:%d, err:%d)\n",
>>>>+ mmc_hostname(host->mmc),
>>>>+ priv->resume_detect_count,
>>>>+ priv->resume_wait_count, err);
>>>>+ } else {
>>>>+ if (priv->resume_wait_count--)
>>>>+ queue_delayed_work(priv->resume_detect_wq,
>>>>+ &priv->resume_detect_work,
>>>>+ RESUME_WAIT_JIFFIES);
>>>>+ else
>>>>+ pr_debug("%s: resume done\n",
>>mmc_hostname(host->mmc));
>>>>+ }
>>>>+}
>>>>+
>>>>+static void sdhci_f_sdh30_resume_detect(struct mmc_host *mmc,
>>>>+ int detect, int wait)
>>>>+{
>>>>+ struct sdhci_host *host = mmc_priv(mmc);
>>>>+ struct f_sdhost_priv *priv = sdhci_priv(host);
>>>>+
>>>>+ priv->resume_detect_count = detect;
>>>>+ priv->resume_wait_count = wait;
>>>>+ queue_delayed_work(priv->resume_detect_wq,
>>>>+ &priv->resume_detect_work, 0);
>>>>+}
>>>>+
>>>> void sdhci_f_sdh30_soft_voltage_switch(struct sdhci_host *host)
>>>> {
>>>> struct f_sdhost_priv *priv = sdhci_priv(host);
>>>>@@ -146,6 +203,12 @@ static int sdhci_f_sdh30_probe(struct
>>>>platform_device *pdev)
>>>> }
>>>> }
>>>>
>>>>+ if (of_find_property(pdev->dev.of_node, "resume-detect-retry",
>>NULL))
>>>>{
>>>>+ dev_info(dev, "Applying resume detect retry quirk\n");
>>>>+ priv->quirks |= F_SDH30_QUIRK_RESUME_DETECT_RETRY;
>>>>+ host->mmc->caps2 |= MMC_CAP2_MANUAL_RESUME;
>>>>+ }
>>>>+
>>>> ret = mmc_of_parse_voltage(pdev->dev.of_node,
>>&host->ocr_mask);
>>>> if (ret) {
>>>> dev_err(dev, "%s: parse voltage error\n", __func__);
>>>>@@ -197,6 +260,18 @@ static int sdhci_f_sdh30_probe(struct
>>>>platform_device *pdev)
>>>> }
>>>> }
>>>>
>>>>+ /* Init workqueue */
>>>>+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY) {
>>>>+ priv->resume_detect_wq =
>>create_workqueue("sdhci_f_sdh30");
>>>>+ if (priv->resume_detect_wq == NULL) {
>>>>+ ret = -ENOMEM;
>>>>+ dev_err(dev, "Failed to create resume
>>detection workqueue\n");
>>>>+ goto err_init_wq;
>>>>+ }
>>>>+ INIT_DELAYED_WORK(&priv->resume_detect_work,
>>>>+
>>sdhci_f_sdh30_resume_detect_work_func);
>>>>+ }
>>>>+
>>>> ret = sdhci_add_host(host);
>>>> if (ret) {
>>>> dev_err(dev, "%s: host add error\n", __func__);
>>>>@@ -229,6 +304,9 @@ static int sdhci_f_sdh30_probe(struct
>>>>platform_device *pdev)
>>>> return 0;
>>>>
>>>> err_add_host:
>>>>+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY)
>>>>+ destroy_workqueue(priv->resume_detect_wq);
>>>>+err_init_wq:
>>>> if (gpio_is_valid(priv->gpio_select_1v8)) {
>>>> gpio_direction_output(priv->gpio_select_1v8, 1);
>>>> gpio_free(priv->gpio_select_1v8);
>>>>@@ -268,6 +346,9 @@ static int sdhci_f_sdh30_remove(struct
>>>>platform_device *pdev)
>>>> gpio_free(priv->gpio_select_1v8);
>>>> }
>>>>
>>>>+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY)
>>>>+ destroy_workqueue(priv->resume_detect_wq);
>>>>+
>>>> sdhci_free_host(host);
>>>> platform_set_drvdata(pdev, NULL);
>>>>
>>>>@@ -285,7 +366,15 @@ static int sdhci_f_sdh30_suspend(struct device
>>>>*dev)
>>>> static int sdhci_f_sdh30_resume(struct device *dev)
>>>> {
>>>> struct sdhci_host *host = dev_get_drvdata(dev);
>>>>+ struct f_sdhost_priv *priv = sdhci_priv(host);
>>>>
>>>>+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY) {
>>>>+ pr_debug("%s: start resume detect\n",
>>>>+ mmc_hostname(host->mmc));
>>>>+ sdhci_f_sdh30_resume_detect(host->mmc,
>>>>+ RESUME_DETECT_COUNT,
>>>>+ RESUME_WAIT_COUNT);
>>>>+ }
>>>> return sdhci_resume_host(host);
>>>> }
>>>> #endif
>>>>diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>>>>index 7960424..55221dd 100644
>>>>--- a/include/linux/mmc/host.h
>>>>+++ b/include/linux/mmc/host.h
>>>>@@ -283,6 +283,7 @@ struct mmc_host {
>>>> #define MMC_CAP2_HS400 (MMC_CAP2_HS400_1_8V | \
>>>> MMC_CAP2_HS400_1_2V)
>>>> #define MMC_CAP2_SDIO_IRQ_NOTHREAD (1 << 17)
>>>>+#define MMC_CAP2_MANUAL_RESUME (1 << 18) /* Resume manually
>>when
>>>>error */
>>>>
>>>> mmc_pm_flag_t pm_caps; /* supported pm
>>features */
>>>>
>>>>@@ -338,6 +339,9 @@ struct mmc_host {
>>>> const struct mmc_bus_ops *bus_ops; /* current bus driver
>>*/
>>>> unsigned int bus_refs; /* reference counter
>>*/
>>>>
>>>>+ unsigned int bus_resume_flags;
>>>>+#define MMC_BUSRESUME_MANUAL_RESUME (1 << 0)
>>>>+
>>>> unsigned int sdio_irqs;
>>>> struct task_struct *sdio_irq_thread;
>>>> bool sdio_irq_pending;
>>>>@@ -384,6 +388,16 @@ static inline void *mmc_priv(struct mmc_host
>>>>*host)
>>>> #define mmc_dev(x) ((x)->parent)
>>>> #define mmc_classdev(x) (&(x)->class_dev)
>>>> #define mmc_hostname(x) (dev_name(&(x)->class_dev))
>>>>+#define mmc_bus_manual_resume(host) ((host)->bus_resume_flags &
>> \
>>>>+ MMC_BUSRESUME_MANUAL_RESUME)
>>>>+
>>>>+static inline void mmc_set_bus_resume_policy(struct mmc_host *host,
>>>>int manual)
>>>>+{
>>>>+ if (manual)
>>>>+ host->bus_resume_flags |= MMC_BUSRESUME_MANUAL_RESUME;
>>>>+ else
>>>>+ host->bus_resume_flags &=
>>~MMC_BUSRESUME_MANUAL_RESUME;
>>>>+}
>>>>
>>>> int mmc_power_save_host(struct mmc_host *host);
>>>> int mmc_power_restore_host(struct mmc_host *host);
>>>
>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [RFC PATCH v2 6/6] mmc: core: add manual resume capability
@ 2014-06-27 11:00 ` Vincent Yang
0 siblings, 0 replies; 30+ messages in thread
From: Vincent Yang @ 2014-06-27 11:00 UTC (permalink / raw)
To: Ulf Hansson
Cc: devicetree, Andy Green, patches, Anton Vorontsov, linux-mmc,
chris, Jaswinder Singh, linuxppc-dev
2014-06-27 17:40 GMT+08:00 Ulf Hansson <ulf.hansson@linaro.org>:
>
>
> On 27 juni 2014 04:23:42 CEST, Vincent Yang <vincent.yang.fujitsu@gmail.com> wrote:
>>2014-06-26 17:42 GMT+08:00 Ulf Hansson <ulf.hansson@linaro.org>:
>>>
>>>
>>> On 26 juni 2014 08:23:32 CEST, Vincent Yang
>><vincent.yang.fujitsu@gmail.com> 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.
>>
>>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:
>>
>>diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>index 169e17d..ed28896 100644
>>--- a/drivers/mmc/host/sdhci.c
>>+++ b/drivers/mmc/host/sdhci.c
>>@@ -1283,7 +1283,7 @@ static void sdhci_set_power(struct sdhci_host
>>*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.
>
>>
>> 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
>
>
>>Thanks a lot for your review!
>>
>>
>>Best regards,
>>Vincent Yang
>>
>>>
>>> Kind regards
>>> Uffe
>>>
>>>>
>>>>[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 <Vincent.Yang@tw.fujitsu.com>
>>>>---
>>>> 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(+)
>>>>
>>>>diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>>>index 764af63..51fce49 100644
>>>>--- a/drivers/mmc/core/core.c
>>>>+++ b/drivers/mmc/core/core.c
>>>>@@ -2648,6 +2648,10 @@ int mmc_pm_notify(struct notifier_block
>>>>*notify_block,
>>>> case PM_POST_RESTORE:
>>>>
>>>> spin_lock_irqsave(&host->lock, flags);
>>>>+ if (mmc_bus_manual_resume(host)) {
>>>>+ spin_unlock_irqrestore(&host->lock, flags);
>>>>+ break;
>>>>+ }
>>>> host->rescan_disable = 0;
>>>> spin_unlock_irqrestore(&host->lock, flags);
>>>> _mmc_detect_change(host, 0, false);
>>>>diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
>>>>index 0c44510..859390d 100644
>>>>--- a/drivers/mmc/core/sd.c
>>>>+++ b/drivers/mmc/core/sd.c
>>>>@@ -1133,6 +1133,10 @@ static int mmc_sd_resume(struct mmc_host
>>*host)
>>>>
>>>> if (!(host->caps & MMC_CAP_RUNTIME_RESUME)) {
>>>> err = _mmc_sd_resume(host);
>>>>+ if ((host->caps2 & MMC_CAP2_MANUAL_RESUME) && err)
>>>>+ mmc_set_bus_resume_policy(host, 1);
>>>>+ else
>>>>+ mmc_set_bus_resume_policy(host, 0);
>>>> pm_runtime_set_active(&host->card->dev);
>>>> pm_runtime_mark_last_busy(&host->card->dev);
>>>> }
>>>>diff --git a/drivers/mmc/host/sdhci_f_sdh30.c
>>>>b/drivers/mmc/host/sdhci_f_sdh30.c
>>>>index 6fae509..67bcff2 100644
>>>>--- a/drivers/mmc/host/sdhci_f_sdh30.c
>>>>+++ b/drivers/mmc/host/sdhci_f_sdh30.c
>>>>@@ -30,6 +30,12 @@
>>>> #include "../core/core.h"
>>>>
>>>> #define DRIVER_NAME "f_sdh30"
>>>>+#define RESUME_WAIT_COUNT 100
>>>>+#define RESUME_WAIT_TIME 50
>>>>+#define RESUME_WAIT_JIFFIES msecs_to_jiffies(RESUME_DETECT_TIME)
>>>>+#define RESUME_DETECT_COUNT 16
>>>>+#define RESUME_DETECT_TIME 50
>>>>+#define RESUME_DETECT_JIFFIES msecs_to_jiffies(RESUME_DETECT_TIME)
>>>>
>>>>
>>>> struct f_sdhost_priv {
>>>>@@ -38,8 +44,59 @@ struct f_sdhost_priv {
>>>> int gpio_select_1v8;
>>>> u32 vendor_hs200;
>>>> struct device *dev;
>>>>+ unsigned int quirks; /* Deviations from spec. */
>>>>+
>>>>+/* retry to detect mmc device when resume */
>>>>+#define F_SDH30_QUIRK_RESUME_DETECT_RETRY (1<<0)
>>>>+
>>>>+ struct workqueue_struct *resume_detect_wq;
>>>>+ struct delayed_work resume_detect_work;
>>>>+ unsigned int resume_detect_count;
>>>>+ unsigned int resume_wait_count;
>>>> };
>>>>
>>>>+static void sdhci_f_sdh30_resume_detect_work_func(struct work_struct
>>>>*work)
>>>>+{
>>>>+ struct f_sdhost_priv *priv = container_of(work, struct
>>f_sdhost_priv,
>>>>+ resume_detect_work.work);
>>>>+ struct sdhci_host *host = dev_get_drvdata(priv->dev);
>>>>+ int err = 0;
>>>>+
>>>>+ if (mmc_bus_manual_resume(host->mmc)) {
>>>>+ pm_runtime_disable(&host->mmc->card->dev);
>>>>+ mmc_card_set_suspended(host->mmc->card);
>>>>+ err = host->mmc->bus_ops->resume(host->mmc);
>>>>+ if (priv->resume_detect_count-- && err)
>>>>+ queue_delayed_work(priv->resume_detect_wq,
>>>>+ &priv->resume_detect_work,
>>>>+ RESUME_DETECT_JIFFIES);
>>>>+ else
>>>>+ pr_debug("%s: resume detection done (count:%d,
>>wait:%d, err:%d)\n",
>>>>+ mmc_hostname(host->mmc),
>>>>+ priv->resume_detect_count,
>>>>+ priv->resume_wait_count, err);
>>>>+ } else {
>>>>+ if (priv->resume_wait_count--)
>>>>+ queue_delayed_work(priv->resume_detect_wq,
>>>>+ &priv->resume_detect_work,
>>>>+ RESUME_WAIT_JIFFIES);
>>>>+ else
>>>>+ pr_debug("%s: resume done\n",
>>mmc_hostname(host->mmc));
>>>>+ }
>>>>+}
>>>>+
>>>>+static void sdhci_f_sdh30_resume_detect(struct mmc_host *mmc,
>>>>+ int detect, int wait)
>>>>+{
>>>>+ struct sdhci_host *host = mmc_priv(mmc);
>>>>+ struct f_sdhost_priv *priv = sdhci_priv(host);
>>>>+
>>>>+ priv->resume_detect_count = detect;
>>>>+ priv->resume_wait_count = wait;
>>>>+ queue_delayed_work(priv->resume_detect_wq,
>>>>+ &priv->resume_detect_work, 0);
>>>>+}
>>>>+
>>>> void sdhci_f_sdh30_soft_voltage_switch(struct sdhci_host *host)
>>>> {
>>>> struct f_sdhost_priv *priv = sdhci_priv(host);
>>>>@@ -146,6 +203,12 @@ static int sdhci_f_sdh30_probe(struct
>>>>platform_device *pdev)
>>>> }
>>>> }
>>>>
>>>>+ if (of_find_property(pdev->dev.of_node, "resume-detect-retry",
>>NULL))
>>>>{
>>>>+ dev_info(dev, "Applying resume detect retry quirk\n");
>>>>+ priv->quirks |= F_SDH30_QUIRK_RESUME_DETECT_RETRY;
>>>>+ host->mmc->caps2 |= MMC_CAP2_MANUAL_RESUME;
>>>>+ }
>>>>+
>>>> ret = mmc_of_parse_voltage(pdev->dev.of_node,
>>&host->ocr_mask);
>>>> if (ret) {
>>>> dev_err(dev, "%s: parse voltage error\n", __func__);
>>>>@@ -197,6 +260,18 @@ static int sdhci_f_sdh30_probe(struct
>>>>platform_device *pdev)
>>>> }
>>>> }
>>>>
>>>>+ /* Init workqueue */
>>>>+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY) {
>>>>+ priv->resume_detect_wq =
>>create_workqueue("sdhci_f_sdh30");
>>>>+ if (priv->resume_detect_wq == NULL) {
>>>>+ ret = -ENOMEM;
>>>>+ dev_err(dev, "Failed to create resume
>>detection workqueue\n");
>>>>+ goto err_init_wq;
>>>>+ }
>>>>+ INIT_DELAYED_WORK(&priv->resume_detect_work,
>>>>+
>>sdhci_f_sdh30_resume_detect_work_func);
>>>>+ }
>>>>+
>>>> ret = sdhci_add_host(host);
>>>> if (ret) {
>>>> dev_err(dev, "%s: host add error\n", __func__);
>>>>@@ -229,6 +304,9 @@ static int sdhci_f_sdh30_probe(struct
>>>>platform_device *pdev)
>>>> return 0;
>>>>
>>>> err_add_host:
>>>>+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY)
>>>>+ destroy_workqueue(priv->resume_detect_wq);
>>>>+err_init_wq:
>>>> if (gpio_is_valid(priv->gpio_select_1v8)) {
>>>> gpio_direction_output(priv->gpio_select_1v8, 1);
>>>> gpio_free(priv->gpio_select_1v8);
>>>>@@ -268,6 +346,9 @@ static int sdhci_f_sdh30_remove(struct
>>>>platform_device *pdev)
>>>> gpio_free(priv->gpio_select_1v8);
>>>> }
>>>>
>>>>+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY)
>>>>+ destroy_workqueue(priv->resume_detect_wq);
>>>>+
>>>> sdhci_free_host(host);
>>>> platform_set_drvdata(pdev, NULL);
>>>>
>>>>@@ -285,7 +366,15 @@ static int sdhci_f_sdh30_suspend(struct device
>>>>*dev)
>>>> static int sdhci_f_sdh30_resume(struct device *dev)
>>>> {
>>>> struct sdhci_host *host = dev_get_drvdata(dev);
>>>>+ struct f_sdhost_priv *priv = sdhci_priv(host);
>>>>
>>>>+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY) {
>>>>+ pr_debug("%s: start resume detect\n",
>>>>+ mmc_hostname(host->mmc));
>>>>+ sdhci_f_sdh30_resume_detect(host->mmc,
>>>>+ RESUME_DETECT_COUNT,
>>>>+ RESUME_WAIT_COUNT);
>>>>+ }
>>>> return sdhci_resume_host(host);
>>>> }
>>>> #endif
>>>>diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>>>>index 7960424..55221dd 100644
>>>>--- a/include/linux/mmc/host.h
>>>>+++ b/include/linux/mmc/host.h
>>>>@@ -283,6 +283,7 @@ struct mmc_host {
>>>> #define MMC_CAP2_HS400 (MMC_CAP2_HS400_1_8V | \
>>>> MMC_CAP2_HS400_1_2V)
>>>> #define MMC_CAP2_SDIO_IRQ_NOTHREAD (1 << 17)
>>>>+#define MMC_CAP2_MANUAL_RESUME (1 << 18) /* Resume manually
>>when
>>>>error */
>>>>
>>>> mmc_pm_flag_t pm_caps; /* supported pm
>>features */
>>>>
>>>>@@ -338,6 +339,9 @@ struct mmc_host {
>>>> const struct mmc_bus_ops *bus_ops; /* current bus driver
>>*/
>>>> unsigned int bus_refs; /* reference counter
>>*/
>>>>
>>>>+ unsigned int bus_resume_flags;
>>>>+#define MMC_BUSRESUME_MANUAL_RESUME (1 << 0)
>>>>+
>>>> unsigned int sdio_irqs;
>>>> struct task_struct *sdio_irq_thread;
>>>> bool sdio_irq_pending;
>>>>@@ -384,6 +388,16 @@ static inline void *mmc_priv(struct mmc_host
>>>>*host)
>>>> #define mmc_dev(x) ((x)->parent)
>>>> #define mmc_classdev(x) (&(x)->class_dev)
>>>> #define mmc_hostname(x) (dev_name(&(x)->class_dev))
>>>>+#define mmc_bus_manual_resume(host) ((host)->bus_resume_flags &
>> \
>>>>+ MMC_BUSRESUME_MANUAL_RESUME)
>>>>+
>>>>+static inline void mmc_set_bus_resume_policy(struct mmc_host *host,
>>>>int manual)
>>>>+{
>>>>+ if (manual)
>>>>+ host->bus_resume_flags |= MMC_BUSRESUME_MANUAL_RESUME;
>>>>+ else
>>>>+ host->bus_resume_flags &=
>>~MMC_BUSRESUME_MANUAL_RESUME;
>>>>+}
>>>>
>>>> int mmc_power_save_host(struct mmc_host *host);
>>>> int mmc_power_restore_host(struct mmc_host *host);
>>>
>
^ permalink raw reply [flat|nested] 30+ messages in thread