| Message ID | 20230804075717.3336709-1-ruanjinjie@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | [-next] mtd: rawnand: brcmnand: Remove unnecessary ternary operators | expand |
Hi Ruan, ruanjinjie@huawei.com wrote on Fri, 4 Aug 2023 15:57:17 +0800: > Ther are a little ternary operators, the true or false judgement > of which is unnecessary in C language semantics. > > Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com> > --- > drivers/mtd/nand/raw/brcmnand/brcmnand.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c > index 03764b589ec5..dc66590eb4cf 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; This function returns a negative errno, please propagate the error instead upon error. > } 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 <= 0; The < 0 case does not exist so instead you can return -ETIMEDOUT if (!sts) and zero otherwise. > } > > return err; Thanks, Miquèl
On 2023/8/4 21:44, Miquel Raynal wrote: > Hi Ruan, > > ruanjinjie@huawei.com wrote on Fri, 4 Aug 2023 15:57:17 +0800: > >> Ther are a little ternary operators, the true or false judgement >> of which is unnecessary in C language semantics. >> >> Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com> >> --- >> drivers/mtd/nand/raw/brcmnand/brcmnand.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c >> index 03764b589ec5..dc66590eb4cf 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; > > This function returns a negative errno, please propagate the error > instead upon error. Yes, the negative errno should be propagated, instead of true or 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; >> + err = sts <= 0; > > The < 0 case does not exist so instead you can return -ETIMEDOUT if > (!sts) and zero otherwise. Right! it can not < 0. I'' fix it in v2. > >> } >> >> return err; > > > Thanks, > Miquèl
diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c index 03764b589ec5..dc66590eb4cf 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 <= 0; } return err;
Ther are a little ternary operators, the true or false judgement of which is unnecessary in C language semantics. Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com> --- drivers/mtd/nand/raw/brcmnand/brcmnand.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)