From mboxrd@z Thu Jan 1 00:00:00 1970 From: Luca Ellero Date: Tue, 16 Dec 2014 09:06:34 +0100 Subject: [U-Boot] [PATCH] mtd: nand: mxs: fix PIO_WORD number In-Reply-To: <201412151214.16493.marex@denx.de> References: <1418199884-2430-1-git-send-email-luca.ellero@brickedbrain.com> <201412131412.43236.marex@denx.de> <548E9F99.3070606@brickedbrain.com> <201412151214.16493.marex@denx.de> Message-ID: <548FE80A.1040005@brickedbrain.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 >>>>>>>>> --- >>>>>>>>> >>>>>>>>> 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 >>>>> >>>>> 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. Regards -- Luca Ellero E-mail: luca.ellero at brickedbrain.com Internet: www.brickedbrain.com