| Message ID | 1350306959-5843-1-git-send-email-lee.jones@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
> 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
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
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
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
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;