linux-kernel - Re: [PATCH v2] mtd: spi-nor: Improve reporting for software reset failures

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20231024091943.GD3208943@black.fi.intel.com>
Date:   Tue, 24 Oct 2023 12:19:43 +0300
From:   Mika Westerberg <mika.westerberg@...ux.intel.com>
To:     Michael Walle <michael@...le.cc>
Cc:     AceLan Kao <acelan.kao@...onical.com>,
        Tudor Ambarus <tudor.ambarus@...aro.org>,
        Pratyush Yadav <pratyush@...nel.org>,
        Miquel Raynal <miquel.raynal@...tlin.com>,
        Richard Weinberger <richard@....at>,
        Vignesh Raghavendra <vigneshr@...com>,
        linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] mtd: spi-nor: Improve reporting for software reset
 failures

Hi,

On Tue, Oct 24, 2023 at 10:59:50AM +0200, Michael Walle wrote:
> [+Mika]
> 
> 
> > From: "Chia-Lin Kao (AceLan)" <acelan.kao@...onical.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
> > 
> > Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao@...onical.com>
> > ---
> >  drivers/mtd/spi-nor/core.c | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> > index 1b0c6770c14e..76920dbc568b 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 == -ENOTSUPP)
> 
> It bothers me that we use ENOTSUPP here. We should really use EOPNOTSUPP.
> The core uses EOPNOTSUPP everywhere except for the intel things.
> 
> Please have a look at changing that to EOPNOTSUPP. See also:
> https://lore.kernel.org/linux-mtd/85f9c462-c155-dc17-dc97-3254acfa55d2@microchip.com/

Makes sense.

> > +			dev_info(nor->dev, "Software reset enable command doesn't support:
> > %d\n", ret);
> 
> I'm not sure this is helpful. It's only the intel SPI controller which
> does magic things (instead of just issuing our commands). Mika, do you
> know wether your controller will do a reset on it's own? I presume so,
> because AFAIR you have some kind of high level controller which also does
> SFDP parsing and read opcode handling on their own.

Yes, that's right.