* [PATCH v1 1/5] mtd: atmel_nand: Do not warn on bitflips
2016-01-13 16:34 ` Romain Izard
@ 2016-01-13 16:34 ` Romain Izard
-1 siblings, 0 replies; 47+ messages in thread
From: Romain Izard @ 2016-01-13 16:34 UTC (permalink / raw)
To: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: Yang Wenyou, Josh Wu, Nicolas Ferre, Romain Izard
When using multi-bit ECC, it is normal for the NAND Flash driver to
correct bit errors during the life of the product. Those errors will
only be cleared once a threshold has been reached, and corrections can
occur regularly before this.
Use only dev_dbg and not dev_info to report the bitflips, to keep the
system log clean when everything works correctly.
Signed-off-by: Romain Izard <romain.izard.pro-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Acked-by: Wenyou Yang <wenyou.yang-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
---
drivers/mtd/nand/atmel_nand.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index 46010bd895b1..9d71f9e6a8de 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -824,7 +824,7 @@ static void pmecc_correct_data(struct mtd_info *mtd, uint8_t *buf, uint8_t *ecc,
*(buf + byte_pos) ^= (1 << bit_pos);
pos = sector_num * host->pmecc_sector_size + byte_pos;
- dev_info(host->dev, "Bit flip in data area, byte_pos: %d, bit_pos: %d, 0x%02x -> 0x%02x\n",
+ dev_dbg(host->dev, "Bit flip in data area, byte_pos: %d, bit_pos: %d, 0x%02x -> 0x%02x\n",
pos, bit_pos, err_byte, *(buf + byte_pos));
} else {
/* Bit flip in OOB area */
@@ -834,7 +834,7 @@ static void pmecc_correct_data(struct mtd_info *mtd, uint8_t *buf, uint8_t *ecc,
ecc[tmp] ^= (1 << bit_pos);
pos = tmp + nand_chip->ecc.layout->eccpos[0];
- dev_info(host->dev, "Bit flip in OOB, oob_byte_pos: %d, bit_pos: %d, 0x%02x -> 0x%02x\n",
+ dev_dbg(host->dev, "Bit flip in OOB, oob_byte_pos: %d, bit_pos: %d, 0x%02x -> 0x%02x\n",
pos, bit_pos, err_byte, ecc[tmp]);
}
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v1 1/5] mtd: atmel_nand: Do not warn on bitflips
@ 2016-01-13 16:34 ` Romain Izard
0 siblings, 0 replies; 47+ messages in thread
From: Romain Izard @ 2016-01-13 16:34 UTC (permalink / raw)
To: linux-mtd, devicetree; +Cc: Yang Wenyou, Josh Wu, Nicolas Ferre, Romain Izard
When using multi-bit ECC, it is normal for the NAND Flash driver to
correct bit errors during the life of the product. Those errors will
only be cleared once a threshold has been reached, and corrections can
occur regularly before this.
Use only dev_dbg and not dev_info to report the bitflips, to keep the
system log clean when everything works correctly.
Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
Acked-by: Wenyou Yang <wenyou.yang@atmel.com>
---
drivers/mtd/nand/atmel_nand.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index 46010bd895b1..9d71f9e6a8de 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -824,7 +824,7 @@ static void pmecc_correct_data(struct mtd_info *mtd, uint8_t *buf, uint8_t *ecc,
*(buf + byte_pos) ^= (1 << bit_pos);
pos = sector_num * host->pmecc_sector_size + byte_pos;
- dev_info(host->dev, "Bit flip in data area, byte_pos: %d, bit_pos: %d, 0x%02x -> 0x%02x\n",
+ dev_dbg(host->dev, "Bit flip in data area, byte_pos: %d, bit_pos: %d, 0x%02x -> 0x%02x\n",
pos, bit_pos, err_byte, *(buf + byte_pos));
} else {
/* Bit flip in OOB area */
@@ -834,7 +834,7 @@ static void pmecc_correct_data(struct mtd_info *mtd, uint8_t *buf, uint8_t *ecc,
ecc[tmp] ^= (1 << bit_pos);
pos = tmp + nand_chip->ecc.layout->eccpos[0];
- dev_info(host->dev, "Bit flip in OOB, oob_byte_pos: %d, bit_pos: %d, 0x%02x -> 0x%02x\n",
+ dev_dbg(host->dev, "Bit flip in OOB, oob_byte_pos: %d, bit_pos: %d, 0x%02x -> 0x%02x\n",
pos, bit_pos, err_byte, ecc[tmp]);
}
--
2.5.0
^ permalink raw reply related [flat|nested] 47+ messages in thread[parent not found: <1452702857-2240-2-git-send-email-romain.izard.pro-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH v1 1/5] mtd: atmel_nand: Do not warn on bitflips
2016-01-13 16:34 ` Romain Izard
@ 2016-01-13 17:54 ` Boris Brezillon
-1 siblings, 0 replies; 47+ messages in thread
From: Boris Brezillon @ 2016-01-13 17:54 UTC (permalink / raw)
To: Romain Izard
Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Josh Wu, Nicolas Ferre,
Yang Wenyou
On Wed, 13 Jan 2016 17:34:13 +0100
Romain Izard <romain.izard.pro-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> When using multi-bit ECC, it is normal for the NAND Flash driver to
> correct bit errors during the life of the product. Those errors will
> only be cleared once a threshold has been reached, and corrections can
> occur regularly before this.
>
> Use only dev_dbg and not dev_info to report the bitflips, to keep the
> system log clean when everything works correctly.
>
> Signed-off-by: Romain Izard <romain.izard.pro-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Acked-by: Wenyou Yang <wenyou.yang-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
Reviewed-by: Boris Brezillon <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
> drivers/mtd/nand/atmel_nand.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
> index 46010bd895b1..9d71f9e6a8de 100644
> --- a/drivers/mtd/nand/atmel_nand.c
> +++ b/drivers/mtd/nand/atmel_nand.c
> @@ -824,7 +824,7 @@ static void pmecc_correct_data(struct mtd_info *mtd, uint8_t *buf, uint8_t *ecc,
> *(buf + byte_pos) ^= (1 << bit_pos);
>
> pos = sector_num * host->pmecc_sector_size + byte_pos;
> - dev_info(host->dev, "Bit flip in data area, byte_pos: %d, bit_pos: %d, 0x%02x -> 0x%02x\n",
> + dev_dbg(host->dev, "Bit flip in data area, byte_pos: %d, bit_pos: %d, 0x%02x -> 0x%02x\n",
> pos, bit_pos, err_byte, *(buf + byte_pos));
> } else {
> /* Bit flip in OOB area */
> @@ -834,7 +834,7 @@ static void pmecc_correct_data(struct mtd_info *mtd, uint8_t *buf, uint8_t *ecc,
> ecc[tmp] ^= (1 << bit_pos);
>
> pos = tmp + nand_chip->ecc.layout->eccpos[0];
> - dev_info(host->dev, "Bit flip in OOB, oob_byte_pos: %d, bit_pos: %d, 0x%02x -> 0x%02x\n",
> + dev_dbg(host->dev, "Bit flip in OOB, oob_byte_pos: %d, bit_pos: %d, 0x%02x -> 0x%02x\n",
> pos, bit_pos, err_byte, ecc[tmp]);
> }
>
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v1 1/5] mtd: atmel_nand: Do not warn on bitflips
@ 2016-01-13 17:54 ` Boris Brezillon
0 siblings, 0 replies; 47+ messages in thread
From: Boris Brezillon @ 2016-01-13 17:54 UTC (permalink / raw)
To: Romain Izard; +Cc: linux-mtd, devicetree, Josh Wu, Nicolas Ferre, Yang Wenyou
On Wed, 13 Jan 2016 17:34:13 +0100
Romain Izard <romain.izard.pro@gmail.com> wrote:
> When using multi-bit ECC, it is normal for the NAND Flash driver to
> correct bit errors during the life of the product. Those errors will
> only be cleared once a threshold has been reached, and corrections can
> occur regularly before this.
>
> Use only dev_dbg and not dev_info to report the bitflips, to keep the
> system log clean when everything works correctly.
>
> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
> Acked-by: Wenyou Yang <wenyou.yang@atmel.com>
Reviewed-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
> drivers/mtd/nand/atmel_nand.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
> index 46010bd895b1..9d71f9e6a8de 100644
> --- a/drivers/mtd/nand/atmel_nand.c
> +++ b/drivers/mtd/nand/atmel_nand.c
> @@ -824,7 +824,7 @@ static void pmecc_correct_data(struct mtd_info *mtd, uint8_t *buf, uint8_t *ecc,
> *(buf + byte_pos) ^= (1 << bit_pos);
>
> pos = sector_num * host->pmecc_sector_size + byte_pos;
> - dev_info(host->dev, "Bit flip in data area, byte_pos: %d, bit_pos: %d, 0x%02x -> 0x%02x\n",
> + dev_dbg(host->dev, "Bit flip in data area, byte_pos: %d, bit_pos: %d, 0x%02x -> 0x%02x\n",
> pos, bit_pos, err_byte, *(buf + byte_pos));
> } else {
> /* Bit flip in OOB area */
> @@ -834,7 +834,7 @@ static void pmecc_correct_data(struct mtd_info *mtd, uint8_t *buf, uint8_t *ecc,
> ecc[tmp] ^= (1 << bit_pos);
>
> pos = tmp + nand_chip->ecc.layout->eccpos[0];
> - dev_info(host->dev, "Bit flip in OOB, oob_byte_pos: %d, bit_pos: %d, 0x%02x -> 0x%02x\n",
> + dev_dbg(host->dev, "Bit flip in OOB, oob_byte_pos: %d, bit_pos: %d, 0x%02x -> 0x%02x\n",
> pos, bit_pos, err_byte, ecc[tmp]);
> }
>
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v1 1/5] mtd: atmel_nand: Do not warn on bitflips
2016-01-13 17:54 ` Boris Brezillon
@ 2016-01-23 20:44 ` Brian Norris
-1 siblings, 0 replies; 47+ messages in thread
From: Brian Norris @ 2016-01-23 20:44 UTC (permalink / raw)
To: Boris Brezillon
Cc: Romain Izard, devicetree-u79uwXL29TY76Z2rM5mHXA, Yang Wenyou,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Nicolas Ferre,
Josh Wu
On Wed, Jan 13, 2016 at 06:54:21PM +0100, Boris Brezillon wrote:
> On Wed, 13 Jan 2016 17:34:13 +0100
> Romain Izard <romain.izard.pro-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
> > When using multi-bit ECC, it is normal for the NAND Flash driver to
> > correct bit errors during the life of the product. Those errors will
> > only be cleared once a threshold has been reached, and corrections can
> > occur regularly before this.
> >
> > Use only dev_dbg and not dev_info to report the bitflips, to keep the
> > system log clean when everything works correctly.
> >
> > Signed-off-by: Romain Izard <romain.izard.pro-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > Acked-by: Wenyou Yang <wenyou.yang-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
>
> Reviewed-by: Boris Brezillon <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Pushed patch 1 to l2-mtd.git/next
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v1 1/5] mtd: atmel_nand: Do not warn on bitflips
@ 2016-01-23 20:44 ` Brian Norris
0 siblings, 0 replies; 47+ messages in thread
From: Brian Norris @ 2016-01-23 20:44 UTC (permalink / raw)
To: Boris Brezillon
Cc: Romain Izard, devicetree, Yang Wenyou, linux-mtd, Nicolas Ferre, Josh Wu
On Wed, Jan 13, 2016 at 06:54:21PM +0100, Boris Brezillon wrote:
> On Wed, 13 Jan 2016 17:34:13 +0100
> Romain Izard <romain.izard.pro@gmail.com> wrote:
>
> > When using multi-bit ECC, it is normal for the NAND Flash driver to
> > correct bit errors during the life of the product. Those errors will
> > only be cleared once a threshold has been reached, and corrections can
> > occur regularly before this.
> >
> > Use only dev_dbg and not dev_info to report the bitflips, to keep the
> > system log clean when everything works correctly.
> >
> > Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
> > Acked-by: Wenyou Yang <wenyou.yang@atmel.com>
>
> Reviewed-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Pushed patch 1 to l2-mtd.git/next
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v1 2/5] mtd: atmel_nand: Support variable RB_EDGE interrupts
2016-01-13 16:34 ` Romain Izard
@ 2016-01-13 16:34 ` Romain Izard
-1 siblings, 0 replies; 47+ messages in thread
From: Romain Izard @ 2016-01-13 16:34 UTC (permalink / raw)
To: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: Yang Wenyou, Josh Wu, Nicolas Ferre, Romain Izard
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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Reviewed-by: Wenyou Yang <Wenyou.yang-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
---
.../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 {
+ 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
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v1 2/5] mtd: atmel_nand: Support variable RB_EDGE interrupts
@ 2016-01-13 16:34 ` Romain Izard
0 siblings, 0 replies; 47+ messages in thread
From: Romain Izard @ 2016-01-13 16:34 UTC (permalink / raw)
To: linux-mtd, devicetree; +Cc: Yang Wenyou, Josh Wu, Nicolas Ferre, Romain Izard
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 {
+ 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
--
2.5.0
^ permalink raw reply related [flat|nested] 47+ messages in thread[parent not found: <1452702857-2240-3-git-send-email-romain.izard.pro-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH v1 2/5] mtd: atmel_nand: Support variable RB_EDGE interrupts
2016-01-13 16:34 ` Romain Izard
@ 2016-01-13 18:14 ` Boris Brezillon
-1 siblings, 0 replies; 47+ messages in thread
From: Boris Brezillon @ 2016-01-13 18:14 UTC (permalink / raw)
To: Romain Izard
Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Josh Wu, Nicolas Ferre,
Yang Wenyou
Hi Romain,
On Wed, 13 Jan 2016 17:34:14 +0100
Romain Izard <romain.izard.pro-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Reviewed-by: Wenyou Yang <Wenyou.yang-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
> ---
> .../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
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v1 2/5] mtd: atmel_nand: Support variable RB_EDGE interrupts
@ 2016-01-13 18:14 ` Boris Brezillon
0 siblings, 0 replies; 47+ messages in thread
From: Boris Brezillon @ 2016-01-13 18:14 UTC (permalink / raw)
To: Romain Izard; +Cc: linux-mtd, devicetree, Josh Wu, Nicolas Ferre, Yang Wenyou
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
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v1 2/5] mtd: atmel_nand: Support variable RB_EDGE interrupts
2016-01-13 18:14 ` Boris Brezillon
@ 2016-01-14 10:16 ` Romain Izard
-1 siblings, 0 replies; 47+ messages in thread
From: Romain Izard @ 2016-01-14 10:16 UTC (permalink / raw)
To: Boris Brezillon
Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Josh Wu, Nicolas Ferre,
Yang Wenyou
Hi Boris,
2016-01-13 19:14 GMT+01:00 Boris Brezillon <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>:
>> 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,
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v1 2/5] mtd: atmel_nand: Support variable RB_EDGE interrupts
@ 2016-01-14 10:16 ` Romain Izard
0 siblings, 0 replies; 47+ messages in thread
From: Romain Izard @ 2016-01-14 10:16 UTC (permalink / raw)
To: Boris Brezillon
Cc: linux-mtd, devicetree, Josh Wu, Nicolas Ferre, Yang Wenyou
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,
^ permalink raw reply [flat|nested] 47+ messages in thread[parent not found: <CAGkQfmMQTBVzOoE3BTyvg73NKjtGD_0veV_zb_jLutk-6bcqGw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH v1 2/5] mtd: atmel_nand: Support variable RB_EDGE interrupts
2016-01-14 10:16 ` Romain Izard
@ 2016-01-14 10:41 ` Boris Brezillon
-1 siblings, 0 replies; 47+ messages in thread
From: Boris Brezillon @ 2016-01-14 10:41 UTC (permalink / raw)
To: Romain Izard
Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Josh Wu, Nicolas Ferre,
Yang Wenyou
On Thu, 14 Jan 2016 11:16:19 +0100
Romain Izard <romain.izard.pro-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Hi Boris,
>
> 2016-01-13 19:14 GMT+01:00 Boris Brezillon <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>:
> >> 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.
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v1 2/5] mtd: atmel_nand: Support variable RB_EDGE interrupts
@ 2016-01-14 10:41 ` Boris Brezillon
0 siblings, 0 replies; 47+ messages in thread
From: Boris Brezillon @ 2016-01-14 10:41 UTC (permalink / raw)
To: Romain Izard; +Cc: linux-mtd, devicetree, Josh Wu, Nicolas Ferre, Yang Wenyou
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.
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v1 2/5] mtd: atmel_nand: Support variable RB_EDGE interrupts
2016-01-13 16:34 ` Romain Izard
@ 2016-01-14 1:19 ` Rob Herring
-1 siblings, 0 replies; 47+ messages in thread
From: Rob Herring @ 2016-01-14 1:19 UTC (permalink / raw)
To: Romain Izard
Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Yang Wenyou, Josh Wu,
Nicolas Ferre
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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Reviewed-by: Wenyou Yang <Wenyou.yang-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
> ---
> .../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-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.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 */
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v1 2/5] mtd: atmel_nand: Support variable RB_EDGE interrupts
@ 2016-01-14 1:19 ` Rob Herring
0 siblings, 0 replies; 47+ messages in thread
From: Rob Herring @ 2016-01-14 1:19 UTC (permalink / raw)
To: Romain Izard; +Cc: linux-mtd, devicetree, Yang Wenyou, Josh Wu, Nicolas Ferre
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 */
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v1 3/5] mtd: atmel_nand: Support PMECC on SAMA5D2
2016-01-13 16:34 ` Romain Izard
@ 2016-01-13 16:34 ` Romain Izard
-1 siblings, 0 replies; 47+ messages in thread
From: Romain Izard @ 2016-01-13 16:34 UTC (permalink / raw)
To: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: Yang Wenyou, Josh Wu, Nicolas Ferre, Romain Izard
Starting with the SAMA5D2, there is a new revision of the Atmel PMECC
controller that can correct 32 bits in each sector. This controller is
not 100% compatible with the previous revision that corrected a maximum
of 24 bits by sector, as some register addresses overlap.
Using information from the device tree, we can configure the driver to
work with both versions.
Signed-off-by: Romain Izard <romain.izard.pro-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
.../devicetree/bindings/mtd/atmel-nand.txt | 7 +++++--
drivers/mtd/nand/atmel_nand.c | 23 +++++++++++++++++++++-
drivers/mtd/nand/atmel_nand_ecc.h | 8 ++++++--
3 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/mtd/atmel-nand.txt b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
index 89b0db9801b0..90887b430f03 100644
--- a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
@@ -1,7 +1,10 @@
Atmel NAND flash
Required properties:
-- compatible : should be "atmel,at91rm9200-nand" or "atmel,sama5d4-nand".
+- compatible: The possible values are:
+ "atmel,at91rm9200-nand"
+ "atmel,sama5d2-nand"
+ "atmel,sama5d4-nand"
- reg : should specify localbus address and size used for the chip,
and hardware ECC controller if available.
If the hardware ECC is PMECC, it should contain address and size for
@@ -22,7 +25,7 @@ Optional properties:
Supported values are: "none", "soft", "hw", "hw_syndrome", "hw_oob_first",
"soft_bch".
- atmel,has-pmecc : boolean to enable Programmable Multibit ECC hardware.
- Only supported by at91sam9x5 or later sam9 product.
+ Supported by AT91SAM9x5 or later SAM9 chips, and SAMA5 chips.
- atmel,pmecc-cap : error correct capability for Programmable Multibit ECC
Controller. Supported values are: 2, 4, 8, 12, 24.
- atmel,pmecc-sector-size : sector size for ECC computation. Supported values
diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index e5d7e7e63f49..6fe50e2d291f 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -65,6 +65,7 @@ module_param(on_flash_bbt, int, 0);
struct atmel_nand_caps {
bool pmecc_correct_erase_page;
+ uint8_t pmecc_max_correction;
};
struct atmel_nand_nfc_priv {
@@ -146,6 +147,7 @@ struct atmel_nand_host {
int pmecc_cw_len; /* Length of codeword */
void __iomem *pmerrloc_base;
+ void __iomem *pmerrloc_el_base;
void __iomem *pmecc_rom_base;
/* lookup table for alpha_to and index_of */
@@ -817,7 +819,7 @@ static void pmecc_correct_data(struct mtd_info *mtd, uint8_t *buf, uint8_t *ecc,
sector_size = host->pmecc_sector_size;
while (err_nbr) {
- tmp = pmerrloc_readl_el_relaxed(host->pmerrloc_base, i) - 1;
+ tmp = pmerrloc_readl_el_relaxed(host->pmerrloc_el_base, i) - 1;
byte_pos = tmp / 8;
bit_pos = tmp % 8;
@@ -1208,6 +1210,8 @@ static int atmel_pmecc_nand_init_params(struct platform_device *pdev,
err_no = PTR_ERR(host->pmerrloc_base);
goto err;
}
+ host->pmerrloc_el_base = host->pmerrloc_base + ATMEL_PMERRLOC_SIGMAx +
+ (host->caps->pmecc_max_correction + 1) * 4;
if (!host->has_no_lookup_table) {
regs_rom = platform_get_resource(pdev, IORESOURCE_MEM, 3);
@@ -2307,17 +2311,34 @@ static int atmel_nand_remove(struct platform_device *pdev)
return 0;
}
+/*
+ * AT91RM9200 does not have PMECC or PMECC Errloc peripherals for
+ * BCH ECC. Combined with the "atmel,has-pmecc", it is used to describe
+ * devices from the SAM9 family that have those.
+ */
static struct atmel_nand_caps at91rm9200_caps = {
.pmecc_correct_erase_page = false,
+ .pmecc_max_correction = 24,
};
static struct atmel_nand_caps sama5d4_caps = {
.pmecc_correct_erase_page = true,
+ .pmecc_max_correction = 24,
+};
+
+/*
+ * The PMECC Errloc controller starting in SAMA5D2 is not compatible,
+ * as the increased correction strength requires more registers.
+ */
+static struct atmel_nand_caps sama5d2_caps = {
+ .pmecc_correct_erase_page = true,
+ .pmecc_max_correction = 32,
};
static const struct of_device_id atmel_nand_dt_ids[] = {
{ .compatible = "atmel,at91rm9200-nand", .data = &at91rm9200_caps },
{ .compatible = "atmel,sama5d4-nand", .data = &sama5d4_caps },
+ { .compatible = "atmel,sama5d2-nand", .data = &sama5d2_caps },
{ /* sentinel */ }
};
diff --git a/drivers/mtd/nand/atmel_nand_ecc.h b/drivers/mtd/nand/atmel_nand_ecc.h
index 668e7358f19b..ec964c43c932 100644
--- a/drivers/mtd/nand/atmel_nand_ecc.h
+++ b/drivers/mtd/nand/atmel_nand_ecc.h
@@ -108,7 +108,11 @@
#define PMERRLOC_ERR_NUM_MASK (0x1f << 8)
#define PMERRLOC_CALC_DONE (1 << 0)
#define ATMEL_PMERRLOC_SIGMAx 0x028 /* Error location SIGMA x */
-#define ATMEL_PMERRLOC_ELx 0x08c /* Error location x */
+
+/*
+ * The ATMEL_PMERRLOC_ELx register location depends from the number of
+ * bits corrected by the PMECC controller. Do not use it.
+ */
/* Register access macros for PMECC */
#define pmecc_readl_relaxed(addr, reg) \
@@ -136,7 +140,7 @@
readl_relaxed((addr) + ATMEL_PMERRLOC_SIGMAx + ((n) * 4))
#define pmerrloc_readl_el_relaxed(addr, n) \
- readl_relaxed((addr) + ATMEL_PMERRLOC_ELx + ((n) * 4))
+ readl_relaxed((addr) + ((n) * 4))
/* Galois field dimension */
#define PMECC_GF_DIMENSION_13 13
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v1 3/5] mtd: atmel_nand: Support PMECC on SAMA5D2
@ 2016-01-13 16:34 ` Romain Izard
0 siblings, 0 replies; 47+ messages in thread
From: Romain Izard @ 2016-01-13 16:34 UTC (permalink / raw)
To: linux-mtd, devicetree; +Cc: Yang Wenyou, Josh Wu, Nicolas Ferre, Romain Izard
Starting with the SAMA5D2, there is a new revision of the Atmel PMECC
controller that can correct 32 bits in each sector. This controller is
not 100% compatible with the previous revision that corrected a maximum
of 24 bits by sector, as some register addresses overlap.
Using information from the device tree, we can configure the driver to
work with both versions.
Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
---
.../devicetree/bindings/mtd/atmel-nand.txt | 7 +++++--
drivers/mtd/nand/atmel_nand.c | 23 +++++++++++++++++++++-
drivers/mtd/nand/atmel_nand_ecc.h | 8 ++++++--
3 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/mtd/atmel-nand.txt b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
index 89b0db9801b0..90887b430f03 100644
--- a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
@@ -1,7 +1,10 @@
Atmel NAND flash
Required properties:
-- compatible : should be "atmel,at91rm9200-nand" or "atmel,sama5d4-nand".
+- compatible: The possible values are:
+ "atmel,at91rm9200-nand"
+ "atmel,sama5d2-nand"
+ "atmel,sama5d4-nand"
- reg : should specify localbus address and size used for the chip,
and hardware ECC controller if available.
If the hardware ECC is PMECC, it should contain address and size for
@@ -22,7 +25,7 @@ Optional properties:
Supported values are: "none", "soft", "hw", "hw_syndrome", "hw_oob_first",
"soft_bch".
- atmel,has-pmecc : boolean to enable Programmable Multibit ECC hardware.
- Only supported by at91sam9x5 or later sam9 product.
+ Supported by AT91SAM9x5 or later SAM9 chips, and SAMA5 chips.
- atmel,pmecc-cap : error correct capability for Programmable Multibit ECC
Controller. Supported values are: 2, 4, 8, 12, 24.
- atmel,pmecc-sector-size : sector size for ECC computation. Supported values
diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index e5d7e7e63f49..6fe50e2d291f 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -65,6 +65,7 @@ module_param(on_flash_bbt, int, 0);
struct atmel_nand_caps {
bool pmecc_correct_erase_page;
+ uint8_t pmecc_max_correction;
};
struct atmel_nand_nfc_priv {
@@ -146,6 +147,7 @@ struct atmel_nand_host {
int pmecc_cw_len; /* Length of codeword */
void __iomem *pmerrloc_base;
+ void __iomem *pmerrloc_el_base;
void __iomem *pmecc_rom_base;
/* lookup table for alpha_to and index_of */
@@ -817,7 +819,7 @@ static void pmecc_correct_data(struct mtd_info *mtd, uint8_t *buf, uint8_t *ecc,
sector_size = host->pmecc_sector_size;
while (err_nbr) {
- tmp = pmerrloc_readl_el_relaxed(host->pmerrloc_base, i) - 1;
+ tmp = pmerrloc_readl_el_relaxed(host->pmerrloc_el_base, i) - 1;
byte_pos = tmp / 8;
bit_pos = tmp % 8;
@@ -1208,6 +1210,8 @@ static int atmel_pmecc_nand_init_params(struct platform_device *pdev,
err_no = PTR_ERR(host->pmerrloc_base);
goto err;
}
+ host->pmerrloc_el_base = host->pmerrloc_base + ATMEL_PMERRLOC_SIGMAx +
+ (host->caps->pmecc_max_correction + 1) * 4;
if (!host->has_no_lookup_table) {
regs_rom = platform_get_resource(pdev, IORESOURCE_MEM, 3);
@@ -2307,17 +2311,34 @@ static int atmel_nand_remove(struct platform_device *pdev)
return 0;
}
+/*
+ * AT91RM9200 does not have PMECC or PMECC Errloc peripherals for
+ * BCH ECC. Combined with the "atmel,has-pmecc", it is used to describe
+ * devices from the SAM9 family that have those.
+ */
static struct atmel_nand_caps at91rm9200_caps = {
.pmecc_correct_erase_page = false,
+ .pmecc_max_correction = 24,
};
static struct atmel_nand_caps sama5d4_caps = {
.pmecc_correct_erase_page = true,
+ .pmecc_max_correction = 24,
+};
+
+/*
+ * The PMECC Errloc controller starting in SAMA5D2 is not compatible,
+ * as the increased correction strength requires more registers.
+ */
+static struct atmel_nand_caps sama5d2_caps = {
+ .pmecc_correct_erase_page = true,
+ .pmecc_max_correction = 32,
};
static const struct of_device_id atmel_nand_dt_ids[] = {
{ .compatible = "atmel,at91rm9200-nand", .data = &at91rm9200_caps },
{ .compatible = "atmel,sama5d4-nand", .data = &sama5d4_caps },
+ { .compatible = "atmel,sama5d2-nand", .data = &sama5d2_caps },
{ /* sentinel */ }
};
diff --git a/drivers/mtd/nand/atmel_nand_ecc.h b/drivers/mtd/nand/atmel_nand_ecc.h
index 668e7358f19b..ec964c43c932 100644
--- a/drivers/mtd/nand/atmel_nand_ecc.h
+++ b/drivers/mtd/nand/atmel_nand_ecc.h
@@ -108,7 +108,11 @@
#define PMERRLOC_ERR_NUM_MASK (0x1f << 8)
#define PMERRLOC_CALC_DONE (1 << 0)
#define ATMEL_PMERRLOC_SIGMAx 0x028 /* Error location SIGMA x */
-#define ATMEL_PMERRLOC_ELx 0x08c /* Error location x */
+
+/*
+ * The ATMEL_PMERRLOC_ELx register location depends from the number of
+ * bits corrected by the PMECC controller. Do not use it.
+ */
/* Register access macros for PMECC */
#define pmecc_readl_relaxed(addr, reg) \
@@ -136,7 +140,7 @@
readl_relaxed((addr) + ATMEL_PMERRLOC_SIGMAx + ((n) * 4))
#define pmerrloc_readl_el_relaxed(addr, n) \
- readl_relaxed((addr) + ATMEL_PMERRLOC_ELx + ((n) * 4))
+ readl_relaxed((addr) + ((n) * 4))
/* Galois field dimension */
#define PMECC_GF_DIMENSION_13 13
--
2.5.0
^ permalink raw reply related [flat|nested] 47+ messages in thread[parent not found: <1452702857-2240-4-git-send-email-romain.izard.pro-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH v1 3/5] mtd: atmel_nand: Support PMECC on SAMA5D2
2016-01-13 16:34 ` Romain Izard
@ 2016-01-14 1:12 ` Rob Herring
-1 siblings, 0 replies; 47+ messages in thread
From: Rob Herring @ 2016-01-14 1:12 UTC (permalink / raw)
To: Romain Izard
Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Yang Wenyou, Josh Wu,
Nicolas Ferre
On Wed, Jan 13, 2016 at 05:34:15PM +0100, Romain Izard wrote:
> Starting with the SAMA5D2, there is a new revision of the Atmel PMECC
> controller that can correct 32 bits in each sector. This controller is
> not 100% compatible with the previous revision that corrected a maximum
> of 24 bits by sector, as some register addresses overlap.
>
> Using information from the device tree, we can configure the driver to
> work with both versions.
>
> Signed-off-by: Romain Izard <romain.izard.pro-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> .../devicetree/bindings/mtd/atmel-nand.txt | 7 +++++--
> drivers/mtd/nand/atmel_nand.c | 23 +++++++++++++++++++++-
> drivers/mtd/nand/atmel_nand_ecc.h | 8 ++++++--
> 3 files changed, 33 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mtd/atmel-nand.txt b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> index 89b0db9801b0..90887b430f03 100644
> --- a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> @@ -1,7 +1,10 @@
> Atmel NAND flash
>
> Required properties:
> -- compatible : should be "atmel,at91rm9200-nand" or "atmel,sama5d4-nand".
> +- compatible: The possible values are:
> + "atmel,at91rm9200-nand"
> + "atmel,sama5d2-nand"
> + "atmel,sama5d4-nand"
> - reg : should specify localbus address and size used for the chip,
> and hardware ECC controller if available.
> If the hardware ECC is PMECC, it should contain address and size for
> @@ -22,7 +25,7 @@ Optional properties:
> Supported values are: "none", "soft", "hw", "hw_syndrome", "hw_oob_first",
> "soft_bch".
> - atmel,has-pmecc : boolean to enable Programmable Multibit ECC hardware.
> - Only supported by at91sam9x5 or later sam9 product.
> + Supported by AT91SAM9x5 or later SAM9 chips, and SAMA5 chips.
What compatible string would AT91SAM9x5 be?
> - atmel,pmecc-cap : error correct capability for Programmable Multibit ECC
> Controller. Supported values are: 2, 4, 8, 12, 24.
> - atmel,pmecc-sector-size : sector size for ECC computation. Supported values
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v1 3/5] mtd: atmel_nand: Support PMECC on SAMA5D2
@ 2016-01-14 1:12 ` Rob Herring
0 siblings, 0 replies; 47+ messages in thread
From: Rob Herring @ 2016-01-14 1:12 UTC (permalink / raw)
To: Romain Izard; +Cc: linux-mtd, devicetree, Yang Wenyou, Josh Wu, Nicolas Ferre
On Wed, Jan 13, 2016 at 05:34:15PM +0100, Romain Izard wrote:
> Starting with the SAMA5D2, there is a new revision of the Atmel PMECC
> controller that can correct 32 bits in each sector. This controller is
> not 100% compatible with the previous revision that corrected a maximum
> of 24 bits by sector, as some register addresses overlap.
>
> Using information from the device tree, we can configure the driver to
> work with both versions.
>
> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
> ---
> .../devicetree/bindings/mtd/atmel-nand.txt | 7 +++++--
> drivers/mtd/nand/atmel_nand.c | 23 +++++++++++++++++++++-
> drivers/mtd/nand/atmel_nand_ecc.h | 8 ++++++--
> 3 files changed, 33 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mtd/atmel-nand.txt b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> index 89b0db9801b0..90887b430f03 100644
> --- a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> @@ -1,7 +1,10 @@
> Atmel NAND flash
>
> Required properties:
> -- compatible : should be "atmel,at91rm9200-nand" or "atmel,sama5d4-nand".
> +- compatible: The possible values are:
> + "atmel,at91rm9200-nand"
> + "atmel,sama5d2-nand"
> + "atmel,sama5d4-nand"
> - reg : should specify localbus address and size used for the chip,
> and hardware ECC controller if available.
> If the hardware ECC is PMECC, it should contain address and size for
> @@ -22,7 +25,7 @@ Optional properties:
> Supported values are: "none", "soft", "hw", "hw_syndrome", "hw_oob_first",
> "soft_bch".
> - atmel,has-pmecc : boolean to enable Programmable Multibit ECC hardware.
> - Only supported by at91sam9x5 or later sam9 product.
> + Supported by AT91SAM9x5 or later SAM9 chips, and SAMA5 chips.
What compatible string would AT91SAM9x5 be?
> - atmel,pmecc-cap : error correct capability for Programmable Multibit ECC
> Controller. Supported values are: 2, 4, 8, 12, 24.
> - atmel,pmecc-sector-size : sector size for ECC computation. Supported values
^ permalink raw reply [flat|nested] 47+ messages in thread
* RE: [PATCH v1 3/5] mtd: atmel_nand: Support PMECC on SAMA5D2
2016-01-14 1:12 ` Rob Herring
@ 2016-01-14 1:17 ` Yang, Wenyou
-1 siblings, 0 replies; 47+ messages in thread
From: Yang, Wenyou @ 2016-01-14 1:17 UTC (permalink / raw)
To: Rob Herring, Romain Izard
Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Josh Wu, Ferre, Nicolas
> -----Original Message-----
> From: Rob Herring [mailto:robh@kernel.org]
> Sent: 2016年1月14日 9:13
> To: Romain Izard <romain.izard.pro@gmail.com>
> Cc: linux-mtd@lists.infradead.org; devicetree@vger.kernel.org; Yang, Wenyou
> <Wenyou.Yang@atmel.com>; Josh Wu <rainyfeeling@outlook.com>; Ferre,
> Nicolas <Nicolas.FERRE@atmel.com>
> Subject: Re: [PATCH v1 3/5] mtd: atmel_nand: Support PMECC on SAMA5D2
>
> On Wed, Jan 13, 2016 at 05:34:15PM +0100, Romain Izard wrote:
> > Starting with the SAMA5D2, there is a new revision of the Atmel PMECC
> > controller that can correct 32 bits in each sector. This controller is
> > not 100% compatible with the previous revision that corrected a
> > maximum of 24 bits by sector, as some register addresses overlap.
> >
> > Using information from the device tree, we can configure the driver to
> > work with both versions.
> >
> > Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
> > ---
> > .../devicetree/bindings/mtd/atmel-nand.txt | 7 +++++--
> > drivers/mtd/nand/atmel_nand.c | 23 +++++++++++++++++++++-
> > drivers/mtd/nand/atmel_nand_ecc.h | 8 ++++++--
> > 3 files changed, 33 insertions(+), 5 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> > b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> > index 89b0db9801b0..90887b430f03 100644
> > --- a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> > +++ b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> > @@ -1,7 +1,10 @@
> > Atmel NAND flash
> >
> > Required properties:
> > -- compatible : should be "atmel,at91rm9200-nand" or "atmel,sama5d4-nand".
> > +- compatible: The possible values are:
> > + "atmel,at91rm9200-nand"
> > + "atmel,sama5d2-nand"
> > + "atmel,sama5d4-nand"
> > - reg : should specify localbus address and size used for the chip,
> > and hardware ECC controller if available.
> > If the hardware ECC is PMECC, it should contain address and size for
> > @@ -22,7 +25,7 @@ Optional properties:
> > Supported values are: "none", "soft", "hw", "hw_syndrome", "hw_oob_first",
> > "soft_bch".
> > - atmel,has-pmecc : boolean to enable Programmable Multibit ECC hardware.
> > - Only supported by at91sam9x5 or later sam9 product.
> > + Supported by AT91SAM9x5 or later SAM9 chips, and SAMA5 chips.
>
> What compatible string would AT91SAM9x5 be?
"atmel,at91rm9200-nand".
>
> > - atmel,pmecc-cap : error correct capability for Programmable Multibit ECC
> > Controller. Supported values are: 2, 4, 8, 12, 24.
> > - atmel,pmecc-sector-size : sector size for ECC computation.
> > Supported values
Best Regards,
Wenyou Yang
^ permalink raw reply [flat|nested] 47+ messages in thread
* RE: [PATCH v1 3/5] mtd: atmel_nand: Support PMECC on SAMA5D2
@ 2016-01-14 1:17 ` Yang, Wenyou
0 siblings, 0 replies; 47+ messages in thread
From: Yang, Wenyou @ 2016-01-14 1:17 UTC (permalink / raw)
To: Rob Herring, Romain Izard; +Cc: linux-mtd, devicetree, Josh Wu, Ferre, Nicolas
> -----Original Message-----
> From: Rob Herring [mailto:robh@kernel.org]
> Sent: 2016年1月14日 9:13
> To: Romain Izard <romain.izard.pro@gmail.com>
> Cc: linux-mtd@lists.infradead.org; devicetree@vger.kernel.org; Yang, Wenyou
> <Wenyou.Yang@atmel.com>; Josh Wu <rainyfeeling@outlook.com>; Ferre,
> Nicolas <Nicolas.FERRE@atmel.com>
> Subject: Re: [PATCH v1 3/5] mtd: atmel_nand: Support PMECC on SAMA5D2
>
> On Wed, Jan 13, 2016 at 05:34:15PM +0100, Romain Izard wrote:
> > Starting with the SAMA5D2, there is a new revision of the Atmel PMECC
> > controller that can correct 32 bits in each sector. This controller is
> > not 100% compatible with the previous revision that corrected a
> > maximum of 24 bits by sector, as some register addresses overlap.
> >
> > Using information from the device tree, we can configure the driver to
> > work with both versions.
> >
> > Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
> > ---
> > .../devicetree/bindings/mtd/atmel-nand.txt | 7 +++++--
> > drivers/mtd/nand/atmel_nand.c | 23 +++++++++++++++++++++-
> > drivers/mtd/nand/atmel_nand_ecc.h | 8 ++++++--
> > 3 files changed, 33 insertions(+), 5 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> > b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> > index 89b0db9801b0..90887b430f03 100644
> > --- a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> > +++ b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> > @@ -1,7 +1,10 @@
> > Atmel NAND flash
> >
> > Required properties:
> > -- compatible : should be "atmel,at91rm9200-nand" or "atmel,sama5d4-nand".
> > +- compatible: The possible values are:
> > + "atmel,at91rm9200-nand"
> > + "atmel,sama5d2-nand"
> > + "atmel,sama5d4-nand"
> > - reg : should specify localbus address and size used for the chip,
> > and hardware ECC controller if available.
> > If the hardware ECC is PMECC, it should contain address and size for
> > @@ -22,7 +25,7 @@ Optional properties:
> > Supported values are: "none", "soft", "hw", "hw_syndrome", "hw_oob_first",
> > "soft_bch".
> > - atmel,has-pmecc : boolean to enable Programmable Multibit ECC hardware.
> > - Only supported by at91sam9x5 or later sam9 product.
> > + Supported by AT91SAM9x5 or later SAM9 chips, and SAMA5 chips.
>
> What compatible string would AT91SAM9x5 be?
"atmel,at91rm9200-nand".
>
> > - atmel,pmecc-cap : error correct capability for Programmable Multibit ECC
> > Controller. Supported values are: 2, 4, 8, 12, 24.
> > - atmel,pmecc-sector-size : sector size for ECC computation.
> > Supported values
Best Regards,
Wenyou Yang
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v1 3/5] mtd: atmel_nand: Support PMECC on SAMA5D2
2016-01-14 1:17 ` Yang, Wenyou
@ 2016-01-14 13:14 ` Rob Herring
-1 siblings, 0 replies; 47+ messages in thread
From: Rob Herring @ 2016-01-14 13:14 UTC (permalink / raw)
To: Yang, Wenyou
Cc: Romain Izard, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Josh Wu, Ferre, Nicolas
On Wed, Jan 13, 2016 at 7:17 PM, Yang, Wenyou <Wenyou.Yang-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org> wrote:
>
>
>> -----Original Message-----
>> From: Rob Herring [mailto:robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org]
>> Sent: 2016年1月14日 9:13
>> To: Romain Izard <romain.izard.pro-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org; devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Yang, Wenyou
>> <Wenyou.Yang-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>; Josh Wu <rainyfeeling-1ViLX0X+lBJBDgjK7y7TUQ@public.gmane.org>; Ferre,
>> Nicolas <Nicolas.FERRE-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
>> Subject: Re: [PATCH v1 3/5] mtd: atmel_nand: Support PMECC on SAMA5D2
>>
>> On Wed, Jan 13, 2016 at 05:34:15PM +0100, Romain Izard wrote:
>> > Starting with the SAMA5D2, there is a new revision of the Atmel PMECC
>> > controller that can correct 32 bits in each sector. This controller is
>> > not 100% compatible with the previous revision that corrected a
>> > maximum of 24 bits by sector, as some register addresses overlap.
>> >
>> > Using information from the device tree, we can configure the driver to
>> > work with both versions.
>> >
>> > Signed-off-by: Romain Izard <romain.izard.pro-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> > ---
>> > .../devicetree/bindings/mtd/atmel-nand.txt | 7 +++++--
>> > drivers/mtd/nand/atmel_nand.c | 23 +++++++++++++++++++++-
>> > drivers/mtd/nand/atmel_nand_ecc.h | 8 ++++++--
>> > 3 files changed, 33 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
>> > b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
>> > index 89b0db9801b0..90887b430f03 100644
>> > --- a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
>> > +++ b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
>> > @@ -1,7 +1,10 @@
>> > Atmel NAND flash
>> >
>> > Required properties:
>> > -- compatible : should be "atmel,at91rm9200-nand" or "atmel,sama5d4-nand".
>> > +- compatible: The possible values are:
>> > + "atmel,at91rm9200-nand"
>> > + "atmel,sama5d2-nand"
>> > + "atmel,sama5d4-nand"
>> > - reg : should specify localbus address and size used for the chip,
>> > and hardware ECC controller if available.
>> > If the hardware ECC is PMECC, it should contain address and size for
>> > @@ -22,7 +25,7 @@ Optional properties:
>> > Supported values are: "none", "soft", "hw", "hw_syndrome", "hw_oob_first",
>> > "soft_bch".
>> > - atmel,has-pmecc : boolean to enable Programmable Multibit ECC hardware.
>> > - Only supported by at91sam9x5 or later sam9 product.
>> > + Supported by AT91SAM9x5 or later SAM9 chips, and SAMA5 chips.
>>
>> What compatible string would AT91SAM9x5 be?
>
> "atmel,at91rm9200-nand".
Answer the question in the binding doc by saying which compatible
strings a property is valid for.
Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v1 3/5] mtd: atmel_nand: Support PMECC on SAMA5D2
@ 2016-01-14 13:14 ` Rob Herring
0 siblings, 0 replies; 47+ messages in thread
From: Rob Herring @ 2016-01-14 13:14 UTC (permalink / raw)
To: Yang, Wenyou; +Cc: Romain Izard, linux-mtd, devicetree, Josh Wu, Ferre, Nicolas
On Wed, Jan 13, 2016 at 7:17 PM, Yang, Wenyou <Wenyou.Yang@atmel.com> wrote:
>
>
>> -----Original Message-----
>> From: Rob Herring [mailto:robh@kernel.org]
>> Sent: 2016年1月14日 9:13
>> To: Romain Izard <romain.izard.pro@gmail.com>
>> Cc: linux-mtd@lists.infradead.org; devicetree@vger.kernel.org; Yang, Wenyou
>> <Wenyou.Yang@atmel.com>; Josh Wu <rainyfeeling@outlook.com>; Ferre,
>> Nicolas <Nicolas.FERRE@atmel.com>
>> Subject: Re: [PATCH v1 3/5] mtd: atmel_nand: Support PMECC on SAMA5D2
>>
>> On Wed, Jan 13, 2016 at 05:34:15PM +0100, Romain Izard wrote:
>> > Starting with the SAMA5D2, there is a new revision of the Atmel PMECC
>> > controller that can correct 32 bits in each sector. This controller is
>> > not 100% compatible with the previous revision that corrected a
>> > maximum of 24 bits by sector, as some register addresses overlap.
>> >
>> > Using information from the device tree, we can configure the driver to
>> > work with both versions.
>> >
>> > Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
>> > ---
>> > .../devicetree/bindings/mtd/atmel-nand.txt | 7 +++++--
>> > drivers/mtd/nand/atmel_nand.c | 23 +++++++++++++++++++++-
>> > drivers/mtd/nand/atmel_nand_ecc.h | 8 ++++++--
>> > 3 files changed, 33 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
>> > b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
>> > index 89b0db9801b0..90887b430f03 100644
>> > --- a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
>> > +++ b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
>> > @@ -1,7 +1,10 @@
>> > Atmel NAND flash
>> >
>> > Required properties:
>> > -- compatible : should be "atmel,at91rm9200-nand" or "atmel,sama5d4-nand".
>> > +- compatible: The possible values are:
>> > + "atmel,at91rm9200-nand"
>> > + "atmel,sama5d2-nand"
>> > + "atmel,sama5d4-nand"
>> > - reg : should specify localbus address and size used for the chip,
>> > and hardware ECC controller if available.
>> > If the hardware ECC is PMECC, it should contain address and size for
>> > @@ -22,7 +25,7 @@ Optional properties:
>> > Supported values are: "none", "soft", "hw", "hw_syndrome", "hw_oob_first",
>> > "soft_bch".
>> > - atmel,has-pmecc : boolean to enable Programmable Multibit ECC hardware.
>> > - Only supported by at91sam9x5 or later sam9 product.
>> > + Supported by AT91SAM9x5 or later SAM9 chips, and SAMA5 chips.
>>
>> What compatible string would AT91SAM9x5 be?
>
> "atmel,at91rm9200-nand".
Answer the question in the binding doc by saying which compatible
strings a property is valid for.
Rob
^ permalink raw reply [flat|nested] 47+ messages in thread
[parent not found: <CAL_JsqL9kbNYrGYeqT7QCsbMneJbPgoXfaHhCvL8S=t97YaCbg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* RE: [PATCH v1 3/5] mtd: atmel_nand: Support PMECC on SAMA5D2
2016-01-14 13:14 ` Rob Herring
@ 2016-01-15 1:17 ` Yang, Wenyou
-1 siblings, 0 replies; 47+ messages in thread
From: Yang, Wenyou @ 2016-01-15 1:17 UTC (permalink / raw)
To: Rob Herring
Cc: Romain Izard, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Josh Wu, Ferre, Nicolas
Hi Rob,
> -----Original Message-----
> From: Rob Herring [mailto:robh@kernel.org]
> Sent: 2016年1月14日 21:15
> To: Yang, Wenyou <Wenyou.Yang@atmel.com>
> Cc: Romain Izard <romain.izard.pro@gmail.com>; linux-mtd@lists.infradead.org;
> devicetree@vger.kernel.org; Josh Wu <rainyfeeling@outlook.com>; Ferre,
> Nicolas <Nicolas.FERRE@atmel.com>
> Subject: Re: [PATCH v1 3/5] mtd: atmel_nand: Support PMECC on SAMA5D2
>
> On Wed, Jan 13, 2016 at 7:17 PM, Yang, Wenyou <Wenyou.Yang@atmel.com>
> wrote:
> >
> >
> >> -----Original Message-----
> >> From: Rob Herring [mailto:robh@kernel.org]
> >> Sent: 2016年1月14日 9:13
> >> To: Romain Izard <romain.izard.pro@gmail.com>
> >> Cc: linux-mtd@lists.infradead.org; devicetree@vger.kernel.org; Yang,
> >> Wenyou <Wenyou.Yang@atmel.com>; Josh Wu <rainyfeeling@outlook.com>;
> >> Ferre, Nicolas <Nicolas.FERRE@atmel.com>
> >> Subject: Re: [PATCH v1 3/5] mtd: atmel_nand: Support PMECC on SAMA5D2
> >>
> >> On Wed, Jan 13, 2016 at 05:34:15PM +0100, Romain Izard wrote:
> >> > Starting with the SAMA5D2, there is a new revision of the Atmel
> >> > PMECC controller that can correct 32 bits in each sector. This
> >> > controller is not 100% compatible with the previous revision that
> >> > corrected a maximum of 24 bits by sector, as some register addresses
> overlap.
> >> >
> >> > Using information from the device tree, we can configure the driver
> >> > to work with both versions.
> >> >
> >> > Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
> >> > ---
> >> > .../devicetree/bindings/mtd/atmel-nand.txt | 7 +++++--
> >> > drivers/mtd/nand/atmel_nand.c | 23
> +++++++++++++++++++++-
> >> > drivers/mtd/nand/atmel_nand_ecc.h | 8 ++++++--
> >> > 3 files changed, 33 insertions(+), 5 deletions(-)
> >> >
> >> > diff --git a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> >> > b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> >> > index 89b0db9801b0..90887b430f03 100644
> >> > --- a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> >> > +++ b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> >> > @@ -1,7 +1,10 @@
> >> > Atmel NAND flash
> >> >
> >> > Required properties:
> >> > -- compatible : should be "atmel,at91rm9200-nand" or "atmel,sama5d4-nand".
> >> > +- compatible: The possible values are:
> >> > + "atmel,at91rm9200-nand"
> >> > + "atmel,sama5d2-nand"
> >> > + "atmel,sama5d4-nand"
> >> > - reg : should specify localbus address and size used for the chip,
> >> > and hardware ECC controller if available.
> >> > If the hardware ECC is PMECC, it should contain address and
> >> > size for @@ -22,7 +25,7 @@ Optional properties:
> >> > Supported values are: "none", "soft", "hw", "hw_syndrome", "hw_oob_first",
> >> > "soft_bch".
> >> > - atmel,has-pmecc : boolean to enable Programmable Multibit ECC
> hardware.
> >> > - Only supported by at91sam9x5 or later sam9 product.
> >> > + Supported by AT91SAM9x5 or later SAM9 chips, and SAMA5 chips.
> >>
> >> What compatible string would AT91SAM9x5 be?
> >
> > "atmel,at91rm9200-nand".
>
> Answer the question in the binding doc by saying which compatible strings a
> property is valid for.
>
Thank you for your advice. I will keep in mind next time.
Best Regards,
Wenyou Yang
^ permalink raw reply [flat|nested] 47+ messages in thread
* RE: [PATCH v1 3/5] mtd: atmel_nand: Support PMECC on SAMA5D2
@ 2016-01-15 1:17 ` Yang, Wenyou
0 siblings, 0 replies; 47+ messages in thread
From: Yang, Wenyou @ 2016-01-15 1:17 UTC (permalink / raw)
To: Rob Herring; +Cc: Romain Izard, linux-mtd, devicetree, Josh Wu, Ferre, Nicolas
Hi Rob,
> -----Original Message-----
> From: Rob Herring [mailto:robh@kernel.org]
> Sent: 2016年1月14日 21:15
> To: Yang, Wenyou <Wenyou.Yang@atmel.com>
> Cc: Romain Izard <romain.izard.pro@gmail.com>; linux-mtd@lists.infradead.org;
> devicetree@vger.kernel.org; Josh Wu <rainyfeeling@outlook.com>; Ferre,
> Nicolas <Nicolas.FERRE@atmel.com>
> Subject: Re: [PATCH v1 3/5] mtd: atmel_nand: Support PMECC on SAMA5D2
>
> On Wed, Jan 13, 2016 at 7:17 PM, Yang, Wenyou <Wenyou.Yang@atmel.com>
> wrote:
> >
> >
> >> -----Original Message-----
> >> From: Rob Herring [mailto:robh@kernel.org]
> >> Sent: 2016年1月14日 9:13
> >> To: Romain Izard <romain.izard.pro@gmail.com>
> >> Cc: linux-mtd@lists.infradead.org; devicetree@vger.kernel.org; Yang,
> >> Wenyou <Wenyou.Yang@atmel.com>; Josh Wu <rainyfeeling@outlook.com>;
> >> Ferre, Nicolas <Nicolas.FERRE@atmel.com>
> >> Subject: Re: [PATCH v1 3/5] mtd: atmel_nand: Support PMECC on SAMA5D2
> >>
> >> On Wed, Jan 13, 2016 at 05:34:15PM +0100, Romain Izard wrote:
> >> > Starting with the SAMA5D2, there is a new revision of the Atmel
> >> > PMECC controller that can correct 32 bits in each sector. This
> >> > controller is not 100% compatible with the previous revision that
> >> > corrected a maximum of 24 bits by sector, as some register addresses
> overlap.
> >> >
> >> > Using information from the device tree, we can configure the driver
> >> > to work with both versions.
> >> >
> >> > Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
> >> > ---
> >> > .../devicetree/bindings/mtd/atmel-nand.txt | 7 +++++--
> >> > drivers/mtd/nand/atmel_nand.c | 23
> +++++++++++++++++++++-
> >> > drivers/mtd/nand/atmel_nand_ecc.h | 8 ++++++--
> >> > 3 files changed, 33 insertions(+), 5 deletions(-)
> >> >
> >> > diff --git a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> >> > b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> >> > index 89b0db9801b0..90887b430f03 100644
> >> > --- a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> >> > +++ b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> >> > @@ -1,7 +1,10 @@
> >> > Atmel NAND flash
> >> >
> >> > Required properties:
> >> > -- compatible : should be "atmel,at91rm9200-nand" or "atmel,sama5d4-nand".
> >> > +- compatible: The possible values are:
> >> > + "atmel,at91rm9200-nand"
> >> > + "atmel,sama5d2-nand"
> >> > + "atmel,sama5d4-nand"
> >> > - reg : should specify localbus address and size used for the chip,
> >> > and hardware ECC controller if available.
> >> > If the hardware ECC is PMECC, it should contain address and
> >> > size for @@ -22,7 +25,7 @@ Optional properties:
> >> > Supported values are: "none", "soft", "hw", "hw_syndrome", "hw_oob_first",
> >> > "soft_bch".
> >> > - atmel,has-pmecc : boolean to enable Programmable Multibit ECC
> hardware.
> >> > - Only supported by at91sam9x5 or later sam9 product.
> >> > + Supported by AT91SAM9x5 or later SAM9 chips, and SAMA5 chips.
> >>
> >> What compatible string would AT91SAM9x5 be?
> >
> > "atmel,at91rm9200-nand".
>
> Answer the question in the binding doc by saying which compatible strings a
> property is valid for.
>
Thank you for your advice. I will keep in mind next time.
Best Regards,
Wenyou Yang
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v1 3/5] mtd: atmel_nand: Support PMECC on SAMA5D2
2016-01-14 13:14 ` Rob Herring
@ 2016-01-15 8:54 ` Romain Izard
-1 siblings, 0 replies; 47+ messages in thread
From: Romain Izard @ 2016-01-15 8:54 UTC (permalink / raw)
To: Rob Herring
Cc: Yang, Wenyou, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Josh Wu, Ferre, Nicolas
Hi Rob, Wenyou,
2016-01-14 14:14 GMT+01:00 Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>:
> On Wed, Jan 13, 2016 at 7:17 PM, Yang, Wenyou <Wenyou.Yang-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org> wrote:
>>
>>
>>> -----Original Message-----
>>> From: Rob Herring [mailto:robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org]
>>> Sent: 2016年1月14日 9:13
>>> To: Romain Izard <romain.izard.pro-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org; devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Yang, Wenyou
>>> <Wenyou.Yang-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>; Josh Wu <rainyfeeling-1ViLX0X+lBJBDgjK7y7TUQ@public.gmane.org>; Ferre,
>>> Nicolas <Nicolas.FERRE-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
>>> Subject: Re: [PATCH v1 3/5] mtd: atmel_nand: Support PMECC on SAMA5D2
>>>
>>> On Wed, Jan 13, 2016 at 05:34:15PM +0100, Romain Izard wrote:
>>> > Starting with the SAMA5D2, there is a new revision of the Atmel PMECC
>>> > controller that can correct 32 bits in each sector. This controller is
>>> > not 100% compatible with the previous revision that corrected a
>>> > maximum of 24 bits by sector, as some register addresses overlap.
>>> >
>>> > Using information from the device tree, we can configure the driver to
>>> > work with both versions.
>>> >
>>> > Signed-off-by: Romain Izard <romain.izard.pro-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> > ---
>>> > .../devicetree/bindings/mtd/atmel-nand.txt | 7 +++++--
>>> > drivers/mtd/nand/atmel_nand.c | 23 +++++++++++++++++++++-
>>> > drivers/mtd/nand/atmel_nand_ecc.h | 8 ++++++--
>>> > 3 files changed, 33 insertions(+), 5 deletions(-)
>>> >
>>> > diff --git a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
>>> > b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
>>> > index 89b0db9801b0..90887b430f03 100644
>>> > --- a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
>>> > +++ b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
>>> > @@ -1,7 +1,10 @@
>>> > Atmel NAND flash
>>> >
>>> > Required properties:
>>> > -- compatible : should be "atmel,at91rm9200-nand" or "atmel,sama5d4-nand".
>>> > +- compatible: The possible values are:
>>> > + "atmel,at91rm9200-nand"
>>> > + "atmel,sama5d2-nand"
>>> > + "atmel,sama5d4-nand"
>>> > - reg : should specify localbus address and size used for the chip,
>>> > and hardware ECC controller if available.
>>> > If the hardware ECC is PMECC, it should contain address and size for
>>> > @@ -22,7 +25,7 @@ Optional properties:
>>> > Supported values are: "none", "soft", "hw", "hw_syndrome", "hw_oob_first",
>>> > "soft_bch".
>>> > - atmel,has-pmecc : boolean to enable Programmable Multibit ECC hardware.
>>> > - Only supported by at91sam9x5 or later sam9 product.
>>> > + Supported by AT91SAM9x5 or later SAM9 chips, and SAMA5 chips.
>>>
>>> What compatible string would AT91SAM9x5 be?
>>
>> "atmel,at91rm9200-nand".
>
> Answer the question in the binding doc by saying which compatible
> strings a property is valid for.
I'm trying to rewrite the documentation for this point, and in the end it
seems to me that the line should be removed instead.
Adding the 'atmel,has-pmecc' property to a SoC device tree is the normal way
to describe that it supports this type of controller. If we need to write it
in the documentation as well, it's a duplicate information that can become
stale quite fast, as it was before.
In the end, I believe the best path is to describe better what PMECC is,
instead of describing as "the controller found in this kind of chip".
Best regards,
--
Romain Izard
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v1 3/5] mtd: atmel_nand: Support PMECC on SAMA5D2
@ 2016-01-15 8:54 ` Romain Izard
0 siblings, 0 replies; 47+ messages in thread
From: Romain Izard @ 2016-01-15 8:54 UTC (permalink / raw)
To: Rob Herring; +Cc: Yang, Wenyou, linux-mtd, devicetree, Josh Wu, Ferre, Nicolas
Hi Rob, Wenyou,
2016-01-14 14:14 GMT+01:00 Rob Herring <robh@kernel.org>:
> On Wed, Jan 13, 2016 at 7:17 PM, Yang, Wenyou <Wenyou.Yang@atmel.com> wrote:
>>
>>
>>> -----Original Message-----
>>> From: Rob Herring [mailto:robh@kernel.org]
>>> Sent: 2016年1月14日 9:13
>>> To: Romain Izard <romain.izard.pro@gmail.com>
>>> Cc: linux-mtd@lists.infradead.org; devicetree@vger.kernel.org; Yang, Wenyou
>>> <Wenyou.Yang@atmel.com>; Josh Wu <rainyfeeling@outlook.com>; Ferre,
>>> Nicolas <Nicolas.FERRE@atmel.com>
>>> Subject: Re: [PATCH v1 3/5] mtd: atmel_nand: Support PMECC on SAMA5D2
>>>
>>> On Wed, Jan 13, 2016 at 05:34:15PM +0100, Romain Izard wrote:
>>> > Starting with the SAMA5D2, there is a new revision of the Atmel PMECC
>>> > controller that can correct 32 bits in each sector. This controller is
>>> > not 100% compatible with the previous revision that corrected a
>>> > maximum of 24 bits by sector, as some register addresses overlap.
>>> >
>>> > Using information from the device tree, we can configure the driver to
>>> > work with both versions.
>>> >
>>> > Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
>>> > ---
>>> > .../devicetree/bindings/mtd/atmel-nand.txt | 7 +++++--
>>> > drivers/mtd/nand/atmel_nand.c | 23 +++++++++++++++++++++-
>>> > drivers/mtd/nand/atmel_nand_ecc.h | 8 ++++++--
>>> > 3 files changed, 33 insertions(+), 5 deletions(-)
>>> >
>>> > diff --git a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
>>> > b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
>>> > index 89b0db9801b0..90887b430f03 100644
>>> > --- a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
>>> > +++ b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
>>> > @@ -1,7 +1,10 @@
>>> > Atmel NAND flash
>>> >
>>> > Required properties:
>>> > -- compatible : should be "atmel,at91rm9200-nand" or "atmel,sama5d4-nand".
>>> > +- compatible: The possible values are:
>>> > + "atmel,at91rm9200-nand"
>>> > + "atmel,sama5d2-nand"
>>> > + "atmel,sama5d4-nand"
>>> > - reg : should specify localbus address and size used for the chip,
>>> > and hardware ECC controller if available.
>>> > If the hardware ECC is PMECC, it should contain address and size for
>>> > @@ -22,7 +25,7 @@ Optional properties:
>>> > Supported values are: "none", "soft", "hw", "hw_syndrome", "hw_oob_first",
>>> > "soft_bch".
>>> > - atmel,has-pmecc : boolean to enable Programmable Multibit ECC hardware.
>>> > - Only supported by at91sam9x5 or later sam9 product.
>>> > + Supported by AT91SAM9x5 or later SAM9 chips, and SAMA5 chips.
>>>
>>> What compatible string would AT91SAM9x5 be?
>>
>> "atmel,at91rm9200-nand".
>
> Answer the question in the binding doc by saying which compatible
> strings a property is valid for.
I'm trying to rewrite the documentation for this point, and in the end it
seems to me that the line should be removed instead.
Adding the 'atmel,has-pmecc' property to a SoC device tree is the normal way
to describe that it supports this type of controller. If we need to write it
in the documentation as well, it's a duplicate information that can become
stale quite fast, as it was before.
In the end, I believe the best path is to describe better what PMECC is,
instead of describing as "the controller found in this kind of chip".
Best regards,
--
Romain Izard
^ permalink raw reply [flat|nested] 47+ messages in thread
[parent not found: <CAGkQfmO94gQ61prZvF52=B_UtOTkbYxd5nnTmDhJam4q-n3T7g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH v1 3/5] mtd: atmel_nand: Support PMECC on SAMA5D2
2016-01-15 8:54 ` Romain Izard
@ 2016-01-17 4:05 ` Rob Herring
-1 siblings, 0 replies; 47+ messages in thread
From: Rob Herring @ 2016-01-17 4:05 UTC (permalink / raw)
To: Romain Izard
Cc: Yang, Wenyou, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Josh Wu, Ferre, Nicolas
On Fri, Jan 15, 2016 at 09:54:07AM +0100, Romain Izard wrote:
> Hi Rob, Wenyou,
>
> 2016-01-14 14:14 GMT+01:00 Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>:
> > On Wed, Jan 13, 2016 at 7:17 PM, Yang, Wenyou <Wenyou.Yang-AIFe0yeh4nA@public.gmane.orgm> wrote:
> >>
> >>
> >>> -----Original Message-----
> >>> From: Rob Herring [mailto:robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org]
> >>> Sent: 2016年1月14日 9:13
> >>> To: Romain Izard <romain.izard.pro-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >>> Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org; devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Yang, Wenyou
> >>> <Wenyou.Yang-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>; Josh Wu <rainyfeeling-1ViLX0X+lBJBDgjK7y7TUQ@public.gmane.org>; Ferre,
> >>> Nicolas <Nicolas.FERRE-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
> >>> Subject: Re: [PATCH v1 3/5] mtd: atmel_nand: Support PMECC on SAMA5D2
> >>>
> >>> On Wed, Jan 13, 2016 at 05:34:15PM +0100, Romain Izard wrote:
> >>> > Starting with the SAMA5D2, there is a new revision of the Atmel PMECC
> >>> > controller that can correct 32 bits in each sector. This controller is
> >>> > not 100% compatible with the previous revision that corrected a
> >>> > maximum of 24 bits by sector, as some register addresses overlap.
> >>> >
> >>> > Using information from the device tree, we can configure the driver to
> >>> > work with both versions.
> >>> >
> >>> > Signed-off-by: Romain Izard <romain.izard.pro-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >>> > ---
> >>> > .../devicetree/bindings/mtd/atmel-nand.txt | 7 +++++--
> >>> > drivers/mtd/nand/atmel_nand.c | 23 +++++++++++++++++++++-
> >>> > drivers/mtd/nand/atmel_nand_ecc.h | 8 ++++++--
> >>> > 3 files changed, 33 insertions(+), 5 deletions(-)
> >>> >
> >>> > diff --git a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> >>> > b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> >>> > index 89b0db9801b0..90887b430f03 100644
> >>> > --- a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> >>> > +++ b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> >>> > @@ -1,7 +1,10 @@
> >>> > Atmel NAND flash
> >>> >
> >>> > Required properties:
> >>> > -- compatible : should be "atmel,at91rm9200-nand" or "atmel,sama5d4-nand".
> >>> > +- compatible: The possible values are:
> >>> > + "atmel,at91rm9200-nand"
> >>> > + "atmel,sama5d2-nand"
> >>> > + "atmel,sama5d4-nand"
> >>> > - reg : should specify localbus address and size used for the chip,
> >>> > and hardware ECC controller if available.
> >>> > If the hardware ECC is PMECC, it should contain address and size for
> >>> > @@ -22,7 +25,7 @@ Optional properties:
> >>> > Supported values are: "none", "soft", "hw", "hw_syndrome", "hw_oob_first",
> >>> > "soft_bch".
> >>> > - atmel,has-pmecc : boolean to enable Programmable Multibit ECC hardware.
> >>> > - Only supported by at91sam9x5 or later sam9 product.
> >>> > + Supported by AT91SAM9x5 or later SAM9 chips, and SAMA5 chips.
> >>>
> >>> What compatible string would AT91SAM9x5 be?
> >>
> >> "atmel,at91rm9200-nand".
> >
> > Answer the question in the binding doc by saying which compatible
> > strings a property is valid for.
>
> I'm trying to rewrite the documentation for this point, and in the end it
> seems to me that the line should be removed instead.
>
> Adding the 'atmel,has-pmecc' property to a SoC device tree is the normal way
> to describe that it supports this type of controller. If we need to write it
> in the documentation as well, it's a duplicate information that can become
> stale quite fast, as it was before.
>
> In the end, I believe the best path is to describe better what PMECC is,
> instead of describing as "the controller found in this kind of chip".
Okay, that is important too. We are wanting to move binding
documentation to something more structured where we can do some amount
of validation of dts files against binding documentation. So that is why
I am thinking in terms of having the information to validate a dts. But
we are a ways off from doing such a thing.
Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v1 3/5] mtd: atmel_nand: Support PMECC on SAMA5D2
@ 2016-01-17 4:05 ` Rob Herring
0 siblings, 0 replies; 47+ messages in thread
From: Rob Herring @ 2016-01-17 4:05 UTC (permalink / raw)
To: Romain Izard; +Cc: Yang, Wenyou, linux-mtd, devicetree, Josh Wu, Ferre, Nicolas
On Fri, Jan 15, 2016 at 09:54:07AM +0100, Romain Izard wrote:
> Hi Rob, Wenyou,
>
> 2016-01-14 14:14 GMT+01:00 Rob Herring <robh@kernel.org>:
> > On Wed, Jan 13, 2016 at 7:17 PM, Yang, Wenyou <Wenyou.Yang@atmel.com> wrote:
> >>
> >>
> >>> -----Original Message-----
> >>> From: Rob Herring [mailto:robh@kernel.org]
> >>> Sent: 2016年1月14日 9:13
> >>> To: Romain Izard <romain.izard.pro@gmail.com>
> >>> Cc: linux-mtd@lists.infradead.org; devicetree@vger.kernel.org; Yang, Wenyou
> >>> <Wenyou.Yang@atmel.com>; Josh Wu <rainyfeeling@outlook.com>; Ferre,
> >>> Nicolas <Nicolas.FERRE@atmel.com>
> >>> Subject: Re: [PATCH v1 3/5] mtd: atmel_nand: Support PMECC on SAMA5D2
> >>>
> >>> On Wed, Jan 13, 2016 at 05:34:15PM +0100, Romain Izard wrote:
> >>> > Starting with the SAMA5D2, there is a new revision of the Atmel PMECC
> >>> > controller that can correct 32 bits in each sector. This controller is
> >>> > not 100% compatible with the previous revision that corrected a
> >>> > maximum of 24 bits by sector, as some register addresses overlap.
> >>> >
> >>> > Using information from the device tree, we can configure the driver to
> >>> > work with both versions.
> >>> >
> >>> > Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
> >>> > ---
> >>> > .../devicetree/bindings/mtd/atmel-nand.txt | 7 +++++--
> >>> > drivers/mtd/nand/atmel_nand.c | 23 +++++++++++++++++++++-
> >>> > drivers/mtd/nand/atmel_nand_ecc.h | 8 ++++++--
> >>> > 3 files changed, 33 insertions(+), 5 deletions(-)
> >>> >
> >>> > diff --git a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> >>> > b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> >>> > index 89b0db9801b0..90887b430f03 100644
> >>> > --- a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> >>> > +++ b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> >>> > @@ -1,7 +1,10 @@
> >>> > Atmel NAND flash
> >>> >
> >>> > Required properties:
> >>> > -- compatible : should be "atmel,at91rm9200-nand" or "atmel,sama5d4-nand".
> >>> > +- compatible: The possible values are:
> >>> > + "atmel,at91rm9200-nand"
> >>> > + "atmel,sama5d2-nand"
> >>> > + "atmel,sama5d4-nand"
> >>> > - reg : should specify localbus address and size used for the chip,
> >>> > and hardware ECC controller if available.
> >>> > If the hardware ECC is PMECC, it should contain address and size for
> >>> > @@ -22,7 +25,7 @@ Optional properties:
> >>> > Supported values are: "none", "soft", "hw", "hw_syndrome", "hw_oob_first",
> >>> > "soft_bch".
> >>> > - atmel,has-pmecc : boolean to enable Programmable Multibit ECC hardware.
> >>> > - Only supported by at91sam9x5 or later sam9 product.
> >>> > + Supported by AT91SAM9x5 or later SAM9 chips, and SAMA5 chips.
> >>>
> >>> What compatible string would AT91SAM9x5 be?
> >>
> >> "atmel,at91rm9200-nand".
> >
> > Answer the question in the binding doc by saying which compatible
> > strings a property is valid for.
>
> I'm trying to rewrite the documentation for this point, and in the end it
> seems to me that the line should be removed instead.
>
> Adding the 'atmel,has-pmecc' property to a SoC device tree is the normal way
> to describe that it supports this type of controller. If we need to write it
> in the documentation as well, it's a duplicate information that can become
> stale quite fast, as it was before.
>
> In the end, I believe the best path is to describe better what PMECC is,
> instead of describing as "the controller found in this kind of chip".
Okay, that is important too. We are wanting to move binding
documentation to something more structured where we can do some amount
of validation of dts files against binding documentation. So that is why
I am thinking in terms of having the information to validate a dts. But
we are a ways off from doing such a thing.
Rob
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v1 3/5] mtd: atmel_nand: Support PMECC on SAMA5D2
2016-01-13 16:34 ` Romain Izard
@ 2016-01-14 9:19 ` Boris Brezillon
-1 siblings, 0 replies; 47+ messages in thread
From: Boris Brezillon @ 2016-01-14 9:19 UTC (permalink / raw)
To: Romain Izard
Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Josh Wu, Nicolas Ferre,
Yang Wenyou
On Wed, 13 Jan 2016 17:34:15 +0100
Romain Izard <romain.izard.pro-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Starting with the SAMA5D2, there is a new revision of the Atmel PMECC
> controller that can correct 32 bits in each sector. This controller is
> not 100% compatible with the previous revision that corrected a maximum
> of 24 bits by sector, as some register addresses overlap.
>
> Using information from the device tree, we can configure the driver to
> work with both versions.
>
> Signed-off-by: Romain Izard <romain.izard.pro-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> .../devicetree/bindings/mtd/atmel-nand.txt | 7 +++++--
> drivers/mtd/nand/atmel_nand.c | 23 +++++++++++++++++++++-
> drivers/mtd/nand/atmel_nand_ecc.h | 8 ++++++--
> 3 files changed, 33 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mtd/atmel-nand.txt b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> index 89b0db9801b0..90887b430f03 100644
> --- a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> @@ -1,7 +1,10 @@
> Atmel NAND flash
>
> Required properties:
> -- compatible : should be "atmel,at91rm9200-nand" or "atmel,sama5d4-nand".
> +- compatible: The possible values are:
> + "atmel,at91rm9200-nand"
> + "atmel,sama5d2-nand"
> + "atmel,sama5d4-nand"
> - reg : should specify localbus address and size used for the chip,
> and hardware ECC controller if available.
> If the hardware ECC is PMECC, it should contain address and size for
> @@ -22,7 +25,7 @@ Optional properties:
> Supported values are: "none", "soft", "hw", "hw_syndrome", "hw_oob_first",
> "soft_bch".
> - atmel,has-pmecc : boolean to enable Programmable Multibit ECC hardware.
> - Only supported by at91sam9x5 or later sam9 product.
> + Supported by AT91SAM9x5 or later SAM9 chips, and SAMA5 chips.
> - atmel,pmecc-cap : error correct capability for Programmable Multibit ECC
> Controller. Supported values are: 2, 4, 8, 12, 24.
> - atmel,pmecc-sector-size : sector size for ECC computation. Supported values
> diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
> index e5d7e7e63f49..6fe50e2d291f 100644
> --- a/drivers/mtd/nand/atmel_nand.c
> +++ b/drivers/mtd/nand/atmel_nand.c
> @@ -65,6 +65,7 @@ module_param(on_flash_bbt, int, 0);
>
> struct atmel_nand_caps {
> bool pmecc_correct_erase_page;
> + uint8_t pmecc_max_correction;
> };
>
> struct atmel_nand_nfc_priv {
> @@ -146,6 +147,7 @@ struct atmel_nand_host {
> int pmecc_cw_len; /* Length of codeword */
>
> void __iomem *pmerrloc_base;
> + void __iomem *pmerrloc_el_base;
> void __iomem *pmecc_rom_base;
>
> /* lookup table for alpha_to and index_of */
> @@ -817,7 +819,7 @@ static void pmecc_correct_data(struct mtd_info *mtd, uint8_t *buf, uint8_t *ecc,
> sector_size = host->pmecc_sector_size;
>
> while (err_nbr) {
> - tmp = pmerrloc_readl_el_relaxed(host->pmerrloc_base, i) - 1;
> + tmp = pmerrloc_readl_el_relaxed(host->pmerrloc_el_base, i) - 1;
> byte_pos = tmp / 8;
> bit_pos = tmp % 8;
>
> @@ -1208,6 +1210,8 @@ static int atmel_pmecc_nand_init_params(struct platform_device *pdev,
> err_no = PTR_ERR(host->pmerrloc_base);
> goto err;
> }
How about putting the comment you added below here:
/*
* The ATMEL_PMERRLOC_ELx register location depends on the number of
* bits corrected by the PMECC controller. Pre-calculate the
* pmerrloc_el_base according to the PMECC capabilities.
*/
> + host->pmerrloc_el_base = host->pmerrloc_base + ATMEL_PMERRLOC_SIGMAx +
> + (host->caps->pmecc_max_correction + 1) * 4;
>
> if (!host->has_no_lookup_table) {
> regs_rom = platform_get_resource(pdev, IORESOURCE_MEM, 3);
> @@ -2307,17 +2311,34 @@ static int atmel_nand_remove(struct platform_device *pdev)
> return 0;
> }
>
> +/*
> + * AT91RM9200 does not have PMECC or PMECC Errloc peripherals for
> + * BCH ECC. Combined with the "atmel,has-pmecc", it is used to describe
> + * devices from the SAM9 family that have those.
> + */
> static struct atmel_nand_caps at91rm9200_caps = {
> .pmecc_correct_erase_page = false,
> + .pmecc_max_correction = 24,
> };
>
> static struct atmel_nand_caps sama5d4_caps = {
> .pmecc_correct_erase_page = true,
> + .pmecc_max_correction = 24,
> +};
> +
> +/*
> + * The PMECC Errloc controller starting in SAMA5D2 is not compatible,
> + * as the increased correction strength requires more registers.
> + */
> +static struct atmel_nand_caps sama5d2_caps = {
> + .pmecc_correct_erase_page = true,
> + .pmecc_max_correction = 32,
> };
>
> static const struct of_device_id atmel_nand_dt_ids[] = {
> { .compatible = "atmel,at91rm9200-nand", .data = &at91rm9200_caps },
> { .compatible = "atmel,sama5d4-nand", .data = &sama5d4_caps },
> + { .compatible = "atmel,sama5d2-nand", .data = &sama5d2_caps },
> { /* sentinel */ }
> };
>
> diff --git a/drivers/mtd/nand/atmel_nand_ecc.h b/drivers/mtd/nand/atmel_nand_ecc.h
> index 668e7358f19b..ec964c43c932 100644
> --- a/drivers/mtd/nand/atmel_nand_ecc.h
> +++ b/drivers/mtd/nand/atmel_nand_ecc.h
> @@ -108,7 +108,11 @@
> #define PMERRLOC_ERR_NUM_MASK (0x1f << 8)
> #define PMERRLOC_CALC_DONE (1 << 0)
> #define ATMEL_PMERRLOC_SIGMAx 0x028 /* Error location SIGMA x */
> -#define ATMEL_PMERRLOC_ELx 0x08c /* Error location x */
> +
> +/*
> + * The ATMEL_PMERRLOC_ELx register location depends from the number of
> + * bits corrected by the PMECC controller. Do not use it.
> + */
>
> /* Register access macros for PMECC */
> #define pmecc_readl_relaxed(addr, reg) \
> @@ -136,7 +140,7 @@
> readl_relaxed((addr) + ATMEL_PMERRLOC_SIGMAx + ((n) * 4))
>
> #define pmerrloc_readl_el_relaxed(addr, n) \
> - readl_relaxed((addr) + ATMEL_PMERRLOC_ELx + ((n) * 4))
> + readl_relaxed((addr) + ((n) * 4))
Another option would have been to pass the maximum ECC strength here,
and modify the ATMEL_PMERRLOC_ELx macro to take a max_strength argument
and return the adjusted ELx offset.
Anyway, both solutions work for me.
>
> /* Galois field dimension */
> #define PMECC_GF_DIMENSION_13 13
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v1 3/5] mtd: atmel_nand: Support PMECC on SAMA5D2
@ 2016-01-14 9:19 ` Boris Brezillon
0 siblings, 0 replies; 47+ messages in thread
From: Boris Brezillon @ 2016-01-14 9:19 UTC (permalink / raw)
To: Romain Izard; +Cc: linux-mtd, devicetree, Josh Wu, Nicolas Ferre, Yang Wenyou
On Wed, 13 Jan 2016 17:34:15 +0100
Romain Izard <romain.izard.pro@gmail.com> wrote:
> Starting with the SAMA5D2, there is a new revision of the Atmel PMECC
> controller that can correct 32 bits in each sector. This controller is
> not 100% compatible with the previous revision that corrected a maximum
> of 24 bits by sector, as some register addresses overlap.
>
> Using information from the device tree, we can configure the driver to
> work with both versions.
>
> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
> ---
> .../devicetree/bindings/mtd/atmel-nand.txt | 7 +++++--
> drivers/mtd/nand/atmel_nand.c | 23 +++++++++++++++++++++-
> drivers/mtd/nand/atmel_nand_ecc.h | 8 ++++++--
> 3 files changed, 33 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mtd/atmel-nand.txt b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> index 89b0db9801b0..90887b430f03 100644
> --- a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> @@ -1,7 +1,10 @@
> Atmel NAND flash
>
> Required properties:
> -- compatible : should be "atmel,at91rm9200-nand" or "atmel,sama5d4-nand".
> +- compatible: The possible values are:
> + "atmel,at91rm9200-nand"
> + "atmel,sama5d2-nand"
> + "atmel,sama5d4-nand"
> - reg : should specify localbus address and size used for the chip,
> and hardware ECC controller if available.
> If the hardware ECC is PMECC, it should contain address and size for
> @@ -22,7 +25,7 @@ Optional properties:
> Supported values are: "none", "soft", "hw", "hw_syndrome", "hw_oob_first",
> "soft_bch".
> - atmel,has-pmecc : boolean to enable Programmable Multibit ECC hardware.
> - Only supported by at91sam9x5 or later sam9 product.
> + Supported by AT91SAM9x5 or later SAM9 chips, and SAMA5 chips.
> - atmel,pmecc-cap : error correct capability for Programmable Multibit ECC
> Controller. Supported values are: 2, 4, 8, 12, 24.
> - atmel,pmecc-sector-size : sector size for ECC computation. Supported values
> diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
> index e5d7e7e63f49..6fe50e2d291f 100644
> --- a/drivers/mtd/nand/atmel_nand.c
> +++ b/drivers/mtd/nand/atmel_nand.c
> @@ -65,6 +65,7 @@ module_param(on_flash_bbt, int, 0);
>
> struct atmel_nand_caps {
> bool pmecc_correct_erase_page;
> + uint8_t pmecc_max_correction;
> };
>
> struct atmel_nand_nfc_priv {
> @@ -146,6 +147,7 @@ struct atmel_nand_host {
> int pmecc_cw_len; /* Length of codeword */
>
> void __iomem *pmerrloc_base;
> + void __iomem *pmerrloc_el_base;
> void __iomem *pmecc_rom_base;
>
> /* lookup table for alpha_to and index_of */
> @@ -817,7 +819,7 @@ static void pmecc_correct_data(struct mtd_info *mtd, uint8_t *buf, uint8_t *ecc,
> sector_size = host->pmecc_sector_size;
>
> while (err_nbr) {
> - tmp = pmerrloc_readl_el_relaxed(host->pmerrloc_base, i) - 1;
> + tmp = pmerrloc_readl_el_relaxed(host->pmerrloc_el_base, i) - 1;
> byte_pos = tmp / 8;
> bit_pos = tmp % 8;
>
> @@ -1208,6 +1210,8 @@ static int atmel_pmecc_nand_init_params(struct platform_device *pdev,
> err_no = PTR_ERR(host->pmerrloc_base);
> goto err;
> }
How about putting the comment you added below here:
/*
* The ATMEL_PMERRLOC_ELx register location depends on the number of
* bits corrected by the PMECC controller. Pre-calculate the
* pmerrloc_el_base according to the PMECC capabilities.
*/
> + host->pmerrloc_el_base = host->pmerrloc_base + ATMEL_PMERRLOC_SIGMAx +
> + (host->caps->pmecc_max_correction + 1) * 4;
>
> if (!host->has_no_lookup_table) {
> regs_rom = platform_get_resource(pdev, IORESOURCE_MEM, 3);
> @@ -2307,17 +2311,34 @@ static int atmel_nand_remove(struct platform_device *pdev)
> return 0;
> }
>
> +/*
> + * AT91RM9200 does not have PMECC or PMECC Errloc peripherals for
> + * BCH ECC. Combined with the "atmel,has-pmecc", it is used to describe
> + * devices from the SAM9 family that have those.
> + */
> static struct atmel_nand_caps at91rm9200_caps = {
> .pmecc_correct_erase_page = false,
> + .pmecc_max_correction = 24,
> };
>
> static struct atmel_nand_caps sama5d4_caps = {
> .pmecc_correct_erase_page = true,
> + .pmecc_max_correction = 24,
> +};
> +
> +/*
> + * The PMECC Errloc controller starting in SAMA5D2 is not compatible,
> + * as the increased correction strength requires more registers.
> + */
> +static struct atmel_nand_caps sama5d2_caps = {
> + .pmecc_correct_erase_page = true,
> + .pmecc_max_correction = 32,
> };
>
> static const struct of_device_id atmel_nand_dt_ids[] = {
> { .compatible = "atmel,at91rm9200-nand", .data = &at91rm9200_caps },
> { .compatible = "atmel,sama5d4-nand", .data = &sama5d4_caps },
> + { .compatible = "atmel,sama5d2-nand", .data = &sama5d2_caps },
> { /* sentinel */ }
> };
>
> diff --git a/drivers/mtd/nand/atmel_nand_ecc.h b/drivers/mtd/nand/atmel_nand_ecc.h
> index 668e7358f19b..ec964c43c932 100644
> --- a/drivers/mtd/nand/atmel_nand_ecc.h
> +++ b/drivers/mtd/nand/atmel_nand_ecc.h
> @@ -108,7 +108,11 @@
> #define PMERRLOC_ERR_NUM_MASK (0x1f << 8)
> #define PMERRLOC_CALC_DONE (1 << 0)
> #define ATMEL_PMERRLOC_SIGMAx 0x028 /* Error location SIGMA x */
> -#define ATMEL_PMERRLOC_ELx 0x08c /* Error location x */
> +
> +/*
> + * The ATMEL_PMERRLOC_ELx register location depends from the number of
> + * bits corrected by the PMECC controller. Do not use it.
> + */
>
> /* Register access macros for PMECC */
> #define pmecc_readl_relaxed(addr, reg) \
> @@ -136,7 +140,7 @@
> readl_relaxed((addr) + ATMEL_PMERRLOC_SIGMAx + ((n) * 4))
>
> #define pmerrloc_readl_el_relaxed(addr, n) \
> - readl_relaxed((addr) + ATMEL_PMERRLOC_ELx + ((n) * 4))
> + readl_relaxed((addr) + ((n) * 4))
Another option would have been to pass the maximum ECC strength here,
and modify the ATMEL_PMERRLOC_ELx macro to take a max_strength argument
and return the adjusted ELx offset.
Anyway, both solutions work for me.
>
> /* Galois field dimension */
> #define PMECC_GF_DIMENSION_13 13
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v1 3/5] mtd: atmel_nand: Support PMECC on SAMA5D2
2016-01-14 9:19 ` Boris Brezillon
@ 2016-01-15 10:06 ` romain izard
-1 siblings, 0 replies; 47+ messages in thread
From: romain izard @ 2016-01-15 10:06 UTC (permalink / raw)
To: Boris Brezillon
Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Josh Wu, Nicolas Ferre,
Yang Wenyou
Hi Boris,
2016-01-14 10:19 GMT+01:00 Boris Brezillon <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>:
> On Wed, 13 Jan 2016 17:34:15 +0100
> Romain Izard <romain.izard.pro-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>> Starting with the SAMA5D2, there is a new revision of the Atmel PMECC
>> controller that can correct 32 bits in each sector. This controller is
>> not 100% compatible with the previous revision that corrected a maximum
>> of 24 bits by sector, as some register addresses overlap.
>>
>> Using information from the device tree, we can configure the driver to
>> work with both versions.
>>
>> Signed-off-by: Romain Izard <romain.izard.pro-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>> .../devicetree/bindings/mtd/atmel-nand.txt | 7 +++++--
>> drivers/mtd/nand/atmel_nand.c | 23 +++++++++++++++++++++-
>> drivers/mtd/nand/atmel_nand_ecc.h | 8 ++++++--
>> 3 files changed, 33 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/atmel-nand.txt b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
>> index 89b0db9801b0..90887b430f03 100644
>> --- a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
>> +++ b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
>> @@ -1,7 +1,10 @@
>> Atmel NAND flash
>>
>> Required properties:
>> -- compatible : should be "atmel,at91rm9200-nand" or "atmel,sama5d4-nand".
>> +- compatible: The possible values are:
>> + "atmel,at91rm9200-nand"
>> + "atmel,sama5d2-nand"
>> + "atmel,sama5d4-nand"
>> - reg : should specify localbus address and size used for the chip,
>> and hardware ECC controller if available.
>> If the hardware ECC is PMECC, it should contain address and size for
>> @@ -22,7 +25,7 @@ Optional properties:
>> Supported values are: "none", "soft", "hw", "hw_syndrome", "hw_oob_first",
>> "soft_bch".
>> - atmel,has-pmecc : boolean to enable Programmable Multibit ECC hardware.
>> - Only supported by at91sam9x5 or later sam9 product.
>> + Supported by AT91SAM9x5 or later SAM9 chips, and SAMA5 chips.
>> - atmel,pmecc-cap : error correct capability for Programmable Multibit ECC
>> Controller. Supported values are: 2, 4, 8, 12, 24.
>> - atmel,pmecc-sector-size : sector size for ECC computation. Supported values
>> diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
>> index e5d7e7e63f49..6fe50e2d291f 100644
>> --- a/drivers/mtd/nand/atmel_nand.c
>> +++ b/drivers/mtd/nand/atmel_nand.c
>> @@ -65,6 +65,7 @@ module_param(on_flash_bbt, int, 0);
>>
>> struct atmel_nand_caps {
>> bool pmecc_correct_erase_page;
>> + uint8_t pmecc_max_correction;
>> };
>>
>> struct atmel_nand_nfc_priv {
>> @@ -146,6 +147,7 @@ struct atmel_nand_host {
>> int pmecc_cw_len; /* Length of codeword */
>>
>> void __iomem *pmerrloc_base;
>> + void __iomem *pmerrloc_el_base;
>> void __iomem *pmecc_rom_base;
>>
>> /* lookup table for alpha_to and index_of */
>> @@ -817,7 +819,7 @@ static void pmecc_correct_data(struct mtd_info *mtd, uint8_t *buf, uint8_t *ecc,
>> sector_size = host->pmecc_sector_size;
>>
>> while (err_nbr) {
>> - tmp = pmerrloc_readl_el_relaxed(host->pmerrloc_base, i) - 1;
>> + tmp = pmerrloc_readl_el_relaxed(host->pmerrloc_el_base, i) - 1;
>> byte_pos = tmp / 8;
>> bit_pos = tmp % 8;
>>
>> @@ -1208,6 +1210,8 @@ static int atmel_pmecc_nand_init_params(struct platform_device *pdev,
>> err_no = PTR_ERR(host->pmerrloc_base);
>> goto err;
>> }
>
> How about putting the comment you added below here:
>
> /*
> * The ATMEL_PMERRLOC_ELx register location depends on the number of
> * bits corrected by the PMECC controller. Pre-calculate the
> * pmerrloc_el_base according to the PMECC capabilities.
> */
>
Here, this comment is paraphrasing the code. That's not very useful.
>> + host->pmerrloc_el_base = host->pmerrloc_base + ATMEL_PMERRLOC_SIGMAx +
>> + (host->caps->pmecc_max_correction + 1) * 4;
>>
>> if (!host->has_no_lookup_table) {
>> regs_rom = platform_get_resource(pdev, IORESOURCE_MEM, 3);
>> @@ -2307,17 +2311,34 @@ static int atmel_nand_remove(struct platform_device *pdev)
>> return 0;
>> }
>>
>> +/*
>> + * AT91RM9200 does not have PMECC or PMECC Errloc peripherals for
>> + * BCH ECC. Combined with the "atmel,has-pmecc", it is used to describe
>> + * devices from the SAM9 family that have those.
>> + */
>> static struct atmel_nand_caps at91rm9200_caps = {
>> .pmecc_correct_erase_page = false,
>> + .pmecc_max_correction = 24,
>> };
>>
>> static struct atmel_nand_caps sama5d4_caps = {
>> .pmecc_correct_erase_page = true,
>> + .pmecc_max_correction = 24,
>> +};
>> +
>> +/*
>> + * The PMECC Errloc controller starting in SAMA5D2 is not compatible,
>> + * as the increased correction strength requires more registers.
>> + */
>> +static struct atmel_nand_caps sama5d2_caps = {
>> + .pmecc_correct_erase_page = true,
>> + .pmecc_max_correction = 32,
>> };
>>
>> static const struct of_device_id atmel_nand_dt_ids[] = {
>> { .compatible = "atmel,at91rm9200-nand", .data = &at91rm9200_caps },
>> { .compatible = "atmel,sama5d4-nand", .data = &sama5d4_caps },
>> + { .compatible = "atmel,sama5d2-nand", .data = &sama5d2_caps },
>> { /* sentinel */ }
>> };
>>
>> diff --git a/drivers/mtd/nand/atmel_nand_ecc.h b/drivers/mtd/nand/atmel_nand_ecc.h
>> index 668e7358f19b..ec964c43c932 100644
>> --- a/drivers/mtd/nand/atmel_nand_ecc.h
>> +++ b/drivers/mtd/nand/atmel_nand_ecc.h
>> @@ -108,7 +108,11 @@
>> #define PMERRLOC_ERR_NUM_MASK (0x1f << 8)
>> #define PMERRLOC_CALC_DONE (1 << 0)
>> #define ATMEL_PMERRLOC_SIGMAx 0x028 /* Error location SIGMA x */
>> -#define ATMEL_PMERRLOC_ELx 0x08c /* Error location x */
>> +
>> +/*
>> + * The ATMEL_PMERRLOC_ELx register location depends from the number of
>> + * bits corrected by the PMECC controller. Do not use it.
>> + */
>>
>> /* Register access macros for PMECC */
>> #define pmecc_readl_relaxed(addr, reg) \
>> @@ -136,7 +140,7 @@
>> readl_relaxed((addr) + ATMEL_PMERRLOC_SIGMAx + ((n) * 4))
>>
>> #define pmerrloc_readl_el_relaxed(addr, n) \
>> - readl_relaxed((addr) + ATMEL_PMERRLOC_ELx + ((n) * 4))
>> + readl_relaxed((addr) + ((n) * 4))
>
> Another option would have been to pass the maximum ECC strength here,
> and modify the ATMEL_PMERRLOC_ELx macro to take a max_strength argument
> and return the adjusted ELx offset.
> Anyway, both solutions work for me.
>
The current solution makes it easier to keep within the 80 character limit.
The memory/computing tradeoff is so small it won't change anything.
>>
>> /* Galois field dimension */
>> #define PMECC_GF_DIMENSION_13 13
Best regards,
--
Romain Izard
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v1 3/5] mtd: atmel_nand: Support PMECC on SAMA5D2
@ 2016-01-15 10:06 ` romain izard
0 siblings, 0 replies; 47+ messages in thread
From: romain izard @ 2016-01-15 10:06 UTC (permalink / raw)
To: Boris Brezillon
Cc: linux-mtd, devicetree, Josh Wu, Nicolas Ferre, Yang Wenyou
Hi Boris,
2016-01-14 10:19 GMT+01:00 Boris Brezillon <boris.brezillon@free-electrons.com>:
> On Wed, 13 Jan 2016 17:34:15 +0100
> Romain Izard <romain.izard.pro@gmail.com> wrote:
>
>> Starting with the SAMA5D2, there is a new revision of the Atmel PMECC
>> controller that can correct 32 bits in each sector. This controller is
>> not 100% compatible with the previous revision that corrected a maximum
>> of 24 bits by sector, as some register addresses overlap.
>>
>> Using information from the device tree, we can configure the driver to
>> work with both versions.
>>
>> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
>> ---
>> .../devicetree/bindings/mtd/atmel-nand.txt | 7 +++++--
>> drivers/mtd/nand/atmel_nand.c | 23 +++++++++++++++++++++-
>> drivers/mtd/nand/atmel_nand_ecc.h | 8 ++++++--
>> 3 files changed, 33 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/atmel-nand.txt b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
>> index 89b0db9801b0..90887b430f03 100644
>> --- a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
>> +++ b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
>> @@ -1,7 +1,10 @@
>> Atmel NAND flash
>>
>> Required properties:
>> -- compatible : should be "atmel,at91rm9200-nand" or "atmel,sama5d4-nand".
>> +- compatible: The possible values are:
>> + "atmel,at91rm9200-nand"
>> + "atmel,sama5d2-nand"
>> + "atmel,sama5d4-nand"
>> - reg : should specify localbus address and size used for the chip,
>> and hardware ECC controller if available.
>> If the hardware ECC is PMECC, it should contain address and size for
>> @@ -22,7 +25,7 @@ Optional properties:
>> Supported values are: "none", "soft", "hw", "hw_syndrome", "hw_oob_first",
>> "soft_bch".
>> - atmel,has-pmecc : boolean to enable Programmable Multibit ECC hardware.
>> - Only supported by at91sam9x5 or later sam9 product.
>> + Supported by AT91SAM9x5 or later SAM9 chips, and SAMA5 chips.
>> - atmel,pmecc-cap : error correct capability for Programmable Multibit ECC
>> Controller. Supported values are: 2, 4, 8, 12, 24.
>> - atmel,pmecc-sector-size : sector size for ECC computation. Supported values
>> diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
>> index e5d7e7e63f49..6fe50e2d291f 100644
>> --- a/drivers/mtd/nand/atmel_nand.c
>> +++ b/drivers/mtd/nand/atmel_nand.c
>> @@ -65,6 +65,7 @@ module_param(on_flash_bbt, int, 0);
>>
>> struct atmel_nand_caps {
>> bool pmecc_correct_erase_page;
>> + uint8_t pmecc_max_correction;
>> };
>>
>> struct atmel_nand_nfc_priv {
>> @@ -146,6 +147,7 @@ struct atmel_nand_host {
>> int pmecc_cw_len; /* Length of codeword */
>>
>> void __iomem *pmerrloc_base;
>> + void __iomem *pmerrloc_el_base;
>> void __iomem *pmecc_rom_base;
>>
>> /* lookup table for alpha_to and index_of */
>> @@ -817,7 +819,7 @@ static void pmecc_correct_data(struct mtd_info *mtd, uint8_t *buf, uint8_t *ecc,
>> sector_size = host->pmecc_sector_size;
>>
>> while (err_nbr) {
>> - tmp = pmerrloc_readl_el_relaxed(host->pmerrloc_base, i) - 1;
>> + tmp = pmerrloc_readl_el_relaxed(host->pmerrloc_el_base, i) - 1;
>> byte_pos = tmp / 8;
>> bit_pos = tmp % 8;
>>
>> @@ -1208,6 +1210,8 @@ static int atmel_pmecc_nand_init_params(struct platform_device *pdev,
>> err_no = PTR_ERR(host->pmerrloc_base);
>> goto err;
>> }
>
> How about putting the comment you added below here:
>
> /*
> * The ATMEL_PMERRLOC_ELx register location depends on the number of
> * bits corrected by the PMECC controller. Pre-calculate the
> * pmerrloc_el_base according to the PMECC capabilities.
> */
>
Here, this comment is paraphrasing the code. That's not very useful.
>> + host->pmerrloc_el_base = host->pmerrloc_base + ATMEL_PMERRLOC_SIGMAx +
>> + (host->caps->pmecc_max_correction + 1) * 4;
>>
>> if (!host->has_no_lookup_table) {
>> regs_rom = platform_get_resource(pdev, IORESOURCE_MEM, 3);
>> @@ -2307,17 +2311,34 @@ static int atmel_nand_remove(struct platform_device *pdev)
>> return 0;
>> }
>>
>> +/*
>> + * AT91RM9200 does not have PMECC or PMECC Errloc peripherals for
>> + * BCH ECC. Combined with the "atmel,has-pmecc", it is used to describe
>> + * devices from the SAM9 family that have those.
>> + */
>> static struct atmel_nand_caps at91rm9200_caps = {
>> .pmecc_correct_erase_page = false,
>> + .pmecc_max_correction = 24,
>> };
>>
>> static struct atmel_nand_caps sama5d4_caps = {
>> .pmecc_correct_erase_page = true,
>> + .pmecc_max_correction = 24,
>> +};
>> +
>> +/*
>> + * The PMECC Errloc controller starting in SAMA5D2 is not compatible,
>> + * as the increased correction strength requires more registers.
>> + */
>> +static struct atmel_nand_caps sama5d2_caps = {
>> + .pmecc_correct_erase_page = true,
>> + .pmecc_max_correction = 32,
>> };
>>
>> static const struct of_device_id atmel_nand_dt_ids[] = {
>> { .compatible = "atmel,at91rm9200-nand", .data = &at91rm9200_caps },
>> { .compatible = "atmel,sama5d4-nand", .data = &sama5d4_caps },
>> + { .compatible = "atmel,sama5d2-nand", .data = &sama5d2_caps },
>> { /* sentinel */ }
>> };
>>
>> diff --git a/drivers/mtd/nand/atmel_nand_ecc.h b/drivers/mtd/nand/atmel_nand_ecc.h
>> index 668e7358f19b..ec964c43c932 100644
>> --- a/drivers/mtd/nand/atmel_nand_ecc.h
>> +++ b/drivers/mtd/nand/atmel_nand_ecc.h
>> @@ -108,7 +108,11 @@
>> #define PMERRLOC_ERR_NUM_MASK (0x1f << 8)
>> #define PMERRLOC_CALC_DONE (1 << 0)
>> #define ATMEL_PMERRLOC_SIGMAx 0x028 /* Error location SIGMA x */
>> -#define ATMEL_PMERRLOC_ELx 0x08c /* Error location x */
>> +
>> +/*
>> + * The ATMEL_PMERRLOC_ELx register location depends from the number of
>> + * bits corrected by the PMECC controller. Do not use it.
>> + */
>>
>> /* Register access macros for PMECC */
>> #define pmecc_readl_relaxed(addr, reg) \
>> @@ -136,7 +140,7 @@
>> readl_relaxed((addr) + ATMEL_PMERRLOC_SIGMAx + ((n) * 4))
>>
>> #define pmerrloc_readl_el_relaxed(addr, n) \
>> - readl_relaxed((addr) + ATMEL_PMERRLOC_ELx + ((n) * 4))
>> + readl_relaxed((addr) + ((n) * 4))
>
> Another option would have been to pass the maximum ECC strength here,
> and modify the ATMEL_PMERRLOC_ELx macro to take a max_strength argument
> and return the adjusted ELx offset.
> Anyway, both solutions work for me.
>
The current solution makes it easier to keep within the 80 character limit.
The memory/computing tradeoff is so small it won't change anything.
>>
>> /* Galois field dimension */
>> #define PMECC_GF_DIMENSION_13 13
Best regards,
--
Romain Izard
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v1 4/5] mtd: atmel_nand: Support 32-bit ECC strength
2016-01-13 16:34 ` Romain Izard
@ 2016-01-13 16:34 ` Romain Izard
-1 siblings, 0 replies; 47+ messages in thread
From: Romain Izard @ 2016-01-13 16:34 UTC (permalink / raw)
To: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: Yang Wenyou, Josh Wu, Nicolas Ferre, Romain Izard
As the SAMA5D2 controller supports the 32-bit ECC strength, accept it
as a valid setting when required by the device tree or the NAND
parameter page.
Then configure the controller to do use this new setting.
Signed-off-by: Romain Izard <romain.izard.pro-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
.../devicetree/bindings/mtd/atmel-nand.txt | 3 ++-
drivers/mtd/nand/atmel_nand.c | 23 ++++++++++++++++++----
drivers/mtd/nand/atmel_nand_ecc.h | 1 +
3 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/mtd/atmel-nand.txt b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
index 90887b430f03..ef0db8e2a0fd 100644
--- a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
@@ -27,7 +27,8 @@ Optional properties:
- atmel,has-pmecc : boolean to enable Programmable Multibit ECC hardware.
Supported by AT91SAM9x5 or later SAM9 chips, and SAMA5 chips.
- atmel,pmecc-cap : error correct capability for Programmable Multibit ECC
- Controller. Supported values are: 2, 4, 8, 12, 24.
+ Controller. Supported values are: 2, 4, 8, 12, 24. SAMA5D2 also supports
+ 32.
- atmel,pmecc-sector-size : sector size for ECC computation. Supported values
are: 512, 1024.
- atmel,pmecc-lookup-table-offset : includes two offsets of lookup table in ROM
diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index 6fe50e2d291f..920a0a315a60 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -474,6 +474,7 @@ static void atmel_write_buf(struct mtd_info *mtd, const u8 *buf, int len)
* 8-bits 13-bytes 14-bytes
* 12-bits 20-bytes 21-bytes
* 24-bits 39-bytes 42-bytes
+ * 32-bits 52-bytes 56-bytes
*/
static int pmecc_get_ecc_bytes(int cap, int sector_size)
{
@@ -1022,6 +1023,9 @@ static void atmel_pmecc_core_init(struct mtd_info *mtd)
case 24:
val = PMECC_CFG_BCH_ERR24;
break;
+ case 32:
+ val = PMECC_CFG_BCH_ERR32;
+ break;
}
if (host->pmecc_sector_size == 512)
@@ -1083,6 +1087,9 @@ static int pmecc_choose_ecc(struct atmel_nand_host *host,
/* If device tree doesn't specify, use NAND's minimum ECC parameters */
if (host->pmecc_corr_cap == 0) {
+ if (*cap > host->caps->pmecc_max_correction)
+ return -EINVAL;
+
/* use the most fitable ecc bits (the near bigger one ) */
if (*cap <= 2)
host->pmecc_corr_cap = 2;
@@ -1094,6 +1101,8 @@ static int pmecc_choose_ecc(struct atmel_nand_host *host,
host->pmecc_corr_cap = 12;
else if (*cap <= 24)
host->pmecc_corr_cap = 24;
+ else if (*cap <= 32)
+ host->pmecc_corr_cap = 32;
else
return -EINVAL;
}
@@ -1554,10 +1563,16 @@ static int atmel_of_init_port(struct atmel_nand_host *host,
* them from NAND ONFI parameters.
*/
if (of_property_read_u32(np, "atmel,pmecc-cap", &val) == 0) {
- if ((val != 2) && (val != 4) && (val != 8) && (val != 12) &&
- (val != 24)) {
+ if (val > host->caps->pmecc_max_correction) {
+ dev_err(host->dev,
+ "Required ECC strength too high: %u max %u\n",
+ val, host->caps->pmecc_max_correction);
+ return -EINVAL;
+ }
+ if ((val != 2) && (val != 4) && (val != 8) &&
+ (val != 12) && (val != 24) && (val != 32)) {
dev_err(host->dev,
- "Unsupported PMECC correction capability: %d; should be 2, 4, 8, 12 or 24\n",
+ "Required ECC strength not supported: %u\n",
val);
return -EINVAL;
}
@@ -1567,7 +1582,7 @@ static int atmel_of_init_port(struct atmel_nand_host *host,
if (of_property_read_u32(np, "atmel,pmecc-sector-size", &val) == 0) {
if ((val != 512) && (val != 1024)) {
dev_err(host->dev,
- "Unsupported PMECC sector size: %d; should be 512 or 1024 bytes\n",
+ "Required ECC sector size not supported: %u\n",
val);
return -EINVAL;
}
diff --git a/drivers/mtd/nand/atmel_nand_ecc.h b/drivers/mtd/nand/atmel_nand_ecc.h
index ec964c43c932..834d694487bd 100644
--- a/drivers/mtd/nand/atmel_nand_ecc.h
+++ b/drivers/mtd/nand/atmel_nand_ecc.h
@@ -43,6 +43,7 @@
#define PMECC_CFG_BCH_ERR8 (2 << 0)
#define PMECC_CFG_BCH_ERR12 (3 << 0)
#define PMECC_CFG_BCH_ERR24 (4 << 0)
+#define PMECC_CFG_BCH_ERR32 (5 << 0)
#define PMECC_CFG_SECTOR512 (0 << 4)
#define PMECC_CFG_SECTOR1024 (1 << 4)
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v1 4/5] mtd: atmel_nand: Support 32-bit ECC strength
@ 2016-01-13 16:34 ` Romain Izard
0 siblings, 0 replies; 47+ messages in thread
From: Romain Izard @ 2016-01-13 16:34 UTC (permalink / raw)
To: linux-mtd, devicetree; +Cc: Yang Wenyou, Josh Wu, Nicolas Ferre, Romain Izard
As the SAMA5D2 controller supports the 32-bit ECC strength, accept it
as a valid setting when required by the device tree or the NAND
parameter page.
Then configure the controller to do use this new setting.
Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
---
.../devicetree/bindings/mtd/atmel-nand.txt | 3 ++-
drivers/mtd/nand/atmel_nand.c | 23 ++++++++++++++++++----
drivers/mtd/nand/atmel_nand_ecc.h | 1 +
3 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/mtd/atmel-nand.txt b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
index 90887b430f03..ef0db8e2a0fd 100644
--- a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
@@ -27,7 +27,8 @@ Optional properties:
- atmel,has-pmecc : boolean to enable Programmable Multibit ECC hardware.
Supported by AT91SAM9x5 or later SAM9 chips, and SAMA5 chips.
- atmel,pmecc-cap : error correct capability for Programmable Multibit ECC
- Controller. Supported values are: 2, 4, 8, 12, 24.
+ Controller. Supported values are: 2, 4, 8, 12, 24. SAMA5D2 also supports
+ 32.
- atmel,pmecc-sector-size : sector size for ECC computation. Supported values
are: 512, 1024.
- atmel,pmecc-lookup-table-offset : includes two offsets of lookup table in ROM
diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index 6fe50e2d291f..920a0a315a60 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -474,6 +474,7 @@ static void atmel_write_buf(struct mtd_info *mtd, const u8 *buf, int len)
* 8-bits 13-bytes 14-bytes
* 12-bits 20-bytes 21-bytes
* 24-bits 39-bytes 42-bytes
+ * 32-bits 52-bytes 56-bytes
*/
static int pmecc_get_ecc_bytes(int cap, int sector_size)
{
@@ -1022,6 +1023,9 @@ static void atmel_pmecc_core_init(struct mtd_info *mtd)
case 24:
val = PMECC_CFG_BCH_ERR24;
break;
+ case 32:
+ val = PMECC_CFG_BCH_ERR32;
+ break;
}
if (host->pmecc_sector_size == 512)
@@ -1083,6 +1087,9 @@ static int pmecc_choose_ecc(struct atmel_nand_host *host,
/* If device tree doesn't specify, use NAND's minimum ECC parameters */
if (host->pmecc_corr_cap == 0) {
+ if (*cap > host->caps->pmecc_max_correction)
+ return -EINVAL;
+
/* use the most fitable ecc bits (the near bigger one ) */
if (*cap <= 2)
host->pmecc_corr_cap = 2;
@@ -1094,6 +1101,8 @@ static int pmecc_choose_ecc(struct atmel_nand_host *host,
host->pmecc_corr_cap = 12;
else if (*cap <= 24)
host->pmecc_corr_cap = 24;
+ else if (*cap <= 32)
+ host->pmecc_corr_cap = 32;
else
return -EINVAL;
}
@@ -1554,10 +1563,16 @@ static int atmel_of_init_port(struct atmel_nand_host *host,
* them from NAND ONFI parameters.
*/
if (of_property_read_u32(np, "atmel,pmecc-cap", &val) == 0) {
- if ((val != 2) && (val != 4) && (val != 8) && (val != 12) &&
- (val != 24)) {
+ if (val > host->caps->pmecc_max_correction) {
+ dev_err(host->dev,
+ "Required ECC strength too high: %u max %u\n",
+ val, host->caps->pmecc_max_correction);
+ return -EINVAL;
+ }
+ if ((val != 2) && (val != 4) && (val != 8) &&
+ (val != 12) && (val != 24) && (val != 32)) {
dev_err(host->dev,
- "Unsupported PMECC correction capability: %d; should be 2, 4, 8, 12 or 24\n",
+ "Required ECC strength not supported: %u\n",
val);
return -EINVAL;
}
@@ -1567,7 +1582,7 @@ static int atmel_of_init_port(struct atmel_nand_host *host,
if (of_property_read_u32(np, "atmel,pmecc-sector-size", &val) == 0) {
if ((val != 512) && (val != 1024)) {
dev_err(host->dev,
- "Unsupported PMECC sector size: %d; should be 512 or 1024 bytes\n",
+ "Required ECC sector size not supported: %u\n",
val);
return -EINVAL;
}
diff --git a/drivers/mtd/nand/atmel_nand_ecc.h b/drivers/mtd/nand/atmel_nand_ecc.h
index ec964c43c932..834d694487bd 100644
--- a/drivers/mtd/nand/atmel_nand_ecc.h
+++ b/drivers/mtd/nand/atmel_nand_ecc.h
@@ -43,6 +43,7 @@
#define PMECC_CFG_BCH_ERR8 (2 << 0)
#define PMECC_CFG_BCH_ERR12 (3 << 0)
#define PMECC_CFG_BCH_ERR24 (4 << 0)
+#define PMECC_CFG_BCH_ERR32 (5 << 0)
#define PMECC_CFG_SECTOR512 (0 << 4)
#define PMECC_CFG_SECTOR1024 (1 << 4)
--
2.5.0
^ permalink raw reply related [flat|nested] 47+ messages in thread[parent not found: <1452702857-2240-5-git-send-email-romain.izard.pro-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH v1 4/5] mtd: atmel_nand: Support 32-bit ECC strength
2016-01-13 16:34 ` Romain Izard
@ 2016-01-14 1:09 ` Rob Herring
-1 siblings, 0 replies; 47+ messages in thread
From: Rob Herring @ 2016-01-14 1:09 UTC (permalink / raw)
To: Romain Izard
Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Yang Wenyou, Josh Wu,
Nicolas Ferre
On Wed, Jan 13, 2016 at 05:34:16PM +0100, Romain Izard wrote:
> As the SAMA5D2 controller supports the 32-bit ECC strength, accept it
> as a valid setting when required by the device tree or the NAND
> parameter page.
>
> Then configure the controller to do use this new setting.
>
> Signed-off-by: Romain Izard <romain.izard.pro-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> .../devicetree/bindings/mtd/atmel-nand.txt | 3 ++-
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> drivers/mtd/nand/atmel_nand.c | 23 ++++++++++++++++++----
> drivers/mtd/nand/atmel_nand_ecc.h | 1 +
> 3 files changed, 22 insertions(+), 5 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v1 4/5] mtd: atmel_nand: Support 32-bit ECC strength
@ 2016-01-14 1:09 ` Rob Herring
0 siblings, 0 replies; 47+ messages in thread
From: Rob Herring @ 2016-01-14 1:09 UTC (permalink / raw)
To: Romain Izard; +Cc: linux-mtd, devicetree, Yang Wenyou, Josh Wu, Nicolas Ferre
On Wed, Jan 13, 2016 at 05:34:16PM +0100, Romain Izard wrote:
> As the SAMA5D2 controller supports the 32-bit ECC strength, accept it
> as a valid setting when required by the device tree or the NAND
> parameter page.
>
> Then configure the controller to do use this new setting.
>
> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
> ---
> .../devicetree/bindings/mtd/atmel-nand.txt | 3 ++-
Acked-by: Rob Herring <robh@kernel.org>
> drivers/mtd/nand/atmel_nand.c | 23 ++++++++++++++++++----
> drivers/mtd/nand/atmel_nand_ecc.h | 1 +
> 3 files changed, 22 insertions(+), 5 deletions(-)
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v1 4/5] mtd: atmel_nand: Support 32-bit ECC strength
2016-01-13 16:34 ` Romain Izard
@ 2016-01-14 9:26 ` Boris Brezillon
-1 siblings, 0 replies; 47+ messages in thread
From: Boris Brezillon @ 2016-01-14 9:26 UTC (permalink / raw)
To: Romain Izard
Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Josh Wu, Nicolas Ferre,
Yang Wenyou
On Wed, 13 Jan 2016 17:34:16 +0100
Romain Izard <romain.izard.pro-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> As the SAMA5D2 controller supports the 32-bit ECC strength, accept it
> as a valid setting when required by the device tree or the NAND
> parameter page.
>
> Then configure the controller to do use this new setting.
>
> Signed-off-by: Romain Izard <romain.izard.pro-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Reviewed-by: Boris Brezillon <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
> .../devicetree/bindings/mtd/atmel-nand.txt | 3 ++-
> drivers/mtd/nand/atmel_nand.c | 23 ++++++++++++++++++----
> drivers/mtd/nand/atmel_nand_ecc.h | 1 +
> 3 files changed, 22 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mtd/atmel-nand.txt b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> index 90887b430f03..ef0db8e2a0fd 100644
> --- a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> @@ -27,7 +27,8 @@ Optional properties:
> - atmel,has-pmecc : boolean to enable Programmable Multibit ECC hardware.
> Supported by AT91SAM9x5 or later SAM9 chips, and SAMA5 chips.
> - atmel,pmecc-cap : error correct capability for Programmable Multibit ECC
> - Controller. Supported values are: 2, 4, 8, 12, 24.
> + Controller. Supported values are: 2, 4, 8, 12, 24. SAMA5D2 also supports
> + 32.
> - atmel,pmecc-sector-size : sector size for ECC computation. Supported values
> are: 512, 1024.
> - atmel,pmecc-lookup-table-offset : includes two offsets of lookup table in ROM
> diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
> index 6fe50e2d291f..920a0a315a60 100644
> --- a/drivers/mtd/nand/atmel_nand.c
> +++ b/drivers/mtd/nand/atmel_nand.c
> @@ -474,6 +474,7 @@ static void atmel_write_buf(struct mtd_info *mtd, const u8 *buf, int len)
> * 8-bits 13-bytes 14-bytes
> * 12-bits 20-bytes 21-bytes
> * 24-bits 39-bytes 42-bytes
> + * 32-bits 52-bytes 56-bytes
> */
> static int pmecc_get_ecc_bytes(int cap, int sector_size)
> {
> @@ -1022,6 +1023,9 @@ static void atmel_pmecc_core_init(struct mtd_info *mtd)
> case 24:
> val = PMECC_CFG_BCH_ERR24;
> break;
> + case 32:
> + val = PMECC_CFG_BCH_ERR32;
> + break;
> }
>
> if (host->pmecc_sector_size == 512)
> @@ -1083,6 +1087,9 @@ static int pmecc_choose_ecc(struct atmel_nand_host *host,
>
> /* If device tree doesn't specify, use NAND's minimum ECC parameters */
> if (host->pmecc_corr_cap == 0) {
> + if (*cap > host->caps->pmecc_max_correction)
> + return -EINVAL;
> +
> /* use the most fitable ecc bits (the near bigger one ) */
> if (*cap <= 2)
> host->pmecc_corr_cap = 2;
> @@ -1094,6 +1101,8 @@ static int pmecc_choose_ecc(struct atmel_nand_host *host,
> host->pmecc_corr_cap = 12;
> else if (*cap <= 24)
> host->pmecc_corr_cap = 24;
> + else if (*cap <= 32)
> + host->pmecc_corr_cap = 32;
> else
> return -EINVAL;
> }
> @@ -1554,10 +1563,16 @@ static int atmel_of_init_port(struct atmel_nand_host *host,
> * them from NAND ONFI parameters.
> */
> if (of_property_read_u32(np, "atmel,pmecc-cap", &val) == 0) {
> - if ((val != 2) && (val != 4) && (val != 8) && (val != 12) &&
> - (val != 24)) {
> + if (val > host->caps->pmecc_max_correction) {
> + dev_err(host->dev,
> + "Required ECC strength too high: %u max %u\n",
> + val, host->caps->pmecc_max_correction);
> + return -EINVAL;
> + }
> + if ((val != 2) && (val != 4) && (val != 8) &&
> + (val != 12) && (val != 24) && (val != 32)) {
> dev_err(host->dev,
> - "Unsupported PMECC correction capability: %d; should be 2, 4, 8, 12 or 24\n",
> + "Required ECC strength not supported: %u\n",
> val);
> return -EINVAL;
> }
> @@ -1567,7 +1582,7 @@ static int atmel_of_init_port(struct atmel_nand_host *host,
> if (of_property_read_u32(np, "atmel,pmecc-sector-size", &val) == 0) {
> if ((val != 512) && (val != 1024)) {
> dev_err(host->dev,
> - "Unsupported PMECC sector size: %d; should be 512 or 1024 bytes\n",
> + "Required ECC sector size not supported: %u\n",
> val);
I'm nitpicking, but this change has nothing to do with the addition
of the 32bits strength. Maybe it should be part of another patch (along
with the other log message rewording).
> return -EINVAL;
> }
> diff --git a/drivers/mtd/nand/atmel_nand_ecc.h b/drivers/mtd/nand/atmel_nand_ecc.h
> index ec964c43c932..834d694487bd 100644
> --- a/drivers/mtd/nand/atmel_nand_ecc.h
> +++ b/drivers/mtd/nand/atmel_nand_ecc.h
> @@ -43,6 +43,7 @@
> #define PMECC_CFG_BCH_ERR8 (2 << 0)
> #define PMECC_CFG_BCH_ERR12 (3 << 0)
> #define PMECC_CFG_BCH_ERR24 (4 << 0)
> +#define PMECC_CFG_BCH_ERR32 (5 << 0)
>
> #define PMECC_CFG_SECTOR512 (0 << 4)
> #define PMECC_CFG_SECTOR1024 (1 << 4)
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v1 4/5] mtd: atmel_nand: Support 32-bit ECC strength
@ 2016-01-14 9:26 ` Boris Brezillon
0 siblings, 0 replies; 47+ messages in thread
From: Boris Brezillon @ 2016-01-14 9:26 UTC (permalink / raw)
To: Romain Izard; +Cc: linux-mtd, devicetree, Josh Wu, Nicolas Ferre, Yang Wenyou
On Wed, 13 Jan 2016 17:34:16 +0100
Romain Izard <romain.izard.pro@gmail.com> wrote:
> As the SAMA5D2 controller supports the 32-bit ECC strength, accept it
> as a valid setting when required by the device tree or the NAND
> parameter page.
>
> Then configure the controller to do use this new setting.
>
> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
Reviewed-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
> .../devicetree/bindings/mtd/atmel-nand.txt | 3 ++-
> drivers/mtd/nand/atmel_nand.c | 23 ++++++++++++++++++----
> drivers/mtd/nand/atmel_nand_ecc.h | 1 +
> 3 files changed, 22 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mtd/atmel-nand.txt b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> index 90887b430f03..ef0db8e2a0fd 100644
> --- a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> @@ -27,7 +27,8 @@ Optional properties:
> - atmel,has-pmecc : boolean to enable Programmable Multibit ECC hardware.
> Supported by AT91SAM9x5 or later SAM9 chips, and SAMA5 chips.
> - atmel,pmecc-cap : error correct capability for Programmable Multibit ECC
> - Controller. Supported values are: 2, 4, 8, 12, 24.
> + Controller. Supported values are: 2, 4, 8, 12, 24. SAMA5D2 also supports
> + 32.
> - atmel,pmecc-sector-size : sector size for ECC computation. Supported values
> are: 512, 1024.
> - atmel,pmecc-lookup-table-offset : includes two offsets of lookup table in ROM
> diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
> index 6fe50e2d291f..920a0a315a60 100644
> --- a/drivers/mtd/nand/atmel_nand.c
> +++ b/drivers/mtd/nand/atmel_nand.c
> @@ -474,6 +474,7 @@ static void atmel_write_buf(struct mtd_info *mtd, const u8 *buf, int len)
> * 8-bits 13-bytes 14-bytes
> * 12-bits 20-bytes 21-bytes
> * 24-bits 39-bytes 42-bytes
> + * 32-bits 52-bytes 56-bytes
> */
> static int pmecc_get_ecc_bytes(int cap, int sector_size)
> {
> @@ -1022,6 +1023,9 @@ static void atmel_pmecc_core_init(struct mtd_info *mtd)
> case 24:
> val = PMECC_CFG_BCH_ERR24;
> break;
> + case 32:
> + val = PMECC_CFG_BCH_ERR32;
> + break;
> }
>
> if (host->pmecc_sector_size == 512)
> @@ -1083,6 +1087,9 @@ static int pmecc_choose_ecc(struct atmel_nand_host *host,
>
> /* If device tree doesn't specify, use NAND's minimum ECC parameters */
> if (host->pmecc_corr_cap == 0) {
> + if (*cap > host->caps->pmecc_max_correction)
> + return -EINVAL;
> +
> /* use the most fitable ecc bits (the near bigger one ) */
> if (*cap <= 2)
> host->pmecc_corr_cap = 2;
> @@ -1094,6 +1101,8 @@ static int pmecc_choose_ecc(struct atmel_nand_host *host,
> host->pmecc_corr_cap = 12;
> else if (*cap <= 24)
> host->pmecc_corr_cap = 24;
> + else if (*cap <= 32)
> + host->pmecc_corr_cap = 32;
> else
> return -EINVAL;
> }
> @@ -1554,10 +1563,16 @@ static int atmel_of_init_port(struct atmel_nand_host *host,
> * them from NAND ONFI parameters.
> */
> if (of_property_read_u32(np, "atmel,pmecc-cap", &val) == 0) {
> - if ((val != 2) && (val != 4) && (val != 8) && (val != 12) &&
> - (val != 24)) {
> + if (val > host->caps->pmecc_max_correction) {
> + dev_err(host->dev,
> + "Required ECC strength too high: %u max %u\n",
> + val, host->caps->pmecc_max_correction);
> + return -EINVAL;
> + }
> + if ((val != 2) && (val != 4) && (val != 8) &&
> + (val != 12) && (val != 24) && (val != 32)) {
> dev_err(host->dev,
> - "Unsupported PMECC correction capability: %d; should be 2, 4, 8, 12 or 24\n",
> + "Required ECC strength not supported: %u\n",
> val);
> return -EINVAL;
> }
> @@ -1567,7 +1582,7 @@ static int atmel_of_init_port(struct atmel_nand_host *host,
> if (of_property_read_u32(np, "atmel,pmecc-sector-size", &val) == 0) {
> if ((val != 512) && (val != 1024)) {
> dev_err(host->dev,
> - "Unsupported PMECC sector size: %d; should be 512 or 1024 bytes\n",
> + "Required ECC sector size not supported: %u\n",
> val);
I'm nitpicking, but this change has nothing to do with the addition
of the 32bits strength. Maybe it should be part of another patch (along
with the other log message rewording).
> return -EINVAL;
> }
> diff --git a/drivers/mtd/nand/atmel_nand_ecc.h b/drivers/mtd/nand/atmel_nand_ecc.h
> index ec964c43c932..834d694487bd 100644
> --- a/drivers/mtd/nand/atmel_nand_ecc.h
> +++ b/drivers/mtd/nand/atmel_nand_ecc.h
> @@ -43,6 +43,7 @@
> #define PMECC_CFG_BCH_ERR8 (2 << 0)
> #define PMECC_CFG_BCH_ERR12 (3 << 0)
> #define PMECC_CFG_BCH_ERR24 (4 << 0)
> +#define PMECC_CFG_BCH_ERR32 (5 << 0)
>
> #define PMECC_CFG_SECTOR512 (0 << 4)
> #define PMECC_CFG_SECTOR1024 (1 << 4)
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v1 5/5] ARM: at91/dt: sama5d2: add nand0 and nfc0 nodes
2016-01-13 16:34 ` Romain Izard
@ 2016-01-13 16:34 ` Romain Izard
-1 siblings, 0 replies; 47+ messages in thread
From: Romain Izard @ 2016-01-13 16:34 UTC (permalink / raw)
To: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: Yang Wenyou, Josh Wu, Nicolas Ferre, Romain Izard
Both nodes are required to access NAND Flash memory. Additional
settings will be necessary at the board level to use it.
Signed-off-by: Romain Izard <romain.izard.pro-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
arch/arm/boot/dts/sama5d2.dtsi | 38 ++++++++++++++++++++++++++++++++++++++
1 file changed, 38 insertions(+)
diff --git a/arch/arm/boot/dts/sama5d2.dtsi b/arch/arm/boot/dts/sama5d2.dtsi
index aee571448342..80420177ec1a 100644
--- a/arch/arm/boot/dts/sama5d2.dtsi
+++ b/arch/arm/boot/dts/sama5d2.dtsi
@@ -265,6 +265,44 @@
cache-level = <2>;
};
+ nand0: nand@80000000 {
+ compatible = "atmel,sama5d2-nand";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+ reg = < /* EBI CS3 */
+ 0x80000000 0x08000000
+ /* SMC PMECC regs */
+ 0xf8014070 0x00000490
+ /* SMC PMECC Error Location regs */
+ 0xf8014500 0x00000200
+ /* ROM Galois tables */
+ 0x00040000 0x00018000
+ >;
+ interrupts = <17 IRQ_TYPE_LEVEL_HIGH 6>;
+ atmel,nand-addr-offset = <21>;
+ atmel,nand-cmd-offset = <22>;
+ atmel,nand-has-dma;
+ atmel,has-pmecc;
+ atmel,pmecc-lookup-table-offset = <0x0 0x8000>;
+ status = "disabled";
+
+ nfc@90000000 {
+ compatible = "atmel,sama5d4-nfc";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ reg = < /* NFC Command Registers */
+ 0xC0000000 0x08000000
+ /* NFC HSMC regs */
+ 0xf8014000 0x00000070
+ /* NFC SRAM banks */
+ 0x00100000 0x00100000
+ >;
+ clocks = <&hsmc_clk>;
+ atmel,write-by-sram;
+ };
+ };
+
sdmmc0: sdio-host@a0000000 {
compatible = "atmel,sama5d2-sdhci";
reg = <0xa0000000 0x300>;
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v1 5/5] ARM: at91/dt: sama5d2: add nand0 and nfc0 nodes
@ 2016-01-13 16:34 ` Romain Izard
0 siblings, 0 replies; 47+ messages in thread
From: Romain Izard @ 2016-01-13 16:34 UTC (permalink / raw)
To: linux-mtd, devicetree; +Cc: Yang Wenyou, Josh Wu, Nicolas Ferre, Romain Izard
Both nodes are required to access NAND Flash memory. Additional
settings will be necessary at the board level to use it.
Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
---
arch/arm/boot/dts/sama5d2.dtsi | 38 ++++++++++++++++++++++++++++++++++++++
1 file changed, 38 insertions(+)
diff --git a/arch/arm/boot/dts/sama5d2.dtsi b/arch/arm/boot/dts/sama5d2.dtsi
index aee571448342..80420177ec1a 100644
--- a/arch/arm/boot/dts/sama5d2.dtsi
+++ b/arch/arm/boot/dts/sama5d2.dtsi
@@ -265,6 +265,44 @@
cache-level = <2>;
};
+ nand0: nand@80000000 {
+ compatible = "atmel,sama5d2-nand";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+ reg = < /* EBI CS3 */
+ 0x80000000 0x08000000
+ /* SMC PMECC regs */
+ 0xf8014070 0x00000490
+ /* SMC PMECC Error Location regs */
+ 0xf8014500 0x00000200
+ /* ROM Galois tables */
+ 0x00040000 0x00018000
+ >;
+ interrupts = <17 IRQ_TYPE_LEVEL_HIGH 6>;
+ atmel,nand-addr-offset = <21>;
+ atmel,nand-cmd-offset = <22>;
+ atmel,nand-has-dma;
+ atmel,has-pmecc;
+ atmel,pmecc-lookup-table-offset = <0x0 0x8000>;
+ status = "disabled";
+
+ nfc@90000000 {
+ compatible = "atmel,sama5d4-nfc";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ reg = < /* NFC Command Registers */
+ 0xC0000000 0x08000000
+ /* NFC HSMC regs */
+ 0xf8014000 0x00000070
+ /* NFC SRAM banks */
+ 0x00100000 0x00100000
+ >;
+ clocks = <&hsmc_clk>;
+ atmel,write-by-sram;
+ };
+ };
+
sdmmc0: sdio-host@a0000000 {
compatible = "atmel,sama5d2-sdhci";
reg = <0xa0000000 0x300>;
--
2.5.0
^ permalink raw reply related [flat|nested] 47+ messages in thread[parent not found: <1452702857-2240-6-git-send-email-romain.izard.pro-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH v1 5/5] ARM: at91/dt: sama5d2: add nand0 and nfc0 nodes
2016-01-13 16:34 ` Romain Izard
(?)
@ 2016-01-13 17:04 ` Nicolas Ferre
-1 siblings, 0 replies; 47+ messages in thread
From: Nicolas Ferre @ 2016-01-13 17:04 UTC (permalink / raw)
To: Romain Izard, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Brian Norris
Cc: Yang Wenyou, Josh Wu, linux-arm-kernel, Alexandre Belloni
Le 13/01/2016 17:34, Romain Izard a écrit :
> Both nodes are required to access NAND Flash memory. Additional
> settings will be necessary at the board level to use it.
>
> Signed-off-by: Romain Izard <romain.izard.pro-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> arch/arm/boot/dts/sama5d2.dtsi | 38 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 38 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sama5d2.dtsi b/arch/arm/boot/dts/sama5d2.dtsi
> index aee571448342..80420177ec1a 100644
> --- a/arch/arm/boot/dts/sama5d2.dtsi
> +++ b/arch/arm/boot/dts/sama5d2.dtsi
> @@ -265,6 +265,44 @@
> cache-level = <2>;
> };
>
> + nand0: nand@80000000 {
> + compatible = "atmel,sama5d2-nand";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> + reg = < /* EBI CS3 */
> + 0x80000000 0x08000000
> + /* SMC PMECC regs */
> + 0xf8014070 0x00000490
> + /* SMC PMECC Error Location regs */
> + 0xf8014500 0x00000200
> + /* ROM Galois tables */
> + 0x00040000 0x00018000
> + >;
> + interrupts = <17 IRQ_TYPE_LEVEL_HIGH 6>;
> + atmel,nand-addr-offset = <21>;
> + atmel,nand-cmd-offset = <22>;
> + atmel,nand-has-dma;
> + atmel,has-pmecc;
> + atmel,pmecc-lookup-table-offset = <0x0 0x8000>;
> + status = "disabled";
> +
> + nfc@90000000 {
It's nfc@c0000000
> + compatible = "atmel,sama5d4-nfc";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + reg = < /* NFC Command Registers */
> + 0xC0000000 0x08000000
Lower case please
> + /* NFC HSMC regs */
> + 0xf8014000 0x00000070
> + /* NFC SRAM banks */
> + 0x00100000 0x00100000
> + >;
> + clocks = <&hsmc_clk>;
> + atmel,write-by-sram;
> + };
> + };
> +
Otherwise, it seems okay. When corrected, you can add my:
Acked-by: Nicolas Ferre <nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
Brian, Wenyou,
I'm okay if this patch goes to Mainline with the mtd subsystem: there
should be no conflict with arm-soc for the upcoming kernel revisions.
Thanks, bye.
> sdmmc0: sdio-host@a0000000 {
> compatible = "atmel,sama5d2-sdhci";
> reg = <0xa0000000 0x300>;
>
--
Nicolas Ferre
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 47+ messages in thread* [PATCH v1 5/5] ARM: at91/dt: sama5d2: add nand0 and nfc0 nodes
@ 2016-01-13 17:04 ` Nicolas Ferre
0 siblings, 0 replies; 47+ messages in thread
From: Nicolas Ferre @ 2016-01-13 17:04 UTC (permalink / raw)
To: linux-arm-kernel
Le 13/01/2016 17:34, Romain Izard a ?crit :
> Both nodes are required to access NAND Flash memory. Additional
> settings will be necessary at the board level to use it.
>
> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
> ---
> arch/arm/boot/dts/sama5d2.dtsi | 38 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 38 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sama5d2.dtsi b/arch/arm/boot/dts/sama5d2.dtsi
> index aee571448342..80420177ec1a 100644
> --- a/arch/arm/boot/dts/sama5d2.dtsi
> +++ b/arch/arm/boot/dts/sama5d2.dtsi
> @@ -265,6 +265,44 @@
> cache-level = <2>;
> };
>
> + nand0: nand at 80000000 {
> + compatible = "atmel,sama5d2-nand";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> + reg = < /* EBI CS3 */
> + 0x80000000 0x08000000
> + /* SMC PMECC regs */
> + 0xf8014070 0x00000490
> + /* SMC PMECC Error Location regs */
> + 0xf8014500 0x00000200
> + /* ROM Galois tables */
> + 0x00040000 0x00018000
> + >;
> + interrupts = <17 IRQ_TYPE_LEVEL_HIGH 6>;
> + atmel,nand-addr-offset = <21>;
> + atmel,nand-cmd-offset = <22>;
> + atmel,nand-has-dma;
> + atmel,has-pmecc;
> + atmel,pmecc-lookup-table-offset = <0x0 0x8000>;
> + status = "disabled";
> +
> + nfc at 90000000 {
It's nfc at c0000000
> + compatible = "atmel,sama5d4-nfc";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + reg = < /* NFC Command Registers */
> + 0xC0000000 0x08000000
Lower case please
> + /* NFC HSMC regs */
> + 0xf8014000 0x00000070
> + /* NFC SRAM banks */
> + 0x00100000 0x00100000
> + >;
> + clocks = <&hsmc_clk>;
> + atmel,write-by-sram;
> + };
> + };
> +
Otherwise, it seems okay. When corrected, you can add my:
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Brian, Wenyou,
I'm okay if this patch goes to Mainline with the mtd subsystem: there
should be no conflict with arm-soc for the upcoming kernel revisions.
Thanks, bye.
> sdmmc0: sdio-host at a0000000 {
> compatible = "atmel,sama5d2-sdhci";
> reg = <0xa0000000 0x300>;
>
--
Nicolas Ferre
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v1 5/5] ARM: at91/dt: sama5d2: add nand0 and nfc0 nodes
@ 2016-01-13 17:04 ` Nicolas Ferre
0 siblings, 0 replies; 47+ messages in thread
From: Nicolas Ferre @ 2016-01-13 17:04 UTC (permalink / raw)
To: Romain Izard, linux-mtd, devicetree, Brian Norris
Cc: Yang Wenyou, Josh Wu, linux-arm-kernel, Alexandre Belloni
Le 13/01/2016 17:34, Romain Izard a écrit :
> Both nodes are required to access NAND Flash memory. Additional
> settings will be necessary at the board level to use it.
>
> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
> ---
> arch/arm/boot/dts/sama5d2.dtsi | 38 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 38 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sama5d2.dtsi b/arch/arm/boot/dts/sama5d2.dtsi
> index aee571448342..80420177ec1a 100644
> --- a/arch/arm/boot/dts/sama5d2.dtsi
> +++ b/arch/arm/boot/dts/sama5d2.dtsi
> @@ -265,6 +265,44 @@
> cache-level = <2>;
> };
>
> + nand0: nand@80000000 {
> + compatible = "atmel,sama5d2-nand";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> + reg = < /* EBI CS3 */
> + 0x80000000 0x08000000
> + /* SMC PMECC regs */
> + 0xf8014070 0x00000490
> + /* SMC PMECC Error Location regs */
> + 0xf8014500 0x00000200
> + /* ROM Galois tables */
> + 0x00040000 0x00018000
> + >;
> + interrupts = <17 IRQ_TYPE_LEVEL_HIGH 6>;
> + atmel,nand-addr-offset = <21>;
> + atmel,nand-cmd-offset = <22>;
> + atmel,nand-has-dma;
> + atmel,has-pmecc;
> + atmel,pmecc-lookup-table-offset = <0x0 0x8000>;
> + status = "disabled";
> +
> + nfc@90000000 {
It's nfc@c0000000
> + compatible = "atmel,sama5d4-nfc";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + reg = < /* NFC Command Registers */
> + 0xC0000000 0x08000000
Lower case please
> + /* NFC HSMC regs */
> + 0xf8014000 0x00000070
> + /* NFC SRAM banks */
> + 0x00100000 0x00100000
> + >;
> + clocks = <&hsmc_clk>;
> + atmel,write-by-sram;
> + };
> + };
> +
Otherwise, it seems okay. When corrected, you can add my:
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Brian, Wenyou,
I'm okay if this patch goes to Mainline with the mtd subsystem: there
should be no conflict with arm-soc for the upcoming kernel revisions.
Thanks, bye.
> sdmmc0: sdio-host@a0000000 {
> compatible = "atmel,sama5d2-sdhci";
> reg = <0xa0000000 0x300>;
>
--
Nicolas Ferre
^ permalink raw reply [flat|nested] 47+ messages in thread