All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk
@ 2016-01-27  5:05 Shawn Lin
  2016-01-27  5:06 ` [RFC PATCH 01/21] mmc: sdhci-pltfm: consolidate parsing path Shawn Lin
                   ` (21 more replies)
  0 siblings, 22 replies; 48+ messages in thread
From: Shawn Lin @ 2016-01-27  5:05 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: bcm-kernel-feedback-list, linux-rpi-kernel, linux-mmc,
	linux-kernel, Shawn Lin


Ulf wants to make sdhci into a library, but it's a huge task
since any improvemts may touch too much platforms. But at least
we should make some effort to push things torwards to this target.

This patchset remove SDHCI_QUIRK_BROKEN_CARD_DETECTION from sdhci
to gradually reduce quirk of sdhci.

Firstly, SDHCI_QUIRK_BROKEN_CARD_DETECTION aims at claiming the slot is
a "broken-cd" one, but "broken-cd" is not a quirk from my view.
In addition, mmc core stack had already obtain "broken-cd" from dts via
mmc_of_parse and pass MMC_CAP_NEEDS_POLL to mmc->caps. So we can reuse it
instead of SDHCI_QUIRK_BROKEN_CARD_DETECTION.

However, before doing the cleanup work, I find that geting _of_ property
for sdhci* is not so pretty good and consistent. Some variant drives use
mmc_of_parse, while another ones use sdhci_get_of_property. I also find some
variant drivers combine these two paths. So in order to make my "broken-cd"
cleanup running, I decide to add mmc_of_parse into sdhci_get_of_property and
replace mmc_of_parse with sdhci_get_of_property for all variant drivers if
needed.

Unfortunately, I don't have all these platforms touched to test my patchset.
I might make some mistakes for these changes, so any comments are welcomed.



Shawn Lin (21):
  mmc: sdhci-pltfm: consolidate parsing path
  mmc: sdhci-iproc: consolidate parsing path
  mmc: sdhci-msm: consolidate parsing path
  mmc: sdhci-of-arasan: consolidate parsing path
  mmc: sdhci-of-at91: consolidate parsing path
  mmc: sdhci-of-esdhc: consolidate parsing path
  mmc: sdhci-pxav3: consolidate parsing path
  mmc: sdhci-sirf: check sdhci_get_of_property return value
  mmc: sdhci_f_sdh30: check sdhci_get_of_property return value
  mmc: sdhci: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
  mmc: sdhci-acpi: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
  mmc: sdhci-bcm-kona: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
  mmc: sdhci-bcm2835: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
  mmc: sdhci-esdhc-imx: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
  mmc: sdhci-msm: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
  mmc: sdhci-of-esdhc: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
  mmc: sdhci-pci-core: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
  mmc: sdhci-pltfm: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
  mmc: sdhci-pxav2: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
  mmc: sdhci-s3c: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
  mmc: sdhci.h: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION

 drivers/mmc/host/sdhci-acpi.c      |  3 +--
 drivers/mmc/host/sdhci-bcm-kona.c  |  3 ---
 drivers/mmc/host/sdhci-bcm2835.c   |  5 +++--
 drivers/mmc/host/sdhci-esdhc-imx.c |  9 +++++----
 drivers/mmc/host/sdhci-iproc.c     |  5 +++--
 drivers/mmc/host/sdhci-msm.c       |  6 ++----
 drivers/mmc/host/sdhci-of-arasan.c | 11 ++++-------
 drivers/mmc/host/sdhci-of-at91.c   |  4 +---
 drivers/mmc/host/sdhci-of-esdhc.c  | 25 ++++++++++++-------------
 drivers/mmc/host/sdhci-pci-core.c  |  5 ++++-
 drivers/mmc/host/sdhci-pltfm.c     | 26 +++++++++++++++++++-------
 drivers/mmc/host/sdhci-pltfm.h     |  2 +-
 drivers/mmc/host/sdhci-pxav2.c     |  1 -
 drivers/mmc/host/sdhci-pxav3.c     |  4 +---
 drivers/mmc/host/sdhci-s3c.c       |  2 +-
 drivers/mmc/host/sdhci-sirf.c      |  4 +++-
 drivers/mmc/host/sdhci.c           | 14 +++++---------
 drivers/mmc/host/sdhci.h           | 30 ++++++++++++++----------------
 drivers/mmc/host/sdhci_f_sdh30.c   |  5 ++++-
 19 files changed, 83 insertions(+), 81 deletions(-)

-- 
2.3.7

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [RFC PATCH 01/21] mmc: sdhci-pltfm: consolidate parsing path
  2016-01-27  5:05 [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk Shawn Lin
@ 2016-01-27  5:06 ` Shawn Lin
  2016-01-27  5:06 ` [RFC PATCH 02/21] mmc: sdhci-iproc: " Shawn Lin
                   ` (20 subsequent siblings)
  21 siblings, 0 replies; 48+ messages in thread
From: Shawn Lin @ 2016-01-27  5:06 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: bcm-kernel-feedback-list, linux-rpi-kernel, linux-mmc,
	linux-kernel, Shawn Lin

This patch wrapper mmc_of_parse into sdhci_get_of_property,
and change the return value for variant drivers to check it.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>

---

 drivers/mmc/host/sdhci-pltfm.c | 23 +++++++++++++++++++----
 drivers/mmc/host/sdhci-pltfm.h |  2 +-
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
index 072bb27..8b0fa73 100644
--- a/drivers/mmc/host/sdhci-pltfm.c
+++ b/drivers/mmc/host/sdhci-pltfm.c
@@ -66,12 +66,19 @@ static bool sdhci_of_wp_inverted(struct device_node *np)
 #endif /* CONFIG_PPC */
 }
 
-void sdhci_get_of_property(struct platform_device *pdev)
+int sdhci_get_of_property(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
 	struct sdhci_host *host = platform_get_drvdata(pdev);
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	u32 bus_width;
+	int ret;
+
+	ret = mmc_of_parse(host->mmc);
+	if (ret) {
+		dev_err(&pdev->dev, "mmc_of_parse failed!\n");
+		return ret;
+	}
 
 	if (of_get_property(np, "sdhci,auto-cmd12", NULL))
 		host->quirks |= SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12;
@@ -107,9 +114,11 @@ void sdhci_get_of_property(struct platform_device *pdev)
 	if (of_property_read_bool(np, "wakeup-source") ||
 	    of_property_read_bool(np, "enable-sdio-wakeup")) /* legacy */
 		host->mmc->pm_caps |= MMC_PM_WAKE_SDIO_IRQ;
+
+	return 0;
 }
 #else
-void sdhci_get_of_property(struct platform_device *pdev) {}
+int sdhci_get_of_property(struct platform_device *pdev) {}
 #endif /* CONFIG_OF */
 EXPORT_SYMBOL_GPL(sdhci_get_of_property);
 
@@ -207,12 +216,18 @@ int sdhci_pltfm_register(struct platform_device *pdev,
 	if (IS_ERR(host))
 		return PTR_ERR(host);
 
-	sdhci_get_of_property(pdev);
+	ret = sdhci_get_of_property(pdev);
+	if (ret)
+		goto err_register;
 
 	ret = sdhci_add_host(host);
 	if (ret)
-		sdhci_pltfm_free(pdev);
+		goto err_register;
+
+	return 0;
 
+err_register:
+	sdhci_pltfm_free(pdev);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(sdhci_pltfm_register);
diff --git a/drivers/mmc/host/sdhci-pltfm.h b/drivers/mmc/host/sdhci-pltfm.h
index 04bc248..2da8065 100644
--- a/drivers/mmc/host/sdhci-pltfm.h
+++ b/drivers/mmc/host/sdhci-pltfm.h
@@ -91,7 +91,7 @@ static inline void sdhci_be32bs_writeb(struct sdhci_host *host, u8 val, int reg)
 }
 #endif /* CONFIG_MMC_SDHCI_BIG_ENDIAN_32BIT_BYTE_SWAPPER */
 
-extern void sdhci_get_of_property(struct platform_device *pdev);
+extern int sdhci_get_of_property(struct platform_device *pdev);
 
 extern struct sdhci_host *sdhci_pltfm_init(struct platform_device *pdev,
 					  const struct sdhci_pltfm_data *pdata,
-- 
2.3.7

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [RFC PATCH 02/21] mmc: sdhci-iproc: consolidate parsing path
  2016-01-27  5:05 [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk Shawn Lin
  2016-01-27  5:06 ` [RFC PATCH 01/21] mmc: sdhci-pltfm: consolidate parsing path Shawn Lin
@ 2016-01-27  5:06 ` Shawn Lin
  2016-01-27  5:06 ` [RFC PATCH 03/21] mmc: sdhci-msm: " Shawn Lin
                   ` (19 subsequent siblings)
  21 siblings, 0 replies; 48+ messages in thread
From: Shawn Lin @ 2016-01-27  5:06 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: bcm-kernel-feedback-list, linux-rpi-kernel, linux-mmc,
	linux-kernel, Shawn Lin, Scott Branden

This patch remove mmc_of_parse and check return value
of sdhci_get_of_property.

Cc: Scott Branden <sbranden@broadcom.com>
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/host/sdhci-iproc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c
index 3b423b0..18cfe2c 100644
--- a/drivers/mmc/host/sdhci-iproc.c
+++ b/drivers/mmc/host/sdhci-iproc.c
@@ -196,8 +196,9 @@ static int sdhci_iproc_probe(struct platform_device *pdev)
 
 	iproc_host->data = iproc_data;
 
-	mmc_of_parse(host->mmc);
-	sdhci_get_of_property(pdev);
+	ret = sdhci_get_of_property(pdev);
+	if (ret)
+		return ret;
 
 	/* Enable EMMC 1/8V DDR capable */
 	host->mmc->caps |= MMC_CAP_1_8V_DDR;
-- 
2.3.7

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [RFC PATCH 03/21] mmc: sdhci-msm: consolidate parsing path
  2016-01-27  5:05 [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk Shawn Lin
  2016-01-27  5:06 ` [RFC PATCH 01/21] mmc: sdhci-pltfm: consolidate parsing path Shawn Lin
  2016-01-27  5:06 ` [RFC PATCH 02/21] mmc: sdhci-iproc: " Shawn Lin
@ 2016-01-27  5:06 ` Shawn Lin
  2016-01-27  5:06 ` [RFC PATCH 04/21] mmc: sdhci-of-arasan: " Shawn Lin
                   ` (18 subsequent siblings)
  21 siblings, 0 replies; 48+ messages in thread
From: Shawn Lin @ 2016-01-27  5:06 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: bcm-kernel-feedback-list, linux-rpi-kernel, linux-mmc,
	linux-kernel, Shawn Lin, Georgi Djakov

This patch remove mmc_of_parse and check return value
of sdhci_get_of_property.

Cc: Georgi Djakov <georgi.djakov@linaro.org>
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/host/sdhci-msm.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 4695bee..dfc6016 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -451,12 +451,10 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 	msm_host->mmc = host->mmc;
 	msm_host->pdev = pdev;
 
-	ret = mmc_of_parse(host->mmc);
+	ret = sdhci_get_of_property(pdev);
 	if (ret)
 		goto pltfm_free;
 
-	sdhci_get_of_property(pdev);
-
 	/* Setup SDCC bus voter clock. */
 	msm_host->bus_clk = devm_clk_get(&pdev->dev, "bus");
 	if (!IS_ERR(msm_host->bus_clk)) {
-- 
2.3.7

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [RFC PATCH 04/21] mmc: sdhci-of-arasan: consolidate parsing path
  2016-01-27  5:05 [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk Shawn Lin
                   ` (2 preceding siblings ...)
  2016-01-27  5:06 ` [RFC PATCH 03/21] mmc: sdhci-msm: " Shawn Lin
@ 2016-01-27  5:06 ` Shawn Lin
  2016-01-27  5:06 ` [RFC PATCH 05/21] mmc: sdhci-of-at91: " Shawn Lin
                   ` (17 subsequent siblings)
  21 siblings, 0 replies; 48+ messages in thread
From: Shawn Lin @ 2016-01-27  5:06 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: bcm-kernel-feedback-list, linux-rpi-kernel, linux-mmc,
	linux-kernel, Shawn Lin, Michal Simek, soren.brinkmann

This patch remove mmc_of_parse and check return value
of sdhci_get_of_property.

Cc: Michal Simek <michal.simek@xilinx.com>
Cc: soren.brinkmann@xilinx.com
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/host/sdhci-of-arasan.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
index 75379cb..b653ec7 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -177,17 +177,14 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
 		host->quirks2 |= SDHCI_QUIRK2_HOST_NO_CMD23;
 	}
 
-	sdhci_get_of_property(pdev);
+	ret = sdhci_get_of_property(pdev);
+	if (ret)
+		goto clk_disable_all;
+
 	pltfm_host = sdhci_priv(host);
 	pltfm_host->priv = sdhci_arasan;
 	pltfm_host->clk = clk_xin;
 
-	ret = mmc_of_parse(host->mmc);
-	if (ret) {
-		dev_err(&pdev->dev, "parsing dt failed (%u)\n", ret);
-		goto clk_disable_all;
-	}
-
 	ret = sdhci_add_host(host);
 	if (ret)
 		goto err_pltfm_free;
-- 
2.3.7

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [RFC PATCH 05/21] mmc: sdhci-of-at91: consolidate parsing path
  2016-01-27  5:05 [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk Shawn Lin
                   ` (3 preceding siblings ...)
  2016-01-27  5:06 ` [RFC PATCH 04/21] mmc: sdhci-of-arasan: " Shawn Lin
@ 2016-01-27  5:06 ` Shawn Lin
  2016-01-27  5:07 ` [RFC PATCH 06/21] mmc: sdhci-of-esdhc: " Shawn Lin
                   ` (16 subsequent siblings)
  21 siblings, 0 replies; 48+ messages in thread
From: Shawn Lin @ 2016-01-27  5:06 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: bcm-kernel-feedback-list, linux-rpi-kernel, linux-mmc,
	linux-kernel, Shawn Lin, Ludovic Desroches

This patch remove mmc_of_parse and check return value
of sdhci_get_of_property.

Cc: Ludovic Desroches <ludovic.desroches@atmel.com>
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/host/sdhci-of-at91.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-at91.c b/drivers/mmc/host/sdhci-of-at91.c
index 7e7d8f0..93b1530 100644
--- a/drivers/mmc/host/sdhci-of-at91.c
+++ b/drivers/mmc/host/sdhci-of-at91.c
@@ -194,12 +194,10 @@ static int sdhci_at91_probe(struct platform_device *pdev)
 	pltfm_host = sdhci_priv(host);
 	pltfm_host->priv = priv;
 
-	ret = mmc_of_parse(host->mmc);
+	ret = sdhci_get_of_property(pdev);
 	if (ret)
 		goto clocks_disable_unprepare;
 
-	sdhci_get_of_property(pdev);
-
 	pm_runtime_get_noresume(&pdev->dev);
 	pm_runtime_set_active(&pdev->dev);
 	pm_runtime_enable(&pdev->dev);
-- 
2.3.7

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [RFC PATCH 06/21] mmc: sdhci-of-esdhc: consolidate parsing path
  2016-01-27  5:05 [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk Shawn Lin
                   ` (4 preceding siblings ...)
  2016-01-27  5:06 ` [RFC PATCH 05/21] mmc: sdhci-of-at91: " Shawn Lin
@ 2016-01-27  5:07 ` Shawn Lin
  2016-01-27  5:07 ` [RFC PATCH 07/21] mmc: sdhci-pxav3: " Shawn Lin
                   ` (15 subsequent siblings)
  21 siblings, 0 replies; 48+ messages in thread
From: Shawn Lin @ 2016-01-27  5:07 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: bcm-kernel-feedback-list, linux-rpi-kernel, linux-mmc,
	linux-kernel, Shawn Lin, yangbo lu

This patch remove mmc_of_parse and check return value
of sdhci_get_of_property.

Cc: yangbo lu <yangbo.lu@freescale.com>
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/host/sdhci-of-esdhc.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c
index 83b1226..7fc0edc 100644
--- a/drivers/mmc/host/sdhci-of-esdhc.c
+++ b/drivers/mmc/host/sdhci-of-esdhc.c
@@ -600,7 +600,9 @@ static int sdhci_esdhc_probe(struct platform_device *pdev)
 
 	esdhc_init(pdev, host);
 
-	sdhci_get_of_property(pdev);
+	ret = sdhci_get_of_property(pdev);
+	if (ret)
+		goto err;
 
 	pltfm_host = sdhci_priv(host);
 	esdhc = pltfm_host->priv;
@@ -629,11 +631,6 @@ static int sdhci_esdhc_probe(struct platform_device *pdev)
 		host->quirks2 |= SDHCI_QUIRK2_BROKEN_HOST_CONTROL;
 	}
 
-	/* call to generic mmc_of_parse to support additional capabilities */
-	ret = mmc_of_parse(host->mmc);
-	if (ret)
-		goto err;
-
 	mmc_of_parse_voltage(np, &host->ocr_mask);
 
 	ret = sdhci_add_host(host);
-- 
2.3.7

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [RFC PATCH 07/21] mmc: sdhci-pxav3: consolidate parsing path
  2016-01-27  5:05 [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk Shawn Lin
                   ` (5 preceding siblings ...)
  2016-01-27  5:07 ` [RFC PATCH 06/21] mmc: sdhci-of-esdhc: " Shawn Lin
@ 2016-01-27  5:07 ` Shawn Lin
  2016-01-27  5:44     ` Jisheng Zhang
  2016-01-27  5:07 ` [RFC PATCH 08/21] mmc: sdhci-sirf: check sdhci_get_of_property return value Shawn Lin
                   ` (14 subsequent siblings)
  21 siblings, 1 reply; 48+ messages in thread
From: Shawn Lin @ 2016-01-27  5:07 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: bcm-kernel-feedback-list, linux-rpi-kernel, linux-mmc,
	linux-kernel, Shawn Lin, Jisheng Zhang, Gregory CLEMENT

This patch remove mmc_of_parse and check return value
of sdhci_get_of_property.

Cc: Jisheng Zhang <jszhang@marvell.com>
Cc: Gregory CLEMENT <gregory.clement@free-electrons.com>
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/host/sdhci-pxav3.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci-pxav3.c b/drivers/mmc/host/sdhci-pxav3.c
index f5edf9d..06eda25 100644
--- a/drivers/mmc/host/sdhci-pxav3.c
+++ b/drivers/mmc/host/sdhci-pxav3.c
@@ -410,10 +410,8 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
 
 	match = of_match_device(of_match_ptr(sdhci_pxav3_of_match), &pdev->dev);
 	if (match) {
-		ret = mmc_of_parse(host->mmc);
-		if (ret)
+		ret = sdhci_get_of_property(pdev);
 			goto err_of_parse;
-		sdhci_get_of_property(pdev);
 		pdata = pxav3_get_mmc_pdata(dev);
 		pdev->dev.platform_data = pdata;
 	} else if (pdata) {
-- 
2.3.7

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [RFC PATCH 08/21] mmc: sdhci-sirf: check sdhci_get_of_property return value
  2016-01-27  5:05 [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk Shawn Lin
                   ` (6 preceding siblings ...)
  2016-01-27  5:07 ` [RFC PATCH 07/21] mmc: sdhci-pxav3: " Shawn Lin
@ 2016-01-27  5:07 ` Shawn Lin
  2016-01-27  5:07 ` [RFC PATCH 09/21] mmc: sdhci_f_sdh30: " Shawn Lin
                   ` (13 subsequent siblings)
  21 siblings, 0 replies; 48+ messages in thread
From: Shawn Lin @ 2016-01-27  5:07 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: bcm-kernel-feedback-list, linux-rpi-kernel, linux-mmc,
	linux-kernel, Shawn Lin, Barry Song, Weijun Yang

sdhci_get_of_property may failed while probing, so we check
the return value here.

Cc: Barry Song <baohua@kernel.org>
Cc: Weijun Yang <york.yang@csr.com>
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/host/sdhci-sirf.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-sirf.c b/drivers/mmc/host/sdhci-sirf.c
index 34866f6..3f8499f 100644
--- a/drivers/mmc/host/sdhci-sirf.c
+++ b/drivers/mmc/host/sdhci-sirf.c
@@ -195,7 +195,9 @@ static int sdhci_sirf_probe(struct platform_device *pdev)
 	priv = sdhci_pltfm_priv(pltfm_host);
 	priv->gpio_cd = gpio_cd;
 
-	sdhci_get_of_property(pdev);
+	ret = sdhci_get_of_property(pdev);
+	if (ret)
+		return ret;
 
 	ret = clk_prepare_enable(pltfm_host->clk);
 	if (ret)
-- 
2.3.7

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [RFC PATCH 09/21] mmc: sdhci_f_sdh30: check sdhci_get_of_property return value
  2016-01-27  5:05 [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk Shawn Lin
                   ` (7 preceding siblings ...)
  2016-01-27  5:07 ` [RFC PATCH 08/21] mmc: sdhci-sirf: check sdhci_get_of_property return value Shawn Lin
@ 2016-01-27  5:07 ` Shawn Lin
  2016-01-27  5:08 ` [RFC PATCH 10/21] mmc: sdhci: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION Shawn Lin
                   ` (12 subsequent siblings)
  21 siblings, 0 replies; 48+ messages in thread
From: Shawn Lin @ 2016-01-27  5:07 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: bcm-kernel-feedback-list, linux-rpi-kernel, linux-mmc,
	linux-kernel, Shawn Lin, Vincent Yang, Andy Green

sdhci_get_of_property may failed while probing, so we check
the return value here.

Cc: Vincent Yang <vincent.yang.fujitsu@gmail.com>
Cc: Andy Green <andy.green@linaro.org>
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/host/sdhci_f_sdh30.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci_f_sdh30.c b/drivers/mmc/host/sdhci_f_sdh30.c
index 983b8b3..405e765 100644
--- a/drivers/mmc/host/sdhci_f_sdh30.c
+++ b/drivers/mmc/host/sdhci_f_sdh30.c
@@ -132,7 +132,10 @@ static int sdhci_f_sdh30_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, host);
 
-	sdhci_get_of_property(pdev);
+	ret = sdhci_get_of_property(pdev);
+	if (ret)
+		goto err;
+
 	host->hw_name = "f_sdh30";
 	host->ops = &sdhci_f_sdh30_ops;
 	host->irq = irq;
-- 
2.3.7

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [RFC PATCH 10/21] mmc: sdhci: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
  2016-01-27  5:05 [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk Shawn Lin
                   ` (8 preceding siblings ...)
  2016-01-27  5:07 ` [RFC PATCH 09/21] mmc: sdhci_f_sdh30: " Shawn Lin
@ 2016-01-27  5:08 ` Shawn Lin
  2016-01-27  7:11     ` Haibo Chen
  2016-01-27  5:08 ` [RFC PATCH 11/21] mmc: sdhci-acpi: " Shawn Lin
                   ` (11 subsequent siblings)
  21 siblings, 1 reply; 48+ messages in thread
From: Shawn Lin @ 2016-01-27  5:08 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: bcm-kernel-feedback-list, linux-rpi-kernel, linux-mmc,
	linux-kernel, Shawn Lin

SDHCI_QUIRK_BROKEN_CARD_DETECTION is for "broken-cd".
If we add MMC_CAP_NONREMOVABLE("non-removeble"), we shoud
not add "broken-cd" together according to mmc.txt for
dt-bingdings. Also, "broken-cd" can obtain from mmc_of_parse,
which will add MMC_CAP_NEEDS_POLL into mmc->caps.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/host/sdhci.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index d622435..b208cb7 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -145,7 +145,7 @@ static void sdhci_set_card_detection(struct sdhci_host *host, bool enable)
 {
 	u32 present;
 
-	if ((host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) ||
+	if ((host->mmc->caps & MMC_CAP_NEEDS_POLL) ||
 	    (host->mmc->caps & MMC_CAP_NONREMOVABLE))
 		return;
 
@@ -1618,7 +1618,8 @@ static int sdhci_do_get_cd(struct sdhci_host *host)
 		return 0;
 
 	/* If nonremovable, assume that the card is always present. */
-	if (host->mmc->caps & MMC_CAP_NONREMOVABLE)
+	if (host->mmc->caps & MMC_CAP_NONREMOVABLE ||
+	    host->mmc->caps & MMC_CAP_NEEDS_POLL)
 		return 1;
 
 	/*
@@ -1628,10 +1629,6 @@ static int sdhci_do_get_cd(struct sdhci_host *host)
 	if (!IS_ERR_VALUE(gpio_cd))
 		return !!gpio_cd;
 
-	/* If polling, assume that the card is always present. */
-	if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
-		return 1;
-
 	/* Host native card detect */
 	return !!(sdhci_readl(host, SDHCI_PRESENT_STATE) & SDHCI_CARD_PRESENT);
 }
@@ -2658,7 +2655,7 @@ void sdhci_enable_irq_wakeups(struct sdhci_host *host)
 	val = sdhci_readb(host, SDHCI_WAKE_UP_CONTROL);
 	val |= mask ;
 	/* Avoid fake wake up */
-	if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
+	if (host->mmc->caps & MMC_CAP_NEEDS_POLL)
 		val &= ~(SDHCI_WAKE_ON_INSERT | SDHCI_WAKE_ON_REMOVE);
 	sdhci_writeb(host, val, SDHCI_WAKE_UP_CONTROL);
 }
@@ -3112,8 +3109,7 @@ int sdhci_add_host(struct sdhci_host *host)
 	if (caps[0] & SDHCI_CAN_DO_HISPD)
 		mmc->caps |= MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED;
 
-	if ((host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) &&
-	    !(mmc->caps & MMC_CAP_NONREMOVABLE) &&
+	if (!(mmc->caps & MMC_CAP_NONREMOVABLE) &&
 	    IS_ERR_VALUE(mmc_gpio_get_cd(host->mmc)))
 		mmc->caps |= MMC_CAP_NEEDS_POLL;
 
-- 
2.3.7

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [RFC PATCH 11/21] mmc: sdhci-acpi: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
  2016-01-27  5:05 [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk Shawn Lin
                   ` (9 preceding siblings ...)
  2016-01-27  5:08 ` [RFC PATCH 10/21] mmc: sdhci: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION Shawn Lin
@ 2016-01-27  5:08 ` Shawn Lin
  2016-01-27  5:08 ` [RFC PATCH 12/21] mmc: sdhci-bcm-kona: " Shawn Lin
                   ` (10 subsequent siblings)
  21 siblings, 0 replies; 48+ messages in thread
From: Shawn Lin @ 2016-01-27  5:08 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: bcm-kernel-feedback-list, linux-rpi-kernel, linux-mmc,
	linux-kernel, Shawn Lin, Adrian Hunter

sdhci-acpi already add MMC_CAP_NONREMOVABLE for caps, so
there is no need to add SDHCI_QUIRK_BROKEN_CARD_DETECTION.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/host/sdhci-acpi.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
index f6047fc..cb43098 100644
--- a/drivers/mmc/host/sdhci-acpi.c
+++ b/drivers/mmc/host/sdhci-acpi.c
@@ -214,8 +214,7 @@ static const struct sdhci_acpi_slot sdhci_acpi_slot_int_emmc = {
 };
 
 static const struct sdhci_acpi_slot sdhci_acpi_slot_int_sdio = {
-	.quirks  = SDHCI_QUIRK_BROKEN_CARD_DETECTION |
-		   SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC,
+	.quirks  = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC,
 	.quirks2 = SDHCI_QUIRK2_HOST_OFF_CARD_ON,
 	.caps    = MMC_CAP_NONREMOVABLE | MMC_CAP_POWER_OFF_CARD |
 		   MMC_CAP_BUS_WIDTH_TEST | MMC_CAP_WAIT_WHILE_BUSY,
-- 
2.3.7

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [RFC PATCH 12/21] mmc: sdhci-bcm-kona: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
  2016-01-27  5:05 [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk Shawn Lin
                   ` (10 preceding siblings ...)
  2016-01-27  5:08 ` [RFC PATCH 11/21] mmc: sdhci-acpi: " Shawn Lin
@ 2016-01-27  5:08 ` Shawn Lin
  2016-01-27  5:08 ` [RFC PATCH 13/21] mmc: sdhci-bcm2835: " Shawn Lin
                   ` (9 subsequent siblings)
  21 siblings, 0 replies; 48+ messages in thread
From: Shawn Lin @ 2016-01-27  5:08 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: bcm-kernel-feedback-list, linux-rpi-kernel, linux-mmc,
	linux-kernel, Shawn Lin, Ray Jui, Florian Fainelli

If MMC_CAP_NONREMOVABLE is provided, SDHCI_QUIRK_BROKEN_CARD_DETECTION
is redunant.

Cc: Ray Jui <rjui@broadcom.com>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/host/sdhci-bcm-kona.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci-bcm-kona.c b/drivers/mmc/host/sdhci-bcm-kona.c
index 00a8a40..2bd541f0 100644
--- a/drivers/mmc/host/sdhci-bcm-kona.c
+++ b/drivers/mmc/host/sdhci-bcm-kona.c
@@ -269,9 +269,6 @@ static int sdhci_bcm_kona_probe(struct platform_device *pdev)
 		(mmc_gpio_get_cd(host->mmc) != -ENOSYS) ? 'Y' : 'N',
 		(mmc_gpio_get_ro(host->mmc) != -ENOSYS) ? 'Y' : 'N');
 
-	if (host->mmc->caps & MMC_CAP_NONREMOVABLE)
-		host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
-
 	dev_dbg(dev, "is_8bit=%c\n",
 		(host->mmc->caps & MMC_CAP_8_BIT_DATA) ? 'Y' : 'N');
 
-- 
2.3.7

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [RFC PATCH 13/21] mmc: sdhci-bcm2835: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
  2016-01-27  5:05 [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk Shawn Lin
                   ` (11 preceding siblings ...)
  2016-01-27  5:08 ` [RFC PATCH 12/21] mmc: sdhci-bcm-kona: " Shawn Lin
@ 2016-01-27  5:08 ` Shawn Lin
  2016-01-27  5:08 ` [RFC PATCH 14/21] mmc: sdhci-esdhc-imx: " Shawn Lin
                   ` (8 subsequent siblings)
  21 siblings, 0 replies; 48+ messages in thread
From: Shawn Lin @ 2016-01-27  5:08 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: bcm-kernel-feedback-list, linux-rpi-kernel, linux-mmc,
	linux-kernel, Shawn Lin, Florian Fainelli, Ray Jui

It seems SDHCI_QUIRK_BROKEN_CARD_DETECTION is a must quirk for
this driver, so we remove SDHCI_QUIRK_BROKEN_CARD_DETECTION and
add MMC_CAP_NEEDS_POLL for it.

Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Ray Jui <rjui@broadcom.com>
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/host/sdhci-bcm2835.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci-bcm2835.c b/drivers/mmc/host/sdhci-bcm2835.c
index 1c65d46..b34115c 100644
--- a/drivers/mmc/host/sdhci-bcm2835.c
+++ b/drivers/mmc/host/sdhci-bcm2835.c
@@ -140,8 +140,7 @@ static const struct sdhci_ops bcm2835_sdhci_ops = {
 };
 
 static const struct sdhci_pltfm_data bcm2835_sdhci_pdata = {
-	.quirks = SDHCI_QUIRK_BROKEN_CARD_DETECTION |
-		  SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK,
+	.quirks = SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK,
 	.ops = &bcm2835_sdhci_ops,
 };
 
@@ -156,6 +155,8 @@ static int bcm2835_sdhci_probe(struct platform_device *pdev)
 	if (IS_ERR(host))
 		return PTR_ERR(host);
 
+	host->mmc->caps |= MMC_CAP_NEEDS_POLL;
+
 	bcm2835_host = devm_kzalloc(&pdev->dev, sizeof(*bcm2835_host),
 					GFP_KERNEL);
 	if (!bcm2835_host) {
-- 
2.3.7

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [RFC PATCH 14/21] mmc: sdhci-esdhc-imx: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
  2016-01-27  5:05 [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk Shawn Lin
                   ` (12 preceding siblings ...)
  2016-01-27  5:08 ` [RFC PATCH 13/21] mmc: sdhci-bcm2835: " Shawn Lin
@ 2016-01-27  5:08 ` Shawn Lin
  2016-01-27  6:54     ` Haibo Chen
  2016-01-27  5:08 ` [RFC PATCH 15/21] mmc: sdhci-msm: " Shawn Lin
                   ` (7 subsequent siblings)
  21 siblings, 1 reply; 48+ messages in thread
From: Shawn Lin @ 2016-01-27  5:08 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: bcm-kernel-feedback-list, linux-rpi-kernel, linux-mmc,
	linux-kernel, Shawn Lin, Dong Aisheng, Haibo Chen

sdhci_esdhc_imx_pdata need SDHCI_QUIRK_BROKEN_CARD_DETECTION,
so we replace it with MMC_CAP_NEEDS_POLL while probing. For other
cases, we directly remove SDHCI_QUIRK_BROKEN_CARD_DETECTION.

Cc: Dong Aisheng <aisheng.dong@freescale.com>
Cc: Haibo Chen <haibo.chen@freescale.com>
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/host/sdhci-esdhc-imx.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index f25f292..5705be1 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -952,8 +952,7 @@ static struct sdhci_ops sdhci_esdhc_ops = {
 static const struct sdhci_pltfm_data sdhci_esdhc_imx_pdata = {
 	.quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_NO_HISPD_BIT
 			| SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC
-			| SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC
-			| SDHCI_QUIRK_BROKEN_CARD_DETECTION,
+			| SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC,
 	.ops = &sdhci_esdhc_ops,
 };
 
@@ -1012,7 +1011,7 @@ sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,
 		return ret;
 
 	if (!IS_ERR_VALUE(mmc_gpio_get_cd(host->mmc)))
-		host->quirks &= ~SDHCI_QUIRK_BROKEN_CARD_DETECTION;
+		host->mmc->caps &= MMC_CAP_NEEDS_POLL;
 
 	return 0;
 }
@@ -1064,7 +1063,7 @@ static int sdhci_esdhc_imx_probe_nondt(struct platform_device *pdev,
 
 	case ESDHC_CD_CONTROLLER:
 		/* we have a working card_detect back */
-		host->quirks &= ~SDHCI_QUIRK_BROKEN_CARD_DETECTION;
+		host->mmc->caps &= MMC_CAP_NEEDS_POLL;
 		break;
 
 	case ESDHC_CD_PERMANENT:
@@ -1104,6 +1103,8 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
 	if (IS_ERR(host))
 		return PTR_ERR(host);
 
+	host->mmc->caps |= MMC_CAP_NEEDS_POLL;
+
 	pltfm_host = sdhci_priv(host);
 
 	imx_data = devm_kzalloc(&pdev->dev, sizeof(*imx_data), GFP_KERNEL);
-- 
2.3.7

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [RFC PATCH 15/21] mmc: sdhci-msm: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
  2016-01-27  5:05 [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk Shawn Lin
                   ` (13 preceding siblings ...)
  2016-01-27  5:08 ` [RFC PATCH 14/21] mmc: sdhci-esdhc-imx: " Shawn Lin
@ 2016-01-27  5:08 ` Shawn Lin
  2016-01-27  5:08 ` [RFC PATCH 16/21] mmc: sdhci-of-esdhc: " Shawn Lin
                   ` (6 subsequent siblings)
  21 siblings, 0 replies; 48+ messages in thread
From: Shawn Lin @ 2016-01-27  5:08 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: bcm-kernel-feedback-list, linux-rpi-kernel, linux-mmc,
	linux-kernel, Shawn Lin, Georgi Djakov

We just replcae SDHCI_QUIRK_BROKEN_CARD_DETECTION with
MMC_CAP_NEEDS_POLL.

Cc: Georgi Djakov <georgi.djakov@linaro.org>
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/host/sdhci-msm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index dfc6016..3274871 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -520,7 +520,7 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 	/* Set HC_MODE_EN bit in HC_MODE register */
 	writel_relaxed(HC_MODE_EN, (msm_host->core_mem + CORE_HC_MODE));
 
-	host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
+	host->mmc->caps |= MMC_CAP_NEEDS_POLL;
 	host->quirks |= SDHCI_QUIRK_SINGLE_POWER_WRITE;
 
 	host_version = readw_relaxed((host->ioaddr + SDHCI_HOST_VERSION));
-- 
2.3.7

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [RFC PATCH 16/21] mmc: sdhci-of-esdhc: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
  2016-01-27  5:05 [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk Shawn Lin
                   ` (14 preceding siblings ...)
  2016-01-27  5:08 ` [RFC PATCH 15/21] mmc: sdhci-msm: " Shawn Lin
@ 2016-01-27  5:08 ` Shawn Lin
  2016-01-27  5:09 ` [RFC PATCH 17/21] mmc: sdhci-pci-core: " Shawn Lin
                   ` (5 subsequent siblings)
  21 siblings, 0 replies; 48+ messages in thread
From: Shawn Lin @ 2016-01-27  5:08 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: bcm-kernel-feedback-list, linux-rpi-kernel, linux-mmc,
	linux-kernel, Shawn Lin, yangbo lu

This patch just remove SDHCI_QUIRK_BROKEN_CARD_DETECTION, and
use MMC_CAP_NEEDS_POLL if needed.

Cc: yangbo lu <yangbo.lu@freescale.com>
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/host/sdhci-of-esdhc.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c
index 7fc0edc..df4fa8b 100644
--- a/drivers/mmc/host/sdhci-of-esdhc.c
+++ b/drivers/mmc/host/sdhci-of-esdhc.c
@@ -549,16 +549,16 @@ static const struct sdhci_ops sdhci_esdhc_le_ops = {
 };
 
 static const struct sdhci_pltfm_data sdhci_esdhc_be_pdata = {
-	.quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_BROKEN_CARD_DETECTION
-		| SDHCI_QUIRK_NO_CARD_NO_RESET
-		| SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC,
+	.quirks = ESDHC_DEFAULT_QUIRKS |
+		  SDHCI_QUIRK_NO_CARD_NO_RESET |
+		  SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC,
 	.ops = &sdhci_esdhc_be_ops,
 };
 
 static const struct sdhci_pltfm_data sdhci_esdhc_le_pdata = {
-	.quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_BROKEN_CARD_DETECTION
-		| SDHCI_QUIRK_NO_CARD_NO_RESET
-		| SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC,
+	.quirks = ESDHC_DEFAULT_QUIRKS |
+		  SDHCI_QUIRK_NO_CARD_NO_RESET |
+		  SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC,
 	.ops = &sdhci_esdhc_le_ops,
 };
 
@@ -595,6 +595,8 @@ static int sdhci_esdhc_probe(struct platform_device *pdev)
 	else
 		host = sdhci_pltfm_init(pdev, &sdhci_esdhc_be_pdata, 0);
 
+	host->mmc->caps |= MMC_CAP_NEEDS_POLL;
+
 	if (IS_ERR(host))
 		return PTR_ERR(host);
 
@@ -618,7 +620,7 @@ static int sdhci_esdhc_probe(struct platform_device *pdev)
 	    of_device_is_compatible(np, "fsl,p1020-esdhc") ||
 	    of_device_is_compatible(np, "fsl,t1040-esdhc") ||
 	    of_device_is_compatible(np, "fsl,ls1021a-esdhc"))
-		host->quirks &= ~SDHCI_QUIRK_BROKEN_CARD_DETECTION;
+		host->caps &= ~MMC_CAP_NEEDS_POLL;
 
 	if (of_device_is_compatible(np, "fsl,ls1021a-esdhc"))
 		host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
-- 
2.3.7

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [RFC PATCH 17/21] mmc: sdhci-pci-core: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
  2016-01-27  5:05 [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk Shawn Lin
                   ` (15 preceding siblings ...)
  2016-01-27  5:08 ` [RFC PATCH 16/21] mmc: sdhci-of-esdhc: " Shawn Lin
@ 2016-01-27  5:09 ` Shawn Lin
  2016-01-27  5:09 ` [RFC PATCH 18/21] mmc: sdhci-pltfm: " Shawn Lin
                   ` (4 subsequent siblings)
  21 siblings, 0 replies; 48+ messages in thread
From: Shawn Lin @ 2016-01-27  5:09 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: bcm-kernel-feedback-list, linux-rpi-kernel, linux-mmc,
	linux-kernel, Shawn Lin, Adrian Hunter

This patch remove SDHCI_QUIRK_BROKEN_CARD_DETECTION and add
MMC_CAP_NEEDS_POLL for PCI_DEVICE_ID_MARVELL_88ALP01_SD

Cc: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/host/sdhci-pci-core.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
index cc851b0..1f6d3f6 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -101,7 +101,6 @@ static const struct sdhci_pci_fixes sdhci_ene_714 = {
 static const struct sdhci_pci_fixes sdhci_cafe = {
 	.quirks		= SDHCI_QUIRK_NO_SIMULT_VDD_AND_POWER |
 			  SDHCI_QUIRK_NO_BUSY_IRQ |
-			  SDHCI_QUIRK_BROKEN_CARD_DETECTION |
 			  SDHCI_QUIRK_BROKEN_TIMEOUT_VAL,
 };
 
@@ -1612,6 +1611,10 @@ static struct sdhci_pci_slot *sdhci_pci_probe_slot(
 	host->quirks = chip->quirks;
 	host->quirks2 = chip->quirks2;
 
+	if ((chip->pdev->vendor == PCI_VENDOR_ID_MARVELL) &&
+	    (chip->pdev->device == PCI_DEVICE_ID_MARVELL_88ALP01_SD))
+	    host->mmc->caps |= MMC_CAP_NEEDS_POLL;
+
 	host->irq = pdev->irq;
 
 	ret = pci_request_region(pdev, bar, mmc_hostname(host->mmc));
-- 
2.3.7

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [RFC PATCH 18/21] mmc: sdhci-pltfm: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
  2016-01-27  5:05 [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk Shawn Lin
                   ` (16 preceding siblings ...)
  2016-01-27  5:09 ` [RFC PATCH 17/21] mmc: sdhci-pci-core: " Shawn Lin
@ 2016-01-27  5:09 ` Shawn Lin
  2016-01-27  5:09 ` [RFC PATCH 19/21] mmc: sdhci-pxav2: " Shawn Lin
                   ` (3 subsequent siblings)
  21 siblings, 0 replies; 48+ messages in thread
From: Shawn Lin @ 2016-01-27  5:09 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: bcm-kernel-feedback-list, linux-rpi-kernel, linux-mmc,
	linux-kernel, Shawn Lin

We already call mmc_of_parse to get "broken-cd", so just
remove it here.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/host/sdhci-pltfm.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
index 8b0fa73..dae5d0ba9 100644
--- a/drivers/mmc/host/sdhci-pltfm.c
+++ b/drivers/mmc/host/sdhci-pltfm.c
@@ -91,9 +91,6 @@ int sdhci_get_of_property(struct platform_device *pdev)
 	if (sdhci_of_wp_inverted(np))
 		host->quirks |= SDHCI_QUIRK_INVERTED_WRITE_PROTECT;
 
-	if (of_get_property(np, "broken-cd", NULL))
-		host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
-
 	if (of_get_property(np, "no-1-8-v", NULL))
 		host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V;
 
-- 
2.3.7

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [RFC PATCH 19/21] mmc: sdhci-pxav2: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
  2016-01-27  5:05 [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk Shawn Lin
                   ` (17 preceding siblings ...)
  2016-01-27  5:09 ` [RFC PATCH 18/21] mmc: sdhci-pltfm: " Shawn Lin
@ 2016-01-27  5:09 ` Shawn Lin
  2016-01-27  5:09 ` [RFC PATCH 20/21] mmc: sdhci-s3c: " Shawn Lin
                   ` (2 subsequent siblings)
  21 siblings, 0 replies; 48+ messages in thread
From: Shawn Lin @ 2016-01-27  5:09 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: bcm-kernel-feedback-list, linux-rpi-kernel, linux-mmc,
	linux-kernel, Shawn Lin, Jisheng Zhang

It already claims MMC_CAP_NONREMOVABLE, so we just remove the
quirk here since we don't need non-removeble and broken-cd together.

Cc: Jisheng Zhang <jszhang@marvell.com>
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/host/sdhci-pxav2.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-pxav2.c b/drivers/mmc/host/sdhci-pxav2.c
index beffd86..8aa2c31 100644
--- a/drivers/mmc/host/sdhci-pxav2.c
+++ b/drivers/mmc/host/sdhci-pxav2.c
@@ -199,7 +199,6 @@ static int sdhci_pxav2_probe(struct platform_device *pdev)
 	if (pdata) {
 		if (pdata->flags & PXA_FLAG_CARD_PERMANENT) {
 			/* on-chip device */
-			host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
 			host->mmc->caps |= MMC_CAP_NONREMOVABLE;
 		}
 
-- 
2.3.7

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [RFC PATCH 20/21] mmc: sdhci-s3c: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
  2016-01-27  5:05 [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk Shawn Lin
                   ` (18 preceding siblings ...)
  2016-01-27  5:09 ` [RFC PATCH 19/21] mmc: sdhci-pxav2: " Shawn Lin
@ 2016-01-27  5:09 ` Shawn Lin
  2016-01-27  5:09 ` [RFC PATCH 21/21] mmc: sdhci.h: " Shawn Lin
  2016-01-27 12:59 ` [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk Adrian Hunter
  21 siblings, 0 replies; 48+ messages in thread
From: Shawn Lin @ 2016-01-27  5:09 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: bcm-kernel-feedback-list, linux-rpi-kernel, linux-mmc,
	linux-kernel, Shawn Lin, Jaehoon Chung

This patch remove SDHCI_QUIRK_BROKEN_CARD_DETECTION and use
MMC_CAP_NEEDS_POLL to claim "broken-cd".

Cc: Jaehoon Chung <jh80.chung@samsung.com>
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/host/sdhci-s3c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
index 70c724b..dd25767 100644
--- a/drivers/mmc/host/sdhci-s3c.c
+++ b/drivers/mmc/host/sdhci-s3c.c
@@ -571,7 +571,7 @@ static int sdhci_s3c_probe(struct platform_device *pdev)
 
 	if (pdata->cd_type == S3C_SDHCI_CD_NONE ||
 	    pdata->cd_type == S3C_SDHCI_CD_PERMANENT)
-		host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
+		host->mmc->caps |= MMC_CAP_NEEDS_POLL;
 
 	if (pdata->cd_type == S3C_SDHCI_CD_PERMANENT)
 		host->mmc->caps = MMC_CAP_NONREMOVABLE;
-- 
2.3.7

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [RFC PATCH 21/21] mmc: sdhci.h: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
  2016-01-27  5:05 [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk Shawn Lin
                   ` (19 preceding siblings ...)
  2016-01-27  5:09 ` [RFC PATCH 20/21] mmc: sdhci-s3c: " Shawn Lin
@ 2016-01-27  5:09 ` Shawn Lin
  2016-01-27 12:59 ` [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk Adrian Hunter
  21 siblings, 0 replies; 48+ messages in thread
From: Shawn Lin @ 2016-01-27  5:09 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: bcm-kernel-feedback-list, linux-rpi-kernel, linux-mmc,
	linux-kernel, Shawn Lin

We remove SDHCI_QUIRK_BROKEN_CARD_DETECTION since now
sdhci core and variant drivers will not use it. And we
also move up the quirk number one-by-one.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/host/sdhci.h | 30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 7654ae5..41988ef 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -356,36 +356,34 @@ struct sdhci_host {
 #define SDHCI_QUIRK_BROKEN_SMALL_PIO			(1<<13)
 /* Controller does not provide transfer-complete interrupt when not busy */
 #define SDHCI_QUIRK_NO_BUSY_IRQ				(1<<14)
-/* Controller has unreliable card detection */
-#define SDHCI_QUIRK_BROKEN_CARD_DETECTION		(1<<15)
 /* Controller reports inverted write-protect state */
-#define SDHCI_QUIRK_INVERTED_WRITE_PROTECT		(1<<16)
+#define SDHCI_QUIRK_INVERTED_WRITE_PROTECT		(1<<15)
 /* Controller does not like fast PIO transfers */
-#define SDHCI_QUIRK_PIO_NEEDS_DELAY			(1<<18)
+#define SDHCI_QUIRK_PIO_NEEDS_DELAY			(1<<16)
 /* Controller has to be forced to use block size of 2048 bytes */
-#define SDHCI_QUIRK_FORCE_BLK_SZ_2048			(1<<20)
+#define SDHCI_QUIRK_FORCE_BLK_SZ_2048			(1<<17)
 /* Controller cannot do multi-block transfers */
-#define SDHCI_QUIRK_NO_MULTIBLOCK			(1<<21)
+#define SDHCI_QUIRK_NO_MULTIBLOCK			(1<<18)
 /* Controller can only handle 1-bit data transfers */
-#define SDHCI_QUIRK_FORCE_1_BIT_DATA			(1<<22)
+#define SDHCI_QUIRK_FORCE_1_BIT_DATA			(1<<19)
 /* Controller needs 10ms delay between applying power and clock */
-#define SDHCI_QUIRK_DELAY_AFTER_POWER			(1<<23)
+#define SDHCI_QUIRK_DELAY_AFTER_POWER			(1<<20)
 /* Controller uses SDCLK instead of TMCLK for data timeouts */
-#define SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK		(1<<24)
+#define SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK		(1<<21)
 /* Controller reports wrong base clock capability */
-#define SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN		(1<<25)
+#define SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN		(1<<22)
 /* Controller cannot support End Attribute in NOP ADMA descriptor */
-#define SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC		(1<<26)
+#define SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC		(1<<23)
 /* Controller is missing device caps. Use caps provided by host */
-#define SDHCI_QUIRK_MISSING_CAPS			(1<<27)
+#define SDHCI_QUIRK_MISSING_CAPS			(1<<24)
 /* Controller uses Auto CMD12 command to stop the transfer */
-#define SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12		(1<<28)
+#define SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12		(1<<25)
 /* Controller doesn't have HISPD bit field in HI-SPEED SD card */
-#define SDHCI_QUIRK_NO_HISPD_BIT			(1<<29)
+#define SDHCI_QUIRK_NO_HISPD_BIT			(1<<26)
 /* Controller treats ADMA descriptors with length 0000h incorrectly */
-#define SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC		(1<<30)
+#define SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC		(1<<27)
 /* The read-only detection via SDHCI_PRESENT_STATE register is unstable */
-#define SDHCI_QUIRK_UNSTABLE_RO_DETECT			(1<<31)
+#define SDHCI_QUIRK_UNSTABLE_RO_DETECT			(1<<28)
 
 	unsigned int quirks2;	/* More deviations from spec. */
 
-- 
2.3.7

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* Re: [RFC PATCH 07/21] mmc: sdhci-pxav3: consolidate parsing path
  2016-01-27  5:07 ` [RFC PATCH 07/21] mmc: sdhci-pxav3: " Shawn Lin
@ 2016-01-27  5:44     ` Jisheng Zhang
  0 siblings, 0 replies; 48+ messages in thread
From: Jisheng Zhang @ 2016-01-27  5:44 UTC (permalink / raw)
  To: Shawn Lin
  Cc: Ulf Hansson, bcm-kernel-feedback-list, linux-rpi-kernel,
	linux-mmc, linux-kernel, Gregory CLEMENT

Hi Shawn,

On Wed, 27 Jan 2016 13:07:36 +0800 Shawn Lin wrote:

> This patch remove mmc_of_parse and check return value
> of sdhci_get_of_property.
> 
> Cc: Jisheng Zhang <jszhang@marvell.com>
> Cc: Gregory CLEMENT <gregory.clement@free-electrons.com>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>

Reviewed-by: Jisheng Zhang <jszhang@marvell.com>


But I have some comments to your patch10:

> SDHCI_QUIRK_BROKEN_CARD_DETECTION is for "broken-cd".
> If we add MMC_CAP_NONREMOVABLE("non-removeble"), we shoud
> not add "broken-cd" together according to mmc.txt for
> dt-bingdings. Also, "broken-cd" can obtain from mmc_of_parse,
> which will add MMC_CAP_NEEDS_POLL into mmc->caps.

The problem is non-dt platforms which need the broken cd quirk don't have
chance to set MMC_CAP_NEEDS_POLL in driver, unless we explicitly do so
in driver. But then, the driver only need to do so on some platforms but
not always.

Thanks,
Jisheng


> ---
> 
>  drivers/mmc/host/sdhci-pxav3.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-pxav3.c b/drivers/mmc/host/sdhci-pxav3.c
> index f5edf9d..06eda25 100644
> --- a/drivers/mmc/host/sdhci-pxav3.c
> +++ b/drivers/mmc/host/sdhci-pxav3.c
> @@ -410,10 +410,8 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
>  
>  	match = of_match_device(of_match_ptr(sdhci_pxav3_of_match), &pdev->dev);
>  	if (match) {
> -		ret = mmc_of_parse(host->mmc);
> -		if (ret)
> +		ret = sdhci_get_of_property(pdev);
>  			goto err_of_parse;
> -		sdhci_get_of_property(pdev);
>  		pdata = pxav3_get_mmc_pdata(dev);
>  		pdev->dev.platform_data = pdata;
>  	} else if (pdata) {

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [RFC PATCH 07/21] mmc: sdhci-pxav3: consolidate parsing path
@ 2016-01-27  5:44     ` Jisheng Zhang
  0 siblings, 0 replies; 48+ messages in thread
From: Jisheng Zhang @ 2016-01-27  5:44 UTC (permalink / raw)
  To: Shawn Lin
  Cc: Ulf Hansson, bcm-kernel-feedback-list, linux-rpi-kernel,
	linux-mmc, linux-kernel, Gregory CLEMENT

Hi Shawn,

On Wed, 27 Jan 2016 13:07:36 +0800 Shawn Lin wrote:

> This patch remove mmc_of_parse and check return value
> of sdhci_get_of_property.
> 
> Cc: Jisheng Zhang <jszhang@marvell.com>
> Cc: Gregory CLEMENT <gregory.clement@free-electrons.com>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>

Reviewed-by: Jisheng Zhang <jszhang@marvell.com>


But I have some comments to your patch10:

> SDHCI_QUIRK_BROKEN_CARD_DETECTION is for "broken-cd".
> If we add MMC_CAP_NONREMOVABLE("non-removeble"), we shoud
> not add "broken-cd" together according to mmc.txt for
> dt-bingdings. Also, "broken-cd" can obtain from mmc_of_parse,
> which will add MMC_CAP_NEEDS_POLL into mmc->caps.

The problem is non-dt platforms which need the broken cd quirk don't have
chance to set MMC_CAP_NEEDS_POLL in driver, unless we explicitly do so
in driver. But then, the driver only need to do so on some platforms but
not always.

Thanks,
Jisheng


> ---
> 
>  drivers/mmc/host/sdhci-pxav3.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-pxav3.c b/drivers/mmc/host/sdhci-pxav3.c
> index f5edf9d..06eda25 100644
> --- a/drivers/mmc/host/sdhci-pxav3.c
> +++ b/drivers/mmc/host/sdhci-pxav3.c
> @@ -410,10 +410,8 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
>  
>  	match = of_match_device(of_match_ptr(sdhci_pxav3_of_match), &pdev->dev);
>  	if (match) {
> -		ret = mmc_of_parse(host->mmc);
> -		if (ret)
> +		ret = sdhci_get_of_property(pdev);
>  			goto err_of_parse;
> -		sdhci_get_of_property(pdev);
>  		pdata = pxav3_get_mmc_pdata(dev);
>  		pdev->dev.platform_data = pdata;
>  	} else if (pdata) {


^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [RFC PATCH 07/21] mmc: sdhci-pxav3: consolidate parsing path
  2016-01-27  5:44     ` Jisheng Zhang
  (?)
@ 2016-01-27  6:17     ` Shawn Lin
  -1 siblings, 0 replies; 48+ messages in thread
From: Shawn Lin @ 2016-01-27  6:17 UTC (permalink / raw)
  To: Jisheng Zhang
  Cc: shawn.lin, Ulf Hansson, bcm-kernel-feedback-list,
	linux-rpi-kernel, linux-mmc, linux-kernel, Gregory CLEMENT

Hi Jisheng,

On 2016/1/27 13:44, Jisheng Zhang wrote:
> Hi Shawn,
>
> On Wed, 27 Jan 2016 13:07:36 +0800 Shawn Lin wrote:
>
>> This patch remove mmc_of_parse and check return value
>> of sdhci_get_of_property.
>>
>> Cc: Jisheng Zhang <jszhang@marvell.com>
>> Cc: Gregory CLEMENT <gregory.clement@free-electrons.com>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>
> Reviewed-by: Jisheng Zhang <jszhang@marvell.com>
>
>
> But I have some comments to your patch10:
>
>> SDHCI_QUIRK_BROKEN_CARD_DETECTION is for "broken-cd".
>> If we add MMC_CAP_NONREMOVABLE("non-removeble"), we shoud
>> not add "broken-cd" together according to mmc.txt for
>> dt-bingdings. Also, "broken-cd" can obtain from mmc_of_parse,
>> which will add MMC_CAP_NEEDS_POLL into mmc->caps.
>
> The problem is non-dt platforms which need the broken cd quirk don't have
> chance to set MMC_CAP_NEEDS_POLL in driver, unless we explicitly do so
> in driver. But then, the driver only need to do so on some platforms but
> not always.
>

Thanks for sharing your thought. That is indeed a historical baggage
to improve sdhci. So, it needs to check each platform case by case.
Let's wait more feedback untile I decide how to respin the patchset. :)

> Thanks,
> Jisheng
>
>
>> ---
>>
>>   drivers/mmc/host/sdhci-pxav3.c | 4 +---
>>   1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-pxav3.c b/drivers/mmc/host/sdhci-pxav3.c
>> index f5edf9d..06eda25 100644
>> --- a/drivers/mmc/host/sdhci-pxav3.c
>> +++ b/drivers/mmc/host/sdhci-pxav3.c
>> @@ -410,10 +410,8 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
>>
>>   	match = of_match_device(of_match_ptr(sdhci_pxav3_of_match), &pdev->dev);
>>   	if (match) {
>> -		ret = mmc_of_parse(host->mmc);
>> -		if (ret)
>> +		ret = sdhci_get_of_property(pdev);
>>   			goto err_of_parse;
>> -		sdhci_get_of_property(pdev);
>>   		pdata = pxav3_get_mmc_pdata(dev);
>>   		pdev->dev.platform_data = pdata;
>>   	} else if (pdata) {
>
>
>
>


-- 
Best Regards
Shawn Lin

^ permalink raw reply	[flat|nested] 48+ messages in thread

* RE: [RFC PATCH 14/21] mmc: sdhci-esdhc-imx: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
@ 2016-01-27  6:54     ` Haibo Chen
  0 siblings, 0 replies; 48+ messages in thread
From: Haibo Chen @ 2016-01-27  6:54 UTC (permalink / raw)
  To: Shawn Lin, Ulf Hansson
  Cc: bcm-kernel-feedback-list, linux-rpi-kernel, linux-mmc,
	linux-kernel, Dong Aisheng, Haibo Chen

Hi Shawn, 

Comments below.



> -----Original Message-----
> From: Shawn Lin [mailto:shawn.lin@rock-chips.com]
> Sent: Wednesday, January 27, 2016 1:09 PM
> To: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: bcm-kernel-feedback-list@broadcom.com; linux-rpi-
> kernel@lists.infradead.org; linux-mmc@vger.kernel.org; linux-
> kernel@vger.kernel.org; Shawn Lin <shawn.lin@rock-chips.com>; Dong
> Aisheng <aisheng.dong@freescale.com>; Haibo Chen
> <haibo.chen@freescale.com>
> Subject: [RFC PATCH 14/21] mmc: sdhci-esdhc-imx: remove
> SDHCI_QUIRK_BROKEN_CARD_DETECTION
> 
> sdhci_esdhc_imx_pdata need SDHCI_QUIRK_BROKEN_CARD_DETECTION,
> so we replace it with MMC_CAP_NEEDS_POLL while probing. For other cases,
> we directly remove SDHCI_QUIRK_BROKEN_CARD_DETECTION.
> 
> Cc: Dong Aisheng <aisheng.dong@freescale.com>
> Cc: Haibo Chen <haibo.chen@freescale.com>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> ---
> 
>  drivers/mmc/host/sdhci-esdhc-imx.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-
> esdhc-imx.c
> index f25f292..5705be1 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -952,8 +952,7 @@ static struct sdhci_ops sdhci_esdhc_ops = {  static const
> struct sdhci_pltfm_data sdhci_esdhc_imx_pdata = {
>  	.quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_NO_HISPD_BIT
>  			| SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC
> -			| SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC
> -			| SDHCI_QUIRK_BROKEN_CARD_DETECTION,
> +			| SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC,
>  	.ops = &sdhci_esdhc_ops,
>  };
> 
> @@ -1012,7 +1011,7 @@ sdhci_esdhc_imx_probe_dt(struct platform_device
> *pdev,
>  		return ret;
> 
>  	if (!IS_ERR_VALUE(mmc_gpio_get_cd(host->mmc)))
> -		host->quirks &= ~SDHCI_QUIRK_BROKEN_CARD_DETECTION;
> +		host->mmc->caps &= MMC_CAP_NEEDS_POLL;

You miss ' ~', seems you need to change to:
host->mmc->caps &=~ MMC_CAP_NEEDS_POLL;

> 
>  	return 0;
>  }
> @@ -1064,7 +1063,7 @@ static int sdhci_esdhc_imx_probe_nondt(struct
> platform_device *pdev,
> 
>  	case ESDHC_CD_CONTROLLER:
>  		/* we have a working card_detect back */
> -		host->quirks &= ~SDHCI_QUIRK_BROKEN_CARD_DETECTION;
> +		host->mmc->caps &= MMC_CAP_NEEDS_POLL;

The same issue.

>  		break;
> 
>  	case ESDHC_CD_PERMANENT:
> @@ -1104,6 +1103,8 @@ static int sdhci_esdhc_imx_probe(struct
> platform_device *pdev)
>  	if (IS_ERR(host))
>  		return PTR_ERR(host);
> 
> +	host->mmc->caps |= MMC_CAP_NEEDS_POLL;
> +
>  	pltfm_host = sdhci_priv(host);
> 
>  	imx_data = devm_kzalloc(&pdev->dev, sizeof(*imx_data),
> GFP_KERNEL);
> --
> 2.3.7
> 

^ permalink raw reply	[flat|nested] 48+ messages in thread

* RE: [RFC PATCH 14/21] mmc: sdhci-esdhc-imx: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
@ 2016-01-27  6:54     ` Haibo Chen
  0 siblings, 0 replies; 48+ messages in thread
From: Haibo Chen @ 2016-01-27  6:54 UTC (permalink / raw)
  To: Shawn Lin, Ulf Hansson
  Cc: Dong Aisheng, Haibo Chen, linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Hi Shawn, 

Comments below.



> -----Original Message-----
> From: Shawn Lin [mailto:shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org]
> Sent: Wednesday, January 27, 2016 1:09 PM
> To: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w@public.gmane.org; linux-rpi-
> kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org; linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-
> kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>; Dong
> Aisheng <aisheng.dong-KZfg59tc24xl57MIdRCFDg@public.gmane.org>; Haibo Chen
> <haibo.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> Subject: [RFC PATCH 14/21] mmc: sdhci-esdhc-imx: remove
> SDHCI_QUIRK_BROKEN_CARD_DETECTION
> 
> sdhci_esdhc_imx_pdata need SDHCI_QUIRK_BROKEN_CARD_DETECTION,
> so we replace it with MMC_CAP_NEEDS_POLL while probing. For other cases,
> we directly remove SDHCI_QUIRK_BROKEN_CARD_DETECTION.
> 
> Cc: Dong Aisheng <aisheng.dong-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> Cc: Haibo Chen <haibo.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> Signed-off-by: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> ---
> 
>  drivers/mmc/host/sdhci-esdhc-imx.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-
> esdhc-imx.c
> index f25f292..5705be1 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -952,8 +952,7 @@ static struct sdhci_ops sdhci_esdhc_ops = {  static const
> struct sdhci_pltfm_data sdhci_esdhc_imx_pdata = {
>  	.quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_NO_HISPD_BIT
>  			| SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC
> -			| SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC
> -			| SDHCI_QUIRK_BROKEN_CARD_DETECTION,
> +			| SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC,
>  	.ops = &sdhci_esdhc_ops,
>  };
> 
> @@ -1012,7 +1011,7 @@ sdhci_esdhc_imx_probe_dt(struct platform_device
> *pdev,
>  		return ret;
> 
>  	if (!IS_ERR_VALUE(mmc_gpio_get_cd(host->mmc)))
> -		host->quirks &= ~SDHCI_QUIRK_BROKEN_CARD_DETECTION;
> +		host->mmc->caps &= MMC_CAP_NEEDS_POLL;

You miss ' ~', seems you need to change to:
host->mmc->caps &=~ MMC_CAP_NEEDS_POLL;

> 
>  	return 0;
>  }
> @@ -1064,7 +1063,7 @@ static int sdhci_esdhc_imx_probe_nondt(struct
> platform_device *pdev,
> 
>  	case ESDHC_CD_CONTROLLER:
>  		/* we have a working card_detect back */
> -		host->quirks &= ~SDHCI_QUIRK_BROKEN_CARD_DETECTION;
> +		host->mmc->caps &= MMC_CAP_NEEDS_POLL;

The same issue.

>  		break;
> 
>  	case ESDHC_CD_PERMANENT:
> @@ -1104,6 +1103,8 @@ static int sdhci_esdhc_imx_probe(struct
> platform_device *pdev)
>  	if (IS_ERR(host))
>  		return PTR_ERR(host);
> 
> +	host->mmc->caps |= MMC_CAP_NEEDS_POLL;
> +
>  	pltfm_host = sdhci_priv(host);
> 
>  	imx_data = devm_kzalloc(&pdev->dev, sizeof(*imx_data),
> GFP_KERNEL);
> --
> 2.3.7
> 

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [RFC PATCH 14/21] mmc: sdhci-esdhc-imx: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
  2016-01-27  6:54     ` Haibo Chen
@ 2016-01-27  6:58       ` Shawn Lin
  -1 siblings, 0 replies; 48+ messages in thread
From: Shawn Lin @ 2016-01-27  6:58 UTC (permalink / raw)
  To: Haibo Chen, Ulf Hansson
  Cc: shawn.lin, bcm-kernel-feedback-list, linux-rpi-kernel, linux-mmc,
	linux-kernel, Dong Aisheng, Haibo Chen

On 2016/1/27 14:54, Haibo Chen wrote:
> Hi Shawn,
>
> Comments below.
>

[...]

>>   		return ret;
>>
>>   	if (!IS_ERR_VALUE(mmc_gpio_get_cd(host->mmc)))
>> -		host->quirks &= ~SDHCI_QUIRK_BROKEN_CARD_DETECTION;
>> +		host->mmc->caps &= MMC_CAP_NEEDS_POLL;
>
> You miss ' ~', seems you need to change to:
> host->mmc->caps &=~ MMC_CAP_NEEDS_POLL;
>

Thanks for pointing out. I will fix this for the next version.

>>
>>   	return 0;
>>   }
>> @@ -1064,7 +1063,7 @@ static int sdhci_esdhc_imx_probe_nondt(struct
>> platform_device *pdev,
>>
>>   	case ESDHC_CD_CONTROLLER:
>>   		/* we have a working card_detect back */
>> -		host->quirks &= ~SDHCI_QUIRK_BROKEN_CARD_DETECTION;
>> +		host->mmc->caps &= MMC_CAP_NEEDS_POLL;
>
> The same issue.
>
>>   		break;
>>
>>   	case ESDHC_CD_PERMANENT:
>> @@ -1104,6 +1103,8 @@ static int sdhci_esdhc_imx_probe(struct
>> platform_device *pdev)
>>   	if (IS_ERR(host))
>>   		return PTR_ERR(host);
>>
>> +	host->mmc->caps |= MMC_CAP_NEEDS_POLL;
>> +
>>   	pltfm_host = sdhci_priv(host);
>>
>>   	imx_data = devm_kzalloc(&pdev->dev, sizeof(*imx_data),
>> GFP_KERNEL);
>> --
>> 2.3.7
>>
>
>
>
>


-- 
Best Regards
Shawn Lin

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [RFC PATCH 14/21] mmc: sdhci-esdhc-imx: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
@ 2016-01-27  6:58       ` Shawn Lin
  0 siblings, 0 replies; 48+ messages in thread
From: Shawn Lin @ 2016-01-27  6:58 UTC (permalink / raw)
  To: Haibo Chen, Ulf Hansson
  Cc: shawn.lin, bcm-kernel-feedback-list, linux-rpi-kernel, linux-mmc,
	linux-kernel, Dong Aisheng, Haibo Chen

On 2016/1/27 14:54, Haibo Chen wrote:
> Hi Shawn,
>
> Comments below.
>

[...]

>>   		return ret;
>>
>>   	if (!IS_ERR_VALUE(mmc_gpio_get_cd(host->mmc)))
>> -		host->quirks &= ~SDHCI_QUIRK_BROKEN_CARD_DETECTION;
>> +		host->mmc->caps &= MMC_CAP_NEEDS_POLL;
>
> You miss ' ~', seems you need to change to:
> host->mmc->caps &=~ MMC_CAP_NEEDS_POLL;
>

Thanks for pointing out. I will fix this for the next version.

>>
>>   	return 0;
>>   }
>> @@ -1064,7 +1063,7 @@ static int sdhci_esdhc_imx_probe_nondt(struct
>> platform_device *pdev,
>>
>>   	case ESDHC_CD_CONTROLLER:
>>   		/* we have a working card_detect back */
>> -		host->quirks &= ~SDHCI_QUIRK_BROKEN_CARD_DETECTION;
>> +		host->mmc->caps &= MMC_CAP_NEEDS_POLL;
>
> The same issue.
>
>>   		break;
>>
>>   	case ESDHC_CD_PERMANENT:
>> @@ -1104,6 +1103,8 @@ static int sdhci_esdhc_imx_probe(struct
>> platform_device *pdev)
>>   	if (IS_ERR(host))
>>   		return PTR_ERR(host);
>>
>> +	host->mmc->caps |= MMC_CAP_NEEDS_POLL;
>> +
>>   	pltfm_host = sdhci_priv(host);
>>
>>   	imx_data = devm_kzalloc(&pdev->dev, sizeof(*imx_data),
>> GFP_KERNEL);
>> --
>> 2.3.7
>>
>
>
>
>


-- 
Best Regards
Shawn Lin


^ permalink raw reply	[flat|nested] 48+ messages in thread

* RE: [RFC PATCH 10/21] mmc: sdhci: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
  2016-01-27  5:08 ` [RFC PATCH 10/21] mmc: sdhci: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION Shawn Lin
@ 2016-01-27  7:11     ` Haibo Chen
  0 siblings, 0 replies; 48+ messages in thread
From: Haibo Chen @ 2016-01-27  7:11 UTC (permalink / raw)
  To: Shawn Lin, Ulf Hansson
  Cc: bcm-kernel-feedback-list, linux-rpi-kernel, linux-mmc, linux-kernel



Hi Shawn,


> -----Original Message-----
> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
> owner@vger.kernel.org] On Behalf Of Shawn Lin
> Sent: Wednesday, January 27, 2016 1:08 PM
> To: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: bcm-kernel-feedback-list@broadcom.com; linux-rpi-
> kernel@lists.infradead.org; linux-mmc@vger.kernel.org; linux-
> kernel@vger.kernel.org; Shawn Lin <shawn.lin@rock-chips.com>
> Subject: [RFC PATCH 10/21] mmc: sdhci: remove
> SDHCI_QUIRK_BROKEN_CARD_DETECTION
> 
> SDHCI_QUIRK_BROKEN_CARD_DETECTION is for "broken-cd".
> If we add MMC_CAP_NONREMOVABLE("non-removeble"), we shoud not add
> "broken-cd" together according to mmc.txt for dt-bingdings. Also, "broken-cd"
> can obtain from mmc_of_parse, which will add MMC_CAP_NEEDS_POLL into
> mmc->caps.
> 
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> ---
> 
>  drivers/mmc/host/sdhci.c | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index
> d622435..b208cb7 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -145,7 +145,7 @@ static void sdhci_set_card_detection(struct sdhci_host
> *host, bool enable)  {
>  	u32 present;
> 
> -	if ((host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) ||
> +	if ((host->mmc->caps & MMC_CAP_NEEDS_POLL) ||
>  	    (host->mmc->caps & MMC_CAP_NONREMOVABLE))
>  		return;
> 
> @@ -1618,7 +1618,8 @@ static int sdhci_do_get_cd(struct sdhci_host *host)
>  		return 0;
> 
>  	/* If nonremovable, assume that the card is always present. */
> -	if (host->mmc->caps & MMC_CAP_NONREMOVABLE)
> +	if (host->mmc->caps & MMC_CAP_NONREMOVABLE ||
> +	    host->mmc->caps & MMC_CAP_NEEDS_POLL)
>  		return 1;
> 
>  	/*
> @@ -1628,10 +1629,6 @@ static int sdhci_do_get_cd(struct sdhci_host *host)
>  	if (!IS_ERR_VALUE(gpio_cd))
>  		return !!gpio_cd;
> 
> -	/* If polling, assume that the card is always present. */
> -	if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
> -		return 1;

Why simply  remove these code rather than use this:
If(host->mmc->caps & MMC_CAP_NEEDS_POLL)
       Return 1;

According to the comment, if polling, need to return 1,
But if you just simply remove this code, it will read the sdhci register SDHCI_PRESENT_STATE


Best Regards

Haibo Chen

> -
>  	/* Host native card detect */
>  	return !!(sdhci_readl(host, SDHCI_PRESENT_STATE) &
> SDHCI_CARD_PRESENT);  } @@ -2658,7 +2655,7 @@ void
> sdhci_enable_irq_wakeups(struct sdhci_host *host)
>  	val = sdhci_readb(host, SDHCI_WAKE_UP_CONTROL);
>  	val |= mask ;
>  	/* Avoid fake wake up */
> -	if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
> +	if (host->mmc->caps & MMC_CAP_NEEDS_POLL)
>  		val &= ~(SDHCI_WAKE_ON_INSERT |
> SDHCI_WAKE_ON_REMOVE);
>  	sdhci_writeb(host, val, SDHCI_WAKE_UP_CONTROL);  } @@ -3112,8
> +3109,7 @@ int sdhci_add_host(struct sdhci_host *host)
>  	if (caps[0] & SDHCI_CAN_DO_HISPD)
>  		mmc->caps |= MMC_CAP_SD_HIGHSPEED |
> MMC_CAP_MMC_HIGHSPEED;
> 
> -	if ((host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) &&
> -	    !(mmc->caps & MMC_CAP_NONREMOVABLE) &&
> +	if (!(mmc->caps & MMC_CAP_NONREMOVABLE) &&
>  	    IS_ERR_VALUE(mmc_gpio_get_cd(host->mmc)))
>  		mmc->caps |= MMC_CAP_NEEDS_POLL;
> 
> --
> 2.3.7
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 48+ messages in thread

* RE: [RFC PATCH 10/21] mmc: sdhci: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
@ 2016-01-27  7:11     ` Haibo Chen
  0 siblings, 0 replies; 48+ messages in thread
From: Haibo Chen @ 2016-01-27  7:11 UTC (permalink / raw)
  To: Shawn Lin, Ulf Hansson
  Cc: bcm-kernel-feedback-list, linux-rpi-kernel, linux-mmc, linux-kernel



Hi Shawn,


> -----Original Message-----
> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
> owner@vger.kernel.org] On Behalf Of Shawn Lin
> Sent: Wednesday, January 27, 2016 1:08 PM
> To: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: bcm-kernel-feedback-list@broadcom.com; linux-rpi-
> kernel@lists.infradead.org; linux-mmc@vger.kernel.org; linux-
> kernel@vger.kernel.org; Shawn Lin <shawn.lin@rock-chips.com>
> Subject: [RFC PATCH 10/21] mmc: sdhci: remove
> SDHCI_QUIRK_BROKEN_CARD_DETECTION
> 
> SDHCI_QUIRK_BROKEN_CARD_DETECTION is for "broken-cd".
> If we add MMC_CAP_NONREMOVABLE("non-removeble"), we shoud not add
> "broken-cd" together according to mmc.txt for dt-bingdings. Also, "broken-cd"
> can obtain from mmc_of_parse, which will add MMC_CAP_NEEDS_POLL into
> mmc->caps.
> 
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> ---
> 
>  drivers/mmc/host/sdhci.c | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index
> d622435..b208cb7 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -145,7 +145,7 @@ static void sdhci_set_card_detection(struct sdhci_host
> *host, bool enable)  {
>  	u32 present;
> 
> -	if ((host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) ||
> +	if ((host->mmc->caps & MMC_CAP_NEEDS_POLL) ||
>  	    (host->mmc->caps & MMC_CAP_NONREMOVABLE))
>  		return;
> 
> @@ -1618,7 +1618,8 @@ static int sdhci_do_get_cd(struct sdhci_host *host)
>  		return 0;
> 
>  	/* If nonremovable, assume that the card is always present. */
> -	if (host->mmc->caps & MMC_CAP_NONREMOVABLE)
> +	if (host->mmc->caps & MMC_CAP_NONREMOVABLE ||
> +	    host->mmc->caps & MMC_CAP_NEEDS_POLL)
>  		return 1;
> 
>  	/*
> @@ -1628,10 +1629,6 @@ static int sdhci_do_get_cd(struct sdhci_host *host)
>  	if (!IS_ERR_VALUE(gpio_cd))
>  		return !!gpio_cd;
> 
> -	/* If polling, assume that the card is always present. */
> -	if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
> -		return 1;

Why simply  remove these code rather than use this:
If(host->mmc->caps & MMC_CAP_NEEDS_POLL)
       Return 1;

According to the comment, if polling, need to return 1,
But if you just simply remove this code, it will read the sdhci register SDHCI_PRESENT_STATE


Best Regards

Haibo Chen

> -
>  	/* Host native card detect */
>  	return !!(sdhci_readl(host, SDHCI_PRESENT_STATE) &
> SDHCI_CARD_PRESENT);  } @@ -2658,7 +2655,7 @@ void
> sdhci_enable_irq_wakeups(struct sdhci_host *host)
>  	val = sdhci_readb(host, SDHCI_WAKE_UP_CONTROL);
>  	val |= mask ;
>  	/* Avoid fake wake up */
> -	if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
> +	if (host->mmc->caps & MMC_CAP_NEEDS_POLL)
>  		val &= ~(SDHCI_WAKE_ON_INSERT |
> SDHCI_WAKE_ON_REMOVE);
>  	sdhci_writeb(host, val, SDHCI_WAKE_UP_CONTROL);  } @@ -3112,8
> +3109,7 @@ int sdhci_add_host(struct sdhci_host *host)
>  	if (caps[0] & SDHCI_CAN_DO_HISPD)
>  		mmc->caps |= MMC_CAP_SD_HIGHSPEED |
> MMC_CAP_MMC_HIGHSPEED;
> 
> -	if ((host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) &&
> -	    !(mmc->caps & MMC_CAP_NONREMOVABLE) &&
> +	if (!(mmc->caps & MMC_CAP_NONREMOVABLE) &&
>  	    IS_ERR_VALUE(mmc_gpio_get_cd(host->mmc)))
>  		mmc->caps |= MMC_CAP_NEEDS_POLL;
> 
> --
> 2.3.7
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [RFC PATCH 10/21] mmc: sdhci: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
  2016-01-27  7:11     ` Haibo Chen
@ 2016-01-27  7:20       ` Shawn Lin
  -1 siblings, 0 replies; 48+ messages in thread
From: Shawn Lin @ 2016-01-27  7:20 UTC (permalink / raw)
  To: Haibo Chen, Ulf Hansson
  Cc: shawn.lin, bcm-kernel-feedback-list, linux-rpi-kernel, linux-mmc,
	linux-kernel

On 2016/1/27 15:11, Haibo Chen wrote:
>
>
> Hi Shawn,
>
>

[...]

>> @@ -1618,7 +1618,8 @@ static int sdhci_do_get_cd(struct sdhci_host *host)
>>   		return 0;
>>
>>   	/* If nonremovable, assume that the card is always present. */
>> -	if (host->mmc->caps & MMC_CAP_NONREMOVABLE)
>> +	if (host->mmc->caps & MMC_CAP_NONREMOVABLE ||
>> +	    host->mmc->caps & MMC_CAP_NEEDS_POLL)
>>   		return 1;
>>
>>   	/*
>> @@ -1628,10 +1629,6 @@ static int sdhci_do_get_cd(struct sdhci_host *host)
>>   	if (!IS_ERR_VALUE(gpio_cd))
>>   		return !!gpio_cd;
>>
>> -	/* If polling, assume that the card is always present. */
>> -	if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
>> -		return 1;
>
> Why simply  remove these code rather than use this:
> If(host->mmc->caps & MMC_CAP_NEEDS_POLL)
>         Return 1;
>
> According to the comment, if polling, need to return 1,
> But if you just simply remove this code, it will read the sdhci register SDHCI_PRESENT_STATE

Hi Haibo

I have combine this check with "host->mmc->caps & MMC_CAP_NONREMOVABLE" 
above. :)

Thanks

>
>
> Best Regards
>
> Haibo Chen
>
>> -
>>   	/* Host native card detect */
>>   	return !!(sdhci_readl(host, SDHCI_PRESENT_STATE) &
>> SDHCI_CARD_PRESENT);  } @@ -2658,7 +2655,7 @@ void
>> sdhci_enable_irq_wakeups(struct sdhci_host *host)
>>   	val = sdhci_readb(host, SDHCI_WAKE_UP_CONTROL);
>>   	val |= mask ;
>>   	/* Avoid fake wake up */
>> -	if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
>> +	if (host->mmc->caps & MMC_CAP_NEEDS_POLL)
>>   		val &= ~(SDHCI_WAKE_ON_INSERT |
>> SDHCI_WAKE_ON_REMOVE);
>>   	sdhci_writeb(host, val, SDHCI_WAKE_UP_CONTROL);  } @@ -3112,8
>> +3109,7 @@ int sdhci_add_host(struct sdhci_host *host)
>>   	if (caps[0] & SDHCI_CAN_DO_HISPD)
>>   		mmc->caps |= MMC_CAP_SD_HIGHSPEED |
>> MMC_CAP_MMC_HIGHSPEED;
>>
>> -	if ((host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) &&
>> -	    !(mmc->caps & MMC_CAP_NONREMOVABLE) &&
>> +	if (!(mmc->caps & MMC_CAP_NONREMOVABLE) &&
>>   	    IS_ERR_VALUE(mmc_gpio_get_cd(host->mmc)))
>>   		mmc->caps |= MMC_CAP_NEEDS_POLL;
>>
>> --
>> 2.3.7
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
>


-- 
Best Regards
Shawn Lin

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [RFC PATCH 10/21] mmc: sdhci: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
@ 2016-01-27  7:20       ` Shawn Lin
  0 siblings, 0 replies; 48+ messages in thread
From: Shawn Lin @ 2016-01-27  7:20 UTC (permalink / raw)
  To: Haibo Chen, Ulf Hansson
  Cc: shawn.lin, bcm-kernel-feedback-list, linux-rpi-kernel, linux-mmc,
	linux-kernel

On 2016/1/27 15:11, Haibo Chen wrote:
>
>
> Hi Shawn,
>
>

[...]

>> @@ -1618,7 +1618,8 @@ static int sdhci_do_get_cd(struct sdhci_host *host)
>>   		return 0;
>>
>>   	/* If nonremovable, assume that the card is always present. */
>> -	if (host->mmc->caps & MMC_CAP_NONREMOVABLE)
>> +	if (host->mmc->caps & MMC_CAP_NONREMOVABLE ||
>> +	    host->mmc->caps & MMC_CAP_NEEDS_POLL)
>>   		return 1;
>>
>>   	/*
>> @@ -1628,10 +1629,6 @@ static int sdhci_do_get_cd(struct sdhci_host *host)
>>   	if (!IS_ERR_VALUE(gpio_cd))
>>   		return !!gpio_cd;
>>
>> -	/* If polling, assume that the card is always present. */
>> -	if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
>> -		return 1;
>
> Why simply  remove these code rather than use this:
> If(host->mmc->caps & MMC_CAP_NEEDS_POLL)
>         Return 1;
>
> According to the comment, if polling, need to return 1,
> But if you just simply remove this code, it will read the sdhci register SDHCI_PRESENT_STATE

Hi Haibo

I have combine this check with "host->mmc->caps & MMC_CAP_NONREMOVABLE" 
above. :)

Thanks

>
>
> Best Regards
>
> Haibo Chen
>
>> -
>>   	/* Host native card detect */
>>   	return !!(sdhci_readl(host, SDHCI_PRESENT_STATE) &
>> SDHCI_CARD_PRESENT);  } @@ -2658,7 +2655,7 @@ void
>> sdhci_enable_irq_wakeups(struct sdhci_host *host)
>>   	val = sdhci_readb(host, SDHCI_WAKE_UP_CONTROL);
>>   	val |= mask ;
>>   	/* Avoid fake wake up */
>> -	if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
>> +	if (host->mmc->caps & MMC_CAP_NEEDS_POLL)
>>   		val &= ~(SDHCI_WAKE_ON_INSERT |
>> SDHCI_WAKE_ON_REMOVE);
>>   	sdhci_writeb(host, val, SDHCI_WAKE_UP_CONTROL);  } @@ -3112,8
>> +3109,7 @@ int sdhci_add_host(struct sdhci_host *host)
>>   	if (caps[0] & SDHCI_CAN_DO_HISPD)
>>   		mmc->caps |= MMC_CAP_SD_HIGHSPEED |
>> MMC_CAP_MMC_HIGHSPEED;
>>
>> -	if ((host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) &&
>> -	    !(mmc->caps & MMC_CAP_NONREMOVABLE) &&
>> +	if (!(mmc->caps & MMC_CAP_NONREMOVABLE) &&
>>   	    IS_ERR_VALUE(mmc_gpio_get_cd(host->mmc)))
>>   		mmc->caps |= MMC_CAP_NEEDS_POLL;
>>
>> --
>> 2.3.7
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
>


-- 
Best Regards
Shawn Lin


^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk
  2016-01-27  5:05 [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk Shawn Lin
                   ` (20 preceding siblings ...)
  2016-01-27  5:09 ` [RFC PATCH 21/21] mmc: sdhci.h: " Shawn Lin
@ 2016-01-27 12:59 ` Adrian Hunter
  2016-01-27 13:23   ` Russell King - ARM Linux
  21 siblings, 1 reply; 48+ messages in thread
From: Adrian Hunter @ 2016-01-27 12:59 UTC (permalink / raw)
  To: Shawn Lin, Ulf Hansson
  Cc: bcm-kernel-feedback-list, linux-rpi-kernel, linux-mmc,
	linux-kernel, Russell King

On 27/01/16 07:05, Shawn Lin wrote:
> Ulf wants to make sdhci into a library, but it's a huge task
> since any improvemts may touch too much platforms. But at least
> we should make some effort to push things torwards to this target.
> 
> This patchset remove SDHCI_QUIRK_BROKEN_CARD_DETECTION from sdhci
> to gradually reduce quirk of sdhci.
> 
> Firstly, SDHCI_QUIRK_BROKEN_CARD_DETECTION aims at claiming the slot is
> a "broken-cd" one, but "broken-cd" is not a quirk from my view.
> In addition, mmc core stack had already obtain "broken-cd" from dts via
> mmc_of_parse and pass MMC_CAP_NEEDS_POLL to mmc->caps. So we can reuse it
> instead of SDHCI_QUIRK_BROKEN_CARD_DETECTION.

That assumes there is no driver that wants to disable the card detect
interrupts and disable the use of the Present State register, but still use
a Card Detect GPIO and therefore not have MMC_CAP_NEEDS_POLL.

A way forward should provide for drivers do to things like that.

One way is to make selected existing functions into library functions and
provide callbacks for them.  In this case do with sdhci_set_card_detection()
what Russell King did with sdhci_reset().  However Ulf is against new callbacks.

I would much prefer the structure for a SDHCI library be put in place, or at
least agreed to, before individual quirks are tackled.

In my view Ulf needs to explain how the SDHCI library is going to work,
particularly in the absence of new callbacks.

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk
  2016-01-27 12:59 ` [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk Adrian Hunter
@ 2016-01-27 13:23   ` Russell King - ARM Linux
  2016-01-27 15:07     ` Ulf Hansson
  0 siblings, 1 reply; 48+ messages in thread
From: Russell King - ARM Linux @ 2016-01-27 13:23 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Shawn Lin, Ulf Hansson, bcm-kernel-feedback-list,
	linux-rpi-kernel, linux-mmc, linux-kernel

On Wed, Jan 27, 2016 at 02:59:14PM +0200, Adrian Hunter wrote:
> In my view Ulf needs to explain how the SDHCI library is going to work,
> particularly in the absence of new callbacks.

We need to add new callbacks as part of the conversion to a library,
otherwise we're very much into a total rewrite from scratch (which
I think is far too much work, and prone to errors) or a big flag day
to switch everything over (which will require a moritorium on sdhci
patches while the effort to switch everything is ongoing.)

Both of those approaches suffer from one huge drawback: there is no
way to bisect between them to locate the cause of a regression.

A piece-meal approach, where the driver is gradually converted is a
far saner approach, because it means that each conversion in the step
can be done as a series of transformations, which not only can be
properly reviewed, but also bisected - and that is _hugely_ important
for the existing state of SDHCI.

The chances of some hardware behavioural quirk being missed while
trying to convert SDHCI to a library are _extremely_ high, and the
only sane approach to this is one which allows a progressive
transformation of the driver.

Also, the last thing we want is for drivers to end up duplicating
entire functions from sdhci.c just because they have one thing
different (eg, because they need to do something in the middle of
a set_ios() call which no other SDHCI driver needs.)  Having such
code duplication will just make maintanence even more of a
nightmare.

set_ios() is probably one of the worst functions in sdhci right now,
and there's no obvious way to split it up into several stand-alone
functions which drivers could chain together.

I think what needs to happen here is that Ulf needs to leave such
decisions about what is acceptable or unacceptable to those who are
trying to convert sdhci to a library, otherwise the conversion will
probably never happen... unless Ulf wants to get directly involved
in the conversion effort, producing patches to make it happen.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk
  2016-01-27 13:23   ` Russell King - ARM Linux
@ 2016-01-27 15:07     ` Ulf Hansson
  2016-01-28  2:17       ` Shawn Lin
  2016-01-28 12:03       ` Adrian Hunter
  0 siblings, 2 replies; 48+ messages in thread
From: Ulf Hansson @ 2016-01-27 15:07 UTC (permalink / raw)
  To: Russell King - ARM Linux, Adrian Hunter
  Cc: Shawn Lin, bcm-kernel-feedback-list, linux-rpi-kernel, linux-mmc,
	linux-kernel

On 27 January 2016 at 14:23, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Wed, Jan 27, 2016 at 02:59:14PM +0200, Adrian Hunter wrote:
>> In my view Ulf needs to explain how the SDHCI library is going to work,
>> particularly in the absence of new callbacks.
>
> We need to add new callbacks as part of the conversion to a library,
> otherwise we're very much into a total rewrite from scratch (which
> I think is far too much work, and prone to errors) or a big flag day
> to switch everything over (which will require a moritorium on sdhci
> patches while the effort to switch everything is ongoing.)
>
> Both of those approaches suffer from one huge drawback: there is no
> way to bisect between them to locate the cause of a regression.
>
> A piece-meal approach, where the driver is gradually converted is a
> far saner approach, because it means that each conversion in the step
> can be done as a series of transformations, which not only can be
> properly reviewed, but also bisected - and that is _hugely_ important
> for the existing state of SDHCI.
>
> The chances of some hardware behavioural quirk being missed while
> trying to convert SDHCI to a library are _extremely_ high, and the
> only sane approach to this is one which allows a progressive
> transformation of the driver.
>
> Also, the last thing we want is for drivers to end up duplicating
> entire functions from sdhci.c just because they have one thing
> different (eg, because they need to do something in the middle of
> a set_ios() call which no other SDHCI driver needs.)  Having such
> code duplication will just make maintanence even more of a
> nightmare.
>
> set_ios() is probably one of the worst functions in sdhci right now,
> and there's no obvious way to split it up into several stand-alone
> functions which drivers could chain together.
>
> I think what needs to happen here is that Ulf needs to leave such
> decisions about what is acceptable or unacceptable to those who are
> trying to convert sdhci to a library, otherwise the conversion will
> probably never happen... unless Ulf wants to get directly involved
> in the conversion effort, producing patches to make it happen.
>

I don't intend to contribute much with actual patches. I am willing to
help review and also help with expertise around the PM related parts.

I do realize that some callbacks may still be needed, even in the end
when sdhci has become a pure library. Although, those should be far
less then those we have today.

Currently I am more or less unable to properly maintain sdhci because
of it's bad code structure. Therefore I have taken a quite simple
approach by rejecting new callbacks and quirks, in a way to prevent it
from being worse. To me, the best way forward would be if some of you
experienced sdhci developers stepped in as a maintainer for it. In
that way, I can trust the development moving in the "library
direction" so I can pull back from nacking potential interim sdhci
callbacks/quirks.

Does it make sense?

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk
  2016-01-27 15:07     ` Ulf Hansson
@ 2016-01-28  2:17       ` Shawn Lin
  2016-01-28 11:29         ` One Thousand Gnomes
  2016-01-28 12:03       ` Adrian Hunter
  1 sibling, 1 reply; 48+ messages in thread
From: Shawn Lin @ 2016-01-28  2:17 UTC (permalink / raw)
  To: Ulf Hansson, Russell King - ARM Linux, Adrian Hunter
  Cc: shawn.lin, bcm-kernel-feedback-list, linux-rpi-kernel, linux-mmc,
	linux-kernel

On 2016/1/27 23:07, Ulf Hansson wrote:
> On 27 January 2016 at 14:23, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
>> On Wed, Jan 27, 2016 at 02:59:14PM +0200, Adrian Hunter wrote:
>>> In my view Ulf needs to explain how the SDHCI library is going to work,
>>> particularly in the absence of new callbacks.
>>
>> We need to add new callbacks as part of the conversion to a library,
>> otherwise we're very much into a total rewrite from scratch (which
>> I think is far too much work, and prone to errors) or a big flag day
>> to switch everything over (which will require a moritorium on sdhci
>> patches while the effort to switch everything is ongoing.)
>>

Totally agreed. Maybe that is the reason that frighten some volunteers
to make the conversion.

>> Both of those approaches suffer from one huge drawback: there is no
>> way to bisect between them to locate the cause of a regression.
>>

[...]

>>
>> I think what needs to happen here is that Ulf needs to leave such
>> decisions about what is acceptable or unacceptable to those who are
>> trying to convert sdhci to a library, otherwise the conversion will
>> probably never happen... unless Ulf wants to get directly involved
>> in the conversion effort, producing patches to make it happen.
>>
>
> I don't intend to contribute much with actual patches. I am willing to
> help review and also help with expertise around the PM related parts.
>
> I do realize that some callbacks may still be needed, even in the end
> when sdhci has become a pure library. Although, those should be far
> less then those we have today.
>
> Currently I am more or less unable to properly maintain sdhci because
> of it's bad code structure. Therefore I have taken a quite simple
> approach by rejecting new callbacks and quirks, in a way to prevent it
> from being worse.

Ulf, I do understand your situation. sdhci makes you exhausted and no
one seems able to maintain it properly. But preventing it from being
worse doesn't mean making it better, right?

I'am not a experienced sdhci expert, so when I read the sdhci code
for the first time last summer, it does shock me a lot. So many
historical burden it takes with lots kinds of *quirks*, I even cannot
undertand why some quirks are need since git-blame just tell me some
useless info because somebody do some coding-style fix or moving code
here and there, which makes me hard to trace the changes. Another fact,
how to test these changes for diff hardwares?? Without the help of
variant drivers folks, it cannot go a step. If we split our "library
direction" movement, do you think all the variant drivers are willing
to test all the patchsets for it? I don't think so.

Here, I come up with a bold and tentative proposal:
If someone is willing to be the maintainer of sdhci, he/she can create
a separate files and rewrite it to be a library. Then we reject to
accept any new variant drivers to use the old sdhci struct and encourage
it to fit the library one. Then ask the variant drivers who use the old
struct to migrate its code to fit the library. Until all done, remove
the old sdhci. Does that make sense?


To me, the best way forward would be if some of you
> experienced sdhci developers stepped in as a maintainer for it. In
> that way, I can trust the development moving in the "library
> direction" so I can pull back from nacking potential interim sdhci
> callbacks/quirks.
>
> Does it make sense?
>
> Kind regards
> Uffe
>
>
>


-- 
Best Regards
Shawn Lin

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk
  2016-01-28  2:17       ` Shawn Lin
@ 2016-01-28 11:29         ` One Thousand Gnomes
  2016-01-28 15:03           ` Ulf Hansson
  0 siblings, 1 reply; 48+ messages in thread
From: One Thousand Gnomes @ 2016-01-28 11:29 UTC (permalink / raw)
  To: Shawn Lin
  Cc: Ulf Hansson, Russell King - ARM Linux, Adrian Hunter,
	bcm-kernel-feedback-list, linux-rpi-kernel, linux-mmc,
	linux-kernel

On Thu, 28 Jan 2016 10:17:11 +0800
Shawn Lin <shawn.lin@rock-chips.com> wrote:

> On 2016/1/27 23:07, Ulf Hansson wrote:
> > On 27 January 2016 at 14:23, Russell King - ARM Linux
> > <linux@arm.linux.org.uk> wrote:  
> >> On Wed, Jan 27, 2016 at 02:59:14PM +0200, Adrian Hunter wrote:  
> >>> In my view Ulf needs to explain how the SDHCI library is going to work,
> >>> particularly in the absence of new callbacks.  
> >>
> >> We need to add new callbacks as part of the conversion to a library,
> >> otherwise we're very much into a total rewrite from scratch (which
> >> I think is far too much work, and prone to errors) or a big flag day
> >> to switch everything over (which will require a moritorium on sdhci
> >> patches while the effort to switch everything is ongoing.)

Agreed that won't work. It never works. You simply end up with a bigger
mess, a giant backlog and lots of people sending patches "on appeal" to
Linus/GregKH because the maintainer is being unreasonable. At that point
it all gets messy. It has been done (IDE) where the old core code simply
couldn't cope with the new hardware, but it was a nightmare

> > of it's bad code structure. Therefore I have taken a quite simple
> > approach by rejecting new callbacks and quirks, in a way to prevent it
> > from being worse.  

Which merely guarantees that the problem gets worse, because everyone
just puts their SD patches into Android trees instead and then when that
device is needed in Linux proper the crap hits the fan or people write
uglier and more hideous hacks buried elsewhere.

Eventually something gives way, and it will always be the maintainer,
because everyone needs to get their devices supported. You can guide new
callbacks in constructive ways but not stop them.

> If someone is willing to be the maintainer of sdhci, he/she can create
> a separate files and rewrite it to be a library. Then we reject to
> accept any new variant drivers to use the old sdhci struct and encourage
> it to fit the library one. Then ask the variant drivers who use the old
> struct to migrate its code to fit the library. Until all done, remove
> the old sdhci. Does that make sense?

I don't think it's the best idea. From past experience with other layers
of code the most effective approach (except for IDE which *was* a dead
loss and rewritten) has been something like this

- Take a small function with a quirk in it that one driver uses
- Split it into a clean function and driver specific function, put one in
  the driver and one in the core
- Replace the call to it with a call to ->ops->whatever()
- Add it to the various drivers operations structure as it gets invented

repeat until done.

Some things are shared so you end up moving vendor common quirks to
perhaps an "intel" file or a "designware" file rather than a single
driver, but the theory is the same and it ends up in effect as a class
with some overridden methods.

After this cycle has been repeated a bit you actually have the real
structure of the core code and the areas it diverges are visible. All your
ops-> methods are the points of diverge and clearly labelled. If you want
you can then go through finding all those that fit the pattern where they
diverge in the form

	do_stuff
	standard_method()
	do_stuff

and if the ops callback is not deeply buried look at kicking the whole
method back out completely so it becomes a true library method that
drivers can wrap or call.

It can be done. The 8250 serial driver has been going through this
process for some time (although it didn't start from quite so bad a
position as the sd layer), and the tty layer went through chunks of it
several times in its history.

And above all - it bisects.

Alan

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk
  2016-01-27 15:07     ` Ulf Hansson
  2016-01-28  2:17       ` Shawn Lin
@ 2016-01-28 12:03       ` Adrian Hunter
  2016-01-28 15:16         ` Ulf Hansson
  1 sibling, 1 reply; 48+ messages in thread
From: Adrian Hunter @ 2016-01-28 12:03 UTC (permalink / raw)
  To: Ulf Hansson, Russell King - ARM Linux
  Cc: Shawn Lin, bcm-kernel-feedback-list, linux-rpi-kernel, linux-mmc,
	linux-kernel, P L Sai Krishna, Wan Zongshun

On 27/01/16 17:07, Ulf Hansson wrote:
> On 27 January 2016 at 14:23, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
>> On Wed, Jan 27, 2016 at 02:59:14PM +0200, Adrian Hunter wrote:
>>> In my view Ulf needs to explain how the SDHCI library is going to work,
>>> particularly in the absence of new callbacks.
>>
>> We need to add new callbacks as part of the conversion to a library,
>> otherwise we're very much into a total rewrite from scratch (which
>> I think is far too much work, and prone to errors) or a big flag day
>> to switch everything over (which will require a moritorium on sdhci
>> patches while the effort to switch everything is ongoing.)
>>
>> Both of those approaches suffer from one huge drawback: there is no
>> way to bisect between them to locate the cause of a regression.
>>
>> A piece-meal approach, where the driver is gradually converted is a
>> far saner approach, because it means that each conversion in the step
>> can be done as a series of transformations, which not only can be
>> properly reviewed, but also bisected - and that is _hugely_ important
>> for the existing state of SDHCI.
>>
>> The chances of some hardware behavioural quirk being missed while
>> trying to convert SDHCI to a library are _extremely_ high, and the
>> only sane approach to this is one which allows a progressive
>> transformation of the driver.
>>
>> Also, the last thing we want is for drivers to end up duplicating
>> entire functions from sdhci.c just because they have one thing
>> different (eg, because they need to do something in the middle of
>> a set_ios() call which no other SDHCI driver needs.)  Having such
>> code duplication will just make maintanence even more of a
>> nightmare.
>>
>> set_ios() is probably one of the worst functions in sdhci right now,
>> and there's no obvious way to split it up into several stand-alone
>> functions which drivers could chain together.
>>
>> I think what needs to happen here is that Ulf needs to leave such
>> decisions about what is acceptable or unacceptable to those who are
>> trying to convert sdhci to a library, otherwise the conversion will
>> probably never happen... unless Ulf wants to get directly involved
>> in the conversion effort, producing patches to make it happen.
>>
> 
> I don't intend to contribute much with actual patches. I am willing to
> help review and also help with expertise around the PM related parts.
> 
> I do realize that some callbacks may still be needed, even in the end
> when sdhci has become a pure library. Although, those should be far
> less then those we have today.
> 
> Currently I am more or less unable to properly maintain sdhci because
> of it's bad code structure. Therefore I have taken a quite simple
> approach by rejecting new callbacks and quirks, in a way to prevent it
> from being worse. To me, the best way forward would be if some of you
> experienced sdhci developers stepped in as a maintainer for it. In
> that way, I can trust the development moving in the "library
> direction" so I can pull back from nacking potential interim sdhci
> callbacks/quirks.
> 
> Does it make sense?

I am happy to help and even be the SDHCI maintainer if Russell King and
others agree.  I have an interest in sdhci-acpi and sdhci-pci and also there
is UHS-II and ADMA3 on the horizon.

I agree with Russell that a re-write would introduce more bugs and more work
than it would be worth.  Making many small steps in the general direction is
preferable.

Initially it would nice to see it made easy for drivers to replace specific
mmc ops and sdhci ops and then call the standard version before/after doing
some custom code.  For example, P L Sai Krishna's auto-tuning problem might
be solved by something to the effect of:

int arasan_execute_tuning(struct mmc_host *mmc, u32 opcode)
{
	struct sdhci_host *host = mmc_priv(mmc);
	int err;

	err = sdhci_execute_tuning(mmc, opcode);
	if (!err)
		arasan_tune_sdclk(host);
	return err;
}

And Wan Zongshun also wanted to be able directly to replace
sdhci_execute_tuning() from sdhci-pci.

As suggested, my get_cd problem could also be solved by replacing the mmc
get_cd op.

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk
  2016-01-28 11:29         ` One Thousand Gnomes
@ 2016-01-28 15:03           ` Ulf Hansson
  2016-01-28 15:54             ` One Thousand Gnomes
  0 siblings, 1 reply; 48+ messages in thread
From: Ulf Hansson @ 2016-01-28 15:03 UTC (permalink / raw)
  To: One Thousand Gnomes
  Cc: Shawn Lin, Russell King - ARM Linux, Adrian Hunter,
	bcm-kernel-feedback-list, linux-rpi-kernel, linux-mmc,
	linux-kernel

[...]

>
>> > of it's bad code structure. Therefore I have taken a quite simple
>> > approach by rejecting new callbacks and quirks, in a way to prevent it
>> > from being worse.
>
> Which merely guarantees that the problem gets worse, because everyone
> just puts their SD patches into Android trees instead and then when that
> device is needed in Linux proper the crap hits the fan or people write
> uglier and more hideous hacks buried elsewhere.
>
> Eventually something gives way, and it will always be the maintainer,
> because everyone needs to get their devices supported. You can guide new
> callbacks in constructive ways but not stop them.

Well, I did stop them at least temporary.

Although, I have been telling people *why* and also trying to give
some guidelines of how I wanted this to move forward.

I understand some become frustrated from getting patches nacked like this.

In principle I have requested them to help evolving sdhci in a new and
better direction, instead of adding yet more hacks. That of course
requires a deeper understanding of both the mmc core, but also sdhci
in general.

[...]

Also, thanks for sharing your experience in this field. You made some
good points!

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk
  2016-01-28 12:03       ` Adrian Hunter
@ 2016-01-28 15:16         ` Ulf Hansson
  2016-01-28 16:27           ` Russell King - ARM Linux
  2016-01-29 12:08           ` Adrian Hunter
  0 siblings, 2 replies; 48+ messages in thread
From: Ulf Hansson @ 2016-01-28 15:16 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Russell King - ARM Linux, Shawn Lin, bcm-kernel-feedback-list,
	linux-rpi-kernel, linux-mmc, linux-kernel, P L Sai Krishna,
	Wan Zongshun

[...]

>> I don't intend to contribute much with actual patches. I am willing to
>> help review and also help with expertise around the PM related parts.
>>
>> I do realize that some callbacks may still be needed, even in the end
>> when sdhci has become a pure library. Although, those should be far
>> less then those we have today.
>>
>> Currently I am more or less unable to properly maintain sdhci because
>> of it's bad code structure. Therefore I have taken a quite simple
>> approach by rejecting new callbacks and quirks, in a way to prevent it
>> from being worse. To me, the best way forward would be if some of you
>> experienced sdhci developers stepped in as a maintainer for it. In
>> that way, I can trust the development moving in the "library
>> direction" so I can pull back from nacking potential interim sdhci
>> callbacks/quirks.
>>
>> Does it make sense?
>
> I am happy to help and even be the SDHCI maintainer if Russell King and
> others agree.  I have an interest in sdhci-acpi and sdhci-pci and also there
> is UHS-II and ADMA3 on the horizon.

That's really great news. Thank you very much Adrian!

Perhaps Russell is willing to help co-maintain it?

>
> I agree with Russell that a re-write would introduce more bugs and more work
> than it would be worth.  Making many small steps in the general direction is
> preferable.
>
> Initially it would nice to see it made easy for drivers to replace specific
> mmc ops and sdhci ops and then call the standard version before/after doing
> some custom code.  For example, P L Sai Krishna's auto-tuning problem might
> be solved by something to the effect of:
>
> int arasan_execute_tuning(struct mmc_host *mmc, u32 opcode)
> {
>         struct sdhci_host *host = mmc_priv(mmc);
>         int err;
>
>         err = sdhci_execute_tuning(mmc, opcode);
>         if (!err)
>                 arasan_tune_sdclk(host);
>         return err;
> }
>
> And Wan Zongshun also wanted to be able directly to replace
> sdhci_execute_tuning() from sdhci-pci.
>
> As suggested, my get_cd problem could also be solved by replacing the mmc
> get_cd op.
>

Sounds like a perfect plan!

Do you want to send a patch to the MAINTAINERS file?

>From my side I can also continue doing the administrative part of the
work, so there's need for you to set up a separate git tree or send
pull request. At least initially.
Instead I will just pick patches that's been acked by you (and
possibly Russell).

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk
  2016-01-28 15:03           ` Ulf Hansson
@ 2016-01-28 15:54             ` One Thousand Gnomes
  0 siblings, 0 replies; 48+ messages in thread
From: One Thousand Gnomes @ 2016-01-28 15:54 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Shawn Lin, Russell King - ARM Linux, Adrian Hunter,
	bcm-kernel-feedback-list, linux-rpi-kernel, linux-mmc,
	linux-kernel

On Thu, 28 Jan 2016 16:03:34 +0100
Ulf Hansson <ulf.hansson@linaro.org> wrote:

> [...]
> 
> >  
> >> > of it's bad code structure. Therefore I have taken a quite simple
> >> > approach by rejecting new callbacks and quirks, in a way to prevent it
> >> > from being worse.  
> >
> > Which merely guarantees that the problem gets worse, because everyone
> > just puts their SD patches into Android trees instead and then when that
> > device is needed in Linux proper the crap hits the fan or people write
> > uglier and more hideous hacks buried elsewhere.
> >
> > Eventually something gives way, and it will always be the maintainer,
> > because everyone needs to get their devices supported. You can guide new
> > callbacks in constructive ways but not stop them.  
> 
> Well, I did stop them at least temporary.

I always describe it as "putting a cork in the sewerage pipe". It might
stop it for a bit but

a) you don't want to be too close when it breaks
b) it's not good what happens further up the pipe

> 
> Although, I have been telling people *why* and also trying to give
> some guidelines of how I wanted this to move forward.
> 
> I understand some become frustrated from getting patches nacked like this.
> 
> In principle I have requested them to help evolving sdhci in a new and
> better direction, instead of adding yet more hacks. That of course
> requires a deeper understanding of both the mmc core, but also sdhci
> in general.
> 
> [...]
> 
> Also, thanks for sharing your experience in this field. You made some
> good points!

I'm happy to help try and sort the code out. Not maintain it - my
knowledge of the intricacies of SDHCI is not good enough.

Alan

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk
  2016-01-28 15:16         ` Ulf Hansson
@ 2016-01-28 16:27           ` Russell King - ARM Linux
  2016-01-29 12:08           ` Adrian Hunter
  1 sibling, 0 replies; 48+ messages in thread
From: Russell King - ARM Linux @ 2016-01-28 16:27 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Adrian Hunter, Shawn Lin, bcm-kernel-feedback-list,
	linux-rpi-kernel, linux-mmc, linux-kernel, P L Sai Krishna,
	Wan Zongshun

On Thu, Jan 28, 2016 at 04:16:27PM +0100, Ulf Hansson wrote:
> That's really great news. Thank you very much Adrian!
> 
> Perhaps Russell is willing to help co-maintain it?

Unfortunately, I'm not really in a position to co-maintain it, as
I'd be doing it in my spare time, and my spare time is already
spread thinly over much of the kernel (I've crypto, phy/sfp,
etnaviv drm, xf86-video-armada, etnaviv mesa, HDMI CEC, and DSA
stuff all wanting attention, and I'm having to tell people "sorry,
I won't be able to do anything on XYZ for a while".)  I'm quite
sure the Marvell Dove maintainers are going to want some more
patches from me for the next merge window soon...

Of course, that could change if Linaro wishes to fund work in this
area, because it would then take priority... ;)

I'm more than willing to put whatever time I can into helping with
SDHCI, which is exactly why you get the occasional large series of
patches from me - the whole reason behind these patch series are to
improve SDHCI in an incremental fashion.  However, I'd be lying if
I didn't say that there's an alterior motive behind it, which is to
get SDHCI on iMX6 stable.  What I realise is that the current
situation is quite dire, and SDHCI needs improving if we're not
going to decend into a totally unmaintainable mess.

Just before Christmas, I was working on a way to change the way we
kick off commands in SDHCI in order to clean up those paths - the
patches which follow on from the 25 I've already posted, and are:

mmc: sdhci: move interrupt enable settings to task
mmc: sdhci: move command and argument to sdhci_cmd_task
mmc: sdhci: move block size and block count into sdhci task structure
mmc: sdhci: compute transfer mode separately from programming it
mmc: sdhci: replace 'set_timeout' method with 'calculate_timeout'
mmc: sdhci: validate command response type
mmc: sdhci: clean up
mmc: sdhci: rearrange sdhci_set_transfer_mode() a little

It's a work-in-progress, which is why I haven't posted these yet.
The outline of it is that we (currently) end up with:

+struct sdhci_cmd_task {
+       bool has_argument2;
+       bool has_data;
+       u32 ier;
+       u32 argument2;
+       u16 block_size;
+       u16 block_count;
+       u32 argument;
+       u16 transfer_mode;
+       u16 command;
+};
+

which allows separation of the preparation step for a MMC command from
touching the hardware - this means that quirkly SDHCI drivers can do
this instead of litering the core code with quirk tests:

	struct sdhci_cmd_task task;

	sdhci_prepare_command(sdhci, &task, mmc_command);

	/* do whatever quirks */

	sdhci_execute_command(sdhci, &task);

This structure means that (eg) some of the quirks such as
SDHCI_QUIRK2_SUPPORT_SINGLE,
SDHCI_QUIRK2_CLEAR_TRANSFERMODE_REG_BEFORE_CMD (which I'm sure is
actually a bug in the SDHCI driver) and eventually
SDHCI_QUIRK_BROKEN_TIMEOUT_VAL can all be eliminated from the core
code and moved out into their respective drivers - because we have
the core code preparing "the standard" set of register settings for
the command to be sent, and then the driver gets the opportunity to
tweak them according to the bugs it has.

... and even after all these changes, I still haven't solved the
problems which bugs me on iMX6, which were my motivation for coming
back to putting some more effort into SDHCI! :p

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk
  2016-01-28 15:16         ` Ulf Hansson
  2016-01-28 16:27           ` Russell King - ARM Linux
@ 2016-01-29 12:08           ` Adrian Hunter
  2016-01-29 17:28             ` Russell King - ARM Linux
  1 sibling, 1 reply; 48+ messages in thread
From: Adrian Hunter @ 2016-01-29 12:08 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Russell King - ARM Linux, Shawn Lin, bcm-kernel-feedback-list,
	linux-rpi-kernel, linux-mmc, linux-kernel, P L Sai Krishna,
	Wan Zongshun

On 28/01/16 17:16, Ulf Hansson wrote:
> [...]
> 
>>> I don't intend to contribute much with actual patches. I am willing to
>>> help review and also help with expertise around the PM related parts.
>>>
>>> I do realize that some callbacks may still be needed, even in the end
>>> when sdhci has become a pure library. Although, those should be far
>>> less then those we have today.
>>>
>>> Currently I am more or less unable to properly maintain sdhci because
>>> of it's bad code structure. Therefore I have taken a quite simple
>>> approach by rejecting new callbacks and quirks, in a way to prevent it
>>> from being worse. To me, the best way forward would be if some of you
>>> experienced sdhci developers stepped in as a maintainer for it. In
>>> that way, I can trust the development moving in the "library
>>> direction" so I can pull back from nacking potential interim sdhci
>>> callbacks/quirks.
>>>
>>> Does it make sense?
>>
>> I am happy to help and even be the SDHCI maintainer if Russell King and
>> others agree.  I have an interest in sdhci-acpi and sdhci-pci and also there
>> is UHS-II and ADMA3 on the horizon.
> 
> That's really great news. Thank you very much Adrian!
> 
> Perhaps Russell is willing to help co-maintain it?
> 
>>
>> I agree with Russell that a re-write would introduce more bugs and more work
>> than it would be worth.  Making many small steps in the general direction is
>> preferable.
>>
>> Initially it would nice to see it made easy for drivers to replace specific
>> mmc ops and sdhci ops and then call the standard version before/after doing
>> some custom code.  For example, P L Sai Krishna's auto-tuning problem might
>> be solved by something to the effect of:
>>
>> int arasan_execute_tuning(struct mmc_host *mmc, u32 opcode)
>> {
>>         struct sdhci_host *host = mmc_priv(mmc);
>>         int err;
>>
>>         err = sdhci_execute_tuning(mmc, opcode);
>>         if (!err)
>>                 arasan_tune_sdclk(host);
>>         return err;
>> }
>>
>> And Wan Zongshun also wanted to be able directly to replace
>> sdhci_execute_tuning() from sdhci-pci.
>>
>> As suggested, my get_cd problem could also be solved by replacing the mmc
>> get_cd op.
>>
> 
> Sounds like a perfect plan!
> 
> Do you want to send a patch to the MAINTAINERS file?

Yes, I'll do that.

> 
>>From my side I can also continue doing the administrative part of the
> work, so there's need for you to set up a separate git tree or send
> pull request. At least initially.
> Instead I will just pick patches that's been acked by you (and
> possibly Russell).

I might make a tree because I want to try to separate Russell's bug fixes
from the clean-ups, and then cc stable on the bug fixes.

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk
  2016-01-29 12:08           ` Adrian Hunter
@ 2016-01-29 17:28             ` Russell King - ARM Linux
  2016-02-01 12:32               ` Adrian Hunter
  0 siblings, 1 reply; 48+ messages in thread
From: Russell King - ARM Linux @ 2016-01-29 17:28 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Ulf Hansson, Shawn Lin, bcm-kernel-feedback-list,
	linux-rpi-kernel, linux-mmc, linux-kernel, P L Sai Krishna,
	Wan Zongshun

On Fri, Jan 29, 2016 at 02:08:21PM +0200, Adrian Hunter wrote:
> I might make a tree because I want to try to separate Russell's bug fixes
> from the clean-ups, and then cc stable on the bug fixes.

It would be good if you could ask for that, I'll look at rearranging
(and re-testing) the patches to achieve that.  Had I known that, I
could've done it before posting the latest set of patches.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk
  2016-01-29 17:28             ` Russell King - ARM Linux
@ 2016-02-01 12:32               ` Adrian Hunter
  0 siblings, 0 replies; 48+ messages in thread
From: Adrian Hunter @ 2016-02-01 12:32 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Ulf Hansson, Shawn Lin, bcm-kernel-feedback-list,
	linux-rpi-kernel, linux-mmc, linux-kernel, P L Sai Krishna,
	Wan Zongshun, Jisheng Zhang

On 29/01/16 19:28, Russell King - ARM Linux wrote:
> On Fri, Jan 29, 2016 at 02:08:21PM +0200, Adrian Hunter wrote:
>> I might make a tree because I want to try to separate Russell's bug fixes
>> from the clean-ups, and then cc stable on the bug fixes.
> 
> It would be good if you could ask for that, I'll look at rearranging
> (and re-testing) the patches to achieve that.  Had I known that, I
> could've done it before posting the latest set of patches.

If you don't mind doing that, that would be great!

So far the fixes, I have identified:

1. mmc: sdhci: command response CRC error handling

Could have "fix" in the subject i.e. "fix command response CRC error handling".

Could change to be dependent only on "mmc: sdhci: move initialisation of
command error member".  Also I would treat end-bit and index errors the same
as CRC errors i.e. probably end up with:

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index d622435d1bcc..6cae93a89eba 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2325,8 +2325,23 @@ static void sdhci_cmd_irq(struct sdhci_host *host,
u32 intmask, u32 *mask)
 	if (intmask & SDHCI_INT_TIMEOUT)
 		host->cmd->error = -ETIMEDOUT;
 	else if (intmask & (SDHCI_INT_CRC | SDHCI_INT_END_BIT |
-			SDHCI_INT_INDEX))
+			SDHCI_INT_INDEX)) {
 		host->cmd->error = -EILSEQ;
+		/*
+		 * If this command initiates a data phase and a response
+		 * CRC error is signalled, the card can start transferring
+		 * data - the card may have received the command without
+		 * error.  We must not terminate the mmc_request early.
+		 *
+		 * If the card did not receive the command or returned an
+		 * error which prevented it sending data, the data phase
+		 * will time out.
+		 */
+		if (host->cmd->data) {
+			host->cmd = NULL;
+			return;
+		}
+	}

 	if (host->cmd->error) {
 		tasklet_schedule(&host->finish_tasklet);

2. mmc: sdhci: avoid unnecessary mapping/unmapping of align buffer

This looks like a bug fix because the mapping can be leaked on the error
path i.e. similar problem to the one fixed by "mmc: sdhci: plug DMA mapping
leak on error"

3. mmc: sdhci: plug DMA mapping leak on error

It looks like the 2nd chunk could be taken as a separate fix without
dependence on other patches.

4. mmc: sdhci-pxav3: fix higher speed mode capabilities

I can't test this so please indicate if you want it for stable.

5. mmc: sdhci: further fix for DMA unmapping in sdhci_post_req()

Could be made independent of other patches.

6. mmc: sdhci: fix data timeout (part 1)
mmc: sdhci: fix data timeout (part 2)

Could be just 1 patch.

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* Re: [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk
  2016-01-27  5:04 Shawn Lin
@ 2016-02-04 10:40 ` Ulf Hansson
  0 siblings, 0 replies; 48+ messages in thread
From: Ulf Hansson @ 2016-02-04 10:40 UTC (permalink / raw)
  To: Shawn Lin, Adrian Hunter
  Cc: bcm-kernel-feedback-list, linux-rpi-kernel, linux-mmc, linux-kernel

+ Adrian

On 27 January 2016 at 06:04, Shawn Lin <shawn.lin@rock-chips.com> wrote:
>
> Ulf wants to make sdhci into a library, but it's a huge task
> since any improvemts may touch too much platforms. But at least
> we should make some effort to push things torwards to this target.
>
> This patchset remove SDHCI_QUIRK_BROKEN_CARD_DETECTION from sdhci
> to gradually reduce quirk of sdhci.
>
> Firstly, SDHCI_QUIRK_BROKEN_CARD_DETECTION aims at claiming the slot is
> a "broken-cd" one, but "broken-cd" is not a quirk from my view.
> In addition, mmc core stack had already obtain "broken-cd" from dts via
> mmc_of_parse and pass MMC_CAP_NEEDS_POLL to mmc->caps. So we can reuse it
> instead of SDHCI_QUIRK_BROKEN_CARD_DETECTION.
>
> However, before doing the cleanup work, I find that geting _of_ property
> for sdhci* is not so pretty good and consistent. Some variant drives use
> mmc_of_parse, while another ones use sdhci_get_of_property. I also find some
> variant drivers combine these two paths. So in order to make my "broken-cd"
> cleanup running, I decide to add mmc_of_parse into sdhci_get_of_property and
> replace mmc_of_parse with sdhci_get_of_property for all variant drivers if
> needed.
>
> Unfortunately, I don't have all these platforms touched to test my patchset.
> I might make some mistakes for these changes, so any comments are welcomed.
>
>
>
> Shawn Lin (21):
>   mmc: sdhci-pltfm: consolidate parsing path
>   mmc: sdhci-iproc: consolidate parsing path
>   mmc: sdhci-msm: consolidate parsing path
>   mmc: sdhci-of-arasan: consolidate parsing path
>   mmc: sdhci-of-at91: consolidate parsing path
>   mmc: sdhci-of-esdhc: consolidate parsing path
>   mmc: sdhci-pxav3: consolidate parsing path
>   mmc: sdhci-sirf: check sdhci_get_of_property return value
>   mmc: sdhci_f_sdh30: check sdhci_get_of_property return value
>   mmc: sdhci: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
>   mmc: sdhci-acpi: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
>   mmc: sdhci-bcm-kona: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
>   mmc: sdhci-bcm2835: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
>   mmc: sdhci-esdhc-imx: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
>   mmc: sdhci-msm: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
>   mmc: sdhci-of-esdhc: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
>   mmc: sdhci-pci-core: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
>   mmc: sdhci-pltfm: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
>   mmc: sdhci-pxav2: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
>   mmc: sdhci-s3c: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
>   mmc: sdhci.h: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
>
>  drivers/mmc/host/sdhci-acpi.c      |  3 +--
>  drivers/mmc/host/sdhci-bcm-kona.c  |  3 ---
>  drivers/mmc/host/sdhci-bcm2835.c   |  5 +++--
>  drivers/mmc/host/sdhci-esdhc-imx.c |  9 +++++----
>  drivers/mmc/host/sdhci-iproc.c     |  5 +++--
>  drivers/mmc/host/sdhci-msm.c       |  6 ++----
>  drivers/mmc/host/sdhci-of-arasan.c | 11 ++++-------
>  drivers/mmc/host/sdhci-of-at91.c   |  4 +---
>  drivers/mmc/host/sdhci-of-esdhc.c  | 25 ++++++++++++-------------
>  drivers/mmc/host/sdhci-pci-core.c  |  5 ++++-
>  drivers/mmc/host/sdhci-pltfm.c     | 26 +++++++++++++++++++-------
>  drivers/mmc/host/sdhci-pltfm.h     |  2 +-
>  drivers/mmc/host/sdhci-pxav2.c     |  1 -
>  drivers/mmc/host/sdhci-pxav3.c     |  4 +---
>  drivers/mmc/host/sdhci-s3c.c       |  2 +-
>  drivers/mmc/host/sdhci-sirf.c      |  4 +++-
>  drivers/mmc/host/sdhci.c           | 14 +++++---------
>  drivers/mmc/host/sdhci.h           | 30 ++++++++++++++----------------
>  drivers/mmc/host/sdhci_f_sdh30.c   |  5 ++++-
>  19 files changed, 83 insertions(+), 81 deletions(-)
>
> --
> 2.3.7
>
>

To avoid confusions. I don't intend to pick up any further sdhci
patches, unless they are acked by Adrian Hunter.

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk
@ 2016-01-27  5:04 Shawn Lin
  2016-02-04 10:40 ` Ulf Hansson
  0 siblings, 1 reply; 48+ messages in thread
From: Shawn Lin @ 2016-01-27  5:04 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: bcm-kernel-feedback-list, linux-rpi-kernel, linux-mmc,
	linux-kernel, Shawn Lin


Ulf wants to make sdhci into a library, but it's a huge task
since any improvemts may touch too much platforms. But at least
we should make some effort to push things torwards to this target.

This patchset remove SDHCI_QUIRK_BROKEN_CARD_DETECTION from sdhci
to gradually reduce quirk of sdhci.

Firstly, SDHCI_QUIRK_BROKEN_CARD_DETECTION aims at claiming the slot is
a "broken-cd" one, but "broken-cd" is not a quirk from my view.
In addition, mmc core stack had already obtain "broken-cd" from dts via
mmc_of_parse and pass MMC_CAP_NEEDS_POLL to mmc->caps. So we can reuse it
instead of SDHCI_QUIRK_BROKEN_CARD_DETECTION.

However, before doing the cleanup work, I find that geting _of_ property
for sdhci* is not so pretty good and consistent. Some variant drives use
mmc_of_parse, while another ones use sdhci_get_of_property. I also find some
variant drivers combine these two paths. So in order to make my "broken-cd"
cleanup running, I decide to add mmc_of_parse into sdhci_get_of_property and
replace mmc_of_parse with sdhci_get_of_property for all variant drivers if
needed.

Unfortunately, I don't have all these platforms touched to test my patchset.
I might make some mistakes for these changes, so any comments are welcomed.



Shawn Lin (21):
  mmc: sdhci-pltfm: consolidate parsing path
  mmc: sdhci-iproc: consolidate parsing path
  mmc: sdhci-msm: consolidate parsing path
  mmc: sdhci-of-arasan: consolidate parsing path
  mmc: sdhci-of-at91: consolidate parsing path
  mmc: sdhci-of-esdhc: consolidate parsing path
  mmc: sdhci-pxav3: consolidate parsing path
  mmc: sdhci-sirf: check sdhci_get_of_property return value
  mmc: sdhci_f_sdh30: check sdhci_get_of_property return value
  mmc: sdhci: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
  mmc: sdhci-acpi: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
  mmc: sdhci-bcm-kona: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
  mmc: sdhci-bcm2835: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
  mmc: sdhci-esdhc-imx: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
  mmc: sdhci-msm: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
  mmc: sdhci-of-esdhc: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
  mmc: sdhci-pci-core: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
  mmc: sdhci-pltfm: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
  mmc: sdhci-pxav2: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
  mmc: sdhci-s3c: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION
  mmc: sdhci.h: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION

 drivers/mmc/host/sdhci-acpi.c      |  3 +--
 drivers/mmc/host/sdhci-bcm-kona.c  |  3 ---
 drivers/mmc/host/sdhci-bcm2835.c   |  5 +++--
 drivers/mmc/host/sdhci-esdhc-imx.c |  9 +++++----
 drivers/mmc/host/sdhci-iproc.c     |  5 +++--
 drivers/mmc/host/sdhci-msm.c       |  6 ++----
 drivers/mmc/host/sdhci-of-arasan.c | 11 ++++-------
 drivers/mmc/host/sdhci-of-at91.c   |  4 +---
 drivers/mmc/host/sdhci-of-esdhc.c  | 25 ++++++++++++-------------
 drivers/mmc/host/sdhci-pci-core.c  |  5 ++++-
 drivers/mmc/host/sdhci-pltfm.c     | 26 +++++++++++++++++++-------
 drivers/mmc/host/sdhci-pltfm.h     |  2 +-
 drivers/mmc/host/sdhci-pxav2.c     |  1 -
 drivers/mmc/host/sdhci-pxav3.c     |  4 +---
 drivers/mmc/host/sdhci-s3c.c       |  2 +-
 drivers/mmc/host/sdhci-sirf.c      |  4 +++-
 drivers/mmc/host/sdhci.c           | 14 +++++---------
 drivers/mmc/host/sdhci.h           | 30 ++++++++++++++----------------
 drivers/mmc/host/sdhci_f_sdh30.c   |  5 ++++-
 19 files changed, 83 insertions(+), 81 deletions(-)

-- 
2.3.7

^ permalink raw reply	[flat|nested] 48+ messages in thread

end of thread, other threads:[~2016-02-04 10:40 UTC | newest]

Thread overview: 48+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-27  5:05 [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk Shawn Lin
2016-01-27  5:06 ` [RFC PATCH 01/21] mmc: sdhci-pltfm: consolidate parsing path Shawn Lin
2016-01-27  5:06 ` [RFC PATCH 02/21] mmc: sdhci-iproc: " Shawn Lin
2016-01-27  5:06 ` [RFC PATCH 03/21] mmc: sdhci-msm: " Shawn Lin
2016-01-27  5:06 ` [RFC PATCH 04/21] mmc: sdhci-of-arasan: " Shawn Lin
2016-01-27  5:06 ` [RFC PATCH 05/21] mmc: sdhci-of-at91: " Shawn Lin
2016-01-27  5:07 ` [RFC PATCH 06/21] mmc: sdhci-of-esdhc: " Shawn Lin
2016-01-27  5:07 ` [RFC PATCH 07/21] mmc: sdhci-pxav3: " Shawn Lin
2016-01-27  5:44   ` Jisheng Zhang
2016-01-27  5:44     ` Jisheng Zhang
2016-01-27  6:17     ` Shawn Lin
2016-01-27  5:07 ` [RFC PATCH 08/21] mmc: sdhci-sirf: check sdhci_get_of_property return value Shawn Lin
2016-01-27  5:07 ` [RFC PATCH 09/21] mmc: sdhci_f_sdh30: " Shawn Lin
2016-01-27  5:08 ` [RFC PATCH 10/21] mmc: sdhci: remove SDHCI_QUIRK_BROKEN_CARD_DETECTION Shawn Lin
2016-01-27  7:11   ` Haibo Chen
2016-01-27  7:11     ` Haibo Chen
2016-01-27  7:20     ` Shawn Lin
2016-01-27  7:20       ` Shawn Lin
2016-01-27  5:08 ` [RFC PATCH 11/21] mmc: sdhci-acpi: " Shawn Lin
2016-01-27  5:08 ` [RFC PATCH 12/21] mmc: sdhci-bcm-kona: " Shawn Lin
2016-01-27  5:08 ` [RFC PATCH 13/21] mmc: sdhci-bcm2835: " Shawn Lin
2016-01-27  5:08 ` [RFC PATCH 14/21] mmc: sdhci-esdhc-imx: " Shawn Lin
2016-01-27  6:54   ` Haibo Chen
2016-01-27  6:54     ` Haibo Chen
2016-01-27  6:58     ` Shawn Lin
2016-01-27  6:58       ` Shawn Lin
2016-01-27  5:08 ` [RFC PATCH 15/21] mmc: sdhci-msm: " Shawn Lin
2016-01-27  5:08 ` [RFC PATCH 16/21] mmc: sdhci-of-esdhc: " Shawn Lin
2016-01-27  5:09 ` [RFC PATCH 17/21] mmc: sdhci-pci-core: " Shawn Lin
2016-01-27  5:09 ` [RFC PATCH 18/21] mmc: sdhci-pltfm: " Shawn Lin
2016-01-27  5:09 ` [RFC PATCH 19/21] mmc: sdhci-pxav2: " Shawn Lin
2016-01-27  5:09 ` [RFC PATCH 20/21] mmc: sdhci-s3c: " Shawn Lin
2016-01-27  5:09 ` [RFC PATCH 21/21] mmc: sdhci.h: " Shawn Lin
2016-01-27 12:59 ` [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk Adrian Hunter
2016-01-27 13:23   ` Russell King - ARM Linux
2016-01-27 15:07     ` Ulf Hansson
2016-01-28  2:17       ` Shawn Lin
2016-01-28 11:29         ` One Thousand Gnomes
2016-01-28 15:03           ` Ulf Hansson
2016-01-28 15:54             ` One Thousand Gnomes
2016-01-28 12:03       ` Adrian Hunter
2016-01-28 15:16         ` Ulf Hansson
2016-01-28 16:27           ` Russell King - ARM Linux
2016-01-29 12:08           ` Adrian Hunter
2016-01-29 17:28             ` Russell King - ARM Linux
2016-02-01 12:32               ` Adrian Hunter
  -- strict thread matches above, loose matches on Subject: below --
2016-01-27  5:04 Shawn Lin
2016-02-04 10:40 ` Ulf Hansson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.