| Message ID | 20230808032943.3890545-1-ruanjinjie@huawei.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [-next,v3,RESEND] mtd: rawnand: Propagate error and simplify ternary operators for brcmstb_nand_wait_for_completion() | expand |
On 08/07/2023 08:29 PM, 'Ruan Jinjie' via BCM-KERNEL-FEEDBACK-LIST,PDL wrote: > As bcmnand_ctrl_poll_status() return negative errno, so return true if > sts < 0. The < 0 case does not exist for wait_for_completion_timeout(), > so return true if sts = 0 and zero otherwise. Both of the true return > of them can be considered as a -ETIMEDOUT err, so return -ETIMEDOUT > if err is true to propagate err from its caller. > > Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com> > --- > v3: > - Update the commit title and message. > - Do not change the return type from bool to int. > - Return -ETIMEDOUT if err is true. > v2: > - Update the commit title and message. > - Propagate the error instead of upon error. > --- > drivers/mtd/nand/raw/brcmnand/brcmnand.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c > index 03764b589ec5..440bef477930 100644 > --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c > +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c > @@ -1666,13 +1666,13 @@ static bool brcmstb_nand_wait_for_completion(struct nand_chip *chip) > disable_ctrl_irqs(ctrl); > sts = bcmnand_ctrl_poll_status(ctrl, NAND_CTRL_RDY, > NAND_CTRL_RDY, 0); > - err = (sts < 0) ? true : false; > + err = sts < 0; > } 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; > + err = !sts; > } > > return err; > @@ -1688,6 +1688,7 @@ static int brcmnand_waitfunc(struct nand_chip *chip) > if (ctrl->cmd_pending) > err = brcmstb_nand_wait_for_completion(chip); > > + ctrl->cmd_pending = 0; > if (err) { > u32 cmd = brcmnand_read_reg(ctrl, BRCMNAND_CMD_START) > >> brcmnand_cmd_shift(ctrl); > @@ -1696,8 +1697,8 @@ static int brcmnand_waitfunc(struct nand_chip *chip) > "timeout waiting for command %#02x\n", cmd); > dev_err_ratelimited(ctrl->dev, "intfc status %08x\n", > brcmnand_read_reg(ctrl, BRCMNAND_INTFC_STATUS)); > + return -ETIMEDOUT; > } > - ctrl->cmd_pending = 0; > return brcmnand_read_reg(ctrl, BRCMNAND_INTFC_STATUS) & > INTFC_FLASH_STATUS; > } > Reviewed-by: William Zhang <william.zhang@broadcom.com>
On Tue, 2023-08-08 at 03:29:43 UTC, Ruan Jinjie wrote: > As bcmnand_ctrl_poll_status() return negative errno, so return true if > sts < 0. The < 0 case does not exist for wait_for_completion_timeout(), > so return true if sts = 0 and zero otherwise. Both of the true return > of them can be considered as a -ETIMEDOUT err, so return -ETIMEDOUT > if err is true to propagate err from its caller. > > Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com> > Reviewed-by: William Zhang <william.zhang@broadcom.com> Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks. Miquel
diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c index 03764b589ec5..440bef477930 100644 --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c @@ -1666,13 +1666,13 @@ static bool brcmstb_nand_wait_for_completion(struct nand_chip *chip) disable_ctrl_irqs(ctrl); sts = bcmnand_ctrl_poll_status(ctrl, NAND_CTRL_RDY, NAND_CTRL_RDY, 0); - err = (sts < 0) ? true : false; + err = sts < 0; } 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; + err = !sts; } return err; @@ -1688,6 +1688,7 @@ static int brcmnand_waitfunc(struct nand_chip *chip) if (ctrl->cmd_pending) err = brcmstb_nand_wait_for_completion(chip); + ctrl->cmd_pending = 0; if (err) { u32 cmd = brcmnand_read_reg(ctrl, BRCMNAND_CMD_START) >> brcmnand_cmd_shift(ctrl); @@ -1696,8 +1697,8 @@ static int brcmnand_waitfunc(struct nand_chip *chip) "timeout waiting for command %#02x\n", cmd); dev_err_ratelimited(ctrl->dev, "intfc status %08x\n", brcmnand_read_reg(ctrl, BRCMNAND_INTFC_STATUS)); + return -ETIMEDOUT; } - ctrl->cmd_pending = 0; return brcmnand_read_reg(ctrl, BRCMNAND_INTFC_STATUS) & INTFC_FLASH_STATUS; }
As bcmnand_ctrl_poll_status() return negative errno, so return true if sts < 0. The < 0 case does not exist for wait_for_completion_timeout(), so return true if sts = 0 and zero otherwise. Both of the true return of them can be considered as a -ETIMEDOUT err, so return -ETIMEDOUT if err is true to propagate err from its caller. Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com> --- v3: - Update the commit title and message. - Do not change the return type from bool to int. - Return -ETIMEDOUT if err is true. v2: - Update the commit title and message. - Propagate the error instead of upon error. --- drivers/mtd/nand/raw/brcmnand/brcmnand.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)