| Message ID | 1361978748-25281-5-git-send-email-shawn.guo@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wednesday 27 February 2013, Shawn Guo wrote: > Required properties: > - compatible: Should be "fsl,<chip>-mmc". The supported chips include > imx23 and imx28. > -- interrupts: Should contain ERROR and DMA interrupts > -- fsl,ssp-dma-channel: APBH DMA channel for the SSP > +- interrupts: Should contain ERROR interrupt number > +- dmas: DMA specifier, consisting of a phandle to DMA controller node > + and SSP DMA channel ID. > + Refer to dma.txt and fsl-mxs-dma.txt for details. I wonder if we should leave support for old device trees files around, at least for a while. Your patch removes a lot of unnecessary code if we decide not to worry about backwards compatibility here, but I could imagine that we see a few surprises here. > +- dma-names: Must be "ssp". I would prefer calling this "data" rather than "ssp". The name only has local significance in the ssp device, so calling the channel "ssp" seems wrong. Arnd
On Wed, Feb 27, 2013 at 08:54:18PM +0000, Arnd Bergmann wrote: > On Wednesday 27 February 2013, Shawn Guo wrote: > > Required properties: > > - compatible: Should be "fsl,<chip>-mmc". The supported chips include > > imx23 and imx28. > > -- interrupts: Should contain ERROR and DMA interrupts > > -- fsl,ssp-dma-channel: APBH DMA channel for the SSP > > +- interrupts: Should contain ERROR interrupt number > > +- dmas: DMA specifier, consisting of a phandle to DMA controller node > > + and SSP DMA channel ID. > > + Refer to dma.txt and fsl-mxs-dma.txt for details. > > I wonder if we should leave support for old device trees files around, > at least for a while. Your patch removes a lot of unnecessary code if > we decide not to worry about backwards compatibility here, but I could > imagine that we see a few surprises here. > I'm less concerned by that at this point. Though platform mxs has been fully converted to device tree, device tree is still quite new for the platform. The incompatible device tree should be acceptable, considering we have noticed that when adding the temporary DMA binding for client devices. /* * TODO: This is a temporary solution and should be changed * to use generic DMA binding later when the helper get in. */ Unless we have these TODO remarks leave in the tree forever, we will break old device tree sooner or later anyway. And I would have it happen sooner than later. And I would start thinking about maintaining the device tree compatibility for mxs after generic DMA binding adoption. > > +- dma-names: Must be "ssp". > > I would prefer calling this "data" rather than "ssp". The name only > has local significance in the ssp device, so calling the channel > "ssp" seems wrong. > Ok, will rename it per the use of the channel in the controller. Shawn
On Thursday 28 February 2013, Shawn Guo wrote: > On Wed, Feb 27, 2013 at 08:54:18PM +0000, Arnd Bergmann wrote: > > On Wednesday 27 February 2013, Shawn Guo wrote: > > > > I wonder if we should leave support for old device trees files around, > > at least for a while. Your patch removes a lot of unnecessary code if > > we decide not to worry about backwards compatibility here, but I could > > imagine that we see a few surprises here. > > > I'm less concerned by that at this point. Though platform mxs has been > fully converted to device tree, device tree is still quite new for the > platform. The incompatible device tree should be acceptable, > considering we have noticed that when adding the temporary DMA binding > for client devices. > > /* > * TODO: This is a temporary solution and should be changed > * to use generic DMA binding later when the helper get in. > */ > > Unless we have these TODO remarks leave in the tree forever, we will > break old device tree sooner or later anyway. And I would have it > happen sooner than later. > > And I would start thinking about maintaining the device tree > compatibility for mxs after generic DMA binding adoption. Right. It's your decision anyway, and removing the code as early as possible has some advantages as well. Arnd
diff --git a/Documentation/devicetree/bindings/mmc/mxs-mmc.txt b/Documentation/devicetree/bindings/mmc/mxs-mmc.txt index 54949f6..9f55d85 100644 --- a/Documentation/devicetree/bindings/mmc/mxs-mmc.txt +++ b/Documentation/devicetree/bindings/mmc/mxs-mmc.txt @@ -9,15 +9,19 @@ and the properties used by the mxsmmc driver. Required properties: - compatible: Should be "fsl,<chip>-mmc". The supported chips include imx23 and imx28. -- interrupts: Should contain ERROR and DMA interrupts -- fsl,ssp-dma-channel: APBH DMA channel for the SSP +- interrupts: Should contain ERROR interrupt number +- dmas: DMA specifier, consisting of a phandle to DMA controller node + and SSP DMA channel ID. + Refer to dma.txt and fsl-mxs-dma.txt for details. +- dma-names: Must be "ssp". Examples: ssp0: ssp@80010000 { compatible = "fsl,imx28-mmc"; reg = <0x80010000 2000>; - interrupts = <96 82>; - fsl,ssp-dma-channel = <0>; + interrupts = <96>; + dmas = <&dma_apbh 0>; + dma-names = "ssp"; bus-width = <8>; }; diff --git a/drivers/mmc/host/mxs-mmc.c b/drivers/mmc/host/mxs-mmc.c index 4efe302..83fe0ad 100644 --- a/drivers/mmc/host/mxs-mmc.c +++ b/drivers/mmc/host/mxs-mmc.c @@ -548,22 +548,6 @@ static const struct mmc_host_ops mxs_mmc_ops = { .enable_sdio_irq = mxs_mmc_enable_sdio_irq, }; -static bool mxs_mmc_dma_filter(struct dma_chan *chan, void *param) -{ - struct mxs_mmc_host *host = param; - struct mxs_ssp *ssp = &host->ssp; - - if (!mxs_dma_is_apbh(chan)) - return false; - - if (chan->chan_id != ssp->dma_channel) - return false; - - chan->private = &ssp->dma_data; - - return true; -} - static struct platform_device_id mxs_ssp_ids[] = { { .name = "imx23-mmc", @@ -591,20 +575,17 @@ static int mxs_mmc_probe(struct platform_device *pdev) struct device_node *np = pdev->dev.of_node; struct mxs_mmc_host *host; struct mmc_host *mmc; - struct resource *iores, *dmares; + struct resource *iores; struct pinctrl *pinctrl; - int ret = 0, irq_err, irq_dma; - dma_cap_mask_t mask; + int ret = 0, irq_err; struct regulator *reg_vmmc; enum of_gpio_flags flags; struct mxs_ssp *ssp; u32 bus_width = 0; iores = platform_get_resource(pdev, IORESOURCE_MEM, 0); - dmares = platform_get_resource(pdev, IORESOURCE_DMA, 0); irq_err = platform_get_irq(pdev, 0); - irq_dma = platform_get_irq(pdev, 1); - if (!iores || irq_err < 0 || irq_dma < 0) + if (!iores || irq_err < 0) return -EINVAL; mmc = mmc_alloc_host(sizeof(struct mxs_mmc_host), &pdev->dev); @@ -620,23 +601,7 @@ static int mxs_mmc_probe(struct platform_device *pdev) goto out_mmc_free; } - if (np) { - ssp->devid = (enum mxs_ssp_id) of_id->data; - /* - * TODO: This is a temporary solution and should be changed - * to use generic DMA binding later when the helpers get in. - */ - ret = of_property_read_u32(np, "fsl,ssp-dma-channel", - &ssp->dma_channel); - if (ret) { - dev_err(mmc_dev(host->mmc), - "failed to get dma channel\n"); - goto out_mmc_free; - } - } else { - ssp->devid = pdev->id_entry->driver_data; - ssp->dma_channel = dmares->start; - } + ssp->devid = (enum mxs_ssp_id) of_id->data; host->mmc = mmc; host->sdio_irq_en = 0; @@ -666,10 +631,7 @@ static int mxs_mmc_probe(struct platform_device *pdev) mxs_mmc_reset(host); - dma_cap_zero(mask); - dma_cap_set(DMA_SLAVE, mask); - ssp->dma_data.chan_irq = irq_dma; - ssp->dmach = dma_request_channel(mask, mxs_mmc_dma_filter, host); + ssp->dmach = dma_request_slave_channel(&pdev->dev, "ssp"); if (!ssp->dmach) { dev_err(mmc_dev(host->mmc), "%s: failed to request dma\n", __func__);
With the generic DMA device tree helper supported by mxs-dma driver, client devices only need to call dma_request_slave_channel() for requesting a DMA channel from dmaengine. Since mxs is a DT only platform now, along with the changes, the non-DT case checking in probe function also gets cleaned up. Signed-off-by: Shawn Guo <shawn.guo@linaro.org> Cc: Chris Ball <cjb@laptop.org> Cc: linux-mmc@vger.kernel.org --- Documentation/devicetree/bindings/mmc/mxs-mmc.txt | 12 ++++-- drivers/mmc/host/mxs-mmc.c | 48 +++------------------ 2 files changed, 13 insertions(+), 47 deletions(-)