* [PATCH v2] mtd: bcm47xxnflash: Add error handling for bcm47xxnflash_ops_bcm4706_ctl_cmd()
@ 2025-04-05 14:33 ` Wentao Liang
0 siblings, 0 replies; 4+ messages in thread
From: Wentao Liang @ 2025-04-05 14:33 UTC (permalink / raw)
To: miquel.raynal, richard, vigneshr; +Cc: linux-mtd, linux-kernel, Wentao Liang
The bcm47xxnflash_ops_bcm4706_cmd_ctrl() calls
bcm47xxnflash_ops_bcm4706_ctl_cmd() without printing error message. A
proper implementation can be found in bcm47xxnflash_ops_bcm4706_write().
Add error log to the bcm47xxnflash_ops_bcm4706_ctl_cmd() to prevent
silent failure.
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c b/drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c
index 6487dfc64258..c89129588bb6 100644
--- a/drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c
+++ b/drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c
@@ -182,7 +182,8 @@ static void bcm47xxnflash_ops_bcm4706_cmd_ctrl(struct nand_chip *nand_chip,
if (cmd != NAND_CMD_RESET)
code |= NCTL_CSA;
- bcm47xxnflash_ops_bcm4706_ctl_cmd(b47n->cc, code);
+ if (bcm47xxnflash_ops_bcm4706_ctl_cmd(b47n->cc, code))
+ pr_err("%s ctl_cmd didn't work!\n", __func__);
}
/* Default nand_select_chip calls cmd_ctrl, which is not used in BCM4706 */
--
2.42.0.windows.2
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH v2] mtd: bcm47xxnflash: Add error handling for bcm47xxnflash_ops_bcm4706_ctl_cmd()
@ 2025-04-05 14:33 ` Wentao Liang
0 siblings, 0 replies; 4+ messages in thread
From: Wentao Liang @ 2025-04-05 14:33 UTC (permalink / raw)
To: miquel.raynal, richard, vigneshr; +Cc: linux-mtd, linux-kernel, Wentao Liang
The bcm47xxnflash_ops_bcm4706_cmd_ctrl() calls
bcm47xxnflash_ops_bcm4706_ctl_cmd() without printing error message. A
proper implementation can be found in bcm47xxnflash_ops_bcm4706_write().
Add error log to the bcm47xxnflash_ops_bcm4706_ctl_cmd() to prevent
silent failure.
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c b/drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c
index 6487dfc64258..c89129588bb6 100644
--- a/drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c
+++ b/drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c
@@ -182,7 +182,8 @@ static void bcm47xxnflash_ops_bcm4706_cmd_ctrl(struct nand_chip *nand_chip,
if (cmd != NAND_CMD_RESET)
code |= NCTL_CSA;
- bcm47xxnflash_ops_bcm4706_ctl_cmd(b47n->cc, code);
+ if (bcm47xxnflash_ops_bcm4706_ctl_cmd(b47n->cc, code))
+ pr_err("%s ctl_cmd didn't work!\n", __func__);
}
/* Default nand_select_chip calls cmd_ctrl, which is not used in BCM4706 */
--
2.42.0.windows.2
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v2] mtd: bcm47xxnflash: Add error handling for bcm47xxnflash_ops_bcm4706_ctl_cmd()
2025-04-05 14:33 ` Wentao Liang
@ 2025-04-07 6:51 ` Miquel Raynal
-1 siblings, 0 replies; 4+ messages in thread
From: Miquel Raynal @ 2025-04-07 6:51 UTC (permalink / raw)
To: Wentao Liang; +Cc: richard, vigneshr, linux-mtd, linux-kernel
Hello Liang,
On 05/04/2025 at 22:33:47 +08, Wentao Liang <vulab@iscas.ac.cn> wrote:
> The bcm47xxnflash_ops_bcm4706_cmd_ctrl() calls
> bcm47xxnflash_ops_bcm4706_ctl_cmd() without printing error message. A
> proper implementation can be found in
> bcm47xxnflash_ops_bcm4706_write().
This is a problem with cmd_ctrl, but this interface has been deprecated
~5y ago and people encouraged to transition to ->exec_op(). Did you
personally experience issues with that or is this just code analysis? If
you are actually using this driver, I'd highly suggest to update the
driver.
> Add error log to the bcm47xxnflash_ops_bcm4706_ctl_cmd() to prevent
> silent failure.
>
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> ---
> drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c b/drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c
> index 6487dfc64258..c89129588bb6 100644
> --- a/drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c
> +++ b/drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c
> @@ -182,7 +182,8 @@ static void bcm47xxnflash_ops_bcm4706_cmd_ctrl(struct nand_chip *nand_chip,
> if (cmd != NAND_CMD_RESET)
> code |= NCTL_CSA;
>
> - bcm47xxnflash_ops_bcm4706_ctl_cmd(b47n->cc, code);
> + if (bcm47xxnflash_ops_bcm4706_ctl_cmd(b47n->cc, code))
> + pr_err("%s ctl_cmd didn't work!\n", __func__);
dev_err() is probably better
__func__ is not needed
If we want to make it a bit usable, I'd say that the return code shall
be there as well.
> }
>
> /* Default nand_select_chip calls cmd_ctrl, which is not used in BCM4706 */
Thanks,
Miquèl
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2] mtd: bcm47xxnflash: Add error handling for bcm47xxnflash_ops_bcm4706_ctl_cmd()
@ 2025-04-07 6:51 ` Miquel Raynal
0 siblings, 0 replies; 4+ messages in thread
From: Miquel Raynal @ 2025-04-07 6:51 UTC (permalink / raw)
To: Wentao Liang; +Cc: richard, vigneshr, linux-mtd, linux-kernel
Hello Liang,
On 05/04/2025 at 22:33:47 +08, Wentao Liang <vulab@iscas.ac.cn> wrote:
> The bcm47xxnflash_ops_bcm4706_cmd_ctrl() calls
> bcm47xxnflash_ops_bcm4706_ctl_cmd() without printing error message. A
> proper implementation can be found in
> bcm47xxnflash_ops_bcm4706_write().
This is a problem with cmd_ctrl, but this interface has been deprecated
~5y ago and people encouraged to transition to ->exec_op(). Did you
personally experience issues with that or is this just code analysis? If
you are actually using this driver, I'd highly suggest to update the
driver.
> Add error log to the bcm47xxnflash_ops_bcm4706_ctl_cmd() to prevent
> silent failure.
>
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> ---
> drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c b/drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c
> index 6487dfc64258..c89129588bb6 100644
> --- a/drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c
> +++ b/drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c
> @@ -182,7 +182,8 @@ static void bcm47xxnflash_ops_bcm4706_cmd_ctrl(struct nand_chip *nand_chip,
> if (cmd != NAND_CMD_RESET)
> code |= NCTL_CSA;
>
> - bcm47xxnflash_ops_bcm4706_ctl_cmd(b47n->cc, code);
> + if (bcm47xxnflash_ops_bcm4706_ctl_cmd(b47n->cc, code))
> + pr_err("%s ctl_cmd didn't work!\n", __func__);
dev_err() is probably better
__func__ is not needed
If we want to make it a bit usable, I'd say that the return code shall
be there as well.
> }
>
> /* Default nand_select_chip calls cmd_ctrl, which is not used in BCM4706 */
Thanks,
Miquèl
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-04-07 6:58 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-05 14:33 [PATCH v2] mtd: bcm47xxnflash: Add error handling for bcm47xxnflash_ops_bcm4706_ctl_cmd() Wentao Liang
2025-04-05 14:33 ` Wentao Liang
2025-04-07 6:51 ` Miquel Raynal
2025-04-07 6:51 ` Miquel Raynal
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.