mbox series

[RFC,0/3] Add support for using external dma in SDHCI

Message ID 1541387810-24867-1-git-send-email-zhang.chunyan@linaro.org
Headers show
Series Add support for using external dma in SDHCI | expand

Message

Chunyan Zhang Nov. 5, 2018, 3:16 a.m. UTC 
Currently the generic SDHCI code in the Linux kernel supports the SD
standard DMA integrated into the host controller but does not have any
support for external DMA controllers implemented using dmaengine meaning
that custom code is needed for any systems that use a generic DMA
controller with SDHCI which in practice means any SDHCI controller that
doesn't have an integrated DMA controller so we should have this as a
generic feature.

There are already a number of controller specific drivers that have dmaengine
code, and some could use sdhci.c actually, but needed to implement mmc_ops->request()
in their specific driver for sending command with external dma using dmaengine
framework, with this patchset, them will take advantage of the generic support.
TI's omap controller is the case as an example.

Any comments are very appreciated.

Thanks,
Chunyan

Chunyan Zhang (3):
  mmc: sdhci: add support for using external DMA devices
  mmc: sdhci-omap: Add using external dma
  dt-bindings: sdhci-omap: Add example for using dmaengine

 .../devicetree/bindings/mmc/sdhci-omap.txt         |   7 ++
 drivers/mmc/host/Kconfig                           |  13 ++
 drivers/mmc/host/sdhci-omap.c                      |   7 ++
 drivers/mmc/host/sdhci.c                           | 137 ++++++++++++++++++++-
 drivers/mmc/host/sdhci.h                           |  12 ++
 5 files changed, 175 insertions(+), 1 deletion(-)

-- 
2.7.4

Comments

Arnd Bergmann Nov. 6, 2018, 12:51 p.m. UTC | #1 
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.

       Arnd
Ulf Hansson Nov. 19, 2018, 4:32 p.m. UTC | #2 
On 9 November 2018 at 06:27, Kishon Vijay Abraham I <kishon@ti.com> wrote:
> 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.


I was under the impression that the internal DMA (for old versions) is
rather poor.

However, in the end this is a software policy decision of what to use.
I have no strong opinion, so feel free to pick what you think makes
best sense.

Maybe Adrian also have some thoughts?

Kind regards
Uffe