diff mbox

[v1,2/5] mtd: atmel_nand: Support variable RB_EDGE interrupts

Message ID 1452702857-2240-3-git-send-email-romain.izard.pro@gmail.com
State Superseded
Headers show

Commit Message

Romain Izard Jan. 13, 2016, 4:34 p.m. UTC
The NFC controller used to accelerate the NAND transfers on SAMA5 chips
can use either RB_EDGE0 or RB_EDGE3 as its ready/busy interrupt bit.

Use the controller's compatible string to select the correct bit.

Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
Reviewed-by: Wenyou Yang <Wenyou.yang@atmel.com>
---
 .../devicetree/bindings/mtd/atmel-nand.txt         |  2 +-
 drivers/mtd/nand/atmel_nand.c                      | 39 +++++++++++++++++-----
 drivers/mtd/nand/atmel_nand_nfc.h                  |  5 ++-
 3 files changed, 36 insertions(+), 10 deletions(-)

Comments

Boris Brezillon Jan. 13, 2016, 6:14 p.m. UTC | #1
Hi Romain,

On Wed, 13 Jan 2016 17:34:14 +0100
Romain Izard <romain.izard.pro@gmail.com> wrote:

> The NFC controller used to accelerate the NAND transfers on SAMA5 chips
> can use either RB_EDGE0 or RB_EDGE3 as its ready/busy interrupt bit.
> 
> Use the controller's compatible string to select the correct bit.
> 
> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
> Reviewed-by: Wenyou Yang <Wenyou.yang@atmel.com>
> ---
>  .../devicetree/bindings/mtd/atmel-nand.txt         |  2 +-
>  drivers/mtd/nand/atmel_nand.c                      | 39 +++++++++++++++++-----
>  drivers/mtd/nand/atmel_nand_nfc.h                  |  5 ++-
>  3 files changed, 36 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/atmel-nand.txt b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> index 7d4c8eb775a5..89b0db9801b0 100644
> --- a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> @@ -34,7 +34,7 @@ Optional properties:
>  - nand-on-flash-bbt: boolean to enable on flash bbt option if not present false
>  - Nand Flash Controller(NFC) is a slave driver under Atmel nand flash
>    - Required properties:
> -    - compatible : "atmel,sama5d3-nfc".
> +    - compatible : "atmel,sama5d3-nfc" or "atmel,sama5d4-nfc".
>      - reg : should specify the address and size used for NFC command registers,
>              NFC registers and NFC Sram. NFC Sram address and size can be absent
>              if don't want to use it.
> diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
> index 9d71f9e6a8de..e5d7e7e63f49 100644
> --- a/drivers/mtd/nand/atmel_nand.c
> +++ b/drivers/mtd/nand/atmel_nand.c
> @@ -67,6 +67,10 @@ struct atmel_nand_caps {
>  	bool pmecc_correct_erase_page;
>  };
>  
> +struct atmel_nand_nfc_priv {

Can you rename this struct into atmel_nfc_caps to be consistant with the
atmel_nand_caps?

> +	uint32_t rb_edge;

Actually, AFAIU, it's not encoding the type of edges, but the available
native R/B lines (by native I mean the R/B lines directly connected to
the NFC IP).
I suggest renaming this field avail_rb_lines, and making it a bitfield
(one bit per possible R/B line).

> +};
> +
>  /* oob layout for large page size
>   * bad block info is on bytes 0 and 1
>   * the bytes have to be consecutives to avoid
> @@ -111,6 +115,7 @@ struct atmel_nfc {
>  	/* Point to the sram bank which include readed data via NFC */
>  	void			*data_in_sram;
>  	bool			will_write_sram;
> +	uint32_t		rb_edge;

Replace this by
	const struct atmel_nfc_caps *caps;

so that next time you have a per-SoC particularity, you can just add it
to your struct atmel_nfc_caps without adding new fields to atmel_nfc.

>  };
>  static struct atmel_nfc	nand_nfc;
>  
> @@ -1675,9 +1680,9 @@ static irqreturn_t hsmc_interrupt(int irq, void *dev_id)
>  		nfc_writel(host->nfc->hsmc_regs, IDR, NFC_SR_XFR_DONE);
>  		ret = IRQ_HANDLED;
>  	}
> -	if (pending & NFC_SR_RB_EDGE) {
> +	if (pending & host->nfc->rb_edge) {
>  		complete(&host->nfc->comp_ready);
> -		nfc_writel(host->nfc->hsmc_regs, IDR, NFC_SR_RB_EDGE);
> +		nfc_writel(host->nfc->hsmc_regs, IDR, host->nfc->rb_edge);

How about creating a macro that would generate the appropriate bitmask
for you?

Something like

#define NFC_SR_RB_EDGE_MASK(x)	((x) << 24)

>  		ret = IRQ_HANDLED;
>  	}
>  	if (pending & NFC_SR_CMD_DONE) {
> @@ -1695,7 +1700,7 @@ static void nfc_prepare_interrupt(struct atmel_nand_host *host, u32 flag)
>  	if (flag & NFC_SR_XFR_DONE)
>  		init_completion(&host->nfc->comp_xfer_done);
>  
> -	if (flag & NFC_SR_RB_EDGE)
> +	if (flag & host->nfc->rb_edge)
>  		init_completion(&host->nfc->comp_ready);
>  
>  	if (flag & NFC_SR_CMD_DONE)
> @@ -1713,7 +1718,7 @@ static int nfc_wait_interrupt(struct atmel_nand_host *host, u32 flag)
>  	if (flag & NFC_SR_XFR_DONE)
>  		comp[index++] = &host->nfc->comp_xfer_done;
>  
> -	if (flag & NFC_SR_RB_EDGE)
> +	if (flag & host->nfc->rb_edge)
>  		comp[index++] = &host->nfc->comp_ready;
>  
>  	if (flag & NFC_SR_CMD_DONE)
> @@ -1781,7 +1786,7 @@ static int nfc_device_ready(struct mtd_info *mtd)
>  		dev_err(host->dev, "Lost the interrupt flags: 0x%08x\n",
>  				mask & status);
>  
> -	return status & NFC_SR_RB_EDGE;
> +	return status & host->nfc->rb_edge;
>  }
>  
>  static void nfc_select_chip(struct mtd_info *mtd, int chip)
> @@ -1954,8 +1959,8 @@ static void nfc_nand_command(struct mtd_info *mtd, unsigned int command,
>  		}
>  		/* fall through */
>  	default:
> -		nfc_prepare_interrupt(host, NFC_SR_RB_EDGE);
> -		nfc_wait_interrupt(host, NFC_SR_RB_EDGE);
> +		nfc_prepare_interrupt(host, host->nfc->rb_edge);
> +		nfc_wait_interrupt(host, host->nfc->rb_edge);
>  	}
>  }
>  
> @@ -2318,9 +2323,12 @@ static const struct of_device_id atmel_nand_dt_ids[] = {
>  
>  MODULE_DEVICE_TABLE(of, atmel_nand_dt_ids);
>  
> +static const struct of_device_id atmel_nand_nfc_match[];
> +
>  static int atmel_nand_nfc_probe(struct platform_device *pdev)
>  {
>  	struct atmel_nfc *nfc = &nand_nfc;
> +	const struct atmel_nand_nfc_priv *priv;
>  	struct resource *nfc_cmd_regs, *nfc_hsmc_regs, *nfc_sram;
>  	int ret;
>  
> @@ -2352,6 +2360,12 @@ static int atmel_nand_nfc_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> +	priv = of_match_device(atmel_nand_nfc_match, &pdev->dev)->data;
> +	if (NULL == priv)
> +		return -ENODEV;
> +
> +	nfc->rb_edge = priv->rb_edge;
> +
>  	nfc_writel(nfc->hsmc_regs, IDR, 0xffffffff);
>  	nfc_readl(nfc->hsmc_regs, SR);	/* clear the NFC_SR */
>  
> @@ -2380,8 +2394,17 @@ static int atmel_nand_nfc_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +static struct atmel_nand_nfc_priv sama5d3_nfc_priv = {
> +	.rb_edge = NFC_SR_RB_EDGE0,
> +};
> +
> +static struct atmel_nand_nfc_priv sama5d4_nfc_priv = {
> +	.rb_edge = NFC_SR_RB_EDGE3,
> +};
> +
>  static const struct of_device_id atmel_nand_nfc_match[] = {
> -	{ .compatible = "atmel,sama5d3-nfc" },
> +	{ .compatible = "atmel,sama5d3-nfc", .data = &sama5d3_nfc_priv },
> +	{ .compatible = "atmel,sama5d4-nfc", .data = &sama5d4_nfc_priv },
>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, atmel_nand_nfc_match);
> diff --git a/drivers/mtd/nand/atmel_nand_nfc.h b/drivers/mtd/nand/atmel_nand_nfc.h
> index 4d5d26221a7e..2cd9c57b1e53 100644
> --- a/drivers/mtd/nand/atmel_nand_nfc.h
> +++ b/drivers/mtd/nand/atmel_nand_nfc.h
> @@ -42,7 +42,10 @@
>  #define		NFC_SR_UNDEF		(1 << 21)
>  #define		NFC_SR_AWB		(1 << 22)
>  #define		NFC_SR_ASE		(1 << 23)
> -#define		NFC_SR_RB_EDGE		(1 << 24)
> +#define		NFC_SR_RB_EDGE0		(1 << 24)
> +#define		NFC_SR_RB_EDGE1		(1 << 25)
> +#define		NFC_SR_RB_EDGE2		(1 << 26)
> +#define		NFC_SR_RB_EDGE3		(1 << 27)

Please replace those 4 definitions by:

#define			NFC_SR_RB_EDGE(x)	BIT(x + 24)

>  
>  #define ATMEL_HSMC_NFC_IER	0x0c
>  #define ATMEL_HSMC_NFC_IDR	0x10

Best Regards,

Boris
Rob Herring Jan. 14, 2016, 1:19 a.m. UTC | #2
On Wed, Jan 13, 2016 at 05:34:14PM +0100, Romain Izard wrote:
> The NFC controller used to accelerate the NAND transfers on SAMA5 chips
> can use either RB_EDGE0 or RB_EDGE3 as its ready/busy interrupt bit.
> 
> Use the controller's compatible string to select the correct bit.
> 
> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
> Reviewed-by: Wenyou Yang <Wenyou.yang@atmel.com>
> ---
>  .../devicetree/bindings/mtd/atmel-nand.txt         |  2 +-
>  drivers/mtd/nand/atmel_nand.c                      | 39 +++++++++++++++++-----
>  drivers/mtd/nand/atmel_nand_nfc.h                  |  5 ++-
>  3 files changed, 36 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/atmel-nand.txt b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> index 7d4c8eb775a5..89b0db9801b0 100644
> --- a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> @@ -34,7 +34,7 @@ Optional properties:
>  - nand-on-flash-bbt: boolean to enable on flash bbt option if not present false
>  - Nand Flash Controller(NFC) is a slave driver under Atmel nand flash
>    - Required properties:
> -    - compatible : "atmel,sama5d3-nfc".
> +    - compatible : "atmel,sama5d3-nfc" or "atmel,sama5d4-nfc".
>      - reg : should specify the address and size used for NFC command registers,
>              NFC registers and NFC Sram. NFC Sram address and size can be absent
>              if don't want to use it.

For the binding: 

Acked-by: Rob Herring <robh@kernel.org>

[...]

> @@ -2352,6 +2360,12 @@ static int atmel_nand_nfc_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> +	priv = of_match_device(atmel_nand_nfc_match, &pdev->dev)->data;

Use of_device_get_match_data

> +	if (NULL == priv)
> +		return -ENODEV;
> +
> +	nfc->rb_edge = priv->rb_edge;
> +
>  	nfc_writel(nfc->hsmc_regs, IDR, 0xffffffff);
>  	nfc_readl(nfc->hsmc_regs, SR);	/* clear the NFC_SR */
>
Romain Izard Jan. 14, 2016, 10:16 a.m. UTC | #3
Hi Boris,

2016-01-13 19:14 GMT+01:00 Boris Brezillon <boris.brezillon@free-electrons.com>:
>> diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
>> index 9d71f9e6a8de..e5d7e7e63f49 100644
>> --- a/drivers/mtd/nand/atmel_nand.c
>> +++ b/drivers/mtd/nand/atmel_nand.c
>> @@ -67,6 +67,10 @@ struct atmel_nand_caps {
>>       bool pmecc_correct_erase_page;
>>  };
>>
>> +struct atmel_nand_nfc_priv {
>
> Can you rename this struct into atmel_nfc_caps to be consistant with the
> atmel_nand_caps?
>
>> +     uint32_t rb_edge;
>
> Actually, AFAIU, it's not encoding the type of edges, but the available
> native R/B lines (by native I mean the R/B lines directly connected to
> the NFC IP).
> I suggest renaming this field avail_rb_lines, and making it a bitfield
> (one bit per possible R/B line).
>

It's already a bitfield in the end: it's the mask for the interrupt status
register. It might have been better if it were called rb_edge_mask.

>> +};
>> +
>>  /* oob layout for large page size
>>   * bad block info is on bytes 0 and 1
>>   * the bytes have to be consecutives to avoid
>> @@ -111,6 +115,7 @@ struct atmel_nfc {
>>       /* Point to the sram bank which include readed data via NFC */
>>       void                    *data_in_sram;
>>       bool                    will_write_sram;
>> +     uint32_t                rb_edge;
>
> Replace this by
>         const struct atmel_nfc_caps *caps;
>
> so that next time you have a per-SoC particularity, you can just add it
> to your struct atmel_nfc_caps without adding new fields to atmel_nfc.
>

I'm reluctant about this, but I will do it. For me, we are exchanging
a single probe-time copy to indirections in each interrupt handler for some
unspecified future user. Let's hope the tradeoff is worth it.

>>  };
>>  static struct atmel_nfc      nand_nfc;
>>
>> @@ -1675,9 +1680,9 @@ static irqreturn_t hsmc_interrupt(int irq, void *dev_id)
>>               nfc_writel(host->nfc->hsmc_regs, IDR, NFC_SR_XFR_DONE);
>>               ret = IRQ_HANDLED;
>>       }
>> -     if (pending & NFC_SR_RB_EDGE) {
>> +     if (pending & host->nfc->rb_edge) {
>>               complete(&host->nfc->comp_ready);
>> -             nfc_writel(host->nfc->hsmc_regs, IDR, NFC_SR_RB_EDGE);
>> +             nfc_writel(host->nfc->hsmc_regs, IDR, host->nfc->rb_edge);
>
> How about creating a macro that would generate the appropriate bitmask
> for you?
>
> Something like
>
> #define NFC_SR_RB_EDGE_MASK(x)  ((x) << 24)
>

It's already so verbose, it will make everything longer. Once the member name
contains the 'mask' part, all the information will be there.


>> diff --git a/drivers/mtd/nand/atmel_nand_nfc.h b/drivers/mtd/nand/atmel_nand_nfc.h
>> index 4d5d26221a7e..2cd9c57b1e53 100644
>> --- a/drivers/mtd/nand/atmel_nand_nfc.h
>> +++ b/drivers/mtd/nand/atmel_nand_nfc.h
>> @@ -42,7 +42,10 @@
>>  #define              NFC_SR_UNDEF            (1 << 21)
>>  #define              NFC_SR_AWB              (1 << 22)
>>  #define              NFC_SR_ASE              (1 << 23)
>> -#define              NFC_SR_RB_EDGE          (1 << 24)
>> +#define              NFC_SR_RB_EDGE0         (1 << 24)
>> +#define              NFC_SR_RB_EDGE1         (1 << 25)
>> +#define              NFC_SR_RB_EDGE2         (1 << 26)
>> +#define              NFC_SR_RB_EDGE3         (1 << 27)
>
> Please replace those 4 definitions by:
>
> #define                 NFC_SR_RB_EDGE(x)       BIT(x + 24)
>

The whole file could be rewritten in this form, but I see this as a
zero-value proposition. I believe it is best to keep with local
consistency. In fact, I will rather remove RB_EDGE1 & RB_EDGE2, as
those are not mentioned in the datasheets.


Best regards,
Boris Brezillon Jan. 14, 2016, 10:41 a.m. UTC | #4
On Thu, 14 Jan 2016 11:16:19 +0100
Romain Izard <romain.izard.pro@gmail.com> wrote:

> Hi Boris,
> 
> 2016-01-13 19:14 GMT+01:00 Boris Brezillon <boris.brezillon@free-electrons.com>:
> >> diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
> >> index 9d71f9e6a8de..e5d7e7e63f49 100644
> >> --- a/drivers/mtd/nand/atmel_nand.c
> >> +++ b/drivers/mtd/nand/atmel_nand.c
> >> @@ -67,6 +67,10 @@ struct atmel_nand_caps {
> >>       bool pmecc_correct_erase_page;
> >>  };
> >>
> >> +struct atmel_nand_nfc_priv {
> >
> > Can you rename this struct into atmel_nfc_caps to be consistant with the
> > atmel_nand_caps?
> >
> >> +     uint32_t rb_edge;
> >
> > Actually, AFAIU, it's not encoding the type of edges, but the available
> > native R/B lines (by native I mean the R/B lines directly connected to
> > the NFC IP).
> > I suggest renaming this field avail_rb_lines, and making it a bitfield
> > (one bit per possible R/B line).
> >
> 
> It's already a bitfield in the end: it's the mask for the interrupt status
> register. It might have been better if it were called rb_edge_mask.

Actually the naming chosen by atmel is a bit misleading, cause the
R/B edge selection is done through the RB_RISE/FALL fields,
and what they call RB_EDGEX is just the R/B line selection, hence the
suggestion to name this field avail_rb_lines.

> 
> >> +};
> >> +
> >>  /* oob layout for large page size
> >>   * bad block info is on bytes 0 and 1
> >>   * the bytes have to be consecutives to avoid
> >> @@ -111,6 +115,7 @@ struct atmel_nfc {
> >>       /* Point to the sram bank which include readed data via NFC */
> >>       void                    *data_in_sram;
> >>       bool                    will_write_sram;
> >> +     uint32_t                rb_edge;
> >
> > Replace this by
> >         const struct atmel_nfc_caps *caps;
> >
> > so that next time you have a per-SoC particularity, you can just add it
> > to your struct atmel_nfc_caps without adding new fields to atmel_nfc.
> >
> 
> I'm reluctant about this, but I will do it. For me, we are exchanging
> a single probe-time copy to indirections in each interrupt handler for some
> unspecified future user. Let's hope the tradeoff is worth it.

You raised a good point, maybe it's worth keeping this rb_edge field
directly in the atmel_nfc struct. If you decide to do so, could you add
a comment explaining why you're doing it?

> 
> >>  };
> >>  static struct atmel_nfc      nand_nfc;
> >>
> >> @@ -1675,9 +1680,9 @@ static irqreturn_t hsmc_interrupt(int irq, void *dev_id)
> >>               nfc_writel(host->nfc->hsmc_regs, IDR, NFC_SR_XFR_DONE);
> >>               ret = IRQ_HANDLED;
> >>       }
> >> -     if (pending & NFC_SR_RB_EDGE) {
> >> +     if (pending & host->nfc->rb_edge) {

In the other hand, I see that we're already having a lot of
indirections here, not sure adding one more will drastically impact the
system.
And if you really care about interrupt latencies in the system, you
should probably register this handler as a threaded irq.

> >>               complete(&host->nfc->comp_ready);
> >> -             nfc_writel(host->nfc->hsmc_regs, IDR, NFC_SR_RB_EDGE);
> >> +             nfc_writel(host->nfc->hsmc_regs, IDR, host->nfc->rb_edge);
> >
> > How about creating a macro that would generate the appropriate bitmask
> > for you?
> >
> > Something like
> >
> > #define NFC_SR_RB_EDGE_MASK(x)  ((x) << 24)
> >
> 
> It's already so verbose, it will make everything longer. Once the member name
> contains the 'mask' part, all the information will be there.

Makes sense, especially if you keep the pre-calculated nfc->rb_edge_mask
field.

> 
> 
> >> diff --git a/drivers/mtd/nand/atmel_nand_nfc.h b/drivers/mtd/nand/atmel_nand_nfc.h
> >> index 4d5d26221a7e..2cd9c57b1e53 100644
> >> --- a/drivers/mtd/nand/atmel_nand_nfc.h
> >> +++ b/drivers/mtd/nand/atmel_nand_nfc.h
> >> @@ -42,7 +42,10 @@
> >>  #define              NFC_SR_UNDEF            (1 << 21)
> >>  #define              NFC_SR_AWB              (1 << 22)
> >>  #define              NFC_SR_ASE              (1 << 23)
> >> -#define              NFC_SR_RB_EDGE          (1 << 24)
> >> +#define              NFC_SR_RB_EDGE0         (1 << 24)
> >> +#define              NFC_SR_RB_EDGE1         (1 << 25)
> >> +#define              NFC_SR_RB_EDGE2         (1 << 26)
> >> +#define              NFC_SR_RB_EDGE3         (1 << 27)
> >
> > Please replace those 4 definitions by:
> >
> > #define                 NFC_SR_RB_EDGE(x)       BIT(x + 24)
> >
> 
> The whole file could be rewritten in this form, but I see this as a
> zero-value proposition. I believe it is best to keep with local
> consistency. In fact, I will rather remove RB_EDGE1 & RB_EDGE2, as
> those are not mentioned in the datasheets.

Hm, I keep thinking using the NFC_SR_RB_EDGE(x) is more future proof,
but I'll let Atmel maintainers decide whether it's relevant or not to
do so.
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/mtd/atmel-nand.txt b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
index 7d4c8eb775a5..89b0db9801b0 100644
--- a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
@@ -34,7 +34,7 @@  Optional properties:
 - nand-on-flash-bbt: boolean to enable on flash bbt option if not present false
 - Nand Flash Controller(NFC) is a slave driver under Atmel nand flash
   - Required properties:
-    - compatible : "atmel,sama5d3-nfc".
+    - compatible : "atmel,sama5d3-nfc" or "atmel,sama5d4-nfc".
     - reg : should specify the address and size used for NFC command registers,
             NFC registers and NFC Sram. NFC Sram address and size can be absent
             if don't want to use it.
diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index 9d71f9e6a8de..e5d7e7e63f49 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -67,6 +67,10 @@  struct atmel_nand_caps {
 	bool pmecc_correct_erase_page;
 };
 
+struct atmel_nand_nfc_priv {
+	uint32_t rb_edge;
+};
+
 /* oob layout for large page size
  * bad block info is on bytes 0 and 1
  * the bytes have to be consecutives to avoid
@@ -111,6 +115,7 @@  struct atmel_nfc {
 	/* Point to the sram bank which include readed data via NFC */
 	void			*data_in_sram;
 	bool			will_write_sram;
+	uint32_t		rb_edge;
 };
 static struct atmel_nfc	nand_nfc;
 
@@ -1675,9 +1680,9 @@  static irqreturn_t hsmc_interrupt(int irq, void *dev_id)
 		nfc_writel(host->nfc->hsmc_regs, IDR, NFC_SR_XFR_DONE);
 		ret = IRQ_HANDLED;
 	}
-	if (pending & NFC_SR_RB_EDGE) {
+	if (pending & host->nfc->rb_edge) {
 		complete(&host->nfc->comp_ready);
-		nfc_writel(host->nfc->hsmc_regs, IDR, NFC_SR_RB_EDGE);
+		nfc_writel(host->nfc->hsmc_regs, IDR, host->nfc->rb_edge);
 		ret = IRQ_HANDLED;
 	}
 	if (pending & NFC_SR_CMD_DONE) {
@@ -1695,7 +1700,7 @@  static void nfc_prepare_interrupt(struct atmel_nand_host *host, u32 flag)
 	if (flag & NFC_SR_XFR_DONE)
 		init_completion(&host->nfc->comp_xfer_done);
 
-	if (flag & NFC_SR_RB_EDGE)
+	if (flag & host->nfc->rb_edge)
 		init_completion(&host->nfc->comp_ready);
 
 	if (flag & NFC_SR_CMD_DONE)
@@ -1713,7 +1718,7 @@  static int nfc_wait_interrupt(struct atmel_nand_host *host, u32 flag)
 	if (flag & NFC_SR_XFR_DONE)
 		comp[index++] = &host->nfc->comp_xfer_done;
 
-	if (flag & NFC_SR_RB_EDGE)
+	if (flag & host->nfc->rb_edge)
 		comp[index++] = &host->nfc->comp_ready;
 
 	if (flag & NFC_SR_CMD_DONE)
@@ -1781,7 +1786,7 @@  static int nfc_device_ready(struct mtd_info *mtd)
 		dev_err(host->dev, "Lost the interrupt flags: 0x%08x\n",
 				mask & status);
 
-	return status & NFC_SR_RB_EDGE;
+	return status & host->nfc->rb_edge;
 }
 
 static void nfc_select_chip(struct mtd_info *mtd, int chip)
@@ -1954,8 +1959,8 @@  static void nfc_nand_command(struct mtd_info *mtd, unsigned int command,
 		}
 		/* fall through */
 	default:
-		nfc_prepare_interrupt(host, NFC_SR_RB_EDGE);
-		nfc_wait_interrupt(host, NFC_SR_RB_EDGE);
+		nfc_prepare_interrupt(host, host->nfc->rb_edge);
+		nfc_wait_interrupt(host, host->nfc->rb_edge);
 	}
 }
 
@@ -2318,9 +2323,12 @@  static const struct of_device_id atmel_nand_dt_ids[] = {
 
 MODULE_DEVICE_TABLE(of, atmel_nand_dt_ids);
 
+static const struct of_device_id atmel_nand_nfc_match[];
+
 static int atmel_nand_nfc_probe(struct platform_device *pdev)
 {
 	struct atmel_nfc *nfc = &nand_nfc;
+	const struct atmel_nand_nfc_priv *priv;
 	struct resource *nfc_cmd_regs, *nfc_hsmc_regs, *nfc_sram;
 	int ret;
 
@@ -2352,6 +2360,12 @@  static int atmel_nand_nfc_probe(struct platform_device *pdev)
 		}
 	}
 
+	priv = of_match_device(atmel_nand_nfc_match, &pdev->dev)->data;
+	if (NULL == priv)
+		return -ENODEV;
+
+	nfc->rb_edge = priv->rb_edge;
+
 	nfc_writel(nfc->hsmc_regs, IDR, 0xffffffff);
 	nfc_readl(nfc->hsmc_regs, SR);	/* clear the NFC_SR */
 
@@ -2380,8 +2394,17 @@  static int atmel_nand_nfc_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static struct atmel_nand_nfc_priv sama5d3_nfc_priv = {
+	.rb_edge = NFC_SR_RB_EDGE0,
+};
+
+static struct atmel_nand_nfc_priv sama5d4_nfc_priv = {
+	.rb_edge = NFC_SR_RB_EDGE3,
+};
+
 static const struct of_device_id atmel_nand_nfc_match[] = {
-	{ .compatible = "atmel,sama5d3-nfc" },
+	{ .compatible = "atmel,sama5d3-nfc", .data = &sama5d3_nfc_priv },
+	{ .compatible = "atmel,sama5d4-nfc", .data = &sama5d4_nfc_priv },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, atmel_nand_nfc_match);
diff --git a/drivers/mtd/nand/atmel_nand_nfc.h b/drivers/mtd/nand/atmel_nand_nfc.h
index 4d5d26221a7e..2cd9c57b1e53 100644
--- a/drivers/mtd/nand/atmel_nand_nfc.h
+++ b/drivers/mtd/nand/atmel_nand_nfc.h
@@ -42,7 +42,10 @@ 
 #define		NFC_SR_UNDEF		(1 << 21)
 #define		NFC_SR_AWB		(1 << 22)
 #define		NFC_SR_ASE		(1 << 23)
-#define		NFC_SR_RB_EDGE		(1 << 24)
+#define		NFC_SR_RB_EDGE0		(1 << 24)
+#define		NFC_SR_RB_EDGE1		(1 << 25)
+#define		NFC_SR_RB_EDGE2		(1 << 26)
+#define		NFC_SR_RB_EDGE3		(1 << 27)
 
 #define ATMEL_HSMC_NFC_IER	0x0c
 #define ATMEL_HSMC_NFC_IDR	0x10