| 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
| ||
|
Message-ID:
<VI2PR04MB111475132F3380BF3B5ED635AE8DEA@VI2PR04MB11147.eurprd04.prod.outlook.com>
Date: Wed, 26 Nov 2025 02:11:14 +0000
From: Carlos Song <carlos.song@....com>
To: Frank Li <frank.li@....com>
CC: "broonie@...nel.org" <broonie@...nel.org>, "hawnguo@...nel.org"
<hawnguo@...nel.org>, "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 6/6] spi: imx: enable DMA mode for target operation
> -----Original Message-----
> From: Frank Li <frank.li@....com>
> Sent: Wednesday, November 26, 2025 12:06 AM
> To: Carlos Song <carlos.song@....com>
> Cc: broonie@...nel.org; hawnguo@...nel.org; 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: [PATCH 6/6] spi: imx: enable DMA mode for target operation
>
> On Tue, Nov 25, 2025 at 06:06:18PM +0800, Carlos Song wrote:
> > Enable DMA mode for SPI IMX in target mode.
> > Disable the word delay feature for target mode, because target mode
> > should always keep high performance to make sure it can follow the
> > master. Target mode continues to operate in dynamic burst mode.
>
> If two paragraph, need extra empty line. If one parapraph, move to previous
> line.
>
Will do this in V2.
> >
> > Signed-off-by: Carlos Song <carlos.song@....com>
> > ---
> > drivers/spi/spi-imx.c | 78
> > +++++++++++++++++++++++++++++++------------
> > 1 file changed, 56 insertions(+), 22 deletions(-)
> >
> ...
> >
> > @@ -1756,23 +1753,51 @@ static int spi_imx_dma_submit(struct
> > spi_imx_data *spi_imx,
> >
> > transfer_timeout = spi_imx_calculate_timeout(spi_imx,
> > transfer->len);
> >
> > - /* Wait SDMA to finish the data transfer.*/
> > - time_left = wait_for_completion_timeout(&spi_imx->dma_tx_completion,
> > - transfer_timeout);
> > - if (!time_left) {
> > - dev_err(spi_imx->dev, "I/O Error in DMA TX\n");
> > - dmaengine_terminate_all(controller->dma_tx);
> > - dmaengine_terminate_all(controller->dma_rx);
> > - return -ETIMEDOUT;
> > - }
> > + if (!spi_imx->target_mode) {
> > + /* Wait SDMA to finish the data transfer.*/
> > + time_left =
> wait_for_completion_timeout(&spi_imx->dma_tx_completion,
> > + transfer_timeout);
> > + if (!time_left) {
> > + dev_err(spi_imx->dev, "I/O Error in DMA TX\n");
> > + dmaengine_terminate_all(controller->dma_tx);
> > + dmaengine_terminate_all(controller->dma_rx);
> > + return -ETIMEDOUT;
> > + }
> >
> > - time_left = wait_for_completion_timeout(&spi_imx->dma_rx_completion,
> > - transfer_timeout);
> > - if (!time_left) {
> > - dev_err(&controller->dev, "I/O Error in DMA RX\n");
> > - spi_imx->devtype_data->reset(spi_imx);
> > - dmaengine_terminate_all(controller->dma_rx);
> > - return -ETIMEDOUT;
> > + time_left =
> wait_for_completion_timeout(&spi_imx->dma_rx_completion,
> > + transfer_timeout);
> > + if (!time_left) {
> > + dev_err(&controller->dev, "I/O Error in DMA RX\n");
> > + spi_imx->devtype_data->reset(spi_imx);
> > + dmaengine_terminate_all(controller->dma_rx);
> > + return -ETIMEDOUT;
> > + }
> > + } else {
> > + spi_imx->target_aborted = false;
> > +
> > + if (wait_for_completion_interruptible(&spi_imx->dma_tx_completion)
> ||
> > + spi_imx->target_aborted) {
>
> Suppose somewhere set target_aborted to true. I think here should use
> READ_ONCE() to make sure read from memory.
>
> Not sure why here use wait_for_completion_interruptible() but at master mode
> use wait_for_completion_timeout().
>
> > + dev_dbg(spi_imx->dev, "I/O Error in DMA TX interrupted\n");
> > + dmaengine_terminate_all(controller->dma_tx);
> > + dmaengine_terminate_all(controller->dma_rx);
> > + return -EINTR;
> > + }
> > +
> > + if (wait_for_completion_interruptible(&spi_imx->dma_rx_completion)
> ||
> > + spi_imx->target_aborted) {
> > + dev_dbg(spi_imx->dev, "I/O Error in DMA RX interrupted\n");
> > + dmaengine_terminate_all(controller->dma_rx);
> > + return -EINTR;
> > + }
> > +
> > + /*
> > + * ECSPI has a HW issue when works in Target mode, after 64 words
> > + * writtern to TXFIFO, even TXFIFO becomes empty, ECSPI_TXDATA
> keeps
> > + * shift out the last word data, so we have to disable ECSPI when in
> > + * target mode after the transfer completes.
> > + */
> > + if (spi_imx->devtype_data->disable)
> > + spi_imx->devtype_data->disable(spi_imx);
> > }
> >
> > return 0;
> > @@ -1895,10 +1920,16 @@ static int spi_imx_dma_package_transfer(struct
> > spi_imx_data *spi_imx, static int spi_imx_dma_transfer(struct spi_imx_data
> *spi_imx,
> > struct spi_transfer *transfer)
> > {
> > - bool word_delay = transfer->word_delay.value != 0;
> > + bool word_delay = transfer->word_delay.value != 0 &&
> > +!spi_imx->target_mode;
> > int ret;
> > int i;
> >
> > + if (transfer->len > MX53_MAX_TRANSFER_BYTES &&
> spi_imx->target_mode)
> > +{
>
> why only target have len limiation?
>
> Frank
> > + dev_err(spi_imx->dev, "Transaction too big, max size is %d bytes\n",
> > + MX53_MAX_TRANSFER_BYTES);
> > + return -EMSGSIZE;
> > + }
> > +
> > ret = spi_imx_dma_data_prepare(spi_imx, transfer, word_delay);
> > if (ret < 0) {
> > transfer->error |= SPI_TRANS_FAIL_NO_START; @@ -2104,7 +2135,7
> @@
> > static int spi_imx_transfer_one(struct spi_controller *controller,
> > while (spi_imx->devtype_data->rx_available(spi_imx))
> > readl(spi_imx->base + MXC_CSPIRXDATA);
> >
> > - if (spi_imx->target_mode)
> > + if (spi_imx->target_mode && !spi_imx->usedma)
> > return spi_imx_pio_transfer_target(spi, transfer);
> >
> > /*
> > @@ -2116,7 +2147,10 @@ static int spi_imx_transfer_one(struct
> spi_controller *controller,
> > 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);
> > + if (spi_imx->target_mode)
> > + return spi_imx_pio_transfer_target(spi, transfer);
> > + else
> > + return spi_imx_pio_transfer(spi, transfer);
> > }
> > return ret;
> > }
> > --
> > 2.34.1
> >
Powered by blists - more mailing lists