2023-10-25 03:05:31

by AceLan Kao

Subject: [PATCH v3] mtd: spi-nor: Improve reporting for software reset failures

From: "Chia-Lin Kao (AceLan)" <[email protected]>

When the software reset command isn't supported, we now report it
as an informational message(dev_info) instead of a warning(dev_warn).
This adjustment helps avoid unnecessary alarm and confusion regarding
software reset capabilities.

v2. only lower the priority for the not supported failure
v3. replace ENOTSUPP with EOPNOTSUPP and check the first command only

Signed-off-by: Chia-Lin Kao (AceLan) <[email protected]>
---
drivers/mtd/spi-nor/core.c | 5 ++++-
drivers/spi/spi-mem.c | 2 +-
2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 1b0c6770c14e..42e52af76289 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -3252,7 +3252,10 @@ static void spi_nor_soft_reset(struct spi_nor *nor)

ret = spi_mem_exec_op(nor->spimem, &op);
if (ret) {
- dev_warn(nor->dev, "Software reset failed: %d\n", ret);
+ if (ret == -EOPNOTSUPP)
+ dev_info(nor->dev, "Software reset enable command doesn't support: %d\n", ret);
+ else
+ dev_warn(nor->dev, "Software reset failed: %d\n", ret);
return;
}

diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
index edd7430d4c05..93b77ac0b798 100644
--- a/drivers/spi/spi-mem.c
+++ b/drivers/spi/spi-mem.c
@@ -323,7 +323,7 @@ int spi_mem_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
return ret;

if (!spi_mem_internal_supports_op(mem, op))
- return -ENOTSUPP;
+ return -EOPNOTSUPP;

if (ctlr->mem_ops && ctlr->mem_ops->exec_op && !spi_get_csgpiod(mem->spi, 0)) {
ret = spi_mem_access_start(mem);
--
2.34.1

2023-10-25 04:26:22

by Michael Walle

Subject: Re: [PATCH v3] mtd: spi-nor: Improve reporting for software reset failures

>When the software reset command isn't supported, we now report it
>as an informational message(dev_info) instead of a warning(dev_warn).
>This adjustment helps avoid unnecessary alarm and confusion regarding
>software reset capabilities.
>
>v2. only lower the priority for the not supported failure
>v3. replace ENOTSUPP with EOPNOTSUPP and check the first command only
>
>Signed-off-by: Chia-Lin Kao (AceLan) <[email protected]>
>---
> drivers/mtd/spi-nor/core.c | 5 ++++-
> drivers/spi/spi-mem.c | 2 +-
> 2 files changed, 5 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
>index 1b0c6770c14e..42e52af76289 100644
>--- a/drivers/mtd/spi-nor/core.c
>+++ b/drivers/mtd/spi-nor/core.c
>@@ -3252,7 +3252,10 @@ static void spi_nor_soft_reset(struct spi_nor *nor)
>
> ret = spi_mem_exec_op(nor->spimem, &op);
> if (ret) {
>- dev_warn(nor->dev, "Software reset failed: %d\n", ret);
>+ if (ret == -EOPNOTSUPP)
>+ dev_info(nor->dev, "Software reset enable command doesn't support: %d\n", ret);

As mentioned in the previous version, this doesn't add any useful information. Please just drop it. That is, just guard the current dev_warn() with "if ret! = - EOPNOTSUPP".

>+ else
>+ dev_warn(nor->dev, "Software reset failed: %d\n", ret);
> return;
> }
>
>diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
>index edd7430d4c05..93b77ac0b798 100644
>--- a/drivers/spi/spi-mem.c
>+++ b/drivers/spi/spi-mem.c
>@@ -323,7 +323,7 @@ int spi_mem_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
> return ret;
>
> if (!spi_mem_internal_supports_op(mem, op))
>- return -ENOTSUPP;
>+ return -EOPNOTSUPP;

This should be an own patch and you'll have to fix current users of it. For example, there is at least one in spi-nor core. Probably more.

-michael

>
> if (ctlr->mem_ops && ctlr->mem_ops->exec_op && !spi_get_csgpiod(mem->spi, 0)) {
> ret = spi_mem_access_start(mem);

2023-10-25 05:05:24

by Mika Westerberg

Subject: Re: [PATCH v3] mtd: spi-nor: Improve reporting for software reset failures

Hi,

On Wed, Oct 25, 2023 at 11:05:01AM +0800, AceLan Kao wrote:
> From: "Chia-Lin Kao (AceLan)" <[email protected]>
>
> When the software reset command isn't supported, we now report it
> as an informational message(dev_info) instead of a warning(dev_warn).
> This adjustment helps avoid unnecessary alarm and confusion regarding
> software reset capabilities.
>
> v2. only lower the priority for the not supported failure
> v3. replace ENOTSUPP with EOPNOTSUPP and check the first command only

In addition to Michael's comments, please put this version information
below the '---' line.