diff mbox

[5/5] RFC spi: sun4i: add DMA support

Message ID ba0d6eb37cc4b0d2c46acbf9fcd7d644b3545ce8.1464130597.git.hramrach@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Michal Suchanek May 26, 2016, 7:25 p.m. UTC
From: Emilio López <emilio@elopez.com.ar>

This patch adds support for 64 byte or bigger transfers on the
sun4i SPI controller. Said transfers will be performed via DMA.

Signed-off-by: Emilio López <emilio@elopez.com.ar>
Signed-off-by: Michal Suchanek <hramrach@gmail.com>

---
v2:
 - fallback to previous behaviour when DMA initialization fails

   + this has the problem that when the driver happens to load before the dma
     driver it will not use dma - can be addressed with a module parameter
 + the issue with dma_async_tx_descriptor *desc_tx = NULL, *desc_rx = NULL; ...
   dma_wait_for_async_tx(desc_rx); remains unaddressed

---
 drivers/spi/spi-sun4i.c | 171 ++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 158 insertions(+), 13 deletions(-)

Comments

Mark Brown May 30, 2016, 11:26 a.m. UTC | #1
On Thu, May 26, 2016 at 07:25:25PM -0000, Michal Suchanek wrote:

>  - fallback to previous behaviour when DMA initialization fails
> 
>    + this has the problem that when the driver happens to load before the dma
>      driver it will not use dma - can be addressed with a module parameter

No, you should pay attention to the error you are getting and let probe
deferral happen if that's the error you get.
Geert Uytterhoeven May 30, 2016, 12:11 p.m. UTC | #2
Hi Mark,

On Mon, May 30, 2016 at 1:26 PM, Mark Brown <broonie@kernel.org> wrote:
> On Thu, May 26, 2016 at 07:25:25PM -0000, Michal Suchanek wrote:
>>  - fallback to previous behaviour when DMA initialization fails
>>
>>    + this has the problem that when the driver happens to load before the dma
>>      driver it will not use dma - can be addressed with a module parameter
>
> No, you should pay attention to the error you are getting and let probe
> deferral happen if that's the error you get.

Unfortunately DMA is an optional feature.

There's no way to distinguish between -EPROBE_DEFER due to the SPI master
driver being probed before the DMA engine driver, and -EPROBE_DEFER due to
support for the DMA engine not having been compiled in.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Mark Brown May 30, 2016, 3:03 p.m. UTC | #3
On Mon, May 30, 2016 at 02:11:51PM +0200, Geert Uytterhoeven wrote:
> On Mon, May 30, 2016 at 1:26 PM, Mark Brown <broonie@kernel.org> wrote:
> > On Thu, May 26, 2016 at 07:25:25PM -0000, Michal Suchanek wrote:
> >>  - fallback to previous behaviour when DMA initialization fails
> >>
> >>    + this has the problem that when the driver happens to load before the dma
> >>      driver it will not use dma - can be addressed with a module parameter

> > No, you should pay attention to the error you are getting and let probe
> > deferral happen if that's the error you get.

> Unfortunately DMA is an optional feature.

> There's no way to distinguish between -EPROBE_DEFER due to the SPI master
> driver being probed before the DMA engine driver, and -EPROBE_DEFER due to
> support for the DMA engine not having been compiled in.

I really don't think it's worth caring too much about cases where the
DMA driver hasn't been compiled in, it's not like SPI is the only thing
that's going to be using it.  I really think it's better to defer the
problem - not getting DMA (or worse, only getting DMA on some boots) is
not great and if people are optimising on that level my feeling is that
they're probably going to be OK with customizing DT to match.  Ideally
we would have something a bit nicer than deferred probe but right now
this seems the more helpful option.
Michal Suchanek May 30, 2016, 3:28 p.m. UTC | #4
Hello,

On 30 May 2016 at 17:03, Mark Brown <broonie@kernel.org> wrote:
> On Mon, May 30, 2016 at 02:11:51PM +0200, Geert Uytterhoeven wrote:
>> On Mon, May 30, 2016 at 1:26 PM, Mark Brown <broonie@kernel.org> wrote:
>> > On Thu, May 26, 2016 at 07:25:25PM -0000, Michal Suchanek wrote:
>> >>  - fallback to previous behaviour when DMA initialization fails
>> >>
>> >>    + this has the problem that when the driver happens to load before the dma
>> >>      driver it will not use dma - can be addressed with a module parameter
>
>> > No, you should pay attention to the error you are getting and let probe
>> > deferral happen if that's the error you get.
>
>> Unfortunately DMA is an optional feature.
>
>> There's no way to distinguish between -EPROBE_DEFER due to the SPI master
>> driver being probed before the DMA engine driver, and -EPROBE_DEFER due to
>> support for the DMA engine not having been compiled in.
>
> I really don't think it's worth caring too much about cases where the
> DMA driver hasn't been compiled in, it's not like SPI is the only thing
> that's going to be using it.  I really think it's better to defer the
> problem - not getting DMA (or worse, only getting DMA on some boots) is
> not great and if people are optimising on that level my feeling is that
> they're probably going to be OK with customizing DT to match.  Ideally
> we would have something a bit nicer than deferred probe but right now
> this seems the more helpful option.

It's what the driver did to start with and it was requested to fall
back to non-DMA in the case DMA is not available.

We get to the much discussed problem that it's never possible to tell
if a driver is available or not.

It's possible to add a parameter like require_dma which could be used
to load the driver without dma if unset. If it was set by default then
driver ordering is not important so long as dma driver is loaded
eventually. Also an informative print that such parameter exists when
probing the driver is deferred would be helpful. It would probably
create quite a bit of log spam, however. The driver can be deferred
several times during boot.

Thanks

Michal
Mark Brown May 30, 2016, 3:50 p.m. UTC | #5
On Mon, May 30, 2016 at 05:28:10PM +0200, Michal Suchanek wrote:
> On 30 May 2016 at 17:03, Mark Brown <broonie@kernel.org> wrote:

> > I really don't think it's worth caring too much about cases where the
> > DMA driver hasn't been compiled in, it's not like SPI is the only thing

> It's what the driver did to start with and it was requested to fall
> back to non-DMA in the case DMA is not available.

Why?  I really can't see any sensible use case for this that doesn't
have a better solution available.

> It's possible to add a parameter like require_dma which could be used
> to load the driver without dma if unset. If it was set by default then
> driver ordering is not important so long as dma driver is loaded
> eventually. Also an informative print that such parameter exists when
> probing the driver is deferred would be helpful. It would probably
> create quite a bit of log spam, however. The driver can be deferred
> several times during boot.

That seems fairly hacky, if we were going to do anything like that it
should be the other way around so that we default to trying to use
resources and even then it seems like something that should be handled
at a framework level rather than having random options in individual
drivers to ignore things.  Having things behave inconsistently between
different drivers is going to lead to a worse user experience and if
this is a good idea for one driver it seems like it'd be a good idea for
all of them.

But really
Michal Suchanek May 31, 2016, 10:44 a.m. UTC | #6
On 30 May 2016 at 17:50, Mark Brown <broonie@kernel.org> wrote:
> On Mon, May 30, 2016 at 05:28:10PM +0200, Michal Suchanek wrote:
>> On 30 May 2016 at 17:03, Mark Brown <broonie@kernel.org> wrote:
>
>> > I really don't think it's worth caring too much about cases where the
>> > DMA driver hasn't been compiled in, it's not like SPI is the only thing
>
>> It's what the driver did to start with and it was requested to fall
>> back to non-DMA in the case DMA is not available.
>
> Why?  I really can't see any sensible use case for this that doesn't
> have a better solution available.

Of course, the solution is to compile in the DMA driver.

It's been argued that some drivers which use only short transfers will
just work.

>
>> It's possible to add a parameter like require_dma which could be used
>> to load the driver without dma if unset. If it was set by default then
>> driver ordering is not important so long as dma driver is loaded
>> eventually. Also an informative print that such parameter exists when
>> probing the driver is deferred would be helpful. It would probably
>> create quite a bit of log spam, however. The driver can be deferred
>> several times during boot.
>
> That seems fairly hacky, if we were going to do anything like that it
> should be the other way around so that we default to trying to use
> resources and even then it seems like something that should be handled
> at a framework level rather than having random options in individual
> drivers to ignore things.  Having things behave inconsistently between
> different drivers is going to lead to a worse user experience and if
> this is a good idea for one driver it seems like it'd be a good idea for
> all of them.

Hacky but doable if desirable. It's awesome for testing SPI transfer
fragmentation with different drivers ;-)

The previous discussion of this driver is here
http://thread.gmane.org/gmane.linux.kernel/2162327

Thanks

Michal
Mark Brown May 31, 2016, 1:27 p.m. UTC | #7
On Tue, May 31, 2016 at 12:44:54PM +0200, Michal Suchanek wrote:
> On 30 May 2016 at 17:50, Mark Brown <broonie@kernel.org> wrote:
> > On Mon, May 30, 2016 at 05:28:10PM +0200, Michal Suchanek wrote:

> >> It's what the driver did to start with and it was requested to fall
> >> back to non-DMA in the case DMA is not available.

> > Why?  I really can't see any sensible use case for this that doesn't
> > have a better solution available.

> Of course, the solution is to compile in the DMA driver.

> It's been argued that some drivers which use only short transfers will
> just work.

With nothing else in the system that needs DMA?  It's making the
performance of the system less reliable for the benefit of a very narrow
use case.
Michal Suchanek May 31, 2016, 2:19 p.m. UTC | #8
On 31 May 2016 at 15:27, Mark Brown <broonie@kernel.org> wrote:
> On Tue, May 31, 2016 at 12:44:54PM +0200, Michal Suchanek wrote:
>> On 30 May 2016 at 17:50, Mark Brown <broonie@kernel.org> wrote:
>> > On Mon, May 30, 2016 at 05:28:10PM +0200, Michal Suchanek wrote:
>
>> >> It's what the driver did to start with and it was requested to fall
>> >> back to non-DMA in the case DMA is not available.
>
>> > Why?  I really can't see any sensible use case for this that doesn't
>> > have a better solution available.
>
>> Of course, the solution is to compile in the DMA driver.
>
>> It's been argued that some drivers which use only short transfers will
>> just work.
>
> With nothing else in the system that needs DMA?  It's making the
> performance of the system less reliable for the benefit of a very narrow
> use case.

Some of the platform devices have dedicated DMA *controller* built
into the device IP so the DMA engine really is optional on many sunxi
devices. Besides SPI you definitely need the DMA engine for audio. You
probably don't need it for storage and graphics. I don't have any idea
if it's used for USB and Ethernet.

Thanks

Michal
Maxime Ripard June 1, 2016, 6 p.m. UTC | #9
Hi,

On Mon, May 30, 2016 at 04:50:16PM +0100, Mark Brown wrote:
> On Mon, May 30, 2016 at 05:28:10PM +0200, Michal Suchanek wrote:
> > On 30 May 2016 at 17:03, Mark Brown <broonie@kernel.org> wrote:
> 
> > > I really don't think it's worth caring too much about cases where the
> > > DMA driver hasn't been compiled in, it's not like SPI is the only thing
> 
> > It's what the driver did to start with and it was requested to fall
> > back to non-DMA in the case DMA is not available.
> 
> Why?  I really can't see any sensible use case for this that doesn't
> have a better solution available.

SPI works just fine without DMA, which might just be considered an
(optional) optimisation.

We've been using it without DMA for years now, and it was working just
fine, and it will work even better with the other patches in this
serie. There's no reason to add a hard dependency on something that we
don't really need.

Maxime
Priit Laes June 2, 2016, 4:42 a.m. UTC | #10
On Wed, 2016-06-01 at 20:00 +0200, Maxime Ripard wrote:
> Hi,
> 
> On Mon, May 30, 2016 at 04:50:16PM +0100, Mark Brown wrote:
> > 
> > On Mon, May 30, 2016 at 05:28:10PM +0200, Michal Suchanek wrote:
> > > 
> > > On 30 May 2016 at 17:03, Mark Brown <broonie@kernel.org> wrote:
> > > 
> > > > 
> > > > I really don't think it's worth caring too much about cases
> > > > where the
> > > > DMA driver hasn't been compiled in, it's not like SPI is the
> > > > only thing
> > > 
> > > It's what the driver did to start with and it was requested to
> > > fall
> > > back to non-DMA in the case DMA is not available.
> > Why?  I really can't see any sensible use case for this that
> > doesn't
> > have a better solution available.
> SPI works just fine without DMA, which might just be considered an
> (optional) optimisation.
> 
> We've been using it without DMA for years now, and it was working
> just
> fine, and it will work even better with the other patches in this
> serie. There's no reason to add a hard dependency on something that
> we
> don't really need.
> 

Actually it non-DMA case works fine if you don't need SPI transfers
larger than SUN4I_FIFO_DEPTH - 1, which is 63 bytes.

This was addressed by this patch, but was never applied:
http://permalink.gmane.org/gmane.linux.kernel.spi.devel/18950
Maxime Ripard June 2, 2016, 8:18 a.m. UTC | #11
On Tue, May 31, 2016 at 04:19:28PM +0200, Michal Suchanek wrote:
> On 31 May 2016 at 15:27, Mark Brown <broonie@kernel.org> wrote:
> > On Tue, May 31, 2016 at 12:44:54PM +0200, Michal Suchanek wrote:
> >> On 30 May 2016 at 17:50, Mark Brown <broonie@kernel.org> wrote:
> >> > On Mon, May 30, 2016 at 05:28:10PM +0200, Michal Suchanek wrote:
> >
> >> >> It's what the driver did to start with and it was requested to fall
> >> >> back to non-DMA in the case DMA is not available.
> >
> >> > Why?  I really can't see any sensible use case for this that doesn't
> >> > have a better solution available.
> >
> >> Of course, the solution is to compile in the DMA driver.
> >
> >> It's been argued that some drivers which use only short transfers will
> >> just work.
> >
> > With nothing else in the system that needs DMA?  It's making the
> > performance of the system less reliable for the benefit of a very narrow
> > use case.
> 
> Some of the platform devices have dedicated DMA *controller* built
> into the device IP so the DMA engine really is optional on many sunxi
> devices. Besides SPI you definitely need the DMA engine for audio. You
> probably don't need it for storage and graphics. I don't have any idea
> if it's used for USB and Ethernet.

USB and Ethernet have their own dedicated DMA engines. So currently,
the only driver that requires it is the audio codec.

Maxime
Mark Brown June 2, 2016, 9:18 a.m. UTC | #12
On Thu, Jun 02, 2016 at 07:42:26AM +0300, Priit Laes wrote:

> This was addressed by this patch, but was never applied:
> http://permalink.gmane.org/gmane.linux.kernel.spi.devel/18950

I've never seen that patch before, if it was CCed to me there's a good
chance it'd have been deleted unread because you put ARM: at the start
of the subject line which says it's a patch for arch/arm.  You'll need
to resubmit.
Michal Suchanek June 2, 2016, 12:14 p.m. UTC | #13
On 2 June 2016 at 06:42, Priit Laes <plaes@plaes.org> wrote:
> On Wed, 2016-06-01 at 20:00 +0200, Maxime Ripard wrote:
>> Hi,
>>
>> On Mon, May 30, 2016 at 04:50:16PM +0100, Mark Brown wrote:
>> >
>> > On Mon, May 30, 2016 at 05:28:10PM +0200, Michal Suchanek wrote:
>> > >
>> > > On 30 May 2016 at 17:03, Mark Brown <broonie@kernel.org> wrote:
>> > >
>> > > >
>> > > > I really don't think it's worth caring too much about cases
>> > > > where the
>> > > > DMA driver hasn't been compiled in, it's not like SPI is the
>> > > > only thing
>> > >
>> > > It's what the driver did to start with and it was requested to
>> > > fall
>> > > back to non-DMA in the case DMA is not available.
>> > Why?  I really can't see any sensible use case for this that
>> > doesn't
>> > have a better solution available.
>> SPI works just fine without DMA, which might just be considered an
>> (optional) optimisation.
>>
>> We've been using it without DMA for years now, and it was working
>> just
>> fine, and it will work even better with the other patches in this
>> serie. There's no reason to add a hard dependency on something that
>> we
>> don't really need.
>>
>
> Actually it non-DMA case works fine if you don't need SPI transfers
> larger than SUN4I_FIFO_DEPTH - 1, which is 63 bytes.
>
> This was addressed by this patch, but was never applied:
> http://permalink.gmane.org/gmane.linux.kernel.spi.devel/18950

And the code added in that patch will never run unless you

1) use long spi transfers
2) compile in/load SPI without DMA support

There is no reason for doing 2) since we have do DMA support for sunxi.

So that's another code path that needs maintenance and testing and
likely will not get it.

Thanks

Michal
Mark Brown June 2, 2016, 2:26 p.m. UTC | #14
On Thu, Jun 02, 2016 at 02:14:26PM +0200, Michal Suchanek wrote:
> On 2 June 2016 at 06:42, Priit Laes <plaes@plaes.org> wrote:
> > On Wed, 2016-06-01 at 20:00 +0200, Maxime Ripard wrote:

> > Actually it non-DMA case works fine if you don't need SPI transfers
> > larger than SUN4I_FIFO_DEPTH - 1, which is 63 bytes.

> > This was addressed by this patch, but was never applied:
> > http://permalink.gmane.org/gmane.linux.kernel.spi.devel/18950

> And the code added in that patch will never run unless you

> 1) use long spi transfers
> 2) compile in/load SPI without DMA support

> There is no reason for doing 2) since we have do DMA support for sunxi.

Well, presumably such code exists and is being worked on?

> So that's another code path that needs maintenance and testing and
> likely will not get it.

Oh, come on.  You might not want to use it yourself but the chances are
that someone will want to use it just like the situation with all the
other SPI drivers.  It's a perfectly reasonable and sensible feature to
support upstream.

I really do not understand why there is such a strong desire to have
these devices be a special snowflake here, the worst that's likely to
happen here is that you're going to end up having to either remove the
DMA controller from the DT or load the driver for it neither of which
seem like the end of the world.
Michal Suchanek June 5, 2016, 11:27 a.m. UTC | #15
On 2 June 2016 at 16:26, Mark Brown <broonie@kernel.org> wrote:
> On Thu, Jun 02, 2016 at 02:14:26PM +0200, Michal Suchanek wrote:
>> On 2 June 2016 at 06:42, Priit Laes <plaes@plaes.org> wrote:
>> > On Wed, 2016-06-01 at 20:00 +0200, Maxime Ripard wrote:
>
>> > Actually it non-DMA case works fine if you don't need SPI transfers
>> > larger than SUN4I_FIFO_DEPTH - 1, which is 63 bytes.
>
>> > This was addressed by this patch, but was never applied:
>> > http://permalink.gmane.org/gmane.linux.kernel.spi.devel/18950
>
>> And the code added in that patch will never run unless you
>
>> 1) use long spi transfers
>> 2) compile in/load SPI without DMA support
>
>> There is no reason for doing 2) since we have do DMA support for sunxi.
>
> Well, presumably such code exists and is being worked on?

Which code are you referring to?

This is a reply to patch which adds DMA support to the SPI driver so
that it can work for arbitrarily long transfers.

So there is code for fully working driver.

>
>> So that's another code path that needs maintenance and testing and
>> likely will not get it.
>
> Oh, come on.  You might not want to use it yourself but the chances are
> that someone will want to use it just like the situation with all the
> other SPI drivers.  It's a perfectly reasonable and sensible feature to
> support upstream.

Is it?

Once we have *one* driver that works for arbitrarily long transfers
and it works out of the box with the board defconfig 99% of people who
will use the SPI driver for anything will use this driver. Any other
variant will go untested.

And for the driver to also work without DMA you have to *tell* it to
probe without DMA because it cannot know you are not going to load a
DMA driver later.

>
> I really do not understand why there is such a strong desire to have
> these devices be a special snowflake here, the worst that's likely to
> happen here is that you're going to end up having to either remove the
> DMA controller from the DT or load the driver for it neither of which
> seem like the end of the world.

Why would you do that?

Thanks

Michal
Mark Brown June 6, 2016, 11:36 a.m. UTC | #16
On Sun, Jun 05, 2016 at 01:27:11PM +0200, Michal Suchanek wrote:
> On 2 June 2016 at 16:26, Mark Brown <broonie@kernel.org> wrote:
> > On Thu, Jun 02, 2016 at 02:14:26PM +0200, Michal Suchanek wrote:

> >> And the code added in that patch will never run unless you

> >> 1) use long spi transfers
> >> 2) compile in/load SPI without DMA support

> >> There is no reason for doing 2) since we have do DMA support for sunxi.

> > Well, presumably such code exists and is being worked on?

> Which code are you referring to?

> This is a reply to patch which adds DMA support to the SPI driver so
> that it can work for arbitrarily long transfers.

It sounded like there might be a missing DMA driver?  Though I think I
was misreading the above.

> > Oh, come on.  You might not want to use it yourself but the chances are
> > that someone will want to use it just like the situation with all the
> > other SPI drivers.  It's a perfectly reasonable and sensible feature to
> > support upstream.

> Is it?

> Once we have *one* driver that works for arbitrarily long transfers
> and it works out of the box with the board defconfig 99% of people who
> will use the SPI driver for anything will use this driver. Any other
> variant will go untested.

We don't want multiple drivers for this and general maintainability
reasons, I'm not sure where the idea that we might want multiple drivers
came from?

> And for the driver to also work without DMA you have to *tell* it to
> probe without DMA because it cannot know you are not going to load a
> DMA driver later.

Yes.  This puts the cost on the special snowflake systems that
absolutely cannot have any DMA support in their systems for some reason
while keeping the driver featureful by default, and keeps that cost
fairly small.

> > I really do not understand why there is such a strong desire to have
> > these devices be a special snowflake here, the worst that's likely to
> > happen here is that you're going to end up having to either remove the
> > DMA controller from the DT or load the driver for it neither of which
> > seem like the end of the world.

> Why would you do that?

Like I say I really don't understand why this is.  I'm just saying there
are ways to do this fairly simply if people really want it on their
systems.
diff mbox

Patch

diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
index e1a75dd6..ed2269c 100644
--- a/drivers/spi/spi-sun4i.c
+++ b/drivers/spi/spi-sun4i.c
@@ -14,6 +14,8 @@ 
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/device.h>
+#include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/module.h>
@@ -34,6 +36,7 @@ 
 #define SUN4I_CTL_CPHA				BIT(2)
 #define SUN4I_CTL_CPOL				BIT(3)
 #define SUN4I_CTL_CS_ACTIVE_LOW			BIT(4)
+#define SUN4I_CTL_DMAMC_DEDICATED		BIT(5)
 #define SUN4I_CTL_LMTF				BIT(6)
 #define SUN4I_CTL_TF_RST			BIT(8)
 #define SUN4I_CTL_RF_RST			BIT(9)
@@ -51,6 +54,8 @@ 
 #define SUN4I_INT_STA_REG		0x10
 
 #define SUN4I_DMA_CTL_REG		0x14
+#define SUN4I_DMA_CTL_RF_READY			BIT(0)
+#define SUN4I_DMA_CTL_TF_NOT_FULL		BIT(10)
 
 #define SUN4I_WAIT_REG			0x18
 
@@ -130,6 +135,13 @@  static inline void sun4i_spi_fill_fifo(struct sun4i_spi *sspi, int len)
 	}
 }
 
+static bool sun4i_spi_can_dma(struct spi_master *master,
+			      struct spi_device *spi,
+			      struct spi_transfer *tfr)
+{
+	return tfr->len >= SUN4I_FIFO_DEPTH;
+}
+
 static void sun4i_spi_set_cs(struct spi_device *spi, bool enable)
 {
 	struct sun4i_spi *sspi = spi_master_get_devdata(spi->master);
@@ -177,17 +189,20 @@  static int sun4i_spi_transfer_one(struct spi_master *master,
 				  struct spi_transfer *tfr)
 {
 	struct sun4i_spi *sspi = spi_master_get_devdata(master);
+	struct dma_async_tx_descriptor *desc_tx = NULL, *desc_rx = NULL;
 	unsigned int mclk_rate, div, timeout;
 	unsigned int start, end, tx_time;
 	unsigned int tx_len = 0;
+	u32 reg, trigger = 0;
 	int ret = 0;
-	u32 reg;
 
-	/* We don't support transfer larger than the FIFO */
-	if (tfr->len > SUN4I_FIFO_DEPTH)
-		return -EMSGSIZE;
-	if (tfr->tx_buf && tfr->len >= SUN4I_FIFO_DEPTH)
-		return -EMSGSIZE;
+	if (!master->can_dma) {
+		/* We don't support transfer larger than the FIFO */
+		if (tfr->len > SUN4I_FIFO_DEPTH)
+			return -EMSGSIZE;
+		if (tfr->tx_buf && tfr->len >= SUN4I_FIFO_DEPTH)
+			return -EMSGSIZE;
+	}
 
 	reinit_completion(&sspi->done);
 	sspi->tx_buf = tfr->tx_buf;
@@ -277,14 +292,67 @@  static int sun4i_spi_transfer_one(struct spi_master *master,
 	sun4i_spi_write(sspi, SUN4I_BURST_CNT_REG, SUN4I_BURST_CNT(tfr->len));
 	sun4i_spi_write(sspi, SUN4I_XMIT_CNT_REG, SUN4I_XMIT_CNT(tx_len));
 
-	/* Fill the TX FIFO */
-	/* Filling the FIFO fully causes timeout for some reason
-	 * at least on spi2 on A10s */
-	sun4i_spi_fill_fifo(sspi, SUN4I_FIFO_DEPTH - 1);
-
 	/* Enable the interrupts */
 	sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, SUN4I_INT_CTL_TC);
 
+	if (sun4i_spi_can_dma(master, spi, tfr)) {
+		dev_dbg(&sspi->master->dev, "Using DMA mode for transfer\n");
+
+		if (sspi->tx_buf) {
+			desc_tx = dmaengine_prep_slave_sg(master->dma_tx,
+					tfr->tx_sg.sgl, tfr->tx_sg.nents,
+					DMA_TO_DEVICE,
+					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+			if (!desc_tx) {
+				dev_err(&sspi->master->dev,
+					"Couldn't prepare dma slave\n");
+				return -EIO;
+			}
+
+			trigger |= SUN4I_DMA_CTL_TF_NOT_FULL;
+
+			dmaengine_submit(desc_tx);
+			dma_async_issue_pending(master->dma_tx);
+
+		}
+
+		if (sspi->rx_buf) {
+			desc_rx = dmaengine_prep_slave_sg(master->dma_rx,
+					tfr->rx_sg.sgl, tfr->rx_sg.nents,
+					DMA_FROM_DEVICE,
+					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+			if (!desc_rx) {
+				dev_err(&sspi->master->dev,
+					"Couldn't prepare dma slave\n");
+				return -EIO;
+			}
+
+			trigger |= SUN4I_DMA_CTL_RF_READY;
+
+			dmaengine_submit(desc_rx);
+			dma_async_issue_pending(master->dma_rx);
+		}
+
+		/* Enable Dedicated DMA requests */
+		reg = sun4i_spi_read(sspi, SUN4I_CTL_REG);
+		reg |= SUN4I_CTL_DMAMC_DEDICATED;
+		sun4i_spi_write(sspi, SUN4I_CTL_REG, reg);
+		sun4i_spi_write(sspi, SUN4I_DMA_CTL_REG, trigger);
+	} else {
+		dev_dbg(&sspi->master->dev, "Using PIO mode for transfer\n");
+
+		/* Disable DMA requests */
+		reg = sun4i_spi_read(sspi, SUN4I_CTL_REG);
+		sun4i_spi_write(sspi, SUN4I_CTL_REG,
+				reg & ~SUN4I_CTL_DMAMC_DEDICATED);
+		sun4i_spi_write(sspi, SUN4I_DMA_CTL_REG, 0);
+
+		/* Fill the TX FIFO */
+		/* Filling the FIFO fully causes timeout for some reason
+		 * at least on spi2 on A10s */
+		sun4i_spi_fill_fifo(sspi, SUN4I_FIFO_DEPTH - 1);
+	}
+
 	/* Start the transfer */
 	reg = sun4i_spi_read(sspi, SUN4I_CTL_REG);
 	sun4i_spi_write(sspi, SUN4I_CTL_REG, reg | SUN4I_CTL_XCH);
@@ -304,7 +372,16 @@  static int sun4i_spi_transfer_one(struct spi_master *master,
 		goto out;
 	}
 
-	sun4i_spi_drain_fifo(sspi, SUN4I_FIFO_DEPTH);
+	if (sun4i_spi_can_dma(master, spi, tfr) && desc_rx) {
+		/* The receive transfer should be the last one to finish */
+			/* This seems to work OK.
+			 * TODO: look for some drivers which do something
+			 * more sensible here. */
+		dma_wait_for_async_tx(desc_rx);
+	} else {
+		/* This should be noop with tx only dma */
+		sun4i_spi_drain_fifo(sspi, SUN4I_FIFO_DEPTH);
+	}
 
 out:
 	sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, 0);
@@ -369,6 +446,7 @@  static int sun4i_spi_runtime_suspend(struct device *dev)
 
 static int sun4i_spi_probe(struct platform_device *pdev)
 {
+	struct dma_slave_config dma_sconfig;
 	struct spi_master *master;
 	struct sun4i_spi *sspi;
 	struct resource	*res;
@@ -404,6 +482,7 @@  static int sun4i_spi_probe(struct platform_device *pdev)
 		goto err_free_master;
 	}
 
+	init_completion(&sspi->done);
 	sspi->master = master;
 	master->max_speed_hz = 100*1000*1000;
 	master->min_speed_hz =        3*1000;
@@ -430,8 +509,55 @@  static int sun4i_spi_probe(struct platform_device *pdev)
 		goto err_free_master;
 	}
 
-	init_completion(&sspi->done);
+	master->dma_tx = dma_request_slave_channel_reason(&pdev->dev, "tx");
+	if (IS_ERR(master->dma_tx)) {
+		dev_err(&pdev->dev, "Unable to acquire DMA channel TX\n");
+		ret = PTR_ERR(master->dma_tx);
+		master->dma_tx = NULL;
+		goto wakeup;
+	}
+
+	dma_sconfig.direction = DMA_MEM_TO_DEV;
+	dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+	dma_sconfig.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+	dma_sconfig.dst_addr = res->start + SUN4I_TXDATA_REG;
+	dma_sconfig.src_maxburst = 1;
+	dma_sconfig.dst_maxburst = 1;
 
+	ret = dmaengine_slave_config(master->dma_tx, &dma_sconfig);
+	if (ret) {
+		dev_err(&pdev->dev, "Unable to configure TX DMA slave\n");
+		goto err_tx_dma_release;
+	}
+
+	master->dma_rx = dma_request_slave_channel_reason(&pdev->dev, "rx");
+	if (IS_ERR(master->dma_rx)) {
+		dev_err(&pdev->dev, "Unable to acquire DMA channel RX\n");
+		ret = PTR_ERR(master->dma_rx);
+		goto err_tx_dma_release;
+	}
+
+	dma_sconfig.direction = DMA_DEV_TO_MEM;
+	dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+	dma_sconfig.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+	dma_sconfig.src_addr = res->start + SUN4I_RXDATA_REG;
+	dma_sconfig.src_maxburst = 1;
+	dma_sconfig.dst_maxburst = 1;
+
+	ret = dmaengine_slave_config(master->dma_rx, &dma_sconfig);
+	if (ret) {
+		dev_err(&pdev->dev, "Unable to configure RX DMA slave\n");
+		goto err_rx_dma_release;
+	}
+
+	/* This is a bit dodgy. If you set can_dma then map_msg in spi.c
+	 * apparently dereferences your dma channels if non-NULL even if your
+	 * can_dma never returns true (and crashes if the channel is an error
+	 * pointer). So just don't set can_dma unless both channels are valid.
+	 */
+	master->can_dma = sun4i_spi_can_dma;
+	master->max_transfer_size = NULL; /* no known limit */
+wakeup:
 	/*
 	 * This wake-up/shutdown pattern is to be able to have the
 	 * device woken up, even if runtime_pm is disabled
@@ -454,18 +580,37 @@  static int sun4i_spi_probe(struct platform_device *pdev)
 
 	return 0;
 
+err_rx_dma_release:
+	dma_release_channel(master->dma_rx);
+err_tx_dma_release:
+	dma_release_channel(master->dma_tx);
+	master->dma_tx = NULL;
+	master->dma_rx = NULL;
+	goto wakeup;
+
 err_pm_disable:
 	pm_runtime_disable(&pdev->dev);
 	sun4i_spi_runtime_suspend(&pdev->dev);
 err_free_master:
+	if (master->can_dma) {
+		dma_release_channel(master->dma_rx);
+		dma_release_channel(master->dma_tx);
+	}
 	spi_master_put(master);
 	return ret;
 }
 
 static int sun4i_spi_remove(struct platform_device *pdev)
 {
+	struct spi_master *master = platform_get_drvdata(pdev);
+
 	pm_runtime_disable(&pdev->dev);
 
+	if (master->can_dma) {
+		dma_release_channel(master->dma_rx);
+		dma_release_channel(master->dma_tx);
+	}
+
 	return 0;
 }