[PATCH v2 1/2] spi: call WATCHDOG_RESET() in spi_nor_wait_till_ready_with_timeout()

[Wolfgang Denk]

Dear Vignesh,

In message <20200320101448.10714-1-rasmus.villemoes at 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

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Common sense and a sense of humor  are  the  same  thing,  moving  at
different speeds.  A sense of humor is just common sense, dancing.
                                                        - Clive James