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

[Luca Ellero]

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?

regards

-- 
Luca Ellero

E-mail: luca.ellero at brickedbrain.com
Internet: www.brickedbrain.com