| Message ID | CAOMZO5BCjV6iLUiQ74GskkOdgGpdqbo2ZB=7eu6VeY=mNwWf0Q@mail.gmail.com |
|---|---|
| State | New, archived |
| Headers | show |
Dear Fabio Estevam, > Hi Marek, > > On Thu, Aug 9, 2012 at 8:53 PM, Marek Vasut <marex@denx.de> wrote: > > This problem is there because the GPMI NAND code doesn't implement verify > > buffer > > > function and defaults to nand_verify_buf() call in nand_base.c: > Yes, you are right. > > > Now the chip->IO_ADDR_R is zero, making the kernel access bogus location, > > and therefore crash. So the correct solution is to properly implement > > the struct nand_chip *'s verify_buf function. > > Right, the patch below prevents the kernel to happen: > > --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c > +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c > @@ -857,6 +857,15 @@ static uint8_t gpmi_read_byte(struct mtd_info *mtd) > return buf[0]; > } > > +/* Used by the upper layer to verify the data in NAND Flash > + * with the data in the buf. */ > +static int gpmi_verify_buf(struct mtd_info *mtd, > + const u_char *buf, int len) > +{ > + /* TODO: implement verify_buf mechanism */ > + return 0; > +} NAK! This is only a workaround, proper implementation is needed. If it's not implemented now, I'm pretty sure such workaround will be there forever. > /* > * Handles block mark swapping. > * It can be called in swapping the block mark, or swapping it back, > @@ -1568,6 +1577,7 @@ static int __devinit gpmi_nfc_init(struct > gpmi_nand_data *this) > chip->ecc.size = 1; > chip->ecc.strength = 8; > chip->ecc.layout = &gpmi_hw_ecclayout; > + chip->verify_buf = gpmi_verify_buf; > if (of_get_nand_on_flash_bbt(this->dev->of_node)) > chip->bbt_options |= NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB; > > Now we need to come up with a real gpmi_verify_buf function ;-) > > Regards, > > Fabio Estevam Best regards, Marek Vasut
Dear Marek Vasut, > Dear Fabio Estevam, > > > Hi Marek, > > > > On Thu, Aug 9, 2012 at 8:53 PM, Marek Vasut <marex@denx.de> wrote: > > > This problem is there because the GPMI NAND code doesn't implement > > > verify buffer > > > > > function and defaults to nand_verify_buf() call in nand_base.c: > > Yes, you are right. > > > > > Now the chip->IO_ADDR_R is zero, making the kernel access bogus > > > location, and therefore crash. So the correct solution is to properly > > > implement the struct nand_chip *'s verify_buf function. > > > > Right, the patch below prevents the kernel to happen: > > > > --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c > > +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c > > @@ -857,6 +857,15 @@ static uint8_t gpmi_read_byte(struct mtd_info *mtd) > > > > return buf[0]; > > > > } > > > > +/* Used by the upper layer to verify the data in NAND Flash > > + * with the data in the buf. */ > > +static int gpmi_verify_buf(struct mtd_info *mtd, > > + const u_char *buf, int len) > > +{ > > + /* TODO: implement verify_buf mechanism */ > > + return 0; > > +} > > NAK! This is only a workaround, proper implementation is needed. If it's > not implemented now, I'm pretty sure such workaround will be there > forever. [...] btw if you want a workaround, make Kconfig entry so it'd disallow VERIFY to be selected for GPMI_NAND, but that's also a wrong way to go. Best regards, Marek Vasut
On Thu, Aug 9, 2012 at 10:41 PM, Marek Vasut <marex@denx.de> wrote: > NAK! This is only a workaround, proper implementation is needed. If it's not > implemented now, I'm pretty sure such workaround will be there forever. I know, please see below. > >> /* >> * Handles block mark swapping. >> * It can be called in swapping the block mark, or swapping it back, >> @@ -1568,6 +1577,7 @@ static int __devinit gpmi_nfc_init(struct >> gpmi_nand_data *this) >> chip->ecc.size = 1; >> chip->ecc.strength = 8; >> chip->ecc.layout = &gpmi_hw_ecclayout; >> + chip->verify_buf = gpmi_verify_buf; >> if (of_get_nand_on_flash_bbt(this->dev->of_node)) >> chip->bbt_options |= NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB; >> >> Now we need to come up with a real gpmi_verify_buf function ;-) As I mentioned, I understand that a proper function needs to be created. Regards, Fabio Estevam
于 2012年08月10日 09:49, Fabio Estevam 写道: > On Thu, Aug 9, 2012 at 10:41 PM, Marek Vasut<marex@denx.de> wrote: > >> NAK! This is only a workaround, proper implementation is needed. If it's not >> implemented now, I'm pretty sure such workaround will be there forever. > I know, please see below. > >>> /* >>> * Handles block mark swapping. >>> * It can be called in swapping the block mark, or swapping it back, >>> @@ -1568,6 +1577,7 @@ static int __devinit gpmi_nfc_init(struct >>> gpmi_nand_data *this) >>> chip->ecc.size = 1; >>> chip->ecc.strength = 8; >>> chip->ecc.layout =&gpmi_hw_ecclayout; >>> + chip->verify_buf = gpmi_verify_buf; >>> if (of_get_nand_on_flash_bbt(this->dev->of_node)) >>> chip->bbt_options |= NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB; >>> >>> Now we need to come up with a real gpmi_verify_buf function ;-) I does have a real gpmi_verify_buf function in our BSP code. I will send it out as soon as possible. thanks Huang Shijie > As I mentioned, I understand that a proper function needs to be created. > > Regards, > > Fabio Estevam >
Dear Huang Shijie, > 于 2012年08月10日 09:49, Fabio Estevam 写道: > > On Thu, Aug 9, 2012 at 10:41 PM, Marek Vasut<marex@denx.de> wrote: > >> NAK! This is only a workaround, proper implementation is needed. If it's > >> not implemented now, I'm pretty sure such workaround will be there > >> forever. > > > > I know, please see below. > > > >>> /* > >>> > >>> * Handles block mark swapping. > >>> * It can be called in swapping the block mark, or swapping it back, > >>> > >>> @@ -1568,6 +1577,7 @@ static int __devinit gpmi_nfc_init(struct > >>> gpmi_nand_data *this) > >>> > >>> chip->ecc.size = 1; > >>> chip->ecc.strength = 8; > >>> chip->ecc.layout =&gpmi_hw_ecclayout; > >>> > >>> + chip->verify_buf = gpmi_verify_buf; > >>> > >>> if (of_get_nand_on_flash_bbt(this->dev->of_node)) > >>> > >>> chip->bbt_options |= NAND_BBT_USE_FLASH | > >>> NAND_BBT_NO_OOB; > >>> > >>> Now we need to come up with a real gpmi_verify_buf function ;-) > > I does have a real gpmi_verify_buf function in our BSP code. > > I will send it out as soon as possible. I believe you should roll it out ASAP and also CC stable! > thanks > Huang Shijie > > > As I mentioned, I understand that a proper function needs to be created. > > > > Regards, > > > > Fabio Estevam Best regards, Marek Vasut
On Thu, Aug 9, 2012 at 11:08 PM, Huang Shijie <b32955@freescale.com> wrote: > I does have a real gpmi_verify_buf function in our BSP code. Well, the same crash happens in FSL BSP. > I will send it out as soon as possible. Ok, great. Thanks, Fabio Estevam
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c @@ -857,6 +857,15 @@ static uint8_t gpmi_read_byte(struct mtd_info *mtd) return buf[0]; } +/* Used by the upper layer to verify the data in NAND Flash + * with the data in the buf. */ +static int gpmi_verify_buf(struct mtd_info *mtd, + const u_char *buf, int len) +{ + /* TODO: implement verify_buf mechanism */ + return 0; +} + /* * Handles block mark swapping. * It can be called in swapping the block mark, or swapping it back, @@ -1568,6 +1577,7 @@ static int __devinit gpmi_nfc_init(struct gpmi_nand_data *this) chip->ecc.size = 1; chip->ecc.strength = 8; chip->ecc.layout = &gpmi_hw_ecclayout; + chip->verify_buf = gpmi_verify_buf; if (of_get_nand_on_flash_bbt(this->dev->of_node)) chip->bbt_options |= NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB;