Dear Vignesh,
In message 20200320101448.10714-1-rasmus.villemoes@prevas.dk Rasmus Villemoes wrote:
I have a board for which doing "sf erase 0x100000 0x80000" consistently causes the external watchdog circuit to reset the board. Make sure to pet the watchdog during slow operations such as erasing or writing large areas of a spi nor flash.
...
drivers/mtd/spi/spi-nor-core.c | 2 ++ drivers/mtd/spi/spi-nor-tiny.c | 2 ++
Rasmus' patch triggers a few interesting questions about the SPI NOR code:
Is there any specific reason why spi-nor-core.c and spi-nor-tiny.c contain large parts of absolutely identical code?
All the functions
spi_nor_read_write_reg() spi_nor_read_reg() spi_nor_write_reg() spi_nor_read_data() mtd_to_spi_nor() spi_nor_convert_opcode() spi_nor_ready() spi_nor_wait_till_ready_with_timeout() spi_nor_wait_till_ready() macronix_quad_enable() spansion_read_cr_quad_enable() spi_nor_set_read_settings()
are absolutely identical;
functions
read_cr() write_sr() write_disable() set_4byte() spi_nor_read() write_sr_cr()
are mostly identical, but I wonder if the differences (like nor->write_reg() versus spi_nor_write_reg()) is indeed intended to save memory footprint or lack an update to later code?
Function
spi_nor_convert_3to4_read() spi_nor_set_4byte_opcodes()
looks like it has not been updated/synced for some time.
Function
spi_nor_sr_ready() spi_nor_fsr_ready()
is lacking error handling in spi-nor-tiny.c; and the rror handling code in spi-nor-core.c is also mostly duplicated a couple or times.
Function
spi_nor_read_id()
differs in "interesting" ways, i. e. we have
370 info = spi_nor_ids; 371 for (; info->sector_size != 0; info++) { 372 if (info->id_len) {
here, and
894 info = spi_nor_ids; 895 for (; info->name; info++) { 896 if (info->id_len) {
there, while all the rest is idential. Lack of synchronization?
Also the differences in
spi_nor_select_read()
make we wonder...
This extensive code duplication looks really painful and error prone to me.
Do you have any intentions to clean this up any time soon?
Best regards,
Wolfgang Denk