| Message ID | 1541387810-24867-3-git-send-email-zhang.chunyan@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [RFC,1/3] mmc: sdhci: add support for using external DMA devices | expand |
Hi, On 05/11/18 8:46 AM, Chunyan Zhang wrote: > sdhci-omap can support both external dma controllers via dmaengine > framework as well as ADMA in which the controller acts as DMA master. > > Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org> > --- > drivers/mmc/host/sdhci-omap.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c > index 88347ce..0a8162c 100644 > --- a/drivers/mmc/host/sdhci-omap.c > +++ b/drivers/mmc/host/sdhci-omap.c > @@ -896,6 +896,7 @@ static int sdhci_omap_probe(struct platform_device *pdev) > const struct of_device_id *match; > struct sdhci_omap_data *data; > const struct soc_device_attribute *soc; > + struct resource *regs; > > match = of_match_device(omap_sdhci_match, dev); > if (!match) > @@ -908,6 +909,10 @@ static int sdhci_omap_probe(struct platform_device *pdev) > } > offset = data->offset; > > + regs = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!regs) > + return -ENXIO; > + > host = sdhci_pltfm_init(pdev, &sdhci_omap_pdata, > sizeof(*omap_host)); > if (IS_ERR(host)) { > @@ -924,6 +929,7 @@ static int sdhci_omap_probe(struct platform_device *pdev) > omap_host->timing = MMC_TIMING_LEGACY; > omap_host->flags = data->flags; > host->ioaddr += offset; > + host->mapbase = regs->start; > > mmc = host->mmc; > sdhci_get_of_property(pdev); > @@ -991,6 +997,7 @@ static int sdhci_omap_probe(struct platform_device *pdev) > host->mmc_host_ops.execute_tuning = sdhci_omap_execute_tuning; > host->mmc_host_ops.enable_sdio_irq = sdhci_omap_enable_sdio_irq; > > + sdhci_switch_extdma(host, true); A number of devices using sdhci-omap supports ADMA. So switching to external DMA shouldn't be unconditional. IMHO sdhci.c should see if the device supports ADMA or SDMA. If not it should try switching to external DMA and if external DMA too is not supported, it should use PIO. Thanks Kishon
On Tue, 6 Nov 2018 at 20:51, Arnd Bergmann <arnd@arndb.de> wrote: > > On 11/5/18, Kishon Vijay Abraham I <kishon@ti.com> wrote: > > On 05/11/18 8:46 AM, Chunyan Zhang wrote: > >> > >> + sdhci_switch_extdma(host, true); > > > > A number of devices using sdhci-omap supports ADMA. So switching to > > external > > DMA shouldn't be unconditional. > > > > IMHO sdhci.c should see if the device supports ADMA or SDMA. If not it > > should > > try switching to external DMA and if external DMA too is not supported, it > > should use PIO. > > What's the reasoning for preferring ADMA/SDMA over external DMA if > both are supported? > > I'd expect that if the external DMA for some reason is worse than > ADMA, we just wouldn't list it in the DT at all, but if it's listed and > ADMA also works, then the driver should try to use it before falling > back to ADMA. Thanks Arnd and Kishon, let's see what Andrian and Ulf would suggest, and then I will address in next version of patchset. Thanks again, Chunyan > > Arnd
Hi Arnd, On 06/11/18 6:21 PM, Arnd Bergmann wrote: > On 11/5/18, Kishon Vijay Abraham I <kishon@ti.com> wrote: >> On 05/11/18 8:46 AM, Chunyan Zhang wrote: >>> >>> + sdhci_switch_extdma(host, true); >> >> A number of devices using sdhci-omap supports ADMA. So switching to >> external >> DMA shouldn't be unconditional. >> >> IMHO sdhci.c should see if the device supports ADMA or SDMA. If not it >> should >> try switching to external DMA and if external DMA too is not supported, it >> should use PIO. > > What's the reasoning for preferring ADMA/SDMA over external DMA if > both are supported? Generally from our experiments we've found ADMA gives better throughput than DMA. I have to ascertain the actual reasons for that. > > I'd expect that if the external DMA for some reason is worse than > ADMA, we just wouldn't list it in the DT at all, but if it's listed and > ADMA also works, then the driver should try to use it before falling > back to ADMA. I would agree that for newer dtbs. However if you consider omap_hsmmc, external DMA bindings are already added in dt. If we try to switch to sdhci-omap with the same bindings, systems with older dtbs will use external DMA and give lesser throughput. Thanks Kishon
diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c index 88347ce..0a8162c 100644 --- a/drivers/mmc/host/sdhci-omap.c +++ b/drivers/mmc/host/sdhci-omap.c @@ -896,6 +896,7 @@ static int sdhci_omap_probe(struct platform_device *pdev) const struct of_device_id *match; struct sdhci_omap_data *data; const struct soc_device_attribute *soc; + struct resource *regs; match = of_match_device(omap_sdhci_match, dev); if (!match) @@ -908,6 +909,10 @@ static int sdhci_omap_probe(struct platform_device *pdev) } offset = data->offset; + regs = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!regs) + return -ENXIO; + host = sdhci_pltfm_init(pdev, &sdhci_omap_pdata, sizeof(*omap_host)); if (IS_ERR(host)) { @@ -924,6 +929,7 @@ static int sdhci_omap_probe(struct platform_device *pdev) omap_host->timing = MMC_TIMING_LEGACY; omap_host->flags = data->flags; host->ioaddr += offset; + host->mapbase = regs->start; mmc = host->mmc; sdhci_get_of_property(pdev); @@ -991,6 +997,7 @@ static int sdhci_omap_probe(struct platform_device *pdev) host->mmc_host_ops.execute_tuning = sdhci_omap_execute_tuning; host->mmc_host_ops.enable_sdio_irq = sdhci_omap_enable_sdio_irq; + sdhci_switch_extdma(host, true); ret = sdhci_setup_host(host); if (ret) goto err_put_sync;
sdhci-omap can support both external dma controllers via dmaengine framework as well as ADMA in which the controller acts as DMA master. Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org> --- drivers/mmc/host/sdhci-omap.c | 7 +++++++ 1 file changed, 7 insertions(+) -- 2.7.4