LinuxLists.cc - [PATCH v3 00/33] Allow dynamic allocations during NAND chip identification phase

2018-07-19 23:01:38

Subject: [PATCH v3 00/33] Allow dynamic allocations during NAND chip identification phase

Hello,

This series make a quite deep change in the NAND framework. Until now,
the NAND chip identification phase could be done in two manners from the
controller driver perspective:

1/ Call nand_scan()

or

1/ Call nand_scan_ident()
2/ Do some controller-dependent configuration
3/ Call nand_scan_tail().

The fact that the identifaction could be split in two operations
involved that in the NAND framework, it was not possible to do any
dynamic allocation without risking a memory leak. What if the core
allocates a structure, then the driver between nand_scan_ident() and
nand_scan_tail() decides it cannot handle the chip and errors out?
The structure allocated by the core is lost: it is a memory leak. One
solution could have been to add a nand_scan_ident_cleanup() function,
but that would mean patching all the drivers anyway to make them call
this function when something fails between nand_scan_ident() and
nand_scan_tail().

To avoid this situation, we migrate all drivers to use nand_scan() in
conjuction with the recently added hooks ->attach_chip() and
->detach_chip() that are part of the nand_controller structure
operations. Drivers that need to tweak their configuration after
nand_scan_ident() should implement it. Any dynamically allocated space
in ->attach_chip() must be freed in the second hook: ->detach_chip().

The ->detach_chip() does not have to be called upon error in the
controller driver probe function. The nand_cleanup() helper already
exists for that and will do the call if needed. Of course, this helper
must be called on error after a successful nand_scan(), just like
before.

Once all drivers not using nand_scan() are migrated, nand_scan_ident()
and nand_scan_tail() are unexported and only available internally.

A previous work [1] removed the ONFI/JEDEC parameter pages and instead
allocated a nand_parameters structure in nand_chip, embedding both
generic entries and ONFI-related ones. The deal was, once dynamic
allocation possible, allocate in nand_scan_ident() the ONFI strcuture
only if actually needed. This is done in the last patches.

This series applies on top of nand/next.

Thank you,
Miquèl

[1] http://lists.infradead.org/pipermail/linux-mtd/2018-March/079456.html

Changes since v2:
=================
* Rebased on top of nand/next.
* Adapted all drivers to declare statically a nand_controller_ops
structure and assign it in the probe().
* Added the migration of the tegra_nand.c driver.
* Moved brcmnand controller ops affectation in the probe().

Changes since v1:
=================
* Rebased on top of nand/next.
* Light rewording of the cover letter about the possibility to have a
nand_scan_ident_cleanup() function (just as example of how this series
could have been done differently).
* Changed the hooks to reside in the nand_hw_ctrl structure instead of
being part of nand_ecc_ctrl as these hooks are more
controller-related.

Miquel Raynal (33):
mtd: rawnand: brcmnand: convert driver to nand_scan()
mtd: rawnand: cafe: convert driver to nand_scan()
mtd: rawnand: davinci: convert driver to nand_scan()
mtd: rawnand: denali: convert to nand_scan()
mtd: rawnand: fsl_elbc: convert driver to nand_scan()
mtd: rawnand: fsl_ifc: convert driver to nand_scan()
mtd: rawnand: fsmc: convert driver to nand_scan()
mtd: rawnand: gpmi: convert driver to nand_scan()
mtd: rawnand: hisi504: convert driver to nand_scan()
mtd: rawnand: jz4780: convert driver to nand_scan()
mtd: rawnand: lpc32xx_mlc: convert driver to nand_scan()
mtd: rawnand: lpc32xx_slc: convert driver to nand_scan()
mtd: rawnand: marvell: convert driver to nand_scan()
mtd: rawnand: mtk: convert driver to nand_scan()
mtd: rawnand: mxc: convert driver to nand_scan()
mtd: rawnand: nandsim: convert driver to nand_scan()
mtd: rawnand: omap2: convert driver to nand_scan()
mtd: rawnand: s3c2410: convert driver to nand_scan()
mtd: rawnand: sh_flctl: move all NAND chip related setup in one
function
mtd: rawnand: sh_flctl: convert driver to nand_scan()
mtd: rawnand: sunxi: convert driver to nand_scan()
mtd: rawnand: tango: convert driver to nand_scan()
mtd: rawnand: txx9ndfmc: rename nand controller internal structure
mtd: rawnand: txx9ndfmc: convert driver to nand_scan()
mtd: rawnand: vf610: convert driver to nand_scan()
mtd: rawnand: atmel: convert driver to nand_scan()
mtd: rawnand: sm_common: convert driver to nand_scan_with_ids()
mtd: rawnand: docg4: convert driver to nand_scan()
mtd: rawnand: qcom: convert driver to nand_scan()
mtd: rawnand: jz4740: convert driver to nand_scan()
mtd: rawnand: tegra: convert driver to nand_scan()
mtd: rawnand: do not export nand_scan_[ident|tail]() anymore
mtd: rawnand: allocate dynamically ONFI parameters during detection

drivers/mtd/nand/raw/atmel/nand-controller.c | 83 ++---
drivers/mtd/nand/raw/brcmnand/brcmnand.c | 47 ++-
drivers/mtd/nand/raw/cafe_nand.c | 130 ++++---
drivers/mtd/nand/raw/davinci_nand.c | 195 +++++-----
drivers/mtd/nand/raw/denali.c | 138 +++----
drivers/mtd/nand/raw/docg4.c | 55 +--
drivers/mtd/nand/raw/fsl_elbc_nand.c | 19 +-
drivers/mtd/nand/raw/fsl_ifc_nand.c | 19 +-
drivers/mtd/nand/raw/fsmc_nand.c | 148 ++++----
drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c | 56 +--
drivers/mtd/nand/raw/hisi504_nand.c | 78 ++--
drivers/mtd/nand/raw/jz4740_nand.c | 46 ++-
drivers/mtd/nand/raw/jz4780_nand.c | 34 +-
drivers/mtd/nand/raw/lpc32xx_mlc.c | 109 +++---
drivers/mtd/nand/raw/lpc32xx_slc.c | 77 ++--
drivers/mtd/nand/raw/marvell_nand.c | 206 ++++++-----
drivers/mtd/nand/raw/mtk_nand.c | 75 ++--
drivers/mtd/nand/raw/mxc_nand.c | 136 +++----
drivers/mtd/nand/raw/nand_base.c | 121 +++++--
drivers/mtd/nand/raw/nand_micron.c | 6 +-
drivers/mtd/nand/raw/nand_timings.c | 12 +-
drivers/mtd/nand/raw/nandsim.c | 82 +++--
drivers/mtd/nand/raw/omap2.c | 521 +++++++++++++--------------
drivers/mtd/nand/raw/qcom_nandc.c | 71 ++--
drivers/mtd/nand/raw/s3c2410.c | 30 +-
drivers/mtd/nand/raw/sh_flctl.c | 57 ++-
drivers/mtd/nand/raw/sm_common.c | 39 +-
drivers/mtd/nand/raw/sunxi_nand.c | 43 +--
drivers/mtd/nand/raw/tango_nand.c | 40 +-
drivers/mtd/nand/raw/tegra_nand.c | 162 +++++----
drivers/mtd/nand/raw/txx9ndfmc.c | 35 +-
drivers/mtd/nand/raw/vf610_nfc.c | 127 +++----
include/linux/mtd/rawnand.h | 17 +-
include/linux/mtd/sh_flctl.h | 1 +
34 files changed, 1603 insertions(+), 1412 deletions(-)

--
2.14.1

2018-07-19 23:02:02

by Miquel Raynal

[permalink] [raw]

Subject: [PATCH v3 14/33] mtd: rawnand: mtk: convert driver to nand_scan()

2018-07-19 23:02:11

by Miquel Raynal

[permalink] [raw]

Subject: [PATCH v3 13/33] mtd: rawnand: marvell: convert driver to nand_scan()

2018-07-19 23:02:20

by Miquel Raynal

[permalink] [raw]

Subject: [PATCH v3 12/33] mtd: rawnand: lpc32xx_slc: convert driver to nand_scan()

2018-07-19 23:02:23

by Miquel Raynal

[permalink] [raw]

Subject: [PATCH v3 15/33] mtd: rawnand: mxc: convert driver to nand_scan()

2018-07-19 23:02:23

by Miquel Raynal

[permalink] [raw]

Subject: [PATCH v3 16/33] mtd: rawnand: nandsim: convert driver to nand_scan()

2018-07-19 23:02:27

by Miquel Raynal

[permalink] [raw]

Subject: [PATCH v3 02/33] mtd: rawnand: cafe: convert driver to nand_scan()

2018-07-19 23:02:32

by Miquel Raynal

[permalink] [raw]

Subject: [PATCH v3 17/33] mtd: rawnand: omap2: convert driver to nand_scan()

Two helpers have been added to the core to make ECC-related
configuration between the detection phase and the final NAND scan. Use
these hooks and convert the driver to just use nand_scan() instead of
both nand_scan_ident() and nand_scan_tail().

Signed-off-by: Miquel Raynal <[email protected]>
---
drivers/mtd/nand/raw/omap2.c | 521 +++++++++++++++++++++----------------------
1 file changed, 259 insertions(+), 262 deletions(-)

diff --git a/drivers/mtd/nand/raw/omap2.c b/drivers/mtd/nand/raw/omap2.c
index e943b2e5a5e2..8ce6546b58ec 100644
--- a/drivers/mtd/nand/raw/omap2.c
+++ b/drivers/mtd/nand/raw/omap2.c
@@ -1915,17 +1915,271 @@ static const struct mtd_ooblayout_ops omap_sw_ooblayout_ops = {
.free = omap_sw_ooblayout_free,
};

+static int omap_nand_attach_chip(struct nand_chip *chip)
+{
+ struct mtd_info *mtd = nand_to_mtd(chip);
+ struct omap_nand_info *info = mtd_to_omap(mtd);
+ struct device *dev = &info->pdev->dev;
+ int min_oobbytes = BADBLOCK_MARKER_LENGTH;
+ int oobbytes_per_step;
+ dma_cap_mask_t mask;
+ int err;
+
+ if (chip->bbt_options & NAND_BBT_USE_FLASH)
+ chip->bbt_options |= NAND_BBT_NO_OOB;
+ else
+ chip->options |= NAND_SKIP_BBTSCAN;
+
+ /* Re-populate low-level callbacks based on xfer modes */
+ switch (info->xfer_type) {
+ case NAND_OMAP_PREFETCH_POLLED:
+ chip->read_buf = omap_read_buf_pref;
+ chip->write_buf = omap_write_buf_pref;
+ break;
+
+ case NAND_OMAP_POLLED:
+ /* Use nand_base defaults for {read,write}_buf */
+ break;
+
+ case NAND_OMAP_PREFETCH_DMA:
+ dma_cap_zero(mask);
+ dma_cap_set(DMA_SLAVE, mask);
+ info->dma = dma_request_chan(dev, "rxtx");
+
+ if (IS_ERR(info->dma)) {
+ dev_err(dev, "DMA engine request failed\n");
+ return PTR_ERR(info->dma);
+ } else {
+ struct dma_slave_config cfg;
+
+ memset(&cfg, 0, sizeof(cfg));
+ cfg.src_addr = info->phys_base;
+ cfg.dst_addr = info->phys_base;
+ cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+ cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+ cfg.src_maxburst = 16;
+ cfg.dst_maxburst = 16;
+ err = dmaengine_slave_config(info->dma, &cfg);
+ if (err) {
+ dev_err(dev,
+ "DMA engine slave config failed: %d\n",
+ err);
+ return err;
+ }
+ chip->read_buf = omap_read_buf_dma_pref;
+ chip->write_buf = omap_write_buf_dma_pref;
+ }
+ break;
+
+ case NAND_OMAP_PREFETCH_IRQ:
+ info->gpmc_irq_fifo = platform_get_irq(info->pdev, 0);
+ if (info->gpmc_irq_fifo <= 0) {
+ dev_err(dev, "Error getting fifo IRQ\n");
+ return -ENODEV;
+ }
+ err = devm_request_irq(dev, info->gpmc_irq_fifo,
+ omap_nand_irq, IRQF_SHARED,
+ "gpmc-nand-fifo", info);
+ if (err) {
+ dev_err(dev, "Requesting IRQ %d, error %d\n",
+ info->gpmc_irq_fifo, err);
+ info->gpmc_irq_fifo = 0;
+ return err;
+ }
+
+ info->gpmc_irq_count = platform_get_irq(info->pdev, 1);
+ if (info->gpmc_irq_count <= 0) {
+ dev_err(dev, "Error getting IRQ count\n");
+ return -ENODEV;
+ }
+ err = devm_request_irq(dev, info->gpmc_irq_count,
+ omap_nand_irq, IRQF_SHARED,
+ "gpmc-nand-count", info);
+ if (err) {
+ dev_err(dev, "Requesting IRQ %d, error %d\n",
+ info->gpmc_irq_count, err);
+ info->gpmc_irq_count = 0;
+ return err;
+ }
+
+ chip->read_buf = omap_read_buf_irq_pref;
+ chip->write_buf = omap_write_buf_irq_pref;
+
+ break;
+
+ default:
+ dev_err(dev, "xfer_type %d not supported!\n", info->xfer_type);
+ return -EINVAL;
+ }
+
+ if (!omap2_nand_ecc_check(info))
+ return -EINVAL;
+
+ /*
+ * Bail out earlier to let NAND_ECC_SOFT code create its own
+ * ooblayout instead of using ours.
+ */
+ if (info->ecc_opt == OMAP_ECC_HAM1_CODE_SW) {
+ chip->ecc.mode = NAND_ECC_SOFT;
+ chip->ecc.algo = NAND_ECC_HAMMING;
+ return 0;
+ }
+
+ /* Populate MTD interface based on ECC scheme */
+ switch (info->ecc_opt) {
+ case OMAP_ECC_HAM1_CODE_HW:
+ dev_info(dev, "nand: using OMAP_ECC_HAM1_CODE_HW\n");
+ chip->ecc.mode = NAND_ECC_HW;
+ chip->ecc.bytes = 3;
+ chip->ecc.size = 512;
+ chip->ecc.strength = 1;
+ chip->ecc.calculate = omap_calculate_ecc;
+ chip->ecc.hwctl = omap_enable_hwecc;
+ chip->ecc.correct = omap_correct_data;
+ mtd_set_ooblayout(mtd, &omap_ooblayout_ops);
+ oobbytes_per_step = chip->ecc.bytes;
+
+ if (!(chip->options & NAND_BUSWIDTH_16))
+ min_oobbytes = 1;
+
+ break;
+
+ case OMAP_ECC_BCH4_CODE_HW_DETECTION_SW:
+ pr_info("nand: using OMAP_ECC_BCH4_CODE_HW_DETECTION_SW\n");
+ chip->ecc.mode = NAND_ECC_HW;
+ chip->ecc.size = 512;
+ chip->ecc.bytes = 7;
+ chip->ecc.strength = 4;
+ chip->ecc.hwctl = omap_enable_hwecc_bch;
+ chip->ecc.correct = nand_bch_correct_data;
+ chip->ecc.calculate = omap_calculate_ecc_bch_sw;
+ mtd_set_ooblayout(mtd, &omap_sw_ooblayout_ops);
+ /* Reserve one byte for the OMAP marker */
+ oobbytes_per_step = chip->ecc.bytes + 1;
+ /* Software BCH library is used for locating errors */
+ chip->ecc.priv = nand_bch_init(mtd);
+ if (!chip->ecc.priv) {
+ dev_err(dev, "Unable to use BCH library\n");
+ return -EINVAL;
+ }
+ break;
+
+ case OMAP_ECC_BCH4_CODE_HW:
+ pr_info("nand: using OMAP_ECC_BCH4_CODE_HW ECC scheme\n");
+ chip->ecc.mode = NAND_ECC_HW;
+ chip->ecc.size = 512;
+ /* 14th bit is kept reserved for ROM-code compatibility */
+ chip->ecc.bytes = 7 + 1;
+ chip->ecc.strength = 4;
+ chip->ecc.hwctl = omap_enable_hwecc_bch;
+ chip->ecc.correct = omap_elm_correct_data;
+ chip->ecc.read_page = omap_read_page_bch;
+ chip->ecc.write_page = omap_write_page_bch;
+ chip->ecc.write_subpage = omap_write_subpage_bch;
+ mtd_set_ooblayout(mtd, &omap_ooblayout_ops);
+ oobbytes_per_step = chip->ecc.bytes;
+
+ err = elm_config(info->elm_dev, BCH4_ECC,
+ mtd->writesize / chip->ecc.size,
+ chip->ecc.size, chip->ecc.bytes);
+ if (err < 0)
+ return err;
+ break;
+
+ case OMAP_ECC_BCH8_CODE_HW_DETECTION_SW:
+ pr_info("nand: using OMAP_ECC_BCH8_CODE_HW_DETECTION_SW\n");
+ chip->ecc.mode = NAND_ECC_HW;
+ chip->ecc.size = 512;
+ chip->ecc.bytes = 13;
+ chip->ecc.strength = 8;
+ chip->ecc.hwctl = omap_enable_hwecc_bch;
+ chip->ecc.correct = nand_bch_correct_data;
+ chip->ecc.calculate = omap_calculate_ecc_bch_sw;
+ mtd_set_ooblayout(mtd, &omap_sw_ooblayout_ops);
+ /* Reserve one byte for the OMAP marker */
+ oobbytes_per_step = chip->ecc.bytes + 1;
+ /* Software BCH library is used for locating errors */
+ chip->ecc.priv = nand_bch_init(mtd);
+ if (!chip->ecc.priv) {
+ dev_err(dev, "unable to use BCH library\n");
+ return -EINVAL;
+ }
+ break;
+
+ case OMAP_ECC_BCH8_CODE_HW:
+ pr_info("nand: using OMAP_ECC_BCH8_CODE_HW ECC scheme\n");
+ chip->ecc.mode = NAND_ECC_HW;
+ chip->ecc.size = 512;
+ /* 14th bit is kept reserved for ROM-code compatibility */
+ chip->ecc.bytes = 13 + 1;
+ chip->ecc.strength = 8;
+ chip->ecc.hwctl = omap_enable_hwecc_bch;
+ chip->ecc.correct = omap_elm_correct_data;
+ chip->ecc.read_page = omap_read_page_bch;
+ chip->ecc.write_page = omap_write_page_bch;
+ chip->ecc.write_subpage = omap_write_subpage_bch;
+ mtd_set_ooblayout(mtd, &omap_ooblayout_ops);
+ oobbytes_per_step = chip->ecc.bytes;
+
+ err = elm_config(info->elm_dev, BCH8_ECC,
+ mtd->writesize / chip->ecc.size,
+ chip->ecc.size, chip->ecc.bytes);
+ if (err < 0)
+ return err;
+
+ break;
+
+ case OMAP_ECC_BCH16_CODE_HW:
+ pr_info("Using OMAP_ECC_BCH16_CODE_HW ECC scheme\n");
+ chip->ecc.mode = NAND_ECC_HW;
+ chip->ecc.size = 512;
+ chip->ecc.bytes = 26;
+ chip->ecc.strength = 16;
+ chip->ecc.hwctl = omap_enable_hwecc_bch;
+ chip->ecc.correct = omap_elm_correct_data;
+ chip->ecc.read_page = omap_read_page_bch;
+ chip->ecc.write_page = omap_write_page_bch;
+ chip->ecc.write_subpage = omap_write_subpage_bch;
+ mtd_set_ooblayout(mtd, &omap_ooblayout_ops);
+ oobbytes_per_step = chip->ecc.bytes;
+
+ err = elm_config(info->elm_dev, BCH16_ECC,
+ mtd->writesize / chip->ecc.size,
+ chip->ecc.size, chip->ecc.bytes);
+ if (err < 0)
+ return err;
+
+ break;
+ default:
+ dev_err(dev, "Invalid or unsupported ECC scheme\n");
+ return -EINVAL;
+ }
+
+ /* Check if NAND device's OOB is enough to store ECC signatures */
+ min_oobbytes += (oobbytes_per_step *
+ (mtd->writesize / chip->ecc.size));
+ if (mtd->oobsize < min_oobbytes) {
+ dev_err(dev,
+ "Not enough OOB bytes: required = %d, available=%d\n",
+ min_oobbytes, mtd->oobsize);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static struct nand_controller_ops omap_nand_controller_ops = {
+ .attach_chip = omap_nand_attach_chip,
+};
+
static int omap_nand_probe(struct platform_device *pdev)
{
struct omap_nand_info *info;
struct mtd_info *mtd;
struct nand_chip *nand_chip;
int err;
- dma_cap_mask_t mask;
struct resource *res;
struct device *dev = &pdev->dev;
- int min_oobbytes = BADBLOCK_MARKER_LENGTH;
- int oobbytes_per_step;

info = devm_kzalloc(&pdev->dev, sizeof(struct omap_nand_info),
GFP_KERNEL);
@@ -1967,6 +2221,7 @@ static int omap_nand_probe(struct platform_device *pdev)
info->phys_base = res->start;

nand_chip->controller = &omap_gpmc_controller;
+ nand_chip->controller->ops = &omap_nand_controller_ops;

nand_chip->IO_ADDR_W = nand_chip->IO_ADDR_R;
nand_chip->cmd_ctrl = omap_hwcontrol;
@@ -1998,266 +2253,8 @@ static int omap_nand_probe(struct platform_device *pdev)

/* scan NAND device connected to chip controller */
nand_chip->options |= info->devsize & NAND_BUSWIDTH_16;
- err = nand_scan_ident(mtd, 1, NULL);
- if (err) {
- dev_err(&info->pdev->dev,
- "scan failed, may be bus-width mismatch\n");
- goto return_error;
- }

- if (nand_chip->bbt_options & NAND_BBT_USE_FLASH)
- nand_chip->bbt_options |= NAND_BBT_NO_OOB;
- else
- nand_chip->options |= NAND_SKIP_BBTSCAN;
-
- /* re-populate low-level callbacks based on xfer modes */
- switch (info->xfer_type) {
- case NAND_OMAP_PREFETCH_POLLED:
- nand_chip->read_buf = omap_read_buf_pref;
- nand_chip->write_buf = omap_write_buf_pref;
- break;
-
- case NAND_OMAP_POLLED:
- /* Use nand_base defaults for {read,write}_buf */
- break;
-
- case NAND_OMAP_PREFETCH_DMA:
- dma_cap_zero(mask);
- dma_cap_set(DMA_SLAVE, mask);
- info->dma = dma_request_chan(pdev->dev.parent, "rxtx");
-
- if (IS_ERR(info->dma)) {
- dev_err(&pdev->dev, "DMA engine request failed\n");
- err = PTR_ERR(info->dma);
- goto return_error;
- } else {
- struct dma_slave_config cfg;
-
- memset(&cfg, 0, sizeof(cfg));
- cfg.src_addr = info->phys_base;
- cfg.dst_addr = info->phys_base;
- cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
- cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
- cfg.src_maxburst = 16;
- cfg.dst_maxburst = 16;
- err = dmaengine_slave_config(info->dma, &cfg);
- if (err) {
- dev_err(&pdev->dev, "DMA engine slave config failed: %d\n",
- err);
- goto return_error;
- }
- nand_chip->read_buf = omap_read_buf_dma_pref;
- nand_chip->write_buf = omap_write_buf_dma_pref;
- }
- break;
-
- case NAND_OMAP_PREFETCH_IRQ:
- info->gpmc_irq_fifo = platform_get_irq(pdev, 0);
- if (info->gpmc_irq_fifo <= 0) {
- dev_err(&pdev->dev, "error getting fifo irq\n");
- err = -ENODEV;
- goto return_error;
- }
- err = devm_request_irq(&pdev->dev, info->gpmc_irq_fifo,
- omap_nand_irq, IRQF_SHARED,
- "gpmc-nand-fifo", info);
- if (err) {
- dev_err(&pdev->dev, "requesting irq(%d) error:%d",
- info->gpmc_irq_fifo, err);
- info->gpmc_irq_fifo = 0;
- goto return_error;
- }
-
- info->gpmc_irq_count = platform_get_irq(pdev, 1);
- if (info->gpmc_irq_count <= 0) {
- dev_err(&pdev->dev, "error getting count irq\n");
- err = -ENODEV;
- goto return_error;
- }
- err = devm_request_irq(&pdev->dev, info->gpmc_irq_count,
- omap_nand_irq, IRQF_SHARED,
- "gpmc-nand-count", info);
- if (err) {
- dev_err(&pdev->dev, "requesting irq(%d) error:%d",
- info->gpmc_irq_count, err);
- info->gpmc_irq_count = 0;
- goto return_error;
- }
-
- nand_chip->read_buf = omap_read_buf_irq_pref;
- nand_chip->write_buf = omap_write_buf_irq_pref;
-
- break;
-
- default:
- dev_err(&pdev->dev,
- "xfer_type(%d) not supported!\n", info->xfer_type);
- err = -EINVAL;
- goto return_error;
- }
-
- if (!omap2_nand_ecc_check(info)) {
- err = -EINVAL;
- goto return_error;
- }
-
- /*
- * Bail out earlier to let NAND_ECC_SOFT code create its own
- * ooblayout instead of using ours.
- */
- if (info->ecc_opt == OMAP_ECC_HAM1_CODE_SW) {
- nand_chip->ecc.mode = NAND_ECC_SOFT;
- nand_chip->ecc.algo = NAND_ECC_HAMMING;
- goto scan_tail;
- }
-
- /* populate MTD interface based on ECC scheme */
- switch (info->ecc_opt) {
- case OMAP_ECC_HAM1_CODE_HW:
- pr_info("nand: using OMAP_ECC_HAM1_CODE_HW\n");
- nand_chip->ecc.mode = NAND_ECC_HW;
- nand_chip->ecc.bytes = 3;
- nand_chip->ecc.size = 512;
- nand_chip->ecc.strength = 1;
- nand_chip->ecc.calculate = omap_calculate_ecc;
- nand_chip->ecc.hwctl = omap_enable_hwecc;
- nand_chip->ecc.correct = omap_correct_data;
- mtd_set_ooblayout(mtd, &omap_ooblayout_ops);
- oobbytes_per_step = nand_chip->ecc.bytes;
-
- if (!(nand_chip->options & NAND_BUSWIDTH_16))
- min_oobbytes = 1;
-
- break;
-
- case OMAP_ECC_BCH4_CODE_HW_DETECTION_SW:
- pr_info("nand: using OMAP_ECC_BCH4_CODE_HW_DETECTION_SW\n");
- nand_chip->ecc.mode = NAND_ECC_HW;
- nand_chip->ecc.size = 512;
- nand_chip->ecc.bytes = 7;
- nand_chip->ecc.strength = 4;
- nand_chip->ecc.hwctl = omap_enable_hwecc_bch;
- nand_chip->ecc.correct = nand_bch_correct_data;
- nand_chip->ecc.calculate = omap_calculate_ecc_bch_sw;
- mtd_set_ooblayout(mtd, &omap_sw_ooblayout_ops);
- /* Reserve one byte for the OMAP marker */
- oobbytes_per_step = nand_chip->ecc.bytes + 1;
- /* software bch library is used for locating errors */
- nand_chip->ecc.priv = nand_bch_init(mtd);
- if (!nand_chip->ecc.priv) {
- dev_err(&info->pdev->dev, "unable to use BCH library\n");
- err = -EINVAL;
- goto return_error;
- }
- break;
-
- case OMAP_ECC_BCH4_CODE_HW:
- pr_info("nand: using OMAP_ECC_BCH4_CODE_HW ECC scheme\n");
- nand_chip->ecc.mode = NAND_ECC_HW;
- nand_chip->ecc.size = 512;
- /* 14th bit is kept reserved for ROM-code compatibility */
- nand_chip->ecc.bytes = 7 + 1;
- nand_chip->ecc.strength = 4;
- nand_chip->ecc.hwctl = omap_enable_hwecc_bch;
- nand_chip->ecc.correct = omap_elm_correct_data;
- nand_chip->ecc.read_page = omap_read_page_bch;
- nand_chip->ecc.write_page = omap_write_page_bch;
- nand_chip->ecc.write_subpage = omap_write_subpage_bch;
- mtd_set_ooblayout(mtd, &omap_ooblayout_ops);
- oobbytes_per_step = nand_chip->ecc.bytes;
-
- err = elm_config(info->elm_dev, BCH4_ECC,
- mtd->writesize / nand_chip->ecc.size,
- nand_chip->ecc.size, nand_chip->ecc.bytes);
- if (err < 0)
- goto return_error;
- break;
-
- case OMAP_ECC_BCH8_CODE_HW_DETECTION_SW:
- pr_info("nand: using OMAP_ECC_BCH8_CODE_HW_DETECTION_SW\n");
- nand_chip->ecc.mode = NAND_ECC_HW;
- nand_chip->ecc.size = 512;
- nand_chip->ecc.bytes = 13;
- nand_chip->ecc.strength = 8;
- nand_chip->ecc.hwctl = omap_enable_hwecc_bch;
- nand_chip->ecc.correct = nand_bch_correct_data;
- nand_chip->ecc.calculate = omap_calculate_ecc_bch_sw;
- mtd_set_ooblayout(mtd, &omap_sw_ooblayout_ops);
- /* Reserve one byte for the OMAP marker */
- oobbytes_per_step = nand_chip->ecc.bytes + 1;
- /* software bch library is used for locating errors */
- nand_chip->ecc.priv = nand_bch_init(mtd);
- if (!nand_chip->ecc.priv) {
- dev_err(&info->pdev->dev, "unable to use BCH library\n");
- err = -EINVAL;
- goto return_error;
- }
- break;
-
- case OMAP_ECC_BCH8_CODE_HW:
- pr_info("nand: using OMAP_ECC_BCH8_CODE_HW ECC scheme\n");
- nand_chip->ecc.mode = NAND_ECC_HW;
- nand_chip->ecc.size = 512;
- /* 14th bit is kept reserved for ROM-code compatibility */
- nand_chip->ecc.bytes = 13 + 1;
- nand_chip->ecc.strength = 8;
- nand_chip->ecc.hwctl = omap_enable_hwecc_bch;
- nand_chip->ecc.correct = omap_elm_correct_data;
- nand_chip->ecc.read_page = omap_read_page_bch;
- nand_chip->ecc.write_page = omap_write_page_bch;
- nand_chip->ecc.write_subpage = omap_write_subpage_bch;
- mtd_set_ooblayout(mtd, &omap_ooblayout_ops);
- oobbytes_per_step = nand_chip->ecc.bytes;
-
- err = elm_config(info->elm_dev, BCH8_ECC,
- mtd->writesize / nand_chip->ecc.size,
- nand_chip->ecc.size, nand_chip->ecc.bytes);
- if (err < 0)
- goto return_error;
-
- break;
-
- case OMAP_ECC_BCH16_CODE_HW:
- pr_info("using OMAP_ECC_BCH16_CODE_HW ECC scheme\n");
- nand_chip->ecc.mode = NAND_ECC_HW;
- nand_chip->ecc.size = 512;
- nand_chip->ecc.bytes = 26;
- nand_chip->ecc.strength = 16;
- nand_chip->ecc.hwctl = omap_enable_hwecc_bch;
- nand_chip->ecc.correct = omap_elm_correct_data;
- nand_chip->ecc.read_page = omap_read_page_bch;
- nand_chip->ecc.write_page = omap_write_page_bch;
- nand_chip->ecc.write_subpage = omap_write_subpage_bch;
- mtd_set_ooblayout(mtd, &omap_ooblayout_ops);
- oobbytes_per_step = nand_chip->ecc.bytes;
-
- err = elm_config(info->elm_dev, BCH16_ECC,
- mtd->writesize / nand_chip->ecc.size,
- nand_chip->ecc.size, nand_chip->ecc.bytes);
- if (err < 0)
- goto return_error;
-
- break;
- default:
- dev_err(&info->pdev->dev, "invalid or unsupported ECC scheme\n");
- err = -EINVAL;
- goto return_error;
- }
-
- /* check if NAND device's OOB is enough to store ECC signatures */
- min_oobbytes += (oobbytes_per_step *
- (mtd->writesize / nand_chip->ecc.size));
- if (mtd->oobsize < min_oobbytes) {
- dev_err(&info->pdev->dev,
- "not enough OOB bytes required = %d, available=%d\n",
- min_oobbytes, mtd->oobsize);
- err = -EINVAL;
- goto return_error;
- }
-
-scan_tail:
- /* second phase scan */
- err = nand_scan_tail(mtd);
+ err = nand_scan(mtd, 1);
if (err)
goto return_error;

--
2.14.1

2018-07-19 23:02:34

by Miquel Raynal

[permalink] [raw]

Subject: [PATCH v3 18/33] mtd: rawnand: s3c2410: convert driver to nand_scan()

2018-07-19 23:02:36

by Miquel Raynal

[permalink] [raw]

Subject: [PATCH v3 19/33] mtd: rawnand: sh_flctl: move all NAND chip related setup in one function

Prepare the conversion of the sh_flctl driver to nand_scan() by moving
all the configuration that must be made after nand_scan_ident() in one
single function. Create a pdata entry in the controller structure for
that.

Signed-off-by: Miquel Raynal <[email protected]>
---
drivers/mtd/nand/raw/sh_flctl.c | 38 +++++++++++++++++++-------------------
include/linux/mtd/sh_flctl.h | 1 +
2 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/drivers/mtd/nand/raw/sh_flctl.c b/drivers/mtd/nand/raw/sh_flctl.c
index c7abceffcc40..d37d1d3ccbf9 100644
--- a/drivers/mtd/nand/raw/sh_flctl.c
+++ b/drivers/mtd/nand/raw/sh_flctl.c
@@ -1007,6 +1007,16 @@ static int flctl_chip_init_tail(struct mtd_info *mtd)
struct sh_flctl *flctl = mtd_to_flctl(mtd);
struct nand_chip *chip = &flctl->chip;

+ if (chip->options & NAND_BUSWIDTH_16) {
+ /*
+ * NAND_BUSWIDTH_16 may have been set by nand_scan_ident().
+ * Add the SEL_16BIT flag in pdata->flcmncr_val and re-assign
+ * flctl->flcmncr_base to pdata->flcmncr_val.
+ */
+ flctl->pdata->flcmncr_val |= SEL_16BIT;
+ flctl->flcmncr_base = flctl->pdata->flcmncr_val;
+ }
+
if (mtd->writesize == 512) {
flctl->page_size = 0;
if (chip->chipsize > (32 << 20)) {
@@ -1122,7 +1132,6 @@ static int flctl_probe(struct platform_device *pdev)
struct sh_flctl *flctl;
struct mtd_info *flctl_mtd;
struct nand_chip *nand;
- struct sh_flctl_platform_data *pdata;
int ret;
int irq;

@@ -1150,11 +1159,11 @@ static int flctl_probe(struct platform_device *pdev)
}

if (pdev->dev.of_node)
- pdata = flctl_parse_dt(&pdev->dev);
+ flctl->pdata = flctl_parse_dt(&pdev->dev);
else
- pdata = dev_get_platdata(&pdev->dev);
+ flctl->pdata = dev_get_platdata(&pdev->dev);

- if (!pdata) {
+ if (!flctl->pdata) {
dev_err(&pdev->dev, "no setup data defined\n");
return -EINVAL;
}
@@ -1165,9 +1174,9 @@ static int flctl_probe(struct platform_device *pdev)
nand_set_flash_node(nand, pdev->dev.of_node);
flctl_mtd->dev.parent = &pdev->dev;
flctl->pdev = pdev;
- flctl->hwecc = pdata->has_hwecc;
- flctl->holden = pdata->use_holden;
- flctl->flcmncr_base = pdata->flcmncr_val;
+ flctl->hwecc = flctl->pdata->has_hwecc;
+ flctl->holden = flctl->pdata->use_holden;
+ flctl->flcmncr_base = flctl->pdata->flcmncr_val;
flctl->flintdmacr_base = flctl->hwecc ? (STERINTE | ECERB) : STERINTE;

/* Set address of hardware control function */
@@ -1183,7 +1192,7 @@ static int flctl_probe(struct platform_device *pdev)
nand->set_features = nand_get_set_features_notsupp;
nand->get_features = nand_get_set_features_notsupp;

- if (pdata->flcmncr_val & SEL_16BIT)
+ if (flctl->pdata->flcmncr_val & SEL_16BIT)
nand->options |= NAND_BUSWIDTH_16;

pm_runtime_enable(&pdev->dev);
@@ -1195,16 +1204,6 @@ static int flctl_probe(struct platform_device *pdev)
if (ret)
goto err_chip;

- if (nand->options & NAND_BUSWIDTH_16) {
- /*
- * NAND_BUSWIDTH_16 may have been set by nand_scan_ident().
- * Add the SEL_16BIT flag in pdata->flcmncr_val and re-assign
- * flctl->flcmncr_base to pdata->flcmncr_val.
- */
- pdata->flcmncr_val |= SEL_16BIT;
- flctl->flcmncr_base = pdata->flcmncr_val;
- }
-
ret = flctl_chip_init_tail(flctl_mtd);
if (ret)
goto err_chip;
@@ -1213,7 +1212,8 @@ static int flctl_probe(struct platform_device *pdev)
if (ret)
goto err_chip;

- ret = mtd_device_register(flctl_mtd, pdata->parts, pdata->nr_parts);
+ ret = mtd_device_register(flctl_mtd, flctl->pdata->parts,
+ flctl->pdata->nr_parts);
if (ret)
goto cleanup_nand;

diff --git a/include/linux/mtd/sh_flctl.h b/include/linux/mtd/sh_flctl.h
index c759d403cbc0..c9f486c6c019 100644
--- a/include/linux/mtd/sh_flctl.h
+++ b/include/linux/mtd/sh_flctl.h
@@ -148,6 +148,7 @@ struct sh_flctl {
struct dev_pm_qos_request pm_qos;
void __iomem *reg;
resource_size_t fifo;
+ struct sh_flctl_platform_data *pdata;

uint8_t done_buff[2048 + 64]; /* max size 2048 + 64 */
int read_bytes;
--
2.14.1

2018-07-19 23:02:50

by Miquel Raynal

[permalink] [raw]

Subject: [PATCH v3 20/33] mtd: rawnand: sh_flctl: convert driver to nand_scan()

2018-07-19 23:03:05

by Miquel Raynal

[permalink] [raw]

Subject: [PATCH v3 21/33] mtd: rawnand: sunxi: convert driver to nand_scan()

2018-07-19 23:03:06

by Miquel Raynal

[permalink] [raw]

Subject: [PATCH v3 22/33] mtd: rawnand: tango: convert driver to nand_scan()

2018-07-19 23:03:10

by Miquel Raynal

[permalink] [raw]

Subject: [PATCH v3 24/33] mtd: rawnand: txx9ndfmc: convert driver to nand_scan()

2018-07-19 23:03:18

by Miquel Raynal

[permalink] [raw]

Subject: [PATCH v3 32/33] mtd: rawnand: do not export nand_scan_[ident|tail]() anymore

Both nand_scan_ident() and nand_scan_tail() helpers used to be called
directly from controller drivers that needed to tweak some ECC-related
parameters before nand_scan_tail(). This separation prevented dynamic
allocations during the phase of NAND identification, which was
inconvenient.

All controller drivers have been moved to use nand_scan(), in
conjunction with the chip->ecc.[attach|detach]_chip() hooks that
actually do the required tweaking sequence between both ident/tail
calls, allowing programmers to use dynamic allocation as they need all
across the scanning sequence.

Signed-off-by: Miquel Raynal <[email protected]>
---
drivers/mtd/nand/raw/nand_base.c | 60 +++++++++++++++++++++++++++++++---------
include/linux/mtd/rawnand.h | 11 ++------
2 files changed, 50 insertions(+), 21 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index dea41fa25be1..dfee2556a8a8 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -5151,6 +5151,7 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
{
struct mtd_info *mtd = nand_to_mtd(chip);
struct nand_onfi_params *p;
+ char *model;
char id[4];
int i, ret, val;

@@ -5225,8 +5226,15 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)

sanitize_string(p->manufacturer, sizeof(p->manufacturer));
sanitize_string(p->model, sizeof(p->model));
- strncpy(chip->parameters.model, p->model,
- sizeof(chip->parameters.model) - 1);
+ model = kmemdup(p->model, sizeof(p->model), GFP_KERNEL);
+ if (!model) {
+ ret = -ENOMEM;
+ goto free_onfi_param_page;
+ }
+
+ chip->parameters.model = model;
+ if (!mtd->name)
+ mtd->name = chip->parameters.model;

mtd->writesize = le32_to_cpu(p->byte_per_page);

@@ -5306,6 +5314,7 @@ static int nand_flash_detect_jedec(struct nand_chip *chip)
struct nand_jedec_params *p;
struct jedec_ecc_info *ecc;
int jedec_version = 0;
+ char *model;
char id[5];
int i, val, ret;

@@ -5356,8 +5365,16 @@ static int nand_flash_detect_jedec(struct nand_chip *chip)

sanitize_string(p->manufacturer, sizeof(p->manufacturer));
sanitize_string(p->model, sizeof(p->model));
- strncpy(chip->parameters.model, p->model,
- sizeof(chip->parameters.model) - 1);
+ model = kzalloc(sizeof(p->model), GFP_KERNEL);
+ if (!p) {
+ ret = -ENOMEM;
+ goto free_jedec_param_page;
+ }
+
+ strncpy(model, p->model, sizeof(model) - 1);
+ chip->parameters.model = model;
+ if (!mtd->name)
+ mtd->name = chip->parameters.model;

mtd->writesize = le32_to_cpu(p->byte_per_page);

@@ -5924,7 +5941,7 @@ static int nand_dt_init(struct nand_chip *chip)
}

/**
- * nand_scan_ident - [NAND Interface] Scan for the NAND device
+ * nand_scan_ident - Scan for the NAND device
* @mtd: MTD device structure
* @maxchips: number of chips to scan for
* @table: alternative NAND ID table
@@ -5932,9 +5949,13 @@ static int nand_dt_init(struct nand_chip *chip)
* This is the first phase of the normal nand_scan() function. It reads the
* flash ID and sets up MTD fields accordingly.
*
+ * This helper used to be called directly from controller drivers that needed
+ * to tweak some ECC-related parameters before nand_scan_tail(). This separation
+ * prevented dynamic allocations during this phase which was unconvenient and
+ * as been banned for the benefit of the ->init_ecc()/cleanup_ecc() hooks.
*/
-int nand_scan_ident(struct mtd_info *mtd, int maxchips,
- struct nand_flash_dev *table)
+static int nand_scan_ident(struct mtd_info *mtd, int maxchips,
+ struct nand_flash_dev *table)
{
int i, nand_maf_id, nand_dev_id;
struct nand_chip *chip = mtd_to_nand(mtd);
@@ -6008,7 +6029,11 @@ int nand_scan_ident(struct mtd_info *mtd, int maxchips,

return 0;
}
-EXPORT_SYMBOL(nand_scan_ident);
+
+static void nand_scan_ident_cleanup(struct nand_chip *chip)
+{
+ kfree(chip->parameters.model);
+}

static int nand_set_ecc_soft_ops(struct mtd_info *mtd)
{
@@ -6385,14 +6410,14 @@ static bool nand_ecc_strength_good(struct mtd_info *mtd)
}

/**
- * nand_scan_tail - [NAND Interface] Scan for the NAND device
+ * nand_scan_tail - Scan for the NAND device
* @mtd: MTD device structure
*
* This is the second phase of the normal nand_scan() function. It fills out
* all the uninitialized function pointers with the defaults and scans for a
* bad block table if appropriate.
*/
-int nand_scan_tail(struct mtd_info *mtd)
+static int nand_scan_tail(struct mtd_info *mtd)
{
struct nand_chip *chip = mtd_to_nand(mtd);
struct nand_ecc_ctrl *ecc = &chip->ecc;
@@ -6716,7 +6741,6 @@ int nand_scan_tail(struct mtd_info *mtd)

return ret;
}
-EXPORT_SYMBOL(nand_scan_tail);

static int nand_attach(struct nand_chip *chip)
{
@@ -6754,11 +6778,18 @@ int nand_scan_with_ids(struct mtd_info *mtd, int maxchips,

ret = nand_attach(chip);
if (ret)
- return ret;
+ goto cleanup_ident;

ret = nand_scan_tail(mtd);
if (ret)
- nand_detach(chip);
+ goto detach_chip;
+
+ return 0;
+
+detach_chip:
+ nand_detach(chip);
+cleanup_ident:
+ nand_scan_ident_cleanup(chip);

return ret;
}
@@ -6790,6 +6821,9 @@ void nand_cleanup(struct nand_chip *chip)

/* Free controller specific allocations after chip identification */
nand_detach(chip);
+
+ /* Free identification phase allocations */
+ nand_scan_ident_cleanup(chip);
}

EXPORT_SYMBOL_GPL(nand_cleanup);
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index a20c78e25878..5723d940a47d 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -36,14 +36,9 @@ static inline int nand_scan(struct mtd_info *mtd, int max_chips)
}

/*
- * Separate phases of nand_scan(), allowing board driver to intervene
- * and override command or ECC setup according to flash type.
+ * Unregister the MTD device and free resources held by the NAND device, must be
+ * called on error after a successful nand_scan().
*/
-int nand_scan_ident(struct mtd_info *mtd, int max_chips,
- struct nand_flash_dev *table);
-int nand_scan_tail(struct mtd_info *mtd);
-
-/* Unregister the MTD device and free resources held by the NAND device */
void nand_release(struct mtd_info *mtd);

/* Internal helper for board drivers which need to override command function */
@@ -487,7 +482,7 @@ struct onfi_params {
*/
struct nand_parameters {
/* Generic parameters */
- char model[100];
+ const char *model;
bool supports_set_get_features;
DECLARE_BITMAP(set_feature_list, ONFI_FEATURE_NUMBER);
DECLARE_BITMAP(get_feature_list, ONFI_FEATURE_NUMBER);
--
2.14.1

2018-07-19 23:03:23

by Miquel Raynal

[permalink] [raw]

Subject: [PATCH v3 33/33] mtd: rawnand: allocate dynamically ONFI parameters during detection

Now that it is possible to do dynamic allocations during the
identification phase, convert the onfi_params structure (which is only
needed with ONFI compliant chips) into a pointer that will be allocated
only if needed.

Signed-off-by: Miquel Raynal <[email protected]>
---
drivers/mtd/nand/raw/nand_base.c | 61 ++++++++++++++++++++++---------------
drivers/mtd/nand/raw/nand_micron.c | 6 ++--
drivers/mtd/nand/raw/nand_timings.c | 12 ++++----
include/linux/mtd/rawnand.h | 6 ++--
4 files changed, 48 insertions(+), 37 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index dfee2556a8a8..4dc248769abb 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -5151,6 +5151,8 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
{
struct mtd_info *mtd = nand_to_mtd(chip);
struct nand_onfi_params *p;
+ struct onfi_params *onfi;
+ int onfi_version = 0;
char *model;
char id[4];
int i, ret, val;
@@ -5207,21 +5209,19 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
/* Check version */
val = le16_to_cpu(p->revision);
if (val & ONFI_VERSION_2_3)
- chip->parameters.onfi.version = 23;
+ onfi_version = 23;
else if (val & ONFI_VERSION_2_2)
- chip->parameters.onfi.version = 22;
+ onfi_version = 22;
else if (val & ONFI_VERSION_2_1)
- chip->parameters.onfi.version = 21;
+ onfi_version = 21;
else if (val & ONFI_VERSION_2_0)
- chip->parameters.onfi.version = 20;
+ onfi_version = 20;
else if (val & ONFI_VERSION_1_0)
- chip->parameters.onfi.version = 10;
+ onfi_version = 10;

- if (!chip->parameters.onfi.version) {
+ if (!onfi_version) {
pr_info("unsupported ONFI version: %d\n", val);
goto free_onfi_param_page;
- } else {
- ret = 1;
}

sanitize_string(p->manufacturer, sizeof(p->manufacturer));
@@ -5262,7 +5262,7 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
if (p->ecc_bits != 0xff) {
chip->ecc_strength_ds = p->ecc_bits;
chip->ecc_step_ds = 512;
- } else if (chip->parameters.onfi.version >= 21 &&
+ } else if (onfi_version >= 21 &&
(le16_to_cpu(p->features) & ONFI_FEATURE_EXT_PARAM_PAGE)) {

/*
@@ -5289,19 +5289,32 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
bitmap_set(chip->parameters.set_feature_list,
ONFI_FEATURE_ADDR_TIMING_MODE, 1);
}
- chip->parameters.onfi.tPROG = le16_to_cpu(p->t_prog);
- chip->parameters.onfi.tBERS = le16_to_cpu(p->t_bers);
- chip->parameters.onfi.tR = le16_to_cpu(p->t_r);
- chip->parameters.onfi.tCCS = le16_to_cpu(p->t_ccs);
- chip->parameters.onfi.async_timing_mode =
- le16_to_cpu(p->async_timing_mode);
- chip->parameters.onfi.vendor_revision =
- le16_to_cpu(p->vendor_revision);
- memcpy(chip->parameters.onfi.vendor, p->vendor,
- sizeof(p->vendor));

+ onfi = kzalloc(sizeof(*onfi), GFP_KERNEL);
+ if (!onfi) {
+ ret = -ENOMEM;
+ goto free_model;
+ }
+ onfi->version = onfi_version;
+ onfi->tPROG = le16_to_cpu(p->t_prog);
+ onfi->tBERS = le16_to_cpu(p->t_bers);
+ onfi->tR = le16_to_cpu(p->t_r);
+ onfi->tCCS = le16_to_cpu(p->t_ccs);
+ onfi->async_timing_mode = le16_to_cpu(p->async_timing_mode);
+ onfi->vendor_revision = le16_to_cpu(p->vendor_revision);
+ memcpy(onfi->vendor, p->vendor, sizeof(p->vendor));
+ chip->parameters.onfi = onfi;
+
+ /* Identification done, free the full ONFI parameter page and exit */
+ kfree(p);
+
+ return 1;
+
+free_model:
+ kfree(model);
free_onfi_param_page:
kfree(p);
+
return ret;
}

@@ -5562,9 +5575,7 @@ static bool find_full_id_nand(struct nand_chip *chip,
chip->ecc_step_ds = NAND_ECC_STEP(type);
chip->onfi_timing_mode_default =
type->onfi_timing_mode_default;
-
- strncpy(chip->parameters.model, type->name,
- sizeof(chip->parameters.model) - 1);
+ chip->parameters.model = type->name;

return true;
}
@@ -5703,7 +5714,6 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
}
}

- chip->parameters.onfi.version = 0;
if (!type->name || !type->pagesize) {
/* Check if the chip is ONFI compliant */
ret = nand_flash_detect_onfi(chip);
@@ -5723,8 +5733,7 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
if (!type->name)
return -ENODEV;

- strncpy(chip->parameters.model, type->name,
- sizeof(chip->parameters.model) - 1);
+ chip->parameters.model = type->name;

chip->chipsize = (uint64_t)type->chipsize << 20;

@@ -6033,6 +6042,8 @@ static int nand_scan_ident(struct mtd_info *mtd, int maxchips,
static void nand_scan_ident_cleanup(struct nand_chip *chip)
{
kfree(chip->parameters.model);
+ kfree(chip->parameters.onfi);
+
}

static int nand_set_ecc_soft_ops(struct mtd_info *mtd)
diff --git a/drivers/mtd/nand/raw/nand_micron.c b/drivers/mtd/nand/raw/nand_micron.c
index 656947d91841..8466a1740b3b 100644
--- a/drivers/mtd/nand/raw/nand_micron.c
+++ b/drivers/mtd/nand/raw/nand_micron.c
@@ -88,9 +88,9 @@ static int micron_nand_setup_read_retry(struct mtd_info *mtd, int retry_mode)
static int micron_nand_onfi_init(struct nand_chip *chip)
{
struct nand_parameters *p = &chip->parameters;
- struct nand_onfi_vendor_micron *micron = (void *)p->onfi.vendor;
+ struct nand_onfi_vendor_micron *micron = (void *)p->onfi->vendor;

- if (chip->parameters.onfi.version && p->onfi.vendor_revision) {
+ if (p->onfi && p->onfi->vendor_revision) {
chip->read_retries = micron->read_retry_options;
chip->setup_read_retry = micron_nand_setup_read_retry;
}
@@ -382,7 +382,7 @@ static int micron_supports_on_die_ecc(struct nand_chip *chip)
u8 id[5];
int ret;

- if (!chip->parameters.onfi.version)
+ if (!chip->parameters.onfi)
return MICRON_ON_DIE_UNSUPPORTED;

if (chip->bits_per_cell != 1)
diff --git a/drivers/mtd/nand/raw/nand_timings.c b/drivers/mtd/nand/raw/nand_timings.c
index 9bb599106a31..ebc7b5f76f77 100644
--- a/drivers/mtd/nand/raw/nand_timings.c
+++ b/drivers/mtd/nand/raw/nand_timings.c
@@ -294,6 +294,7 @@ int onfi_fill_data_interface(struct nand_chip *chip,
int timing_mode)
{
struct nand_data_interface *iface = &chip->data_interface;
+ struct onfi_params *onfi = chip->parameters.onfi;

if (type != NAND_SDR_IFACE)
return -EINVAL;
@@ -308,17 +309,16 @@ int onfi_fill_data_interface(struct nand_chip *chip,
* tPROG, tBERS, tR and tCCS.
* These information are part of the ONFI parameter page.
*/
- if (chip->parameters.onfi.version) {
- struct nand_parameters *params = &chip->parameters;
+ if (onfi) {
struct nand_sdr_timings *timings = &iface->timings.sdr;

/* microseconds -> picoseconds */
- timings->tPROG_max = 1000000ULL * params->onfi.tPROG;
- timings->tBERS_max = 1000000ULL * params->onfi.tBERS;
- timings->tR_max = 1000000ULL * params->onfi.tR;
+ timings->tPROG_max = 1000000ULL * onfi->tPROG;
+ timings->tBERS_max = 1000000ULL * onfi->tBERS;
+ timings->tR_max = 1000000ULL * onfi->tR;

/* nanoseconds -> picoseconds */
- timings->tCCS_min = 1000UL * params->onfi.tCCS;
+ timings->tCCS_min = 1000UL * onfi->tCCS;
} else {
struct nand_sdr_timings *timings = &iface->timings.sdr;
/*
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 5723d940a47d..8074cbd4e3fe 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -488,7 +488,7 @@ struct nand_parameters {
DECLARE_BITMAP(get_feature_list, ONFI_FEATURE_NUMBER);

/* ONFI parameters */
- struct onfi_params onfi;
+ struct onfi_params *onfi;
};

/* The maximum expected count of bytes in the NAND ID sequence */
@@ -1618,10 +1618,10 @@ struct platform_nand_data {
/* return the supported asynchronous timing mode. */
static inline int onfi_get_async_timing_mode(struct nand_chip *chip)
{
- if (!chip->parameters.onfi.version)
+ if (!chip->parameters.onfi)
return ONFI_TIMING_MODE_UNKNOWN;

- return chip->parameters.onfi.async_timing_mode;
+ return chip->parameters.onfi->async_timing_mode;
}

int onfi_fill_data_interface(struct nand_chip *chip,
--
2.14.1

2018-07-19 23:03:28

by Miquel Raynal

[permalink] [raw]

Subject: [PATCH v3 31/33] mtd: rawnand: tegra: convert driver to nand_scan()

2018-07-19 23:03:50

by Miquel Raynal

[permalink] [raw]

Subject: [PATCH v3 30/33] mtd: rawnand: jz4740: convert driver to nand_scan()

2018-07-19 23:04:03

by Miquel Raynal

[permalink] [raw]

Subject: [PATCH v3 25/33] mtd: rawnand: vf610: convert driver to nand_scan()

2018-07-19 23:04:15

by Miquel Raynal

[permalink] [raw]

Subject: [PATCH v3 27/33] mtd: rawnand: sm_common: convert driver to nand_scan_with_ids()

Two helpers have been added to the core to make ECC-related
configuration between the detection phase and the final NAND scan. Use
these hooks and convert the driver to just use nand_scan_with_ids()
(alternative to nand_scan() for passing a flash IDs table) instead of
both nand_scan_ident() and nand_scan_tail().

Signed-off-by: Miquel Raynal <[email protected]>
---
drivers/mtd/nand/raw/sm_common.c | 39 +++++++++++++++++++++++++--------------
1 file changed, 25 insertions(+), 14 deletions(-)

diff --git a/drivers/mtd/nand/raw/sm_common.c b/drivers/mtd/nand/raw/sm_common.c
index 7f5044a79f01..92e04acfea3b 100644
--- a/drivers/mtd/nand/raw/sm_common.c
+++ b/drivers/mtd/nand/raw/sm_common.c
@@ -160,19 +160,9 @@ static struct nand_flash_dev nand_xd_flash_ids[] = {
{NULL}
};

-int sm_register_device(struct mtd_info *mtd, int smartmedia)
+static int sm_attach_chip(struct nand_chip *chip)
{
- struct nand_chip *chip = mtd_to_nand(mtd);
- int ret;
-
- chip->options |= NAND_SKIP_BBTSCAN;
-
- /* Scan for card properties */
- ret = nand_scan_ident(mtd, 1, smartmedia ?
- nand_smartmedia_flash_ids : nand_xd_flash_ids);
-
- if (ret)
- return ret;
+ struct mtd_info *mtd = nand_to_mtd(chip);

/* Bad block marker position */
chip->badblockpos = 0x05;
@@ -187,12 +177,33 @@ int sm_register_device(struct mtd_info *mtd, int smartmedia)
else
return -ENODEV;

- ret = nand_scan_tail(mtd);
+ return 0;
+}

+static struct nand_controller_ops sm_controller_ops = {
+ .attach_chip = sm_attach_chip,
+};
+
+int sm_register_device(struct mtd_info *mtd, int smartmedia)
+{
+ struct nand_chip *chip = mtd_to_nand(mtd);
+ struct nand_flash_dev *flash_ids;
+ int ret;
+
+ chip->options |= NAND_SKIP_BBTSCAN;
+
+ /* Scan for card properties */
+ chip->dummy_controller.ops = &sm_controller_ops;
+ flash_ids = smartmedia ? nand_smartmedia_flash_ids : nand_xd_flash_ids;
+ ret = nand_scan_with_ids(mtd, 1, flash_ids);
if (ret)
return ret;

- return mtd_device_register(mtd, NULL, 0);
+ ret = mtd_device_register(mtd, NULL, 0);
+ if (ret)
+ nand_release(mtd);
+
+ return ret;
}
EXPORT_SYMBOL_GPL(sm_register_device);

--
2.14.1

2018-07-19 23:04:16

by Miquel Raynal

[permalink] [raw]

Subject: [PATCH v3 23/33] mtd: rawnand: txx9ndfmc: rename nand controller internal structure

As already done in the core, calling a struct nand_controller
'hw_control' is misleading. Use the same name as in nand_base.c:
'controller'.

Signed-off-by: Miquel Raynal <[email protected]>
---
drivers/mtd/nand/raw/txx9ndfmc.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/raw/txx9ndfmc.c b/drivers/mtd/nand/raw/txx9ndfmc.c
index 8f5bbbac4612..9019022774f7 100644
--- a/drivers/mtd/nand/raw/txx9ndfmc.c
+++ b/drivers/mtd/nand/raw/txx9ndfmc.c
@@ -73,7 +73,7 @@ struct txx9ndfmc_drvdata {
void __iomem *base;
unsigned char hold; /* in gbusclock */
unsigned char spw; /* in gbusclock */
- struct nand_controller hw_control;
+ struct nand_controller controller;
};

static struct platform_device *mtd_to_platdev(struct mtd_info *mtd)
@@ -303,7 +303,7 @@ static int __init txx9ndfmc_probe(struct platform_device *dev)
dev_info(&dev->dev, "CLK:%ldMHz HOLD:%d SPW:%d\n",
(gbusclk + 500000) / 1000000, hold, spw);

- nand_controller_init(&drvdata->hw_control);
+ nand_controller_init(&drvdata->controller);

platform_set_drvdata(dev, drvdata);
txx9ndfmc_initialize(dev);
@@ -337,7 +337,7 @@ static int __init txx9ndfmc_probe(struct platform_device *dev)
chip->ecc.bytes = 3;
chip->ecc.strength = 1;
chip->chip_delay = 100;
- chip->controller = &drvdata->hw_control;
+ chip->controller = &drvdata->controller;

nand_set_controller_data(chip, txx9_priv);
txx9_priv->dev = dev;
--
2.14.1

2018-07-19 23:04:26

by Miquel Raynal

[permalink] [raw]

Subject: [PATCH v3 28/33] mtd: rawnand: docg4: convert driver to nand_scan()

2018-07-19 23:04:37

by Miquel Raynal

[permalink] [raw]

Subject: [PATCH v3 26/33] mtd: rawnand: atmel: convert driver to nand_scan()

2018-07-19 23:04:38

by Miquel Raynal

[permalink] [raw]

Subject: [PATCH v3 09/33] mtd: rawnand: hisi504: convert driver to nand_scan()

2018-07-19 23:04:41

by Miquel Raynal

[permalink] [raw]

Subject: [PATCH v3 29/33] mtd: rawnand: qcom: convert driver to nand_scan()

2018-07-19 23:04:44

by Miquel Raynal

[permalink] [raw]

Subject: [PATCH v3 08/33] mtd: rawnand: gpmi: convert driver to nand_scan()

2018-07-19 23:04:51

by Miquel Raynal

[permalink] [raw]

Subject: [PATCH v3 07/33] mtd: rawnand: fsmc: convert driver to nand_scan()

2018-07-19 23:05:00

by Miquel Raynal

[permalink] [raw]

Subject: [PATCH v3 04/33] mtd: rawnand: denali: convert to nand_scan()

2018-07-19 23:05:01

by Miquel Raynal

[permalink] [raw]

Subject: [PATCH v3 03/33] mtd: rawnand: davinci: convert driver to nand_scan()

Two helpers have been added to the core to make ECC-related
configuration between the detection phase and the final NAND scan. Use
these hooks and convert the driver to just use nand_scan() instead of
both nand_scan_ident() and nand_scan_tail().

Also change the unused "struct device *dev" parameter of the driver
structure into a platform device to reuse it in the ->attach_chip()
hook.

Signed-off-by: Miquel Raynal <[email protected]>
---
drivers/mtd/nand/raw/davinci_nand.c | 195 +++++++++++++++++++-----------------
1 file changed, 102 insertions(+), 93 deletions(-)

diff --git a/drivers/mtd/nand/raw/davinci_nand.c b/drivers/mtd/nand/raw/davinci_nand.c
index 626c9363e460..ae8b07541519 100644
--- a/drivers/mtd/nand/raw/davinci_nand.c
+++ b/drivers/mtd/nand/raw/davinci_nand.c
@@ -53,7 +53,7 @@
struct davinci_nand_info {
struct nand_chip chip;

- struct device *dev;
+ struct platform_device *pdev;

bool is_readmode;

@@ -605,6 +605,104 @@ static struct davinci_nand_pdata
}
#endif

+static int davinci_nand_attach_chip(struct nand_chip *chip)
+{
+ struct mtd_info *mtd = nand_to_mtd(chip);
+ struct davinci_nand_info *info = to_davinci_nand(mtd);
+ struct davinci_nand_pdata *pdata = nand_davinci_get_pdata(info->pdev);
+ int ret = 0;
+
+ if (IS_ERR(pdata))
+ return PTR_ERR(pdata);
+
+ switch (info->chip.ecc.mode) {
+ case NAND_ECC_NONE:
+ pdata->ecc_bits = 0;
+ break;
+ case NAND_ECC_SOFT:
+ pdata->ecc_bits = 0;
+ /*
+ * This driver expects Hamming based ECC when ecc_mode is set
+ * to NAND_ECC_SOFT. Force ecc.algo to NAND_ECC_HAMMING to
+ * avoid adding an extra ->ecc_algo field to
+ * davinci_nand_pdata.
+ */
+ info->chip.ecc.algo = NAND_ECC_HAMMING;
+ break;
+ case NAND_ECC_HW:
+ if (pdata->ecc_bits == 4) {
+ /*
+ * No sanity checks: CPUs must support this,
+ * and the chips may not use NAND_BUSWIDTH_16.
+ */
+
+ /* No sharing 4-bit hardware between chipselects yet */
+ spin_lock_irq(&davinci_nand_lock);
+ if (ecc4_busy)
+ ret = -EBUSY;
+ else
+ ecc4_busy = true;
+ spin_unlock_irq(&davinci_nand_lock);
+
+ if (ret == -EBUSY)
+ return ret;
+
+ info->chip.ecc.calculate = nand_davinci_calculate_4bit;
+ info->chip.ecc.correct = nand_davinci_correct_4bit;
+ info->chip.ecc.hwctl = nand_davinci_hwctl_4bit;
+ info->chip.ecc.bytes = 10;
+ info->chip.ecc.options = NAND_ECC_GENERIC_ERASED_CHECK;
+ info->chip.ecc.algo = NAND_ECC_BCH;
+ } else {
+ /* 1bit ecc hamming */
+ info->chip.ecc.calculate = nand_davinci_calculate_1bit;
+ info->chip.ecc.correct = nand_davinci_correct_1bit;
+ info->chip.ecc.hwctl = nand_davinci_hwctl_1bit;
+ info->chip.ecc.bytes = 3;
+ info->chip.ecc.algo = NAND_ECC_HAMMING;
+ }
+ info->chip.ecc.size = 512;
+ info->chip.ecc.strength = pdata->ecc_bits;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ /*
+ * Update ECC layout if needed ... for 1-bit HW ECC, the default
+ * is OK, but it allocates 6 bytes when only 3 are needed (for
+ * each 512 bytes). For the 4-bit HW ECC, that default is not
+ * usable: 10 bytes are needed, not 6.
+ */
+ if (pdata->ecc_bits == 4) {
+ int chunks = mtd->writesize / 512;
+
+ if (!chunks || mtd->oobsize < 16) {
+ dev_dbg(&info->pdev->dev, "too small\n");
+ return -EINVAL;
+ }
+
+ /* For small page chips, preserve the manufacturer's
+ * badblock marking data ... and make sure a flash BBT
+ * table marker fits in the free bytes.
+ */
+ if (chunks == 1) {
+ mtd_set_ooblayout(mtd, &hwecc4_small_ooblayout_ops);
+ } else if (chunks == 4 || chunks == 8) {
+ mtd_set_ooblayout(mtd, &nand_ooblayout_lp_ops);
+ info->chip.ecc.mode = NAND_ECC_HW_OOB_FIRST;
+ } else {
+ return -EIO;
+ }
+ }
+
+ return ret;
+}
+
+static struct nand_controller_ops davinci_nand_controller_ops = {
+ .attach_chip = davinci_nand_attach_chip,
+};
+
static int nand_davinci_probe(struct platform_device *pdev)
{
struct davinci_nand_pdata *pdata;
@@ -658,7 +756,7 @@ static int nand_davinci_probe(struct platform_device *pdev)
return -EADDRNOTAVAIL;
}

- info->dev = &pdev->dev;
+ info->pdev = pdev;
info->base = base;
info->vaddr = vaddr;

@@ -708,97 +806,13 @@ static int nand_davinci_probe(struct platform_device *pdev)
spin_unlock_irq(&davinci_nand_lock);

/* Scan to find existence of the device(s) */
- ret = nand_scan_ident(mtd, pdata->mask_chipsel ? 2 : 1, NULL);
+ info->chip.dummy_controller.ops = &davinci_nand_controller_ops;
+ ret = nand_scan(mtd, pdata->mask_chipsel ? 2 : 1);
if (ret < 0) {
dev_dbg(&pdev->dev, "no NAND chip(s) found\n");
return ret;
}

- switch (info->chip.ecc.mode) {
- case NAND_ECC_NONE:
- pdata->ecc_bits = 0;
- break;
- case NAND_ECC_SOFT:
- pdata->ecc_bits = 0;
- /*
- * This driver expects Hamming based ECC when ecc_mode is set
- * to NAND_ECC_SOFT. Force ecc.algo to NAND_ECC_HAMMING to
- * avoid adding an extra ->ecc_algo field to
- * davinci_nand_pdata.
- */
- info->chip.ecc.algo = NAND_ECC_HAMMING;
- break;
- case NAND_ECC_HW:
- if (pdata->ecc_bits == 4) {
- /* No sanity checks: CPUs must support this,
- * and the chips may not use NAND_BUSWIDTH_16.
- */
-
- /* No sharing 4-bit hardware between chipselects yet */
- spin_lock_irq(&davinci_nand_lock);
- if (ecc4_busy)
- ret = -EBUSY;
- else
- ecc4_busy = true;
- spin_unlock_irq(&davinci_nand_lock);
-
- if (ret == -EBUSY)
- return ret;
-
- info->chip.ecc.calculate = nand_davinci_calculate_4bit;
- info->chip.ecc.correct = nand_davinci_correct_4bit;
- info->chip.ecc.hwctl = nand_davinci_hwctl_4bit;
- info->chip.ecc.bytes = 10;
- info->chip.ecc.options = NAND_ECC_GENERIC_ERASED_CHECK;
- info->chip.ecc.algo = NAND_ECC_BCH;
- } else {
- /* 1bit ecc hamming */
- info->chip.ecc.calculate = nand_davinci_calculate_1bit;
- info->chip.ecc.correct = nand_davinci_correct_1bit;
- info->chip.ecc.hwctl = nand_davinci_hwctl_1bit;
- info->chip.ecc.bytes = 3;
- info->chip.ecc.algo = NAND_ECC_HAMMING;
- }
- info->chip.ecc.size = 512;
- info->chip.ecc.strength = pdata->ecc_bits;
- break;
- default:
- return -EINVAL;
- }
-
- /* Update ECC layout if needed ... for 1-bit HW ECC, the default
- * is OK, but it allocates 6 bytes when only 3 are needed (for
- * each 512 bytes). For the 4-bit HW ECC, that default is not
- * usable: 10 bytes are needed, not 6.
- */
- if (pdata->ecc_bits == 4) {
- int chunks = mtd->writesize / 512;
-
- if (!chunks || mtd->oobsize < 16) {
- dev_dbg(&pdev->dev, "too small\n");
- ret = -EINVAL;
- goto err;
- }
-
- /* For small page chips, preserve the manufacturer's
- * badblock marking data ... and make sure a flash BBT
- * table marker fits in the free bytes.
- */
- if (chunks == 1) {
- mtd_set_ooblayout(mtd, &hwecc4_small_ooblayout_ops);
- } else if (chunks == 4 || chunks == 8) {
- mtd_set_ooblayout(mtd, &nand_ooblayout_lp_ops);
- info->chip.ecc.mode = NAND_ECC_HW_OOB_FIRST;
- } else {
- ret = -EIO;
- goto err;
- }
- }
-
- ret = nand_scan_tail(mtd);
- if (ret < 0)
- goto err;
-
if (pdata->parts)
ret = mtd_device_register(mtd, pdata->parts, pdata->nr_parts);
else
@@ -815,11 +829,6 @@ static int nand_davinci_probe(struct platform_device *pdev)
err_cleanup_nand:
nand_cleanup(&info->chip);

-err:
- spin_lock_irq(&davinci_nand_lock);
- if (info->chip.ecc.mode == NAND_ECC_HW_SYNDROME)
- ecc4_busy = false;
- spin_unlock_irq(&davinci_nand_lock);
return ret;
}

--
2.14.1

2018-07-19 23:05:08

by Miquel Raynal

[permalink] [raw]

Subject: [PATCH v3 11/33] mtd: rawnand: lpc32xx_mlc: convert driver to nand_scan()

2018-07-19 23:05:14

by Miquel Raynal

[permalink] [raw]

Subject: [PATCH v3 10/33] mtd: rawnand: jz4780: convert driver to nand_scan()

2018-07-19 23:05:29

by Miquel Raynal

[permalink] [raw]

Subject: [PATCH v3 05/33] mtd: rawnand: fsl_elbc: convert driver to nand_scan()

2018-07-19 23:05:32

by Miquel Raynal

[permalink] [raw]

Subject: [PATCH v3 06/33] mtd: rawnand: fsl_ifc: convert driver to nand_scan()

2018-07-19 23:05:46

by Miquel Raynal

[permalink] [raw]

Subject: [PATCH v3 01/33] mtd: rawnand: brcmnand: convert driver to nand_scan()

2018-07-19 23:18:21

by Boris Brezillon

[permalink] [raw]

Subject: Re: [PATCH v3 01/33] mtd: rawnand: brcmnand: convert driver to nand_scan()

On Fri, 20 Jul 2018 00:59:54 +0200
Miquel Raynal <[email protected]> wrote:

> +
> +static struct nand_controller_ops brcmnand_controller_ops = {

static const ...

The same comment applies to all patches of this series.

> + .attach_chip = brcmnand_attach_chip,
> +};

2018-07-19 23:29:10

by Boris Brezillon

[permalink] [raw]

Subject: Re: [PATCH v3 28/33] mtd: rawnand: docg4: convert driver to nand_scan()

On Fri, 20 Jul 2018 01:00:21 +0200
Miquel Raynal <[email protected]> wrote:

> Two helpers have been added to the core to make ECC-related
> configuration between the detection phase and the final NAND scan. Use
> these hooks and convert the driver to just use nand_scan() instead of
> both nand_scan_ident() and nand_scan_tail().
>
> Signed-off-by: Miquel Raynal <[email protected]>
> ---
> drivers/mtd/nand/raw/docg4.c | 55 ++++++++++++++++++++++++++------------------
> 1 file changed, 32 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/docg4.c b/drivers/mtd/nand/raw/docg4.c
> index 4dccdfba6140..2f6fcd4efab2 100644
> --- a/drivers/mtd/nand/raw/docg4.c
> +++ b/drivers/mtd/nand/raw/docg4.c
> @@ -1227,10 +1227,9 @@ static void __init init_mtd_structs(struct mtd_info *mtd)
> * required within a nand driver because they are performed by the nand
> * infrastructure code as part of nand_scan(). In this case they need
> * to be initialized here because we skip call to nand_scan_ident() (the
> - * first half of nand_scan()). The call to nand_scan_ident() is skipped
> - * because for this device the chip id is not read in the manner of a
> - * standard nand device. Unfortunately, nand_scan_ident() does other
> - * things as well, such as call nand_set_defaults().
> + * first half of nand_scan()). The call to nand_scan_ident() could be
> + * skipped because for this device the chip id is not read in the manner
> + * of a standard nand device.
> */
>
> struct nand_chip *nand = mtd_to_nand(mtd);
> @@ -1315,6 +1314,27 @@ static int __init read_id_reg(struct mtd_info *mtd)
>
> static char const *part_probes[] = { "cmdlinepart", "saftlpart", NULL };
>
> +static int docg4_attach_chip(struct nand_chip *chip)
> +{
> + struct mtd_info *mtd = nand_to_mtd(chip);
> + struct docg4_priv *doc = (struct docg4_priv *)(chip + 1);
> +
> + init_mtd_structs(mtd);
> +
> + /* Initialize kernel BCH algorithm */
> + doc->bch = init_bch(DOCG4_M, DOCG4_T, DOCG4_PRIMITIVE_POLY);
> + if (!doc->bch)
> + return -EINVAL;
> +
> + reset(mtd);
> +
> + return read_id_reg(mtd);
> +}
> +
> +static struct nand_controller_ops docg4_controller_ops = {
> + .attach_chip = docg4_attach_chip,
> +};
> +
> static int __init probe_docg4(struct platform_device *pdev)
> {
> struct mtd_info *mtd;
> @@ -1350,26 +1370,16 @@ static int __init probe_docg4(struct platform_device *pdev)
> mtd->dev.parent = &pdev->dev;
> doc->virtadr = virtadr;
> doc->dev = dev;
> -
> - init_mtd_structs(mtd);
> -
> - /* initialize kernel bch algorithm */
> - doc->bch = init_bch(DOCG4_M, DOCG4_T, DOCG4_PRIMITIVE_POLY);
> - if (doc->bch == NULL) {
> - retval = -EINVAL;
> - goto free_nand;
> - }
> -
> platform_set_drvdata(pdev, doc);
>
> - reset(mtd);
> - retval = read_id_reg(mtd);
> - if (retval == -ENODEV) {
> - dev_warn(dev, "No diskonchip G4 device found.\n");
> - goto free_bch;
> - }
> -
> - retval = nand_scan_tail(mtd);
> + /*
> + * Asking for 0 chips is useless here but it warns the user that the use
> + * of the nand_scan() function is a bit abused here because the
> + * initialization is actually a bit specific and re-handled again in the
> + * ->attach_chip() hook. It will probably leak some memory though.
> + */
> + nand->dummy_controller.ops = &docg4_controller_ops;
> + retval = nand_scan(mtd, 0);
> if (retval)
> goto free_bch;

Hm, not sure this works. The driver only calls nand_scan_tail(), but
you replace that by a call to nand_scan(), which will call both
nand_scan_ident() and nand_scan_tail(), and I'm pretty sure
nand_scan_ident() will fail here.

>
> @@ -1389,7 +1399,6 @@ static int __init probe_docg4(struct platform_device *pdev)
> nand_cleanup(nand);
> free_bch:
> free_bch(doc->bch);
> -free_nand:
> kfree(nand);
> unmap:
> iounmap(virtadr);

2018-07-19 23:43:35

by Boris Brezillon

[permalink] [raw]

Subject: Re: [PATCH v3 33/33] mtd: rawnand: allocate dynamically ONFI parameters during detection

On Fri, 20 Jul 2018 01:00:26 +0200
Miquel Raynal <[email protected]> wrote:

> Now that it is possible to do dynamic allocations during the
> identification phase, convert the onfi_params structure (which is only
> needed with ONFI compliant chips) into a pointer that will be allocated
> only if needed.
>
> Signed-off-by: Miquel Raynal <[email protected]>
> ---
> drivers/mtd/nand/raw/nand_base.c | 61 ++++++++++++++++++++++---------------
> drivers/mtd/nand/raw/nand_micron.c | 6 ++--
> drivers/mtd/nand/raw/nand_timings.c | 12 ++++----
> include/linux/mtd/rawnand.h | 6 ++--
> 4 files changed, 48 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index dfee2556a8a8..4dc248769abb 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -5151,6 +5151,8 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
> {
> struct mtd_info *mtd = nand_to_mtd(chip);
> struct nand_onfi_params *p;
> + struct onfi_params *onfi;
> + int onfi_version = 0;
> char *model;
> char id[4];
> int i, ret, val;
> @@ -5207,21 +5209,19 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
> /* Check version */
> val = le16_to_cpu(p->revision);
> if (val & ONFI_VERSION_2_3)
> - chip->parameters.onfi.version = 23;
> + onfi_version = 23;
> else if (val & ONFI_VERSION_2_2)
> - chip->parameters.onfi.version = 22;
> + onfi_version = 22;
> else if (val & ONFI_VERSION_2_1)
> - chip->parameters.onfi.version = 21;
> + onfi_version = 21;
> else if (val & ONFI_VERSION_2_0)
> - chip->parameters.onfi.version = 20;
> + onfi_version = 20;
> else if (val & ONFI_VERSION_1_0)
> - chip->parameters.onfi.version = 10;
> + onfi_version = 10;
>
> - if (!chip->parameters.onfi.version) {
> + if (!onfi_version) {
> pr_info("unsupported ONFI version: %d\n", val);
> goto free_onfi_param_page;
> - } else {
> - ret = 1;
> }
>
> sanitize_string(p->manufacturer, sizeof(p->manufacturer));
> @@ -5262,7 +5262,7 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
> if (p->ecc_bits != 0xff) {
> chip->ecc_strength_ds = p->ecc_bits;
> chip->ecc_step_ds = 512;
> - } else if (chip->parameters.onfi.version >= 21 &&
> + } else if (onfi_version >= 21 &&
> (le16_to_cpu(p->features) & ONFI_FEATURE_EXT_PARAM_PAGE)) {
>
> /*
> @@ -5289,19 +5289,32 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
> bitmap_set(chip->parameters.set_feature_list,
> ONFI_FEATURE_ADDR_TIMING_MODE, 1);
> }
> - chip->parameters.onfi.tPROG = le16_to_cpu(p->t_prog);
> - chip->parameters.onfi.tBERS = le16_to_cpu(p->t_bers);
> - chip->parameters.onfi.tR = le16_to_cpu(p->t_r);
> - chip->parameters.onfi.tCCS = le16_to_cpu(p->t_ccs);
> - chip->parameters.onfi.async_timing_mode =
> - le16_to_cpu(p->async_timing_mode);
> - chip->parameters.onfi.vendor_revision =
> - le16_to_cpu(p->vendor_revision);
> - memcpy(chip->parameters.onfi.vendor, p->vendor,
> - sizeof(p->vendor));
>
> + onfi = kzalloc(sizeof(*onfi), GFP_KERNEL);
> + if (!onfi) {
> + ret = -ENOMEM;
> + goto free_model;
> + }
> + onfi->version = onfi_version;
> + onfi->tPROG = le16_to_cpu(p->t_prog);
> + onfi->tBERS = le16_to_cpu(p->t_bers);
> + onfi->tR = le16_to_cpu(p->t_r);
> + onfi->tCCS = le16_to_cpu(p->t_ccs);
> + onfi->async_timing_mode = le16_to_cpu(p->async_timing_mode);
> + onfi->vendor_revision = le16_to_cpu(p->vendor_revision);
> + memcpy(onfi->vendor, p->vendor, sizeof(p->vendor));
> + chip->parameters.onfi = onfi;
> +
> + /* Identification done, free the full ONFI parameter page and exit */
> + kfree(p);
> +
> + return 1;
> +
> +free_model:
> + kfree(model);
> free_onfi_param_page:
> kfree(p);
> +
> return ret;
> }
>
> @@ -5562,9 +5575,7 @@ static bool find_full_id_nand(struct nand_chip *chip,
> chip->ecc_step_ds = NAND_ECC_STEP(type);
> chip->onfi_timing_mode_default =
> type->onfi_timing_mode_default;
> -
> - strncpy(chip->parameters.model, type->name,
> - sizeof(chip->parameters.model) - 1);
> + chip->parameters.model = type->name;
>
> return true;
> }
> @@ -5703,7 +5714,6 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
> }
> }
>
> - chip->parameters.onfi.version = 0;
> if (!type->name || !type->pagesize) {
> /* Check if the chip is ONFI compliant */
> ret = nand_flash_detect_onfi(chip);
> @@ -5723,8 +5733,7 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
> if (!type->name)
> return -ENODEV;
>
> - strncpy(chip->parameters.model, type->name,
> - sizeof(chip->parameters.model) - 1);

Okay, so you have regression in patch 32, because
chip->parameters.model has been turned into a const char *, but you
keep copying things into this pointer without allocating the buffer
before.

> + chip->parameters.model = type->name;

Uh, that's wrong. You free chip->parameters.model in the cleanup, but
you do not allocate it here. You should use kstrdup(type->name) here.

>
> chip->chipsize = (uint64_t)type->chipsize << 20;
>
> @@ -6033,6 +6042,8 @@ static int nand_scan_ident(struct mtd_info *mtd, int maxchips,
> static void nand_scan_ident_cleanup(struct nand_chip *chip)
> {
> kfree(chip->parameters.model);
> + kfree(chip->parameters.onfi);
> +
> }
>
> static int nand_set_ecc_soft_ops(struct mtd_info *mtd)
> diff --git a/drivers/mtd/nand/raw/nand_micron.c b/drivers/mtd/nand/raw/nand_micron.c
> index 656947d91841..8466a1740b3b 100644
> --- a/drivers/mtd/nand/raw/nand_micron.c
> +++ b/drivers/mtd/nand/raw/nand_micron.c
> @@ -88,9 +88,9 @@ static int micron_nand_setup_read_retry(struct mtd_info *mtd, int retry_mode)
> static int micron_nand_onfi_init(struct nand_chip *chip)
> {
> struct nand_parameters *p = &chip->parameters;
> - struct nand_onfi_vendor_micron *micron = (void *)p->onfi.vendor;
> + struct nand_onfi_vendor_micron *micron = (void *)p->onfi->vendor;
>
> - if (chip->parameters.onfi.version && p->onfi.vendor_revision) {
> + if (p->onfi && p->onfi->vendor_revision) {
> chip->read_retries = micron->read_retry_options;
> chip->setup_read_retry = micron_nand_setup_read_retry;
> }
> @@ -382,7 +382,7 @@ static int micron_supports_on_die_ecc(struct nand_chip *chip)
> u8 id[5];
> int ret;
>
> - if (!chip->parameters.onfi.version)
> + if (!chip->parameters.onfi)
> return MICRON_ON_DIE_UNSUPPORTED;
>
> if (chip->bits_per_cell != 1)
> diff --git a/drivers/mtd/nand/raw/nand_timings.c b/drivers/mtd/nand/raw/nand_timings.c
> index 9bb599106a31..ebc7b5f76f77 100644
> --- a/drivers/mtd/nand/raw/nand_timings.c
> +++ b/drivers/mtd/nand/raw/nand_timings.c
> @@ -294,6 +294,7 @@ int onfi_fill_data_interface(struct nand_chip *chip,
> int timing_mode)
> {
> struct nand_data_interface *iface = &chip->data_interface;
> + struct onfi_params *onfi = chip->parameters.onfi;
>
> if (type != NAND_SDR_IFACE)
> return -EINVAL;
> @@ -308,17 +309,16 @@ int onfi_fill_data_interface(struct nand_chip *chip,
> * tPROG, tBERS, tR and tCCS.
> * These information are part of the ONFI parameter page.
> */
> - if (chip->parameters.onfi.version) {
> - struct nand_parameters *params = &chip->parameters;
> + if (onfi) {
> struct nand_sdr_timings *timings = &iface->timings.sdr;
>
> /* microseconds -> picoseconds */
> - timings->tPROG_max = 1000000ULL * params->onfi.tPROG;
> - timings->tBERS_max = 1000000ULL * params->onfi.tBERS;
> - timings->tR_max = 1000000ULL * params->onfi.tR;
> + timings->tPROG_max = 1000000ULL * onfi->tPROG;
> + timings->tBERS_max = 1000000ULL * onfi->tBERS;
> + timings->tR_max = 1000000ULL * onfi->tR;
>
> /* nanoseconds -> picoseconds */
> - timings->tCCS_min = 1000UL * params->onfi.tCCS;
> + timings->tCCS_min = 1000UL * onfi->tCCS;
> } else {
> struct nand_sdr_timings *timings = &iface->timings.sdr;
> /*
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index 5723d940a47d..8074cbd4e3fe 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -488,7 +488,7 @@ struct nand_parameters {
> DECLARE_BITMAP(get_feature_list, ONFI_FEATURE_NUMBER);
>
> /* ONFI parameters */
> - struct onfi_params onfi;
> + struct onfi_params *onfi;
> };
>
> /* The maximum expected count of bytes in the NAND ID sequence */
> @@ -1618,10 +1618,10 @@ struct platform_nand_data {
> /* return the supported asynchronous timing mode. */
> static inline int onfi_get_async_timing_mode(struct nand_chip *chip)
> {
> - if (!chip->parameters.onfi.version)
> + if (!chip->parameters.onfi)
> return ONFI_TIMING_MODE_UNKNOWN;
>
> - return chip->parameters.onfi.async_timing_mode;
> + return chip->parameters.onfi->async_timing_mode;
> }
>
> int onfi_fill_data_interface(struct nand_chip *chip,

2018-07-19 23:46:42

by Boris Brezillon

[permalink] [raw]

Subject: Re: [PATCH v3 32/33] mtd: rawnand: do not export nand_scan_[ident|tail]() anymore

On Fri, 20 Jul 2018 01:00:25 +0200
Miquel Raynal <[email protected]> wrote:

> Both nand_scan_ident() and nand_scan_tail() helpers used to be called
> directly from controller drivers that needed to tweak some ECC-related
> parameters before nand_scan_tail(). This separation prevented dynamic
> allocations during the phase of NAND identification, which was
> inconvenient.
>
> All controller drivers have been moved to use nand_scan(), in
> conjunction with the chip->ecc.[attach|detach]_chip() hooks that
> actually do the required tweaking sequence between both ident/tail
> calls, allowing programmers to use dynamic allocation as they need all
> across the scanning sequence.
>
> Signed-off-by: Miquel Raynal <[email protected]>
> ---
> drivers/mtd/nand/raw/nand_base.c | 60 +++++++++++++++++++++++++++++++---------
> include/linux/mtd/rawnand.h | 11 ++------
> 2 files changed, 50 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index dea41fa25be1..dfee2556a8a8 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -5151,6 +5151,7 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
> {
> struct mtd_info *mtd = nand_to_mtd(chip);
> struct nand_onfi_params *p;
> + char *model;
> char id[4];
> int i, ret, val;
>
> @@ -5225,8 +5226,15 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
>
> sanitize_string(p->manufacturer, sizeof(p->manufacturer));
> sanitize_string(p->model, sizeof(p->model));
> - strncpy(chip->parameters.model, p->model,
> - sizeof(chip->parameters.model) - 1);
> + model = kmemdup(p->model, sizeof(p->model), GFP_KERNEL);
> + if (!model) {
> + ret = -ENOMEM;
> + goto free_onfi_param_page;
> + }
> +
> + chip->parameters.model = model;
> + if (!mtd->name)
> + mtd->name = chip->parameters.model;
>
> mtd->writesize = le32_to_cpu(p->byte_per_page);
>
> @@ -5306,6 +5314,7 @@ static int nand_flash_detect_jedec(struct nand_chip *chip)
> struct nand_jedec_params *p;
> struct jedec_ecc_info *ecc;
> int jedec_version = 0;
> + char *model;
> char id[5];
> int i, val, ret;
>
> @@ -5356,8 +5365,16 @@ static int nand_flash_detect_jedec(struct nand_chip *chip)
>
> sanitize_string(p->manufacturer, sizeof(p->manufacturer));
> sanitize_string(p->model, sizeof(p->model));
> - strncpy(chip->parameters.model, p->model,
> - sizeof(chip->parameters.model) - 1);
> + model = kzalloc(sizeof(p->model), GFP_KERNEL);
> + if (!p) {
> + ret = -ENOMEM;
> + goto free_jedec_param_page;
> + }
> +
> + strncpy(model, p->model, sizeof(model) - 1);
> + chip->parameters.model = model;
> + if (!mtd->name)
> + mtd->name = chip->parameters.model;

All changes related to the the char model[] -> const char *mode
transition should be done in a separate commit. Plus, as pointed in
patch 33, you have a regression here.

>
> mtd->writesize = le32_to_cpu(p->byte_per_page);
>
> @@ -5924,7 +5941,7 @@ static int nand_dt_init(struct nand_chip *chip)
> }
>
> /**
> - * nand_scan_ident - [NAND Interface] Scan for the NAND device
> + * nand_scan_ident - Scan for the NAND device
> * @mtd: MTD device structure
> * @maxchips: number of chips to scan for
> * @table: alternative NAND ID table
> @@ -5932,9 +5949,13 @@ static int nand_dt_init(struct nand_chip *chip)
> * This is the first phase of the normal nand_scan() function. It reads the
> * flash ID and sets up MTD fields accordingly.
> *
> + * This helper used to be called directly from controller drivers that needed
> + * to tweak some ECC-related parameters before nand_scan_tail(). This separation
> + * prevented dynamic allocations during this phase which was unconvenient and
> + * as been banned for the benefit of the ->init_ecc()/cleanup_ecc() hooks.
> */
> -int nand_scan_ident(struct mtd_info *mtd, int maxchips,
> - struct nand_flash_dev *table)
> +static int nand_scan_ident(struct mtd_info *mtd, int maxchips,
> + struct nand_flash_dev *table)
> {
> int i, nand_maf_id, nand_dev_id;
> struct nand_chip *chip = mtd_to_nand(mtd);
> @@ -6008,7 +6029,11 @@ int nand_scan_ident(struct mtd_info *mtd, int maxchips,
>
> return 0;
> }
> -EXPORT_SYMBOL(nand_scan_ident);
> +
> +static void nand_scan_ident_cleanup(struct nand_chip *chip)
> +{
> + kfree(chip->parameters.model);
> +}
>
> static int nand_set_ecc_soft_ops(struct mtd_info *mtd)
> {
> @@ -6385,14 +6410,14 @@ static bool nand_ecc_strength_good(struct mtd_info *mtd)
> }
>
> /**
> - * nand_scan_tail - [NAND Interface] Scan for the NAND device
> + * nand_scan_tail - Scan for the NAND device
> * @mtd: MTD device structure
> *
> * This is the second phase of the normal nand_scan() function. It fills out
> * all the uninitialized function pointers with the defaults and scans for a
> * bad block table if appropriate.
> */
> -int nand_scan_tail(struct mtd_info *mtd)
> +static int nand_scan_tail(struct mtd_info *mtd)
> {
> struct nand_chip *chip = mtd_to_nand(mtd);
> struct nand_ecc_ctrl *ecc = &chip->ecc;
> @@ -6716,7 +6741,6 @@ int nand_scan_tail(struct mtd_info *mtd)
>
> return ret;
> }
> -EXPORT_SYMBOL(nand_scan_tail);
>
> static int nand_attach(struct nand_chip *chip)
> {
> @@ -6754,11 +6778,18 @@ int nand_scan_with_ids(struct mtd_info *mtd, int maxchips,
>
> ret = nand_attach(chip);
> if (ret)
> - return ret;
> + goto cleanup_ident;
>
> ret = nand_scan_tail(mtd);
> if (ret)
> - nand_detach(chip);
> + goto detach_chip;
> +
> + return 0;
> +
> +detach_chip:
> + nand_detach(chip);
> +cleanup_ident:
> + nand_scan_ident_cleanup(chip);
>
> return ret;
> }
> @@ -6790,6 +6821,9 @@ void nand_cleanup(struct nand_chip *chip)
>
> /* Free controller specific allocations after chip identification */
> nand_detach(chip);
> +
> + /* Free identification phase allocations */
> + nand_scan_ident_cleanup(chip);
> }
>
> EXPORT_SYMBOL_GPL(nand_cleanup);
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index a20c78e25878..5723d940a47d 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -36,14 +36,9 @@ static inline int nand_scan(struct mtd_info *mtd, int max_chips)
> }
>
> /*
> - * Separate phases of nand_scan(), allowing board driver to intervene
> - * and override command or ECC setup according to flash type.
> + * Unregister the MTD device and free resources held by the NAND device, must be
> + * called on error after a successful nand_scan().
> */
> -int nand_scan_ident(struct mtd_info *mtd, int max_chips,
> - struct nand_flash_dev *table);
> -int nand_scan_tail(struct mtd_info *mtd);
> -
> -/* Unregister the MTD device and free resources held by the NAND device */
> void nand_release(struct mtd_info *mtd);
>
> /* Internal helper for board drivers which need to override command function */
> @@ -487,7 +482,7 @@ struct onfi_params {
> */
> struct nand_parameters {
> /* Generic parameters */
> - char model[100];
> + const char *model;
> bool supports_set_get_features;
> DECLARE_BITMAP(set_feature_list, ONFI_FEATURE_NUMBER);
> DECLARE_BITMAP(get_feature_list, ONFI_FEATURE_NUMBER);

2018-07-20 07:12:59

by Miquel Raynal

[permalink] [raw]

Subject: Re: [PATCH v3 01/33] mtd: rawnand: brcmnand: convert driver to nand_scan()

Hi Boris,

Boris Brezillon <[email protected]> wrote on Fri, 20 Jul 2018
01:17:19 +0200:

> On Fri, 20 Jul 2018 00:59:54 +0200
> Miquel Raynal <[email protected]> wrote:
>
> > +
> > +static struct nand_controller_ops brcmnand_controller_ops = {
>
> static const ...

Mmmh right, I'll constify every controller_ops definition.

>
> The same comment applies to all patches of this series.
>
> > + .attach_chip = brcmnand_attach_chip,
> > +};
>

Miquèl

2018-07-20 07:18:46

by Miquel Raynal

[permalink] [raw]

Subject: Re: [PATCH v3 28/33] mtd: rawnand: docg4: convert driver to nand_scan()

Hi Boris,

Boris Brezillon <[email protected]> wrote on Fri, 20 Jul 2018
01:27:32 +0200:

> On Fri, 20 Jul 2018 01:00:21 +0200
> Miquel Raynal <[email protected]> wrote:
>
> > Two helpers have been added to the core to make ECC-related
> > configuration between the detection phase and the final NAND scan. Use
> > these hooks and convert the driver to just use nand_scan() instead of
> > both nand_scan_ident() and nand_scan_tail().
> >
> > Signed-off-by: Miquel Raynal <[email protected]>
> > ---
> > drivers/mtd/nand/raw/docg4.c | 55 ++++++++++++++++++++++++++------------------
> > 1 file changed, 32 insertions(+), 23 deletions(-)
> >
> > diff --git a/drivers/mtd/nand/raw/docg4.c b/drivers/mtd/nand/raw/docg4.c
> > index 4dccdfba6140..2f6fcd4efab2 100644
> > --- a/drivers/mtd/nand/raw/docg4.c
> > +++ b/drivers/mtd/nand/raw/docg4.c
> > @@ -1227,10 +1227,9 @@ static void __init init_mtd_structs(struct mtd_info *mtd)
> > * required within a nand driver because they are performed by the nand
> > * infrastructure code as part of nand_scan(). In this case they need
> > * to be initialized here because we skip call to nand_scan_ident() (the
> > - * first half of nand_scan()). The call to nand_scan_ident() is skipped
> > - * because for this device the chip id is not read in the manner of a
> > - * standard nand device. Unfortunately, nand_scan_ident() does other
> > - * things as well, such as call nand_set_defaults().
> > + * first half of nand_scan()). The call to nand_scan_ident() could be
> > + * skipped because for this device the chip id is not read in the manner
> > + * of a standard nand device.
> > */
> >
> > struct nand_chip *nand = mtd_to_nand(mtd);
> > @@ -1315,6 +1314,27 @@ static int __init read_id_reg(struct mtd_info *mtd)
> >
> > static char const *part_probes[] = { "cmdlinepart", "saftlpart", NULL };
> >
> > +static int docg4_attach_chip(struct nand_chip *chip)
> > +{
> > + struct mtd_info *mtd = nand_to_mtd(chip);
> > + struct docg4_priv *doc = (struct docg4_priv *)(chip + 1);
> > +
> > + init_mtd_structs(mtd);
> > +
> > + /* Initialize kernel BCH algorithm */
> > + doc->bch = init_bch(DOCG4_M, DOCG4_T, DOCG4_PRIMITIVE_POLY);
> > + if (!doc->bch)
> > + return -EINVAL;
> > +
> > + reset(mtd);
> > +
> > + return read_id_reg(mtd);
> > +}
> > +
> > +static struct nand_controller_ops docg4_controller_ops = {
> > + .attach_chip = docg4_attach_chip,
> > +};
> > +
> > static int __init probe_docg4(struct platform_device *pdev)
> > {
> > struct mtd_info *mtd;
> > @@ -1350,26 +1370,16 @@ static int __init probe_docg4(struct platform_device *pdev)
> > mtd->dev.parent = &pdev->dev;
> > doc->virtadr = virtadr;
> > doc->dev = dev;
> > -
> > - init_mtd_structs(mtd);
> > -
> > - /* initialize kernel bch algorithm */
> > - doc->bch = init_bch(DOCG4_M, DOCG4_T, DOCG4_PRIMITIVE_POLY);
> > - if (doc->bch == NULL) {
> > - retval = -EINVAL;
> > - goto free_nand;
> > - }
> > -
> > platform_set_drvdata(pdev, doc);
> >
> > - reset(mtd);
> > - retval = read_id_reg(mtd);
> > - if (retval == -ENODEV) {
> > - dev_warn(dev, "No diskonchip G4 device found.\n");
> > - goto free_bch;
> > - }
> > -
> > - retval = nand_scan_tail(mtd);
> > + /*
> > + * Asking for 0 chips is useless here but it warns the user that the use
> > + * of the nand_scan() function is a bit abused here because the
> > + * initialization is actually a bit specific and re-handled again in the
> > + * ->attach_chip() hook. It will probably leak some memory though.
> > + */
> > + nand->dummy_controller.ops = &docg4_controller_ops;
> > + retval = nand_scan(mtd, 0);
> > if (retval)
> > goto free_bch;
>
> Hm, not sure this works. The driver only calls nand_scan_tail(), but
> you replace that by a call to nand_scan(), which will call both
> nand_scan_ident() and nand_scan_tail(), and I'm pretty sure
> nand_scan_ident() will fail here.

I know docg4 is a bit specific and could maybe be moved out of the raw/
subdirectory. But in the meantime I don't want to block the series for
this. The better I can propose right now (open to other ideas as
well) would be to return 0 in nand_scan_ident() if the maxchip parameter
is 0 which is the case only in this driver AFAIS.

Miquèl

2018-07-20 07:36:32

by Boris Brezillon

[permalink] [raw]

Subject: Re: [PATCH v3 28/33] mtd: rawnand: docg4: convert driver to nand_scan()

On Fri, 20 Jul 2018 09:17:45 +0200
Miquel Raynal <[email protected]> wrote:

> Hi Boris,
>
> Boris Brezillon <[email protected]> wrote on Fri, 20 Jul 2018
> 01:27:32 +0200:
>
> > On Fri, 20 Jul 2018 01:00:21 +0200
> > Miquel Raynal <[email protected]> wrote:
> >
> > > Two helpers have been added to the core to make ECC-related
> > > configuration between the detection phase and the final NAND scan. Use
> > > these hooks and convert the driver to just use nand_scan() instead of
> > > both nand_scan_ident() and nand_scan_tail().
> > >
> > > Signed-off-by: Miquel Raynal <[email protected]>
> > > ---
> > > drivers/mtd/nand/raw/docg4.c | 55 ++++++++++++++++++++++++++------------------
> > > 1 file changed, 32 insertions(+), 23 deletions(-)
> > >
> > > diff --git a/drivers/mtd/nand/raw/docg4.c b/drivers/mtd/nand/raw/docg4.c
> > > index 4dccdfba6140..2f6fcd4efab2 100644
> > > --- a/drivers/mtd/nand/raw/docg4.c
> > > +++ b/drivers/mtd/nand/raw/docg4.c
> > > @@ -1227,10 +1227,9 @@ static void __init init_mtd_structs(struct mtd_info *mtd)
> > > * required within a nand driver because they are performed by the nand
> > > * infrastructure code as part of nand_scan(). In this case they need
> > > * to be initialized here because we skip call to nand_scan_ident() (the
> > > - * first half of nand_scan()). The call to nand_scan_ident() is skipped
> > > - * because for this device the chip id is not read in the manner of a
> > > - * standard nand device. Unfortunately, nand_scan_ident() does other
> > > - * things as well, such as call nand_set_defaults().
> > > + * first half of nand_scan()). The call to nand_scan_ident() could be
> > > + * skipped because for this device the chip id is not read in the manner
> > > + * of a standard nand device.
> > > */
> > >
> > > struct nand_chip *nand = mtd_to_nand(mtd);
> > > @@ -1315,6 +1314,27 @@ static int __init read_id_reg(struct mtd_info *mtd)
> > >
> > > static char const *part_probes[] = { "cmdlinepart", "saftlpart", NULL };
> > >
> > > +static int docg4_attach_chip(struct nand_chip *chip)
> > > +{
> > > + struct mtd_info *mtd = nand_to_mtd(chip);
> > > + struct docg4_priv *doc = (struct docg4_priv *)(chip + 1);
> > > +
> > > + init_mtd_structs(mtd);
> > > +
> > > + /* Initialize kernel BCH algorithm */
> > > + doc->bch = init_bch(DOCG4_M, DOCG4_T, DOCG4_PRIMITIVE_POLY);
> > > + if (!doc->bch)
> > > + return -EINVAL;
> > > +
> > > + reset(mtd);
> > > +
> > > + return read_id_reg(mtd);
> > > +}
> > > +
> > > +static struct nand_controller_ops docg4_controller_ops = {
> > > + .attach_chip = docg4_attach_chip,
> > > +};
> > > +
> > > static int __init probe_docg4(struct platform_device *pdev)
> > > {
> > > struct mtd_info *mtd;
> > > @@ -1350,26 +1370,16 @@ static int __init probe_docg4(struct platform_device *pdev)
> > > mtd->dev.parent = &pdev->dev;
> > > doc->virtadr = virtadr;
> > > doc->dev = dev;
> > > -
> > > - init_mtd_structs(mtd);
> > > -
> > > - /* initialize kernel bch algorithm */
> > > - doc->bch = init_bch(DOCG4_M, DOCG4_T, DOCG4_PRIMITIVE_POLY);
> > > - if (doc->bch == NULL) {
> > > - retval = -EINVAL;
> > > - goto free_nand;
> > > - }
> > > -
> > > platform_set_drvdata(pdev, doc);
> > >
> > > - reset(mtd);
> > > - retval = read_id_reg(mtd);
> > > - if (retval == -ENODEV) {
> > > - dev_warn(dev, "No diskonchip G4 device found.\n");
> > > - goto free_bch;
> > > - }
> > > -
> > > - retval = nand_scan_tail(mtd);
> > > + /*
> > > + * Asking for 0 chips is useless here but it warns the user that the use
> > > + * of the nand_scan() function is a bit abused here because the
> > > + * initialization is actually a bit specific and re-handled again in the
> > > + * ->attach_chip() hook. It will probably leak some memory though.
> > > + */
> > > + nand->dummy_controller.ops = &docg4_controller_ops;
> > > + retval = nand_scan(mtd, 0);
> > > if (retval)
> > > goto free_bch;
> >
> > Hm, not sure this works. The driver only calls nand_scan_tail(), but
> > you replace that by a call to nand_scan(), which will call both
> > nand_scan_ident() and nand_scan_tail(), and I'm pretty sure
> > nand_scan_ident() will fail here.
>
> I know docg4 is a bit specific and could maybe be moved out of the raw/
> subdirectory. But in the meantime I don't want to block the series for
> this. The better I can propose right now (open to other ideas as
> well) would be to return 0 in nand_scan_ident() if the maxchip parameter
> is 0 which is the case only in this driver AFAIS.

Sounds good. Just document this particular case in nand_scan_ident()
kernel-doc header.