[PATCH 1/2] mmc: core: Support all MMC capabilities when booting from Device Tree

[PATCH 1/2] mmc: core: Support all MMC capabilities when booting from Device Tree

[Lee Jones]

Capabilities are an important part of the MMC subsystem. Much
supported functionality would be lost if we didn't provide the
same level of support when booting Device Tree as we currently
do when the subsystem is passed capabilities via platform data.
This patch supplies this support with one simple call to a
DT parsing function.

Cc: Chris Ball <cjb@laptop.org>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: linux-mmc@vger.kernel.org
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mmc/core/host.c  |   93 ++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mmc/host.h |    3 +-
 2 files changed, 95 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index ee2e16b..61a02db 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -20,6 +20,7 @@
 #include <linux/leds.h>
 #include <linux/slab.h>
 #include <linux/suspend.h>
+#include <linux/of.h>
 
 #include <linux/mmc/host.h>
 #include <linux/mmc/card.h>
@@ -436,3 +437,95 @@ void mmc_free_host(struct mmc_host *host)
 }
 
 EXPORT_SYMBOL(mmc_free_host);
+
+/**
+ *	mmc_of_populate_caps - support all MMC capabilities from Device Tree
+ *	@np: MMC OF device node
+ *	@caps: Host capabilities - as per linux/mmc/host.h
+ *	@caps2: More host cabibilies - as per linux/mmc/host.h
+ *
+ *	Read capability string from the device node provided and populate
+ *	the capability container accordingly.
+ */
+void mmc_of_populate_caps(struct device_node *np,
+			  unsigned long *caps,
+			  unsigned int *caps2)
+{
+	if(of_property_read_bool(np, "mmc-cap-4-bit-data"))
+		*caps |= MMC_CAP_4_BIT_DATA;
+	if(of_property_read_bool(np, "mmc-cap-mmc-highspeed"))
+		*caps |= MMC_CAP_MMC_HIGHSPEED;
+	if(of_property_read_bool(np, "mmc-cap-sd-highspeed"))
+		*caps |= MMC_CAP_SD_HIGHSPEED;
+	if(of_property_read_bool(np, "mmc-cap-sdio-irq"))
+		*caps |= MMC_CAP_SDIO_IRQ;
+	if(of_property_read_bool(np, "mmc-cap-spi"))
+		*caps |= MMC_CAP_SPI;
+	if(of_property_read_bool(np, "mmc-cap-needs-poll"))
+		*caps |= MMC_CAP_NEEDS_POLL;
+	if(of_property_read_bool(np, "mmc-cap-8-bit-data"))
+		*caps |= MMC_CAP_8_BIT_DATA;
+	if(of_property_read_bool(np, "mmc-cap-nonremovable"))
+		*caps |= MMC_CAP_NONREMOVABLE;
+	if(of_property_read_bool(np, "mmc-cap-wait-while-busy"))
+		*caps |= MMC_CAP_WAIT_WHILE_BUSY;
+	if(of_property_read_bool(np, "mmc-cap-erase"))
+		*caps |= MMC_CAP_ERASE;
+	if(of_property_read_bool(np, "mmc-cap-1-8v-ddr"))
+		*caps |= MMC_CAP_1_8V_DDR;
+	if(of_property_read_bool(np, "mmc-cap-1-2v-ddr"))
+		*caps |= MMC_CAP_1_2V_DDR;
+	if(of_property_read_bool(np, "mmc-cap-power-off-card"))
+		*caps |= MMC_CAP_POWER_OFF_CARD;
+	if(of_property_read_bool(np, "mmc-cap-bus-width-test"))
+		*caps |= MMC_CAP_BUS_WIDTH_TEST;
+	if(of_property_read_bool(np, "mmc-cap-uhs-sdr12"))
+		*caps |= MMC_CAP_UHS_SDR12;
+	if(of_property_read_bool(np, "mmc-cap-uhs-sdr25"))
+		*caps |= MMC_CAP_UHS_SDR25;
+	if(of_property_read_bool(np, "mmc-cap-uhs-sdr50"))
+		*caps |= MMC_CAP_UHS_SDR50;
+	if(of_property_read_bool(np, "mmc-cap-uhs-sdr104"))
+		*caps |= MMC_CAP_UHS_SDR104;
+	if(of_property_read_bool(np, "mmc-cap-uhs-ddr50"))
+		*caps |= MMC_CAP_UHS_DDR50;
+	if(of_property_read_bool(np, "mmc-cap-driver-type-a"))
+		*caps |= MMC_CAP_DRIVER_TYPE_A;
+	if(of_property_read_bool(np, "mmc-cap-driver-type-c"))
+		*caps |= MMC_CAP_DRIVER_TYPE_C;
+	if(of_property_read_bool(np, "mmc-cap-driver-type-d"))
+		*caps |= MMC_CAP_DRIVER_TYPE_D;
+	if(of_property_read_bool(np, "mmc-cap-cmd23"))
+		*caps |= MMC_CAP_CMD23;
+	if(of_property_read_bool(np, "mmc-cap-hw-reset"))
+		*caps |= MMC_CAP_HW_RESET;
+
+	if(of_property_read_bool(np, "mmc-cap2-bootpart-noacc"))
+		*caps2 |= MMC_CAP2_BOOTPART_NOACC;
+	if(of_property_read_bool(np, "mmc-cap2-cache-ctrl"))
+		*caps2 |= MMC_CAP2_CACHE_CTRL;
+	if(of_property_read_bool(np, "mmc-cap2-poweroff-notify"))
+		*caps2 |= MMC_CAP2_POWEROFF_NOTIFY;
+	if(of_property_read_bool(np, "mmc-cap2-no-multi-read"))
+		*caps2 |= MMC_CAP2_NO_MULTI_READ;
+	if(of_property_read_bool(np, "mmc-cap2-no-sleep-cmd"))
+		*caps2 |= MMC_CAP2_NO_SLEEP_CMD;
+	if(of_property_read_bool(np, "mmc-cap2-hs200-1-8v-sdr"))
+		*caps2 |= MMC_CAP2_HS200_1_8V_SDR;
+	if(of_property_read_bool(np, "mmc-cap2-hs200-1-2v-sdr"))
+		*caps2 |= MMC_CAP2_HS200_1_2V_SDR;
+	if(of_property_read_bool(np, "mmc-cap2-hs200"))
+		*caps2 |= MMC_CAP2_HS200;
+	if(of_property_read_bool(np, "mmc-cap2-broken-voltage"))
+		*caps2 |= MMC_CAP2_BROKEN_VOLTAGE;
+	if(of_property_read_bool(np, "mmc-cap2-detect-on-err"))
+		*caps2 |= MMC_CAP2_DETECT_ON_ERR;
+	if(of_property_read_bool(np, "mmc-cap2-hc-erase-sz"))
+		*caps2 |= MMC_CAP2_HC_ERASE_SZ;
+	if(of_property_read_bool(np, "mmc-cap2-cd-active-high"))
+		*caps2 |= MMC_CAP2_CD_ACTIVE_HIGH;
+	if(of_property_read_bool(np, "mmc-cap2-ro-active-high"))
+		*caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
+}
+
+EXPORT_SYMBOL(mmc_of_populate_caps);
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 7abb0e1..612cf0e 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -344,7 +344,8 @@ extern struct mmc_host *mmc_alloc_host(int extra, struct device *);
 extern int mmc_add_host(struct mmc_host *);
 extern void mmc_remove_host(struct mmc_host *);
 extern void mmc_free_host(struct mmc_host *);
-
+extern void mmc_of_populate_caps(struct device_node *,
+				 unsigned long *, unsigned int *);
 static inline void *mmc_priv(struct mmc_host *host)
 {
 	return (void *)host->private;
-- 
1.7.9.5

[PATCH 2/2] mmc: mmci: Make use of new DT capability parsing function

[Lee Jones]

Instead of rolling our own parsers for each new capability we
wish to support, we can make use of a call-once function which
parses all known capability strings and populates the
necessary properties for us. All we have to do is remove our
own cruft and invoke the call.

Cc: Chris Ball <cjb@laptop.org>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: linux-mmc@vger.kernel.org
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mmc/host/mmci.c |   20 +-------------------
 1 file changed, 1 insertion(+), 19 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index edc3e9b..bc02f05 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -1227,25 +1227,7 @@ static void mmci_dt_populate_generic_pdata(struct device_node *np,
 	if (!pdata->f_max)
 		pr_warn("%s has no 'max-frequency' property\n", np->full_name);
 
-	if (of_get_property(np, "mmc-cap-mmc-highspeed", NULL))
-		pdata->capabilities |= MMC_CAP_MMC_HIGHSPEED;
-	if (of_get_property(np, "mmc-cap-sd-highspeed", NULL))
-		pdata->capabilities |= MMC_CAP_SD_HIGHSPEED;
-
-	of_property_read_u32(np, "bus-width", &bus_width);
-	switch (bus_width) {
-	case 0 :
-		/* No bus-width supplied. */
-		break;
-	case 4 :
-		pdata->capabilities |= MMC_CAP_4_BIT_DATA;
-		break;
-	case 8 :
-		pdata->capabilities |= MMC_CAP_8_BIT_DATA;
-		break;
-	default :
-		pr_warn("%s: Unsupported bus width\n", np->full_name);
-	}
+	mmc_of_populate_caps(np, &pdata->capabilities, &pdata->capabilities2);
 }
 #else
 static void mmci_dt_populate_generic_pdata(struct device_node *np,
-- 
1.7.9.5

Re: [PATCH 1/2] mmc: core: Support all MMC capabilities when booting from Device Tree

[Arnd Bergmann]

On Monday 15 October 2012, Lee Jones wrote:
> Capabilities are an important part of the MMC subsystem. Much
> supported functionality would be lost if we didn't provide the
> same level of support when booting Device Tree as we currently
> do when the subsystem is passed capabilities via platform data.
> This patch supplies this support with one simple call to a
> DT parsing function.

We already document all the commonly used properties
in Documentation/devicetree/bindings/mmc/mmc.txt

Please don't add any duplicates or those that are not used
so far.

> +	if(of_property_read_bool(np, "mmc-cap-4-bit-data"))
> +		*caps |= MMC_CAP_4_BIT_DATA;

see "bus-width" property.

> +	if(of_property_read_bool(np, "mmc-cap-mmc-highspeed"))
> +		*caps |= MMC_CAP_MMC_HIGHSPEED;
> +	if(of_property_read_bool(np, "mmc-cap-sd-highspeed"))
> +		*caps |= MMC_CAP_SD_HIGHSPEED;

implied by "max-frequency" property.

> +	if(of_property_read_bool(np, "mmc-cap-sdio-irq"))
> +		*caps |= MMC_CAP_SDIO_IRQ;

implied by presence of SDIO irq property.

> +	if(of_property_read_bool(np, "mmc-cap-spi"))
> +		*caps |= MMC_CAP_SPI;

Only used by the mmc_spi driver, can be hardcoded there.

> +	if(of_property_read_bool(np, "mmc-cap-needs-poll"))
> +		*caps |= MMC_CAP_NEEDS_POLL;

implied by absence of irqs property.

> +	if(of_property_read_bool(np, "mmc-cap-8-bit-data"))
> +		*caps |= MMC_CAP_8_BIT_DATA;

see "bus-width" property.

> +	if(of_property_read_bool(np, "mmc-cap-nonremovable"))
> +		*caps |= MMC_CAP_NONREMOVABLE;

see "non-removable property.

> +	if(of_property_read_bool(np, "mmc-cap-wait-while-busy"))
> +		*caps |= MMC_CAP_WAIT_WHILE_BUSY;

This seems to be a linux device driver specific quirk that doesn't
belong into a hardware description.

> +	if(of_property_read_bool(np, "mmc-cap-erase"))
> +		*caps |= MMC_CAP_ERASE;

driver specific.

> ...

and so on. What are you actually missing in the properties that
are already there?

	Arnd

Re: [PATCH 1/2] mmc: core: Support all MMC capabilities when booting from Device Tree

[Lee Jones]

> On Monday 15 October 2012, Lee Jones wrote:
> > Capabilities are an important part of the MMC subsystem. Much
> > supported functionality would be lost if we didn't provide the
> > same level of support when booting Device Tree as we currently
> > do when the subsystem is passed capabilities via platform data.
> > This patch supplies this support with one simple call to a
> > DT parsing function.
> 
> We already document all the commonly used properties
> in Documentation/devicetree/bindings/mmc/mmc.txt

<snip>

> and so on. What are you actually missing in the properties that
> are already there?

MMC_CAP_ERASE
MMC_CAP_UHS_SDR12
MMC_CAP_UHS_SDR25
MMC_CAP_UHS_DDR50
MMC_CAP_1_8V_DDR

MMC_CAP2_DETECT_ON_ERR
MMC_CAP2_NO_SLEEP_CMD

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 1/2] mmc: core: Support all MMC capabilities when booting from Device Tree

[Arnd Bergmann]

On Monday 15 October 2012, Lee Jones wrote:
> > and so on. What are you actually missing in the properties that
> > are already there?
> 
> MMC_CAP_ERASE

This one seems to be set unconditionally on some controllers but
not on others. Why would it need to be configurable?

> MMC_CAP_UHS_SDR12
> MMC_CAP_UHS_SDR25
> MMC_CAP_UHS_DDR50

Could this be derived from max-frequency?

> MMC_CAP_1_8V_DDR

Right, I suppose we need this. Should we have a minimum and maximum
voltage added to the common properties for this?

> MMC_CAP2_DETECT_ON_ERR
> MMC_CAP2_NO_SLEEP_CMD

I don't see these ones being set anywhere, but they were both
added by Ulf. Maybe he can comment on if or why they are needed
in devicetree, rather than being set by the driver unconditionally
or for specific versions of the host controller.

	Arnd

Re: [PATCH 1/2] mmc: core: Support all MMC capabilities when booting from Device Tree

[Ulf Hansson]

On 17 October 2012 15:38, Arnd Bergmann <arnd@arndb.de> wrote:
> On Monday 15 October 2012, Lee Jones wrote:
>> > and so on. What are you actually missing in the properties that
>> > are already there?
>>
>> MMC_CAP_ERASE
>
> This one seems to be set unconditionally on some controllers but
> not on others. Why would it need to be configurable?
>
>> MMC_CAP_UHS_SDR12
>> MMC_CAP_UHS_SDR25
>> MMC_CAP_UHS_DDR50
>
> Could this be derived from max-frequency?

No, this is likely depending on what the hw controller supports. Not
connected to the freq.
UHS also means 1.8 V I/O voltage.

>
>> MMC_CAP_1_8V_DDR
>
> Right, I suppose we need this. Should we have a minimum and maximum
> voltage added to the common properties for this?
>
>> MMC_CAP2_DETECT_ON_ERR
>> MMC_CAP2_NO_SLEEP_CMD
>
> I don't see these ones being set anywhere, but they were both
> added by Ulf. Maybe he can comment on if or why they are needed
> in devicetree, rather than being set by the driver unconditionally
> or for specific versions of the host controller.

>From ux500 perspective there are patches not been up-streamed yet
which are using these host caps, for whatever it is worth for you to
know and consider.

Actually, I think quite a few of the host caps in mmc could be debated
whether those should exist at all.
Some are directly mapped to what the host controller hw support, some
are purely what the host driver (sw) support, but then there are
others kind of "mmc/sd/sdio software support configuration" which are
kind of strange host caps to me. For example MMC_CAP2_DETECT_ON_ERR
which I invented. :-). I think it especially these "software support
configuration" caps that might be causing this dt issues.

Would be very interesting to hear if someone is sharing my thoughts
around the host caps. Or if I am totally wrong here.

Kind regards
Ulf Hansson

Re: [PATCH 1/2] mmc: core: Support all MMC capabilities when booting from Device Tree

[Philip Rakity]

On 17 Oct 2012, at 14:38, Arnd Bergmann <arnd@arndb.de> wrote:

> On Monday 15 October 2012, Lee Jones wrote:
>>> and so on. What are you actually missing in the properties that
>>> are already there?
>> 
>> MMC_CAP_ERASE
> 
> This one seems to be set unconditionally on some controllers but
> not on others. Why would it need to be configurable?
> 
>> MMC_CAP_UHS_SDR12
>> MMC_CAP_UHS_SDR25
>> MMC_CAP_UHS_DDR50
> 
> Could this be derived from max-frequency?

The problem is the controller may signal it supports DDR but the host cannot.  For example no voltage at correct level.
Same issue with 8 bit support.  Controller could say supports it but board has only 4 "wires"
> 
>> MMC_CAP_1_8V_DDR
> 
> Right, I suppose we need this. Should we have a minimum and maximum
> voltage added to the common properties for this?
> 
>> MMC_CAP2_DETECT_ON_ERR
>> MMC_CAP2_NO_SLEEP_CMD
> 
> I don't see these ones being set anywhere, but they were both
> added by Ulf. Maybe he can comment on if or why they are needed
> in devicetree, rather than being set by the driver unconditionally
> or for specific versions of the host controller.
> 
> 	Arnd
> --
> 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