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

[Vijaya Krishna Nivarthi]

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