On 24/03/20 8:11 pm, Wolfgang Denk wrote:
Dear Vignesh,
In message 05694b0e-50a1-de5d-25d8-0444a2caeff3@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
[...]