Re: [PATCH v3 08/23] mtd: spi-nor: winbond: Prepare introduction of W25QxxRV-Q/N parts

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hello,

>> +static const struct spi_nor_fixups winbond_nor_partname_fixups = {
>> +	.post_sfdp = winbond_nor_partname_post_sfdp_fixups,
>> +};
>> +
>> +static bool is_w25qxxrv(const struct spi_nor *nor)
>> +{
>> +	struct sfdp_header *sfdp_h = (struct sfdp_header *)nor->sfdp->dwords;
>
> nitpick, spi_nor_sfdp_get_header()?

Yup.

>> +	 * W25QxxRV chips re-use the same ID as the W25QxxJV family.
>> +	 *
>> +	 * Chips are very similar, W25QxxRV brings mostly performance and power
>> +	 * consumption improvements. The RV family does not require the multi
>> +	 * die fixup.
>> +	 *
>> +	 * They can be distinguished based on their SFDP minor revision:
>> +	 * W25QxxJV:        JESD216A, minor revision == 05h
>> +	 * W25Q512/01/02JV: JESD216B, minor revision == 06h
>> +	 * W25QxxRV:        JESD216F, minor revision >= 0Ah
>> +	 */
>> +	return sfdp_h->minor >= SFDP_JESD216F_MINOR;
>> +}
>> +
>> +static bool winbond_jv_match(const struct spi_nor *nor)
>> +{
>> +	return !nor->sfdp || !is_w25qxxrv(nor);
>
> So how do we know if nor->sfdp is already there for a given fixup.
> Without having looked at the code, there could potentially be fixups
> before SFDP is parsed (and the nor->sfdp is populated), right? Might
> be worth to be mentioned somewhere.

You are right, it is worth mentioning. In practice only default_init()
is too early.

Thanks,
Miquèl