| Message ID | 1383758172-29798-2-git-send-email-festevam@gmail.com |
|---|---|
| State | Rejected |
| Headers | show |
On Wed, Nov 6, 2013 at 9:16 AM, Fabio Estevam <festevam@gmail.com> wrote: > From: Fabio Estevam <fabio.estevam@freescale.com> > > Commit f720e7ce (mtd: gpmi: remove the nand_scan()) caused the following > regression on mx23: ... > This reverts commit f720e7ce510bf79f029be45ce200ccfce5551280. > > Cc: <stable@vger.kernel.org> #3.12 > Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com> > --- > Kernel 3.11 boots fine on mx23, but 3.12 fails because of this commit. I seriously doubt that a total revert is the correct answer to this problem. Can you pick up the discussion with Huang on the mailing list, or at least explain what the actual root cause is? Have you been going through some private communication on this issue? A proper bugfix with an identified issue is preferable to a blind revert. Huang, is the DMA patch you sent related, or is it a totally different bug? Brian
Hi Brian, On Wed, Nov 6, 2013 at 4:08 PM, Brian Norris <computersforpeace@gmail.com> wrote: > I seriously doubt that a total revert is the correct answer to this > problem. Can you pick up the discussion with Huang on the mailing Agree that it would be better to come up with a better localized fix rather than the complete revert, but I currently don't have the time to spend on debugging this issue. On the other hand, I would not like to see mx23 broken in 3.12 because of this. It can't even reach the prompt. > list, or at least explain what the actual root cause is? Have you been > going through some private communication on this issue? A proper > bugfix with an identified issue is preferable to a blind revert. No private communication. I reported this issue in the mailing list: http://lists.infradead.org/pipermail/linux-mtd/2013-November/049641.html Regards, Fabio Estevam
On Wed, Nov 6, 2013 at 10:42 AM, Fabio Estevam <festevam@gmail.com> wrote: > On Wed, Nov 6, 2013 at 4:08 PM, Brian Norris > <computersforpeace@gmail.com> wrote: >> I seriously doubt that a total revert is the correct answer to this >> problem. Can you pick up the discussion with Huang on the mailing > > Agree that it would be better to come up with a better localized fix > rather than the complete revert, but I currently don't have the time > to spend on debugging this issue. OK. And if Huang doesn't have the time to clean up his mess, then we will have to revert things. > On the other hand, I would not like to see mx23 broken in 3.12 because > of this. It can't even reach the prompt. 3.12 is already out the door, so we now have some time to get a proper fix into -stable. >> list, or at least explain what the actual root cause is? Have you been >> going through some private communication on this issue? A proper >> bugfix with an identified issue is preferable to a blind revert. > > No private communication. I reported this issue in the mailing list: > http://lists.infradead.org/pipermail/linux-mtd/2013-November/049641.html Right. But this ended with you pointing Huang to the right U-boot setup, then everything went silent. And today, I get a blunt revert request from you as well as an independent DMA fix from Huang. Since you guys are both from Freescale, I wasn't sure if there was any coordination/correlation, or if this is really as uncoordinated as it looks. Brian
Hi Brian, On Wed, Nov 6, 2013 at 4:58 PM, Brian Norris <computersforpeace@gmail.com> wrote: >> No private communication. I reported this issue in the mailing list: >> http://lists.infradead.org/pipermail/linux-mtd/2013-November/049641.html > > Right. But this ended with you pointing Huang to the right U-boot > setup, then everything went silent. And today, I get a blunt revert > request from you as well as an independent DMA fix from Huang. Since > you guys are both from Freescale, I wasn't sure if there was any > coordination/correlation, or if this is really as uncoordinated as it > looks. Sorry for the confusion. The DMA fix Huang sent today is unrelated to this mx23 kernel crash on boot. Regards, Fabio Estevam
Dear Brian Norris, > On Wed, Nov 6, 2013 at 10:42 AM, Fabio Estevam <festevam@gmail.com> wrote: > > On Wed, Nov 6, 2013 at 4:08 PM, Brian Norris > > > > <computersforpeace@gmail.com> wrote: > >> I seriously doubt that a total revert is the correct answer to this > >> problem. Can you pick up the discussion with Huang on the mailing > > > > Agree that it would be better to come up with a better localized fix > > rather than the complete revert, but I currently don't have the time > > to spend on debugging this issue. > > OK. And if Huang doesn't have the time to clean up his mess, then we > will have to revert things. I have an MX23EVK here, don't make me use it! ;-) > > On the other hand, I would not like to see mx23 broken in 3.12 because > > of this. It can't even reach the prompt. > > 3.12 is already out the door, so we now have some time to get a proper > fix into -stable. > > >> list, or at least explain what the actual root cause is? Have you been > >> going through some private communication on this issue? A proper > >> bugfix with an identified issue is preferable to a blind revert. > > > > No private communication. I reported this issue in the mailing list: > > http://lists.infradead.org/pipermail/linux-mtd/2013-November/049641.html > > Right. But this ended with you pointing Huang to the right U-boot > setup, then everything went silent. And today, I get a blunt revert > request from you as well as an independent DMA fix from Huang. Since > you guys are both from Freescale, I wasn't sure if there was any > coordination/correlation, or if this is really as uncoordinated as it > looks. Left hand doesn't know what the right one does in such big companies :( Best regards, Marek Vasut
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c index a5c60c4..5eaefdb 100644 --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c @@ -217,6 +217,7 @@ static bool set_geometry_by_ecc_info(struct gpmi_nand_data *this) if (geo->page_size < mtd->writesize + mtd->oobsize) { of->offset = geo->page_size - mtd->writesize; of->length = mtd->oobsize - of->offset; + mtd->oobavail = gpmi_hw_ecclayout.oobavail = of->length; } geo->payload_size = mtd->writesize; @@ -1591,22 +1592,19 @@ static int gpmi_pre_bbt_scan(struct gpmi_nand_data *this) if (ret) return ret; + /* Adjust the ECC strength according to the chip. */ + this->nand.ecc.strength = this->bch_geometry.ecc_strength; + this->mtd.ecc_strength = this->bch_geometry.ecc_strength; + this->mtd.bitflip_threshold = this->bch_geometry.ecc_strength; + /* NAND boot init, depends on the gpmi_set_geometry(). */ return nand_boot_init(this); } -static void gpmi_nfc_exit(struct gpmi_nand_data *this) -{ - nand_release(&this->mtd); - gpmi_free_dma_buffer(this); -} - -static int gpmi_init_last(struct gpmi_nand_data *this) +static int gpmi_scan_bbt(struct mtd_info *mtd) { - struct mtd_info *mtd = &this->mtd; struct nand_chip *chip = mtd->priv; - struct nand_ecc_ctrl *ecc = &chip->ecc; - struct bch_geometry *bch_geo = &this->bch_geometry; + struct gpmi_nand_data *this = chip->priv; int ret; /* Prepare for the BBT scan. */ @@ -1614,16 +1612,6 @@ static int gpmi_init_last(struct gpmi_nand_data *this) if (ret) return ret; - /* Init the nand_ecc_ctrl{} */ - ecc->read_page = gpmi_ecc_read_page; - ecc->write_page = gpmi_ecc_write_page; - ecc->read_oob = gpmi_ecc_read_oob; - ecc->write_oob = gpmi_ecc_write_oob; - ecc->mode = NAND_ECC_HW; - ecc->size = bch_geo->ecc_chunk_size; - ecc->strength = bch_geo->ecc_strength; - ecc->layout = &gpmi_hw_ecclayout; - /* * Can we enable the extra features? such as EDO or Sync mode. * @@ -1632,7 +1620,14 @@ static int gpmi_init_last(struct gpmi_nand_data *this) */ gpmi_extra_init(this); - return 0; + /* use the default BBT implementation */ + return nand_default_bbt(mtd); +} + +static void gpmi_nfc_exit(struct gpmi_nand_data *this) +{ + nand_release(&this->mtd); + gpmi_free_dma_buffer(this); } static int gpmi_nfc_init(struct gpmi_nand_data *this) @@ -1658,33 +1653,33 @@ static int 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->ecc.read_page = gpmi_ecc_read_page; + chip->ecc.write_page = gpmi_ecc_write_page; + chip->ecc.read_oob = gpmi_ecc_read_oob; + chip->ecc.write_oob = gpmi_ecc_write_oob; + chip->scan_bbt = gpmi_scan_bbt; chip->badblock_pattern = &gpmi_bbt_descr; chip->block_markbad = gpmi_block_markbad; chip->options |= NAND_NO_SUBPAGE_WRITE; + chip->ecc.mode = NAND_ECC_HW; + chip->ecc.size = 1; + chip->ecc.strength = 8; + chip->ecc.layout = &gpmi_hw_ecclayout; if (of_get_nand_on_flash_bbt(this->dev->of_node)) chip->bbt_options |= NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB; - /* - * Allocate a temporary DMA buffer for reading ID in the - * nand_scan_ident(). - */ + /* Allocate a temporary DMA buffer for reading ID in the nand_scan() */ this->bch_geometry.payload_size = 1024; this->bch_geometry.auxiliary_size = 128; ret = gpmi_alloc_dma_buffer(this); if (ret) goto err_out; - ret = nand_scan_ident(mtd, 1, NULL); - if (ret) - goto err_out; - - ret = gpmi_init_last(this); - if (ret) - goto err_out; - - ret = nand_scan_tail(mtd); - if (ret) + ret = nand_scan(mtd, 1); + if (ret) { + pr_err("Chip scan failed\n"); goto err_out; + } ppdata.of_node = this->pdev->dev.of_node; ret = mtd_device_parse_register(mtd, NULL, &ppdata, NULL, 0);