Re: [PATCH v1 RFC 1/2] spi: introduce fallback to pio

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2533 bytes --]

On Fri, Jun 12, 2020 at 01:48:41PM +0000, Robin Gong wrote:
> On 2020/06/12 18:14 Mark Brown <broonie@kernel.org> wrote: 

> > Please look at the formatting of your e-mails - they're really hard to read.  The
> > line length is over 80 columns and there's no breaks between paragraphs.

> Sorry for that, seems my outlook format issue, hope it's ok now this time :)

Yes, looks good thanks!

> > Client could enable this feature by choosing SPI_MASTER_FALLBACK freely
> > without any impact on others.

> > SPI_MASTER_FALLBACK.  If this works why would any driver not enable the
> > flag?

> Just make sure little impact if it's not good enough and potential issue may
> come out after it's merged into mainline. TBH, I'm not sure if it has taken
> care all in spi core. Besides, I don't know if other spi client need this feature.

It's not something that's going to come up a lot for most devices, it'd
be a mapping failure due to running out of memory or something, but your
point about that being possible is valid.

> > > Any error happen in DMA could fallback to PIO , seems a nice to have,
> > because it could
> > > give chance to run in PIO which is more reliable. But if there is also error in

> > PIO, thus may loop here, it's better adding limit try times here?

> > An error doesn't mean nothing happened on the bus, an error could for
> > example also be something like a FIFO overrun which corrupts data.

> Do you mean fallback to PIO may cause FIFO overrun since some latency
> involved so that this patch seems not useful as expected?

No, I mean that the reason the DMA transfer fails may be something that
happens after we've started putting things on the bus - the bit about
FIFOs is just a random example of an error that could happen.

> > It *could* but only in extreme situations, and again this isn't just handling
> > errors from failure to prepare the hardware but also anything that happens
> > after it.

> Okay, understood your point. You prefer to some interface provided by dma
> engine before dmaengine_prep_slave_sg so that can_dma() can know if
> this dma channel is ready indeed. But unfortunately, seems there is no one....

Well, this is free software and everything can be modified!  The other
option would be framework changes in SPI that allowed us to indicate
from the driver that an error occured before we started doing anything
to the hardware (like happens here) through something like a special
error code or splitting up the API.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]