Messages in this thread | | | Date | Wed, 12 Apr 2023 20:59:06 +0530 | Subject | Re: [PATCH 2/2] spi: spi-qcom-qspi: Add DMA mode support | From | 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
| |