Re: [PATCH 2/2] spi: spi-qcom-qspi: Add DMA mode support

From: Vijaya Krishna Nivarthi
Date: Wed Apr 12 2023 - 11:29:45 EST


Thank you for the responses...


On 4/6/2023 8:58 PM, Mark Brown wrote:
On Thu, Apr 06, 2023 at 08:23:21PM +0530, Vijaya Krishna Nivarthi wrote:
On 4/4/2023 11:47 PM, Mark Brown wrote:
On Tue, Apr 04, 2023 at 11:33:20PM +0530, Vijaya Krishna Nivarthi wrote:
+ uint32_t reserved2:7;
+ uint32_t length:16;
+ //------------------------//
What does this mean?
That separates the part of cmd_desc that is visible to the HW and the part
that is required by the SW after xfer is complete.
I can add a comment in v2?
Yes, please.


Added comments


+ for (ii = 0; ii < sgt->nents; ii++)
+ sg_total_len += sg_dma_len(sgt->sgl + ii);
Why are we calling the iterator ii here?
Calling it ii helps in finding iterator more easily in code.
should I stick to i in v2?
Given that multiple people queried this...


Renamed.


+ if (ctrl->xfer.dir == QSPI_READ)
+ byte_ptr = (uint8_t *)xfer->rx_buf;
+ else
+ byte_ptr = (uint8_t *)xfer->tx_buf;
If we need to cast to or from void * there's some sort of problem.
the tx_buf is a const void*
in v2 I will cast for tx_buf only?
Or just keep byte_ptr as const - we're not modifying it are we?


We are modifying it, hence did cast for tx_buf only


+#if NO_TX_DATA_DELAY_FOR_DMA
+ mstr_cfg &= ~(TX_DATA_OE_DELAY_MSK | TX_DATA_DELAY_MSK);
+#endif
Why would we add extra delays if we don't need them, might someone set
this and if so when?
I believe its used when some slave devices need a delay between clock and
data.
Its configured as 1 for PIO_MODE(FIFO) right now.
For DMA_MODE we are not using same, both seem to work for DMA.
If some devices need this to be configured it needs to be configured
either from the driver for that device or via DT depending on the exact
requirements.


Retained same MSTR_CONFIG as PIO mode, hence not touching any of the DELAYs