* [PATCH] mtd: gpmi: add NAND write verify support @ 2012-08-10 3:18 Huang Shijie 2012-08-10 4:12 ` Fabio Estevam 2012-08-10 11:25 ` Marek Vasut 0 siblings, 2 replies; 7+ messages in thread From: Huang Shijie @ 2012-08-10 3:18 UTC (permalink / raw) To: linux-arm-kernel Add NAND write verify support in gpmi-nand driver. Signed-off-by: Huang Shijie <b32955@freescale.com> --- drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 17 +++++++++++++++++ 1 files changed, 17 insertions(+), 0 deletions(-) diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c index 8c0d2f0..6394483 100644 --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c @@ -1488,6 +1488,22 @@ static int gpmi_set_geometry(struct gpmi_nand_data *this) return gpmi_alloc_dma_buffer(this); } +#define MAX_PAGESIZE 8192 +static uint8_t verify_buf[MAX_PAGESIZE]; + +static int gpmi_verify_buf(struct mtd_info *mtd, const uint8_t *buf, int len) +{ + struct nand_chip *nand = mtd->priv; + int ret; + + ret = gpmi_ecc_read_page(mtd, nand, verify_buf, 0, 0); + if (ret) + return -EFAULT; + if (memcmp(buf, verify_buf, len)) + return -EFAULT; + return 0; +} + static int gpmi_pre_bbt_scan(struct gpmi_nand_data *this) { int ret; @@ -1556,6 +1572,7 @@ static int __devinit gpmi_nfc_init(struct gpmi_nand_data *this) chip->read_byte = gpmi_read_byte; chip->read_buf = gpmi_read_buf; chip->write_buf = gpmi_write_buf; + chip->verify_buf = gpmi_verify_buf; chip->ecc.read_page = gpmi_ecc_read_page; chip->ecc.write_page = gpmi_ecc_write_page; chip->ecc.read_oob = gpmi_ecc_read_oob; -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH] mtd: gpmi: add NAND write verify support 2012-08-10 3:18 [PATCH] mtd: gpmi: add NAND write verify support Huang Shijie @ 2012-08-10 4:12 ` Fabio Estevam 2012-08-10 11:25 ` Marek Vasut 1 sibling, 0 replies; 7+ messages in thread From: Fabio Estevam @ 2012-08-10 4:12 UTC (permalink / raw) To: linux-arm-kernel On Fri, Aug 10, 2012 at 12:18 AM, Huang Shijie <b32955@freescale.com> wrote: > Add NAND write verify support in gpmi-nand driver. > > Signed-off-by: Huang Shijie <b32955@freescale.com> Tested by: Fabio Estevam <fabio.estevam@freescale.com> ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] mtd: gpmi: add NAND write verify support 2012-08-10 3:18 [PATCH] mtd: gpmi: add NAND write verify support Huang Shijie 2012-08-10 4:12 ` Fabio Estevam @ 2012-08-10 11:25 ` Marek Vasut 2012-08-10 14:15 ` Huang Shijie 1 sibling, 1 reply; 7+ messages in thread From: Marek Vasut @ 2012-08-10 11:25 UTC (permalink / raw) To: linux-arm-kernel Dear Huang Shijie, > Add NAND write verify support in gpmi-nand driver. > > Signed-off-by: Huang Shijie <b32955@freescale.com> > --- > drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 17 +++++++++++++++++ > 1 files changed, 17 insertions(+), 0 deletions(-) > > diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c > b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c index 8c0d2f0..6394483 100644 > --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c > +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c > @@ -1488,6 +1488,22 @@ static int gpmi_set_geometry(struct gpmi_nand_data > *this) return gpmi_alloc_dma_buffer(this); > } > > +#define MAX_PAGESIZE 8192 Use NAND_MAX_PAGESIZE, see include/linux/mtd/nand.h . > +static uint8_t verify_buf[MAX_PAGESIZE]; What will happen when you have two NANDs attached to GPMI controller and they hit this place both at once? Race condition, causing this function to fail for both? Possibly devm_kmalloc() such buffer per-controller? > +static int gpmi_verify_buf(struct mtd_info *mtd, const uint8_t *buf, int > len) +{ > + struct nand_chip *nand = mtd->priv; > + int ret; > + > + ret = gpmi_ecc_read_page(mtd, nand, verify_buf, 0, 0); mtd->chip.ecc.read_page() ? > + if (ret) > + return -EFAULT; > + if (memcmp(buf, verify_buf, len)) > + return -EFAULT; > + return 0; > +} > + > static int gpmi_pre_bbt_scan(struct gpmi_nand_data *this) > { > int ret; > @@ -1556,6 +1572,7 @@ static int __devinit gpmi_nfc_init(struct > gpmi_nand_data *this) chip->read_byte = gpmi_read_byte; > chip->read_buf = gpmi_read_buf; > chip->write_buf = gpmi_write_buf; > + chip->verify_buf = gpmi_verify_buf; > chip->ecc.read_page = gpmi_ecc_read_page; > chip->ecc.write_page = gpmi_ecc_write_page; > chip->ecc.read_oob = gpmi_ecc_read_oob; Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] mtd: gpmi: add NAND write verify support 2012-08-10 11:25 ` Marek Vasut @ 2012-08-10 14:15 ` Huang Shijie 2012-08-10 15:04 ` Marek Vasut 0 siblings, 1 reply; 7+ messages in thread From: Huang Shijie @ 2012-08-10 14:15 UTC (permalink / raw) To: linux-arm-kernel On Fri, Aug 10, 2012 at 7:25 AM, Marek Vasut <marex@denx.de> wrote: > Dear Huang Shijie, > >> Add NAND write verify support in gpmi-nand driver. >> >> Signed-off-by: Huang Shijie <b32955@freescale.com> >> --- >> drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 17 +++++++++++++++++ >> 1 files changed, 17 insertions(+), 0 deletions(-) >> >> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c >> b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c index 8c0d2f0..6394483 100644 >> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c >> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c >> @@ -1488,6 +1488,22 @@ static int gpmi_set_geometry(struct gpmi_nand_data >> *this) return gpmi_alloc_dma_buffer(this); >> } >> >> +#define MAX_PAGESIZE 8192 > > Use NAND_MAX_PAGESIZE, see include/linux/mtd/nand.h . > >> +static uint8_t verify_buf[MAX_PAGESIZE]; ok, thanks. > > What will happen when you have two NANDs attached to GPMI controller and they > hit this place both at once? Race condition, causing this function to fail for > both? > Even there are two nand chips, there is only one gpmi controller. And the current gpmi-nand does not support two nands now. does the race occur with only one nand chip? > Possibly devm_kmalloc() such buffer per-controller? > >> +static int gpmi_verify_buf(struct mtd_info *mtd, const uint8_t *buf, int >> len) +{ >> + struct nand_chip *nand = mtd->priv; >> + int ret; >> + >> + ret = gpmi_ecc_read_page(mtd, nand, verify_buf, 0, 0); > > mtd->chip.ecc.read_page() ? In actually, they are the same. Best Regards Huang Shijie ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] mtd: gpmi: add NAND write verify support 2012-08-10 14:15 ` Huang Shijie @ 2012-08-10 15:04 ` Marek Vasut 2012-08-10 17:15 ` Fabio Estevam 0 siblings, 1 reply; 7+ messages in thread From: Marek Vasut @ 2012-08-10 15:04 UTC (permalink / raw) To: linux-arm-kernel Dear Huang Shijie, > On Fri, Aug 10, 2012 at 7:25 AM, Marek Vasut <marex@denx.de> wrote: > > Dear Huang Shijie, > > > >> Add NAND write verify support in gpmi-nand driver. > >> > >> Signed-off-by: Huang Shijie <b32955@freescale.com> > >> --- > >> > >> drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 17 +++++++++++++++++ > >> 1 files changed, 17 insertions(+), 0 deletions(-) > >> > >> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c > >> b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c index 8c0d2f0..6394483 100644 > >> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c > >> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c > >> @@ -1488,6 +1488,22 @@ static int gpmi_set_geometry(struct > >> gpmi_nand_data *this) return gpmi_alloc_dma_buffer(this); > >> > >> } > >> > >> +#define MAX_PAGESIZE 8192 > > > > Use NAND_MAX_PAGESIZE, see include/linux/mtd/nand.h . > > > >> +static uint8_t verify_buf[MAX_PAGESIZE]; > > ok, thanks. > > > What will happen when you have two NANDs attached to GPMI controller and > > they hit this place both at once? Race condition, causing this function > > to fail for both? > > Even there are two nand chips, there is only one gpmi controller. For now ... yes. > And the current gpmi-nand does not support two nands now. > does the race occur with only one nand chip? With one chip and one GPMI controller, no ... but that's no reason to introduce crappy and error prone code! > > Possibly devm_kmalloc() such buffer per-controller? > > > >> +static int gpmi_verify_buf(struct mtd_info *mtd, const uint8_t *buf, > >> int len) +{ > >> + struct nand_chip *nand = mtd->priv; > >> + int ret; > >> + > >> + ret = gpmi_ecc_read_page(mtd, nand, verify_buf, 0, 0); > > > > mtd->chip.ecc.read_page() ? > > In actually, they are the same. They are, but if you rename the function, this is one less place to care for. > > Best Regards > Huang Shijie Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] mtd: gpmi: add NAND write verify support 2012-08-10 15:04 ` Marek Vasut @ 2012-08-10 17:15 ` Fabio Estevam 2012-08-10 19:33 ` Marek Vasut 0 siblings, 1 reply; 7+ messages in thread From: Fabio Estevam @ 2012-08-10 17:15 UTC (permalink / raw) To: linux-arm-kernel Hi Marek, On Fri, Aug 10, 2012 at 12:04 PM, Marek Vasut <marex@denx.de> wrote: >> Even there are two nand chips, there is only one gpmi controller. > > For now ... yes. Does this look better? diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand index 8c0d2f0..5262ef3 100644 --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c @@ -1533,6 +1533,21 @@ void gpmi_nfc_exit(struct gpmi_nand_data *this) gpmi_free_dma_buffer(this); } +static int gpmi_verify_buf(struct mtd_info *mtd, const uint8_t *buf, int len) +{ + struct nand_chip *nand = mtd->priv; + struct gpmi_nand_data *data = container_of(mtd, struct gpmi_nand_data, + mtd); + int ret; + + ret = gpmi_ecc_read_page(mtd, nand, data->verify_buf, 0, 0); + if (ret) + return -EFAULT; + if (memcmp(buf, data->verify_buf, len)) + return -EFAULT; + return 0; +} + static int __devinit gpmi_nfc_init(struct gpmi_nand_data *this) { struct mtd_info *mtd = &this->mtd; @@ -1555,6 +1570,7 @@ static int __devinit gpmi_nfc_init(struct gpmi_nand_data * chip->dev_ready = gpmi_dev_ready; chip->read_byte = gpmi_read_byte; chip->read_buf = gpmi_read_buf; + chip->verify_buf = gpmi_verify_buf; chip->write_buf = gpmi_write_buf; chip->ecc.read_page = gpmi_ecc_read_page; chip->ecc.write_page = gpmi_ecc_write_page; diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h b/drivers/mtd/nand/gpmi-nand index 1547a60..cd9bdf7 100644 --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h @@ -148,6 +148,7 @@ struct gpmi_nand_data { /* General-use Variables */ int current_chip; unsigned int command_length; + uint8_t verify_buf[NAND_MAX_PAGESIZE]; /* passed from upper layer */ uint8_t *upper_buf; and then I can also send this separate change to allow multi-instances: @@ -1630,7 +1646,7 @@ static int __devinit gpmi_nand_probe(struct platform_devic return -ENOMEM; } - this = kzalloc(sizeof(*this), GFP_KERNEL); + this = devm_kzalloc(&pdev->dev, sizeof(*this), GFP_KERNEL); if (!this) { pr_err("Failed to allocate per-device memory\n"); return -ENOMEM; ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH] mtd: gpmi: add NAND write verify support 2012-08-10 17:15 ` Fabio Estevam @ 2012-08-10 19:33 ` Marek Vasut 0 siblings, 0 replies; 7+ messages in thread From: Marek Vasut @ 2012-08-10 19:33 UTC (permalink / raw) To: linux-arm-kernel Dear Fabio Estevam, > Hi Marek, > > On Fri, Aug 10, 2012 at 12:04 PM, Marek Vasut <marex@denx.de> wrote: > >> Even there are two nand chips, there is only one gpmi controller. > > > > For now ... yes. > > Does this look better? > > diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c > b/drivers/mtd/nand/gpmi-nand index 8c0d2f0..5262ef3 100644 > --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c > +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c > @@ -1533,6 +1533,21 @@ void gpmi_nfc_exit(struct gpmi_nand_data *this) > gpmi_free_dma_buffer(this); > } > > +static int gpmi_verify_buf(struct mtd_info *mtd, const uint8_t *buf, int > len) +{ > + struct nand_chip *nand = mtd->priv; > + struct gpmi_nand_data *data = container_of(mtd, struct > gpmi_nand_data, + > mtd); + int ret; > + > + ret = gpmi_ecc_read_page(mtd, nand, data->verify_buf, 0, 0); You ignored my comment on this one. > + if (ret) > + return -EFAULT; > + if (memcmp(buf, data->verify_buf, len)) > + return -EFAULT; > + return 0; > +} > + > static int __devinit gpmi_nfc_init(struct gpmi_nand_data *this) > { > struct mtd_info *mtd = &this->mtd; > @@ -1555,6 +1570,7 @@ static int __devinit gpmi_nfc_init(struct > gpmi_nand_data * chip->dev_ready = gpmi_dev_ready; > chip->read_byte = gpmi_read_byte; > chip->read_buf = gpmi_read_buf; > + chip->verify_buf = gpmi_verify_buf; > chip->write_buf = gpmi_write_buf; > chip->ecc.read_page = gpmi_ecc_read_page; > chip->ecc.write_page = gpmi_ecc_write_page; > diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h > b/drivers/mtd/nand/gpmi-nand index 1547a60..cd9bdf7 100644 > --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h > +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h > @@ -148,6 +148,7 @@ struct gpmi_nand_data { > /* General-use Variables */ > int current_chip; > unsigned int command_length; > + uint8_t verify_buf[NAND_MAX_PAGESIZE]; Yes, this is definitelly better. > /* passed from upper layer */ > uint8_t *upper_buf; > > and then I can also send this separate change to allow multi-instances: > > @@ -1630,7 +1646,7 @@ static int __devinit gpmi_nand_probe(struct > platform_devic return -ENOMEM; > } > > - this = kzalloc(sizeof(*this), GFP_KERNEL); > + this = devm_kzalloc(&pdev->dev, sizeof(*this), GFP_KERNEL); devm_clk_* functions could be used too from a quick glance ... devm_request_irq is in place iirc, but check that too. > if (!this) { > pr_err("Failed to allocate per-device memory\n"); > return -ENOMEM; Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2012-08-10 19:33 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-08-10 3:18 [PATCH] mtd: gpmi: add NAND write verify support Huang Shijie 2012-08-10 4:12 ` Fabio Estevam 2012-08-10 11:25 ` Marek Vasut 2012-08-10 14:15 ` Huang Shijie 2012-08-10 15:04 ` Marek Vasut 2012-08-10 17:15 ` Fabio Estevam 2012-08-10 19:33 ` Marek Vasut
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).