Re: [PATCH 1/2] mtd: spi-nor: core: move Spansion SR ready codes out of core

[<Tudor.Ambarus@microchip.com>]

On 3/1/21 9:08 PM, Pratyush Yadav wrote:
>> +     if (nor->manufacturer && nor->manufacturer->fixups &&
>> +         nor->manufacturer->fixups->sr_ready)
>> +             return nor->manufacturer->fixups->sr_ready(nor);
> I don't think nor->info->fixups is the correct place for this hook.
> Those should be about fixing up incorrect or missing information about
> the flash. It should instead be placed in nor->params.
> 

Pratyush is right.

> This eliminates the need for the call to
> nor->manufacturer->fixups->sr_ready. Now you can simply call
> nor->params->sr_ready(). The fixup hooks will take care of populating it
> with the correct function based on the flash or manufacturer.
> 

For operations that require a different sequence of opcodes issued,
or different checks needed, I would use something like from below.

struct spi_nor_ops {
	int (*ready)(struct spi_nor *nor);
};

struct spi_nor_flash_parameter {
	...
	struct spi_nor_erase_map        erase_map;
	struct spi_nor_ops		ops;
	...
};

You'll have to get rid of the SNOR_F_USE_FSR, SNOR_F_READY_XSR_RDY and
SNOR_F_USE_CLSR. You'll init the default spi_nor_ready() in
spi_nor_info_init_params(), and you will overwrite it for the 3 cases
from above in the manufacturer or flash default_init() hook.
You'll then use across the core nor->params->ops.ready().

Not related to your patch, but related with the overall architecture:
there is another case, where the just the opcode differs, where manufacturers
use different hex opcodes for the same command (ex. some manufacturers use 0x35
for reading CR, others use 0x15). That will require a struct spi_nor_opcodes.

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