| Message ID | 52649FDA.1030804@freescale.com |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, Oct 20, 2013 at 8:30 PM, Huang Shijie <b32955@freescale.com> wrote: > 于 2013年10月19日 01:03, Tim Harvey 写道: >> >> The patch you made to obtain ECC info from the chip >> >> (http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/mtd/nand/gpmi-nand?id=2febcdf84b75aae627c61f0a5bf531a69299966c) >> has caused a regression for an i.MX6 board I'm working with that uses >> NAND and ubifs. ... > I am in an embarrass state now: > [1] we can support the jffs2, when we use the set_geometry_by_ecc_info() to > set the NAND layout. > > [2] if we still use the legacy_set_geometry() to set the NAND layout, we can > not support the jffs2 for GPMI. > > > When i submitted the gpmi to the kernel, the mtd code can not provide me the > ECC info. I had to use all the OOB > with legacy_set_geometry(). After i submitted several patches for mtd, it > can provide us the ECC info now, so i switch > to use the set_geometry_by_ecc_info(). > > it's easy to fix your problem with a patch like this: > ----------------------------------------------------------------------------------------------------------------- > > drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c > b/drivers/mtd/nand/gpmi-nand > index 5a1bbc7..8eeead6 100644 > --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c > +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c > @@ -352,7 +352,8 @@ static int legacy_set_geometry(struct gpmi_nand_data > *this) > > int common_nfc_set_geometry(struct gpmi_nand_data *this) > { > - return set_geometry_by_ecc_info(this) ? 0 : > legacy_set_geometry(this); > + return (legacy_set_geometry(this) == 0) ? 0 : > + set_geometry_by_ecc_info(this); > } > > struct dma_chan *get_dma_chan(struct gpmi_nand_data *this) > > > ----------------------------------------------------------------------------------------------------------------- So you really have two wholly incompatible layout methods, one which targeted using the maximum ECC strength that fits and one which targets the minimum required ECC strength? It seems that you can only use a new ECC/geometry if the user prompts you to do so (via device tree, for instance). Doing otherwise is bound to create regressions like this. > But after apply this patch, the gpmi may can not support the JFFS2 any more. Well, we do prioritize "no regressions", rather than new features. So we may have to do this until a proper solution can be formed. Brian
于 2013年10月21日 15:21, Brian Norris 写道: > Well, we do prioritize "no regressions", rather than new features. So > we may have to do this until a proper solution can be formed. ok. I will create a patch to fix this regression. And it seems i have to add new devicetree property to enable the JFFS2 support. thanks Huang Shijie
On Mon, Oct 21, 2013 at 1:04 AM, Huang Shijie <b32955@freescale.com> wrote: > 于 2013年10月21日 15:21, Brian Norris 写道: >> Well, we do prioritize "no regressions", rather than new features. So >> we may have to do this until a proper solution can be formed. > ok. > > I will create a patch to fix this regression. Yes. We should either get it into 3.12 or mark it -stable. > And it seems i have to add new devicetree property to enable the JFFS2 > support. We cannot use device-tree to enable JFFS2 (a purely software issue), but you could use it to specify the ECC strength or layout. Be sure to CC devicetree@vger.kernel.org on any new bindings, since they need to review bindings and tend to have good input. Brian
Dear Huang Shijie, > 于 2013年10月19日 01:03, Tim Harvey 写道: > > Huang, > > > > The patch you made to obtain ECC info from the chip > > (http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/dr > > ivers/mtd/nand/gpmi-nand?id=2febcdf84b75aae627c61f0a5bf531a69299966c) has > > caused a regression for an i.MX6 board I'm working with that uses NAND > > and ubifs. > > > > I'm using a Micron MT29F8G08 > > (http://download.micron.com/pdf/datasheets/flash/nand/2_4_8gb_nand_m49a.p > > df) which has: > > page size: 2112 bytes (2048 + 64 bytes) > > block size: 1056 words (1024 + 32 words) > > plane size: 2 planes x 1024 blocks per plane > > device size: 2Gb: 2048 blocks > > ecc: 4bit > > > > The legacy_set_geometry function comes up with: > > gf_len=13 > > ecc_strength=8 > > page_size=2112 > > metadata_size=10 > > ecc_chunk_size=512 > > ecc_chunk_count=4 > > payload_size=2048 > > auxiliary_size=16 > > auxiliary_status_offset=12 > > block_mark_byte_offset=1999 > > > > and the new set_geometry_by_ecc_info comes up with: > > gf_len=13 > > ecc_strength=4 > > page_size=2084 > > metadata_size=10 > > ecc_chunk_size=512 > > ecc_chunk_count=4 > > payload_size=2048 > > auxiliary_size=16 > > auxiliary_status_offset=12 > > block_mark_byte_offset=2018 > > block_mark_bit_offset=4 > > > > There are two discrepancies above: > > a) ecc_strength - this part has 4bit ecc but being detected as 8? > > b) page_size - the legacy function includes oob in page size, and the > > > > new one does not > > > > which is correct? > > In theory, both are correct. > > I am in an embarrass state now: > [1] we can support the jffs2, when we use the set_geometry_by_ecc_info() > to set the NAND layout. > > [2] if we still use the legacy_set_geometry() to set the NAND layout, we > can not support the jffs2 for GPMI. It's exactly the other way around if you're talking about JFFS2 compatibility with the 2.6.35 FSL kernel ;-)
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand index 5a1bbc7..8eeead6 100644 --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c @@ -352,7 +352,8 @@ static int legacy_set_geometry(struct gpmi_nand_data *this) int common_nfc_set_geometry(struct gpmi_nand_data *this) { - return set_geometry_by_ecc_info(this) ? 0 : legacy_set_geometry(this); + return (legacy_set_geometry(this) == 0) ? 0 : + set_geometry_by_ecc_info(this); } struct dma_chan *get_dma_chan(struct gpmi_nand_data *this)