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

[Carlos Song]

> -----Original Message-----
> From: Marc Kleine-Budde <mkl at pengutronix.de>
> Sent: Wednesday, November 26, 2025 4:28 PM
> To: Carlos Song <carlos.song at nxp.com>
> Cc: broonie at kernel.org; Frank Li <frank.li at nxp.com>; hawnguo at kernel.org;
> s.hauer at pengutronix.de; kernel at pengutronix.de; festevam at gmail.com;
> linux-spi at vger.kernel.org; imx at lists.linux.dev;
> linux-arm-kernel at lists.infradead.org; linux-kernel at vger.kernel.org
> Subject: [EXT] Re: [PATCH 5/6] spi: imx: support dynamic burst length for ECSPI
> DMA mode
> 
> On 25.11.2025 18:06:17, Carlos Song wrote:
> > +static int spi_imx_dma_data_prepare(struct spi_imx_data *spi_imx,
> > +				    struct spi_transfer *transfer,
> > +				    bool word_delay)
> > +{
> > +	u32 pre_bl, tail_bl;
> > +	u32 ctrl;
> > +	int ret;
> > +
> > +	/*
> > +	 * ECSPI supports a maximum burst of 512 bytes. When xfer->len exceeds
> 512
> > +	 * and is not a multiple of 512, a tail transfer is required. In this case,
> > +	 * an extra DMA request is issued for the remaining data.
> > +	 */
> > +	ctrl = readl(spi_imx->base + MX51_ECSPI_CTRL);
> > +	if (word_delay) {
> > +		/*
> > +		 * When SPI IMX need to support word delay, according to "Sample
> Period Control
> > +		 * Register" shows, The Sample Period Control Register
> (ECSPI_PERIODREG)
> > +		 * provides software a way to insert delays (wait states) between
> consecutive
> > +		 * SPI transfers. As a result, ECSPI can only transfer one word per
> frame, and
> > +		 * the delay occurs between frames.
> > +		 */
> > +		spi_imx->dma_package_num = 1;
> > +		pre_bl = spi_imx->bits_per_word - 1;
> > +	} else if (transfer->len <= MX51_ECSPI_CTRL_MAX_BURST) {
> > +		spi_imx->dma_package_num = 1;
> > +		pre_bl = transfer->len * BITS_PER_BYTE - 1;
> > +	} else if (!(transfer->len % MX51_ECSPI_CTRL_MAX_BURST)) {
> > +		spi_imx->dma_package_num = 1;
> > +		pre_bl = MX51_ECSPI_CTRL_MAX_BURST * BITS_PER_BYTE - 1;
> > +	} else {
> > +		spi_imx->dma_package_num = 2;
> > +		pre_bl = MX51_ECSPI_CTRL_MAX_BURST * BITS_PER_BYTE - 1;
> > +		tail_bl = (transfer->len % MX51_ECSPI_CTRL_MAX_BURST) *
> BITS_PER_BYTE - 1;
> > +	}
> > +
> > +	spi_imx->dma_data = kmalloc_array(spi_imx->dma_package_num,
> > +					  sizeof(struct dma_data_package),
> > +					  GFP_KERNEL | __GFP_ZERO);
> > +	if (!spi_imx->dma_data) {
> > +		dev_err(spi_imx->dev, "Failed to allocate DMA package buffer!\n");
> > +		return -ENOMEM;
> > +	}
> > +
> > +	if (spi_imx->dma_package_num == 1) {
> > +		ctrl &= ~MX51_ECSPI_CTRL_BL_MASK;
> > +		ctrl |= pre_bl << MX51_ECSPI_CTRL_BL_OFFSET;
> > +		spi_imx->dma_data[0].cmd_word = ctrl;
> > +		spi_imx->dma_data[0].data_len = transfer->len;
> > +		ret = spi_imx_dma_tx_data_handle(spi_imx, &spi_imx->dma_data[0],
> transfer->tx_buf,
> > +						 word_delay);
> > +		if (ret) {
> > +			kfree(spi_imx->dma_data);
> > +			return ret;
> > +		}
> > +	} else {
> > +		ctrl &= ~MX51_ECSPI_CTRL_BL_MASK;
> > +		ctrl |= pre_bl << MX51_ECSPI_CTRL_BL_OFFSET;
> > +		spi_imx->dma_data[0].cmd_word = ctrl;
> > +		spi_imx->dma_data[0].data_len =
> DIV_ROUND_DOWN_ULL(transfer->len,
> > +								   MX51_ECSPI_CTRL_MAX_BURST)
> > +								   * MX51_ECSPI_CTRL_MAX_BURST;
> 
> What mathematical operation do you want to do? Why do you use a 64 bit div?
> What about round_down()?
> 

I want to round down xfer->len by MX51_ECSPI_CTRL_MAX_BURST to calculate how many MX51_ECSPI_CTRL_MAX_BURST are in xfer->len.
When len =512*3 +511. So after this I can get spi_imx->dma_data[0].data_len = 512*3.
round_down() is enough(u32). Don't need 64 bit.. I can fix it in v2

> > +		ret = spi_imx_dma_tx_data_handle(spi_imx, &spi_imx->dma_data[0],
> transfer->tx_buf,
> > +						 false);
> > +		if (ret) {
> > +			kfree(spi_imx->dma_data);
> > +			return ret;
> > +		}
> > +
> > +		ctrl &= ~MX51_ECSPI_CTRL_BL_MASK;
> > +		ctrl |= tail_bl << MX51_ECSPI_CTRL_BL_OFFSET;
> > +		spi_imx->dma_data[1].cmd_word = ctrl;
> > +		spi_imx->dma_data[1].data_len = transfer->len %
> MX51_ECSPI_CTRL_MAX_BURST;
> > +		ret = spi_imx_dma_tx_data_handle(spi_imx, &spi_imx->dma_data[1],
> > +						 transfer->tx_buf +
> spi_imx->dma_data[0].data_len,
> > +						 false);
> > +		if (ret) {
> > +			kfree(spi_imx->dma_data[0].dma_tx_buf);
> > +			kfree(spi_imx->dma_data[0].dma_rx_buf);
> > +			kfree(spi_imx->dma_data);
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
> 
> regards,
> 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   |