Re: [PATCH v3 3/5] mtd: rawnand: brcmnand: Fix crash during the panic_write

[Miquel Raynal]

Hi William,

william.zhang@xxxxxxxxxxxx wrote on Tue, 4 Jul 2023 17:40:03 -0700:

> Hi Miquel,
> 
> On 07/04/2023 08:26 AM, Miquel Raynal wrote:
> > Hi William,
> > 
> > william.zhang@xxxxxxxxxxxx wrote on Tue, 27 Jun 2023 12:37:36 -0700:
> >   
> >> During the panic write path to execute another nand command, if
> >> there is a pending command, we should wait for the command instead of
> >> calling BUG_ON so we don't crash while crashing.
> >>
> >> Fixes: 27c5b17cd1b1 ("mtd: nand: add NAND driver "library" for Broadcom STB NAND controller")  
> > 
> > The Fixes tag looks wrong.
> >   
> The brcmnand_send_cmd function and BUG_ON line were added by this commit and the function didn't changed much since then. Not sure why you think it is wrong?

Ok, the title of that commit let me think it was moving code rather
than adding it. Alright.

> >> Signed-off-by: William Zhang <william.zhang@xxxxxxxxxxxx>
> >> Reviewed-by: Florian Fainelli <florian.fainelli@xxxxxxxxxxxx>
> >> Reviewed-by: Kursad Oney <kursad.oney@xxxxxxxxxxxx>
> >> Reviewed-by: Kamal Dasu <kamal.dasu@xxxxxxxxxxxx>
> >> ---
> >>
> >> Changes in v3: None
> >> Changes in v2: None
> >>
> >>   drivers/mtd/nand/raw/brcmnand/brcmnand.c | 12 +++++++++++-
> >>   1 file changed, 11 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> >> index 37c2c7cfa00e..ea03104692bf 100644
> >> --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> >> +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> >> @@ -1608,7 +1608,17 @@ static void brcmnand_send_cmd(struct brcmnand_host *host, int cmd)  
> >>   >>   	dev_dbg(ctrl->dev, "send native cmd %d addr 0x%llx\n", cmd, cmd_addr);
> >>   >> -	BUG_ON(ctrl->cmd_pending != 0);  
> >> +	/*
> >> +	 * If we came here through _panic_write and there is a pending
> >> +	 * command, try to wait for it. If it times out, rather than
> >> +	 * hitting BUG_ON, just return so we don't crash while crashing.
> >> +	 */
> >> +	if (oops_in_progress) {
> >> +		if (ctrl->cmd_pending &&
> >> +			bcmnand_ctrl_poll_status(ctrl, NAND_CTRL_RDY, NAND_CTRL_RDY, 0))
> >> +			return;
> >> +	} else
> >> +		BUG_ON(ctrl->cmd_pending != 0);
> >>   	ctrl->cmd_pending = cmd;  
> >>   >>   	ret = bcmnand_ctrl_poll_status(ctrl, NAND_CTRL_RDY, NAND_CTRL_RDY, 0);  
> > 
> > 
> > Thanks,
> > Miquèl
> >   


Thanks,
Miquèl