[21/39] mtd: nand: denali: support 64bit capable DMA engine

The current driver only supports the DMA engine up to 32 bit
physical address, but there also exists 64 bit capable DMA engine
for this IP.

The data DMA setup sequence is completely different, so I added the
64 bit DMA code as a new function denali_setup_dma64().  The 32 bit
one has been renamed to denali_setup_dma32().

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

---

 drivers/mtd/nand/denali.c | 39 +++++++++++++++++++++++++++++++++++----
 drivers/mtd/nand/denali.h |  1 +
 2 files changed, 36 insertions(+), 4 deletions(-)

-- 
2.7.4

On Sun, 27 Nov 2016 03:06:07 +0900
Masahiro Yamada <yamada.masahiro@socionext.com> wrote:

> The current driver only supports the DMA engine up to 32 bit

> physical address, but there also exists 64 bit capable DMA engine

> for this IP.

> 

> The data DMA setup sequence is completely different, so I added the

> 64 bit DMA code as a new function denali_setup_dma64().  The 32 bit

> one has been renamed to denali_setup_dma32().

> 

> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

> ---

> 

>  drivers/mtd/nand/denali.c | 39 +++++++++++++++++++++++++++++++++++----

>  drivers/mtd/nand/denali.h |  1 +

>  2 files changed, 36 insertions(+), 4 deletions(-)

> 

> diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c

> index ae44c01..752ad98 100644

> --- a/drivers/mtd/nand/denali.c

> +++ b/drivers/mtd/nand/denali.c

> @@ -995,8 +995,30 @@ static void denali_enable_dma(struct denali_nand_info *denali, bool en)

>  	ioread32(denali->flash_reg + DMA_ENABLE);

>  }

>  

> -/* setups the HW to perform the data DMA */

> -static void denali_setup_dma(struct denali_nand_info *denali, int op)

> +static void denali_setup_dma64(struct denali_nand_info *denali, int op)

> +{

> +	u32 mode;

> +	const int page_count = 1;

> +	u64 addr = denali->buf.dma_buf;

> +

> +	mode = MODE_10 | BANK(denali->flash_bank) | denali->page;

> +

> +	/* DMA is a three step process */

> +

> +	/*

> +	 * 1. setup transfer type, interrupt when complete,

> +	 *    burst len = 64 bytes, the number of pages

> +	 */

> +	index_addr(denali, mode, 0x01002000 | (64 << 16) | op | page_count);

> +

> +	/* 2. set memory low address */

> +	index_addr(denali, mode, addr);

> +

> +	/* 3. set memory high address */

> +	index_addr(denali, mode, addr >> 32);

> +}

> +

> +static void denali_setup_dma32(struct denali_nand_info *denali, int op)

>  {

>  	u32 mode;

>  	const int page_count = 1;

> @@ -1019,6 +1041,14 @@ static void denali_setup_dma(struct denali_nand_info *denali, int op)

>  	index_addr(denali, mode | 0x14000, 0x2400);

>  }

>  

> +static void denali_setup_dma(struct denali_nand_info *denali, int op)

> +{

> +	if (denali->caps & DENALI_CAPS_DMA_64BIT)

> +		denali_setup_dma64(denali, op);

> +	else

> +		denali_setup_dma32(denali, op);

> +}

> +


If you follow my suggestions of definition function pointers, you'll
just have to add a function pointer to the denali_caps struct:

	void (*setup_dma)(struct denali_nand_info *denali, int op);


>  /*

>   * writes a page. user specifies type, and this function handles the

>   * configuration details.

> @@ -1495,8 +1525,9 @@ int denali_init(struct denali_nand_info *denali)

>  		goto failed_req_irq;

>  	}

>  

> -	/* Is 32-bit DMA supported? */

> -	ret = dma_set_mask(denali->dev, DMA_BIT_MASK(32));

> +	ret = dma_set_mask(denali->dev,

> +			   DMA_BIT_MASK(denali->caps & DENALI_CAPS_DMA_64BIT ?

> +					64 : 32));

>  	if (ret) {

>  		dev_err(denali->dev, "no usable DMA configuration\n");

>  		goto failed_req_irq;

> diff --git a/drivers/mtd/nand/denali.h b/drivers/mtd/nand/denali.h

> index beadc8a..9bdf037 100644

> --- a/drivers/mtd/nand/denali.h

> +++ b/drivers/mtd/nand/denali.h

> @@ -435,6 +435,7 @@ struct denali_nand_info {

>  	u32 max_banks;

>  	unsigned int caps;

>  #define DENALI_CAPS_HW_ECC_FIXUP		BIT(0)

> +#define DENALI_CAPS_DMA_64BIT			BIT(1)

>  };

>  

>  extern int denali_init(struct denali_nand_info *denali);

Hi Boris,

2016-11-28 1:16 GMT+09:00 Boris Brezillon <boris.brezillon@free-electrons.com>:
>

> If you follow my suggestions of definition function pointers, you'll

> just have to add a function pointer to the denali_caps struct:

>

>         void (*setup_dma)(struct denali_nand_info *denali, int op);

>



This is possible because both 32B and 64B variants have
the same function prototype.


However, I am not keen on doing so for ecc_handle(),
so, at this moment, I am not keen on doing so only for setup_dma() either.

If I see more cases that fit in callback handler approach,
I will think about switching to it with consistency.

-- 
Best Regards
Masahiro Yamada

On Wed, 30 Nov 2016 16:11:53 +0900
Masahiro Yamada <yamada.masahiro@socionext.com> wrote:

> Hi Boris,

> 

> 2016-11-28 1:16 GMT+09:00 Boris Brezillon <boris.brezillon@free-electrons.com>:

> >

> > If you follow my suggestions of definition function pointers, you'll

> > just have to add a function pointer to the denali_caps struct:

> >

> >         void (*setup_dma)(struct denali_nand_info *denali, int op);

> >  

> 

> 

> This is possible because both 32B and 64B variants have

> the same function prototype.

> 

> 

> However, I am not keen on doing so for ecc_handle(),

> so, at this moment, I am not keen on doing so only for setup_dma() either.

> 

> If I see more cases that fit in callback handler approach,

> I will think about switching to it with consistency.

> 


Pretty much all the flags you define in this series could be turned
into function pointers. I still think the function pointers approach is
more future-proof, but I won't fight on that, so do as you wish.