| Message ID | 20231025030501.490355-1-acelan.kao@canonical.com |
|---|---|
| State | Superseded |
| Delegated to: | Ambarus Tudor |
| Headers | show |
| Series | [v3] mtd: spi-nor: Improve reporting for software reset failures | expand |
>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) <acelan.kao@canonical.com> >--- > 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);
Hi, On Wed, Oct 25, 2023 at 11:05:01AM +0800, AceLan Kao wrote: > From: "Chia-Lin Kao (AceLan)" <acelan.kao@canonical.com> > > 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.
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);