From mboxrd@z Thu Jan 1 00:00:00 1970 From: svenkatr@ti.com (Venkatraman S) Date: Wed, 5 May 2010 21:55:48 +0530 Subject: [PATCH v8 1/2] sDMA: descriptor autoloading feature In-Reply-To: References: Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Wed, May 5, 2010 at 5:31 PM, Shilimkar, Santosh wrote: > > >> -----Original Message----- >> From: svenkatr at gmail.com [mailto:svenkatr at gmail.com] On Behalf Of Venkatraman S >> Sent: Wednesday, May 05, 2010 5:20 PM >> To: Shilimkar, Santosh >> Cc: linux-omap at vger.kernel.org; linux-mmc at vger.kernel.org; linux-arm-kernel at lists.infradead.org; >> 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 >> 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 at vger.kernel.org > More majordomo info at ?http://vger.kernel.org/majordomo-info.html >