On Fri, Jun 12, 2020 at 02:18:32AM +0000, Robin Gong wrote: > On 2020/06/11 21: 41 Mark Brown <broonie@xxxxxxxxxx> 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.
Attachment:
signature.asc
Description: PGP signature
