On Friday, December 12, 2014 at 04:03:14 PM, Luca Ellero wrote: > Hi Marek, > > On 12/12/2014 13:58, Marek Vasut wrote: > > On Friday, December 12, 2014 at 01:43:22 PM, Stefan Roese wrote: > >> Hi Luca, > >> > >> On 12.12.2014 13:40, Luca Ellero wrote: > >>>> On 10.12.2014 09:24, Luca Ellero wrote: > >>>>> There is only one pio_word in this DMA transaction so data field must > >>>>> be 1. > >>>>> > >>>>> Signed-off-by: Luca Ellero <luca.ell...@brickedbrain.com> > >>>>> --- > >>>>> > >>>>> drivers/mtd/nand/mxs_nand.c | 2 +- > >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>>>> > >>>>> diff --git a/drivers/mtd/nand/mxs_nand.c > >>>>> b/drivers/mtd/nand/mxs_nand.c index 7a064ab..616c9ca 100644 > >>>>> --- a/drivers/mtd/nand/mxs_nand.c > >>>>> +++ b/drivers/mtd/nand/mxs_nand.c > >>>>> @@ -305,7 +305,7 @@ static void mxs_nand_cmd_ctrl(struct mtd_info > >>>>> *mtd, int data, unsigned int ctrl) > >>>>> > >>>>> d->cmd.data = > >>>>> > >>>>> MXS_DMA_DESC_COMMAND_DMA_READ | MXS_DMA_DESC_IRQ | > >>>>> MXS_DMA_DESC_CHAIN | MXS_DMA_DESC_DEC_SEM | > >>>>> > >>>>> - MXS_DMA_DESC_WAIT4END | (3 << MXS_DMA_DESC_PIO_WORDS_OFFSET) > >>>>> | + MXS_DMA_DESC_WAIT4END | (1 << > >>>>> MXS_DMA_DESC_PIO_WORDS_OFFSET) | > >>>>> > >>>>> (nand_info->cmd_queue_len << MXS_DMA_DESC_BYTES_OFFSET); > >>>> > >>>> What error or problem does this incorrect setup cause in your case? > >>>> I'm asking since I'm also using this driver in some mx6 system and > >>>> have not seen any issues. > >>> > >>> As far as I can see, it doesn't seem to cause any issue. But, if you > >>> read the iMX6 Reference Manual (chapter 14.2) this field should reflect > >>> the number of PIO_WORDS appended to the DMA command, in this case 1. > >> > >> Okay. I just wanted to check if this patch fixes a real problem that you > >> have experienced. Thanks for the explanation. > >> > >> Reviewed-by: Stefan Roese <s...@denx.de> > > > > The patch does in fact change the behavior such that it no longer clears > > the ECCCTRL and COMPARE registers both on MX28 and on MX6 . Could this > > have some impact? > > I'm not sure. The manual doesn't tell much about it. Anyway if we want > to clear COMPARE and ECCCTRL register, we should at least ensure that > pio_words 1 and 2 are 0 before executing the DMA chain. > > Something like this: > > d->cmd.pio_words[1] = 0; > d->cmd.pio_words[2] = 0; > > What do you think?
I believe the descriptor is zeroed out in mxs_nand_return_dma_descs(), though I admit depending on such behavior is pretty iffy. The question is, does your patch introduce a side-effect ? My proposal would be to schedule the patch for -next and see what happens. I believe the patch would be just fine and won't break anything. What do you think ? Best regards, Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
