[PATCH 0/3] An alternative to SPI NAND

[Qi Wang 王起 (qiwang)]

Hi Ezequiel,

On 01/08/2015 11:27 AM, Ezequiel Garcia wrote:
>
>Hi Qi Wang,
>
>On 01/07/2015 11:45 PM, Qi Wang 王起 (qiwang) wrote:
>> Hi Brian,
>>
>> On Thu, Jan 08, 2015 at 9:03:24AM +0000, Brian Norris wrote:
>>>
>>> On Thu, Jan 08, 2015 at 12:47:24AM +0000, Peter Pan 潘栋 (peterpandong)
>>> wrote:
>>>> Documentation/devicetree/bindings/mtd/spi-nand.txt |   22 +
>>>> drivers/mtd/Kconfig                                |    2 +
>>>> drivers/mtd/Makefile                               |    1 +
>>>> drivers/mtd/spi-nand/Kconfig                       |    7 +
>>>> drivers/mtd/spi-nand/Makefile                      |    3 +
>>>> drivers/mtd/spi-nand/spi-nand-base.c               | 2034
>>> ++++++++++++++++++++
>>>> drivers/mtd/spi-nand/spi-nand-bbt.c                | 1279 ++++++++++++
>>>
>>> I can already tell by the diffstat that I don't like this. We probably
>>> don't need 3000 new lines of code for this, but we especially don't want
>>> to duplicate nand_bbt.c. It won't take a lot of work to augment
>>> nand_bbt.c to make it shareable. (I can whip that patch up if needed.)
>>
>> Yes, I agree with you, Nand_bbt.c do can be shared by Parallel NAND and
>> SPI NAND. Actually, we are working at this now. Will send patches to you
>> Once we finished it.
>>
>
>Thanks for the quick submission!
>
>However, Brian is right, this code duplication is a no go.
>
>Perhaps a more valid approach would be to first identify the code that
>needs to be shared in nand_bbt.c and nand_base.c, and export those
>symbols (or maybe do the required refactor).

Yes, I agree Brian's suggestion in another mail. 

" The BBT code is something we definitely want to share, but it's actually
not very closely tied to nand_base.c, and it looks pretty easy to adapt
to any MTD that implements mtd_read_oob()/mtd_write_oob(). We'd just
need to parameterize a few relevant device details into a new nand_bbt
struct, rather than using struct nand_chip directly."

To abstract a new nand_bbt struct instead of nand_chip to make SPI NAND 
and parallel NAND can share nand_bbt.c file, I already begin to work on 
this.

For code shared in nand_base.c, I agree it would be better if we can find
a good method to share nand_base.c code between spi nand and parallel nand.
But frankly speaking, I'm not satisfied for the remap command method. This
method make code difficult to maintain when SPI NAND and Parallel NAND 
evolve much differently in the future.

Take some example, 
If one new command (cache operation, multiple plane operation) implemented 
in parallel NAND code, and is used in nand_read or nand_write, that will 
cause maintainer to modify SPI NAND code to remap this new command, though 
this modification probably could be slight. That means modification on 
Parallel NAND flash need to consider SPI NAND as well. 

How do you think about this?

For Peter Pan's patchset, if we do some modification to make nand_bbt.c to
make it shareable for Parallel and SPI NAND. The code line should be 2000.
I believe I can review this spi-nand-base.c to remove some redundant code
that may hundreds line. Is 1700 or 1800 code line is more acceptable?

Let me know your opinions.

>
>Then, separate the SPI NAND upper and lower logic (in a similar to my
>proposal, which I still consider turned out to be clean).
>
>These two things would lead to a simpler and smaller patchset. I also
>suggest to cut off everything that we don't utterly need on a first
>submission, so it's easier to review.
>--
>Ezequiel


--
Qi Wang