* [PATCH] mtd: nand: Support controllers with custom page accessors
@ 2016-11-14 9:47 Marc Gonzalez
2016-11-14 10:11 ` Boris Brezillon
0 siblings, 1 reply; 10+ messages in thread
From: Marc Gonzalez @ 2016-11-14 9:47 UTC (permalink / raw)
To: Boris Brezillon, Richard Weinberger; +Cc: linux-mtd, Mason, Sebastian Frias
If your controller already sends the required NAND commands when
reading or writing a page, then the framework is not supposed to
send READ0 and SEQIN/PAGEPROG respectively.
Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
---
drivers/mtd/nand/nand_base.c | 31 ++++++++++++++++++++++++++++---
drivers/mtd/nand/tango_nand.c | 21 ++++++++++++---------
include/linux/mtd/nand.h | 12 ++++++++++++
3 files changed, 52 insertions(+), 12 deletions(-)
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 50cdf37cb8e4..b8ea6b397c16 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -1970,7 +1970,8 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
__func__, buf);
read_retry:
- chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);
+ if (nand_default_page_accessors(&chip->ecc))
+ chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);
/*
* Now read the page into the buffer. Absent an error,
@@ -2658,7 +2659,8 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
else
subpage = 0;
- chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
+ if (nand_default_page_accessors(&chip->ecc))
+ chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
if (unlikely(raw))
status = chip->ecc.write_page_raw(mtd, chip, buf,
@@ -2681,7 +2683,8 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
if (!cached || !NAND_HAS_CACHEPROG(chip)) {
- chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
+ if (nand_default_page_accessors(&chip->ecc))
+ chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
status = chip->waitfunc(mtd, chip);
/*
* See if operation failed and additional status checks are
@@ -4539,6 +4542,25 @@ static bool nand_ecc_strength_good(struct mtd_info *mtd)
return corr >= ds_corr && ecc->strength >= chip->ecc_strength_ds;
}
+static bool invalid_ecc_page_accessors(struct nand_chip *chip)
+{
+ struct nand_ecc_ctrl *ecc = &chip->ecc;
+
+ if (nand_default_page_accessors(ecc))
+ return false;
+
+ /*
+ * NAND_ECC_CUSTOM_PAGE_ACCESS flag is set, make sure the NAND
+ * controller driver implements all the page accessors because
+ * default helpers are not suitable when the core does not
+ * send the READ0/PAGEPROG commands.
+ */
+ return (!ecc->read_page || !ecc->write_page ||
+ !ecc->read_page_raw || !ecc->write_page_raw ||
+ (NAND_HAS_SUBPAGE_READ(chip) && !ecc->read_subpage) ||
+ (NAND_HAS_SUBPAGE_WRITE(chip) && !ecc->write_subpage));
+}
+
/**
* nand_scan_tail - [NAND Interface] Scan for the NAND device
* @mtd: MTD device structure
@@ -4559,6 +4581,9 @@ int nand_scan_tail(struct mtd_info *mtd)
!(chip->bbt_options & NAND_BBT_USE_FLASH)))
return -EINVAL;
+ if (WARN_ON(invalid_ecc_page_accessors(chip)))
+ return -EINVAL;
+
if (!(chip->options & NAND_OWN_BUFFERS)) {
nbuf = kzalloc(sizeof(*nbuf) + mtd->writesize
+ mtd->oobsize * 3, GFP_KERNEL);
diff --git a/drivers/mtd/nand/tango_nand.c b/drivers/mtd/nand/tango_nand.c
index fd8cf414cb2b..7ed35348993e 100644
--- a/drivers/mtd/nand/tango_nand.c
+++ b/drivers/mtd/nand/tango_nand.c
@@ -343,7 +343,7 @@ static void aux_write(struct nand_chip *chip, const u8 **buf, int len, int *pos)
* buf = | PKT_0 | ... | PKT_N |
* +-----------------+-----+-----------------+
*/
-static int raw_read(struct nand_chip *chip, u8 *buf, u8 *oob)
+static void raw_read(struct nand_chip *chip, u8 *buf, u8 *oob)
{
u8 *oob_orig = oob;
const int page_size = chip->mtd.writesize;
@@ -367,11 +367,9 @@ static int raw_read(struct nand_chip *chip, u8 *buf, u8 *oob)
aux_read(chip, &oob_orig, BBM_SIZE, &pos);
aux_read(chip, &buf, pkt_size - rem, &pos);
aux_read(chip, &oob, ecc_size, &pos);
-
- return 0;
}
-static int raw_write(struct nand_chip *chip, const u8 *buf, const u8 *oob)
+static void raw_write(struct nand_chip *chip, const u8 *buf, const u8 *oob)
{
const u8 *oob_orig = oob;
const int page_size = chip->mtd.writesize;
@@ -395,27 +393,31 @@ static int raw_write(struct nand_chip *chip, const u8 *buf, const u8 *oob)
aux_write(chip, &oob_orig, BBM_SIZE, &pos);
aux_write(chip, &buf, pkt_size - rem, &pos);
aux_write(chip, &oob, ecc_size, &pos);
-
- return 0;
}
static int tango_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
u8 *buf, int oob_required, int page)
{
- return raw_read(chip, buf, chip->oob_poi);
+ chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
+ raw_read(chip, buf, chip->oob_poi);
+ return 0;
}
static int tango_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
const u8 *buf, int oob_required, int page)
{
- return raw_write(chip, buf, chip->oob_poi);
+ chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0, page);
+ raw_write(chip, buf, chip->oob_poi);
+ chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
+ return 0;
}
static int tango_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
int page)
{
chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
- return raw_read(chip, NULL, chip->oob_poi);
+ raw_read(chip, NULL, chip->oob_poi);
+ return 0;
}
static int tango_write_oob(struct mtd_info *mtd, struct nand_chip *chip,
@@ -553,6 +555,7 @@ static int chip_init(struct device *dev, struct device_node *np)
ecc->write_page = tango_write_page;
ecc->read_oob = tango_read_oob;
ecc->write_oob = tango_write_oob;
+ ecc->options = NAND_ECC_CUSTOM_PAGE_ACCESS;
err = nand_scan_tail(mtd);
if (err)
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 06d0c9d740f7..a95a27a30005 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -142,6 +142,12 @@ enum nand_ecc_algo {
*/
#define NAND_ECC_GENERIC_ERASED_CHECK BIT(0)
#define NAND_ECC_MAXIMIZE BIT(1)
+/*
+ * If your controller already sends the required NAND commands when
+ * reading or writing a page, then the framework is not supposed to
+ * send READ0 and SEQIN/PAGEPROG respectively.
+ */
+#define NAND_ECC_CUSTOM_PAGE_ACCESS BIT(2)
/* Bit mask for flags passed to do_nand_read_ecc */
#define NAND_GET_DEVICE 0x80
@@ -186,6 +192,7 @@ enum nand_ecc_algo {
/* Macros to identify the above */
#define NAND_HAS_CACHEPROG(chip) ((chip->options & NAND_CACHEPRG))
#define NAND_HAS_SUBPAGE_READ(chip) ((chip->options & NAND_SUBPAGE_READ))
+#define NAND_HAS_SUBPAGE_WRITE(chip) !(chip->options & NAND_NO_SUBPAGE_WRITE)
/* Non chip related options */
/* This option skips the bbt scan during initialization. */
@@ -568,6 +575,11 @@ struct nand_ecc_ctrl {
int page);
};
+static inline int nand_default_page_accessors(struct nand_ecc_ctrl *ecc)
+{
+ return !(ecc->options & NAND_ECC_CUSTOM_PAGE_ACCESS);
+}
+
/**
* struct nand_buffers - buffer structure for read/write
* @ecccalc: buffer pointer for calculated ECC, size is oobsize.
--
2.9.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] mtd: nand: Support controllers with custom page accessors
2016-11-14 9:47 [PATCH] mtd: nand: Support controllers with custom page accessors Marc Gonzalez
@ 2016-11-14 10:11 ` Boris Brezillon
2016-11-14 17:01 ` [PATCH v2 1/2] mtd: nand: Support controllers with custom page Marc Gonzalez
2016-11-14 17:06 ` [PATCH v2 2/2] mtd: nand: tango: Enable custom page accessors Marc Gonzalez
0 siblings, 2 replies; 10+ messages in thread
From: Boris Brezillon @ 2016-11-14 10:11 UTC (permalink / raw)
To: Marc Gonzalez; +Cc: Richard Weinberger, linux-mtd, Mason, Sebastian Frias
Hi Marc,
On Mon, 14 Nov 2016 10:47:38 +0100
Marc Gonzalez <marc_gonzalez@sigmadesigns.com> wrote:
> If your controller already sends the required NAND commands when
> reading or writing a page, then the framework is not supposed to
> send READ0 and SEQIN/PAGEPROG respectively.
>
> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
> ---
> drivers/mtd/nand/nand_base.c | 31 ++++++++++++++++++++++++++++---
> drivers/mtd/nand/tango_nand.c | 21 ++++++++++++---------
> include/linux/mtd/nand.h | 12 ++++++++++++
> 3 files changed, 52 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 50cdf37cb8e4..b8ea6b397c16 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -1970,7 +1970,8 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
> __func__, buf);
>
> read_retry:
> - chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);
> + if (nand_default_page_accessors(&chip->ecc))
> + chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);
>
> /*
> * Now read the page into the buffer. Absent an error,
> @@ -2658,7 +2659,8 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
> else
> subpage = 0;
>
> - chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
> + if (nand_default_page_accessors(&chip->ecc))
> + chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
>
> if (unlikely(raw))
> status = chip->ecc.write_page_raw(mtd, chip, buf,
> @@ -2681,7 +2683,8 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
>
> if (!cached || !NAND_HAS_CACHEPROG(chip)) {
>
> - chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
> + if (nand_default_page_accessors(&chip->ecc))
> + chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
> status = chip->waitfunc(mtd, chip);
> /*
> * See if operation failed and additional status checks are
> @@ -4539,6 +4542,25 @@ static bool nand_ecc_strength_good(struct mtd_info *mtd)
> return corr >= ds_corr && ecc->strength >= chip->ecc_strength_ds;
> }
>
> +static bool invalid_ecc_page_accessors(struct nand_chip *chip)
> +{
> + struct nand_ecc_ctrl *ecc = &chip->ecc;
> +
> + if (nand_default_page_accessors(ecc))
> + return false;
> +
> + /*
> + * NAND_ECC_CUSTOM_PAGE_ACCESS flag is set, make sure the NAND
> + * controller driver implements all the page accessors because
> + * default helpers are not suitable when the core does not
> + * send the READ0/PAGEPROG commands.
> + */
> + return (!ecc->read_page || !ecc->write_page ||
> + !ecc->read_page_raw || !ecc->write_page_raw ||
> + (NAND_HAS_SUBPAGE_READ(chip) && !ecc->read_subpage) ||
> + (NAND_HAS_SUBPAGE_WRITE(chip) && !ecc->write_subpage));
> +}
> +
> /**
> * nand_scan_tail - [NAND Interface] Scan for the NAND device
> * @mtd: MTD device structure
> @@ -4559,6 +4581,9 @@ int nand_scan_tail(struct mtd_info *mtd)
> !(chip->bbt_options & NAND_BBT_USE_FLASH)))
> return -EINVAL;
>
> + if (WARN_ON(invalid_ecc_page_accessors(chip)))
> + return -EINVAL;
> +
> if (!(chip->options & NAND_OWN_BUFFERS)) {
> nbuf = kzalloc(sizeof(*nbuf) + mtd->writesize
> + mtd->oobsize * 3, GFP_KERNEL);
> diff --git a/drivers/mtd/nand/tango_nand.c b/drivers/mtd/nand/tango_nand.c
> index fd8cf414cb2b..7ed35348993e 100644
> --- a/drivers/mtd/nand/tango_nand.c
> +++ b/drivers/mtd/nand/tango_nand.c
> @@ -343,7 +343,7 @@ static void aux_write(struct nand_chip *chip, const u8 **buf, int len, int *pos)
> * buf = | PKT_0 | ... | PKT_N |
> * +-----------------+-----+-----------------+
> */
> -static int raw_read(struct nand_chip *chip, u8 *buf, u8 *oob)
> +static void raw_read(struct nand_chip *chip, u8 *buf, u8 *oob)
This change is completely unrelated to the custom page accessors patch.
Please separate the function prototype changes in a separate patch.
> {
> u8 *oob_orig = oob;
> const int page_size = chip->mtd.writesize;
> @@ -367,11 +367,9 @@ static int raw_read(struct nand_chip *chip, u8 *buf, u8 *oob)
> aux_read(chip, &oob_orig, BBM_SIZE, &pos);
> aux_read(chip, &buf, pkt_size - rem, &pos);
> aux_read(chip, &oob, ecc_size, &pos);
> -
> - return 0;
> }
>
> -static int raw_write(struct nand_chip *chip, const u8 *buf, const u8 *oob)
> +static void raw_write(struct nand_chip *chip, const u8 *buf, const u8 *
> {
> const u8 *oob_orig = oob;
> const int page_size = chip->mtd.writesize;
> @@ -395,27 +393,31 @@ static int raw_write(struct nand_chip *chip, const u8 *buf, const u8 *oob)
> aux_write(chip, &oob_orig, BBM_SIZE, &pos);
> aux_write(chip, &buf, pkt_size - rem, &pos);
> aux_write(chip, &oob, ecc_size, &pos);
> -
> - return 0;
> }
>
> static int tango_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
> u8 *buf, int oob_required, int page)
> {
> - return raw_read(chip, buf, chip->oob_poi);
> + chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
> + raw_read(chip, buf, chip->oob_poi);
> + return 0;
> }
>
> static int tango_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
> const u8 *buf, int oob_required, int page)
> {
> - return raw_write(chip, buf, chip->oob_poi);
> + chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0, page);
> + raw_write(chip, buf, chip->oob_poi);
> + chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
> + return 0;
> }
>
> static int tango_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
> int page)
> {
> chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
> - return raw_read(chip, NULL, chip->oob_poi);
> + raw_read(chip, NULL, chip->oob_poi);
> + return 0;
> }
>
> static int tango_write_oob(struct mtd_info *mtd, struct nand_chip *chip,
> @@ -553,6 +555,7 @@ static int chip_init(struct device *dev, struct device_node *np)
> ecc->write_page = tango_write_page;
> ecc->read_oob = tango_read_oob;
> ecc->write_oob = tango_write_oob;
> + ecc->options = NAND_ECC_CUSTOM_PAGE_ACCESS;
>
> err = nand_scan_tail(mtd);
> if (err)
> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> index 06d0c9d740f7..a95a27a30005 100644
> --- a/include/linux/mtd/nand.h
> +++ b/include/linux/mtd/nand.h
> @@ -142,6 +142,12 @@ enum nand_ecc_algo {
> */
> #define NAND_ECC_GENERIC_ERASED_CHECK BIT(0)
> #define NAND_ECC_MAXIMIZE BIT(1)
> +/*
> + * If your controller already sends the required NAND commands when
> + * reading or writing a page, then the framework is not supposed to
> + * send READ0 and SEQIN/PAGEPROG respectively.
> + */
> +#define NAND_ECC_CUSTOM_PAGE_ACCESS BIT(2)
>
> /* Bit mask for flags passed to do_nand_read_ecc */
> #define NAND_GET_DEVICE 0x80
> @@ -186,6 +192,7 @@ enum nand_ecc_algo {
> /* Macros to identify the above */
> #define NAND_HAS_CACHEPROG(chip) ((chip->options & NAND_CACHEPRG))
> #define NAND_HAS_SUBPAGE_READ(chip) ((chip->options & NAND_SUBPAGE_READ))
> +#define NAND_HAS_SUBPAGE_WRITE(chip) !(chip->options & NAND_NO_SUBPAGE_WRITE)
(chip)->options
Just in case the chip parameter is something like that &x->chip.
>
> /* Non chip related options */
> /* This option skips the bbt scan during initialization. */
> @@ -568,6 +575,11 @@ struct nand_ecc_ctrl {
> int page);
> };
>
> +static inline int nand_default_page_accessors(struct nand_ecc_ctrl *ecc)
Not sure the term default is correct here: some drivers implement
their own ->read/write_page() methods, but expect the core to send the
READ/PROG commands.
How about renaming it nand_standard_page_accessors()?
> +{
> + return !(ecc->options & NAND_ECC_CUSTOM_PAGE_ACCESS);
> +}
> +
> /**
> * struct nand_buffers - buffer structure for read/write
> * @ecccalc: buffer pointer for calculated ECC, size is oobsize.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 1/2] mtd: nand: Support controllers with custom page
2016-11-14 10:11 ` Boris Brezillon
@ 2016-11-14 17:01 ` Marc Gonzalez
2016-11-14 17:34 ` Boris Brezillon
2016-11-14 17:06 ` [PATCH v2 2/2] mtd: nand: tango: Enable custom page accessors Marc Gonzalez
1 sibling, 1 reply; 10+ messages in thread
From: Marc Gonzalez @ 2016-11-14 17:01 UTC (permalink / raw)
To: Boris Brezillon, Richard Weinberger; +Cc: linux-mtd, Mason, Sebastian Frias
If your controller already sends the required NAND commands when
reading or writing a page, then the framework is not supposed to
send READ0 and SEQIN/PAGEPROG respectively.
Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
---
v2:
(chip) in NAND_HAS_SUBPAGE_WRITE macro
s/nand_default_page_accessors/nand_standard_page_accessors/
scripts/checkpatch.pl --strict v2-0001-mtd-nand-Support-controllers-with-custom-page-acc.patch
total: 0 errors, 0 warnings, 0 checks, 91 lines checked
---
drivers/mtd/nand/nand_base.c | 31 ++++++++++++++++++++++++++++---
include/linux/mtd/nand.h | 12 ++++++++++++
2 files changed, 40 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 50cdf37cb8e4..db5f27db3748 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -1970,7 +1970,8 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
__func__, buf);
read_retry:
- chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);
+ if (nand_standard_page_accessors(&chip->ecc))
+ chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);
/*
* Now read the page into the buffer. Absent an error,
@@ -2658,7 +2659,8 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
else
subpage = 0;
- chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
+ if (nand_standard_page_accessors(&chip->ecc))
+ chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
if (unlikely(raw))
status = chip->ecc.write_page_raw(mtd, chip, buf,
@@ -2681,7 +2683,8 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
if (!cached || !NAND_HAS_CACHEPROG(chip)) {
- chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
+ if (nand_standard_page_accessors(&chip->ecc))
+ chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
status = chip->waitfunc(mtd, chip);
/*
* See if operation failed and additional status checks are
@@ -4539,6 +4542,25 @@ static bool nand_ecc_strength_good(struct mtd_info *mtd)
return corr >= ds_corr && ecc->strength >= chip->ecc_strength_ds;
}
+static bool invalid_ecc_page_accessors(struct nand_chip *chip)
+{
+ struct nand_ecc_ctrl *ecc = &chip->ecc;
+
+ if (nand_standard_page_accessors(ecc))
+ return false;
+
+ /*
+ * NAND_ECC_CUSTOM_PAGE_ACCESS flag is set, make sure the NAND
+ * controller driver implements all the page accessors because
+ * default helpers are not suitable when the core does not
+ * send the READ0/PAGEPROG commands.
+ */
+ return (!ecc->read_page || !ecc->write_page ||
+ !ecc->read_page_raw || !ecc->write_page_raw ||
+ (NAND_HAS_SUBPAGE_READ(chip) && !ecc->read_subpage) ||
+ (NAND_HAS_SUBPAGE_WRITE(chip) && !ecc->write_subpage));
+}
+
/**
* nand_scan_tail - [NAND Interface] Scan for the NAND device
* @mtd: MTD device structure
@@ -4559,6 +4581,9 @@ int nand_scan_tail(struct mtd_info *mtd)
!(chip->bbt_options & NAND_BBT_USE_FLASH)))
return -EINVAL;
+ if (WARN_ON(invalid_ecc_page_accessors(chip)))
+ return -EINVAL;
+
if (!(chip->options & NAND_OWN_BUFFERS)) {
nbuf = kzalloc(sizeof(*nbuf) + mtd->writesize
+ mtd->oobsize * 3, GFP_KERNEL);
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 06d0c9d740f7..c5f3a012ae62 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -142,6 +142,12 @@ enum nand_ecc_algo {
*/
#define NAND_ECC_GENERIC_ERASED_CHECK BIT(0)
#define NAND_ECC_MAXIMIZE BIT(1)
+/*
+ * If your controller already sends the required NAND commands when
+ * reading or writing a page, then the framework is not supposed to
+ * send READ0 and SEQIN/PAGEPROG respectively.
+ */
+#define NAND_ECC_CUSTOM_PAGE_ACCESS BIT(2)
/* Bit mask for flags passed to do_nand_read_ecc */
#define NAND_GET_DEVICE 0x80
@@ -186,6 +192,7 @@ enum nand_ecc_algo {
/* Macros to identify the above */
#define NAND_HAS_CACHEPROG(chip) ((chip->options & NAND_CACHEPRG))
#define NAND_HAS_SUBPAGE_READ(chip) ((chip->options & NAND_SUBPAGE_READ))
+#define NAND_HAS_SUBPAGE_WRITE(chip) !((chip)->options & NAND_NO_SUBPAGE_WRITE)
/* Non chip related options */
/* This option skips the bbt scan during initialization. */
@@ -568,6 +575,11 @@ struct nand_ecc_ctrl {
int page);
};
+static inline int nand_standard_page_accessors(struct nand_ecc_ctrl *ecc)
+{
+ return !(ecc->options & NAND_ECC_CUSTOM_PAGE_ACCESS);
+}
+
/**
* struct nand_buffers - buffer structure for read/write
* @ecccalc: buffer pointer for calculated ECC, size is oobsize.
--
2.9.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 2/2] mtd: nand: tango: Enable custom page accessors
2016-11-14 10:11 ` Boris Brezillon
2016-11-14 17:01 ` [PATCH v2 1/2] mtd: nand: Support controllers with custom page Marc Gonzalez
@ 2016-11-14 17:06 ` Marc Gonzalez
2016-11-14 17:38 ` Boris Brezillon
1 sibling, 1 reply; 10+ messages in thread
From: Marc Gonzalez @ 2016-11-14 17:06 UTC (permalink / raw)
To: Boris Brezillon, Richard Weinberger; +Cc: linux-mtd, Mason, Sebastian Frias
Enable NAND_ECC_CUSTOM_PAGE_ACCESS in tango NFC driver.
Fixup "raw" page accessors to send proper NAND commands.
Drop raw_write/raw_read return values (no longer used).
Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
---
v2:
split from previous v1 patch
---
drivers/mtd/nand/tango_nand.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/mtd/nand/tango_nand.c b/drivers/mtd/nand/tango_nand.c
index fd8cf414cb2b..7ed35348993e 100644
--- a/drivers/mtd/nand/tango_nand.c
+++ b/drivers/mtd/nand/tango_nand.c
@@ -343,7 +343,7 @@ static void aux_write(struct nand_chip *chip, const u8 **buf, int len, int *pos)
* buf = | PKT_0 | ... | PKT_N |
* +-----------------+-----+-----------------+
*/
-static int raw_read(struct nand_chip *chip, u8 *buf, u8 *oob)
+static void raw_read(struct nand_chip *chip, u8 *buf, u8 *oob)
{
u8 *oob_orig = oob;
const int page_size = chip->mtd.writesize;
@@ -367,11 +367,9 @@ static int raw_read(struct nand_chip *chip, u8 *buf, u8 *oob)
aux_read(chip, &oob_orig, BBM_SIZE, &pos);
aux_read(chip, &buf, pkt_size - rem, &pos);
aux_read(chip, &oob, ecc_size, &pos);
-
- return 0;
}
-static int raw_write(struct nand_chip *chip, const u8 *buf, const u8 *oob)
+static void raw_write(struct nand_chip *chip, const u8 *buf, const u8 *oob)
{
const u8 *oob_orig = oob;
const int page_size = chip->mtd.writesize;
@@ -395,27 +393,31 @@ static int raw_write(struct nand_chip *chip, const u8 *buf, const u8 *oob)
aux_write(chip, &oob_orig, BBM_SIZE, &pos);
aux_write(chip, &buf, pkt_size - rem, &pos);
aux_write(chip, &oob, ecc_size, &pos);
-
- return 0;
}
static int tango_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
u8 *buf, int oob_required, int page)
{
- return raw_read(chip, buf, chip->oob_poi);
+ chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
+ raw_read(chip, buf, chip->oob_poi);
+ return 0;
}
static int tango_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
const u8 *buf, int oob_required, int page)
{
- return raw_write(chip, buf, chip->oob_poi);
+ chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0, page);
+ raw_write(chip, buf, chip->oob_poi);
+ chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
+ return 0;
}
static int tango_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
int page)
{
chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
- return raw_read(chip, NULL, chip->oob_poi);
+ raw_read(chip, NULL, chip->oob_poi);
+ return 0;
}
static int tango_write_oob(struct mtd_info *mtd, struct nand_chip *chip,
@@ -553,6 +555,7 @@ static int chip_init(struct device *dev, struct device_node *np)
ecc->write_page = tango_write_page;
ecc->read_oob = tango_read_oob;
ecc->write_oob = tango_write_oob;
+ ecc->options = NAND_ECC_CUSTOM_PAGE_ACCESS;
err = nand_scan_tail(mtd);
if (err)
--
2.9.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] mtd: nand: Support controllers with custom page
2016-11-14 17:01 ` [PATCH v2 1/2] mtd: nand: Support controllers with custom page Marc Gonzalez
@ 2016-11-14 17:34 ` Boris Brezillon
2016-11-15 9:56 ` [PATCH v3 1/3] " Marc Gonzalez
0 siblings, 1 reply; 10+ messages in thread
From: Boris Brezillon @ 2016-11-14 17:34 UTC (permalink / raw)
To: Marc Gonzalez; +Cc: Richard Weinberger, linux-mtd, Mason, Sebastian Frias
Hi Marc,
On Mon, 14 Nov 2016 18:01:27 +0100
Marc Gonzalez <marc_gonzalez@sigmadesigns.com> wrote:
> If your controller already sends the required NAND commands when
> reading or writing a page, then the framework is not supposed to
> send READ0 and SEQIN/PAGEPROG respectively.
>
> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
> ---
> v2:
> (chip) in NAND_HAS_SUBPAGE_WRITE macro
> s/nand_default_page_accessors/nand_standard_page_accessors/
> scripts/checkpatch.pl --strict v2-0001-mtd-nand-Support-controllers-with-custom-page-acc.patch
> total: 0 errors, 0 warnings, 0 checks, 91 lines checked
> ---
> drivers/mtd/nand/nand_base.c | 31 ++++++++++++++++++++++++++++---
> include/linux/mtd/nand.h | 12 ++++++++++++
> 2 files changed, 40 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 50cdf37cb8e4..db5f27db3748 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -1970,7 +1970,8 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
> __func__, buf);
>
> read_retry:
> - chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);
> + if (nand_standard_page_accessors(&chip->ecc))
> + chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);
>
> /*
> * Now read the page into the buffer. Absent an error,
> @@ -2658,7 +2659,8 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
> else
> subpage = 0;
>
> - chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
> + if (nand_standard_page_accessors(&chip->ecc))
> + chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
>
> if (unlikely(raw))
> status = chip->ecc.write_page_raw(mtd, chip, buf,
> @@ -2681,7 +2683,8 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
>
> if (!cached || !NAND_HAS_CACHEPROG(chip)) {
>
> - chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
> + if (nand_standard_page_accessors(&chip->ecc))
> + chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
> status = chip->waitfunc(mtd, chip);
> /*
> * See if operation failed and additional status checks are
> @@ -4539,6 +4542,25 @@ static bool nand_ecc_strength_good(struct mtd_info *mtd)
> return corr >= ds_corr && ecc->strength >= chip->ecc_strength_ds;
> }
>
> +static bool invalid_ecc_page_accessors(struct nand_chip *chip)
> +{
> + struct nand_ecc_ctrl *ecc = &chip->ecc;
> +
> + if (nand_standard_page_accessors(ecc))
> + return false;
> +
> + /*
> + * NAND_ECC_CUSTOM_PAGE_ACCESS flag is set, make sure the NAND
> + * controller driver implements all the page accessors because
> + * default helpers are not suitable when the core does not
> + * send the READ0/PAGEPROG commands.
> + */
> + return (!ecc->read_page || !ecc->write_page ||
> + !ecc->read_page_raw || !ecc->write_page_raw ||
> + (NAND_HAS_SUBPAGE_READ(chip) && !ecc->read_subpage) ||
> + (NAND_HAS_SUBPAGE_WRITE(chip) && !ecc->write_subpage));
Just had a closer look at the nand_write_page() function, and it seems
that !NAND_NO_SUBPAGE_WRITE && !ecc->write_subpage is valid use case.
In this situation, nand_write_page() uses ecc->write_page().
I know it's a real mess, and each time I look at this subpage support
detection, I have a hard time remembering how it works, and why it's
done this way. The idea is that, even if the controller does not
explicitly implement ->write_subpage(), the core will fill the page
buffer with 0xff, and since programming NANDs is just about moving bits
from 1 to zero, when you leave bits to one, they are just and-ed with
the previous content.
The correct test is:
return (!ecc->read_page || !ecc->write_page ||
!ecc->read_page_raw || !ecc->write_page_raw ||
(NAND_HAS_SUBPAGE_READ(chip) && !ecc->read_subpage) ||
(NAND_HAS_SUBPAGE_WRITE(chip) && !ecc->write_subpage &&
ecc->hwctl && ecc->calculate))
> +}
> +
> /**
> * nand_scan_tail - [NAND Interface] Scan for the NAND device
> * @mtd: MTD device structure
> @@ -4559,6 +4581,9 @@ int nand_scan_tail(struct mtd_info *mtd)
> !(chip->bbt_options & NAND_BBT_USE_FLASH)))
> return -EINVAL;
>
> + if (WARN_ON(invalid_ecc_page_accessors(chip)))
> + return -EINVAL;
> +
Sorry, I didn't notice that one in my previous review. We're currently
trying to get rid of all BUG() and WARN_ON() calls in the NAND
framework. Can you replace this WARN_ON() by a pr_err() message?
> if (!(chip->options & NAND_OWN_BUFFERS)) {
> nbuf = kzalloc(sizeof(*nbuf) + mtd->writesize
> + mtd->oobsize * 3, GFP_KERNEL);
> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> index 06d0c9d740f7..c5f3a012ae62 100644
> --- a/include/linux/mtd/nand.h
> +++ b/include/linux/mtd/nand.h
> @@ -142,6 +142,12 @@ enum nand_ecc_algo {
> */
> #define NAND_ECC_GENERIC_ERASED_CHECK BIT(0)
> #define NAND_ECC_MAXIMIZE BIT(1)
> +/*
> + * If your controller already sends the required NAND commands when
> + * reading or writing a page, then the framework is not supposed to
> + * send READ0 and SEQIN/PAGEPROG respectively.
> + */
> +#define NAND_ECC_CUSTOM_PAGE_ACCESS BIT(2)
>
> /* Bit mask for flags passed to do_nand_read_ecc */
> #define NAND_GET_DEVICE 0x80
> @@ -186,6 +192,7 @@ enum nand_ecc_algo {
> /* Macros to identify the above */
> #define NAND_HAS_CACHEPROG(chip) ((chip->options & NAND_CACHEPRG))
> #define NAND_HAS_SUBPAGE_READ(chip) ((chip->options & NAND_SUBPAGE_READ))
> +#define NAND_HAS_SUBPAGE_WRITE(chip) !((chip)->options & NAND_NO_SUBPAGE_WRITE)
>
> /* Non chip related options */
> /* This option skips the bbt scan during initialization. */
> @@ -568,6 +575,11 @@ struct nand_ecc_ctrl {
> int page);
> };
>
> +static inline int nand_standard_page_accessors(struct nand_ecc_ctrl *ecc)
> +{
> + return !(ecc->options & NAND_ECC_CUSTOM_PAGE_ACCESS);
> +}
> +
> /**
> * struct nand_buffers - buffer structure for read/write
> * @ecccalc: buffer pointer for calculated ECC, size is oobsize.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] mtd: nand: tango: Enable custom page accessors
2016-11-14 17:06 ` [PATCH v2 2/2] mtd: nand: tango: Enable custom page accessors Marc Gonzalez
@ 2016-11-14 17:38 ` Boris Brezillon
2016-11-15 10:05 ` [PATCH v3 2/3] " Marc Gonzalez
2016-11-15 10:08 ` [PATCH v3 3/3] mtd: nand: tango: Cleanup raw_write and raw_read Marc Gonzalez
0 siblings, 2 replies; 10+ messages in thread
From: Boris Brezillon @ 2016-11-14 17:38 UTC (permalink / raw)
To: Marc Gonzalez; +Cc: Richard Weinberger, linux-mtd, Mason, Sebastian Frias
On Mon, 14 Nov 2016 18:06:45 +0100
Marc Gonzalez <marc_gonzalez@sigmadesigns.com> wrote:
> Enable NAND_ECC_CUSTOM_PAGE_ACCESS in tango NFC driver.
> Fixup "raw" page accessors to send proper NAND commands.
> Drop raw_write/raw_read return values (no longer used).
I'd still prefer to have two separate patches:
1/ change the raw_read() raw_write() prototypes
2/ switch to NAND_ECC_CUSTOM_PAGE_ACCESS
>
> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
> ---
> v2:
> split from previous v1 patch
> ---
> drivers/mtd/nand/tango_nand.c | 21 ++++++++++++---------
> 1 file changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/mtd/nand/tango_nand.c b/drivers/mtd/nand/tango_nand.c
> index fd8cf414cb2b..7ed35348993e 100644
> --- a/drivers/mtd/nand/tango_nand.c
> +++ b/drivers/mtd/nand/tango_nand.c
> @@ -343,7 +343,7 @@ static void aux_write(struct nand_chip *chip, const u8 **buf, int len, int *pos)
> * buf = | PKT_0 | ... | PKT_N |
> * +-----------------+-----+-----------------+
> */
> -static int raw_read(struct nand_chip *chip, u8 *buf, u8 *oob)
> +static void raw_read(struct nand_chip *chip, u8 *buf, u8 *oob)
> {
> u8 *oob_orig = oob;
> const int page_size = chip->mtd.writesize;
> @@ -367,11 +367,9 @@ static int raw_read(struct nand_chip *chip, u8 *buf, u8 *oob)
> aux_read(chip, &oob_orig, BBM_SIZE, &pos);
> aux_read(chip, &buf, pkt_size - rem, &pos);
> aux_read(chip, &oob, ecc_size, &pos);
> -
> - return 0;
> }
>
> -static int raw_write(struct nand_chip *chip, const u8 *buf, const u8 *oob)
> +static void raw_write(struct nand_chip *chip, const u8 *buf, const u8 *oob)
> {
> const u8 *oob_orig = oob;
> const int page_size = chip->mtd.writesize;
> @@ -395,27 +393,31 @@ static int raw_write(struct nand_chip *chip, const u8 *buf, const u8 *oob)
> aux_write(chip, &oob_orig, BBM_SIZE, &pos);
> aux_write(chip, &buf, pkt_size - rem, &pos);
> aux_write(chip, &oob, ecc_size, &pos);
> -
> - return 0;
> }
>
> static int tango_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
> u8 *buf, int oob_required, int page)
> {
> - return raw_read(chip, buf, chip->oob_poi);
> + chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
> + raw_read(chip, buf, chip->oob_poi);
> + return 0;
> }
>
> static int tango_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
> const u8 *buf, int oob_required, int page)
> {
> - return raw_write(chip, buf, chip->oob_poi);
> + chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0, page);
> + raw_write(chip, buf, chip->oob_poi);
> + chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
> + return 0;
> }
>
> static int tango_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
> int page)
> {
> chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
> - return raw_read(chip, NULL, chip->oob_poi);
> + raw_read(chip, NULL, chip->oob_poi);
> + return 0;
> }
>
> static int tango_write_oob(struct mtd_info *mtd, struct nand_chip *chip,
> @@ -553,6 +555,7 @@ static int chip_init(struct device *dev, struct device_node *np)
> ecc->write_page = tango_write_page;
> ecc->read_oob = tango_read_oob;
> ecc->write_oob = tango_write_oob;
> + ecc->options = NAND_ECC_CUSTOM_PAGE_ACCESS;
>
> err = nand_scan_tail(mtd);
> if (err)
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 1/3] mtd: nand: Support controllers with custom page
2016-11-14 17:34 ` Boris Brezillon
@ 2016-11-15 9:56 ` Marc Gonzalez
2016-11-19 18:40 ` Boris Brezillon
0 siblings, 1 reply; 10+ messages in thread
From: Marc Gonzalez @ 2016-11-15 9:56 UTC (permalink / raw)
To: Boris Brezillon; +Cc: Richard Weinberger, linux-mtd, Mason, Sebastian Frias
If your controller already sends the required NAND commands when
reading or writing a page, then the framework is not supposed to
send READ0 and SEQIN/PAGEPROG respectively.
Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
---
v3:
ecc->write_subpage handling
don't use WARN_ON
---
drivers/mtd/nand/nand_base.c | 34 +++++++++++++++++++++++++++++++---
include/linux/mtd/nand.h | 12 ++++++++++++
2 files changed, 43 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 50cdf37cb8e4..e7034f3bcace 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -1970,7 +1970,8 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
__func__, buf);
read_retry:
- chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);
+ if (nand_standard_page_accessors(&chip->ecc))
+ chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);
/*
* Now read the page into the buffer. Absent an error,
@@ -2658,7 +2659,8 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
else
subpage = 0;
- chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
+ if (nand_standard_page_accessors(&chip->ecc))
+ chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
if (unlikely(raw))
status = chip->ecc.write_page_raw(mtd, chip, buf,
@@ -2681,7 +2683,8 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
if (!cached || !NAND_HAS_CACHEPROG(chip)) {
- chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
+ if (nand_standard_page_accessors(&chip->ecc))
+ chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
status = chip->waitfunc(mtd, chip);
/*
* See if operation failed and additional status checks are
@@ -4539,6 +4542,26 @@ static bool nand_ecc_strength_good(struct mtd_info *mtd)
return corr >= ds_corr && ecc->strength >= chip->ecc_strength_ds;
}
+static bool invalid_ecc_page_accessors(struct nand_chip *chip)
+{
+ struct nand_ecc_ctrl *ecc = &chip->ecc;
+
+ if (nand_standard_page_accessors(ecc))
+ return false;
+
+ /*
+ * NAND_ECC_CUSTOM_PAGE_ACCESS flag is set, make sure the NAND
+ * controller driver implements all the page accessors because
+ * default helpers are not suitable when the core does not
+ * send the READ0/PAGEPROG commands.
+ */
+ return (!ecc->read_page || !ecc->write_page ||
+ !ecc->read_page_raw || !ecc->write_page_raw ||
+ (NAND_HAS_SUBPAGE_READ(chip) && !ecc->read_subpage) ||
+ (NAND_HAS_SUBPAGE_WRITE(chip) && !ecc->write_subpage &&
+ ecc->hwctl && ecc->calculate));
+}
+
/**
* nand_scan_tail - [NAND Interface] Scan for the NAND device
* @mtd: MTD device structure
@@ -4559,6 +4582,11 @@ int nand_scan_tail(struct mtd_info *mtd)
!(chip->bbt_options & NAND_BBT_USE_FLASH)))
return -EINVAL;
+ if (invalid_ecc_page_accessors(chip)) {
+ pr_err("Invalid ECC page accessors setup\n");
+ return -EINVAL;
+ }
+
if (!(chip->options & NAND_OWN_BUFFERS)) {
nbuf = kzalloc(sizeof(*nbuf) + mtd->writesize
+ mtd->oobsize * 3, GFP_KERNEL);
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 06d0c9d740f7..c5f3a012ae62 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -142,6 +142,12 @@ enum nand_ecc_algo {
*/
#define NAND_ECC_GENERIC_ERASED_CHECK BIT(0)
#define NAND_ECC_MAXIMIZE BIT(1)
+/*
+ * If your controller already sends the required NAND commands when
+ * reading or writing a page, then the framework is not supposed to
+ * send READ0 and SEQIN/PAGEPROG respectively.
+ */
+#define NAND_ECC_CUSTOM_PAGE_ACCESS BIT(2)
/* Bit mask for flags passed to do_nand_read_ecc */
#define NAND_GET_DEVICE 0x80
@@ -186,6 +192,7 @@ enum nand_ecc_algo {
/* Macros to identify the above */
#define NAND_HAS_CACHEPROG(chip) ((chip->options & NAND_CACHEPRG))
#define NAND_HAS_SUBPAGE_READ(chip) ((chip->options & NAND_SUBPAGE_READ))
+#define NAND_HAS_SUBPAGE_WRITE(chip) !((chip)->options & NAND_NO_SUBPAGE_WRITE)
/* Non chip related options */
/* This option skips the bbt scan during initialization. */
@@ -568,6 +575,11 @@ struct nand_ecc_ctrl {
int page);
};
+static inline int nand_standard_page_accessors(struct nand_ecc_ctrl *ecc)
+{
+ return !(ecc->options & NAND_ECC_CUSTOM_PAGE_ACCESS);
+}
+
/**
* struct nand_buffers - buffer structure for read/write
* @ecccalc: buffer pointer for calculated ECC, size is oobsize.
--
2.9.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 2/3] mtd: nand: tango: Enable custom page accessors
2016-11-14 17:38 ` Boris Brezillon
@ 2016-11-15 10:05 ` Marc Gonzalez
2016-11-15 10:08 ` [PATCH v3 3/3] mtd: nand: tango: Cleanup raw_write and raw_read Marc Gonzalez
1 sibling, 0 replies; 10+ messages in thread
From: Marc Gonzalez @ 2016-11-15 10:05 UTC (permalink / raw)
To: Boris Brezillon; +Cc: Richard Weinberger, linux-mtd, Mason, Sebastian Frias
Enable NAND_ECC_CUSTOM_PAGE_ACCESS in the tango NFC driver.
Fixup the "raw" page accessors to send the proper NAND commands.
Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
---
drivers/mtd/nand/tango_nand.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/tango_nand.c b/drivers/mtd/nand/tango_nand.c
index fd8cf414cb2b..bacc2a02b7fe 100644
--- a/drivers/mtd/nand/tango_nand.c
+++ b/drivers/mtd/nand/tango_nand.c
@@ -402,13 +402,17 @@ static int raw_write(struct nand_chip *chip, const u8 *buf, const u8 *oob)
static int tango_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
u8 *buf, int oob_required, int page)
{
+ chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
return raw_read(chip, buf, chip->oob_poi);
}
static int tango_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
const u8 *buf, int oob_required, int page)
{
- return raw_write(chip, buf, chip->oob_poi);
+ chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0, page);
+ raw_write(chip, buf, chip->oob_poi);
+ chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
+ return 0;
}
static int tango_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
@@ -553,6 +557,7 @@ static int chip_init(struct device *dev, struct device_node *np)
ecc->write_page = tango_write_page;
ecc->read_oob = tango_read_oob;
ecc->write_oob = tango_write_oob;
+ ecc->options = NAND_ECC_CUSTOM_PAGE_ACCESS;
err = nand_scan_tail(mtd);
if (err)
--
2.9.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 3/3] mtd: nand: tango: Cleanup raw_write and raw_read
2016-11-14 17:38 ` Boris Brezillon
2016-11-15 10:05 ` [PATCH v3 2/3] " Marc Gonzalez
@ 2016-11-15 10:08 ` Marc Gonzalez
1 sibling, 0 replies; 10+ messages in thread
From: Marc Gonzalez @ 2016-11-15 10:08 UTC (permalink / raw)
To: Boris Brezillon; +Cc: Richard Weinberger, linux-mtd, Mason, Sebastian Frias
Drop raw_write return value (no longer used).
Drop raw_read return value (for symmetry).
Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
---
drivers/mtd/nand/tango_nand.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/mtd/nand/tango_nand.c b/drivers/mtd/nand/tango_nand.c
index bacc2a02b7fe..7ed35348993e 100644
--- a/drivers/mtd/nand/tango_nand.c
+++ b/drivers/mtd/nand/tango_nand.c
@@ -343,7 +343,7 @@ static void aux_write(struct nand_chip *chip, const u8 **buf, int len, int *pos)
* buf = | PKT_0 | ... | PKT_N |
* +-----------------+-----+-----------------+
*/
-static int raw_read(struct nand_chip *chip, u8 *buf, u8 *oob)
+static void raw_read(struct nand_chip *chip, u8 *buf, u8 *oob)
{
u8 *oob_orig = oob;
const int page_size = chip->mtd.writesize;
@@ -367,11 +367,9 @@ static int raw_read(struct nand_chip *chip, u8 *buf, u8 *oob)
aux_read(chip, &oob_orig, BBM_SIZE, &pos);
aux_read(chip, &buf, pkt_size - rem, &pos);
aux_read(chip, &oob, ecc_size, &pos);
-
- return 0;
}
-static int raw_write(struct nand_chip *chip, const u8 *buf, const u8 *oob)
+static void raw_write(struct nand_chip *chip, const u8 *buf, const u8 *oob)
{
const u8 *oob_orig = oob;
const int page_size = chip->mtd.writesize;
@@ -395,15 +393,14 @@ static int raw_write(struct nand_chip *chip, const u8 *buf, const u8 *oob)
aux_write(chip, &oob_orig, BBM_SIZE, &pos);
aux_write(chip, &buf, pkt_size - rem, &pos);
aux_write(chip, &oob, ecc_size, &pos);
-
- return 0;
}
static int tango_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
u8 *buf, int oob_required, int page)
{
chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
- return raw_read(chip, buf, chip->oob_poi);
+ raw_read(chip, buf, chip->oob_poi);
+ return 0;
}
static int tango_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
@@ -419,7 +416,8 @@ static int tango_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
int page)
{
chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
- return raw_read(chip, NULL, chip->oob_poi);
+ raw_read(chip, NULL, chip->oob_poi);
+ return 0;
}
static int tango_write_oob(struct mtd_info *mtd, struct nand_chip *chip,
--
2.9.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/3] mtd: nand: Support controllers with custom page
2016-11-15 9:56 ` [PATCH v3 1/3] " Marc Gonzalez
@ 2016-11-19 18:40 ` Boris Brezillon
0 siblings, 0 replies; 10+ messages in thread
From: Boris Brezillon @ 2016-11-19 18:40 UTC (permalink / raw)
To: Marc Gonzalez; +Cc: Richard Weinberger, linux-mtd, Mason, Sebastian Frias
Hi Mark,
On Tue, 15 Nov 2016 10:56:20 +0100
Marc Gonzalez <marc_gonzalez@sigmadesigns.com> wrote:
> If your controller already sends the required NAND commands when
> reading or writing a page, then the framework is not supposed to
> send READ0 and SEQIN/PAGEPROG respectively.
>
> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
Applied the whole series.
Just a small request for your future contributions, can you please
group the patches of a specific version in the same thread instead of
making them answers to my previous reviews?
Thanks,
Boris
> ---
> v3:
> ecc->write_subpage handling
> don't use WARN_ON
> ---
> drivers/mtd/nand/nand_base.c | 34 +++++++++++++++++++++++++++++++---
> include/linux/mtd/nand.h | 12 ++++++++++++
> 2 files changed, 43 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 50cdf37cb8e4..e7034f3bcace 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -1970,7 +1970,8 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
> __func__, buf);
>
> read_retry:
> - chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);
> + if (nand_standard_page_accessors(&chip->ecc))
> + chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);
>
> /*
> * Now read the page into the buffer. Absent an error,
> @@ -2658,7 +2659,8 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
> else
> subpage = 0;
>
> - chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
> + if (nand_standard_page_accessors(&chip->ecc))
> + chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
>
> if (unlikely(raw))
> status = chip->ecc.write_page_raw(mtd, chip, buf,
> @@ -2681,7 +2683,8 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
>
> if (!cached || !NAND_HAS_CACHEPROG(chip)) {
>
> - chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
> + if (nand_standard_page_accessors(&chip->ecc))
> + chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
> status = chip->waitfunc(mtd, chip);
> /*
> * See if operation failed and additional status checks are
> @@ -4539,6 +4542,26 @@ static bool nand_ecc_strength_good(struct mtd_info *mtd)
> return corr >= ds_corr && ecc->strength >= chip->ecc_strength_ds;
> }
>
> +static bool invalid_ecc_page_accessors(struct nand_chip *chip)
> +{
> + struct nand_ecc_ctrl *ecc = &chip->ecc;
> +
> + if (nand_standard_page_accessors(ecc))
> + return false;
> +
> + /*
> + * NAND_ECC_CUSTOM_PAGE_ACCESS flag is set, make sure the NAND
> + * controller driver implements all the page accessors because
> + * default helpers are not suitable when the core does not
> + * send the READ0/PAGEPROG commands.
> + */
> + return (!ecc->read_page || !ecc->write_page ||
> + !ecc->read_page_raw || !ecc->write_page_raw ||
> + (NAND_HAS_SUBPAGE_READ(chip) && !ecc->read_subpage) ||
> + (NAND_HAS_SUBPAGE_WRITE(chip) && !ecc->write_subpage &&
> + ecc->hwctl && ecc->calculate));
> +}
> +
> /**
> * nand_scan_tail - [NAND Interface] Scan for the NAND device
> * @mtd: MTD device structure
> @@ -4559,6 +4582,11 @@ int nand_scan_tail(struct mtd_info *mtd)
> !(chip->bbt_options & NAND_BBT_USE_FLASH)))
> return -EINVAL;
>
> + if (invalid_ecc_page_accessors(chip)) {
> + pr_err("Invalid ECC page accessors setup\n");
> + return -EINVAL;
> + }
> +
> if (!(chip->options & NAND_OWN_BUFFERS)) {
> nbuf = kzalloc(sizeof(*nbuf) + mtd->writesize
> + mtd->oobsize * 3, GFP_KERNEL);
> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> index 06d0c9d740f7..c5f3a012ae62 100644
> --- a/include/linux/mtd/nand.h
> +++ b/include/linux/mtd/nand.h
> @@ -142,6 +142,12 @@ enum nand_ecc_algo {
> */
> #define NAND_ECC_GENERIC_ERASED_CHECK BIT(0)
> #define NAND_ECC_MAXIMIZE BIT(1)
> +/*
> + * If your controller already sends the required NAND commands when
> + * reading or writing a page, then the framework is not supposed to
> + * send READ0 and SEQIN/PAGEPROG respectively.
> + */
> +#define NAND_ECC_CUSTOM_PAGE_ACCESS BIT(2)
>
> /* Bit mask for flags passed to do_nand_read_ecc */
> #define NAND_GET_DEVICE 0x80
> @@ -186,6 +192,7 @@ enum nand_ecc_algo {
> /* Macros to identify the above */
> #define NAND_HAS_CACHEPROG(chip) ((chip->options & NAND_CACHEPRG))
> #define NAND_HAS_SUBPAGE_READ(chip) ((chip->options & NAND_SUBPAGE_READ))
> +#define NAND_HAS_SUBPAGE_WRITE(chip) !((chip)->options & NAND_NO_SUBPAGE_WRITE)
>
> /* Non chip related options */
> /* This option skips the bbt scan during initialization. */
> @@ -568,6 +575,11 @@ struct nand_ecc_ctrl {
> int page);
> };
>
> +static inline int nand_standard_page_accessors(struct nand_ecc_ctrl *ecc)
> +{
> + return !(ecc->options & NAND_ECC_CUSTOM_PAGE_ACCESS);
> +}
> +
> /**
> * struct nand_buffers - buffer structure for read/write
> * @ecccalc: buffer pointer for calculated ECC, size is oobsize.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-11-19 18:40 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-14 9:47 [PATCH] mtd: nand: Support controllers with custom page accessors Marc Gonzalez
2016-11-14 10:11 ` Boris Brezillon
2016-11-14 17:01 ` [PATCH v2 1/2] mtd: nand: Support controllers with custom page Marc Gonzalez
2016-11-14 17:34 ` Boris Brezillon
2016-11-15 9:56 ` [PATCH v3 1/3] " Marc Gonzalez
2016-11-19 18:40 ` Boris Brezillon
2016-11-14 17:06 ` [PATCH v2 2/2] mtd: nand: tango: Enable custom page accessors Marc Gonzalez
2016-11-14 17:38 ` Boris Brezillon
2016-11-15 10:05 ` [PATCH v3 2/3] " Marc Gonzalez
2016-11-15 10:08 ` [PATCH v3 3/3] mtd: nand: tango: Cleanup raw_write and raw_read Marc Gonzalez
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.