On Tuesday, December 16, 2014 at 09:06:34 AM, Luca Ellero wrote: > On 15/12/2014 12:14, Marek Vasut wrote: > > On Monday, December 15, 2014 at 09:45:13 AM, Luca Ellero wrote: > >> Hi Marek, > >> > >> On 13/12/2014 14:12, Marek Vasut wrote: > >>> 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 ? > >> > >> Scheduling the patch for -next it's ok for me. > >> However there are other two points where pio_words number doesn't > >> reflect the pio_words really initiated, one is in mxs_nand_read_buf() > >> and one is in mxs_nand_write_buf(). Each one declares 4 pio_words but > >> only one is initiated. > >> I wonder what we should do in this cases. > > > > You can fix those as well. I recall that all this goop came from the > > original (2.6.35) GPMI NAND driver, which is likely where all those bugs > > came from as well. > > > > Thank you! > > OK, I will send a patch to fix them.
Thanks! _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
