On Wed, May 5, 2010 at 5:31 PM, Shilimkar, Santosh <santosh.shilimkar@xxxxxx> wrote: > > >> -----Original Message----- >> From: svenkatr@xxxxxxxxx [mailto:svenkatr@xxxxxxxxx] On Behalf Of Venkatraman S >> Sent: Wednesday, May 05, 2010 5:20 PM >> To: Shilimkar, Santosh >> Cc: linux-omap@xxxxxxxxxxxxxxx; linux-mmc@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; >> Chikkature Rajashekar, Madhusudhan; Adrian Hunter; Tony Lindgren >> Subject: Re: [PATCH v8 1/2] sDMA: descriptor autoloading feature >> >> [Long sections have been trimmed to the context of the discussion] >> On Wed, May 5, 2010 at 3:02 PM, Shilimkar, Santosh >> <santosh.shilimkar@xxxxxx> wrote: >> >> -----Original Message----- >> >> +static int dma_sglist_set_phy_params(struct omap_dma_sglist_node *sghead, >> >> + dma_addr_t phyaddr, int nelem) >> >> +{ >> >> + struct omap_dma_sglist_node *sgcurr, *sgprev; >> >> + dma_addr_t elem_paddr = phyaddr; >> >> + >> >> + for (sgprev = sghead; >> >> + sgprev < sghead + nelem; >> >> + sgprev++) { >> >> + >> >> + sgcurr = sgprev + 1; >> >> + sgprev->next = sgcurr; >> >> + elem_paddr += (int)sizeof(*sgcurr); >> >> + sgprev->next_desc_add_ptr = elem_paddr; >> >> + >> >> + switch (sgcurr->desc_type) { >> >> + case OMAP_DMA_SGLIST_DESCRIPTOR_TYPE1: >> >> + omap_dma_list_set_ntype(sgprev, 1); >> >> + break; >> >> + >> >> + case OMAP_DMA_SGLIST_DESCRIPTOR_TYPE2a: >> >> + /* intentional no break */ >> >> + case OMAP_DMA_SGLIST_DESCRIPTOR_TYPE2b: >> >> + omap_dma_list_set_ntype(sgprev, 2); >> >> + break; >> >> + >> >> + case OMAP_DMA_SGLIST_DESCRIPTOR_TYPE3a: >> >> + /* intentional no break */ >> >> + case OMAP_DMA_SGLIST_DESCRIPTOR_TYPE3b: >> >> + omap_dma_list_set_ntype(sgprev, 3); >> >> + break; >> >> + >> >> + default: >> >> + return -EINVAL; >> >> + >> >> + } >> > Are we supporting all the descriptor types. I think only type2a is >> > supported. In that case please add FIXME, or WARN message here. >> >> From DMA perspective, all are supported - no restrictions. Only I have >> not figured >> out how to use type 2b and type 3b descriptors. It's not the fault of >> DMA driver or >> specification :-) It's actually upto the client to select the right type. > OK. Then the question which I wanted to ask. > For TX, 2b should have been better choice than 2a isn't it? Not much of a difference (as the space allocation is common), but I couldn't get 2b working correctly. Will try that once I get some clarification from hardware team. >> >> >> + >> >> + lcfg->sghead = sgparams; >> >> + lcfg->num_elem = nelem; >> >> + lcfg->sgheadphy = padd; >> >> + lcfg->pausenode = -1; >> >> + >> >> + >> >> + if (NULL == chparams) >> > Minute point really. Better readability "ch_params" >> OK >> >> >> + dma_write(l, CDP(lch)); >> >> + dma_write((lcfg->sgheadphy), CNDP(lch)); >> >> + /** >> >> + * Barrier needed as writes to the >> >> + * descriptor memory needs to be flushed >> >> + * before it's used by DMA controller >> >> + */ >> > Little bit of re-wording if you can. >> > Also you don't wanted the double ** >> > /* >> > * Memory barrier is needed because data may still be >> > * in the write buffer. The barrier drains write buffers and >> > * ensures that DMA sees correct descriptors >> > */ >> OK >> >> >> + wmb(); >> >> + omap_start_dma(lch); >> >> + >> >> + /* Maintain the pause state in descriptor */ >> >> + omap_set_dma_sglist_pausebit(lcfg, lcfg->pausenode, 0); >> >> + omap_set_dma_sglist_pausebit(lcfg, pauseafter, 1); >> >> + >> >> + /** >> >> + * Barrier needed as writes to the >> >> + * descriptor memory needs to be flushed >> >> + * before it's used by DMA controller >> >> + */ >> > Description change if possible >> OK >> >> >> + wmb(); >> >> + >> >> + /* Errata i557 - pausebit should be cleared in no standby mode */ >> > This should have been >> >> I couldn't understand this comment. > This should have been a separate patch since it's an errata. OK. >> >> >> + sys_cf = dma_read(OCP_SYSCONFIG); >> >> + l = sys_cf; >> >> + /* Middle mode reg set no Standby */ >> >> + l &= ~(BIT(12) | BIT(13)); >> >> + dma_write(l, OCP_SYSCONFIG); > -- > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
