Re: [PATCH v4] mtd: sunxi-nand: add mdma support for allwinner h3

[Boris Brezillon <boris.brezillon@collabora.com>]

On Thu, 8 Oct 2020 16:51:50 +0200 (CEST)
Manuel Dipolt <mdipolt@robart.cc> wrote:

> > >  				       int page)
> > >  {
> > >  	struct mtd_info *mtd = nand_to_mtd(nand);
> > > @@ -1277,6 +1338,8 @@ static int sunxi_nfc_hw_ecc_write_page_dma(struct nand_chip *nand,
> > >  	struct nand_ecc_ctrl *ecc = &nand->ecc;
> > >  	struct scatterlist sg;
> > >  	int ret, i;
> > > +	u32 wait_event_flags;  
> > 
> > Or just "wait".
> >   
> 
> 
> removed variable cause waiting again only for nfc interrupt

I think you should wait for both.

> 
> 
> > > +	__u32 mem_addr;
> > >  
> > >  	sunxi_nfc_select_chip(nand, nand->cur_cs);
> > >  
> > > @@ -1284,10 +1347,14 @@ static int sunxi_nfc_hw_ecc_write_page_dma(struct nand_chip *nand,
> > >  	if (ret)
> > >  		return ret;
> > >  
> > > -	ret = sunxi_nfc_dma_op_prepare(nfc, buf, ecc->size, ecc->steps,
> > > -				       DMA_TO_DEVICE, &sg);
> > > +	if (nfc->caps->has_mdma)
> > > +		ret = sunxi_nfc_mdma_op_prepare(nfc, buf, ecc->size, ecc->steps,
> > > +						DMA_TO_DEVICE, &mem_addr);
> > > +	else
> > > +		ret = sunxi_nfc_dma_op_prepare(nfc, buf, ecc->size, ecc->steps, DMA_TO_DEVICE, &sg);
> > > +
> > >  	if (ret)
> > > -		goto pio_fallback;
> > > +		return ret;
> > >  
> > >  	for (i = 0; i < ecc->steps; i++) {
> > >  		const u8 *oob = nand->oob_poi + (i * (ecc->bytes + 4));
> > > @@ -1304,20 +1371,28 @@ static int sunxi_nfc_hw_ecc_write_page_dma(struct nand_chip *nand,
> > >  	writel((NAND_CMD_RNDIN << 8) | NAND_CMD_PAGEPROG,
> > >  	       nfc->regs + NFC_REG_WCMD_SET);
> > >  
> > > -	dma_async_issue_pending(nfc->dmac);
> > > +	wait_event_flags = NFC_CMD_INT_FLAG;
> > > +
> > > +	if (nfc->caps->has_mdma)
> > > +		wait_event_flags = NFC_DMA_INT_FLAG;  
> > 
> > Are you sure you don't need the NFC_CMD_INT_FLAG flag in that case? I'm
> > pretty sure the DMA transfer is done before the PAGEPROG command is
> > issued, meaning that you might be queuing new operations before the
> > operation is actually finished.  
> 
> 
> can never be sure without any proper documentation about this nand controller ):
> was guessing cause i saw dma int triggered,
> tested again always both flags were set, so changed it to NFC_CMD_INT_FLAG to be on safe side

Actually, if you want to be on safe side, you should probably set both
(and that applies to the read path as well, unless no commands are
issued in that case).


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/