[PATCH v4 0/8] mmc: sdhci-pxav2: Add support for PXA168

[PATCH v4 0/8] mmc: sdhci-pxav2: Add support for PXA168

[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).

Here is the original patch series this is based on:
https://lore.kernel.org/linux-mmc/1363544206-3671-1-git-send-email-tanmay.upadhyay@einfochips.com/

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 v4:
- Rebase on latest mmc/next to fix conflict with DT binding

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(-)

-- 
2.34.1

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

[Doug Brown]

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 <doug@schmorgal.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci-pxav2.c | 40 +++++++++++++++++++++++++++-------
 1 file changed, 32 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/sdhci-pxav2.c b/drivers/mmc/host/sdhci-pxav2.c
index f18906b5575f..5707d597ecae 100644
--- a/drivers/mmc/host/sdhci-pxav2.c
+++ b/drivers/mmc/host/sdhci-pxav2.c
@@ -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)
-- 
2.34.1

[PATCH v4 2/8] mmc: sdhci-pxav2: enable CONFIG_MMC_SDHCI_IO_ACCESSORS

[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 <doug@schmorgal.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 5e19a961c34d..b9e9185c86a6 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -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
-- 
2.34.1

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

[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 <doug@schmorgal.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci-pxav2.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/mmc/host/sdhci-pxav2.c b/drivers/mmc/host/sdhci-pxav2.c
index 5707d597ecae..5e01dab94426 100644
--- a/drivers/mmc/host/sdhci-pxav2.c
+++ b/drivers/mmc/host/sdhci-pxav2.c
@@ -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,
-- 
2.34.1

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

[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 <doug@schmorgal.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci-pxav2.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci-pxav2.c b/drivers/mmc/host/sdhci-pxav2.c
index 5e01dab94426..f5c86e1ba734 100644
--- a/drivers/mmc/host/sdhci-pxav2.c
+++ b/drivers/mmc/host/sdhci-pxav2.c
@@ -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;
 	}
 
-- 
2.34.1

[PATCH v4 5/8] mmc: sdhci-pxav2: add optional core clock

[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 <doug@schmorgal.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci-pxav2.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-pxav2.c b/drivers/mmc/host/sdhci-pxav2.c
index f5c86e1ba734..b10f55b478fc 100644
--- a/drivers/mmc/host/sdhci-pxav2.c
+++ b/drivers/mmc/host/sdhci-pxav2.c
@@ -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;
-- 
2.34.1

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

[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 <doug@schmorgal.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci-pxav2.c | 56 +++++++++++++++++++++++++++++++++-
 1 file changed, 55 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-pxav2.c b/drivers/mmc/host/sdhci-pxav2.c
index b10f55b478fc..10fa9de14ad4 100644
--- a/drivers/mmc/host/sdhci-pxav2.c
+++ b/drivers/mmc/host/sdhci-pxav2.c
@@ -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);
 
-- 
2.34.1

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

[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 <doug@schmorgal.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci-pxav2.c | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-pxav2.c b/drivers/mmc/host/sdhci-pxav2.c
index 10fa9de14ad4..38edd1fcc992 100644
--- a/drivers/mmc/host/sdhci-pxav2.c
+++ b/drivers/mmc/host/sdhci-pxav2.c
@@ -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;
-- 
2.34.1

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

[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 <doug@schmorgal.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../devicetree/bindings/mmc/sdhci-pxa.yaml    | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mmc/sdhci-pxa.yaml b/Documentation/devicetree/bindings/mmc/sdhci-pxa.yaml
index 3d46c2525787..09455f9fa8de 100644
--- a/Documentation/devicetree/bindings/mmc/sdhci-pxa.yaml
+++ b/Documentation/devicetree/bindings/mmc/sdhci-pxa.yaml
@@ -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
+title: Marvell PXA SDHCI v1/v2/v3
 
 maintainers:
   - Ulf Hansson <ulf.hansson@linaro.org>
@@ -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
-- 
2.34.1

Re: [PATCH v4 5/8] mmc: sdhci-pxav2: add optional core clock

[Dan Carpenter]

Hi Doug,

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Doug-Brown/mmc-sdhci-pxav2-add-initial-support-for-PXA168-V1-controller/20230112-102921
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:    https://lore.kernel.org/r/20230112022416.8474-6-doug%40schmorgal.com
patch subject: [PATCH v4 5/8] mmc: sdhci-pxav2: add optional core clock
config: riscv-randconfig-m041-20230113
compiler: riscv64-linux-gcc (GCC) 12.1.0

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>

smatch warnings:
drivers/mmc/host/sdhci-pxav2.c:220 sdhci_pxav2_probe() warn: missing error code 'ret'

vim +/ret +220 drivers/mmc/host/sdhci-pxav2.c

c3be1efd41a97f Bill Pemberton        2012-11-19  185  static int sdhci_pxav2_probe(struct platform_device *pdev)
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  186  {
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  187  	struct sdhci_pltfm_host *pltfm_host;
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  188  	struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  189  	struct device *dev = &pdev->dev;
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  190  	struct sdhci_host *host = NULL;
568536d7eb1969 Doug Brown            2023-01-11  191  	const struct sdhci_pxa_variant *variant;
b650352dd3df36 Chris Ball            2012-04-10  192  
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  193  	int ret;
d8981da5ec7505 Doug Brown            2023-01-11  194  	struct clk *clk, *clk_core;
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  195  
0e748234293f5f Christian Daudt       2013-05-29  196  	host = sdhci_pltfm_init(pdev, NULL, 0);
6a686c31324c9e Sebastian Hesselbarth 2014-10-21  197  	if (IS_ERR(host))
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  198  		return PTR_ERR(host);
6a686c31324c9e Sebastian Hesselbarth 2014-10-21  199  
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  200  	pltfm_host = sdhci_priv(host);
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  201  
edf4ccd94bbef1 Doug Brown            2023-01-11  202  	clk = devm_clk_get(dev, "io");
edf4ccd94bbef1 Doug Brown            2023-01-11  203  	if (IS_ERR(clk) && PTR_ERR(clk) != -EPROBE_DEFER)
edf4ccd94bbef1 Doug Brown            2023-01-11  204  		clk = devm_clk_get(dev, NULL);
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  205  	if (IS_ERR(clk)) {
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  206  		ret = PTR_ERR(clk);
edf4ccd94bbef1 Doug Brown            2023-01-11  207  		dev_err_probe(dev, ret, "failed to get io clock\n");
3fd1d86f03cbcc Masahiro Yamada       2017-08-23  208  		goto free;
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  209  	}
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  210  	pltfm_host->clk = clk;
21b22284619bbb Alexey Khoroshilov    2017-02-11  211  	ret = clk_prepare_enable(clk);
21b22284619bbb Alexey Khoroshilov    2017-02-11  212  	if (ret) {
edf4ccd94bbef1 Doug Brown            2023-01-11  213  		dev_err(dev, "failed to enable io clock\n");
3fd1d86f03cbcc Masahiro Yamada       2017-08-23  214  		goto free;
21b22284619bbb Alexey Khoroshilov    2017-02-11  215  	}
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  216  
d8981da5ec7505 Doug Brown            2023-01-11  217  	clk_core = devm_clk_get_optional_enabled(dev, "core");
d8981da5ec7505 Doug Brown            2023-01-11  218  	if (IS_ERR(clk_core)) {
d8981da5ec7505 Doug Brown            2023-01-11  219  		dev_err_probe(dev, PTR_ERR(clk_core), "failed to enable core clock\n");
d8981da5ec7505 Doug Brown            2023-01-11 @220  		goto disable_clk;

ret = PTR_ERR(clk_core);

d8981da5ec7505 Doug Brown            2023-01-11  221  	}
d8981da5ec7505 Doug Brown            2023-01-11  222  
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  223  	host->quirks = SDHCI_QUIRK_BROKEN_ADMA
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  224  		| SDHCI_QUIRK_BROKEN_TIMEOUT_VAL
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  225  		| SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN;
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  226  
568536d7eb1969 Doug Brown            2023-01-11  227  	variant = of_device_get_match_data(dev);
568536d7eb1969 Doug Brown            2023-01-11  228  	if (variant)
b650352dd3df36 Chris Ball            2012-04-10  229  		pdata = pxav2_get_mmc_pdata(dev);
568536d7eb1969 Doug Brown            2023-01-11  230  	else
568536d7eb1969 Doug Brown            2023-01-11  231  		variant = &pxav2_variant;
568536d7eb1969 Doug Brown            2023-01-11  232  
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  233  	if (pdata) {
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  234  		if (pdata->flags & PXA_FLAG_CARD_PERMANENT) {
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  235  			/* on-chip device */
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  236  			host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  237  			host->mmc->caps |= MMC_CAP_NONREMOVABLE;
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  238  		}
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  239  
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  240  		/* If slot design supports 8 bit data, indicate this to MMC. */
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  241  		if (pdata->flags & PXA_FLAG_SD_8_BIT_CAPABLE_SLOT)
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  242  			host->mmc->caps |= MMC_CAP_8_BIT_DATA;
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  243  
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  244  		if (pdata->quirks)
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  245  			host->quirks |= pdata->quirks;
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  246  		if (pdata->host_caps)
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  247  			host->mmc->caps |= pdata->host_caps;
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  248  		if (pdata->pm_caps)
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  249  			host->mmc->pm_caps |= pdata->pm_caps;
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  250  	}
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  251  
568536d7eb1969 Doug Brown            2023-01-11  252  	host->quirks |= variant->extra_quirks;
568536d7eb1969 Doug Brown            2023-01-11  253  	host->ops = variant->ops;
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  254  
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  255  	ret = sdhci_add_host(host);
fb8617e1ee4d40 Jisheng Zhang         2018-05-25  256  	if (ret)
3fd1d86f03cbcc Masahiro Yamada       2017-08-23  257  		goto disable_clk;
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  258  
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  259  	return 0;
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  260  
3fd1d86f03cbcc Masahiro Yamada       2017-08-23  261  disable_clk:
164378efe7612a Chao Xie              2012-07-31  262  	clk_disable_unprepare(clk);
3fd1d86f03cbcc Masahiro Yamada       2017-08-23  263  free:
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  264  	sdhci_pltfm_free(pdev);
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  265  	return ret;
9f5d71e4a78a02 Zhangfei Gao          2011-06-08  266  }

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

Re: [PATCH v4 5/8] mmc: sdhci-pxav2: add optional core clock

[Doug Brown]

Hi Dan,

On 1/14/2023 12:01 AM, Dan Carpenter wrote:
> Hi Doug,
> 
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Doug-Brown/mmc-sdhci-pxav2-add-initial-support-for-PXA168-V1-controller/20230112-102921
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
> patch link:    https://lore.kernel.org/r/20230112022416.8474-6-doug%40schmorgal.com
> patch subject: [PATCH v4 5/8] mmc: sdhci-pxav2: add optional core clock
> config: riscv-randconfig-m041-20230113
> compiler: riscv64-linux-gcc (GCC) 12.1.0
> 
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <error27@gmail.com>
> 
> smatch warnings:
> drivers/mmc/host/sdhci-pxav2.c:220 sdhci_pxav2_probe() warn: missing error code 'ret'

Thanks for passing this on. I definitely forgot an assignment to ret.
Since this is correcting an error in my patch that hasn't been accepted
yet, is it safe to assume I should omit those Reported-by tags from the
next version of my patch, since they don't apply to the patch itself?

> 
> vim +/ret +220 drivers/mmc/host/sdhci-pxav2.c
> 
> c3be1efd41a97f Bill Pemberton        2012-11-19  185  static int sdhci_pxav2_probe(struct platform_device *pdev)
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  186  {
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  187  	struct sdhci_pltfm_host *pltfm_host;
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  188  	struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  189  	struct device *dev = &pdev->dev;
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  190  	struct sdhci_host *host = NULL;
> 568536d7eb1969 Doug Brown            2023-01-11  191  	const struct sdhci_pxa_variant *variant;
> b650352dd3df36 Chris Ball            2012-04-10  192
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  193  	int ret;
> d8981da5ec7505 Doug Brown            2023-01-11  194  	struct clk *clk, *clk_core;
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  195
> 0e748234293f5f Christian Daudt       2013-05-29  196  	host = sdhci_pltfm_init(pdev, NULL, 0);
> 6a686c31324c9e Sebastian Hesselbarth 2014-10-21  197  	if (IS_ERR(host))
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  198  		return PTR_ERR(host);
> 6a686c31324c9e Sebastian Hesselbarth 2014-10-21  199
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  200  	pltfm_host = sdhci_priv(host);
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  201
> edf4ccd94bbef1 Doug Brown            2023-01-11  202  	clk = devm_clk_get(dev, "io");
> edf4ccd94bbef1 Doug Brown            2023-01-11  203  	if (IS_ERR(clk) && PTR_ERR(clk) != -EPROBE_DEFER)
> edf4ccd94bbef1 Doug Brown            2023-01-11  204  		clk = devm_clk_get(dev, NULL);
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  205  	if (IS_ERR(clk)) {
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  206  		ret = PTR_ERR(clk);
> edf4ccd94bbef1 Doug Brown            2023-01-11  207  		dev_err_probe(dev, ret, "failed to get io clock\n");
> 3fd1d86f03cbcc Masahiro Yamada       2017-08-23  208  		goto free;
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  209  	}
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  210  	pltfm_host->clk = clk;
> 21b22284619bbb Alexey Khoroshilov    2017-02-11  211  	ret = clk_prepare_enable(clk);
> 21b22284619bbb Alexey Khoroshilov    2017-02-11  212  	if (ret) {
> edf4ccd94bbef1 Doug Brown            2023-01-11  213  		dev_err(dev, "failed to enable io clock\n");
> 3fd1d86f03cbcc Masahiro Yamada       2017-08-23  214  		goto free;
> 21b22284619bbb Alexey Khoroshilov    2017-02-11  215  	}
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  216
> d8981da5ec7505 Doug Brown            2023-01-11  217  	clk_core = devm_clk_get_optional_enabled(dev, "core");
> d8981da5ec7505 Doug Brown            2023-01-11  218  	if (IS_ERR(clk_core)) {
> d8981da5ec7505 Doug Brown            2023-01-11  219  		dev_err_probe(dev, PTR_ERR(clk_core), "failed to enable core clock\n");
> d8981da5ec7505 Doug Brown            2023-01-11 @220  		goto disable_clk;
> 
> ret = PTR_ERR(clk_core);
> 
> d8981da5ec7505 Doug Brown            2023-01-11  221  	}
> d8981da5ec7505 Doug Brown            2023-01-11  222
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  223  	host->quirks = SDHCI_QUIRK_BROKEN_ADMA
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  224  		| SDHCI_QUIRK_BROKEN_TIMEOUT_VAL
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  225  		| SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN;
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  226
> 568536d7eb1969 Doug Brown            2023-01-11  227  	variant = of_device_get_match_data(dev);
> 568536d7eb1969 Doug Brown            2023-01-11  228  	if (variant)
> b650352dd3df36 Chris Ball            2012-04-10  229  		pdata = pxav2_get_mmc_pdata(dev);
> 568536d7eb1969 Doug Brown            2023-01-11  230  	else
> 568536d7eb1969 Doug Brown            2023-01-11  231  		variant = &pxav2_variant;
> 568536d7eb1969 Doug Brown            2023-01-11  232
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  233  	if (pdata) {
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  234  		if (pdata->flags & PXA_FLAG_CARD_PERMANENT) {
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  235  			/* on-chip device */
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  236  			host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  237  			host->mmc->caps |= MMC_CAP_NONREMOVABLE;
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  238  		}
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  239
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  240  		/* If slot design supports 8 bit data, indicate this to MMC. */
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  241  		if (pdata->flags & PXA_FLAG_SD_8_BIT_CAPABLE_SLOT)
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  242  			host->mmc->caps |= MMC_CAP_8_BIT_DATA;
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  243
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  244  		if (pdata->quirks)
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  245  			host->quirks |= pdata->quirks;
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  246  		if (pdata->host_caps)
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  247  			host->mmc->caps |= pdata->host_caps;
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  248  		if (pdata->pm_caps)
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  249  			host->mmc->pm_caps |= pdata->pm_caps;
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  250  	}
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  251
> 568536d7eb1969 Doug Brown            2023-01-11  252  	host->quirks |= variant->extra_quirks;
> 568536d7eb1969 Doug Brown            2023-01-11  253  	host->ops = variant->ops;
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  254
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  255  	ret = sdhci_add_host(host);
> fb8617e1ee4d40 Jisheng Zhang         2018-05-25  256  	if (ret)
> 3fd1d86f03cbcc Masahiro Yamada       2017-08-23  257  		goto disable_clk;
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  258
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  259  	return 0;
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  260
> 3fd1d86f03cbcc Masahiro Yamada       2017-08-23  261  disable_clk:
> 164378efe7612a Chao Xie              2012-07-31  262  	clk_disable_unprepare(clk);
> 3fd1d86f03cbcc Masahiro Yamada       2017-08-23  263  free:
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  264  	sdhci_pltfm_free(pdev);
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  265  	return ret;
> 9f5d71e4a78a02 Zhangfei Gao          2011-06-08  266  }
> 

Re: [PATCH v4 5/8] mmc: sdhci-pxav2: add optional core clock

[Dan Carpenter]

On Sat, Jan 14, 2023 at 02:49:07PM -0800, Doug Brown wrote:
> Hi Dan,
> 
> On 1/14/2023 12:01 AM, Dan Carpenter wrote:
> > Hi Doug,
> > 
> > https://git-scm.com/docs/git-format-patch#_base_tree_information]
> > 
> > url:    https://github.com/intel-lab-lkp/linux/commits/Doug-Brown/mmc-sdhci-pxav2-add-initial-support-for-PXA168-V1-controller/20230112-102921
> > base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
> > patch link:    https://lore.kernel.org/r/20230112022416.8474-6-doug%40schmorgal.com
> > patch subject: [PATCH v4 5/8] mmc: sdhci-pxav2: add optional core clock
> > config: riscv-randconfig-m041-20230113
> > compiler: riscv64-linux-gcc (GCC) 12.1.0
> > 
> > If you fix the issue, kindly add following tag where applicable
> > | Reported-by: kernel test robot <lkp@intel.com>
> > | Reported-by: Dan Carpenter <error27@gmail.com>
> > 
> > smatch warnings:
> > drivers/mmc/host/sdhci-pxav2.c:220 sdhci_pxav2_probe() warn: missing error code 'ret'
> 
> Thanks for passing this on. I definitely forgot an assignment to ret.
> Since this is correcting an error in my patch that hasn't been accepted
> yet, is it safe to assume I should omit those Reported-by tags from the
> next version of my patch, since they don't apply to the patch itself?
> 

These emails are from the kbuild team and not from me.  I just look them
over and hit the forward button.  I'm sure it helps the kbuild team in
their marketing when people use the tags...  Right now I'm applying to
jobs outside the Linux community so the tags give me a measurable thing
to say I've helped fix thousands of bugs or whatever...

I've always argued that there should be a different Fixes-from: tag for
people who find bugs during review (as opposed to just complaining about
white space which is its own reward and I do that for free).  So far I
haven't convinced anyone on this though.

Anyway, there isn't a policy one way or the other.  Some people add
them and some don't.  Some people add them below the --- cut off line,
but I don't know if that's deliberate or what the story is there.  That
seems like it might be a good compromise.

regards,
dan carpenter

Re: [PATCH v4 5/8] mmc: sdhci-pxav2: add optional core clock

[Krzysztof Kozlowski]

On 16/01/2023 10:02, Dan Carpenter wrote:
> On Sat, Jan 14, 2023 at 02:49:07PM -0800, Doug Brown wrote:
>> Hi Dan,
>>
>> On 1/14/2023 12:01 AM, Dan Carpenter wrote:
>>> Hi Doug,
>>>
>>> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>>>
>>> url:    https://github.com/intel-lab-lkp/linux/commits/Doug-Brown/mmc-sdhci-pxav2-add-initial-support-for-PXA168-V1-controller/20230112-102921
>>> base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
>>> patch link:    https://lore.kernel.org/r/20230112022416.8474-6-doug%40schmorgal.com
>>> patch subject: [PATCH v4 5/8] mmc: sdhci-pxav2: add optional core clock
>>> config: riscv-randconfig-m041-20230113
>>> compiler: riscv64-linux-gcc (GCC) 12.1.0
>>>
>>> If you fix the issue, kindly add following tag where applicable
>>> | Reported-by: kernel test robot <lkp@intel.com>
>>> | Reported-by: Dan Carpenter <error27@gmail.com>
>>>
>>> smatch warnings:
>>> drivers/mmc/host/sdhci-pxav2.c:220 sdhci_pxav2_probe() warn: missing error code 'ret'
>>
>> Thanks for passing this on. I definitely forgot an assignment to ret.
>> Since this is correcting an error in my patch that hasn't been accepted
>> yet, is it safe to assume I should omit those Reported-by tags from the
>> next version of my patch, since they don't apply to the patch itself?
>>
> 
> These emails are from the kbuild team and not from me.  I just look them
> over and hit the forward button.  I'm sure it helps the kbuild team in
> their marketing when people use the tags...  Right now I'm applying to
> jobs outside the Linux community so the tags give me a measurable thing
> to say I've helped fix thousands of bugs or whatever...
> 
> I've always argued that there should be a different Fixes-from: tag for
> people who find bugs during review (as opposed to just complaining about
> white space which is its own reward and I do that for free).  So far I
> haven't convinced anyone on this though.

Oh, I am in. This is considerable effort to find bugs on the mailing
lists or in linux-next. Even if it is pure bisect and test, it's a lot
of time. If it is not credited, we loose some valuable feedback and
incentives.

Best regards,
Krzysztof

Re: [PATCH v4 5/8] mmc: sdhci-pxav2: add optional core clock

[Doug Brown]

On 1/16/2023 1:02 AM, Dan Carpenter wrote:
> On Sat, Jan 14, 2023 at 02:49:07PM -0800, Doug Brown wrote:
>> Hi Dan,
>>
>> On 1/14/2023 12:01 AM, Dan Carpenter wrote:
>>> Hi Doug,
>>>
>>> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>>>
>>> url:    https://github.com/intel-lab-lkp/linux/commits/Doug-Brown/mmc-sdhci-pxav2-add-initial-support-for-PXA168-V1-controller/20230112-102921
>>> base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
>>> patch link:    https://lore.kernel.org/r/20230112022416.8474-6-doug%40schmorgal.com
>>> patch subject: [PATCH v4 5/8] mmc: sdhci-pxav2: add optional core clock
>>> config: riscv-randconfig-m041-20230113
>>> compiler: riscv64-linux-gcc (GCC) 12.1.0
>>>
>>> If you fix the issue, kindly add following tag where applicable
>>> | Reported-by: kernel test robot <lkp@intel.com>
>>> | Reported-by: Dan Carpenter <error27@gmail.com>
>>>
>>> smatch warnings:
>>> drivers/mmc/host/sdhci-pxav2.c:220 sdhci_pxav2_probe() warn: missing error code 'ret'
>>
>> Thanks for passing this on. I definitely forgot an assignment to ret.
>> Since this is correcting an error in my patch that hasn't been accepted
>> yet, is it safe to assume I should omit those Reported-by tags from the
>> next version of my patch, since they don't apply to the patch itself?
>>
> 
> These emails are from the kbuild team and not from me.  I just look them
> over and hit the forward button.  I'm sure it helps the kbuild team in
> their marketing when people use the tags...  Right now I'm applying to
> jobs outside the Linux community so the tags give me a measurable thing
> to say I've helped fix thousands of bugs or whatever...
> 
> I've always argued that there should be a different Fixes-from: tag for
> people who find bugs during review (as opposed to just complaining about
> white space which is its own reward and I do that for free).  So far I
> haven't convinced anyone on this though.
> 
> Anyway, there isn't a policy one way or the other.  Some people add
> them and some don't.  Some people add them below the --- cut off line,
> but I don't know if that's deliberate or what the story is there.  That
> seems like it might be a good compromise.

Thanks for all the added context. I knew you had forwarded it from the
kbuild team but I wasn't sure who to ask for clarification. Your idea
for a future Fixes-from: tag for this type of thing makes perfect sense.
For now, it seems that if the git log might be used for obtaining
metrics, I should just go ahead and put the Reported-by: tags in. I
really appreciate that you took the time to explain this to me.