| Message ID | 20180109101148.13728-7-s.hauer@pengutronix.de |
|---|---|
| State | Superseded |
| Delegated to: | Boris Brezillon |
| Headers | show |
| Series | [1/8] mtd: nand: mxc: reorder functions to avoid forward declarations | expand |
On Tue, 9 Jan 2018 11:11:46 +0100 Sascha Hauer <s.hauer@pengutronix.de> wrote: > Now that we have our own read_page function add a write_page function > for consistency aswell. This can be a lot easier than the generic > function since we do not have to iterate over subpages but can write > the whole page at once. Also add write_page_raw and write_oob for > proper raw and oob write support. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > drivers/mtd/nand/mxc_nand.c | 56 +++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 56 insertions(+) > > diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c > index 83bb01ab7079..6fbf98851a96 100644 > --- a/drivers/mtd/nand/mxc_nand.c > +++ b/drivers/mtd/nand/mxc_nand.c > @@ -893,6 +893,59 @@ static int mxc_nand_read_oob(struct mtd_info *mtd, struct nand_chip *chip, > page); > } > > +static int __mxc_nand_write_page(struct mtd_info *mtd, struct nand_chip *chip, I'm not a big fan of those '__' prefixes. How about keeping mxc_nand_write_page() for this one, renaming the other mxc_nand_write_page() into mxc_nand_write_page_ecc() and keeping the other ones unchanged. > + const uint8_t *buf, int oob_required, int page) Why don't you pass a 'bool enable_ecc' argument here so that you can directly do host->devtype_data->enable_hwecc(nand_chip, enable_ecc); from here instead of duplicating it in all wrappers? > +{ > + struct mxc_nand_host *host = nand_get_controller_data(chip); > + > + host->devtype_data->send_cmd(host, NAND_CMD_SEQIN, false); > + mxc_do_addr_cycle(mtd, 0, page); > + > + memcpy32_toio(host->main_area0, buf, mtd->writesize); > + copy_spare(mtd, false, chip->oob_poi); > + > + host->devtype_data->send_page(mtd, NFC_INPUT); > + host->devtype_data->send_cmd(host, NAND_CMD_PAGEPROG, true); > + mxc_do_addr_cycle(mtd, 0, page); > + > + return 0; > +} > + > +static int mxc_nand_write_page(struct mtd_info *mtd, struct nand_chip *chip, > + const uint8_t *buf, int oob_required, int page) > +{ > + struct nand_chip *nand_chip = mtd_to_nand(mtd); > + struct mxc_nand_host *host = nand_get_controller_data(chip); > + > + host->devtype_data->enable_hwecc(nand_chip, true); > + > + return __mxc_nand_write_page(mtd, chip, buf, oob_required, page); > +} > + > +static int mxc_nand_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip, > + const uint8_t *buf, int oob_required, int page) > +{ > + struct nand_chip *nand_chip = mtd_to_nand(mtd); > + struct mxc_nand_host *host = nand_get_controller_data(chip); > + > + host->devtype_data->enable_hwecc(nand_chip, false); > + > + return __mxc_nand_write_page(mtd, chip, buf, oob_required, page); > +} > + > +static int mxc_nand_write_oob(struct mtd_info *mtd, struct nand_chip *chip, > + int page) > +{ > + struct nand_chip *nand_chip = mtd_to_nand(mtd); > + struct mxc_nand_host *host = nand_get_controller_data(chip); > + > + memset(host->data_buf, 0xff, mtd->writesize); > + > + host->devtype_data->enable_hwecc(nand_chip, false); > + > + return __mxc_nand_write_page(mtd, chip, host->data_buf, 1, page); > +} > + > static int mxc_nand_calculate_ecc(struct mtd_info *mtd, const u_char *dat, > u_char *ecc_code) > { > @@ -1907,6 +1960,9 @@ static int mxcnd_probe(struct platform_device *pdev) > this->ecc.read_page = mxc_nand_read_page; > this->ecc.read_page_raw = mxc_nand_read_page_raw; > this->ecc.read_oob = mxc_nand_read_oob; > + this->ecc.write_page = mxc_nand_write_page; > + this->ecc.write_page_raw = mxc_nand_write_page_raw; > + this->ecc.write_oob = mxc_nand_write_oob; > this->ecc.calculate = mxc_nand_calculate_ecc; > this->ecc.hwctl = mxc_nand_enable_hwecc; > this->ecc.correct = host->devtype_data->correct_data;
Hi Sascha, On Tue, 9 Jan 2018 11:11:46 +0100 Sascha Hauer <s.hauer@pengutronix.de> wrote: > Now that we have our own read_page function add a write_page function > for consistency aswell. This can be a lot easier than the generic > function since we do not have to iterate over subpages but can write > the whole page at once. Also add write_page_raw and write_oob for > proper raw and oob write support. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > drivers/mtd/nand/mxc_nand.c | 56 > +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 > insertions(+) > > diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c > index 83bb01ab7079..6fbf98851a96 100644 > --- a/drivers/mtd/nand/mxc_nand.c > +++ b/drivers/mtd/nand/mxc_nand.c > @@ -893,6 +893,59 @@ static int mxc_nand_read_oob(struct mtd_info > *mtd, struct nand_chip *chip, page); > } > > +static int __mxc_nand_write_page(struct mtd_info *mtd, struct > nand_chip *chip, > + const uint8_t *buf, int > oob_required, int page) +{ > + struct mxc_nand_host *host = nand_get_controller_data(chip); > + > + host->devtype_data->send_cmd(host, NAND_CMD_SEQIN, false); > + mxc_do_addr_cycle(mtd, 0, page); > + > + memcpy32_toio(host->main_area0, buf, mtd->writesize); > + copy_spare(mtd, false, chip->oob_poi); If I remember correctly, there was a "spare only" mechanism to avoid doing full page read/write when unnecessary. What about using it here when (buf == NULL && oob_required)? > + > + host->devtype_data->send_page(mtd, NFC_INPUT); > + host->devtype_data->send_cmd(host, NAND_CMD_PAGEPROG, true); > + mxc_do_addr_cycle(mtd, 0, page); > + > + return 0; > +} > + > +static int mxc_nand_write_page(struct mtd_info *mtd, struct > nand_chip *chip, > + const uint8_t *buf, int oob_required, > int page) +{ > + struct nand_chip *nand_chip = mtd_to_nand(mtd); > + struct mxc_nand_host *host = nand_get_controller_data(chip); > + > + host->devtype_data->enable_hwecc(nand_chip, true); > + > + return __mxc_nand_write_page(mtd, chip, buf, oob_required, > page); +} > + > +static int mxc_nand_write_page_raw(struct mtd_info *mtd, struct > nand_chip *chip, > + const uint8_t *buf, int > oob_required, int page) +{ > + struct nand_chip *nand_chip = mtd_to_nand(mtd); > + struct mxc_nand_host *host = nand_get_controller_data(chip); > + > + host->devtype_data->enable_hwecc(nand_chip, false); > + > + return __mxc_nand_write_page(mtd, chip, buf, oob_required, > page); +} > + > +static int mxc_nand_write_oob(struct mtd_info *mtd, struct nand_chip > *chip, > + int page) > +{ > + struct nand_chip *nand_chip = mtd_to_nand(mtd); > + struct mxc_nand_host *host = nand_get_controller_data(chip); > + > + memset(host->data_buf, 0xff, mtd->writesize); If the above solution works, this won't be needed anymore. > + > + host->devtype_data->enable_hwecc(nand_chip, false); > + > + return __mxc_nand_write_page(mtd, chip, host->data_buf, 1, > page); And this could be: return __mxc_nand_write_page(mtd, chip, NULL, 1, page); > +} > + > static int mxc_nand_calculate_ecc(struct mtd_info *mtd, const u_char > *dat, u_char *ecc_code) > { > @@ -1907,6 +1960,9 @@ static int mxcnd_probe(struct platform_device > *pdev) this->ecc.read_page = mxc_nand_read_page; > this->ecc.read_page_raw = mxc_nand_read_page_raw; > this->ecc.read_oob = mxc_nand_read_oob; > + this->ecc.write_page = mxc_nand_write_page; > + this->ecc.write_page_raw = mxc_nand_write_page_raw; > + this->ecc.write_oob = mxc_nand_write_oob; > this->ecc.calculate = mxc_nand_calculate_ecc; > this->ecc.hwctl = mxc_nand_enable_hwecc; > this->ecc.correct = host->devtype_data->correct_data; Thanks, Miquèl
Hi Miquel, On Sat, Jan 13, 2018 at 05:25:51PM +0100, Miquel Raynal wrote: > Hi Sascha, > > On Tue, 9 Jan 2018 11:11:46 +0100 > Sascha Hauer <s.hauer@pengutronix.de> wrote: > > > Now that we have our own read_page function add a write_page function > > for consistency aswell. This can be a lot easier than the generic > > function since we do not have to iterate over subpages but can write > > the whole page at once. Also add write_page_raw and write_oob for > > proper raw and oob write support. > > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > > --- > > drivers/mtd/nand/mxc_nand.c | 56 > > +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 > > insertions(+) > > > > diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c > > index 83bb01ab7079..6fbf98851a96 100644 > > --- a/drivers/mtd/nand/mxc_nand.c > > +++ b/drivers/mtd/nand/mxc_nand.c > > @@ -893,6 +893,59 @@ static int mxc_nand_read_oob(struct mtd_info > > *mtd, struct nand_chip *chip, page); > > } > > > > +static int __mxc_nand_write_page(struct mtd_info *mtd, struct > > nand_chip *chip, > > + const uint8_t *buf, int > > oob_required, int page) +{ > > + struct mxc_nand_host *host = nand_get_controller_data(chip); > > + > > + host->devtype_data->send_cmd(host, NAND_CMD_SEQIN, false); > > + mxc_do_addr_cycle(mtd, 0, page); > > + > > + memcpy32_toio(host->main_area0, buf, mtd->writesize); > > + copy_spare(mtd, false, chip->oob_poi); > > If I remember correctly, there was a "spare only" mechanism to avoid > doing full page read/write when unnecessary. What about using it here > when (buf == NULL && oob_required)? You are probably referring to the SP_EN bit in the NAND_FLASH_CONFIG1 register right? This is described as: > NAND Flash spare enable. This bit determines whether host reads/writes > are to NAND Flash spare data only or NAND Flash main and spare data. > This feature is supported only with memories with 1/2K page size. > Note: There is no ECC in this mode of operation. > 0 NAND Flash main and spare data is enabled > 1 NAND Flash spare only data is enabled At least the i.MX25 has the limitation of 1/2k page size for this feature. It's not in the i.MX27 manual. I don't know if the i.MX27 documentation is inaccurate or if this limitation is there already. Anyway, I don't think it's worth optimizing for this case given that we still need to support a full page write for 2k pages (which should be much more widespread nowadays than the 512b page size NANDs). Sascha
Hi Sascha, On Wed, 17 Jan 2018 11:40:09 +0100 Sascha Hauer <s.hauer@pengutronix.de> wrote: > Hi Miquel, > > On Sat, Jan 13, 2018 at 05:25:51PM +0100, Miquel Raynal wrote: > > Hi Sascha, > > > > On Tue, 9 Jan 2018 11:11:46 +0100 > > Sascha Hauer <s.hauer@pengutronix.de> wrote: > > > > > Now that we have our own read_page function add a write_page > > > function for consistency aswell. This can be a lot easier than > > > the generic function since we do not have to iterate over > > > subpages but can write the whole page at once. Also add > > > write_page_raw and write_oob for proper raw and oob write support. > > > > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > > > --- > > > drivers/mtd/nand/mxc_nand.c | 56 > > > +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 > > > insertions(+) > > > > > > diff --git a/drivers/mtd/nand/mxc_nand.c > > > b/drivers/mtd/nand/mxc_nand.c index 83bb01ab7079..6fbf98851a96 > > > 100644 --- a/drivers/mtd/nand/mxc_nand.c > > > +++ b/drivers/mtd/nand/mxc_nand.c > > > @@ -893,6 +893,59 @@ static int mxc_nand_read_oob(struct mtd_info > > > *mtd, struct nand_chip *chip, page); > > > } > > > > > > +static int __mxc_nand_write_page(struct mtd_info *mtd, struct > > > nand_chip *chip, > > > + const uint8_t *buf, int > > > oob_required, int page) +{ > > > + struct mxc_nand_host *host = > > > nand_get_controller_data(chip); + > > > + host->devtype_data->send_cmd(host, NAND_CMD_SEQIN, > > > false); > > > + mxc_do_addr_cycle(mtd, 0, page); > > > + > > > + memcpy32_toio(host->main_area0, buf, mtd->writesize); > > > + copy_spare(mtd, false, chip->oob_poi); > > > > If I remember correctly, there was a "spare only" mechanism to avoid > > doing full page read/write when unnecessary. What about using it > > here when (buf == NULL && oob_required)? > > You are probably referring to the SP_EN bit in the NAND_FLASH_CONFIG1 > register right? This is described as: > > > NAND Flash spare enable. This bit determines whether host > > reads/writes are to NAND Flash spare data only or NAND Flash main > > and spare data. This feature is supported only with memories with > > 1/2K page size. Note: There is no ECC in this mode of operation. > > 0 NAND Flash main and spare data is enabled > > 1 NAND Flash spare only data is enabled > > At least the i.MX25 has the limitation of 1/2k page size for this > feature. It's not in the i.MX27 manual. I don't know if the i.MX27 > documentation is inaccurate or if this limitation is there already. Ok, I ignored this limitation on i.MX25. Better write generic code then. > > Anyway, I don't think it's worth optimizing for this case given that > we still need to support a full page write for 2k pages (which should > be much more widespread nowadays than the 512b page size NANDs). Sure! Thanks, Miquèl
diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c index 83bb01ab7079..6fbf98851a96 100644 --- a/drivers/mtd/nand/mxc_nand.c +++ b/drivers/mtd/nand/mxc_nand.c @@ -893,6 +893,59 @@ static int mxc_nand_read_oob(struct mtd_info *mtd, struct nand_chip *chip, page); } +static int __mxc_nand_write_page(struct mtd_info *mtd, struct nand_chip *chip, + const uint8_t *buf, int oob_required, int page) +{ + struct mxc_nand_host *host = nand_get_controller_data(chip); + + host->devtype_data->send_cmd(host, NAND_CMD_SEQIN, false); + mxc_do_addr_cycle(mtd, 0, page); + + memcpy32_toio(host->main_area0, buf, mtd->writesize); + copy_spare(mtd, false, chip->oob_poi); + + host->devtype_data->send_page(mtd, NFC_INPUT); + host->devtype_data->send_cmd(host, NAND_CMD_PAGEPROG, true); + mxc_do_addr_cycle(mtd, 0, page); + + return 0; +} + +static int mxc_nand_write_page(struct mtd_info *mtd, struct nand_chip *chip, + const uint8_t *buf, int oob_required, int page) +{ + struct nand_chip *nand_chip = mtd_to_nand(mtd); + struct mxc_nand_host *host = nand_get_controller_data(chip); + + host->devtype_data->enable_hwecc(nand_chip, true); + + return __mxc_nand_write_page(mtd, chip, buf, oob_required, page); +} + +static int mxc_nand_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip, + const uint8_t *buf, int oob_required, int page) +{ + struct nand_chip *nand_chip = mtd_to_nand(mtd); + struct mxc_nand_host *host = nand_get_controller_data(chip); + + host->devtype_data->enable_hwecc(nand_chip, false); + + return __mxc_nand_write_page(mtd, chip, buf, oob_required, page); +} + +static int mxc_nand_write_oob(struct mtd_info *mtd, struct nand_chip *chip, + int page) +{ + struct nand_chip *nand_chip = mtd_to_nand(mtd); + struct mxc_nand_host *host = nand_get_controller_data(chip); + + memset(host->data_buf, 0xff, mtd->writesize); + + host->devtype_data->enable_hwecc(nand_chip, false); + + return __mxc_nand_write_page(mtd, chip, host->data_buf, 1, page); +} + static int mxc_nand_calculate_ecc(struct mtd_info *mtd, const u_char *dat, u_char *ecc_code) { @@ -1907,6 +1960,9 @@ static int mxcnd_probe(struct platform_device *pdev) this->ecc.read_page = mxc_nand_read_page; this->ecc.read_page_raw = mxc_nand_read_page_raw; this->ecc.read_oob = mxc_nand_read_oob; + this->ecc.write_page = mxc_nand_write_page; + this->ecc.write_page_raw = mxc_nand_write_page_raw; + this->ecc.write_oob = mxc_nand_write_oob; this->ecc.calculate = mxc_nand_calculate_ecc; this->ecc.hwctl = mxc_nand_enable_hwecc; this->ecc.correct = host->devtype_data->correct_data;
Now that we have our own read_page function add a write_page function for consistency aswell. This can be a lot easier than the generic function since we do not have to iterate over subpages but can write the whole page at once. Also add write_page_raw and write_oob for proper raw and oob write support. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- drivers/mtd/nand/mxc_nand.c | 56 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+)