| Message ID | 1558024913-26502-2-git-send-email-kdasu.kdev@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Miquel Raynal |
| Headers | show |
| Series | [v3,1/2] mtd: Add flag to indicate panic_write | expand |
On Thu, May 16, 2019 at 6:42 PM Kamal Dasu <kdasu.kdev@gmail.com> wrote: > > If mtd_oops is in progress, switch to polling during NAND command > completion instead of relying on DMA/interrupts so that the mtd_oops > buffer can be completely written in the assigned NAND partition. With the new flag the semantics change, as soon a panic write happened, the flag will stay and *all* future operates will take the polling/pio path. IMHO this is fine since the kernel cannot recover from an oops. But just to make sure we all get this. :-) An alternative would be to block all further non-panic writes.
On Fri, May 17, 2019 at 4:12 AM Richard Weinberger <richard.weinberger@gmail.com> wrote: > > On Thu, May 16, 2019 at 6:42 PM Kamal Dasu <kdasu.kdev@gmail.com> wrote: > > > > If mtd_oops is in progress, switch to polling during NAND command > > completion instead of relying on DMA/interrupts so that the mtd_oops > > buffer can be completely written in the assigned NAND partition. > > With the new flag the semantics change, as soon a panic write happened, > the flag will stay and *all* future operates will take the polling/pio path. > Yes that is true. > IMHO this is fine since the kernel cannot recover from an oops. > But just to make sure we all get this. :-) > An alternative would be to block all further non-panic writes. Capturing the panic writes into an mtd device reliably is what the low level driver is trying to do.If there are non panic writes they will use pio and interrupt polling as well in this case. > -- > Thanks, > //richard Thanks Kamal
Richard, You have any other review comments/concerns with this patch, if not can you please sign off on it. Thanks Kamal On Fri, May 17, 2019 at 7:56 AM Kamal Dasu <kdasu.kdev@gmail.com> wrote: > > On Fri, May 17, 2019 at 4:12 AM Richard Weinberger > <richard.weinberger@gmail.com> wrote: > > > > On Thu, May 16, 2019 at 6:42 PM Kamal Dasu <kdasu.kdev@gmail.com> wrote: > > > > > > If mtd_oops is in progress, switch to polling during NAND command > > > completion instead of relying on DMA/interrupts so that the mtd_oops > > > buffer can be completely written in the assigned NAND partition. > > > > With the new flag the semantics change, as soon a panic write happened, > > the flag will stay and *all* future operates will take the polling/pio path. > > > > Yes that is true. > > > IMHO this is fine since the kernel cannot recover from an oops. > > But just to make sure we all get this. :-) > > An alternative would be to block all further non-panic writes. > > Capturing the panic writes into an mtd device reliably is what the low > level driver is trying to do.If there are non panic writes they will > use pio and interrupt polling as well in this case. > > > -- > > Thanks, > > //richard > > Thanks > Kamal
On Tue, Jun 11, 2019 at 10:03 PM Kamal Dasu <kdasu.kdev@gmail.com> wrote: > > Richard, > > You have any other review comments/concerns with this patch, if not > can you please sign off on it. I'm fine with that approach. I hoped to get some input from other MTD folks too :-(
Hi Richard, Richard Weinberger <richard.weinberger@gmail.com> wrote on Tue, 11 Jun 2019 22:16:36 +0200: > On Tue, Jun 11, 2019 at 10:03 PM Kamal Dasu <kdasu.kdev@gmail.com> wrote: > > > > Richard, > > > > You have any other review comments/concerns with this patch, if not > > can you please sign off on it. > > I'm fine with that approach. > I hoped to get some input from other MTD folks too :-( > Sorry for my late answer but yes, I am totally fine with this approach. I'll carry this through the nand branch. Thanks, Miquèl
diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c index ce0b8ff..dca8eb8 100644 --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c @@ -159,6 +159,7 @@ struct brcmnand_controller { u32 nand_cs_nand_xor; u32 corr_stat_threshold; u32 flash_dma_mode; + bool pio_poll_mode; }; struct brcmnand_cfg { @@ -823,6 +824,20 @@ static inline bool has_flash_dma(struct brcmnand_controller *ctrl) return ctrl->flash_dma_base; } +static inline void disable_ctrl_irqs(struct brcmnand_controller *ctrl) +{ + if (ctrl->pio_poll_mode) + return; + + if (has_flash_dma(ctrl)) { + ctrl->flash_dma_base = 0; + disable_irq(ctrl->dma_irq); + } + + disable_irq(ctrl->irq); + ctrl->pio_poll_mode = true; +} + static inline bool flash_dma_buf_ok(const void *buf) { return buf && !is_vmalloc_addr(buf) && @@ -1237,15 +1252,42 @@ static void brcmnand_cmd_ctrl(struct nand_chip *chip, int dat, /* intentionally left blank */ } +static bool brcmstb_nand_wait_for_completion(struct nand_chip *chip) +{ + struct brcmnand_host *host = nand_get_controller_data(chip); + struct brcmnand_controller *ctrl = host->ctrl; + struct mtd_info *mtd = nand_to_mtd(chip); + bool err = false; + int sts; + + if (mtd->oops_panic_write) { + /* switch to interrupt polling and PIO mode */ + disable_ctrl_irqs(ctrl); + sts = bcmnand_ctrl_poll_status(ctrl, NAND_CTRL_RDY, + NAND_CTRL_RDY, 0); + err = (sts < 0) ? true : false; + } else { + unsigned long timeo = msecs_to_jiffies( + NAND_POLL_STATUS_TIMEOUT_MS); + /* wait for completion interrupt */ + sts = wait_for_completion_timeout(&ctrl->done, timeo); + err = (sts <= 0) ? true : false; + } + + return err; +} + static int brcmnand_waitfunc(struct nand_chip *chip) { struct brcmnand_host *host = nand_get_controller_data(chip); struct brcmnand_controller *ctrl = host->ctrl; - unsigned long timeo = msecs_to_jiffies(100); + bool err = false; dev_dbg(ctrl->dev, "wait on native cmd %d\n", ctrl->cmd_pending); - if (ctrl->cmd_pending && - wait_for_completion_timeout(&ctrl->done, timeo) <= 0) { + if (ctrl->cmd_pending) + err = brcmstb_nand_wait_for_completion(chip); + + if (err) { u32 cmd = brcmnand_read_reg(ctrl, BRCMNAND_CMD_START) >> brcmnand_cmd_shift(ctrl);
If mtd_oops is in progress, switch to polling during NAND command completion instead of relying on DMA/interrupts so that the mtd_oops buffer can be completely written in the assigned NAND partition. Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com> --- drivers/mtd/nand/raw/brcmnand/brcmnand.c | 48 ++++++++++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 3 deletions(-)