[U-Boot] [PATCH] mtd: nand: mxs: fix PIO_WORD number

[Marek Vasut]

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.ellero at 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 <sr at 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