mmc: sdhci-pxav2: Add support for PXA168

PATCH Superseded Doug Brown 2 participants 12 messages Dec 29, 2022 – Jan 12, 2023

Subsystems (3)

Device tree bindings MMC, SD and SDIO Secure digital host controller interface (SDHCI) driver

lore

Share & tools

lore

Doug Brown

author

2022-12-29 20:04 UTC #0 lore raw

v3 cover mmc: sdhci-pxav2: Add support for PXA168

This is a revival of an earlier patch series from 2013 to add support for the PXA168 SDHC controller, with an additional SDIO IRQ errata fix. It also cleans up the clock naming to be consistent with the existing DT schema shared with the pxav3 driver (in a backwards-compatible way).

Here is the original patch series this is based on:
https://lore.kernel.org/linux-mmc/[email protected]/

Note that I left out the platform_specific_completion and clock gating changes from the original patches. They both seemed controversial, and don't seem necessary based on my testing. I've been running this code on a PXA168 for months without any issues.

Changes in v3:
- Use OF match data rather than of_match_device and of_device_is_compatible
- Simplify some instances of pdev->dev that could have just been "dev"
- Handle EPROBE_DEFER when getting the clock
- Use devm_clk_get_optional_enabled for the core clock (it's simpler)
- Clear sdio_mrq before calling mmc_request_done
- Small tweaks to devicetree binding requested by Krzysztof

Changes in v2:
- Fix mistakes in devicetree binding
- Use cleaner code for pxav1_readw suggested by Adrian
- Switch to request_done() and irq() for SDIO workaround CMD0 handling

Doug Brown (8):
mmc: sdhci-pxav2: add initial support for PXA168 V1 controller
mmc: sdhci-pxav2: enable CONFIG_MMC_SDHCI_IO_ACCESSORS
mmc: sdhci-pxav2: add register workaround for PXA168 silicon bug
mmc: sdhci-pxav2: change clock name to match DT bindings
mmc: sdhci-pxav2: add optional core clock
mmc: sdhci-pxav2: add SDIO card IRQ workaround for PXA168 V1
controller
mmc: sdhci-pxav2: add optional pinctrl for SDIO IRQ workaround
dt-bindings: mmc: sdhci-pxa: add pxav1

 .../devicetree/bindings/mmc/sdhci-pxa.yaml    |  19 ++-
 drivers/mmc/host/Kconfig                      |   1 +
 drivers/mmc/host/sdhci-pxav2.c                | 153 ++++++++++++++++--
 3 files changed, 159 insertions(+), 14 deletions(-)

Signature · 1 line

2.34.1

Doug Brown

author

2022-12-29 20:04 UTC #1 lore raw

PATCH v3 1/8 mmc: sdhci-pxav2: add initial support for PXA168 V1 controller

Add a new compatible string for the version 1 controller used in the PXA168, along with necessary quirks. Use a separate ops struct in preparation for a silicon bug workaround only necessary on V1.

Signed-off-by Doug Brown <[email protected]>

---
 drivers/mmc/host/sdhci-pxav2.c | 40 +++++++++++++++++++++++++++-------
 1 file changed, 32 insertions(+), 8 deletions(-)

drivers/mmc/host/sdhci-pxav2.c +32 -8

index f18906b5575f..5707d597ecae 100644
@@ -101,6 +101,24 @@ static void pxav2_mmc_set_bus_width(struct sdhci_host *host, int width)
 	writeb(ctrl, host->ioaddr + SDHCI_HOST_CONTROL);
 }
 
+struct sdhci_pxa_variant {
+	const struct sdhci_ops *ops;
+	unsigned int extra_quirks;
+};
+
+static const struct sdhci_ops pxav1_sdhci_ops = {
+	.set_clock     = sdhci_set_clock,
+	.get_max_clock = sdhci_pltfm_clk_get_max_clock,
+	.set_bus_width = pxav2_mmc_set_bus_width,
+	.reset         = pxav2_reset,
+	.set_uhs_signaling = sdhci_set_uhs_signaling,
+};
+
+static const struct sdhci_pxa_variant __maybe_unused pxav1_variant = {
+	.ops = &pxav1_sdhci_ops,
+	.extra_quirks = SDHCI_QUIRK_NO_BUSY_IRQ | SDHCI_QUIRK_32BIT_DMA_SIZE,
+};
+
 static const struct sdhci_ops pxav2_sdhci_ops = {
 	.set_clock     = sdhci_set_clock,
 	.get_max_clock = sdhci_pltfm_clk_get_max_clock,

@@ -109,11 +127,14 @@ static const struct sdhci_ops pxav2_sdhci_ops = {
 	.set_uhs_signaling = sdhci_set_uhs_signaling,
 };
 
+static const struct sdhci_pxa_variant pxav2_variant = {
+	.ops = &pxav2_sdhci_ops,
+};
+
 #ifdef CONFIG_OF
 static const struct of_device_id sdhci_pxav2_of_match[] = {
-	{
-		.compatible = "mrvl,pxav2-mmc",
-	},
+	{ .compatible = "mrvl,pxav1-mmc", .data = &pxav1_variant, },
+	{ .compatible = "mrvl,pxav2-mmc", .data = &pxav2_variant, },
 	{},
 };
 MODULE_DEVICE_TABLE(of, sdhci_pxav2_of_match);

@@ -157,7 +178,7 @@ static int sdhci_pxav2_probe(struct platform_device *pdev)
 	struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
 	struct device *dev = &pdev->dev;
 	struct sdhci_host *host = NULL;
-	const struct of_device_id *match;
+	const struct sdhci_pxa_variant *variant;
 
 	int ret;
 	struct clk *clk;

@@ -185,10 +206,12 @@ static int sdhci_pxav2_probe(struct platform_device *pdev)
 		| SDHCI_QUIRK_BROKEN_TIMEOUT_VAL
 		| SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN;
 
-	match = of_match_device(of_match_ptr(sdhci_pxav2_of_match), &pdev->dev);
-	if (match) {
+	variant = of_device_get_match_data(dev);
+	if (variant)
 		pdata = pxav2_get_mmc_pdata(dev);
-	}
+	else
+		variant = &pxav2_variant;
+
 	if (pdata) {
 		if (pdata->flags & PXA_FLAG_CARD_PERMANENT) {
 			/* on-chip device */

@@ -208,7 +231,8 @@ static int sdhci_pxav2_probe(struct platform_device *pdev)
 			host->mmc->pm_caps |= pdata->pm_caps;
 	}
 
-	host->ops = &pxav2_sdhci_ops;
+	host->quirks |= variant->extra_quirks;
+	host->ops = variant->ops;
 
 	ret = sdhci_add_host(host);
 	if (ret)

Signature · 1 line

2.34.1

Doug Brown

author

2022-12-29 20:04 UTC #2 lore raw

PATCH v3 2/8 mmc: sdhci-pxav2: enable CONFIG_MMC_SDHCI_IO_ACCESSORS

In reply to Doug Brown

Enable CONFIG_MMC_SDHCI_IO_ACCESSORS for the pxav2 driver. The read_w callback is needed for a silicon bug workaround in the PXA168.

Signed-off-by Doug Brown <[email protected]>

---
 drivers/mmc/host/Kconfig | 1 +
 1 file changed, 1 insertion(+)

drivers/mmc/host/Kconfig +1 -0

index 5e19a961c34d..b9e9185c86a6 100644
@@ -360,6 +360,7 @@ config MMC_SDHCI_PXAV2
 	depends on MMC_SDHCI_PLTFM
 	depends on ARCH_MMP || COMPILE_TEST
 	default CPU_PXA910
+	select MMC_SDHCI_IO_ACCESSORS
 	help
 	  This selects the Marvell(R) PXAV2 SD Host Controller.
 	  If you have a PXA9XX platform with SD Host Controller

Signature · 1 line

2.34.1

Doug Brown

author

2022-12-29 20:05 UTC #3 lore raw

PATCH v3 3/8 mmc: sdhci-pxav2: add register workaround for PXA168 silicon bug

In reply to Doug Brown

The PXA168 has a documented silicon bug that results in a data abort exception when accessing the SDHCI_HOST_VERSION register on SDH2 and SDH4 through a 16-bit read. Implement the workaround described in the errata, which performs a 32-bit read from a lower address instead. This is safe to use on all four SDH peripherals.

Signed-off-by Doug Brown <[email protected]>

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

drivers/mmc/host/sdhci-pxav2.c +10 -0

index 5707d597ecae..5e01dab94426 100644
@@ -80,6 +80,15 @@ static void pxav2_reset(struct sdhci_host *host, u8 mask)
 	}
 }
 
+static u16 pxav1_readw(struct sdhci_host *host, int reg)
+{
+	/* Workaround for data abort exception on SDH2 and SDH4 on PXA168 */
+	if (reg == SDHCI_HOST_VERSION)
+		return readl(host->ioaddr + SDHCI_HOST_VERSION - 2) >> 16;
+
+	return readw(host->ioaddr + reg);
+}
+
 static void pxav2_mmc_set_bus_width(struct sdhci_host *host, int width)
 {
 	u8 ctrl;

@@ -107,6 +116,7 @@ struct sdhci_pxa_variant {
 };
 
 static const struct sdhci_ops pxav1_sdhci_ops = {
+	.read_w        = pxav1_readw,
 	.set_clock     = sdhci_set_clock,
 	.get_max_clock = sdhci_pltfm_clk_get_max_clock,
 	.set_bus_width = pxav2_mmc_set_bus_width,

Signature · 1 line

2.34.1

Doug Brown

author

2022-12-29 20:05 UTC #4 lore raw

PATCH v3 4/8 mmc: sdhci-pxav2: change clock name to match DT bindings

In reply to Doug Brown

The devicetree bindings for this driver specify that the two allowed clock names are io and core. Change this driver to look for io, but allow any name if it fails for backwards compatibility. Follow the same pattern used in sdhci-pxav3, but add support for EPROBE_DEFER.

Get rid of an unnecessary pdev->dev while we're at it.

Signed-off-by Doug Brown <[email protected]>

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

drivers/mmc/host/sdhci-pxav2.c +5 -3

index 5e01dab94426..f5c86e1ba734 100644
@@ -199,16 +199,18 @@ static int sdhci_pxav2_probe(struct platform_device *pdev)
 
 	pltfm_host = sdhci_priv(host);
 
-	clk = devm_clk_get(dev, "PXA-SDHCLK");
+	clk = devm_clk_get(dev, "io");
+	if (IS_ERR(clk) && PTR_ERR(clk) != -EPROBE_DEFER)
+		clk = devm_clk_get(dev, NULL);
 	if (IS_ERR(clk)) {
-		dev_err(dev, "failed to get io clock\n");
 		ret = PTR_ERR(clk);
+		dev_err_probe(dev, ret, "failed to get io clock\n");
 		goto free;
 	}
 	pltfm_host->clk = clk;
 	ret = clk_prepare_enable(clk);
 	if (ret) {
-		dev_err(&pdev->dev, "failed to enable io clock\n");
+		dev_err(dev, "failed to enable io clock\n");
 		goto free;
 	}

Signature · 1 line

2.34.1

Doug Brown

author

2022-12-29 20:05 UTC #5 lore raw

PATCH v3 5/8 mmc: sdhci-pxav2: add optional core clock

In reply to Doug Brown

Add ability to have an optional core clock just like the pxav3 driver. The PXA168 needs this because its SDHC controllers have separate core and io clocks that both need to be enabled. This also correctly matches the documented devicetree bindings for this driver.

Signed-off-by Doug Brown <[email protected]>

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

drivers/mmc/host/sdhci-pxav2.c +7 -1

index f5c86e1ba734..b10f55b478fc 100644
@@ -191,7 +191,7 @@ static int sdhci_pxav2_probe(struct platform_device *pdev)
 	const struct sdhci_pxa_variant *variant;
 
 	int ret;
-	struct clk *clk;
+	struct clk *clk, *clk_core;
 
 	host = sdhci_pltfm_init(pdev, NULL, 0);
 	if (IS_ERR(host))

@@ -214,6 +214,12 @@ static int sdhci_pxav2_probe(struct platform_device *pdev)
 		goto free;
 	}
 
+	clk_core = devm_clk_get_optional_enabled(dev, "core");
+	if (IS_ERR(clk_core)) {
+		dev_err_probe(dev, PTR_ERR(clk_core), "failed to enable core clock\n");
+		goto disable_clk;
+	}
+
 	host->quirks = SDHCI_QUIRK_BROKEN_ADMA
 		| SDHCI_QUIRK_BROKEN_TIMEOUT_VAL
 		| SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN;

Signature · 1 line

2.34.1

Doug Brown

author

2022-12-29 20:05 UTC #6 lore raw

PATCH v3 6/8 mmc: sdhci-pxav2: add SDIO card IRQ workaround for PXA168 V1 controller

In reply to Doug Brown

The PXA168 has a documented silicon bug that causes SDIO card IRQs to be missed. Implement the first half of the suggested workaround, which involves resetting the data port logic and issuing a dummy CMD0 to restart the clock.

Signed-off-by Doug Brown <[email protected]>

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

drivers/mmc/host/sdhci-pxav2.c +55 -1

index b10f55b478fc..10fa9de14ad4 100644
@@ -20,6 +20,8 @@
 #include <linux/slab.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/mmc/sdio.h>
+#include <linux/mmc/mmc.h>
 
 #include "sdhci.h"
 #include "sdhci-pltfm.h"

@@ -41,6 +43,10 @@
 #define MMC_CARD		0x1000
 #define MMC_WIDTH		0x0100
 
+struct sdhci_pxav2_host {
+	struct mmc_request *sdio_mrq;
+};
+
 static void pxav2_reset(struct sdhci_host *host, u8 mask)
 {
 	struct platform_device *pdev = to_platform_device(mmc_dev(host->mmc));

@@ -89,6 +95,52 @@ static u16 pxav1_readw(struct sdhci_host *host, int reg)
 	return readw(host->ioaddr + reg);
 }
 
+static u32 pxav1_irq(struct sdhci_host *host, u32 intmask)
+{
+	struct sdhci_pxav2_host *pxav2_host = sdhci_pltfm_priv(sdhci_priv(host));
+	struct mmc_request *sdio_mrq;
+
+	if (pxav2_host->sdio_mrq && (intmask & SDHCI_INT_CMD_MASK)) {
+		/* The dummy CMD0 for the SDIO workaround just completed */
+		sdhci_writel(host, intmask & SDHCI_INT_CMD_MASK, SDHCI_INT_STATUS);
+		intmask &= ~SDHCI_INT_CMD_MASK;
+		sdio_mrq = pxav2_host->sdio_mrq;
+		pxav2_host->sdio_mrq = NULL;
+		mmc_request_done(host->mmc, sdio_mrq);
+	}
+
+	return intmask;
+}
+
+static void pxav1_request_done(struct sdhci_host *host, struct mmc_request *mrq)
+{
+	u16 tmp;
+	struct sdhci_pxav2_host *pxav2_host;
+
+	/* If this is an SDIO command, perform errata workaround for silicon bug */
+	if (mrq->cmd && !mrq->cmd->error &&
+	    (mrq->cmd->opcode == SD_IO_RW_DIRECT ||
+	     mrq->cmd->opcode == SD_IO_RW_EXTENDED)) {
+		/* Reset data port */
+		tmp = readw(host->ioaddr + SDHCI_TIMEOUT_CONTROL);
+		tmp |= 0x400;
+		writew(tmp, host->ioaddr + SDHCI_TIMEOUT_CONTROL);
+
+		/* Clock is now stopped, so restart it by sending a dummy CMD0 */
+		pxav2_host = sdhci_pltfm_priv(sdhci_priv(host));
+		pxav2_host->sdio_mrq = mrq;
+		sdhci_writel(host, 0, SDHCI_ARGUMENT);
+		sdhci_writew(host, 0, SDHCI_TRANSFER_MODE);
+		sdhci_writew(host, SDHCI_MAKE_CMD(MMC_GO_IDLE_STATE, SDHCI_CMD_RESP_NONE),
+			     SDHCI_COMMAND);
+
+		/* Don't finish this request until the dummy CMD0 finishes */
+		return;
+	}
+
+	mmc_request_done(host->mmc, mrq);
+}
+
 static void pxav2_mmc_set_bus_width(struct sdhci_host *host, int width)
 {
 	u8 ctrl;

@@ -118,10 +170,12 @@ struct sdhci_pxa_variant {
 static const struct sdhci_ops pxav1_sdhci_ops = {
 	.read_w        = pxav1_readw,
 	.set_clock     = sdhci_set_clock,
+	.irq           = pxav1_irq,
 	.get_max_clock = sdhci_pltfm_clk_get_max_clock,
 	.set_bus_width = pxav2_mmc_set_bus_width,
 	.reset         = pxav2_reset,
 	.set_uhs_signaling = sdhci_set_uhs_signaling,
+	.request_done  = pxav1_request_done,
 };
 
 static const struct sdhci_pxa_variant __maybe_unused pxav1_variant = {

@@ -193,7 +247,7 @@ static int sdhci_pxav2_probe(struct platform_device *pdev)
 	int ret;
 	struct clk *clk, *clk_core;
 
-	host = sdhci_pltfm_init(pdev, NULL, 0);
+	host = sdhci_pltfm_init(pdev, NULL, sizeof(struct sdhci_pxav2_host));
 	if (IS_ERR(host))
 		return PTR_ERR(host);

Signature · 1 line

2.34.1

Doug Brown

author

2022-12-29 20:05 UTC #7 lore raw

PATCH v3 7/8 mmc: sdhci-pxav2: add optional pinctrl for SDIO IRQ workaround

In reply to Doug Brown

The PXA168 errata recommends that the CMD signal should be detached from the SD bus while performing the dummy CMD0 to restart the clock. Implement this using pinctrl states.

Signed-off-by Doug Brown <[email protected]>

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

drivers/mmc/host/sdhci-pxav2.c +32 -1

index 10fa9de14ad4..38edd1fcc992 100644
@@ -22,6 +22,7 @@
 #include <linux/of_device.h>
 #include <linux/mmc/sdio.h>
 #include <linux/mmc/mmc.h>
+#include <linux/pinctrl/consumer.h>
 
 #include "sdhci.h"
 #include "sdhci-pltfm.h"

@@ -45,6 +46,9 @@
 
 struct sdhci_pxav2_host {
 	struct mmc_request *sdio_mrq;
+	struct pinctrl *pinctrl;
+	struct pinctrl_state *pins_default;
+	struct pinctrl_state *pins_cmd_gpio;
 };
 
 static void pxav2_reset(struct sdhci_host *host, u8 mask)

@@ -104,6 +108,11 @@ static u32 pxav1_irq(struct sdhci_host *host, u32 intmask)
 		/* The dummy CMD0 for the SDIO workaround just completed */
 		sdhci_writel(host, intmask & SDHCI_INT_CMD_MASK, SDHCI_INT_STATUS);
 		intmask &= ~SDHCI_INT_CMD_MASK;
+
+		/* Restore MMC function to CMD pin */
+		if (pxav2_host->pinctrl && pxav2_host->pins_default)
+			pinctrl_select_state(pxav2_host->pinctrl, pxav2_host->pins_default);
+
 		sdio_mrq = pxav2_host->sdio_mrq;
 		pxav2_host->sdio_mrq = NULL;
 		mmc_request_done(host->mmc, sdio_mrq);

@@ -129,6 +138,11 @@ static void pxav1_request_done(struct sdhci_host *host, struct mmc_request *mrq)
 		/* Clock is now stopped, so restart it by sending a dummy CMD0 */
 		pxav2_host = sdhci_pltfm_priv(sdhci_priv(host));
 		pxav2_host->sdio_mrq = mrq;
+
+		/* Set CMD as high output rather than MMC function while we do CMD0 */
+		if (pxav2_host->pinctrl && pxav2_host->pins_cmd_gpio)
+			pinctrl_select_state(pxav2_host->pinctrl, pxav2_host->pins_cmd_gpio);
+
 		sdhci_writel(host, 0, SDHCI_ARGUMENT);
 		sdhci_writew(host, 0, SDHCI_TRANSFER_MODE);
 		sdhci_writew(host, SDHCI_MAKE_CMD(MMC_GO_IDLE_STATE, SDHCI_CMD_RESP_NONE),

@@ -240,6 +254,7 @@ static int sdhci_pxav2_probe(struct platform_device *pdev)
 {
 	struct sdhci_pltfm_host *pltfm_host;
 	struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
+	struct sdhci_pxav2_host *pxav2_host;
 	struct device *dev = &pdev->dev;
 	struct sdhci_host *host = NULL;
 	const struct sdhci_pxa_variant *variant;

@@ -247,11 +262,12 @@ static int sdhci_pxav2_probe(struct platform_device *pdev)
 	int ret;
 	struct clk *clk, *clk_core;
 
-	host = sdhci_pltfm_init(pdev, NULL, sizeof(struct sdhci_pxav2_host));
+	host = sdhci_pltfm_init(pdev, NULL, sizeof(*pxav2_host));
 	if (IS_ERR(host))
 		return PTR_ERR(host);
 
 	pltfm_host = sdhci_priv(host);
+	pxav2_host = sdhci_pltfm_priv(pltfm_host);
 
 	clk = devm_clk_get(dev, "io");
 	if (IS_ERR(clk) && PTR_ERR(clk) != -EPROBE_DEFER)

@@ -306,6 +322,21 @@ static int sdhci_pxav2_probe(struct platform_device *pdev)
 	host->quirks |= variant->extra_quirks;
 	host->ops = variant->ops;
 
+	/* Set up optional pinctrl for PXA168 SDIO IRQ fix */
+	pxav2_host->pinctrl = devm_pinctrl_get(dev);
+	if (!IS_ERR(pxav2_host->pinctrl)) {
+		pxav2_host->pins_cmd_gpio = pinctrl_lookup_state(pxav2_host->pinctrl,
+								 "state_cmd_gpio");
+		if (IS_ERR(pxav2_host->pins_cmd_gpio))
+			pxav2_host->pins_cmd_gpio = NULL;
+		pxav2_host->pins_default = pinctrl_lookup_state(pxav2_host->pinctrl,
+								"default");
+		if (IS_ERR(pxav2_host->pins_default))
+			pxav2_host->pins_default = NULL;
+	} else {
+		pxav2_host->pinctrl = NULL;
+	}
+
 	ret = sdhci_add_host(host);
 	if (ret)
 		goto disable_clk;

Signature · 1 line

2.34.1

Doug Brown

author

2022-12-29 20:05 UTC #8 lore raw

PATCH v3 8/8 dt-bindings: mmc: sdhci-pxa: add pxav1

In reply to Doug Brown

Add a compatible for the pxav1 controller in the PXA168, along with optional pinctrl properties to use for an errata workaround.

Signed-off-by Doug Brown <[email protected]>

Reviewed-by Krzysztof Kozlowski <[email protected]>

---
 .../devicetree/bindings/mmc/sdhci-pxa.yaml    | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

Documentation/devicetree/bindings/mmc/sdhci-pxa.yaml +18 -1

index 1c87f4218e18..09455f9fa8de 100644
@@ -4,7 +4,7 @@
 $id: http://devicetree.org/schemas/mmc/sdhci-pxa.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Marvell PXA SDHCI v2/v3 bindings
+title: Marvell PXA SDHCI v1/v2/v3
 
 maintainers:
   - Ulf Hansson <[email protected]>

@@ -34,6 +34,7 @@ allOf:
 properties:
   compatible:
     enum:
+      - mrvl,pxav1-mmc
       - mrvl,pxav2-mmc
       - mrvl,pxav3-mmc
       - marvell,armada-380-sdhci

@@ -61,6 +62,22 @@ properties:
       - const: io
       - const: core
 
+  pinctrl-names:
+    description:
+      Optional for supporting PXA168 SDIO IRQ errata to switch CMD pin between
+      SDIO CMD and GPIO mode.
+    items:
+      - const: default
+      - const: state_cmd_gpio
+
+  pinctrl-0:
+    description:
+      Should contain default pinctrl.
+
+  pinctrl-1:
+    description:
+      Should switch CMD pin to GPIO mode as a high output.
+
   mrvl,clk-delay-cycles:
     description: Specify a number of cycles to delay for tuning.
     $ref: /schemas/types.yaml#/definitions/uint32

Signature · 1 line

2.34.1

Adrian Hunter

maintainer

2023-01-11 12:44 UTC #9 lore raw

Re: [PATCH v3 8/8] dt-bindings: mmc: sdhci-pxa: add pxav1

Quoted 5 lines · Doug Brown

Add a compatible for the pxav1 controller in the PXA168, along with optional pinctrl properties to use for an errata workaround.
Signed-off-by Doug Brown <[email protected]>
Reviewed-by Krzysztof Kozlowski <[email protected]>

Doesn't apply cleanly anymore

Quoted 48 lines

---
 .../devicetree/bindings/mmc/sdhci-pxa.yaml    | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

Documentation/devicetree/bindings/mmc/sdhci-pxa.yaml +18 -1

index 1c87f4218e18..09455f9fa8de 100644
@@ -4,7 +4,7 @@
 $id: http://devicetree.org/schemas/mmc/sdhci-pxa.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Marvell PXA SDHCI v2/v3 bindings
+title: Marvell PXA SDHCI v1/v2/v3
 
 maintainers:
   - Ulf Hansson <[email protected]>

@@ -34,6 +34,7 @@ allOf:
 properties:
   compatible:
     enum:
+      - mrvl,pxav1-mmc
       - mrvl,pxav2-mmc
       - mrvl,pxav3-mmc
       - marvell,armada-380-sdhci

@@ -61,6 +62,22 @@ properties:
       - const: io
       - const: core
 
+  pinctrl-names:
+    description:
+      Optional for supporting PXA168 SDIO IRQ errata to switch CMD pin between
+      SDIO CMD and GPIO mode.
+    items:
+      - const: default
+      - const: state_cmd_gpio
+
+  pinctrl-0:
+    description:
+      Should contain default pinctrl.
+
+  pinctrl-1:
+    description:
+      Should switch CMD pin to GPIO mode as a high output.
+
   mrvl,clk-delay-cycles:
     description: Specify a number of cycles to delay for tuning.
     $ref: /schemas/types.yaml#/definitions/uint32

Doug Brown

author

2023-01-12 02:20 UTC #10 lore raw

Re: [PATCH v3 8/8] dt-bindings: mmc: sdhci-pxa: add pxav1

Quoted 8 lines · Adrian Hunter

Quoted 5 lines · Doug Brown
Add a compatible for the pxav1 controller in the PXA168, along with optional pinctrl properties to use for an errata workaround.
Signed-off-by Doug Brown <[email protected]>
Reviewed-by Krzysztof Kozlowski <[email protected]>
Doesn't apply cleanly anymore

Sorry about that. Krzysztof beat me to the "bindings in title" removal he asked me to do. Will send a v4 series rebased on the latest mmc/next.

Quoted 50 lines

Quoted 49 lines

---
  .../devicetree/bindings/mmc/sdhci-pxa.yaml    | 19 ++++++++++++++++++-
  1 file changed, 18 insertions(+), 1 deletion(-)

Documentation/devicetree/bindings/mmc/sdhci-pxa.yaml +18 -1

index 1c87f4218e18..09455f9fa8de 100644
@@ -4,7 +4,7 @@
  $id: http://devicetree.org/schemas/mmc/sdhci-pxa.yaml#
  $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Marvell PXA SDHCI v2/v3 bindings
+title: Marvell PXA SDHCI v1/v2/v3
 
  maintainers:
    - Ulf Hansson <[email protected]>

@@ -34,6 +34,7 @@ allOf:
  properties:
    compatible:
      enum:
+      - mrvl,pxav1-mmc
        - mrvl,pxav2-mmc
        - mrvl,pxav3-mmc
        - marvell,armada-380-sdhci

@@ -61,6 +62,22 @@ properties:
        - const: io
        - const: core
 
+  pinctrl-names:
+    description:
+      Optional for supporting PXA168 SDIO IRQ errata to switch CMD pin between
+      SDIO CMD and GPIO mode.
+    items:
+      - const: default
+      - const: state_cmd_gpio
+
+  pinctrl-0:
+    description:
+      Should contain default pinctrl.
+
+  pinctrl-1:
+    description:
+      Should switch CMD pin to GPIO mode as a high output.
+
    mrvl,clk-delay-cycles:
      description: Specify a number of cycles to delay for tuning.
      $ref: /schemas/types.yaml#/definitions/uint32

Adrian Hunter

maintainer Acked-by on the v3 series

2023-01-11 12:43 UTC #11 lore raw

In reply to Doug Brown

Quoted 5 lines · Doug Brown

This is a revival of an earlier patch series from 2013 to add support for the PXA168 SDHC controller, with an additional SDIO IRQ errata fix. It also cleans up the clock naming to be consistent with the existing DT schema shared with the pxav3 driver (in a backwards-compatible way).

Quoted 7 lines

Here is the original patch series this is based on:
https://lore.kernel.org/linux-mmc/[email protected]/
Note that I left out the platform_specific_completion and clock gating changes from the original patches. They both seemed controversial, and don't seem necessary based on my testing. I've been running this code on a PXA168 for months without any issues.

For patch 1-7:

Acked-by Adrian Hunter <[email protected]>

Quoted 30 lines

Changes in v3:
- Use OF match data rather than of_match_device and of_device_is_compatible
- Simplify some instances of pdev->dev that could have just been "dev"
- Handle EPROBE_DEFER when getting the clock
- Use devm_clk_get_optional_enabled for the core clock (it's simpler)
- Clear sdio_mrq before calling mmc_request_done
- Small tweaks to devicetree binding requested by Krzysztof
Changes in v2:
- Fix mistakes in devicetree binding
- Use cleaner code for pxav1_readw suggested by Adrian
- Switch to request_done() and irq() for SDIO workaround CMD0 handling
Doug Brown (8):
mmc: sdhci-pxav2: add initial support for PXA168 V1 controller
mmc: sdhci-pxav2: enable CONFIG_MMC_SDHCI_IO_ACCESSORS
mmc: sdhci-pxav2: add register workaround for PXA168 silicon bug
mmc: sdhci-pxav2: change clock name to match DT bindings
mmc: sdhci-pxav2: add optional core clock
mmc: sdhci-pxav2: add SDIO card IRQ workaround for PXA168 V1
controller
mmc: sdhci-pxav2: add optional pinctrl for SDIO IRQ workaround
dt-bindings: mmc: sdhci-pxa: add pxav1
 .../devicetree/bindings/mmc/sdhci-pxa.yaml    |  19 ++-
 drivers/mmc/host/Kconfig                      |   1 +
 drivers/mmc/host/sdhci-pxav2.c                | 153 ++++++++++++++++--
 3 files changed, 159 insertions(+), 14 deletions(-)