| Message ID | 1386901645-28895-4-git-send-email-ynvich@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Friday 13 December 2013, Sergei Ianovich wrote: > Non-dts implementation supply required DMA channel numbers as > IORESOURCE_DMA. However, there is was no way to get them from > device tree. Please update this changeset comment, I think it still refers to the earlier version. > Signed-off-by: Sergei Ianovich <ynvich@gmail.com> > CC: Daniel Mack <zonque@gmail.com> > CC: Arnd Bergmann <arnd@arndb.de> The patch looks ok in case we are merging your patches for 3.14 and Daniel's patches later than that. If they end up in the same merge window however, we'd have to be care to resolve the obvious conflict in a proper way. 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
On Sat, 2013-12-14 at 20:06 +0100, Arnd Bergmann wrote: > On Friday 13 December 2013, Sergei Ianovich wrote: > > Non-dts implementation supply required DMA channel numbers as > > IORESOURCE_DMA. However, there is was no way to get them from > > device tree. > > Please update this changeset comment, I think it still refers to > the earlier version. --- Non-dts implementation supply required DMA channel numbers as IORESOURCE_DMA. We can also get them from the device tree, if it is present. --- Is this fine? Should I post v3, if so? > > Signed-off-by: Sergei Ianovich <ynvich@gmail.com> > > CC: Daniel Mack <zonque@gmail.com> > > CC: Arnd Bergmann <arnd@arndb.de> > > The patch looks ok in case we are merging your patches for 3.14 > and Daniel's patches later than that. If they end up in the > same merge window however, we'd have to be care to resolve > the obvious conflict in a proper way. The most recently published Daniel's patch (Aug 2013) wraps IORESOURCE_DMA handling on DT presence in a similar way, but doesn't do any actual DT processing. So there is no conflict at the moment. Daniel can comment better on this, though. -- 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
On Saturday 14 December 2013, Sergei Ianovich wrote: > On Sat, 2013-12-14 at 20:06 +0100, Arnd Bergmann wrote: > > On Friday 13 December 2013, Sergei Ianovich wrote: > > > Non-dts implementation supply required DMA channel numbers as > > > IORESOURCE_DMA. However, there is was no way to get them from > > > device tree. > > > > Please update this changeset comment, I think it still refers to > > the earlier version. > > --- > Non-dts implementation supply required DMA channel numbers as > IORESOURCE_DMA. We can also get them from the device tree, if it > is present. > --- > > Is this fine? I would mention that the binding is designed to work with the dmaengine framework as soon as we have figured out how to make that work without the problems you found. > Should I post v3, if so? I'd wait a little longer for more comments. 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
On Mon, 2013-12-16 at 10:58 +0100, Daniel Mack wrote: > On 12/14/2013 08:34 PM, Sergei Ianovich wrote: > > On Sat, 2013-12-14 at 20:06 +0100, Arnd Bergmann wrote: > > >> The patch looks ok in case we are merging your patches for 3.14 > >> and Daniel's patches later than that. If they end up in the > >> same merge window however, we'd have to be care to resolve > >> the obvious conflict in a proper way. > > > > The most recently published Daniel's patch (Aug 2013) wraps > > IORESOURCE_DMA handling on DT presence in a similar way, > > Erm, no it doesn't. My patch uses dma_request_slave_channel_compat() in > DT case, and that works fine with the current version of pdma, and > there's no need to read the "dmas" properties directly. > > If you want to provide a way to simply denote the dma channel numbers, > without looking at the actual phandle, then yes, we could merge this > patch first, but it would be effectively reverted a proper implementation. Daniel is right. His patch doesn't need to read "dmas" directly. So my patch won't need to change drivers/mmc/host/pxamci.c at all, if it is applied after Daniel's one. What are we going to do in this context? -- 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
On 12/16/2013 12:47 PM, Sergei Ianovich wrote: > On Mon, 2013-12-16 at 10:58 +0100, Daniel Mack wrote: >> On 12/14/2013 08:34 PM, Sergei Ianovich wrote: >>> On Sat, 2013-12-14 at 20:06 +0100, Arnd Bergmann wrote: >> >>>> The patch looks ok in case we are merging your patches for 3.14 >>>> and Daniel's patches later than that. If they end up in the >>>> same merge window however, we'd have to be care to resolve >>>> the obvious conflict in a proper way. >>> >>> The most recently published Daniel's patch (Aug 2013) wraps >>> IORESOURCE_DMA handling on DT presence in a similar way, >> >> Erm, no it doesn't. My patch uses dma_request_slave_channel_compat() in >> DT case, and that works fine with the current version of pdma, and >> there's no need to read the "dmas" properties directly. >> >> If you want to provide a way to simply denote the dma channel numbers, >> without looking at the actual phandle, then yes, we could merge this >> patch first, but it would be effectively reverted a proper implementation. > > Daniel is right. His patch doesn't need to read "dmas" directly. So my > patch won't need to change drivers/mmc/host/pxamci.c at all, if it is > applied after Daniel's one. > Btw. any driver that parses the dmas property manually should be considered broken. This is a classical layering violation. The layout of the dma specifier is DMA controller specific and should be completely transparent to the device driver. - Lars -- 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
On Mon, 2013-12-16 at 12:58 +0100, Lars-Peter Clausen wrote: > Btw. any driver that parses the dmas property manually should be considered > broken. This is a classical layering violation. The layout of the dma > specifier is DMA controller specific and should be completely transparent to > the device driver. PXA as a whole is broken in this respect at the moment. The problem is how to move to a "fixed" state without breaking things which work now. -- 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/Documentation/devicetree/bindings/mmc/pxa-mmc.txt b/Documentation/devicetree/bindings/mmc/pxa-mmc.txt index b7025de..9f54c69 100644 --- a/Documentation/devicetree/bindings/mmc/pxa-mmc.txt +++ b/Documentation/devicetree/bindings/mmc/pxa-mmc.txt @@ -5,6 +5,8 @@ Driver bindings for the PXA MCI (MMC/SDIO) interfaces Required properties: - compatible: Should be "marvell,pxa-mmc". - vmmc-supply: A regulator for VMMC +- dmas: Should be DMA specifiers for RX and TX +- dma-names: Should be "rx" and "tx" Optional properties: - marvell,detect-delay-ms: sets the detection delay timeout in ms. @@ -21,5 +23,8 @@ mmc0: mmc@41100000 { interrupts = <23>; cd-gpios = <&gpio 23 0>; wp-gpios = <&gpio 24 0>; + dmas = <&dma 21 + &dma 22>; + dma-names = "rx", "tx"; }; diff --git a/arch/arm/boot/dts/pxa27x.dtsi b/arch/arm/boot/dts/pxa27x.dtsi index 44df554..c2a90c5 100644 --- a/arch/arm/boot/dts/pxa27x.dtsi +++ b/arch/arm/boot/dts/pxa27x.dtsi @@ -11,10 +11,24 @@ marvell,intc-nr-irqs = <34>; }; + dma: dma-controller@40000000 { + compatible = "marvell,pdma-1.0"; + reg = <0x40000000 0x10000>; + interrupts = <25>; + #dma-cells = <1>; + dma-channels = <32>; + }; + gpio: gpio@40e00000 { compatible = "intel,pxa27x-gpio"; interrupts = <8>, <9>, <10>; interrupt-names = "gpio0", "gpio1", "gpio_mux"; }; + + mmc@41100000 { + dmas = <&dma 21 + &dma 22>; + dma-names = "rx", "tx"; + }; }; }; diff --git a/drivers/mmc/host/pxamci.c b/drivers/mmc/host/pxamci.c index 32fe113..6b67764 100644 --- a/drivers/mmc/host/pxamci.c +++ b/drivers/mmc/host/pxamci.c @@ -613,11 +613,46 @@ static int pxamci_of_init(struct platform_device *pdev) return 0; } + +static int pxamci_of_init_dma(struct platform_device *pdev, + struct pxamci_host *host) +{ + struct device_node *np = pdev->dev.of_node; + u32 tmp; + int i; + int ret; + + i = of_property_match_string(np, "dma-names", "rx"); + if (i < 0) + return i; + + ret = of_property_read_u32_index(np, "dmas", 2 * i + 1, &tmp); + if (ret < 0) + return ret; + host->dma_drcmrrx = tmp; + + i = of_property_match_string(np, "dma-names", "tx"); + if (i < 0) + return i; + + ret = of_property_read_u32_index(np, "dmas", 2 * i + 1, &tmp); + if (ret < 0) + return ret; + host->dma_drcmrtx = tmp; + + return 0; +} #else static int pxamci_of_init(struct platform_device *pdev) { return 0; } + +static int pxamci_of_init_dma(struct platform_device *pdev, + struct pxamci_host *host) +{ + return -ENODATA; +} #endif static int pxamci_probe(struct platform_device *pdev) @@ -741,19 +776,21 @@ static int pxamci_probe(struct platform_device *pdev) platform_set_drvdata(pdev, mmc); - dmarx = platform_get_resource(pdev, IORESOURCE_DMA, 0); - if (!dmarx) { - ret = -ENXIO; - goto out; - } - host->dma_drcmrrx = dmarx->start; + if (pxamci_of_init_dma(pdev, host) < 0) { + dmarx = platform_get_resource(pdev, IORESOURCE_DMA, 0); + if (!dmarx) { + ret = -ENXIO; + goto out; + } + host->dma_drcmrrx = dmarx->start; - dmatx = platform_get_resource(pdev, IORESOURCE_DMA, 1); - if (!dmatx) { - ret = -ENXIO; - goto out; + dmatx = platform_get_resource(pdev, IORESOURCE_DMA, 1); + if (!dmatx) { + ret = -ENXIO; + goto out; + } + host->dma_drcmrtx = dmatx->start; } - host->dma_drcmrtx = dmatx->start; if (host->pdata) { gpio_cd = host->pdata->gpio_card_detect;
Non-dts implementation supply required DMA channel numbers as IORESOURCE_DMA. However, there is was no way to get them from device tree. Signed-off-by: Sergei Ianovich <ynvich@gmail.com> CC: Daniel Mack <zonque@gmail.com> CC: Arnd Bergmann <arnd@arndb.de> --- v1..v2 * add binding for next-gen dma controller * use correct dma declararion * number changed from 5 to 3 Documentation/devicetree/bindings/mmc/pxa-mmc.txt | 5 ++ arch/arm/boot/dts/pxa27x.dtsi | 14 ++++++ drivers/mmc/host/pxamci.c | 59 ++++++++++++++++++----- 3 files changed, 67 insertions(+), 11 deletions(-)