2024-03-13 10:27:51

by Maxim Korotkov

[permalink] [raw]
Subject: [PATCH] mtd: rawnand: hynix: fixed typo

The function hynix_nand_rr_init() should probably return an error code.
Judging by the usage, it seems that the return code is passed up
the call stack.
Right now, it always returns 0 and the function hynix_nand_cleanup()
in hynix_nand_init() has never been called.

Found by RASU JSC and Linux Verification Center (linuxtesting.org)

Fixes: 626994e07480 ("mtd: nand: hynix: Add read-retry support for 1x nm MLC NANDs")

Signed-off-by: Maxim Korotkov <[email protected]>
---
drivers/mtd/nand/raw/nand_hynix.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/nand_hynix.c b/drivers/mtd/nand/raw/nand_hynix.c
index 39076735a3fb..9695f07b5eb2 100644
--- a/drivers/mtd/nand/raw/nand_hynix.c
+++ b/drivers/mtd/nand/raw/nand_hynix.c
@@ -402,7 +402,7 @@ static int hynix_nand_rr_init(struct nand_chip *chip)
if (ret)
pr_warn("failed to initialize read-retry infrastructure");

- return 0;
+ return ret;
}

static void hynix_nand_extract_oobsize(struct nand_chip *chip,
--
2.34.1



2024-03-15 11:23:25

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH] mtd: rawnand: hynix: fixed typo

Hi Maxim,

[email protected] wrote on Wed, 13 Mar 2024 13:27:20 +0300:

> The function hynix_nand_rr_init() should probably return an error code.
> Judging by the usage, it seems that the return code is passed up
> the call stack.
> Right now, it always returns 0 and the function hynix_nand_cleanup()
> in hynix_nand_init() has never been called.

Isn't it done like that on purpose? Read-retry is not a mandatory
feature.

> Found by RASU JSC and Linux Verification Center (linuxtesting.org)
>
> Fixes: 626994e07480 ("mtd: nand: hynix: Add read-retry support for 1x nm MLC NANDs")
>
> Signed-off-by: Maxim Korotkov <[email protected]>
> ---

Thanks,
Miquèl

2024-03-15 11:45:52

by Maxim Korotkov

[permalink] [raw]
Subject: Re: [PATCH] mtd: rawnand: hynix: fixed typo

Hi,

On 15.03.2024 14:23, Miquel Raynal wrote:
> Isn't it done like that on purpose? Read-retry is not a mandatory
> feature.

Works it as designed? I'm not sure about it.
I wouldn't rule it out, but it's a little weird that the
hynix_nand_rr_init() function
is used as if it can return an error but never returns.

Probably checking of value returned from hynix_nand_rr_init()
should be removed in hynix_nand_init()

Best regards, Max





2024-03-15 12:14:29

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH] mtd: rawnand: hynix: fixed typo

Hi Maxim,

[email protected] wrote on Fri, 15 Mar 2024 14:45:47 +0300:

> Hi,
>
> On 15.03.2024 14:23, Miquel Raynal wrote:
> > Isn't it done like that on purpose? Read-retry is not a mandatory
> > feature.
>
> Works it as designed? I'm not sure about it.
> I wouldn't rule it out, but it's a little weird that the hynix_nand_rr_init() function
> is used as if it can return an error but never returns.
>
> Probably checking of value returned from hynix_nand_rr_init()
> should be removed in hynix_nand_init()

It is indeed confusing. I'll take it.

Thanks,
Miquèl

2024-03-25 14:21:39

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH] mtd: rawnand: hynix: fixed typo

On Wed, 2024-03-13 at 10:27:20 UTC, Maxim Korotkov wrote:
> The function hynix_nand_rr_init() should probably return an error code.
> Judging by the usage, it seems that the return code is passed up
> the call stack.
> Right now, it always returns 0 and the function hynix_nand_cleanup()
> in hynix_nand_init() has never been called.
>
> Found by RASU JSC and Linux Verification Center (linuxtesting.org)
>
> Fixes: 626994e07480 ("mtd: nand: hynix: Add read-retry support for 1x nm MLC NANDs")
>
> Signed-off-by: Maxim Korotkov <[email protected]>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.

Miquel

2024-03-25 14:24:16

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH] mtd: rawnand: hynix: fixed typo

On Wed, 2024-03-13 at 10:27:20 UTC, Maxim Korotkov wrote:
> The function hynix_nand_rr_init() should probably return an error code.
> Judging by the usage, it seems that the return code is passed up
> the call stack.
> Right now, it always returns 0 and the function hynix_nand_cleanup()
> in hynix_nand_init() has never been called.
>
> Found by RASU JSC and Linux Verification Center (linuxtesting.org)
>
> Fixes: 626994e07480 ("mtd: nand: hynix: Add read-retry support for 1x nm MLC NANDs")
>
> Signed-off-by: Maxim Korotkov <[email protected]>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.

Miquel

2024-03-25 17:24:41

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH] mtd: rawnand: hynix: fixed typo

On Wed, Mar 13, 2024 at 01:27:20PM +0300, Maxim Korotkov wrote:
> The function hynix_nand_rr_init() should probably return an error code.
> Judging by the usage, it seems that the return code is passed up
> the call stack.
> Right now, it always returns 0 and the function hynix_nand_cleanup()
> in hynix_nand_init() has never been called.
>
> Found by RASU JSC and Linux Verification Center (linuxtesting.org)

> Fixes: 626994e07480 ("mtd: nand: hynix: Add read-retry support for 1x nm MLC NANDs")
>
> Signed-off-by: Maxim Korotkov <[email protected]>

FWIW, the tag block shouldn't have blank lines.

--
With Best Regards,
Andy Shevchenko