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. > > > + 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... > > > + 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? > > > +#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.
Attachment:
signature.asc
Description: PGP signature
