[PATCH 1/4] mci: core: bus-width property should override driver default

[PATCH 1/4] mci: core: bus-width property should override driver default

[Trent Piepho]

The OF code for parsing bus-width would only add the specified width
to those the driver might have already set capability flags for.

Because of this, if the driver had set 8 or 4 bit width, it wasn't
possible for the DT to specify that fewer pins were used on the board
and a smaller width was necessary.

Change this so the width in the DT overrides whatever widths the
driver says it supports.  There is no reason to have an incorrect
device tree and it makes far more sense for the DT to override the
driver default than for the driver default to override the DT.

The widths the driver puts in host_caps before calling mci_of_parse()
are considered the default if the DT doesn't specify bus-width.  This
should cause the least amount of change to existing boards, as despite
a comment that no bus-width meant to use 1 bit, using the driver
default is what was really happening.

Unfortunately, half of existing drivers default to the largest width
they support while the other half default to the smallest.  Boards
should just stick the width in the device tree.

Signed-off-by: Trent Piepho <tpiepho@kymetacorp.com>
---
 drivers/mci/mci-core.c | 38 ++++++++++++++++++++++----------------
 include/mci.h          |  2 ++
 2 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
index 29c0d54..4e6b83b 100644
--- a/drivers/mci/mci-core.c
+++ b/drivers/mci/mci-core.c
@@ -1819,23 +1819,29 @@ void mci_of_parse(struct mci_host *host)
 
 	/* "bus-width" is translated to MMC_CAP_*_BIT_DATA flags */
 	if (of_property_read_u32(np, "bus-width", &bus_width) < 0) {
+		/* If bus-width is missing we get the driver's default, which
+		 * is, unfortunately, not consistent from driver to driver.
+		 * Better to specify it in the device tree.  */
 		dev_dbg(host->hw_dev,
-			"\"bus-width\" property is missing, assuming 1 bit.\n");
-		bus_width = 1;
-	}
-
-	switch (bus_width) {
-	case 8:
-		host->host_caps |= MMC_CAP_8_BIT_DATA;
-		/* Hosts capable of 8-bit transfers can also do 4 bits */
-	case 4:
-		host->host_caps |= MMC_CAP_4_BIT_DATA;
-		break;
-	case 1:
-		break;
-	default:
-		dev_err(host->hw_dev,
-			"Invalid \"bus-width\" value %u!\n", bus_width);
+			"\"bus-width\" property missing, default is %d\n",
+			(host->host_caps & MMC_CAP_8_BIT_DATA) ? 8 :
+			(host->host_caps & MMC_CAP_4_BIT_DATA) ? 4 : 1);
+	} else {
+		/* Set data width caps to exactly those specified in the DT.
+		 * bus-width isn't a list, so widths smaller than the specified
+		 * value are implictly supported as well.  */
+		host->host_caps &= ~MMC_CAP_BIT_DATA_MASK;
+		switch (bus_width) {
+		case 8:
+			host->host_caps |= MMC_CAP_8_BIT_DATA;
+		case 4: /* note fall through from above */
+			host->host_caps |= MMC_CAP_4_BIT_DATA;
+		case 1:
+			break;
+		default:
+			dev_err(host->hw_dev,
+				"Invalid \"bus-width\" value %u!\n", bus_width);
+		}
 	}
 
 	/* f_max is obtained from the optional "max-frequency" property */
diff --git a/include/mci.h b/include/mci.h
index 41a757e..174d150 100644
--- a/include/mci.h
+++ b/include/mci.h
@@ -56,6 +56,8 @@
 #define MMC_CAP_SD_HIGHSPEED		(1 << 3)
 #define MMC_CAP_MMC_HIGHSPEED		(1 << 4)
 #define MMC_CAP_MMC_HIGHSPEED_52MHZ	(1 << 5)
+/* Mask of all caps for bus width */
+#define MMC_CAP_BIT_DATA_MASK		(MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA)
 
 #define SD_DATA_4BIT		0x00040000
 
-- 
1.8.3.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/4] mci: dw_mmc: socfpga: Supply bus-width in platform_data

[Trent Piepho]

Since there is no OF support in the xloader on socfpga it uses the
platform_data system.  There needs to be a way to supply the
equivalent of the DT property bus-width this way to support devices
that need to use a smaller bus.

So that we don't need to put every flag that might get added to the
MMC_CAP list into platform_data, just put the bus width ones into
platform_data.

The socfpga dts sources specify a bus-width of 4 so use that in the
platform_data for socfpga.

Signed-off-by: Trent Piepho <tpiepho@kymetacorp.com>
---
 arch/arm/mach-socfpga/xload.c  |  2 ++
 drivers/mci/dw_mmc.c           | 24 +++++++++++++-----------
 include/platform_data/dw_mmc.h |  1 +
 3 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/arch/arm/mach-socfpga/xload.c b/arch/arm/mach-socfpga/xload.c
index 272896b..cf05ff3 100644
--- a/arch/arm/mach-socfpga/xload.c
+++ b/arch/arm/mach-socfpga/xload.c
@@ -10,6 +10,7 @@
 #include <linux/sizes.h>
 #include <fs.h>
 #include <io.h>
+#include <mci.h>
 
 #include <linux/clkdev.h>
 #include <linux/stat.h>
@@ -33,6 +34,7 @@ enum socfpga_clks {
 static struct clk *clks[clk_max];
 
 static struct dw_mmc_platform_data mmc_pdata = {
+	.bus_width_caps = MMC_CAP_4_BIT_DATA,
 	.ciu_div = 3,
 };
 
diff --git a/drivers/mci/dw_mmc.c b/drivers/mci/dw_mmc.c
index acdf795..f8ff389 100644
--- a/drivers/mci/dw_mmc.c
+++ b/drivers/mci/dw_mmc.c
@@ -698,9 +698,22 @@ static int dw_mmc_probe(struct device_d *dev)
 	if (IS_ERR(host->ioaddr))
 		return PTR_ERR(host->ioaddr);
 
+	host->idmac = dma_alloc_coherent(sizeof(*host->idmac) * DW_MMC_NUM_IDMACS,
+					 DMA_ADDRESS_BROKEN);
+
+	host->mci.send_cmd = dwmci_cmd;
+	host->mci.set_ios = dwmci_set_ios;
+	host->mci.init = dwmci_init;
+	host->mci.card_present = dwmci_card_present;
+	host->mci.hw_dev = dev;
+	host->mci.voltages = MMC_VDD_32_33 | MMC_VDD_33_34;
+	host->mci.host_caps = MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA;
+
 	if (pdata) {
 		mci->devname = pdata->devname;
 		host->ciu_div = pdata->ciu_div;
+		host->mci.host_caps &= ~MMC_CAP_BIT_DATA_MASK;
+		host->mci.host_caps |= pdata->bus_width_caps;
 	} else if (dev->device_node) {
 		const char *alias = of_alias_get(dev->device_node);
 		if (alias)
@@ -712,17 +725,6 @@ static int dw_mmc_probe(struct device_d *dev)
 	/* divider is 0 based in pdata and 1 based in our private struct */
 	host->ciu_div++;
 
-	host->idmac = dma_alloc_coherent(sizeof(*host->idmac) * DW_MMC_NUM_IDMACS,
-					 DMA_ADDRESS_BROKEN);
-
-	host->mci.send_cmd = dwmci_cmd;
-	host->mci.set_ios = dwmci_set_ios;
-	host->mci.init = dwmci_init;
-	host->mci.card_present = dwmci_card_present;
-	host->mci.hw_dev = dev;
-	host->mci.voltages = MMC_VDD_32_33 | MMC_VDD_33_34;
-	host->mci.host_caps = MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA;
-
 	if (of_device_is_compatible(dev->device_node,
 	    "rockchip,rk2928-dw-mshc"))
 		host->pwren_value = 0;
diff --git a/include/platform_data/dw_mmc.h b/include/platform_data/dw_mmc.h
index 48faa76..ce1b339 100644
--- a/include/platform_data/dw_mmc.h
+++ b/include/platform_data/dw_mmc.h
@@ -3,6 +3,7 @@
 
 struct dw_mmc_platform_data {
 	char *devname;
+	unsigned int bus_width_caps;
 	int ciu_div;
 };
 
-- 
1.8.3.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 3/4] mci: dw_mmc: Delete devname in platform_data

[Trent Piepho]

Nothing used it.

Also delete the local mci alias pointer to host->mci in
dw_mmc_probe().  It only saved a few characters and all the references
but one are using host->mci.

Signed-off-by: Trent Piepho <tpiepho@kymetacorp.com>
---
 drivers/mci/dw_mmc.c           | 5 +----
 include/platform_data/dw_mmc.h | 1 -
 2 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/mci/dw_mmc.c b/drivers/mci/dw_mmc.c
index f8ff389..18ddd16 100644
--- a/drivers/mci/dw_mmc.c
+++ b/drivers/mci/dw_mmc.c
@@ -676,11 +676,9 @@ static int dw_mmc_detect(struct device_d *dev)
 static int dw_mmc_probe(struct device_d *dev)
 {
 	struct dwmci_host *host;
-	struct mci_host *mci;
 	struct dw_mmc_platform_data *pdata = dev->platform_data;
 
 	host = xzalloc(sizeof(*host));
-	mci = &host->mci;
 
 	host->clk_biu = clk_get(dev, "biu");
 	if (IS_ERR(host->clk_biu))
@@ -710,14 +708,13 @@ static int dw_mmc_probe(struct device_d *dev)
 	host->mci.host_caps = MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA;
 
 	if (pdata) {
-		mci->devname = pdata->devname;
 		host->ciu_div = pdata->ciu_div;
 		host->mci.host_caps &= ~MMC_CAP_BIT_DATA_MASK;
 		host->mci.host_caps |= pdata->bus_width_caps;
 	} else if (dev->device_node) {
 		const char *alias = of_alias_get(dev->device_node);
 		if (alias)
-			mci->devname = xstrdup(alias);
+			host->mci.devname = xstrdup(alias);
 		of_property_read_u32(dev->device_node, "dw-mshc-ciu-div",
 				&host->ciu_div);
 	}
diff --git a/include/platform_data/dw_mmc.h b/include/platform_data/dw_mmc.h
index ce1b339..4325a4f 100644
--- a/include/platform_data/dw_mmc.h
+++ b/include/platform_data/dw_mmc.h
@@ -2,7 +2,6 @@
 #define __INCLUDE_PLATFORM_DATA_DW_MMC_H
 
 struct dw_mmc_platform_data {
-	char *devname;
 	unsigned int bus_width_caps;
 	int ciu_div;
 };
-- 
1.8.3.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 4/4] mci: dw_mmc: Add support for high speed modes

[Trent Piepho]

The Synopsys DesignWare core supports 52 MHz MMC mode and 50 MHz SD
mode, so add the cap flags so they are used.

This works on socfpga.  The other user of this driver is Rockchip and
the datasheet I found for the RK30xx indicated it supported highspeed
modes as well.

The Linux kernel has DT properties, e.g. "cap-mmc-highspeed", but none
of the drivers in barebox support this at all.  They all specify flags
in their code.

Signed-off-by: Trent Piepho <tpiepho@kymetacorp.com>
---
 drivers/mci/dw_mmc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mci/dw_mmc.c b/drivers/mci/dw_mmc.c
index 18ddd16..cbd3f00 100644
--- a/drivers/mci/dw_mmc.c
+++ b/drivers/mci/dw_mmc.c
@@ -706,6 +706,8 @@ static int dw_mmc_probe(struct device_d *dev)
 	host->mci.hw_dev = dev;
 	host->mci.voltages = MMC_VDD_32_33 | MMC_VDD_33_34;
 	host->mci.host_caps = MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA;
+	host->mci.host_caps |= MMC_CAP_MMC_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED_52MHZ |
+			       MMC_CAP_SD_HIGHSPEED;
 
 	if (pdata) {
 		host->ciu_div = pdata->ciu_div;
-- 
1.8.3.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/4] mci: core: bus-width property should override driver default

[Sascha Hauer]

On Wed, Nov 18, 2015 at 09:06:46PM +0000, Trent Piepho wrote:
> The OF code for parsing bus-width would only add the specified width
> to those the driver might have already set capability flags for.
> 
> Because of this, if the driver had set 8 or 4 bit width, it wasn't
> possible for the DT to specify that fewer pins were used on the board
> and a smaller width was necessary.
> 
> Change this so the width in the DT overrides whatever widths the
> driver says it supports.  There is no reason to have an incorrect
> device tree and it makes far more sense for the DT to override the
> driver default than for the driver default to override the DT.
> 
> The widths the driver puts in host_caps before calling mci_of_parse()
> are considered the default if the DT doesn't specify bus-width.  This
> should cause the least amount of change to existing boards, as despite
> a comment that no bus-width meant to use 1 bit, using the driver
> default is what was really happening.
> 
> Unfortunately, half of existing drivers default to the largest width
> they support while the other half default to the smallest.  Boards
> should just stick the width in the device tree.
> 
> Signed-off-by: Trent Piepho <tpiepho@kymetacorp.com>
> ---
>  drivers/mci/mci-core.c | 38 ++++++++++++++++++++++----------------
>  include/mci.h          |  2 ++
>  2 files changed, 24 insertions(+), 16 deletions(-)

Applied, thanks

Sascha



-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox