linux-kernel - Re: [PATCH 5/6] spi: imx: support dynamic burst length for ECSPI DMA mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID:
 <VI2PR04MB11147F966EEDF316FC8122A96E8DFA@VI2PR04MB11147.eurprd04.prod.outlook.com>
Date: Thu, 27 Nov 2025 02:58:54 +0000
From: Carlos Song <carlos.song@....com>
To: Marc Kleine-Budde <mkl@...gutronix.de>
CC: "broonie@...nel.org" <broonie@...nel.org>, Frank Li <frank.li@....com>,
	"s.hauer@...gutronix.de" <s.hauer@...gutronix.de>, "kernel@...gutronix.de"
	<kernel@...gutronix.de>, "festevam@...il.com" <festevam@...il.com>,
	"linux-spi@...r.kernel.org" <linux-spi@...r.kernel.org>,
	"imx@...ts.linux.dev" <imx@...ts.linux.dev>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 5/6] spi: imx: support dynamic burst length for ECSPI DMA
 mode



> -----Original Message-----
> From: Marc Kleine-Budde <mkl@...gutronix.de>
> Sent: Wednesday, November 26, 2025 8:52 PM
> To: Carlos Song <carlos.song@....com>
> Cc: broonie@...nel.org; Frank Li <frank.li@....com>;
> s.hauer@...gutronix.de; kernel@...gutronix.de; festevam@...il.com;
> linux-spi@...r.kernel.org; imx@...ts.linux.dev;
> linux-arm-kernel@...ts.infradead.org; linux-kernel@...r.kernel.org
> Subject: Re: RE: [EXT] Re: [PATCH 5/6] spi: imx: support dynamic burst length
> for ECSPI DMA mode
> 
> On 26.11.2025 12:29:35, Carlos Song wrote:
> > > On 25.11.2025 18:06:17, Carlos Song wrote:
> > > >  static int spi_imx_pio_transfer(struct spi_device *spi,
> > > >  				struct spi_transfer *transfer)  { @@ -1780,9
> +2112,14 @@
> > > > static int spi_imx_transfer_one(struct
> > > spi_controller *controller,
> > > >  	 * transfer, the SPI transfer has already been mapped, so we
> > > >  	 * have to do the DMA transfer here.
> > > >  	 */
> > > > -	if (spi_imx->usedma)
> > > > -		return spi_imx_dma_transfer(spi_imx, transfer);
> > > > -
> > > > +	if (spi_imx->usedma) {
> > > > +		ret = spi_imx_dma_transfer(spi_imx, transfer);
> > > > +		if (transfer->error & SPI_TRANS_FAIL_NO_START) {
> > > > +			spi_imx->usedma = false;
> > > > +			return spi_imx_pio_transfer(spi, transfer);
> > > > +		}
> > > > +		return ret;
> > > > +	}
> > >
> > > Why do you do this? AFAICS the framework already does this for you see:
> > > spi_transfer_one_message().
> > >
> > Hi,
> >
> > In frame work:
> > 				if ((xfer->tx_sg_mapped || xfer->rx_sg_mapped) &&
> > 				    (xfer->error & SPI_TRANS_FAIL_NO_START)) {
> > 					__spi_unmap_msg(ctlr, msg);
> > 					ctlr->fallback = true;
> > 					xfer->error &= ~SPI_TRANS_FAIL_NO_START;
> > 					goto fallback_pio;
> > 				}
> >
> > It only will help do this for "framework finished DMA map case". But DMA
> dynamic burst feature is mapped in driver.
> > So this condition never meet: (xfer->tx_sg_mapped || xfer->rx_sg_mapped) I
> think..
> 
> ...and you remove "controller->can_dma" this deserves some comments in the
> commit message. That you fool the framework, that you cannot do DMA, and
> do all the mapping on your own.
> 
Hi,

Yes, I will add this in V2 commit log.

Hahaha, I think it is not forced handle DMA in framework. What I understand is
which path to go by " controller->can_dma "(just like a flag), if I have special DMA configure, I don't need
code this then do DMA on my own, I have never had any fool framework ideas, I just do
what I should do.

> If you can make it work with DMA_SLAVE_BUSWIDTH_1_BYTES then you can
> use
> controller->prepare_message and controller->unprepare_message to do the
> byte swapping. These functions are called before and after the DMA sync.
> 

Just I said, DMA_SLAVE_BUSWIDTH_1_BYTES is not match ECSPI HW when enable dynamic burst.
Because SPI in IMX using sdma(external common function DMA), it is not dedicated SPI DMA,
so improve speed while maintaining compatibility with two different hardware components, these configurations were necessary.

Carlos
> Marc
> 
> --
> Pengutronix e.K.                 | Marc Kleine-Budde          |
> Embedded Linux                   | https://www.pengutronix.de |
> Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
> Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |