From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Brezillon Subject: Re: [RFC 02/12] mtd: nand: force drivers to explicitly send READ/PROG commands Date: Fri, 20 Oct 2017 13:18:34 +0200 Message-ID: <20171020131834.704e2adc@bbrezillon> References: <20171018143629.29302-1-miquel.raynal@free-electrons.com> <20171018143629.29302-3-miquel.raynal@free-electrons.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: driverdev-devel-bounces@linuxdriverproject.org Sender: "devel" To: Stefan Agner Cc: Mark Rutland , Andrew Lunn , Catalin Marinas , Hanna Hawa , Will Deacon , Nadav Haklai , Masahiro Yamada , linux-mtd@lists.infradead.org, Matthias Brugger , Robert Jarzmik , devel@driverdev.osuosl.org, Maxim Levitsky , Kamal Dasu , Josh Wu , Russell King , Marek Vasut , Chen-Yu Tsai , bcm-kernel-feedback-list@broadcom.com, Sebastian Hesselbarth , Ezequiel Garcia , Sylvain Lemieux , Marc Gonzalez , devicetree@vger.ker List-Id: devicetree@vger.kernel.org On Fri, 20 Oct 2017 11:29:18 +0200 Stefan Agner wrote: > Hi Miquel, > > Thanks for the work on this, happy to see the new interface is moving > forward. Some comments below. > > On 2017-10-18 16:36, Miquel Raynal wrote: > > From: Boris Brezillon > > > > The core currently send the READ0 and SEQIN+PAGEPROG commands in > > nand_do_read/write_ops(). This is inconsistent with > > ->read/write_oob[_raw]() hooks behavior which are expected to send > > these commands. > > > > There's already a flag (NAND_ECC_CUSTOM_PAGE_ACCESS) to inform the core > > that a specific controller wants to send the READ/SEQIN+PAGEPROG > > commands on its own, but it's an opt-in flag, and existing drivers are > > unlikely to be updated to pass it. > > > > Moreover, some controllers cannot dissociate the READ/PAGEPROG commands > > from the associated data transfer and ECC engine activation, and > > developers have to hack things in their ->cmdfunc() implementation to > > handle such complex cases, or have to accept the perf penalty of sending > > twice the same command. > > To address this problem we are planning on adding a new interface which > > is passed all information about a NAND operation (including the amount > > of data to transfer) and replacing all calls to ->cmdfunc() to calls to > > this new ->exec_op() hook. But, in order to do that, we need to have all > > ->cmdfunc() calls placed near their associated ->read/write_buf/byte() > > calls. > > > > Modify the core and relevant drivers to make NAND_ECC_CUSTOM_PAGE_ACCESS > > the default case, and remove this flag. > > > > Signed-off-by: Boris Brezillon > > [miquel.raynal@free-electrons.com: rebased and fixed some conflicts] > > Signed-off-by: Miquel Raynal > > --- > > drivers/mtd/nand/atmel/nand-controller.c | 7 ++- > > drivers/mtd/nand/bf5xx_nand.c | 6 ++- > > drivers/mtd/nand/brcmnand/brcmnand.c | 13 +++-- > > drivers/mtd/nand/cafe_nand.c | 6 +-- > > drivers/mtd/nand/denali.c | 14 +++++- > > drivers/mtd/nand/docg4.c | 12 +++-- > > drivers/mtd/nand/fsl_elbc_nand.c | 6 +-- > > drivers/mtd/nand/fsl_ifc_nand.c | 6 +-- > > drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 30 ++++++------ > > drivers/mtd/nand/hisi504_nand.c | 6 +-- > > drivers/mtd/nand/lpc32xx_mlc.c | 5 +- > > drivers/mtd/nand/lpc32xx_slc.c | 11 +++-- > > drivers/mtd/nand/mtk_nand.c | 10 ++-- > > drivers/mtd/nand/nand_base.c | 68 +++++++++------------------ > > drivers/mtd/nand/nand_micron.c | 1 - > > drivers/mtd/nand/sh_flctl.c | 6 +-- > > drivers/mtd/nand/sunxi_nand.c | 34 +++++++++----- > > drivers/mtd/nand/tango_nand.c | 1 - > > drivers/mtd/nand/vf610_nfc.c | 6 +-- > > drivers/staging/mt29f_spinand/mt29f_spinand.c | 7 ++- > > include/linux/mtd/rawnand.h | 11 ----- > > 21 files changed, 142 insertions(+), 124 deletions(-) > > > > > > > diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c > > b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c > > index c8ceaecd8065..3f2b903158c1 100644 > > --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c > > +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c > > @@ -1043,6 +1043,8 @@ static int gpmi_ecc_read_page(struct mtd_info > > *mtd, struct nand_chip *chip, > > unsigned int max_bitflips = 0; > > int ret; > > > > + nand_read_page_op(chip, page, 0, NULL, 0); > > + > > dev_dbg(this->dev, "page number is : %d\n", page); > > ret = read_page_prepare(this, buf, nfc_geo->payload_size, > > this->payload_virt, this->payload_phys, > > @@ -1220,12 +1222,13 @@ static int gpmi_ecc_read_subpage(struct > > mtd_info *mtd, struct nand_chip *chip, > > meta = geo->metadata_size; > > if (first) { > > col = meta + (size + ecc_parity_size) * first; > > - chip->cmdfunc(mtd, NAND_CMD_RNDOUT, col, -1); > > Shouldn't we add nand_change_read_column_op here? > > > > > > meta = 0; > > buf = buf + first * size; > > } > > > > + nand_read_page_op(chip, page, col, NULL, 0); > > + > > /* Save the old environment */ > > r1_old = r1_new = readl(bch_regs + HW_BCH_FLASH0LAYOUT0); > > r2_old = r2_new = readl(bch_regs + HW_BCH_FLASH0LAYOUT1); > > @@ -1277,6 +1280,9 @@ static int gpmi_ecc_write_page(struct mtd_info > > *mtd, struct nand_chip *chip, > > int ret; > > > > dev_dbg(this->dev, "ecc write page.\n"); > > + > > + nand_prog_page_begin_op(chip, page, 0, NULL, 0); > > + > > if (this->swap_block_mark) { > > /* > > * If control arrives here, we're doing block mark swapping. > > @@ -1338,7 +1344,10 @@ static int gpmi_ecc_write_page(struct mtd_info > > *mtd, struct nand_chip *chip, > > payload_virt, payload_phys); > > } > > > > - return 0; > > + if (ret) > > + return ret; > > + > > + return nand_prog_page_end_op(chip); > > } > > > > /* > > @@ -1472,8 +1481,8 @@ static int gpmi_ecc_read_page_raw(struct mtd_info *mtd, > > uint8_t *oob = chip->oob_poi; > > int step; > > > > - chip->read_buf(mtd, tmp_buf, > > - mtd->writesize + mtd->oobsize); > > + nand_read_page_op(chip, page, 0, tmp_buf, > > + mtd->writesize + mtd->oobsize); > > > > /* > > * If required, swap the bad block marker and the data stored in the > > @@ -1617,24 +1626,19 @@ static int gpmi_ecc_write_page_raw(struct mtd_info *mtd, > > tmp_buf[mtd->writesize] = swap; > > } > > > > - chip->write_buf(mtd, tmp_buf, mtd->writesize + mtd->oobsize); > > - > > - return 0; > > + return nand_prog_page_op(chip, page, 0, tmp_buf, > > + mtd->writesize + mtd->oobsize); > > } > > > > static int gpmi_ecc_read_oob_raw(struct mtd_info *mtd, struct nand_chip *chip, > > int page) > > { > > - nand_read_page_op(chip, page, 0, NULL, 0); > > - > > return gpmi_ecc_read_page_raw(mtd, chip, NULL, 1, page); > > } > > > > static int gpmi_ecc_write_oob_raw(struct mtd_info *mtd, struct nand_chip *chip, > > int page) > > { > > - nand_prog_page_begin_op(chip, page, 0, NULL, 0); > > - > > return gpmi_ecc_write_page_raw(mtd, chip, NULL, 1, page); > > } > > > > @@ -1806,9 +1810,7 @@ static int mx23_write_transcription_stamp(struct > > gpmi_nand_data *this) > > /* Write the first page of the current stride. */ > > dev_dbg(dev, "Writing an NCB fingerprint in page 0x%x\n", page); > > > > - nand_prog_page_begin_op(chip, page, 0, NULL, 0); > > - chip->ecc.write_page_raw(mtd, chip, buffer, 0, page); > > - status = nand_prog_page_end_op(chip); > > + status = chip->ecc.write_page_raw(mtd, chip, buffer, 0, page); > > if (status) > > dev_err(dev, "[%s] Write failed.\n", __func__); > > } > > > > > diff --git a/drivers/mtd/nand/vf610_nfc.c b/drivers/mtd/nand/vf610_nfc.c > > index 8037d4b48a05..80d31a58e558 100644 > > --- a/drivers/mtd/nand/vf610_nfc.c > > +++ b/drivers/mtd/nand/vf610_nfc.c > > @@ -560,7 +560,7 @@ static int vf610_nfc_read_page(struct mtd_info > > *mtd, struct nand_chip *chip, > > int eccsize = chip->ecc.size; > > int stat; > > > > - vf610_nfc_read_buf(mtd, buf, eccsize); > > + nand_read_page_op(chip, page, 0, buf, eccsize); > > if (oob_required) > > vf610_nfc_read_buf(mtd, chip->oob_poi, mtd->oobsize); > > > > @@ -580,7 +580,7 @@ static int vf610_nfc_write_page(struct mtd_info > > *mtd, struct nand_chip *chip, > > { > > struct vf610_nfc *nfc = mtd_to_nfc(mtd); > > > > - vf610_nfc_write_buf(mtd, buf, mtd->writesize); > > We currently ignore NAND_CMD_SEQIN in ->cmdfunc anyway, so I think this > change would not even be necessary here. Well, the thing is, we're trying to keep the logic unchanged. Even if for your driver ->cmdfunc(NAND_CMD_SEQIN) is a NOP, I'd prefer to be consistent and still replace this ->write_buf() call by a nand_prog_page_begin_op() one. > > This is a NAND controller which will benefit from the new interface. I > plan to convert the driver to the new interface, maybe I manage to do > that soon so we have a second driver making use of the new interface. Great!!! Let us know if you need any help. Regards, Boris From mboxrd@z Thu Jan 1 00:00:00 1970 Date: Fri, 20 Oct 2017 13:18:34 +0200 From: Boris Brezillon To: Stefan Agner Cc: Miquel Raynal , Andrew Lunn , bcm-kernel-feedback-list@broadcom.com, Brian Norris , Catalin Marinas , Chen-Yu Tsai , Cyrille Pitchen , Daniel Mack , David Woodhouse , devel@driverdev.osuosl.org, devicetree@vger.kernel.org, Ezequiel Garcia , Greg Kroah-Hartman , Gregory Clement , Han Xu , Haojian Zhuang , Jason Cooper , Josh Wu , Kamal Dasu , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org, linux-mtd@lists.infradead.org, Marc Gonzalez , Marek Vasut , Mark Rutland , Masahiro Yamada , Matthias Brugger , Maxime Ripard , Maxim Levitsky , Richard Weinberger , Robert Jarzmik , Rob Herring , Russell King , Sebastian Hesselbarth , Sylvain Lemieux , Vladimir Zapolskiy , Wenyou Yang , Will Deacon , Thomas Petazzoni , Antoine Tenart , Igor Grinberg , Nadav Haklai , Ofer Heifetz , Neta Zur Hershkovits , Hanna Hawa Subject: Re: [RFC 02/12] mtd: nand: force drivers to explicitly send READ/PROG commands Message-ID: <20171020131834.704e2adc@bbrezillon> In-Reply-To: References: <20171018143629.29302-1-miquel.raynal@free-electrons.com> <20171018143629.29302-3-miquel.raynal@free-electrons.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Fri, 20 Oct 2017 11:29:18 +0200 Stefan Agner wrote: > Hi Miquel, > > Thanks for the work on this, happy to see the new interface is moving > forward. Some comments below. > > On 2017-10-18 16:36, Miquel Raynal wrote: > > From: Boris Brezillon > > > > The core currently send the READ0 and SEQIN+PAGEPROG commands in > > nand_do_read/write_ops(). This is inconsistent with > > ->read/write_oob[_raw]() hooks behavior which are expected to send > > these commands. > > > > There's already a flag (NAND_ECC_CUSTOM_PAGE_ACCESS) to inform the core > > that a specific controller wants to send the READ/SEQIN+PAGEPROG > > commands on its own, but it's an opt-in flag, and existing drivers are > > unlikely to be updated to pass it. > > > > Moreover, some controllers cannot dissociate the READ/PAGEPROG commands > > from the associated data transfer and ECC engine activation, and > > developers have to hack things in their ->cmdfunc() implementation to > > handle such complex cases, or have to accept the perf penalty of sending > > twice the same command. > > To address this problem we are planning on adding a new interface which > > is passed all information about a NAND operation (including the amount > > of data to transfer) and replacing all calls to ->cmdfunc() to calls to > > this new ->exec_op() hook. But, in order to do that, we need to have all > > ->cmdfunc() calls placed near their associated ->read/write_buf/byte() > > calls. > > > > Modify the core and relevant drivers to make NAND_ECC_CUSTOM_PAGE_ACCESS > > the default case, and remove this flag. > > > > Signed-off-by: Boris Brezillon > > [miquel.raynal@free-electrons.com: rebased and fixed some conflicts] > > Signed-off-by: Miquel Raynal > > --- > > drivers/mtd/nand/atmel/nand-controller.c | 7 ++- > > drivers/mtd/nand/bf5xx_nand.c | 6 ++- > > drivers/mtd/nand/brcmnand/brcmnand.c | 13 +++-- > > drivers/mtd/nand/cafe_nand.c | 6 +-- > > drivers/mtd/nand/denali.c | 14 +++++- > > drivers/mtd/nand/docg4.c | 12 +++-- > > drivers/mtd/nand/fsl_elbc_nand.c | 6 +-- > > drivers/mtd/nand/fsl_ifc_nand.c | 6 +-- > > drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 30 ++++++------ > > drivers/mtd/nand/hisi504_nand.c | 6 +-- > > drivers/mtd/nand/lpc32xx_mlc.c | 5 +- > > drivers/mtd/nand/lpc32xx_slc.c | 11 +++-- > > drivers/mtd/nand/mtk_nand.c | 10 ++-- > > drivers/mtd/nand/nand_base.c | 68 +++++++++------------------ > > drivers/mtd/nand/nand_micron.c | 1 - > > drivers/mtd/nand/sh_flctl.c | 6 +-- > > drivers/mtd/nand/sunxi_nand.c | 34 +++++++++----- > > drivers/mtd/nand/tango_nand.c | 1 - > > drivers/mtd/nand/vf610_nfc.c | 6 +-- > > drivers/staging/mt29f_spinand/mt29f_spinand.c | 7 ++- > > include/linux/mtd/rawnand.h | 11 ----- > > 21 files changed, 142 insertions(+), 124 deletions(-) > > > > > > > diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c > > b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c > > index c8ceaecd8065..3f2b903158c1 100644 > > --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c > > +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c > > @@ -1043,6 +1043,8 @@ static int gpmi_ecc_read_page(struct mtd_info > > *mtd, struct nand_chip *chip, > > unsigned int max_bitflips = 0; > > int ret; > > > > + nand_read_page_op(chip, page, 0, NULL, 0); > > + > > dev_dbg(this->dev, "page number is : %d\n", page); > > ret = read_page_prepare(this, buf, nfc_geo->payload_size, > > this->payload_virt, this->payload_phys, > > @@ -1220,12 +1222,13 @@ static int gpmi_ecc_read_subpage(struct > > mtd_info *mtd, struct nand_chip *chip, > > meta = geo->metadata_size; > > if (first) { > > col = meta + (size + ecc_parity_size) * first; > > - chip->cmdfunc(mtd, NAND_CMD_RNDOUT, col, -1); > > Shouldn't we add nand_change_read_column_op here? > > > > > > meta = 0; > > buf = buf + first * size; > > } > > > > + nand_read_page_op(chip, page, col, NULL, 0); > > + > > /* Save the old environment */ > > r1_old = r1_new = readl(bch_regs + HW_BCH_FLASH0LAYOUT0); > > r2_old = r2_new = readl(bch_regs + HW_BCH_FLASH0LAYOUT1); > > @@ -1277,6 +1280,9 @@ static int gpmi_ecc_write_page(struct mtd_info > > *mtd, struct nand_chip *chip, > > int ret; > > > > dev_dbg(this->dev, "ecc write page.\n"); > > + > > + nand_prog_page_begin_op(chip, page, 0, NULL, 0); > > + > > if (this->swap_block_mark) { > > /* > > * If control arrives here, we're doing block mark swapping. > > @@ -1338,7 +1344,10 @@ static int gpmi_ecc_write_page(struct mtd_info > > *mtd, struct nand_chip *chip, > > payload_virt, payload_phys); > > } > > > > - return 0; > > + if (ret) > > + return ret; > > + > > + return nand_prog_page_end_op(chip); > > } > > > > /* > > @@ -1472,8 +1481,8 @@ static int gpmi_ecc_read_page_raw(struct mtd_info *mtd, > > uint8_t *oob = chip->oob_poi; > > int step; > > > > - chip->read_buf(mtd, tmp_buf, > > - mtd->writesize + mtd->oobsize); > > + nand_read_page_op(chip, page, 0, tmp_buf, > > + mtd->writesize + mtd->oobsize); > > > > /* > > * If required, swap the bad block marker and the data stored in the > > @@ -1617,24 +1626,19 @@ static int gpmi_ecc_write_page_raw(struct mtd_info *mtd, > > tmp_buf[mtd->writesize] = swap; > > } > > > > - chip->write_buf(mtd, tmp_buf, mtd->writesize + mtd->oobsize); > > - > > - return 0; > > + return nand_prog_page_op(chip, page, 0, tmp_buf, > > + mtd->writesize + mtd->oobsize); > > } > > > > static int gpmi_ecc_read_oob_raw(struct mtd_info *mtd, struct nand_chip *chip, > > int page) > > { > > - nand_read_page_op(chip, page, 0, NULL, 0); > > - > > return gpmi_ecc_read_page_raw(mtd, chip, NULL, 1, page); > > } > > > > static int gpmi_ecc_write_oob_raw(struct mtd_info *mtd, struct nand_chip *chip, > > int page) > > { > > - nand_prog_page_begin_op(chip, page, 0, NULL, 0); > > - > > return gpmi_ecc_write_page_raw(mtd, chip, NULL, 1, page); > > } > > > > @@ -1806,9 +1810,7 @@ static int mx23_write_transcription_stamp(struct > > gpmi_nand_data *this) > > /* Write the first page of the current stride. */ > > dev_dbg(dev, "Writing an NCB fingerprint in page 0x%x\n", page); > > > > - nand_prog_page_begin_op(chip, page, 0, NULL, 0); > > - chip->ecc.write_page_raw(mtd, chip, buffer, 0, page); > > - status = nand_prog_page_end_op(chip); > > + status = chip->ecc.write_page_raw(mtd, chip, buffer, 0, page); > > if (status) > > dev_err(dev, "[%s] Write failed.\n", __func__); > > } > > > > > diff --git a/drivers/mtd/nand/vf610_nfc.c b/drivers/mtd/nand/vf610_nfc.c > > index 8037d4b48a05..80d31a58e558 100644 > > --- a/drivers/mtd/nand/vf610_nfc.c > > +++ b/drivers/mtd/nand/vf610_nfc.c > > @@ -560,7 +560,7 @@ static int vf610_nfc_read_page(struct mtd_info > > *mtd, struct nand_chip *chip, > > int eccsize = chip->ecc.size; > > int stat; > > > > - vf610_nfc_read_buf(mtd, buf, eccsize); > > + nand_read_page_op(chip, page, 0, buf, eccsize); > > if (oob_required) > > vf610_nfc_read_buf(mtd, chip->oob_poi, mtd->oobsize); > > > > @@ -580,7 +580,7 @@ static int vf610_nfc_write_page(struct mtd_info > > *mtd, struct nand_chip *chip, > > { > > struct vf610_nfc *nfc = mtd_to_nfc(mtd); > > > > - vf610_nfc_write_buf(mtd, buf, mtd->writesize); > > We currently ignore NAND_CMD_SEQIN in ->cmdfunc anyway, so I think this > change would not even be necessary here. Well, the thing is, we're trying to keep the logic unchanged. Even if for your driver ->cmdfunc(NAND_CMD_SEQIN) is a NOP, I'd prefer to be consistent and still replace this ->write_buf() call by a nand_prog_page_begin_op() one. > > This is a NAND controller which will benefit from the new interface. I > plan to convert the driver to the new interface, maybe I manage to do > that soon so we have a second driver making use of the new interface. Great!!! Let us know if you need any help. Regards, Boris From mboxrd@z Thu Jan 1 00:00:00 1970 From: boris.brezillon@free-electrons.com (Boris Brezillon) Date: Fri, 20 Oct 2017 13:18:34 +0200 Subject: [RFC 02/12] mtd: nand: force drivers to explicitly send READ/PROG commands In-Reply-To: References: <20171018143629.29302-1-miquel.raynal@free-electrons.com> <20171018143629.29302-3-miquel.raynal@free-electrons.com> Message-ID: <20171020131834.704e2adc@bbrezillon> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Fri, 20 Oct 2017 11:29:18 +0200 Stefan Agner wrote: > Hi Miquel, > > Thanks for the work on this, happy to see the new interface is moving > forward. Some comments below. > > On 2017-10-18 16:36, Miquel Raynal wrote: > > From: Boris Brezillon > > > > The core currently send the READ0 and SEQIN+PAGEPROG commands in > > nand_do_read/write_ops(). This is inconsistent with > > ->read/write_oob[_raw]() hooks behavior which are expected to send > > these commands. > > > > There's already a flag (NAND_ECC_CUSTOM_PAGE_ACCESS) to inform the core > > that a specific controller wants to send the READ/SEQIN+PAGEPROG > > commands on its own, but it's an opt-in flag, and existing drivers are > > unlikely to be updated to pass it. > > > > Moreover, some controllers cannot dissociate the READ/PAGEPROG commands > > from the associated data transfer and ECC engine activation, and > > developers have to hack things in their ->cmdfunc() implementation to > > handle such complex cases, or have to accept the perf penalty of sending > > twice the same command. > > To address this problem we are planning on adding a new interface which > > is passed all information about a NAND operation (including the amount > > of data to transfer) and replacing all calls to ->cmdfunc() to calls to > > this new ->exec_op() hook. But, in order to do that, we need to have all > > ->cmdfunc() calls placed near their associated ->read/write_buf/byte() > > calls. > > > > Modify the core and relevant drivers to make NAND_ECC_CUSTOM_PAGE_ACCESS > > the default case, and remove this flag. > > > > Signed-off-by: Boris Brezillon > > [miquel.raynal at free-electrons.com: rebased and fixed some conflicts] > > Signed-off-by: Miquel Raynal > > --- > > drivers/mtd/nand/atmel/nand-controller.c | 7 ++- > > drivers/mtd/nand/bf5xx_nand.c | 6 ++- > > drivers/mtd/nand/brcmnand/brcmnand.c | 13 +++-- > > drivers/mtd/nand/cafe_nand.c | 6 +-- > > drivers/mtd/nand/denali.c | 14 +++++- > > drivers/mtd/nand/docg4.c | 12 +++-- > > drivers/mtd/nand/fsl_elbc_nand.c | 6 +-- > > drivers/mtd/nand/fsl_ifc_nand.c | 6 +-- > > drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 30 ++++++------ > > drivers/mtd/nand/hisi504_nand.c | 6 +-- > > drivers/mtd/nand/lpc32xx_mlc.c | 5 +- > > drivers/mtd/nand/lpc32xx_slc.c | 11 +++-- > > drivers/mtd/nand/mtk_nand.c | 10 ++-- > > drivers/mtd/nand/nand_base.c | 68 +++++++++------------------ > > drivers/mtd/nand/nand_micron.c | 1 - > > drivers/mtd/nand/sh_flctl.c | 6 +-- > > drivers/mtd/nand/sunxi_nand.c | 34 +++++++++----- > > drivers/mtd/nand/tango_nand.c | 1 - > > drivers/mtd/nand/vf610_nfc.c | 6 +-- > > drivers/staging/mt29f_spinand/mt29f_spinand.c | 7 ++- > > include/linux/mtd/rawnand.h | 11 ----- > > 21 files changed, 142 insertions(+), 124 deletions(-) > > > > > > > diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c > > b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c > > index c8ceaecd8065..3f2b903158c1 100644 > > --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c > > +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c > > @@ -1043,6 +1043,8 @@ static int gpmi_ecc_read_page(struct mtd_info > > *mtd, struct nand_chip *chip, > > unsigned int max_bitflips = 0; > > int ret; > > > > + nand_read_page_op(chip, page, 0, NULL, 0); > > + > > dev_dbg(this->dev, "page number is : %d\n", page); > > ret = read_page_prepare(this, buf, nfc_geo->payload_size, > > this->payload_virt, this->payload_phys, > > @@ -1220,12 +1222,13 @@ static int gpmi_ecc_read_subpage(struct > > mtd_info *mtd, struct nand_chip *chip, > > meta = geo->metadata_size; > > if (first) { > > col = meta + (size + ecc_parity_size) * first; > > - chip->cmdfunc(mtd, NAND_CMD_RNDOUT, col, -1); > > Shouldn't we add nand_change_read_column_op here? > > > > > > meta = 0; > > buf = buf + first * size; > > } > > > > + nand_read_page_op(chip, page, col, NULL, 0); > > + > > /* Save the old environment */ > > r1_old = r1_new = readl(bch_regs + HW_BCH_FLASH0LAYOUT0); > > r2_old = r2_new = readl(bch_regs + HW_BCH_FLASH0LAYOUT1); > > @@ -1277,6 +1280,9 @@ static int gpmi_ecc_write_page(struct mtd_info > > *mtd, struct nand_chip *chip, > > int ret; > > > > dev_dbg(this->dev, "ecc write page.\n"); > > + > > + nand_prog_page_begin_op(chip, page, 0, NULL, 0); > > + > > if (this->swap_block_mark) { > > /* > > * If control arrives here, we're doing block mark swapping. > > @@ -1338,7 +1344,10 @@ static int gpmi_ecc_write_page(struct mtd_info > > *mtd, struct nand_chip *chip, > > payload_virt, payload_phys); > > } > > > > - return 0; > > + if (ret) > > + return ret; > > + > > + return nand_prog_page_end_op(chip); > > } > > > > /* > > @@ -1472,8 +1481,8 @@ static int gpmi_ecc_read_page_raw(struct mtd_info *mtd, > > uint8_t *oob = chip->oob_poi; > > int step; > > > > - chip->read_buf(mtd, tmp_buf, > > - mtd->writesize + mtd->oobsize); > > + nand_read_page_op(chip, page, 0, tmp_buf, > > + mtd->writesize + mtd->oobsize); > > > > /* > > * If required, swap the bad block marker and the data stored in the > > @@ -1617,24 +1626,19 @@ static int gpmi_ecc_write_page_raw(struct mtd_info *mtd, > > tmp_buf[mtd->writesize] = swap; > > } > > > > - chip->write_buf(mtd, tmp_buf, mtd->writesize + mtd->oobsize); > > - > > - return 0; > > + return nand_prog_page_op(chip, page, 0, tmp_buf, > > + mtd->writesize + mtd->oobsize); > > } > > > > static int gpmi_ecc_read_oob_raw(struct mtd_info *mtd, struct nand_chip *chip, > > int page) > > { > > - nand_read_page_op(chip, page, 0, NULL, 0); > > - > > return gpmi_ecc_read_page_raw(mtd, chip, NULL, 1, page); > > } > > > > static int gpmi_ecc_write_oob_raw(struct mtd_info *mtd, struct nand_chip *chip, > > int page) > > { > > - nand_prog_page_begin_op(chip, page, 0, NULL, 0); > > - > > return gpmi_ecc_write_page_raw(mtd, chip, NULL, 1, page); > > } > > > > @@ -1806,9 +1810,7 @@ static int mx23_write_transcription_stamp(struct > > gpmi_nand_data *this) > > /* Write the first page of the current stride. */ > > dev_dbg(dev, "Writing an NCB fingerprint in page 0x%x\n", page); > > > > - nand_prog_page_begin_op(chip, page, 0, NULL, 0); > > - chip->ecc.write_page_raw(mtd, chip, buffer, 0, page); > > - status = nand_prog_page_end_op(chip); > > + status = chip->ecc.write_page_raw(mtd, chip, buffer, 0, page); > > if (status) > > dev_err(dev, "[%s] Write failed.\n", __func__); > > } > > > > > diff --git a/drivers/mtd/nand/vf610_nfc.c b/drivers/mtd/nand/vf610_nfc.c > > index 8037d4b48a05..80d31a58e558 100644 > > --- a/drivers/mtd/nand/vf610_nfc.c > > +++ b/drivers/mtd/nand/vf610_nfc.c > > @@ -560,7 +560,7 @@ static int vf610_nfc_read_page(struct mtd_info > > *mtd, struct nand_chip *chip, > > int eccsize = chip->ecc.size; > > int stat; > > > > - vf610_nfc_read_buf(mtd, buf, eccsize); > > + nand_read_page_op(chip, page, 0, buf, eccsize); > > if (oob_required) > > vf610_nfc_read_buf(mtd, chip->oob_poi, mtd->oobsize); > > > > @@ -580,7 +580,7 @@ static int vf610_nfc_write_page(struct mtd_info > > *mtd, struct nand_chip *chip, > > { > > struct vf610_nfc *nfc = mtd_to_nfc(mtd); > > > > - vf610_nfc_write_buf(mtd, buf, mtd->writesize); > > We currently ignore NAND_CMD_SEQIN in ->cmdfunc anyway, so I think this > change would not even be necessary here. Well, the thing is, we're trying to keep the logic unchanged. Even if for your driver ->cmdfunc(NAND_CMD_SEQIN) is a NOP, I'd prefer to be consistent and still replace this ->write_buf() call by a nand_prog_page_begin_op() one. > > This is a NAND controller which will benefit from the new interface. I > plan to convert the driver to the new interface, maybe I manage to do > that soon so we have a second driver making use of the new interface. Great!!! Let us know if you need any help. Regards, Boris