linux-kernel - Re: [RESEND PATCH 1/2] mtd: spi-nor: add macros related to MICRON flash

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] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 20 Sep 2018 12:20:15 +0300
From:   Tudor Ambarus <tudor.ambarus@...rochip.com>
To:     Yogesh Narayan Gaur <yogeshnarayan.gaur@....com>,
        "linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
        "linux-spi@...r.kernel.org" <linux-spi@...r.kernel.org>
CC:     "boris.brezillon@...tlin.com" <boris.brezillon@...tlin.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "marek.vasut@...il.com" <marek.vasut@...il.com>,
        "frieder.schrempf@...eet.de" <frieder.schrempf@...eet.de>,
        "cyrille.pitchen@...ev4u.fr" <cyrille.pitchen@...ev4u.fr>,
        "computersforpeace@...il.com" <computersforpeace@...il.com>
Subject: Re: [RESEND PATCH 1/2] mtd: spi-nor: add macros related to MICRON
 flash


>>> diff --git a/drivers/mtd/spi-nor/spi-nor.c
>>> @@ -271,6 +271,7 @@ static inline int set_4byte(struct spi_nor *nor, const
>> struct flash_info *info,
>>>  	u8 cmd;
>>>
>>>  	switch (JEDEC_MFR(info)) {
>>> +	case SNOR_MFR_ST:
>>
>> We should mark switch cases where we are expecting to fall through, so that we
>> will be prepared when enabling -Wimplicit-fallthrough.
> 
> Please explain more, not able to get this comment. Sorry for ignorance.
> 
>>
>>>  	case SNOR_MFR_MICRON:
>>>  		/* Some Micron need WREN command; all will accept it */
>>>  		need_wren = true;

We can ignore my comment, it's not in the scope of this patch.

I wrongly suggested to do this:
case SNOR_MFR_ST:
	/* fall through */
case SNOR_MFR_MICRON:

in order to suppress the -Wimplicit-fallthrough warning that I thought it will
appear on gcc 7. The suggestion is wrong because the warning is not emitted for
empty case bodies, so it's not needed here.

However, when compiling with gcc 7 and -Wimplicit-fallthrough the following can
be seen:

drivers/mtd/spi-nor/spi-nor.c: In function ‘set_4byte’:
drivers/mtd/spi-nor/spi-nor.c:290:13: warning: this statement may fall through
[-Wimplicit-fallthrough=]
   need_wren = true;
   ~~~~~~~~~~^~~~~~
drivers/mtd/spi-nor/spi-nor.c:291:2: note: here
  case SNOR_MFR_MACRONIX:
  ^~~~

We should add a /* fall through */ comment after setting need_wren = true;, but
it's not in the scope of this patch.

Cheers,
ta