On 14-06-20, 13:04, Robin Gong wrote: > On 2020/06/12 22:16 Mark Brown <broo...@kernel.org> wrote: > > On Fri, Jun 12, 2020 at 01:48:41PM +0000, Robin Gong wrote: > > > On 2020/06/12 18:14 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. > > > > > 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. > > > Sorry Mark for that I can't get your point... The bus error such as data > corrupt > seems not the spi core's business since it can only be caught in spi > controller > driver or upper level such as mtd driver (spi-nor) which know what's the > failure > happen at spi bus HW level or what's the correct data/message. In other words, > spi core can't detect such error by transfer_one(). > > > > > 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. > Yes, but both assume spi controller driver could detect such dma failure > before > dmaengine_prep_*(). Let's wait Vinod's comment for that if > dmaengine_slave_config > could keep direction.
The direction is already in the prep_ call, so sending in dmaengine_slave_config is not required, pls pass it in the prep_ call > But despite of that case, do you think this patch is valid for transfer_one() > failue > in dma and fallback to pio? -- ~Vinod
