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

[Vignesh Raghavendra]

On 24/03/20 8:11 pm, Wolfgang Denk wrote:
> Dear Vignesh,
> 
> In message <05694b0e-50a1-de5d-25d8-0444a2caeff3 at ti.com> you wrote:
>>
>> Aim of spi-nor-tiny.c is to have a tiny stack that can be used in
>> SPL/TPL or on resource constraint boards to only support _reading_ from
>> the flash. So tiny stack would be subset of spi-nor-core.
> 
> I fully understand this goal.
> 
>> There are few options here:
>> One is to have single driver and hide things that are not required for
>> tiny stack under #ifdef. But this makes code harder to read and modify
> 
> #ifdef is one way to implement conditioan code, plain C code like
> 
> 	if (IS_ENABLED(CONFIG_<something)) {
> 		...
> 	}
> 
> is another, usually much cleaner.
> 
>> Second option, is to factor out common functions into a separate file as
>> a library. This would avoid ifdef'ry. But whenever a new feature is
>> added that would add to the size of these common functions, it would be
>> probably mean moving it out of common library and into individual
>> stacks. This may need to unnecessary code churn whenever a new feature
>> is added.
> 
> This is all speculation, and experience says that this risks and
> disadvantages of duplicated code are much higher.
> 
>> So, suggestion was to add a parallel tiny stack (which was supposed to
>> plug into tiny read only MTD stack) that only supports reading from
>> flash. This would mean that new features can be freely added to
>> spi-nor-core.c without worrying about bloating SPL for older devices.
> 
> Yes, and the result is that you have two different implementations
> that are out of sync from day one after you created them, bugs get
> fixed here but no there, support for new chips same, etc.
> 

I fully understand your concerns and will work on unifying the two
stacks with IS_ENABLED() macro so that there will still be a tiny stack
with same memory footprint.

But I want to state that the differences currently in spi-nor-tiny.c vs
spi-nor-core.c are intentional. I don't think there have been any fixes
in the main code missing from tiny stack.
Features such as Octal mode and other stuff have been intentionally kept
out of spi-nor-tiny to avoid code size increase.

Appreciate all the suggestions!

Regards
Vignesh

[...]