Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Content-Type: text/plain;
        charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.120.23.2.4\))
Subject: Re: [PATCH] spi: spi-sun6i: implement DMA-based transfer mode
From:   Alexander Kochetkov <al.kochet@gmail.com>
In-Reply-To: <20201019082129.myxpxla5xwoqwldo@gilmour.lan>
Date:   Mon, 19 Oct 2020 16:17:18 +0300
Cc:     Mark Brown <broonie@kernel.org>, Chen-Yu Tsai <wens@csie.org>,
        linux-spi@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-kernel@vger.kernel.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <4B0B0459-DFCF-4307-8CAE-A2B579B0AF5E@gmail.com>
References: <20201015154740.20825-1-al.kochet@gmail.com>
 <20201019082129.myxpxla5xwoqwldo@gilmour.lan>
To:     Maxime Ripard <maxime@cerno.tech>
Precedence: bulk

Hi, Maxime! Thanks for reviewing patches!


>>=20
>> +static int sun6i_spi_prepare_dma(struct sun6i_spi *sspi,
>> +				 struct spi_transfer *tfr)
>> +{
>> +	struct dma_async_tx_descriptor *rxdesc, *txdesc;
>> +	struct spi_master *master =3D sspi->master;
>> +
>> +	rxdesc =3D NULL;
>> +	if (tfr->rx_buf) {
>> +		struct dma_slave_config rxconf =3D {
>> +			.direction =3D DMA_DEV_TO_MEM,
>> +			.src_addr =3D sspi->dma_addr_rx,
>> +			.src_addr_width =3D 1,
>> +			.src_maxburst =3D 1,
>> +		};
>=20
> That doesn't really look optimal, the controller seems to be able to
> read / write 32 bits at a time from its FIFO and we probably can
> increase the burst length too?


I had doubts if it would work. I didn=E2=80=99t know will DMA work for =
transfers with lengths not
aligned to 32 bits. For example, if we init DMA with src_addr_width =3D =
1 and
.src_maxburst =3D 8 will DMA work for transfer with length 11? I expect =
that DMA fill FIFO
with 16 bytes and SPI transfer only 11 bytes and 5 bytes will leave in =
TX fifo.  I did the test
and there is no additional data left in the fifo buffer. Also at =
reception the is no memory
overwrites.

I made test with src_addr_width =3D 4, src_maxburst =3D 1 and transfer =
length 3. Looks
like in that case DMA doesn=E2=80=99t issue 4 bytes transfer.

For test with src_addr_width =3D 4, src_maxburst =3D 8 I had to adjust =
RF_RDY_TRIG_LEVEL_BITS
and TF_ERQ_TRIG_LEVEL_BITS of FIFO_CTL_REG to half of FIFO (32 bytes). =
With the config
DMA will transfer burst of half of FIFO length during transfer and =
remaining bytes at the end of
transfer.


>>=20
>> @@ -343,7 +436,8 @@ static irqreturn_t sun6i_spi_handler(int irq, =
void *dev_id)
>> 	/* Transfer complete */
>> 	if (status & SUN6I_INT_CTL_TC) {
>> 		sun6i_spi_write(sspi, SUN6I_INT_STA_REG, =
SUN6I_INT_CTL_TC);
>> -		sun6i_spi_drain_fifo(sspi);
>> +		if (!sspi->use_dma)
>> +			sun6i_spi_drain_fifo(sspi);
>=20
> Is it causing any issue? The FIFO will be drained only if there's
> something remaining in the FIFO, which shouldn't happen with DMA?
>=20

No. It=E2=80=99s for make code patch explicit.
Remove the change?

Alexander.