[v9,RESEND,01/13] spi: imx: add dma_sync_sg_for_device after fallback from dma
diff mbox series

Message ID 1591485677-20533-2-git-send-email-yibin.gong@nxp.com
State New
Headers show
Series
  • add ecspi ERR009165 for i.mx6/7 soc family
Related show

Commit Message

Robin Gong June 6, 2020, 11:21 p.m. UTC
In case dma transfer failed and fallback to pio, tx_buf/rx_buf need to be
taken care cache since they have already been maintained by spi.c

Fixes: bcd8e7761ec9("spi: imx: fallback to PIO if dma setup failure")
Signed-off-by: Robin Gong <yibin.gong@nxp.com>
Reported-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
Link: https://lore.kernel.org/linux-arm-kernel/5d246dd81607bb6e5cb9af86ad4e53f7a7a99c50.camel@ew.tq-group.com/
---
 drivers/spi/spi-imx.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Mark Brown June 8, 2020, 2:34 p.m. UTC | #1
On Sun, Jun 07, 2020 at 07:21:05AM +0800, Robin Gong wrote:
> In case dma transfer failed and fallback to pio, tx_buf/rx_buf need to be
> taken care cache since they have already been maintained by spi.c

Is this needed as part of this series?  This looks like an independent
fix and it seems better to get this in independently. 

> Fixes: bcd8e7761ec9("spi: imx: fallback to PIO if dma setup failure")
> Signed-off-by: Robin Gong <yibin.gong@nxp.com>
> Reported-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> Link: https://lore.kernel.org/linux-arm-kernel/5d246dd81607bb6e5cb9af86ad4e53f7a7a99c50.camel@ew.tq-group.com/

The Link is usually to the patch on the list.

> --- a/drivers/spi/spi-imx.c
> +++ b/drivers/spi/spi-imx.c
> @@ -1456,6 +1456,13 @@ static int spi_imx_pio_transfer(struct spi_device *spi,
>  		return -ETIMEDOUT;
>  	}
>  
> +	if (transfer->rx_sg.sgl) {
> +		struct device *rx_dev = spi->controller->dma_rx->device->dev;
> +
> +		dma_sync_sg_for_device(rx_dev, transfer->rx_sg.sgl,
> +				       transfer->rx_sg.nents, DMA_TO_DEVICE);
> +	}
> +
>  	return transfer->len;
>  }

This is confusing - why are we DMA mapping to the device after doing a
PIO transfer?
Robin Gong June 8, 2020, 3:08 p.m. UTC | #2
On 2020/06/08 22:35 Mark Brown <broonie@kernel.org> wrote:
> On Sun, Jun 07, 2020 at 07:21:05AM +0800, Robin Gong wrote:
> > In case dma transfer failed and fallback to pio, tx_buf/rx_buf need to
> > be taken care cache since they have already been maintained by spi.c
> 
> Is this needed as part of this series?  This looks like an independent fix and it
> seems better to get this in independently.
But that's used to fix one patch [05/13]of the v8 patch set. To be honest, I'm also
not sure how to handle it so that I merged both into first v9....For now, I think you
are right, since 'fallback pio' patch could be independent this series. Will resend in
v10.
> 
> > Fixes: bcd8e7761ec9("spi: imx: fallback to PIO if dma setup failure")
> > Signed-off-by: Robin Gong <yibin.gong@nxp.com>
> > Reported-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> > Link:
> > https://lore.kernel.org/linux-arm-kernel/5d246dd81607bb6e5cb9af86ad4e5
> > 3f7a7a99c50.camel@ew.tq-group.com/
> 
> The Link is usually to the patch on the list.
Okay, will remove it.
> 
> > --- a/drivers/spi/spi-imx.c
> > +++ b/drivers/spi/spi-imx.c
> > @@ -1456,6 +1456,13 @@ static int spi_imx_pio_transfer(struct spi_device
> *spi,
> >  		return -ETIMEDOUT;
> >  	}
> >
> > +	if (transfer->rx_sg.sgl) {
> > +		struct device *rx_dev = spi->controller->dma_rx->device->dev;
> > +
> > +		dma_sync_sg_for_device(rx_dev, transfer->rx_sg.sgl,
> > +				       transfer->rx_sg.nents, DMA_TO_DEVICE);
> > +	}
> > +
> >  	return transfer->len;
> >  }
> 
> This is confusing - why are we DMA mapping to the device after doing a PIO
> transfer?
'transfer->rx_sg.sgl' condition check that's the case fallback PIO after DMA transfer
failed. But the spi core still think the buffer should be in 'device' while spi driver
touch it by PIO(CPU), so sync it back to device to ensure all received data flush to DDR.
Mark Brown June 8, 2020, 3:31 p.m. UTC | #3
On Mon, Jun 08, 2020 at 03:08:45PM +0000, Robin Gong wrote:

> > > +	if (transfer->rx_sg.sgl) {
> > > +		struct device *rx_dev = spi->controller->dma_rx->device->dev;
> > > +
> > > +		dma_sync_sg_for_device(rx_dev, transfer->rx_sg.sgl,
> > > +				       transfer->rx_sg.nents, DMA_TO_DEVICE);
> > > +	}
> > > +

> > This is confusing - why are we DMA mapping to the device after doing a PIO
> > transfer?

> 'transfer->rx_sg.sgl' condition check that's the case fallback PIO after DMA transfer
> failed. But the spi core still think the buffer should be in 'device' while spi driver
> touch it by PIO(CPU), so sync it back to device to ensure all received data flush to DDR.

So we sync it back to the device so that we can then do another sync to
CPU?  TBH I'm a bit surprised that there's a requirement that we
explicitly undo a sync and that a redundant double sync in the same
direction might be an issue but I've not had a need to care so I'm
perfectly prepared to believe there is.

At the very least this needs a comment.
Robin Murphy June 8, 2020, 4:44 p.m. UTC | #4
On 2020-06-08 16:31, Mark Brown wrote:
> On Mon, Jun 08, 2020 at 03:08:45PM +0000, Robin Gong wrote:
> 
>>>> +	if (transfer->rx_sg.sgl) {
>>>> +		struct device *rx_dev = spi->controller->dma_rx->device->dev;
>>>> +
>>>> +		dma_sync_sg_for_device(rx_dev, transfer->rx_sg.sgl,
>>>> +				       transfer->rx_sg.nents, DMA_TO_DEVICE);
>>>> +	}
>>>> +
> 
>>> This is confusing - why are we DMA mapping to the device after doing a PIO
>>> transfer?
> 
>> 'transfer->rx_sg.sgl' condition check that's the case fallback PIO after DMA transfer
>> failed. But the spi core still think the buffer should be in 'device' while spi driver
>> touch it by PIO(CPU), so sync it back to device to ensure all received data flush to DDR.
> 
> So we sync it back to the device so that we can then do another sync to
> CPU?  TBH I'm a bit surprised that there's a requirement that we
> explicitly undo a sync and that a redundant double sync in the same
> direction might be an issue but I've not had a need to care so I'm
> perfectly prepared to believe there is.
> 
> At the very least this needs a comment.

Yeah, something's off here - at the very least, syncing with 
DMA_TO_DEVICE on the Rx buffer that was mapped with DMA_FROM_DEVICE is 
clearly wrong. CONFIG_DMA_API_DEBUG should scream about that.

If the device has written to the buffer at all since dma_map_sg() was 
called then you do need a dma_sync_sg_for_cpu() call before touching it 
from a CPU fallback path, but if nobody's going to touch it from that 
point until it's unmapped then there's no point syncing it again. The 
my_card_interrupt_handler() example in DMA-API_HOWTO.txt demonstrates this.

Robin.
Robin Gong June 9, 2020, 2:45 a.m. UTC | #5
On 2020/06/08 23:32 Mark Brown <broonie@kernel.org> wrote: 
> On Mon, Jun 08, 2020 at 03:08:45PM +0000, Robin Gong wrote:
> 
> > > > +	if (transfer->rx_sg.sgl) {
> > > > +		struct device *rx_dev = spi->controller->dma_rx->device->dev;
> > > > +
> > > > +		dma_sync_sg_for_device(rx_dev, transfer->rx_sg.sgl,
> > > > +				       transfer->rx_sg.nents, DMA_TO_DEVICE);
> > > > +	}
> > > > +
> 
> > > This is confusing - why are we DMA mapping to the device after doing
> > > a PIO transfer?
> 
> > 'transfer->rx_sg.sgl' condition check that's the case fallback PIO
> > after DMA transfer failed. But the spi core still think the buffer
> > should be in 'device' while spi driver touch it by PIO(CPU), so sync it back to
> device to ensure all received data flush to DDR.
> 
> So we sync it back to the device so that we can then do another sync to CPU?
Yes, spi.c will sync to CPU again in spi_unmap_buf() after transfer done finally.
Otherwise, the fresh received data by CPU in this fallback case may be invalidated
by spi.c, which led to the data corrupt on Matthias's side.

> TBH I'm a bit surprised that there's a requirement that we explicitly undo a
> sync and that a redundant double sync in the same direction might be an issue
Considering DMA transfer may be failed(for example, sdma firmware may not be
updated as ERR009165 depends on), we'd better fallback to PIO to ensure no any
function break here. Thus should clean fresh rx data from cache into external memory
as real 'device' received by DMA. Understood a bit confusing here, but that way could
be avoided by any code changing in spi.c. Or make some code changes in spi.c to cancel
spi_unmap_buf() in such fallback case?

> but I've not had a need to care so I'm perfectly prepared to believe there is.
> 
> At the very least this needs a comment.
Okay, I'll add comment here in next.
Robin Gong June 9, 2020, 5:21 a.m. UTC | #6
On 2020/06/09 0:44 Robin Murphy <robin.murphy@arm.com> wrote:
> On 2020-06-08 16:31, Mark Brown wrote:
> > On Mon, Jun 08, 2020 at 03:08:45PM +0000, Robin Gong wrote:
> >
> >>>> +	if (transfer->rx_sg.sgl) {
> >>>> +		struct device *rx_dev = spi->controller->dma_rx->device->dev;
> >>>> +
> >>>> +		dma_sync_sg_for_device(rx_dev, transfer->rx_sg.sgl,
> >>>> +				       transfer->rx_sg.nents, DMA_TO_DEVICE);
> >>>> +	}
> >>>> +
> >
> >>> This is confusing - why are we DMA mapping to the device after doing
> >>> a PIO transfer?
> >
> >> 'transfer->rx_sg.sgl' condition check that's the case fallback PIO
> >> after DMA transfer failed. But the spi core still think the buffer
> >> should be in 'device' while spi driver touch it by PIO(CPU), so sync it back to
> device to ensure all received data flush to DDR.
> >
> > So we sync it back to the device so that we can then do another sync
> > to CPU?  TBH I'm a bit surprised that there's a requirement that we
> > explicitly undo a sync and that a redundant double sync in the same
> > direction might be an issue but I've not had a need to care so I'm
> > perfectly prepared to believe there is.
> >
> > At the very least this needs a comment.
> 
> Yeah, something's off here - at the very least, syncing with DMA_TO_DEVICE on
> the Rx buffer that was mapped with DMA_FROM_DEVICE is clearly wrong.
> CONFIG_DMA_API_DEBUG should scream about that.
> 
> If the device has written to the buffer at all since dma_map_sg() was called
> then you do need a dma_sync_sg_for_cpu() call before touching it from a CPU
> fallback path, but if nobody's going to touch it from that point until it's
> unmapped then there's no point syncing it again. The
> my_card_interrupt_handler() example in DMA-API_HOWTO.txt demonstrates
> this.
Thanks for you post, but sorry, that's not spi-imx case now, because the rx data in device memory is not truly updated from 'device'/DMA, but from PIO, so that dma_sync_sg_for_cpu with DMA_FROM_DEVICE can't be used, otherwise the fresh data in cache will be invalidated.
But you're right, kernel warning comes out if CONFIG_DMA_API_DEBUG enabled...
Robin Murphy June 9, 2020, 10 a.m. UTC | #7
On 2020-06-09 06:21, Robin Gong wrote:
> On 2020/06/09 0:44 Robin Murphy <robin.murphy@arm.com> wrote:
>> On 2020-06-08 16:31, Mark Brown wrote:
>>> On Mon, Jun 08, 2020 at 03:08:45PM +0000, Robin Gong wrote:
>>>
>>>>>> +	if (transfer->rx_sg.sgl) {
>>>>>> +		struct device *rx_dev = spi->controller->dma_rx->device->dev;
>>>>>> +
>>>>>> +		dma_sync_sg_for_device(rx_dev, transfer->rx_sg.sgl,
>>>>>> +				       transfer->rx_sg.nents, DMA_TO_DEVICE);
>>>>>> +	}
>>>>>> +
>>>
>>>>> This is confusing - why are we DMA mapping to the device after doing
>>>>> a PIO transfer?
>>>
>>>> 'transfer->rx_sg.sgl' condition check that's the case fallback PIO
>>>> after DMA transfer failed. But the spi core still think the buffer
>>>> should be in 'device' while spi driver touch it by PIO(CPU), so sync it back to
>> device to ensure all received data flush to DDR.
>>>
>>> So we sync it back to the device so that we can then do another sync
>>> to CPU?  TBH I'm a bit surprised that there's a requirement that we
>>> explicitly undo a sync and that a redundant double sync in the same
>>> direction might be an issue but I've not had a need to care so I'm
>>> perfectly prepared to believe there is.
>>>
>>> At the very least this needs a comment.
>>
>> Yeah, something's off here - at the very least, syncing with DMA_TO_DEVICE on
>> the Rx buffer that was mapped with DMA_FROM_DEVICE is clearly wrong.
>> CONFIG_DMA_API_DEBUG should scream about that.
>>
>> If the device has written to the buffer at all since dma_map_sg() was called
>> then you do need a dma_sync_sg_for_cpu() call before touching it from a CPU
>> fallback path, but if nobody's going to touch it from that point until it's
>> unmapped then there's no point syncing it again. The
>> my_card_interrupt_handler() example in DMA-API_HOWTO.txt demonstrates
>> this.
> Thanks for you post, but sorry, that's not spi-imx case now, because the rx data in device memory is not truly updated from 'device'/DMA, but from PIO, so that dma_sync_sg_for_cpu with DMA_FROM_DEVICE can't be used, otherwise the fresh data in cache will be invalidated.
> But you're right, kernel warning comes out if CONFIG_DMA_API_DEBUG enabled...

Ah, I think I understand what's going on now. That's... really ugly :(

Looking at the SPI core code, I think a better way to handle this would 
be to have your fallback path call spi_unmap_buf() directly (or perform 
the same actions, if exporting that to drivers is unacceptable), then 
make sure ->can_dma() returns false after that such that spi_unmap_msg() 
won't try to unmap it again. That's a lot more reasonable than trying to 
fake up a DMA_TO_DEVICE transfer in the middle of a DMA_FROM_DEVICE 
operation on the same buffer.

Alternatively, is it feasible to initiate a dummy DMA request during 
probe, such that you can detect the failure condition and give up on the 
DMA channel early, and not have to deal with it during a real SPI transfer?

Robin.
Matthias Schiffer June 9, 2020, 10:09 a.m. UTC | #8
On Tue, 2020-06-09 at 11:00 +0100, Robin Murphy wrote:
> On 2020-06-09 06:21, Robin Gong wrote:
> > On 2020/06/09 0:44 Robin Murphy <robin.murphy@arm.com> wrote:
> > > On 2020-06-08 16:31, Mark Brown wrote:
> > > > On Mon, Jun 08, 2020 at 03:08:45PM +0000, Robin Gong wrote:
> > > > 
> > > > > > > +	if (transfer->rx_sg.sgl) {
> > > > > > > +		struct device *rx_dev = spi->controller-
> > > > > > > >dma_rx->device->dev;
> > > > > > > +
> > > > > > > +		dma_sync_sg_for_device(rx_dev, transfer-
> > > > > > > >rx_sg.sgl,
> > > > > > > +				       transfer->rx_sg.nents,
> > > > > > > DMA_TO_DEVICE);
> > > > > > > +	}
> > > > > > > +
> > > > > > This is confusing - why are we DMA mapping to the device
> > > > > > after doing
> > > > > > a PIO transfer?
> > > > > 'transfer->rx_sg.sgl' condition check that's the case
> > > > > fallback PIO
> > > > > after DMA transfer failed. But the spi core still think the
> > > > > buffer
> > > > > should be in 'device' while spi driver touch it by PIO(CPU),
> > > > > so sync it back to
> > > 
> > > device to ensure all received data flush to DDR.
> > > > 
> > > > So we sync it back to the device so that we can then do another
> > > > sync
> > > > to CPU?  TBH I'm a bit surprised that there's a requirement
> > > > that we
> > > > explicitly undo a sync and that a redundant double sync in the
> > > > same
> > > > direction might be an issue but I've not had a need to care so
> > > > I'm
> > > > perfectly prepared to believe there is.
> > > > 
> > > > At the very least this needs a comment.
> > > 
> > > Yeah, something's off here - at the very least, syncing with
> > > DMA_TO_DEVICE on
> > > the Rx buffer that was mapped with DMA_FROM_DEVICE is clearly
> > > wrong.
> > > CONFIG_DMA_API_DEBUG should scream about that.
> > > 
> > > If the device has written to the buffer at all since dma_map_sg()
> > > was called
> > > then you do need a dma_sync_sg_for_cpu() call before touching it
> > > from a CPU
> > > fallback path, but if nobody's going to touch it from that point
> > > until it's
> > > unmapped then there's no point syncing it again. The
> > > my_card_interrupt_handler() example in DMA-API_HOWTO.txt
> > > demonstrates
> > > this.
> > 
> > Thanks for you post, but sorry, that's not spi-imx case now,
> > because the rx data in device memory is not truly updated from
> > 'device'/DMA, but from PIO, so that dma_sync_sg_for_cpu with
> > DMA_FROM_DEVICE can't be used, otherwise the fresh data in cache
> > will be invalidated.
> > But you're right, kernel warning comes out if CONFIG_DMA_API_DEBUG
> > enabled...
> 
> Ah, I think I understand what's going on now. That's... really ugly
> :(
> 
> Looking at the SPI core code, I think a better way to handle this
> would 
> be to have your fallback path call spi_unmap_buf() directly (or
> perform 
> the same actions, if exporting that to drivers is unacceptable),
> then 
> make sure ->can_dma() returns false after that such that
> spi_unmap_msg() 
> won't try to unmap it again. That's a lot more reasonable than trying
> to 
> fake up a DMA_TO_DEVICE transfer in the middle of a DMA_FROM_DEVICE 
> operation on the same buffer.
> 
> Alternatively, is it feasible to initiate a dummy DMA request during 
> probe, such that you can detect the failure condition and give up on
> the 
> DMA channel early, and not have to deal with it during a real SPI
> transfer?
> 
> Robin.


Would this cover the transient DMA failure that is happening between
SDMA registration and firmware load? This is exactly the case for which
the PIO fallback is triggered for us: As soon as the SDMA driver is
registered, the SPI driver can be probed as well, usually failing its
first DMA transfer, as the SDMA firmware is not loaded yet. We would
still like the SPI controller to use DMA as soon as it's actually
available.

I assume the actual issue is that the SDMA controller is considered
registered before the firmware load has finished, but I have no idea
how feasible it would be to change that (some comments in the code
explain why this currently isn't the case).

Matthias
Robin Gong June 9, 2020, 10:10 a.m. UTC | #9
On 2020/06/09 Robin Murphy <robin.murphy@arm.com> wrote: 
> On 2020-06-09 06:21, Robin Gong wrote:
> > On 2020/06/09 0:44 Robin Murphy <robin.murphy@arm.com> wrote:
> >> On 2020-06-08 16:31, Mark Brown wrote:
> >>> On Mon, Jun 08, 2020 at 03:08:45PM +0000, Robin Gong wrote:
> >>>
> >>>>>> +	if (transfer->rx_sg.sgl) {
> >>>>>> +		struct device *rx_dev = spi->controller->dma_rx->device->dev;
> >>>>>> +
> >>>>>> +		dma_sync_sg_for_device(rx_dev, transfer->rx_sg.sgl,
> >>>>>> +				       transfer->rx_sg.nents, DMA_TO_DEVICE);
> >>>>>> +	}
> >>>>>> +
> >>>
> >>>>> This is confusing - why are we DMA mapping to the device after
> >>>>> doing a PIO transfer?
> >>>
> >>>> 'transfer->rx_sg.sgl' condition check that's the case fallback PIO
> >>>> after DMA transfer failed. But the spi core still think the buffer
> >>>> should be in 'device' while spi driver touch it by PIO(CPU), so
> >>>> sync it back to
> >> device to ensure all received data flush to DDR.
> >>>
> >>> So we sync it back to the device so that we can then do another sync
> >>> to CPU?  TBH I'm a bit surprised that there's a requirement that we
> >>> explicitly undo a sync and that a redundant double sync in the same
> >>> direction might be an issue but I've not had a need to care so I'm
> >>> perfectly prepared to believe there is.
> >>>
> >>> At the very least this needs a comment.
> >>
> >> Yeah, something's off here - at the very least, syncing with
> >> DMA_TO_DEVICE on the Rx buffer that was mapped with
> DMA_FROM_DEVICE is clearly wrong.
> >> CONFIG_DMA_API_DEBUG should scream about that.
> >>
> >> If the device has written to the buffer at all since dma_map_sg() was
> >> called then you do need a dma_sync_sg_for_cpu() call before touching
> >> it from a CPU fallback path, but if nobody's going to touch it from
> >> that point until it's unmapped then there's no point syncing it
> >> again. The
> >> my_card_interrupt_handler() example in DMA-API_HOWTO.txt
> demonstrates
> >> this.
> > Thanks for you post, but sorry, that's not spi-imx case now, because the rx
> data in device memory is not truly updated from 'device'/DMA, but from PIO,
> so that dma_sync_sg_for_cpu with DMA_FROM_DEVICE can't be used,
> otherwise the fresh data in cache will be invalidated.
> > But you're right, kernel warning comes out if CONFIG_DMA_API_DEBUG
> enabled...
> 
> Ah, I think I understand what's going on now. That's... really ugly :(
Yeah...The only reason is to avoid touch any spi core code...I'm trying to implement fallback at spi core so that can spi_unmap_buf directly if dma transfer error and no need
such dma_sync_* in spi client driver. Not sure if Mark could accept it. Thanks for your below great thoughts :) 
> 
> Looking at the SPI core code, I think a better way to handle this would be to
> have your fallback path call spi_unmap_buf() directly (or perform the same
> actions, if exporting that to drivers is unacceptable), then make sure
> ->can_dma() returns false after that such that spi_unmap_msg() won't try to
> unmap it again. That's a lot more reasonable than trying to fake up a
> DMA_TO_DEVICE transfer in the middle of a DMA_FROM_DEVICE operation on
> the same buffer.
> 
> Alternatively, is it feasible to initiate a dummy DMA request during probe, such
> that you can detect the failure condition and give up on the DMA channel early,
> and not have to deal with it during a real SPI transfer?
> 
> Robin.
Mark Brown June 9, 2020, 1:26 p.m. UTC | #10
On Tue, Jun 09, 2020 at 12:09:09PM +0200, Matthias Schiffer wrote:

> I assume the actual issue is that the SDMA controller is considered
> registered before the firmware load has finished, but I have no idea
> how feasible it would be to change that (some comments in the code
> explain why this currently isn't the case).

Right, this is what's causing trouble (or at least the DMA driver not
doing PIO behind the driver I guess but that's pretty nasty).
Mark Brown June 9, 2020, 1:36 p.m. UTC | #11
On Tue, Jun 09, 2020 at 11:00:33AM +0100, Robin Murphy wrote:

> Ah, I think I understand what's going on now. That's... really ugly :(

> Looking at the SPI core code, I think a better way to handle this would be
> to have your fallback path call spi_unmap_buf() directly (or perform the
> same actions, if exporting that to drivers is unacceptable), then make sure
> ->can_dma() returns false after that such that spi_unmap_msg() won't try to
> unmap it again. That's a lot more reasonable than trying to fake up a
> DMA_TO_DEVICE transfer in the middle of a DMA_FROM_DEVICE operation on the
> same buffer.

Ideally the driver would be checking in can_dma() if the DMA controller
is able to perform transactions rather than letting things run as far as 
trying to actually do the transfer, that's a whole lot cleaner and more
manageable than running into an error doing the transfer.  I'm surprised
that there's no DMA API way to figure this out TBH.

We'll also need some handling for this changing at runtime, we're not
expecting this to be dynamic at all - we're expecting it to be a static
property of the controller/transfer combination, we didn't contemplate
this varying randomly at runtime.  Instead of rechecking can_dma() we
ought to have a flag saying if we did the mapping (which the bodge Robin
suggests above could clear).

Patch
diff mbox series

diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index b7a85e3..84aebee 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -1456,6 +1456,13 @@  static int spi_imx_pio_transfer(struct spi_device *spi,
 		return -ETIMEDOUT;
 	}
 
+	if (transfer->rx_sg.sgl) {
+		struct device *rx_dev = spi->controller->dma_rx->device->dev;
+
+		dma_sync_sg_for_device(rx_dev, transfer->rx_sg.sgl,
+				       transfer->rx_sg.nents, DMA_TO_DEVICE);
+	}
+
 	return transfer->len;
 }
 
@@ -1521,10 +1528,15 @@  static int spi_imx_transfer(struct spi_device *spi,
 	 * firmware may not be updated as ERR009165 required.
 	 */
 	if (spi_imx->usedma) {
+		struct device *tx_dev = spi->controller->dma_tx->device->dev;
+
 		ret = spi_imx_dma_transfer(spi_imx, transfer);
 		if (ret != -EINVAL)
 			return ret;
 
+		dma_sync_sg_for_cpu(tx_dev, transfer->tx_sg.sgl,
+				    transfer->tx_sg.nents, DMA_FROM_DEVICE);
+
 		spi_imx->devtype_data->disable_dma(spi_imx);
 
 		spi_imx->usedma = false;