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

[Tudor Ambarus <tudor.ambarus@xxxxxxxxxxxxx>]

>>> 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

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/