| Message ID | 20220916113951.228398-5-vincent.whitchurch@axis.com |
|---|---|
| State | New |
| Headers | show |
| Series | spi: Fix DMA bugs in (not only) spi-s3c64xx | expand |
On 16/09/2022 13:39, Vincent Whitchurch wrote: > The COUNT_VALUE in the PACKET_CNT register is 16-bit so the maximum > value is 65535. Asking the driver to transfer a larger size currently > leads to the DMA transfer timing out. Fix this by splitting the > transfer as needed. > > With this, the len>64 KiB tests in spi-loopback-test pass. > > (Note that len==64 KiB tests work even without this patch for some reason. > The driver programs 0 to the COUNT_VALUE field in that case, but it's > unclear if it's by design, since the hardware documentation doesn't say > anything about the behaviour when COUNT_VALUE == 0, so play it safe and > split at 65535.) > > Fixes: 230d42d422e7b69 ("spi: Add s3c64xx SPI Controller driver") > Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
On Fri, Sep 16, 2022 at 01:39:51PM +0200, Vincent Whitchurch wrote: > The COUNT_VALUE in the PACKET_CNT register is 16-bit so the maximum > value is 65535. Asking the driver to transfer a larger size currently > leads to the DMA transfer timing out. Fix this by splitting the > transfer as needed. The driver should just set max_transfer_size and let the core handle this.
On Mon, Sep 19, 2022 at 03:38:29PM +0100, Mark Brown wrote: > On Fri, Sep 16, 2022 at 01:39:51PM +0200, Vincent Whitchurch wrote: > > The COUNT_VALUE in the PACKET_CNT register is 16-bit so the maximum > > value is 65535. Asking the driver to transfer a larger size currently > > leads to the DMA transfer timing out. Fix this by splitting the > > transfer as needed. > > The driver should just set max_transfer_size and let the core > handle this. Hmm, AFAICS, the core doesn't actually do anything with max_transfer_size? It's only used from spi-mem and a handful of other clients via spi_max_transfer_size().
On Mon, Sep 26, 2022 at 03:42:30PM +0200, Vincent Whitchurch wrote: > On Mon, Sep 19, 2022 at 03:38:29PM +0100, Mark Brown wrote: > > The driver should just set max_transfer_size and let the core > > handle this. > Hmm, AFAICS, the core doesn't actually do anything with > max_transfer_size? It's only used from spi-mem and a handful of other > clients via spi_max_transfer_size(). Then we should fix the core to handle it properly.
diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c index 7f346866614a..85e1d1f90109 100644 --- a/drivers/spi/spi-s3c64xx.c +++ b/drivers/spi/spi-s3c64xx.c @@ -701,6 +701,16 @@ static int s3c64xx_spi_prepare_message(struct spi_master *master, struct spi_device *spi = msg->spi; struct s3c64xx_spi_csinfo *cs = spi->controller_data; + if (master->can_dma) { + int ret; + + /* Limited by size of PACKET_CNT.COUNT_VALUE. */ + ret = spi_split_transfers_maxsize(master, msg, 65535, + GFP_KERNEL | GFP_DMA); + if (ret) + return ret; + } + /* Configure feedback delay */ if (!cs) /* No delay if not defined */
The COUNT_VALUE in the PACKET_CNT register is 16-bit so the maximum value is 65535. Asking the driver to transfer a larger size currently leads to the DMA transfer timing out. Fix this by splitting the transfer as needed. With this, the len>64 KiB tests in spi-loopback-test pass. (Note that len==64 KiB tests work even without this patch for some reason. The driver programs 0 to the COUNT_VALUE field in that case, but it's unclear if it's by design, since the hardware documentation doesn't say anything about the behaviour when COUNT_VALUE == 0, so play it safe and split at 65535.) Fixes: 230d42d422e7b69 ("spi: Add s3c64xx SPI Controller driver") Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com> --- drivers/spi/spi-s3c64xx.c | 10 ++++++++++ 1 file changed, 10 insertions(+)