| Message ID | 1363779852-24083-4-git-send-email-prakash.pm@ti.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
On Wednesday 20 March 2013, Manjunathappa, Prakash wrote: > Adds device tree support for davinci_mmc. Also add binding documentation. > As of now in non-dma PIO mode and without GPIO card_detect/write_protect > option because of dependencies on EDMA and GPIO module DT support. Shouldn't the binding at least specify the names for the DMA channels? It's fine if the driver works without those, but I think there is no strict dependency here: If dma_request_slave_channel does not find a channel, the driver can just continue in PIO mode, and as soon as the EDMA driver is merged, it will work with DMA. Arnd
Hi Arnd, On Wed, Mar 20, 2013 at 18:56:09, Arnd Bergmann wrote: > On Wednesday 20 March 2013, Manjunathappa, Prakash wrote: > > Adds device tree support for davinci_mmc. Also add binding documentation. > > As of now in non-dma PIO mode and without GPIO card_detect/write_protect > > option because of dependencies on EDMA and GPIO module DT support. > > Shouldn't the binding at least specify the names for the DMA channels? > It's fine if the driver works without those, but I think there is no > strict dependency here: If dma_request_slave_channel does not find > a channel, the driver can just continue in PIO mode, and as soon > as the EDMA driver is merged, it will work with DMA. > In that case, don't I have to add support for it in driver also using dma_request_slave_channel_compat? As we do not have edma support yet on da850, I will not be able to test it. Thanks, Prakash
On Wednesday 27 March 2013, Manjunathappa, Prakash wrote: > On Wed, Mar 20, 2013 at 18:56:09, Arnd Bergmann wrote: > > On Wednesday 20 March 2013, Manjunathappa, Prakash wrote: > > > Adds device tree support for davinci_mmc. Also add binding documentation. > > > As of now in non-dma PIO mode and without GPIO card_detect/write_protect > > > option because of dependencies on EDMA and GPIO module DT support. > > > > Shouldn't the binding at least specify the names for the DMA channels? > > It's fine if the driver works without those, but I think there is no > > strict dependency here: If dma_request_slave_channel does not find > > a channel, the driver can just continue in PIO mode, and as soon > > as the EDMA driver is merged, it will work with DMA. > > In that case, don't I have to add support for it in driver also using > dma_request_slave_channel_compat? As we do not have edma support yet on > da850, I will not be able to test it. The code is independent of the binding in this case. If you already know that the hardware supports DMA channels and you will add support in the code later, I would recommend writing up the binding the way it should be used. Arnd
On Wed, Mar 27, 2013 at 16:13:51, Arnd Bergmann wrote: > On Wednesday 27 March 2013, Manjunathappa, Prakash wrote: > > On Wed, Mar 20, 2013 at 18:56:09, Arnd Bergmann wrote: > > > On Wednesday 20 March 2013, Manjunathappa, Prakash wrote: > > > > Adds device tree support for davinci_mmc. Also add binding documentation. > > > > As of now in non-dma PIO mode and without GPIO card_detect/write_protect > > > > option because of dependencies on EDMA and GPIO module DT support. > > > > > > Shouldn't the binding at least specify the names for the DMA channels? > > > It's fine if the driver works without those, but I think there is no > > > strict dependency here: If dma_request_slave_channel does not find > > > a channel, the driver can just continue in PIO mode, and as soon > > > as the EDMA driver is merged, it will work with DMA. > > > > In that case, don't I have to add support for it in driver also using > > dma_request_slave_channel_compat? As we do not have edma support yet on > > da850, I will not be able to test it. > > The code is independent of the binding in this case. If you already know > that the hardware supports DMA channels and you will add support in the > code later, I would recommend writing up the binding the way it should > be used. > Ok I will add support for DMA bindings with a note saying "not tested dma capability via DT". Thanks, Prakash
diff --git a/Documentation/devicetree/bindings/mmc/davinci_mmc.txt b/Documentation/devicetree/bindings/mmc/davinci_mmc.txt new file mode 100644 index 0000000..c4413f7 --- /dev/null +++ b/Documentation/devicetree/bindings/mmc/davinci_mmc.txt @@ -0,0 +1,25 @@ +* TI Highspeed MMC host controller for DaVinci + +The Highspeed MMC Host Controller on TI DaVinci family +provides an interface for MMC, SD and SDIO types of memory cards. + +This file documents the properties used by the davinci_mmc driver. + +Required properties: +- compatible: + Should be "ti,da830-mmc": for da830, da850, dm365 + Should be "ti,dm355-mmc": for dm355, dm644x + +Optional properties: +- bus-width: Number of data lines, can be <1>, <4>, or <8>, default <1> +- max-frequency: Maximum operating clock frequency, default 25MHz. + +Example: + mmc0: mmc@1c40000 { + compatible = "ti,da830-mmc", + reg = <0x40000 0x1000>; + interrupts = <16>; + status = "okay"; + bus-width = <4>; + max-frequency = <50000000>; + }; diff --git a/drivers/mmc/host/davinci_mmc.c b/drivers/mmc/host/davinci_mmc.c index 7d0e684..8aada43 100644 --- a/drivers/mmc/host/davinci_mmc.c +++ b/drivers/mmc/host/davinci_mmc.c @@ -34,6 +34,8 @@ #include <linux/dma-mapping.h> #include <linux/edma.h> #include <linux/mmc/mmc.h> +#include <linux/of.h> +#include <linux/of_device.h> #include <linux/platform_data/edma.h> #include <linux/platform_data/mmc-davinci.h> @@ -1170,9 +1172,62 @@ static struct platform_device_id davinci_mmc_devtype[] = { }; MODULE_DEVICE_TABLE(platform, davinci_mmc_devtype); -static int __init davinci_mmcsd_probe(struct platform_device *pdev) +static const struct of_device_id davinci_mmc_dt_ids[] = { + { + .compatible = "ti,dm6441-mmc", + .data = &davinci_mmc_devtype[MMC_CTLR_VERSION_1], + }, + { + .compatible = "ti,da830-mmc", + .data = &davinci_mmc_devtype[MMC_CTLR_VERSION_2], + }, + {}, +}; +MODULE_DEVICE_TABLE(of, davinci_mmc_dt_ids); + +static struct davinci_mmc_config + *mmc_parse_pdata(struct platform_device *pdev) { + struct device_node *np; struct davinci_mmc_config *pdata = pdev->dev.platform_data; + const struct of_device_id *match = + of_match_device(of_match_ptr(davinci_mmc_dt_ids), &pdev->dev); + u32 data; + + np = pdev->dev.of_node; + if (!np) + return pdata; + + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL); + if (!pdata) { + dev_err(&pdev->dev, "Failed to allocate memory for struct davinci_mmc_config\n"); + goto nodata; + } + + if (match) + pdev->id_entry = match->data; + + if (of_property_read_u32(np, "max-frequency", &pdata->max_freq)) + dev_info(&pdev->dev, "'max-frequency' property not specified, defaulting to 25MHz\n"); + + of_property_read_u32(np, "bus-width", &data); + switch (data) { + case 1: + case 4: + case 8: + pdata->wires = data; + break; + default: + pdata->wires = 1; + dev_info(&pdev->dev, "Unsupported buswidth, defaulting to 1 bit\n"); + } +nodata: + return pdata; +} + +static int __init davinci_mmcsd_probe(struct platform_device *pdev) +{ + struct davinci_mmc_config *pdata = NULL; struct mmc_davinci_host *host = NULL; struct mmc_host *mmc = NULL; struct resource *r, *mem = NULL; @@ -1180,7 +1235,11 @@ static int __init davinci_mmcsd_probe(struct platform_device *pdev) size_t mem_size; const struct platform_device_id *id_entry; - /* REVISIT: when we're fully converted, fail if pdata is NULL */ + pdata = mmc_parse_pdata(pdev); + if (pdata == NULL) { + dev_err(&pdev->dev, "Couldn't get platform data\n"); + return -ENOENT; + } ret = -ENODEV; r = platform_get_resource(pdev, IORESOURCE_MEM, 0); @@ -1424,6 +1483,7 @@ static struct platform_driver davinci_mmcsd_driver = { .name = "davinci_mmc", .owner = THIS_MODULE, .pm = davinci_mmcsd_pm_ops, + .of_match_table = of_match_ptr(davinci_mmc_dt_ids), }, .remove = __exit_p(davinci_mmcsd_remove), .id_table = davinci_mmc_devtype,