[PATCH -next v2] mtd: rawnand: Propagate error values for brcmstb_nand_wait_for_completion()

[William Zhang]

On 08/07/2023 02:42 AM, Ruan Jinjie wrote:
> 
> 
> On 2023/8/6 14:09, William Zhang wrote:
>> Hi Jinjie,
>>
>> On 08/04/2023 10:01 PM, 'Ruan Jinjie' via BCM-KERNEL-FEEDBACK-LIST,PDL
>> wrote:
>>> As bcmnand_ctrl_poll_status() return negative errno, and the < 0
>>> case does not exist for wait_for_completion_timeout(), so return
>>> -ETIMEDOUT if sts = 0 and zero otherwise. It is not sensible for
>>> brcmstb_nand_wait_for_completion() to return bool value, change it
>>> to int to propagate the error.
>>>
>>> Signed-off-by: Ruan Jinjie <ruanjinjie at huawei.com>
>>> ---
>>> v2:
>>> - Update the commit title and message.
>>> - Propagate the error instead of upon error.
>>> ---
>>>    drivers/mtd/nand/raw/brcmnand/brcmnand.c | 9 +++++----
>>>    1 file changed, 5 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
>>> b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
>>> index 03764b589ec5..25e5b15fbd6a 100644
>>> --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
>>> +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
>>> @@ -1653,12 +1653,12 @@ 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)
>>> +static int 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 err = 0;
>>>        int sts;
>>>          if (mtd->oops_panic_write || ctrl->irq < 0) {
>>> @@ -1666,13 +1666,14 @@ 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;
>>>        } 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;
>>> +        if (!sts)
>>> +            err = -ETIMEDOUT;
>>>        }
>>>          return err;
>>>
>> Sorry that I missed your first patch but the original goal of this
>> function is to return if time out happens. So you can simplify the
>> ternary operator but propagate -ETIMEDOUT from its caller
>> brcmnand_waitfunc instead.
> 
> So not change the return from bool to int? Just simplify ternary
> operator. 
Yes

> How to propagate -ETIMEDOUT to brcmstb_nand_wait_for_completion?
In the brcmnand_waitfunc,  if err is true, return -ETIMEDOUT. Otherwise, 
keep the original execution path.
I can submit a separate patch late for this if it is more than what you 
want to fix. Totally up to you.




-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4212 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://lists.infradead.org/pipermail/linux-mtd/attachments/20230807/367857a4/attachment-0001.p7s>