Re: [PATCH v4 1/3] mtd: spi-nor: Add the SNOR_F_4B_OPCODES flag

[<Tudor.Ambarus@xxxxxxxxxxxxx>]

On 12/05/2018 05:19 PM, Boris Brezillon wrote:
>>> @@ -3750,6 +3747,10 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
>>>  	if (info->flags & SPI_NOR_NO_FR)
>>>  		params.hwcaps.mask &= ~SNOR_HWCAPS_READ_FAST;
>>>  
>>> +	if (info->flags & SPI_NOR_4B_OPCODES ||
>>> +	    (JEDEC_MFR(info) == SNOR_MFR_SPANSION && mtd->size > SZ_16M))
>>> +		nor->flags |= SNOR_F_4B_OPCODES;
>>> +  
>> you are potentially overwriting the SNOR_F_4B_OPCODES that may be set in SFDP. I
>> suggest to set SNOR_F_4B_OPCODES flag inside of the "if (mtd->size > 0x1000000)"
>> block.
> Shouldn't we override this value anyway? I mean, I thought flash_info
> flags had precedence on the SFDP ones. Also, just because the flash is

I tend to say that we shouldn't. We have some "JEDEC knows better" attitude in
the code: we are overwriting platform ID if we find a different ID in sfdp, we
choose addr_width from SFDP even if set in info->addr_width, and we are
overwriting all the settings based on flash_info when sfdp parsing succeeds in
spi_nor_init_params().

> smaller than 16MB, doesn't mean it does not support 4B opcodes. We
> probably won't use the 4B opcodes in that case, but still.
> 

I agree that manufacturers have a sense of humor and this might be possible. But
there's no need to use 4B opcodes in this case, so a post_sfdp fixup will help
here too.

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