Re: [PATCH v2 00/12] mtd: nand_bbt: introduce independent nand BBT
Hi Boris,
On Wed, Jan 6, 2016 at 10:29 PM, Boris Brezillon
wrote:
> On Wed, 30 Dec 2015 09:31:06 +0100
> Boris Brezillon wrote:
>
>> Hi Peter,
>>
>> On Wed, 30 Dec 2015 15:18:39 +0800
>> 潘栋 wrote:
>>
>> > Hi Boris and Ezequiel,
>> >
>> > 2015-12-29 23:11 GMT+08:00 Boris Brezillon
>> > :
>> > > On Tue, 29 Dec 2015 12:07:50 -0300
>> > > Ezequiel Garcia wrote:
>> > >
>> > >> On 29 December 2015 at 06:35, Boris Brezillon
>> > >> wrote:
>> > >> > Hi,
>> > >> >
>> > >> > On Mon, 28 Dec 2015 17:42:50 -0300
>> > >> > Ezequiel Garcia wrote:
>> > >> >
>> > >> >> This is looking a lot better, thanks for the good work!
>> > >> >>
>> > >> >> On 15 December 2015 at 02:59, Peter Pan
>> > >> >> wrote:
>> > >> >> > Currently nand_bbt.c is tied with struct nand_chip, and it makes
>> > >> >> > other
>> > >> >> > NAND family chips hard to use nand_bbt.c. Maybe it's the reason why
>> > >> >> > onenand has own bbt(onenand_bbt.c).
>> > >> >> >
>> > >> >> > Separate struct nand_chip from BBT code can make current BBT
>> > >> >> > shareable.
>> > >> >> > We create struct nand_bbt to take place of nand_chip in nand_bbt.c.
>> > >> >> > Struct nand_bbt contains all the information BBT needed from
>> > >> >> > outside and
>> > >> >> > it should be embedded into NAND family chip struct (such as struct
>> > >> >> > nand_chip).
>> > >> >> > NAND family driver should allocate, initialize and free struct
>> > >> >> > nand_bbt.
>> > >> >> >
>> > >> >> > Below is mtd folder structure we want:
>> > >> >> > mtd
>> > >> >> > ├── Kconfig
>> > >> >> > ├── Makefile
>> > >> >> > ├── ...
>> > >> >> > ├── nand_bbt.c
>> > >> >>
>> > >> >> Hm.. I'm not sure about having nand_bbt.c in drivers/mtd.
>> > >> >> What's wrong with drivers/mtd/nand ?
>> > >> >
>> > >> > I haven't reviewed the series yet, but I agree. If the BBT code is
>> > >> > only
>> > >> > meant to be used on NAND based devices, it should probably stay in
>> > >> > drivers/mtd/nand.
>> > >> >
>> > >> >>
>> > >> >> In fact, I was thinking we could go further and clean up the
>> > >> >> directories a bit
>> > >> >> by separating core code, from controllers code, from SPI NAND code:
>> > >> >>
>> > >> >> drivers/mtd/nand/
>> > >> >> drivers/mtd/nand/controllers
>> > >> >> drivers/mtd/nand/spi
>> > >> >>
>> > >> >> Makes any sense?
>> > >> >
>> > >> > Actually I had the secret plan of moving all (raw) NAND controller
>> > >> > drivers into the drivers/mtd/nand/controllers directory, though this
>> > >> > was for a different reason: I'd like to create another directory for
>> > >> > manufacturer specific code in order to support some advanced features
>> > >> > on NANDs that do not implement (or only partially implement) the ONFI
>> > >> > standard.
>> > >> >
>> > >> > The separation you're talking about here is more related to the
>> > >> > interface used to communicate with the NAND chip.
>> > >> >
>> > >> > How about using the following hierarchy?
>> > >> >
>> > >> > drivers/mtd/nand/
>> > >> > drivers/mtd/nand/interfaces/raw/
>> > >> > drivers/mtd/nand/interfaces/raw/controllers/
>> > >> > drivers/mtd/nand/interfaces/spi/
>> > >> > drivers/mtd/nand/interfaces/onenand/
>> > >> > drivers/mtd/nand/chips/
>> > >> >
>> > >> > What do you think?
>> > >> >
>> > >>
>> > >> I believe we are bikeshedding here, but what the heck.
>> > >>
>> > >> That seems too involved. A simpler hierarchy could be clear enough,
>> > >> and seems to follow what other subsystems do:
>> > >>
>> > >> drivers/mtd/nand/
>> > >> drivers/mtd/nand/raw/
>> > >
>> > > And probably some common logic in there too.
>> > >
>> > >> drivers/mtd/nand/spi/
>> > >> drivers/mtd/nand/onenand/
>> > >> drivers/mtd/nand/chips/
>> > >>
>> > >
>> > > I'm fine with this one too ;-).
>> >
>> > I'm fine with this structure too. drivers/mtd/nand folder becomes top
>> > folder for
>> > all NAND based devices. Because (raw)NAND, SPI-NAND and ONENAND have
>> > different command set and feature, each has its own core - nand_base.c
>> > spi-nand-base.c
>> > and onenand_base.c. So maybe it'll take a lot effort to abstract a
>> > all-nand-core-code
>> > (of course BBT should be one of them). What's your opinion?
>>
>> Absolutely, that was the idea: move everything into the
>> drivers/mtd/nand directory (with the structure described above), keep
>> some specific logic for each interface type, and see if we can factor
>> out some common code (I noticed that SPI NAND devices have a parameter
>> page which looks similar to the one exposed by ONFI compliant devices,
>> except this parameter page is retrieved using a different command, the
>> same goes for the ->{set,get}_features() functions).
>> But let's focus on the nand_bbt code for now.
>>
>> >
>> > Also, please review the BBT patch if you have time. I think it's
>> > helpful on the new NAND code
>> > hierarchy.
>>
>> I'll try to review it this week.
>
> I'm a bit late, but I think I've reviewed most of it now.
I already go through your comment
Re: [PATCH v2 00/12] mtd: nand_bbt: introduce independent nand BBT
On Wed, 30 Dec 2015 09:31:06 +0100
Boris Brezillon wrote:
> Hi Peter,
>
> On Wed, 30 Dec 2015 15:18:39 +0800
> 潘栋 wrote:
>
> > Hi Boris and Ezequiel,
> >
> > 2015-12-29 23:11 GMT+08:00 Boris Brezillon
> > :
> > > On Tue, 29 Dec 2015 12:07:50 -0300
> > > Ezequiel Garcia wrote:
> > >
> > >> On 29 December 2015 at 06:35, Boris Brezillon
> > >> wrote:
> > >> > Hi,
> > >> >
> > >> > On Mon, 28 Dec 2015 17:42:50 -0300
> > >> > Ezequiel Garcia wrote:
> > >> >
> > >> >> This is looking a lot better, thanks for the good work!
> > >> >>
> > >> >> On 15 December 2015 at 02:59, Peter Pan
> > >> >> wrote:
> > >> >> > Currently nand_bbt.c is tied with struct nand_chip, and it makes
> > >> >> > other
> > >> >> > NAND family chips hard to use nand_bbt.c. Maybe it's the reason why
> > >> >> > onenand has own bbt(onenand_bbt.c).
> > >> >> >
> > >> >> > Separate struct nand_chip from BBT code can make current BBT
> > >> >> > shareable.
> > >> >> > We create struct nand_bbt to take place of nand_chip in nand_bbt.c.
> > >> >> > Struct nand_bbt contains all the information BBT needed from
> > >> >> > outside and
> > >> >> > it should be embedded into NAND family chip struct (such as struct
> > >> >> > nand_chip).
> > >> >> > NAND family driver should allocate, initialize and free struct
> > >> >> > nand_bbt.
> > >> >> >
> > >> >> > Below is mtd folder structure we want:
> > >> >> > mtd
> > >> >> > ├── Kconfig
> > >> >> > ├── Makefile
> > >> >> > ├── ...
> > >> >> > ├── nand_bbt.c
> > >> >>
> > >> >> Hm.. I'm not sure about having nand_bbt.c in drivers/mtd.
> > >> >> What's wrong with drivers/mtd/nand ?
> > >> >
> > >> > I haven't reviewed the series yet, but I agree. If the BBT code is only
> > >> > meant to be used on NAND based devices, it should probably stay in
> > >> > drivers/mtd/nand.
> > >> >
> > >> >>
> > >> >> In fact, I was thinking we could go further and clean up the
> > >> >> directories a bit
> > >> >> by separating core code, from controllers code, from SPI NAND code:
> > >> >>
> > >> >> drivers/mtd/nand/
> > >> >> drivers/mtd/nand/controllers
> > >> >> drivers/mtd/nand/spi
> > >> >>
> > >> >> Makes any sense?
> > >> >
> > >> > Actually I had the secret plan of moving all (raw) NAND controller
> > >> > drivers into the drivers/mtd/nand/controllers directory, though this
> > >> > was for a different reason: I'd like to create another directory for
> > >> > manufacturer specific code in order to support some advanced features
> > >> > on NANDs that do not implement (or only partially implement) the ONFI
> > >> > standard.
> > >> >
> > >> > The separation you're talking about here is more related to the
> > >> > interface used to communicate with the NAND chip.
> > >> >
> > >> > How about using the following hierarchy?
> > >> >
> > >> > drivers/mtd/nand/
> > >> > drivers/mtd/nand/interfaces/raw/
> > >> > drivers/mtd/nand/interfaces/raw/controllers/
> > >> > drivers/mtd/nand/interfaces/spi/
> > >> > drivers/mtd/nand/interfaces/onenand/
> > >> > drivers/mtd/nand/chips/
> > >> >
> > >> > What do you think?
> > >> >
> > >>
> > >> I believe we are bikeshedding here, but what the heck.
> > >>
> > >> That seems too involved. A simpler hierarchy could be clear enough,
> > >> and seems to follow what other subsystems do:
> > >>
> > >> drivers/mtd/nand/
> > >> drivers/mtd/nand/raw/
> > >
> > > And probably some common logic in there too.
> > >
> > >> drivers/mtd/nand/spi/
> > >> drivers/mtd/nand/onenand/
> > >> drivers/mtd/nand/chips/
> > >>
> > >
> > > I'm fine with this one too ;-).
> >
> > I'm fine with this structure too. drivers/mtd/nand folder becomes top
> > folder for
> > all NAND based devices. Because (raw)NAND, SPI-NAND and ONENAND have
> > different command set and feature, each has its own core - nand_base.c
> > spi-nand-base.c
> > and onenand_base.c. So maybe it'll take a lot effort to abstract a
> > all-nand-core-code
> > (of course BBT should be one of them). What's your opinion?
>
> Absolutely, that was the idea: move everything into the
> drivers/mtd/nand directory (with the structure described above), keep
> some specific logic for each interface type, and see if we can factor
> out some common code (I noticed that SPI NAND devices have a parameter
> page which looks similar to the one exposed by ONFI compliant devices,
> except this parameter page is retrieved using a different command, the
> same goes for the ->{set,get}_features() functions).
> But let's focus on the nand_bbt code for now.
>
> >
> > Also, please review the BBT patch if you have time. I think it's
> > helpful on the new NAND code
> > hierarchy.
>
> I'll try to review it this week.
I'm a bit late, but I think I've reviewed most of it now.
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More maj
Re: [PATCH v2 00/12] mtd: nand_bbt: introduce independent nand BBT
Hi Peter,
On Wed, 30 Dec 2015 15:18:39 +0800
潘栋 wrote:
> Hi Boris and Ezequiel,
>
> 2015-12-29 23:11 GMT+08:00 Boris Brezillon
> :
> > On Tue, 29 Dec 2015 12:07:50 -0300
> > Ezequiel Garcia wrote:
> >
> >> On 29 December 2015 at 06:35, Boris Brezillon
> >> wrote:
> >> > Hi,
> >> >
> >> > On Mon, 28 Dec 2015 17:42:50 -0300
> >> > Ezequiel Garcia wrote:
> >> >
> >> >> This is looking a lot better, thanks for the good work!
> >> >>
> >> >> On 15 December 2015 at 02:59, Peter Pan wrote:
> >> >> > Currently nand_bbt.c is tied with struct nand_chip, and it makes other
> >> >> > NAND family chips hard to use nand_bbt.c. Maybe it's the reason why
> >> >> > onenand has own bbt(onenand_bbt.c).
> >> >> >
> >> >> > Separate struct nand_chip from BBT code can make current BBT
> >> >> > shareable.
> >> >> > We create struct nand_bbt to take place of nand_chip in nand_bbt.c.
> >> >> > Struct nand_bbt contains all the information BBT needed from outside
> >> >> > and
> >> >> > it should be embedded into NAND family chip struct (such as struct
> >> >> > nand_chip).
> >> >> > NAND family driver should allocate, initialize and free struct
> >> >> > nand_bbt.
> >> >> >
> >> >> > Below is mtd folder structure we want:
> >> >> > mtd
> >> >> > ├── Kconfig
> >> >> > ├── Makefile
> >> >> > ├── ...
> >> >> > ├── nand_bbt.c
> >> >>
> >> >> Hm.. I'm not sure about having nand_bbt.c in drivers/mtd.
> >> >> What's wrong with drivers/mtd/nand ?
> >> >
> >> > I haven't reviewed the series yet, but I agree. If the BBT code is only
> >> > meant to be used on NAND based devices, it should probably stay in
> >> > drivers/mtd/nand.
> >> >
> >> >>
> >> >> In fact, I was thinking we could go further and clean up the
> >> >> directories a bit
> >> >> by separating core code, from controllers code, from SPI NAND code:
> >> >>
> >> >> drivers/mtd/nand/
> >> >> drivers/mtd/nand/controllers
> >> >> drivers/mtd/nand/spi
> >> >>
> >> >> Makes any sense?
> >> >
> >> > Actually I had the secret plan of moving all (raw) NAND controller
> >> > drivers into the drivers/mtd/nand/controllers directory, though this
> >> > was for a different reason: I'd like to create another directory for
> >> > manufacturer specific code in order to support some advanced features
> >> > on NANDs that do not implement (or only partially implement) the ONFI
> >> > standard.
> >> >
> >> > The separation you're talking about here is more related to the
> >> > interface used to communicate with the NAND chip.
> >> >
> >> > How about using the following hierarchy?
> >> >
> >> > drivers/mtd/nand/
> >> > drivers/mtd/nand/interfaces/raw/
> >> > drivers/mtd/nand/interfaces/raw/controllers/
> >> > drivers/mtd/nand/interfaces/spi/
> >> > drivers/mtd/nand/interfaces/onenand/
> >> > drivers/mtd/nand/chips/
> >> >
> >> > What do you think?
> >> >
> >>
> >> I believe we are bikeshedding here, but what the heck.
> >>
> >> That seems too involved. A simpler hierarchy could be clear enough,
> >> and seems to follow what other subsystems do:
> >>
> >> drivers/mtd/nand/
> >> drivers/mtd/nand/raw/
> >
> > And probably some common logic in there too.
> >
> >> drivers/mtd/nand/spi/
> >> drivers/mtd/nand/onenand/
> >> drivers/mtd/nand/chips/
> >>
> >
> > I'm fine with this one too ;-).
>
> I'm fine with this structure too. drivers/mtd/nand folder becomes top folder
> for
> all NAND based devices. Because (raw)NAND, SPI-NAND and ONENAND have
> different command set and feature, each has its own core - nand_base.c
> spi-nand-base.c
> and onenand_base.c. So maybe it'll take a lot effort to abstract a
> all-nand-core-code
> (of course BBT should be one of them). What's your opinion?
Absolutely, that was the idea: move everything into the
drivers/mtd/nand directory (with the structure described above), keep
some specific logic for each interface type, and see if we can factor
out some common code (I noticed that SPI NAND devices have a parameter
page which looks similar to the one exposed by ONFI compliant devices,
except this parameter page is retrieved using a different command, the
same goes for the ->{set,get}_features() functions).
But let's focus on the nand_bbt code for now.
>
> Also, please review the BBT patch if you have time. I think it's
> helpful on the new NAND code
> hierarchy.
I'll try to review it this week.
Best Regards,
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 00/12] mtd: nand_bbt: introduce independent nand BBT
Hi Boris and Ezequiel, 2015-12-29 23:11 GMT+08:00 Boris Brezillon : > On Tue, 29 Dec 2015 12:07:50 -0300 > Ezequiel Garcia wrote: > >> On 29 December 2015 at 06:35, Boris Brezillon >> wrote: >> > Hi, >> > >> > On Mon, 28 Dec 2015 17:42:50 -0300 >> > Ezequiel Garcia wrote: >> > >> >> This is looking a lot better, thanks for the good work! >> >> >> >> On 15 December 2015 at 02:59, Peter Pan wrote: >> >> > Currently nand_bbt.c is tied with struct nand_chip, and it makes other >> >> > NAND family chips hard to use nand_bbt.c. Maybe it's the reason why >> >> > onenand has own bbt(onenand_bbt.c). >> >> > >> >> > Separate struct nand_chip from BBT code can make current BBT shareable. >> >> > We create struct nand_bbt to take place of nand_chip in nand_bbt.c. >> >> > Struct nand_bbt contains all the information BBT needed from outside and >> >> > it should be embedded into NAND family chip struct (such as struct >> >> > nand_chip). >> >> > NAND family driver should allocate, initialize and free struct nand_bbt. >> >> > >> >> > Below is mtd folder structure we want: >> >> > mtd >> >> > ├── Kconfig >> >> > ├── Makefile >> >> > ├── ... >> >> > ├── nand_bbt.c >> >> >> >> Hm.. I'm not sure about having nand_bbt.c in drivers/mtd. >> >> What's wrong with drivers/mtd/nand ? >> > >> > I haven't reviewed the series yet, but I agree. If the BBT code is only >> > meant to be used on NAND based devices, it should probably stay in >> > drivers/mtd/nand. >> > >> >> >> >> In fact, I was thinking we could go further and clean up the directories >> >> a bit >> >> by separating core code, from controllers code, from SPI NAND code: >> >> >> >> drivers/mtd/nand/ >> >> drivers/mtd/nand/controllers >> >> drivers/mtd/nand/spi >> >> >> >> Makes any sense? >> > >> > Actually I had the secret plan of moving all (raw) NAND controller >> > drivers into the drivers/mtd/nand/controllers directory, though this >> > was for a different reason: I'd like to create another directory for >> > manufacturer specific code in order to support some advanced features >> > on NANDs that do not implement (or only partially implement) the ONFI >> > standard. >> > >> > The separation you're talking about here is more related to the >> > interface used to communicate with the NAND chip. >> > >> > How about using the following hierarchy? >> > >> > drivers/mtd/nand/ >> > drivers/mtd/nand/interfaces/raw/ >> > drivers/mtd/nand/interfaces/raw/controllers/ >> > drivers/mtd/nand/interfaces/spi/ >> > drivers/mtd/nand/interfaces/onenand/ >> > drivers/mtd/nand/chips/ >> > >> > What do you think? >> > >> >> I believe we are bikeshedding here, but what the heck. >> >> That seems too involved. A simpler hierarchy could be clear enough, >> and seems to follow what other subsystems do: >> >> drivers/mtd/nand/ >> drivers/mtd/nand/raw/ > > And probably some common logic in there too. > >> drivers/mtd/nand/spi/ >> drivers/mtd/nand/onenand/ >> drivers/mtd/nand/chips/ >> > > I'm fine with this one too ;-). I'm fine with this structure too. drivers/mtd/nand folder becomes top folder for all NAND based devices. Because (raw)NAND, SPI-NAND and ONENAND have different command set and feature, each has its own core - nand_base.c spi-nand-base.c and onenand_base.c. So maybe it'll take a lot effort to abstract a all-nand-core-code (of course BBT should be one of them). What's your opinion? Also, please review the BBT patch if you have time. I think it's helpful on the new NAND code hierarchy. > > -- > Boris Brezillon, Free Electrons > Embedded Linux and Kernel engineering > http://free-electrons.com Thanks Peter Pan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 00/12] mtd: nand_bbt: introduce independent nand BBT
On Tue, 29 Dec 2015 12:07:50 -0300 Ezequiel Garcia wrote: > On 29 December 2015 at 06:35, Boris Brezillon > wrote: > > Hi, > > > > On Mon, 28 Dec 2015 17:42:50 -0300 > > Ezequiel Garcia wrote: > > > >> This is looking a lot better, thanks for the good work! > >> > >> On 15 December 2015 at 02:59, Peter Pan wrote: > >> > Currently nand_bbt.c is tied with struct nand_chip, and it makes other > >> > NAND family chips hard to use nand_bbt.c. Maybe it's the reason why > >> > onenand has own bbt(onenand_bbt.c). > >> > > >> > Separate struct nand_chip from BBT code can make current BBT shareable. > >> > We create struct nand_bbt to take place of nand_chip in nand_bbt.c. > >> > Struct nand_bbt contains all the information BBT needed from outside and > >> > it should be embedded into NAND family chip struct (such as struct > >> > nand_chip). > >> > NAND family driver should allocate, initialize and free struct nand_bbt. > >> > > >> > Below is mtd folder structure we want: > >> > mtd > >> > ├── Kconfig > >> > ├── Makefile > >> > ├── ... > >> > ├── nand_bbt.c > >> > >> Hm.. I'm not sure about having nand_bbt.c in drivers/mtd. > >> What's wrong with drivers/mtd/nand ? > > > > I haven't reviewed the series yet, but I agree. If the BBT code is only > > meant to be used on NAND based devices, it should probably stay in > > drivers/mtd/nand. > > > >> > >> In fact, I was thinking we could go further and clean up the directories > >> a bit > >> by separating core code, from controllers code, from SPI NAND code: > >> > >> drivers/mtd/nand/ > >> drivers/mtd/nand/controllers > >> drivers/mtd/nand/spi > >> > >> Makes any sense? > > > > Actually I had the secret plan of moving all (raw) NAND controller > > drivers into the drivers/mtd/nand/controllers directory, though this > > was for a different reason: I'd like to create another directory for > > manufacturer specific code in order to support some advanced features > > on NANDs that do not implement (or only partially implement) the ONFI > > standard. > > > > The separation you're talking about here is more related to the > > interface used to communicate with the NAND chip. > > > > How about using the following hierarchy? > > > > drivers/mtd/nand/ > > drivers/mtd/nand/interfaces/raw/ > > drivers/mtd/nand/interfaces/raw/controllers/ > > drivers/mtd/nand/interfaces/spi/ > > drivers/mtd/nand/interfaces/onenand/ > > drivers/mtd/nand/chips/ > > > > What do you think? > > > > I believe we are bikeshedding here, but what the heck. > > That seems too involved. A simpler hierarchy could be clear enough, > and seems to follow what other subsystems do: > > drivers/mtd/nand/ > drivers/mtd/nand/raw/ And probably some common logic in there too. > drivers/mtd/nand/spi/ > drivers/mtd/nand/onenand/ > drivers/mtd/nand/chips/ > I'm fine with this one too ;-). -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 00/12] mtd: nand_bbt: introduce independent nand BBT
On 29 December 2015 at 06:35, Boris Brezillon wrote: > Hi, > > On Mon, 28 Dec 2015 17:42:50 -0300 > Ezequiel Garcia wrote: > >> This is looking a lot better, thanks for the good work! >> >> On 15 December 2015 at 02:59, Peter Pan wrote: >> > Currently nand_bbt.c is tied with struct nand_chip, and it makes other >> > NAND family chips hard to use nand_bbt.c. Maybe it's the reason why >> > onenand has own bbt(onenand_bbt.c). >> > >> > Separate struct nand_chip from BBT code can make current BBT shareable. >> > We create struct nand_bbt to take place of nand_chip in nand_bbt.c. >> > Struct nand_bbt contains all the information BBT needed from outside and >> > it should be embedded into NAND family chip struct (such as struct >> > nand_chip). >> > NAND family driver should allocate, initialize and free struct nand_bbt. >> > >> > Below is mtd folder structure we want: >> > mtd >> > ├── Kconfig >> > ├── Makefile >> > ├── ... >> > ├── nand_bbt.c >> >> Hm.. I'm not sure about having nand_bbt.c in drivers/mtd. >> What's wrong with drivers/mtd/nand ? > > I haven't reviewed the series yet, but I agree. If the BBT code is only > meant to be used on NAND based devices, it should probably stay in > drivers/mtd/nand. > >> >> In fact, I was thinking we could go further and clean up the directories a >> bit >> by separating core code, from controllers code, from SPI NAND code: >> >> drivers/mtd/nand/ >> drivers/mtd/nand/controllers >> drivers/mtd/nand/spi >> >> Makes any sense? > > Actually I had the secret plan of moving all (raw) NAND controller > drivers into the drivers/mtd/nand/controllers directory, though this > was for a different reason: I'd like to create another directory for > manufacturer specific code in order to support some advanced features > on NANDs that do not implement (or only partially implement) the ONFI > standard. > > The separation you're talking about here is more related to the > interface used to communicate with the NAND chip. > > How about using the following hierarchy? > > drivers/mtd/nand/ > drivers/mtd/nand/interfaces/raw/ > drivers/mtd/nand/interfaces/raw/controllers/ > drivers/mtd/nand/interfaces/spi/ > drivers/mtd/nand/interfaces/onenand/ > drivers/mtd/nand/chips/ > > What do you think? > I believe we are bikeshedding here, but what the heck. That seems too involved. A simpler hierarchy could be clear enough, and seems to follow what other subsystems do: drivers/mtd/nand/ drivers/mtd/nand/raw/ drivers/mtd/nand/spi/ drivers/mtd/nand/onenand/ drivers/mtd/nand/chips/ -- Ezequiel García, VanguardiaSur www.vanguardiasur.com.ar -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 00/12] mtd: nand_bbt: introduce independent nand BBT
Hi, On Mon, 28 Dec 2015 17:42:50 -0300 Ezequiel Garcia wrote: > This is looking a lot better, thanks for the good work! > > On 15 December 2015 at 02:59, Peter Pan wrote: > > Currently nand_bbt.c is tied with struct nand_chip, and it makes other > > NAND family chips hard to use nand_bbt.c. Maybe it's the reason why > > onenand has own bbt(onenand_bbt.c). > > > > Separate struct nand_chip from BBT code can make current BBT shareable. > > We create struct nand_bbt to take place of nand_chip in nand_bbt.c. > > Struct nand_bbt contains all the information BBT needed from outside and > > it should be embedded into NAND family chip struct (such as struct > > nand_chip). > > NAND family driver should allocate, initialize and free struct nand_bbt. > > > > Below is mtd folder structure we want: > > mtd > > ├── Kconfig > > ├── Makefile > > ├── ... > > ├── nand_bbt.c > > Hm.. I'm not sure about having nand_bbt.c in drivers/mtd. > What's wrong with drivers/mtd/nand ? I haven't reviewed the series yet, but I agree. If the BBT code is only meant to be used on NAND based devices, it should probably stay in drivers/mtd/nand. > > In fact, I was thinking we could go further and clean up the directories a > bit > by separating core code, from controllers code, from SPI NAND code: > > drivers/mtd/nand/ > drivers/mtd/nand/controllers > drivers/mtd/nand/spi > > Makes any sense? Actually I had the secret plan of moving all (raw) NAND controller drivers into the drivers/mtd/nand/controllers directory, though this was for a different reason: I'd like to create another directory for manufacturer specific code in order to support some advanced features on NANDs that do not implement (or only partially implement) the ONFI standard. The separation you're talking about here is more related to the interface used to communicate with the NAND chip. How about using the following hierarchy? drivers/mtd/nand/ drivers/mtd/nand/interfaces/raw/ drivers/mtd/nand/interfaces/raw/controllers/ drivers/mtd/nand/interfaces/spi/ drivers/mtd/nand/interfaces/onenand/ drivers/mtd/nand/chips/ What do you think? Best Regards, Boris -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 00/12] mtd: nand_bbt: introduce independent nand BBT
This is looking a lot better, thanks for the good work! On 15 December 2015 at 02:59, Peter Pan wrote: > Currently nand_bbt.c is tied with struct nand_chip, and it makes other > NAND family chips hard to use nand_bbt.c. Maybe it's the reason why > onenand has own bbt(onenand_bbt.c). > > Separate struct nand_chip from BBT code can make current BBT shareable. > We create struct nand_bbt to take place of nand_chip in nand_bbt.c. > Struct nand_bbt contains all the information BBT needed from outside and > it should be embedded into NAND family chip struct (such as struct nand_chip). > NAND family driver should allocate, initialize and free struct nand_bbt. > > Below is mtd folder structure we want: > mtd > ├── Kconfig > ├── Makefile > ├── ... > ├── nand_bbt.c Hm.. I'm not sure about having nand_bbt.c in drivers/mtd. What's wrong with drivers/mtd/nand ? In fact, I was thinking we could go further and clean up the directories a bit by separating core code, from controllers code, from SPI NAND code: drivers/mtd/nand/ drivers/mtd/nand/controllers drivers/mtd/nand/spi Makes any sense? -- Ezequiel García, VanguardiaSur www.vanguardiasur.com.ar -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 00/12] mtd: nand_bbt: introduce independent nand BBT
Currently nand_bbt.c is tied with struct nand_chip, and it makes other
NAND family chips hard to use nand_bbt.c. Maybe it's the reason why
onenand has own bbt(onenand_bbt.c).
Separate struct nand_chip from BBT code can make current BBT shareable.
We create struct nand_bbt to take place of nand_chip in nand_bbt.c.
Struct nand_bbt contains all the information BBT needed from outside and
it should be embedded into NAND family chip struct (such as struct nand_chip).
NAND family driver should allocate, initialize and free struct nand_bbt.
Below is mtd folder structure we want:
mtd
├── Kconfig
├── Makefile
├── ...
├── nand_bbt.c
├── nand
│ ├── Kconfig
│ ├── Makefile
│ ├── nand_base.c
│ ├── nand_ids.c
│ ├── ...
│ └── xway_nand.c
├── spi-nand
│ ├── Kconfig
│ ├── Makefile
│ ├── spi-nand-base.c
│ ├── ...
│ └── spi-nand-device.c
└── ...
Most of the patch is borrowed from Brian Norris .
http://git.infradead.org/users/norris/linux-mtd.git/shortlog/refs/heads/nand-bbt
Based on Brian's suggestion, I make my previous BBT patch into 12 independent
patches. Previous patch is http://patchwork.ozlabs.org/patch/492066/
Beside the patch split, I also moved nand_bbt.c to mtd folder, which didn't in
previous patch.
Patch 3, 7, 8, 9, 10 and 11 are totally borrowed from Brian's git tree. I just
test and split the code into independent patch. Patch 1, 2, 5 and 6 are partial
borrowed. I make some changes from Brian's git tree and the changes are recorded
in commit log. Patch 4 and 12 are written by me.
The patch is tested on Zed board.
v2 changes:
rebase patch series on master branch of l2-mtd.git
Brian Norris (10):
mtd: nand_bbt: new header for nand family BBT
mtd: nand_bbt: introduce struct nand_bbt
mtd: nand_bbt: add new API definitions
mtd: nand: use new BBT API instead of old ones
mtd: nand_bbt: use erase() and is_bad_bbm() hook in BBT
mtd: nand: make nand_erase_nand() static
mtd: nand_bbt: remove struct nand_chip from nand_bbt.c
mtd: nand_bbt: remove old API definitions
mtd: nand_bbt: remove NAND_BBT_DYNAMICSTRUCT macro
mtd: nand: remove nand_chip.bbt
Peter Pan (2):
mtd: nand_bbt: add nand_bbt_markbad_factory() interface
mtd: nand-bbt: move nand_bbt.c to mtd folder
drivers/mtd/Kconfig | 7 +
drivers/mtd/Makefile | 1 +
drivers/mtd/nand/Kconfig | 2 +-
drivers/mtd/nand/Makefile | 2 +-
drivers/mtd/nand/docg4.c | 6 +-
drivers/mtd/nand/nand_base.c | 145 +-
drivers/mtd/{nand => }/nand_bbt.c | 542 --
include/linux/mtd/bbm.h | 96 +--
include/linux/mtd/nand.h | 16 +-
include/linux/mtd/nand_bbt.h | 177 +
10 files changed, 562 insertions(+), 432 deletions(-)
rename drivers/mtd/{nand => }/nand_bbt.c (68%)
create mode 100644 include/linux/mtd/nand_bbt.h
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
