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, shawnguo@kernel.org,
	s.hauer@pengutronix.de, linux-kernel@vger.kernel.org,
	robin.murphy@arm.com, linux-spi@vger.kernel.org,
	linux-imx@nxp.com, kernel@pengutronix.de, festevam@gmail.com,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v1 RFC 1/2] spi: introduce fallback to pio
Date: Thu, 11 Jun 2020 14:40:42 +0100	[thread overview]
Message-ID: <20200611134042.GG4671@sirena.org.uk> (raw)
In-Reply-To: <1591880310-1813-2-git-send-email-yibin.gong@nxp.com>

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

On Thu, Jun 11, 2020 at 08:58:29PM +0800, Robin Gong wrote:
> Add SPI_CONTROLLER_FALLBACK to fallback to pio mode in case dma transfer
> failed.
> If spi client driver want to enable this feature please set master->flags
> with SPI_MASTER_FALLBACK and add master->fallback checking in its can_dma()
> as spi-imx.c

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...

>  			ret = ctlr->transfer_one(ctlr, msg->spi, xfer);
>  			if (ret < 0) {
> +				if (ctlr->cur_msg_mapped &&
> +				   (ctlr->flags & SPI_CONTROLLER_FALLBACK)) {
> +					__spi_unmap_msg(ctlr, msg);
> +					ctlr->fallback = true;
> +					goto fallback_pio;
> +				}

...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 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.

[-- 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