On Thu, Oct 16, 2014 at 09:42:33PM +0530, Vinod Koul wrote: > On Thu, Oct 16, 2014 at 04:10:48PM +0200, Ludovic Desroches wrote: > > > This is pretty odd, I dont see a reason why we can have proper case values > > > and converted ones. It would have made sense if we use this for conversion > > > but in case values in quite puzzling > > > > > > > + case 1: > > > > + csize = AT_XDMAC_CC_CSIZE_CHK_2; > > > and looking at values this can be ffs(maxburst) - 1 for valid cases > > > > Yes I can return ffs(maxburst) - 1 for valid cases. > > > > > > + break; > > > > + case 2: > > > > + csize = AT_XDMAC_CC_CSIZE_CHK_4; > > > > + break; > > > > + case 3: > > > > + csize = AT_XDMAC_CC_CSIZE_CHK_8; > > > > + break; > > > > + case 4: > > > > + csize = AT_XDMAC_CC_CSIZE_CHK_16; > > > > + break; > > > > + default: > > > > + csize = AT_XDMAC_CC_CSIZE_CHK_1; > > > why? > > > > Because some devices don't set maxburst, for example, our serial driver. > Then pls send a patch against that as well. returniung error here will > ensure that they fix that as well Ok, I'll return an error and have a look to fix our serial driver. > > > > > + > > > > + prev = desc; > > > > + if (!first) > > > > + first = desc; > > > > + > > > > + dev_dbg(chan2dev(chan), "%s: add desc 0x%p to descs_list 0x%p\n", > > > > + __func__, desc, first); > > > > + list_add_tail(&desc->desc_node, &first->descs_list); > > > > + } > > > > + > > > > + spin_unlock_bh(&atchan->lock); > > > > + > > > > + first->tx_dma_desc.cookie = -EBUSY; > > > why are you init cookie here > > > > Inspired by other driver. What is the right place so? > You dont. Cookie will be set to correct value when the descriptor is > submitted > > > > For memcpy why should we need slave_config. The system memory source and > > > destination width could be assumed to relastic values and then burst sizes > > > maxed for performance. These values make more sense for periphral where we > > > have to match up with the periphral > > > > I don't tell I need slave_config. We have already talked about this. I don't > > see the problem. It is only a comment, a reminder. The only information > > I may need, one day, is the direction because we have to set src and dst > > interfaces. At the moment, all our products are done in a way nand flash > > and DDR are on the same interface so we don't have to care about > > direction. > > Since we don't have the direction, two solutions: > > - remember this limitation for next products, that's why there is this reminder, > > - change our nand driver in order to see nand as a peripheral instead of > > a memory. > I think treating NAND as memory may not be a right model. It should be > treated as periphral with incrementing and decrementing address value. That > way you should be able to set the right properties for it. > > The system memory copy is right model for memcpy. Ok, I'll discuss of it with the atmel nand maintener. Even if there is something to improve here, I hope it is not considered as a blocking point to get the xdmac driver included into 3.19 because at the moment we have no DMA on the SAMA5D4 recently introduced. Ludovic -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
