On Fri, Jun 12, 2020 at 02:18:32AM +0000, Robin Gong wrote: > On 2020/06/11 21: 41 Mark Brown <broo...@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. > > If we were going to do this I don't see why we'd have a flag for this > > rather than > > just doing it unconditionally but... > What do you mean flag here, 'master->flags' or SPI_MASTER_FALLBACK? > 'master->flags' > could let client fallback to PIO finally and spi core clear this flag once > this transfer done, > so that DMA could be tried again in the next transfer. 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? > > ...I don't think this can work sensibly - this is going to try PIO if > > there's *any* > > error. We might have had some sort of issue during the transfer for example > > so have some noise on the bus. Like I said on a prior version of this I > > really > 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. > > think that we need to be figuring out if the DMA controller can support the > > transaction before we even map the buffer for it, having the controller just > > randomly fail underneath the consumer just does not sound robust. > But dmaengine_prep_slave_sg still may return failure even if anything about > DMA is ok before spi transfer start, such as dma description malloc failure. > This > patch seems could make spi a bit robust... 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.
signature.asc
Description: PGP signature
