Re: [PATCH 5/6] mmc: Add OF bindings support for mmc host controller capabilities
On 5 November 2011 01:27, Olof Johansson o...@lixom.net wrote: On Thu, Nov 03, 2011 at 02:06:02AM +0530, Thomas Abraham wrote: Device nodes representing sd/mmc controllers in a device tree would include mmc host controller capabilities. Add support for parsing of mmc host controller capabilities included in device nodes. Signed-off-by: Thomas Abraham thomas.abra...@linaro.org --- .../devicetree/bindings/mmc/linux-mmc-host.txt | 13 drivers/mmc/core/host.c | 31 include/linux/mmc/host.h | 1 + 3 files changed, 45 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/mmc/linux-mmc-host.txt diff --git a/Documentation/devicetree/bindings/mmc/linux-mmc-host.txt b/Documentation/devicetree/bindings/mmc/linux-mmc-host.txt new file mode 100644 index 000..714b2b1 --- /dev/null +++ b/Documentation/devicetree/bindings/mmc/linux-mmc-host.txt @@ -0,0 +1,13 @@ +* Linux MMC Host Controller Capabilities + +The following bindings can be used in a device node to specify any board +specific mmc host controller capabilities. + +- linux,mmc_cap_4_bit_data - Host can do 4-bit transfers +- linux,mmc_cap_mmc_highspeed - Host can do MMC high-speed timing +- linux,mmc_cap_sd_highspeed - Host can do SD high-speed timing +- linux,mmc_cap_needs_poll - Host needs polling for card detection +- linux,mmc_cap_8_bit_data - Host can do 8-bit transfer +- linux,mmc_cap_disable - Host can be disabled and re-enabled to save power +- linux,mmc_cap_nonremovable - Host is connected to nonremovable media +- linux,mmc_cap_erase - Host allows erase/trim commands linux-prefixed properties are a big red flag. The device tree should describe the hardware, not what linux does with the hardware. The vendor-prefixes documentation for device tree bindings includes 'linux' as an option. And I was trying to encode the linux specific host controller capabilities using the above bindings. See previous comments about support-8bit for encoding exactly the same hardware capability in a linux-agnostic way. What the sdhci driver chooses to do with it is up to the driver, and in some cases it means it will set the capabilities flag. Same goes for the other properties. It should not go in as it is implemented in this patch, it needs to be fixed up. Ok. I will remove all these linux specific bindings and replace with a more generic ones. Bindings will be defined for all the linux defined host capabilities. Non-linux platforms can add additional bindings as required. A function to parse such properties from a controller device node could actually be shared among all the mmc/sd host controller drivers in linux. I will redo this patch and submit again. Thanks Olof for your review and comments. Regards, Thomas. -Olof -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/6] mmc: Add OF bindings support for mmc host controller capabilities
On Mon, Nov 07, 2011 at 07:51:26PM +0530, Thomas Abraham wrote: On 5 November 2011 01:27, Olof Johansson o...@lixom.net wrote: On Thu, Nov 03, 2011 at 02:06:02AM +0530, Thomas Abraham wrote: Device nodes representing sd/mmc controllers in a device tree would include mmc host controller capabilities. Add support for parsing of mmc host controller capabilities included in device nodes. Signed-off-by: Thomas Abraham thomas.abra...@linaro.org --- .../devicetree/bindings/mmc/linux-mmc-host.txt | 13 drivers/mmc/core/host.c | 31 include/linux/mmc/host.h | 1 + 3 files changed, 45 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/mmc/linux-mmc-host.txt diff --git a/Documentation/devicetree/bindings/mmc/linux-mmc-host.txt b/Documentation/devicetree/bindings/mmc/linux-mmc-host.txt new file mode 100644 index 000..714b2b1 --- /dev/null +++ b/Documentation/devicetree/bindings/mmc/linux-mmc-host.txt @@ -0,0 +1,13 @@ +* Linux MMC Host Controller Capabilities + +The following bindings can be used in a device node to specify any board +specific mmc host controller capabilities. + +- linux,mmc_cap_4_bit_data - Host can do 4-bit transfers +- linux,mmc_cap_mmc_highspeed - Host can do MMC high-speed timing +- linux,mmc_cap_sd_highspeed - Host can do SD high-speed timing +- linux,mmc_cap_needs_poll - Host needs polling for card detection +- linux,mmc_cap_8_bit_data - Host can do 8-bit transfer +- linux,mmc_cap_disable - Host can be disabled and re-enabled to save power +- linux,mmc_cap_nonremovable - Host is connected to nonremovable media +- linux,mmc_cap_erase - Host allows erase/trim commands linux-prefixed properties are a big red flag. The device tree should describe the hardware, not what linux does with the hardware. The vendor-prefixes documentation for device tree bindings includes 'linux' as an option. And I was trying to encode the linux specific host controller capabilities using the above bindings. See previous comments about support-8bit for encoding exactly the same hardware capability in a linux-agnostic way. What the sdhci driver chooses to do with it is up to the driver, and in some cases it means it will set the capabilities flag. Same goes for the other properties. It should not go in as it is implemented in this patch, it needs to be fixed up. Ok. I will remove all these linux specific bindings and replace with a more generic ones. Bindings will be defined for all the linux defined host capabilities. Non-linux platforms can add additional bindings as required. A function to parse such properties from a controller device node could actually be shared among all the mmc/sd host controller drivers in linux. I will redo this patch and submit again. Thanks Olof for your review and comments. This patch appears to be conceptually wrong. Many of these properties are merely static capabilities of each individual device which the driver should already have knowledge of, and be setting the capability bits appropriately on its own. So, you /don't/ want to simply create a 1:1 list between the current linux capability bits (which are subject to change) and the device tree properties (which ideally must not be changed after they are defined). What you need to do is figure out which properties are required to describe the hardware about things that the driver wouldn't already know. For example, 'polling' is something the driver would already know because it is an aspect of the specific device, but 'nonremovable' is a physical characteristic of some boards. 'disable' and the speed timings are also something I would expect the driver to know about and what to set itself. So, only define properties for things that the device-specific driver cannot know for itself; or are the sort of parameters that a multi-device driver is already using for differentiation (ie. appears in pdata instead of directly set by the driver). You should be able to justify the need for each property that gets defined. g. -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/6] mmc: Add OF bindings support for mmc host controller capabilities
Device nodes representing sd/mmc controllers in a device tree would include
mmc host controller capabilities. Add support for parsing of mmc host
controller capabilities included in device nodes.
Signed-off-by: Thomas Abraham thomas.abra...@linaro.org
---
.../devicetree/bindings/mmc/linux-mmc-host.txt | 13
drivers/mmc/core/host.c| 31
include/linux/mmc/host.h |1 +
3 files changed, 45 insertions(+), 0 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mmc/linux-mmc-host.txt
diff --git a/Documentation/devicetree/bindings/mmc/linux-mmc-host.txt
b/Documentation/devicetree/bindings/mmc/linux-mmc-host.txt
new file mode 100644
index 000..714b2b1
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/linux-mmc-host.txt
@@ -0,0 +1,13 @@
+* Linux MMC Host Controller Capabilities
+
+The following bindings can be used in a device node to specify any board
+specific mmc host controller capabilities.
+
+- linux,mmc_cap_4_bit_data - Host can do 4-bit transfers
+- linux,mmc_cap_mmc_highspeed - Host can do MMC high-speed timing
+- linux,mmc_cap_sd_highspeed - Host can do SD high-speed timing
+- linux,mmc_cap_needs_poll - Host needs polling for card detection
+- linux,mmc_cap_8_bit_data - Host can do 8-bit transfer
+- linux,mmc_cap_disable - Host can be disabled and re-enabled to save power
+- linux,mmc_cap_nonremovable - Host is connected to nonremovable media
+- linux,mmc_cap_erase - Host allows erase/trim commands
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index ca2e4f5..aabf440 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -19,6 +19,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
@@ -396,3 +397,33 @@ void mmc_free_host(struct mmc_host *host)
}
EXPORT_SYMBOL(mmc_free_host);
+
+#ifdef CONFIG_OF
+/**
+ * mmc_of_parse_host_caps - parse mmc host capabilities from device node
+ * @np: pointer to device node in device tree
+ * @caps: pointer to host caps value to be returned
+ *
+ * Search the device node in device tree for mmc host capabilities.
+ */
+void mmc_of_parse_host_caps(struct device_node *np, unsigned long *caps)
+{
+ if (of_find_property(np, linux,mmc_cap_4_bit_data, NULL))
+ *caps |= MMC_CAP_4_BIT_DATA;
+ if (of_find_property(np, linux,mmc_cap_mmc_highspeed, NULL))
+ *caps |= MMC_CAP_MMC_HIGHSPEED;
+ if (of_find_property(np, linux,mmc_cap_sd_highspeed, NULL))
+ *caps |= MMC_CAP_SD_HIGHSPEED;
+ if (of_find_property(np, linux,mmc_cap_needs_poll, NULL))
+ *caps |= MMC_CAP_NEEDS_POLL;
+ if (of_find_property(np, linux,mmc_cap_8_bit_data, NULL))
+ *caps |= MMC_CAP_8_BIT_DATA;
+ if (of_find_property(np, linux,mmc_cap_disable, NULL))
+ *caps |= MMC_CAP_DISABLE;
+ if (of_find_property(np, linux,mmc_cap_nonremovable, NULL))
+ *caps |= MMC_CAP_NONREMOVABLE;
+ if (of_find_property(np, linux,mmc_cap_erase, NULL))
+ *caps |= MMC_CAP_ERASE;
+}
+EXPORT_SYMBOL(mmc_of_parse_host_caps);
+#endif /* CONFIG_OF */
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index a3ac9c4..c81c6e8 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -330,6 +330,7 @@ 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_parse_host_caps(struct device_node *np, unsigned long
*caps);
static inline void *mmc_priv(struct mmc_host *host)
{
--
1.6.6.rc2
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
