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 <[email protected]>
---
drivers/mtd/nand/raw/brcmnand/brcmnand.c | 48 ++++++++++++++++++++++++++++++--
1 file changed, 45 insertions(+), 3 deletions(-)
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);
--
1.9.0.138.g2de3478
On Thu, May 16, 2019 at 6:42 PM Kamal Dasu <[email protected]> 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.
--
Thanks,
//richard
On Fri, May 17, 2019 at 4:12 AM Richard Weinberger
<[email protected]> wrote:
>
> On Thu, May 16, 2019 at 6:42 PM Kamal Dasu <[email protected]> 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 <[email protected]> wrote:
>
> On Fri, May 17, 2019 at 4:12 AM Richard Weinberger
> <[email protected]> wrote:
> >
> > On Thu, May 16, 2019 at 6:42 PM Kamal Dasu <[email protected]> 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 <[email protected]> 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 :-(
--
Thanks,
//richard
Hi Richard,
Richard Weinberger <[email protected]> wrote on Tue, 11 Jun
2019 22:16:36 +0200:
> On Tue, Jun 11, 2019 at 10:03 PM Kamal Dasu <[email protected]> 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