Re: [PATCH 02/11] mtd: nand_bbt: introduce BBT related data structure

[Peter Pan]

Hi Boris,

Firstly, sorry for late reply.

On Thu, May 5, 2016 at 4:33 AM, Boris Brezillon
<boris.brezillon@xxxxxxxxxxxxxxxxxx> wrote:
> Hi Peter,
>
> On Wed, 4 May 2016 09:36:05 +0800
> Peter Pan <peterpansjtu@xxxxxxxxx> wrote:
>
>> Hi Boris,
>>
>> On Tue, Apr 19, 2016 at 3:34 PM, Boris Brezillon
>> <boris.brezillon@xxxxxxxxxxxxxxxxxx> wrote:
>> > Hi Peter,
>> >
>> > On Tue, 19 Apr 2016 08:40:40 +0800
>> > Peter Pan <peterpansjtu@xxxxxxxxx> wrote:
>> >>
>> >> >
>> >> >> So it's true, it
>> >> >> should still be numchips in nand_bbt.c?  I just came out this question when
>> >> >> making v4. :)
>> >> >
>> >> > BTW, I have something for you [1]. I started to move things around to
>> >> > allow spinand and onenand layers to lie under drivers/mtd/nand/, and I
>> >> > wonder if we shouldn't do this move before reworking the nand_bbt code
>> >> > to make it generic.
>> >> > Note that this rework is not finished yet, but it gives a rough idea of
>> >> > what I'd like to see.
>> >>
>> >> I saw you also rework BBT in your git tree, which is a bit duplicate
>> >> with my BBT patch,
>> >> so should I continue my BBT patch by join part of your BBT rework code
>> >> or continue
>> >> your git tree ?
>> >
>> > Well, if you ask me what I'd prefer, it's clearly the 2nd solution.
>> > Note that my branch should just serve as a reference of what I expect,
>> > it just a pile of rework that should probably be reordered and cleaned
>> > up.
>> >
>> > Here's the sequencing I'd like to see:
>> >
>> > 1/ Move include/linux/mtd/nand.h into include/linux/mtd/rawnand.h and
>> >    move all files under drivers/mtd/nand/ into
>> >    drivers/mtd/nand/rawnand (including nand_bbt.c). This can be done in
>> >    several patches
>> >
>> > 2/ Add the generic nand layer (include/linux/mtd/nand.h and
>> >    drivers/mtd/nand/core.c). In my version I put everything in
>> >    include/linux/mtd/nand.h, but maybe we'll need a few functions to be
>> >    defined in drivers/mtd/nand/core.c.
>> >
>> > 3/ Create a rawnand_device structure inheriting from nand_device, and
>> >    then make nand_chip inherit from rawnand_device. Patch the
>> >    nand_base.c code to initialize all the nand_device fields properly,
>> >    so that we'll be ready to switch to the generic BBT code.
>> >
>> > 4/ Modify the nand_bbt.c code to make use of the generic NAND interface
>> >    instead of the MTD and rawnand one (this implies identifying all the
>> >    generic helpers you might need, and implementing them in
>> >    include/linux/mtd/nand.h or drivers/mtd/nand/core.c).
>> >
>> > 5/ Move drivers/mtd/nand/rawnand/nand_bbt.c into
>> >    drivers/mtd/nand/bbt.c
>> >
>> > 6/[optional] Implement your spinand layer in drivers/mtd/nand/spinand
>> >
>> > I know I'm asking a lot, especially given that you already spent a lot
>> > of time iterating on this BBT rework series. But your goal is to move
>> > mt29f driver out of staging, and you'll need to do the generic NAND
>> > layer to achieve that, so I'd really prefer having the BBT code use
>> > this generic layer instead of directly using the MTD API + an extra set
>> > of NAND specific structs (like the nand_chip_layout_info one).
>>
>> Yes, I want to upstreaming my SPI NAND frameworks and it's indeed better
>> to have a nand core. In fact, I already finished a SPI NAND framework with
>> the BBT patch I sent which is directly under MTD (don't have a NAND core layer).
>>
>> Actually, I'm interested in this NAND framework refining work. And I
>> know you already
>> gave a speech on ELC about this. But due to the resource limitation, I may not
>> to do all of the things. So how about I continue my BBT patch with
>> your NAND refining
>> ideas. I'll try to make the BBT patch compatible with the refining
>> work. What do you
>> think?
>
> The thing is, I'm not happy with these intermediate reworks, which in my
> opinion are adding more confusion and will make things even harder to
> rework afterward.
> You said you already developed your SPI NAND framework and it's not
> based on the generic NAND layer, which means you (or someone else) will
> have to migrate it to this approach at some point, and this extra work
> is kind of useless, especially since we seem to agree that the generic
> NAND layer is the way to go for SPI NAND (and other NAND based devices)
> support.
>
> Since I'm the one who pushed for this transition to an intermediate
> "NAND core" layer, I'm willing to help you with this task. I actually
> reworked my series [1] to move the BBT code in drivers/mtd/nand/bbt.c
> and move raw NAND code into drivers/mtd/nand/rawnand/ (still have to
> rework the commit logs, and test the implementation, but the different
> steps are there and we end-up with something clean in
> drivers/mtd/nand/).
>
> Could you help me debug this code and base your SPI NAND framework on
> top of it?

Yes I can. Actually I already clone your git tree and start to go
through your commits.
And I'll let you know when I have questions.

>
> Again, I'm sorry that you had to be the one supporting this transition,
> but I don't want to introduce any more quick-and-dirty hacks that we'll
> have to maintain until someone decides to tackle the real problem.

No sorry needed. I'd like to do the contribution.

Thanks
Peter Pan