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

From: Mark Brown <broonie@kernel.org>
To: Robin Gong <yibin.gong@nxp.com>
Cc: "matthias.schiffer@ew.tq-group.com"
	<matthias.schiffer@ew.tq-group.com>,
	"shawnguo@kernel.org" <shawnguo@kernel.org>,
	"s.hauer@pengutronix.de" <s.hauer@pengutronix.de>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"robin.murphy@arm.com" <robin.murphy@arm.com>,
	"linux-spi@vger.kernel.org" <linux-spi@vger.kernel.org>,
	dl-linux-imx <linux-imx@nxp.com>,
	"kernel@pengutronix.de" <kernel@pengutronix.de>,
	"festevam@gmail.com" <festevam@gmail.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v1 RFC 1/2] spi: introduce fallback to pio
Date: Fri, 12 Jun 2020 11:13:57 +0100	[thread overview]
Message-ID: <20200612101357.GA5396@sirena.org.uk> (raw)
In-Reply-To: <VE1PR04MB66383245FAD2AE33CFEA76F789810@VE1PR04MB6638.eurprd04.prod.outlook.com>

[-- Attachment #1.1: Type: text/plain, Size: 2104 bytes --]

On Fri, Jun 12, 2020 at 02:18:32AM +0000, Robin Gong wrote:
> On 2020/06/11 21: 41 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.

> > 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 #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel