* [PATCH 1/3 -v3] MTD: Add __nand_calculate_ecc() to NAND ECC functions
From: Akinobu Mita @ 2009-10-21 8:34 UTC (permalink / raw)
To: linux-mtd; +Cc: Artem Bityutskiy, David Woodhouse, Akinobu Mita, vimal singh
Add __nand_calculate_ecc() which does not take struct mtd_info.
The built-in 256/512 software ECC calculation and correction tester
will use it.
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
Cc: linux-mtd@lists.infradead.org
Cc: vimal singh <vimal.newwork@gmail.com>
---
drivers/mtd/nand/nand_ecc.c | 25 ++++++++++++++++++++-----
include/linux/mtd/nand_ecc.h | 10 ++++++++--
2 files changed, 28 insertions(+), 7 deletions(-)
diff --git a/drivers/mtd/nand/nand_ecc.c b/drivers/mtd/nand/nand_ecc.c
index db7ae9d..809fb53 100644
--- a/drivers/mtd/nand/nand_ecc.c
+++ b/drivers/mtd/nand/nand_ecc.c
@@ -150,20 +150,19 @@ static const char addressbits[256] = {
};
/**
- * nand_calculate_ecc - [NAND Interface] Calculate 3-byte ECC for 256/512-byte
+ * __nand_calculate_ecc - [NAND Interface] Calculate 3-byte ECC for 256/512-byte
* block
- * @mtd: MTD block structure
* @buf: input buffer with raw data
+ * @eccsize: data bytes per ecc step (256 or 512)
* @code: output buffer with ECC
*/
-int nand_calculate_ecc(struct mtd_info *mtd, const unsigned char *buf,
+void __nand_calculate_ecc(const unsigned char *buf, unsigned int eccsize,
unsigned char *code)
{
int i;
const uint32_t *bp = (uint32_t *)buf;
/* 256 or 512 bytes/ecc */
- const uint32_t eccsize_mult =
- (((struct nand_chip *)mtd->priv)->ecc.size) >> 8;
+ const uint32_t eccsize_mult = eccsize >> 8;
uint32_t cur; /* current value in buffer */
/* rp0..rp15..rp17 are the various accumulated parities (per byte) */
uint32_t rp0, rp1, rp2, rp3, rp4, rp5, rp6, rp7;
@@ -412,6 +411,22 @@ int nand_calculate_ecc(struct mtd_info *mtd, const unsigned char *buf,
(invparity[par & 0x55] << 2) |
(invparity[rp17] << 1) |
(invparity[rp16] << 0);
+}
+EXPORT_SYMBOL(__nand_calculate_ecc);
+
+/**
+ * nand_calculate_ecc - [NAND Interface] Calculate 3-byte ECC for 256/512-byte
+ * block
+ * @mtd: MTD block structure
+ * @buf: input buffer with raw data
+ * @code: output buffer with ECC
+ */
+int nand_calculate_ecc(struct mtd_info *mtd, const unsigned char *buf,
+ unsigned char *code)
+{
+ __nand_calculate_ecc(buf,
+ ((struct nand_chip *)mtd->priv)->ecc.size, code);
+
return 0;
}
EXPORT_SYMBOL(nand_calculate_ecc);
diff --git a/include/linux/mtd/nand_ecc.h b/include/linux/mtd/nand_ecc.h
index 052ea8c..41bc013 100644
--- a/include/linux/mtd/nand_ecc.h
+++ b/include/linux/mtd/nand_ecc.h
@@ -16,7 +16,13 @@
struct mtd_info;
/*
- * Calculate 3 byte ECC code for 256 byte block
+ * Calculate 3 byte ECC code for eccsize byte block
+ */
+void __nand_calculate_ecc(const u_char *dat, unsigned int eccsize,
+ u_char *ecc_code);
+
+/*
+ * Calculate 3 byte ECC code for 256/512 byte block
*/
int nand_calculate_ecc(struct mtd_info *mtd, const u_char *dat, u_char *ecc_code);
@@ -27,7 +33,7 @@ int __nand_correct_data(u_char *dat, u_char *read_ecc, u_char *calc_ecc,
unsigned int eccsize);
/*
- * Detect and correct a 1 bit error for 256 byte block
+ * Detect and correct a 1 bit error for 256/512 byte block
*/
int nand_correct_data(struct mtd_info *mtd, u_char *dat, u_char *read_ecc, u_char *calc_ecc);
--
1.5.4.3
^ permalink raw reply related
* Re: [PATCH 2/2] MTD: Add nand_ecc test module
From: Artem Bityutskiy @ 2009-10-21 6:52 UTC (permalink / raw)
To: Akinobu Mita; +Cc: David Woodhouse, linux-mtd@lists.infradead.org
In-Reply-To: <20091021044614.GA10336@localhost.localdomain>
On Wed, 2009-10-21 at 06:46 +0200, ext Akinobu Mita wrote:
> On Tue, Oct 20, 2009 at 02:50:01PM +0300, Artem Bityutskiy wrote:
> > > diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
> > > index ecf90f5..abe5c7f 100644
> > > --- a/drivers/mtd/Kconfig
> > > +++ b/drivers/mtd/Kconfig
> > > @@ -33,6 +33,12 @@ config MTD_TESTS
> > > should normally be compiled as kernel modules. The modules perform
> > > various checks and verifications when loaded.
> > >
> > > +config MTD_NAND_TESTS
> > > + tristate "MTD NAND tests support"
> > > + depends on MTD_TESTS && MTD_NAND
> > > + help
> > > + This option enables MTD tests which require NAND Device support.
> > > +
> > > config MTD_CONCAT
> > > tristate "MTD concatenating support"
> > > help
> >
> > Could this please be a separate patch? Also, some of the existing tests
> > are NAND only as well, so could do corresponding Makefile changes in the
> > same patch and move the to the MTD_NAND_TESTS set? The tests are:
> >
> > mtd_oobtest.c
> > mtd_pagetest.c
> > mtd_subpagetest.c
>
> Should these MTD NAND tests support OneNAND device, too?
>
> If so, the config with !CONFIG_MTD_NAND && MTD_ONENAND=m cannot select
> this new MTD_NAND_TESTS. So Kconfig dependency should be:
>
> config MTD_NAND_TESTS
> tristate "MTD NAND tests support"
> depends on MTD_TEST
> depends on MTD_NAND || MTD_ONENAND
>
> But nand_ecc-test obviously needs CONFIG_MTD_NAND and cannot exist
> in the same group.
Oh, I see. May be your original idea with ifdefs in the code was not
bad at all :-) ?
--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
^ permalink raw reply
* Re: should/can we do ubidetach on reboot
From: Norbert van Bolhuis @ 2009-10-21 6:07 UTC (permalink / raw)
Cc: Mtd List
In-Reply-To: <4ADDDF57.4050602@aimvalley.nl>
Norbert van Bolhuis wrote:
>
> I guess it's safe/normal to detach an MTD device from UBI before
> rebooting ?
>
> I'm asking because detaching is the only way to shut down the UBI
> background task (ubi_bgt0d). I do not want this task to start a flash erase
> operation or whatsoever just before the actual reboot takes place.
>
>
>
Oh, I miss "[PATCH] MTD: Add UBI reboot notifier".
This solves the problem.
^ permalink raw reply
* Re: [PATCH 2/2] MTD: Add nand_ecc test module
From: Akinobu Mita @ 2009-10-21 4:46 UTC (permalink / raw)
To: Artem Bityutskiy; +Cc: David Woodhouse, linux-mtd
In-Reply-To: <1256039401.29856.216.camel@localhost>
On Tue, Oct 20, 2009 at 02:50:01PM +0300, Artem Bityutskiy wrote:
> > diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
> > index ecf90f5..abe5c7f 100644
> > --- a/drivers/mtd/Kconfig
> > +++ b/drivers/mtd/Kconfig
> > @@ -33,6 +33,12 @@ config MTD_TESTS
> > should normally be compiled as kernel modules. The modules perform
> > various checks and verifications when loaded.
> >
> > +config MTD_NAND_TESTS
> > + tristate "MTD NAND tests support"
> > + depends on MTD_TESTS && MTD_NAND
> > + help
> > + This option enables MTD tests which require NAND Device support.
> > +
> > config MTD_CONCAT
> > tristate "MTD concatenating support"
> > help
>
> Could this please be a separate patch? Also, some of the existing tests
> are NAND only as well, so could do corresponding Makefile changes in the
> same patch and move the to the MTD_NAND_TESTS set? The tests are:
>
> mtd_oobtest.c
> mtd_pagetest.c
> mtd_subpagetest.c
Should these MTD NAND tests support OneNAND device, too?
If so, the config with !CONFIG_MTD_NAND && MTD_ONENAND=m cannot select
this new MTD_NAND_TESTS. So Kconfig dependency should be:
config MTD_NAND_TESTS
tristate "MTD NAND tests support"
depends on MTD_TEST
depends on MTD_NAND || MTD_ONENAND
But nand_ecc-test obviously needs CONFIG_MTD_NAND and cannot exist
in the same group.
^ permalink raw reply
* [PATCH] mtd: fix error path in physmap.c
From: H Hartley Sweeten @ 2009-10-21 1:09 UTC (permalink / raw)
To: linux-mtd; +Cc: David Woodhouse
During the physmap probe, add_mtd_partitions() and add_mtd_device()
can both return an error. Add an error path to handle this.
Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
Cc: David Woodhouse <dwmw2@infradead.org>
---
diff --git a/drivers/mtd/maps/physmap.c b/drivers/mtd/maps/physmap.c
index 3f13a96..966b25d 100644
--- a/drivers/mtd/maps/physmap.c
+++ b/drivers/mtd/maps/physmap.c
@@ -169,24 +169,28 @@ static int physmap_flash_probe(struct platform_device *dev)
goto err_out;
#ifdef CONFIG_MTD_PARTITIONS
- err = parse_mtd_partitions(info->cmtd, part_probe_types,
- &info->parts, 0);
- if (err > 0) {
- add_mtd_partitions(info->cmtd, info->parts, err);
- info->nr_parts = err;
- return 0;
+ info->nr_parts = parse_mtd_partitions(info->cmtd, part_probe_types,
+ &info->parts, 0);
+ if (info->nr_parts > 0) {
+ err = add_mtd_partitions(info->cmtd, info->parts,
+ info->nr_parts);
+ goto out;
}
if (physmap_data->nr_parts) {
printk(KERN_NOTICE "Using physmap partition information\n");
- add_mtd_partitions(info->cmtd, physmap_data->parts,
- physmap_data->nr_parts);
- return 0;
+ err = add_mtd_partitions(info->cmtd, physmap_data->parts,
+ physmap_data->nr_parts);
+ goto out;
}
#endif
- add_mtd_device(info->cmtd);
- return 0;
+ if (add_mtd_device(info->cmtd) == 1)
+ err = -ENODEV;
+
+out:
+ if (!err)
+ return 0;
err_out:
physmap_flash_remove(dev);
^ permalink raw reply related
* RE: [PATCH] mtd: Fix kernel NULL pointer dereference in physmap.c
From: H Hartley Sweeten @ 2009-10-20 22:28 UTC (permalink / raw)
To: David Woodhouse; +Cc: Atsushi Nemoto, linux-mtd
In-Reply-To: <1256074658.4230.6.camel@macbook.infradead.org>
On Tuesday, October 20, 2009 2:38 PM, David Woodhouse wrote:
> On Tue, 2009-10-20 at 12:23 -0400, H Hartley Sweeten wrote:
>> During the probe for physmap platform flash devices there are a
>> number error exit conditions that all do a goto err_out which
>> then calls physmap_flash_remove(). In that function one of the
>> cleanup steps is:
>>
>> #ifdef CONFIG_MTD_CONCAT
>> if (info->cmtd != info->mtd[0])
>> mtd_concat_destroy(info->cmtd);
>> #endif
>>
>> This test will succeed since info->cmtd == NULL and info->mtd[0] is
>> valid, which then causes a NULL pointer dereference when mtd_concat_destroy()
>> is called. Fix this by moving the mtd_concat_destroy() step into the
>> if (info->cmtd) condition.
>>
>> Also, move the kfree(info->parts) cleanup to remove an #ifdef.
>>
>> Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
>> Cc: David Woodhouse <dwmw2@infradead.org>
>> Cc: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
>>
>> ---
>>
>> V2 - As pointed out by Atsushi Nemoto, the map_destroy loop should not
>> be skipped even when info->cmtd == NULL.
>
> Thanks.
>
> In an attempt to improve my responsiveness as maintainer, I'd already
> committed the first version. How does this look:
Very responsive indeed. ;-)
Sorry about introducing the bug. Your amended patch looks like it serves
the same purpose as my updated one. Thanks for fixing that.
Regards,
Hartley
^ permalink raw reply
* A little off-topic
From: Frederic Praca @ 2009-10-20 21:55 UTC (permalink / raw)
To: linux-mtd
Hello guys,
well, it's a little... No it's completely off-topic but I just bought a
SBC-GXm motherboard on eBay and I did't get the provided
CD-Rom with code samples and so on. I read that some of you had this
board in the past and wanted to know if someone still owns a copy of the
CD-Rom.
Thanks in advance
Fred
--
Les enfants commencent par aimer leurs parents ; en grandissant,
ils se mettent a les juger - parfois ils leur pardonnent.
-+- Oscar Wilde -+-
^ permalink raw reply
* RE: [PATCH] mtd: Fix kernel NULL pointer dereference in physmap.c
From: David Woodhouse @ 2009-10-20 21:37 UTC (permalink / raw)
To: H Hartley Sweeten; +Cc: Atsushi Nemoto, linux-mtd
In-Reply-To: <BD79186B4FD85F4B8E60E381CAEE190901E24827@mi8nycmail19.Mi8.com>
On Tue, 2009-10-20 at 12:23 -0400, H Hartley Sweeten wrote:
> During the probe for physmap platform flash devices there are a
> number error exit conditions that all do a goto err_out which
> then calls physmap_flash_remove(). In that function one of the
> cleanup steps is:
>
> #ifdef CONFIG_MTD_CONCAT
> if (info->cmtd != info->mtd[0])
> mtd_concat_destroy(info->cmtd);
> #endif
>
> This test will succeed since info->cmtd == NULL and info->mtd[0] is
> valid, which then causes a NULL pointer dereference when mtd_concat_destroy()
> is called. Fix this by moving the mtd_concat_destroy() step into the
> if (info->cmtd) condition.
>
> Also, move the kfree(info->parts) cleanup to remove an #ifdef.
>
> Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
>
> ---
>
> V2 - As pointed out by Atsushi Nemoto, the map_destroy loop should not
> be skipped even when info->cmtd == NULL.
Thanks.
In an attempt to improve my responsiveness as maintainer, I'd already
committed the first version. How does this look:
commit 8ce110ac19bc88b82e3feacfbb3a2ee08a07fe22
Author: H Hartley Sweeten <hartleys@visionengravers.com>
Date: Tue Oct 20 12:23:33 2009 -0400
mtd: Fix compile failure and error path in physmap.c
Commit 4b56ffcacee937a85bf39e14872dd141e23ee85f ("mtd: Fix kernel NULL
pointer dereference in physmap.c") introduced a couple of bugs.
It neglected to run the loop of map_destroy() calls in
physmap_flash_remove(), if !info->cmtd, which would happen if that
function was called to clean up errors during probe.
It also failed to compile if CONFIG_MTD_PARTITIONS was not defined.
Reported-By: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
diff --git a/drivers/mtd/maps/physmap.c b/drivers/mtd/maps/physmap.c
index 65f52d4..3f13a96 100644
--- a/drivers/mtd/maps/physmap.c
+++ b/drivers/mtd/maps/physmap.c
@@ -44,12 +44,10 @@ static int physmap_flash_remove(struct platform_device *dev)
return 0;
platform_set_drvdata(dev, NULL);
- if (info->cmtd == NULL)
- return 0;
-
physmap_data = dev->dev.platform_data;
- if (mtd_has_partitions()) {
+ if (info->cmtd) {
+#ifdef CONFIG_MTD_PARTITIONS
if (info->nr_parts || physmap_data->nr_parts) {
del_mtd_partitions(info->cmtd);
@@ -58,14 +56,14 @@ static int physmap_flash_remove(struct platform_device *dev)
} else {
del_mtd_device(info->cmtd);
}
- } else {
+#else
del_mtd_device(info->cmtd);
- }
-
+#endif
#ifdef CONFIG_MTD_CONCAT
- if (info->cmtd != info->mtd[0])
- mtd_concat_destroy(info->cmtd);
+ if (info->cmtd != info->mtd[0])
+ mtd_concat_destroy(info->cmtd);
#endif
+ }
for (i = 0; i < MAX_RESOURCES; i++) {
if (info->mtd[i] != NULL)
@@ -170,22 +168,22 @@ static int physmap_flash_probe(struct platform_device *dev)
if (err)
goto err_out;
- if (mtd_has_partitions()) {
- err = parse_mtd_partitions(info->cmtd, part_probe_types,
- &info->parts, 0);
- if (err > 0) {
- add_mtd_partitions(info->cmtd, info->parts, err);
- info->nr_parts = err;
- return 0;
- }
+#ifdef CONFIG_MTD_PARTITIONS
+ err = parse_mtd_partitions(info->cmtd, part_probe_types,
+ &info->parts, 0);
+ if (err > 0) {
+ add_mtd_partitions(info->cmtd, info->parts, err);
+ info->nr_parts = err;
+ return 0;
+ }
- if (physmap_data->nr_parts) {
- printk(KERN_NOTICE "Using physmap partition information\n");
- add_mtd_partitions(info->cmtd, physmap_data->parts,
- physmap_data->nr_parts);
- return 0;
- }
+ if (physmap_data->nr_parts) {
+ printk(KERN_NOTICE "Using physmap partition information\n");
+ add_mtd_partitions(info->cmtd, physmap_data->parts,
+ physmap_data->nr_parts);
+ return 0;
}
+#endif
add_mtd_device(info->cmtd);
return 0;
--
dwmw2
^ permalink raw reply related
* Re: About the mxc nand driver
From: Juergen Beisert @ 2009-10-20 17:08 UTC (permalink / raw)
To: Berland, Mathieu
Cc: Vladimir Barinov, 'Sascha Hauer', linux-mtd, Eric Benard
In-Reply-To: <2132956280B09448AB947D9C8A291AC6011F88BE@SMFIDF806A.main.fr.ds.corp>
On Dienstag, 20. Oktober 2009, Berland, Mathieu wrote:
> =============== About Freescale driver (NOT mainline driver)
>
> >The original Freescale driver uses this ecc layout for 8/16 bit busses:
> >
> >static struct nand_ecclayout nand_hw_eccoob_8 = {
> > .eccbytes = 5,
> > .eccpos = {6, 7, 8, 9, 10},
> > .oobfree = {{0, 5}, {11, 5}}
> >};
> >
> >static struct nand_ecclayout nand_hw_eccoob_16 = {
> > .eccbytes = 5,
> > .eccpos = {6, 7, 8, 9, 10},
> > .oobfree = {{0, 6}, {12, 4}}
> >};
> >
> >I understand the first one, but doesn't the second one specify a oobfree
>
> which overlaps with ecc data?
>
> ECC is not overlapped for 16 bits since the second fields of oobfree are
> lengths which is confusing.
>
> These settings comply with the MXC Nand Flash controller specification, at
> least with mine which might be outdated :
>
> 8 bits bus :
> Bad block marker @ 5
> ECC @ [6..10] (understand from 6 to 10, both included) = {6, 7, 8,
> 9, 10}
> Free @ [0..4] [11..15] = {{0, 5}, {11, 5}}
>
> 16 bits bus :
> Bad block marker @ 11
> ECC @ [6..10] = {6, 7, 8, 9, 10}
> Free @ [0..5] [12..15] = {{0, 6}, {12, 4}}
>
> The datasheet says :
> "The host can use all of the spare area except for the BI (bad block
> marker) and ECC code areas".
>
> The ECC is managed by the hardware. It seems that the bad block marker is
> not (not sure).
Bad block marker is handled by the NAND framework. And in the most worst case
this bad block marker is overwritten by the ECC hardware...
> What's the problem with the old Freescale's driver ?
> The problem is that Flash chips with 2K pages are not really handled.
>
>
> =============== About "SPARE AREA ASSIGNMENT STANDARD" from Samsung (?)
>
> I found one document in Samsung website. It is said :
>
> Small pages (16B OOB) && 8 bits bus :
> Bad block marker @ 5 => In my Samsung chip, all the bytes of the OOB
> are set to 0 when a block is marked bad in factory.
> ECC @ [6..10] => Chip founders don't care I guess.
It seems every hardware vendor does it in a different way.
> Small pages (16B OOB) && 16 bits bus :
> Bad block marker @ 11
> ECC @ [6..10]
You must register your own bad block marker structure
(member 'badblock_pattern' in struct nand_chip) to use offset 11 instead of
the offset 5.
[...]
> =============== About the mainline driver
>
> Generic BBT code used by mxc_nand.c :
>
> Small pages
> Bad block marker @ 5
>
> Large pages
> Bad block marker @ [0..1]
These are the defaults if the driver keeps member 'badblock_pattern' in struct
nand_chip NULL.
> Here is what Vladimir did :
>
> 8 bytes OOB or SW ECC (?) :
> Bad block marker @ [5]
Default for page size equal to 512.
> ECC @ [6..10]
> Free @ [0..4] [11..15]
>
> Small pages (16B OOB) :
> Bad block marker @ [5]
Default for page size equal to 512.
> ECC @ [6..10]
> Free @ [0..4] [11..15]
>
> Large pages (64B OOB) :
> Bad block marker @ [0..1]
Default if page size is larger than 512 bytes.
> ECC @ [6..10] [22..26] [38..42] [54..58]
> Free @ [2..5] [11..20] [27..36] [43..52] [59..63]
> [...]
> =============== Some improvements ?
>
> The only thing that's worrying me is the fact that the Nand Flash
> Controller might use these bad block marker bytes. In this case, we could
> experience problems.
That's why I'm using a separate bbt descriptor.
> I have somes proposals which should be compatible with current Flash
> loaders. This is *only* if someone is experiencing problems during the
> lifecyle of boards :
>
> 1)
>
> 8 bytes OOB :
> No more supported
>
> Small pages (16B OOB) && 8 bits bus :
> Requirement : keep default bad block marker location, same as today
>
> Bad block marker @ 5
> ECC @ [6..10]
> Free @ [0..4] [11..15]
Bad block marker at offset 5 is described in structure 'smallpage_flashbased'
in 'nand_bbt.c' and will be used if the driver doesn't touch
the 'badblock_pattern' structure member.
> Small pages (16B OOB) && 16 bits bus :
> Requirement : keep default bad block marker location, same as today
> Requirement : byte 11 is not free anymore because it might be used
> by HW for bad block flag (don't known)
>
> Bad block marker @ 5
> ECC @ [6..10]
> Free @ [0..4] [12..15]
Same here. because 'nand_bbt.c' does not distinguish between date width.
> Large pages (64B OOB) && 8 bits bus :
> Requirement : keep default bad block marker location, same as today
> Requirement : byte 5+16*n are not free because it might be used by
> HW for bad block flag (don't known)
>
> Bad block marker @ [0..1]
Yes, because here 'largepage_flashbased' is used for all page sizes larger
than 512 bytes (until the driver touches the 'badblock_pattern' structure
member).
> ECC & [6..10] [22..26] [38..42] [54..58]
> Free @ [2..4] [11..20] [27..36] [43..52] [59..63]
>
> Large pages (64B OOB) && 16 bits bus :
> Requirement : keep default bad block marker location, same as today
> Requirement : byte 11+16*n are not free because it might be used by
> HW for bad block flag (don't known)
>
> Bad block marker @ [0..1]
Yes. No difference between 8 or 16 bits.
jbe
--
Pengutronix e.K. | Juergen Beisert |
Linux Solutions for Science and Industry | Phone: +49-8766-939 228 |
Vertretung Sued/Muenchen, Germany | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de/ |
^ permalink raw reply
* RE: [PATCH] mtd: Fix kernel NULL pointer dereference in physmap.c
From: H Hartley Sweeten @ 2009-10-20 16:52 UTC (permalink / raw)
To: Atsushi Nemoto; +Cc: dwmw2, linux-mtd
In-Reply-To: <20091021.011750.71092490.anemo@mba.ocn.ne.jp>
On Tuesday, October 20, 2009 9:18 AM, Atsushi Nemoto wrote:
> On Tue, 20 Oct 2009 12:08:00 -0400, "H Hartley Sweeten" <hartleys@visionengravers.com> wrote:
>>>> Also, cleanup the #ifdef CONFIG_MTD_PARTITIONS stuff by using
>>>> mtd_has_partitions().
>>>
>>> And this cleanup cause build errors when CONFIG_MTD_PARTITIONS was not
>>> set. A separate patch might be better for such cleanup.
>>
>> Hmm.. Not sure why that would cause a build error. Regardless, I will
>> remove that change from this patch.
>
> Thank you. The build errors are something like:
>
> physmap.c:53: error: 'struct physmap_flash_info' has no member named 'nr_parts'
> physmap.c:174: error: 'part_probe_types' undeclared (first use in this function)
Ok. That makes sense.
struct physmap_flash_info {
struct mtd_info *mtd[MAX_RESOURCES];
struct mtd_info *cmtd;
struct map_info map[MAX_RESOURCES];
#ifdef CONFIG_MTD_PARTITIONS
int nr_parts;
struct mtd_partition *parts;
#endif
};
#ifdef CONFIG_MTD_PARTITIONS
static const char *part_probe_types[] = { "cmdlinepart", "RedBoot", NULL };
#endif
I assumed that mtd_has_partitions() when CONFIG_MTD_PARTITIONS was not defined
would end up as something like this when compiled:
if (mtd_has_partitions()) {
/* ... */
}
Would become:
if (0) {
/* ... */
}
Then it would just optimze away. It appears the code in the if (0) condition is
still parsed by the compiler so you get the errors above since the symbols are
#ifdef'ed out.
Oh well... I did post the updated patch based on your other comment.
Regards,
Hartley
^ permalink raw reply
* RE: [PATCH] mtd: Fix kernel NULL pointer dereference in physmap.c
From: H Hartley Sweeten @ 2009-10-20 16:23 UTC (permalink / raw)
To: linux-mtd; +Cc: Atsushi Nemoto, dwmw2
In-Reply-To: <20091021.002941.41633716.anemo@mba.ocn.ne.jp>
During the probe for physmap platform flash devices there are a
number error exit conditions that all do a goto err_out which
then calls physmap_flash_remove(). In that function one of the
cleanup steps is:
#ifdef CONFIG_MTD_CONCAT
if (info->cmtd != info->mtd[0])
mtd_concat_destroy(info->cmtd);
#endif
This test will succeed since info->cmtd == NULL and info->mtd[0] is
valid, which then causes a NULL pointer dereference when mtd_concat_destroy()
is called. Fix this by moving the mtd_concat_destroy() step into the
if (info->cmtd) condition.
Also, move the kfree(info->parts) cleanup to remove an #ifdef.
Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
---
V2 - As pointed out by Atsushi Nemoto, the map_destroy loop should not
be skipped even when info->cmtd == NULL.
diff --git a/drivers/mtd/maps/physmap.c b/drivers/mtd/maps/physmap.c
index 380648e..3f13a96 100644
--- a/drivers/mtd/maps/physmap.c
+++ b/drivers/mtd/maps/physmap.c
@@ -48,23 +48,22 @@ static int physmap_flash_remove(struct platform_device *dev)
if (info->cmtd) {
#ifdef CONFIG_MTD_PARTITIONS
- if (info->nr_parts || physmap_data->nr_parts)
+ if (info->nr_parts || physmap_data->nr_parts) {
del_mtd_partitions(info->cmtd);
- else
+
+ if (info->nr_parts)
+ kfree(info->parts);
+ } else {
del_mtd_device(info->cmtd);
+ }
#else
del_mtd_device(info->cmtd);
#endif
- }
-#ifdef CONFIG_MTD_PARTITIONS
- if (info->nr_parts)
- kfree(info->parts);
-#endif
-
#ifdef CONFIG_MTD_CONCAT
- if (info->cmtd != info->mtd[0])
- mtd_concat_destroy(info->cmtd);
+ if (info->cmtd != info->mtd[0])
+ mtd_concat_destroy(info->cmtd);
#endif
+ }
for (i = 0; i < MAX_RESOURCES; i++) {
if (info->mtd[i] != NULL)
^ permalink raw reply related
* Re: [PATCH] mtd: Fix kernel NULL pointer dereference in physmap.c
From: Atsushi Nemoto @ 2009-10-20 16:17 UTC (permalink / raw)
To: hartleys; +Cc: dwmw2, linux-mtd
In-Reply-To: <BD79186B4FD85F4B8E60E381CAEE190901E2481C@mi8nycmail19.Mi8.com>
On Tue, 20 Oct 2009 12:08:00 -0400, "H Hartley Sweeten" <hartleys@visionengravers.com> wrote:
> >> Also, cleanup the #ifdef CONFIG_MTD_PARTITIONS stuff by using
> >> mtd_has_partitions().
> >
> > And this cleanup cause build errors when CONFIG_MTD_PARTITIONS was not
> > set. A separate patch might be better for such cleanup.
>
> Hmm.. Not sure why that would cause a build error. Regardless, I will
> remove that change from this patch.
Thank you. The build errors are something like:
physmap.c:53: error: 'struct physmap_flash_info' has no member named 'nr_parts'
physmap.c:174: error: 'part_probe_types' undeclared (first use in this function)
---
Atsushi Nemoto
^ permalink raw reply
* RE: [PATCH] mtd: Fix kernel NULL pointer dereference in physmap.c
From: H Hartley Sweeten @ 2009-10-20 16:08 UTC (permalink / raw)
To: Atsushi Nemoto; +Cc: dwmw2, linux-mtd
In-Reply-To: <20091021.002941.41633716.anemo@mba.ocn.ne.jp>
On Tuesday, October 20, 2009 8:30 AM, Atsushi Nemoto wrote:
> On Mon, 19 Oct 2009 13:31:46 -0400, "H Hartley Sweeten" <hartleys@visionengravers.com> wrote:
>> During the probe for physmap platform flash devices there are a
>> number error exit conditions that all do a goto err_out which
>> then calls physmap_flash_remove(). In that function one of the
>> cleanup steps is:
>>
>> #ifdef CONFIG_MTD_CONCAT
>> if (info->cmtd != info->mtd[0])
>> mtd_concat_destroy(info->cmtd);
>> #endif
>>
>> This test will succeed since info->cmtd == NULL and info->mtd[0] is
>> valid.
>
> Oh I had missed this case when fixing physmap_flash_remove last time.
>
>> Fix this by exiting the remove function when info->cmtd == NULL.
>
> No, map_destroy loop at the end of the function should not be skipped
> even when info->cmtd == NULL.
Missed that part. I will modify the patch and repost.
>> Also, cleanup the #ifdef CONFIG_MTD_PARTITIONS stuff by using
>> mtd_has_partitions().
>
> And this cleanup cause build errors when CONFIG_MTD_PARTITIONS was not
> set. A separate patch might be better for such cleanup.
Hmm.. Not sure why that would cause a build error. Regardless, I will
remove that change from this patch.
Regards,
Hartley
^ permalink raw reply
* should/can we do ubidetach on reboot
From: Norbert van Bolhuis @ 2009-10-20 16:03 UTC (permalink / raw)
To: Mtd List
I guess it's safe/normal to detach an MTD device from UBI before rebooting ?
I'm asking because detaching is the only way to shut down the UBI
background task (ubi_bgt0d). I do not want this task to start a flash erase
operation or whatsoever just before the actual reboot takes place.
^ permalink raw reply
* Re: UBIFS problem on MPC8536DS
From: Felix Radensky @ 2009-10-20 15:30 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev@ozlabs.org, linux-mtd@lists.infradead.org
In-Reply-To: <4ADCC247.6080107@freescale.com>
Scott Wood wrote:
> Felix Radensky wrote:
>> OK, no problem. I just wanted to get an idea of what should be done.
>> Should the NOR code poll some eLBC register to wait for completion of
>> NAND special operation ? Can you tell what register is relevant ?
>
> I was thinking you'd just share a mutex with the NAND code.
>
OK, I have NOR and NAND synchronized properly now.
Thanks a lot for your help.
Felix.
^ permalink raw reply
* Re: [PATCH] mtd: Fix kernel NULL pointer dereference in physmap.c
From: Atsushi Nemoto @ 2009-10-20 15:29 UTC (permalink / raw)
To: hartleys; +Cc: dwmw2, linux-mtd
In-Reply-To: <BD79186B4FD85F4B8E60E381CAEE190901E246A0@mi8nycmail19.Mi8.com>
On Mon, 19 Oct 2009 13:31:46 -0400, "H Hartley Sweeten" <hartleys@visionengravers.com> wrote:
> During the probe for physmap platform flash devices there are a
> number error exit conditions that all do a goto err_out which
> then calls physmap_flash_remove(). In that function one of the
> cleanup steps is:
>
> #ifdef CONFIG_MTD_CONCAT
> if (info->cmtd != info->mtd[0])
> mtd_concat_destroy(info->cmtd);
> #endif
>
> This test will succeed since info->cmtd == NULL and info->mtd[0] is
> valid.
Oh I had missed this case when fixing physmap_flash_remove last time.
> Fix this by exiting the remove function when info->cmtd == NULL.
No, map_destroy loop at the end of the function should not be skipped
even when info->cmtd == NULL.
> Also, cleanup the #ifdef CONFIG_MTD_PARTITIONS stuff by using
> mtd_has_partitions().
And this cleanup cause build errors when CONFIG_MTD_PARTITIONS was not
set. A separate patch might be better for such cleanup.
---
Atsushi Nemoto
^ permalink raw reply
* Re: [PATCH] [OneNAND] OTP support re-implementation 1/1
From: Adrian Hunter @ 2009-10-20 14:13 UTC (permalink / raw)
To: dedekind1@gmail.com
Cc: Amul Kumar Saha, Kyungmin Park, David Woodhouse,
linux-mtd@lists.infradead.org
In-Reply-To: <1256028984.29856.144.camel@localhost>
Artem Bityutskiy wrote:
> I'm still waiting for Adrian to review this. Yesterday he said he will
> do this soon. In any case, I remember about this patch, just FYI.
Seems fine except why aren't
onenand_otp_command()
onenand_otp_write_oob_nolock()
inside an
#ifdef CONFIG_MTD_ONENAND_OTP
...
#endif
> On Mon, 2009-10-12 at 11:31 +0530, Amul Kumar Saha wrote:
>> What is OTP in OneNAND?
>> The device includes,
>> 1. one block-sized OTP (One Time Programmable) area and
>> 2. user-controlled 1st block OTP(Block 0)
>> that can be used to increase system security or to provide identification capabilities.
>>
>> What is done?
>> In OneNAND, one block of the NAND Array is set aside as an OTP memory area, and 1st Block (Block 0)
>> can be used as OTP area.
>> This area, available to the user, can be configured and locked with secured user information.
>> The OTP block can be read, programmed and locked using the same operations as any other NAND Flash
>> Array
>> memory block. After issuing an OTP-Lock, OTP block cannot be erased. OTP block is fully-guaranteed
>> to be a valid block.
>>
>> Why it is done? (Impact)
>> Locking the 1st Block OTP has the effect of a 'Write-protect' to guard against accidental
>> re-programming of data stored in the 1st block and OTP Block.
>>
>> Which problem it solves?
>> OTP support is provided in the existing implementation of OneNAND/Flex-OneNAND driver, but it is not
>> working with OneNAND devices.
>> Have observed the following in current OTP OneNAND Implmentation,
>> 1. DataSheet specific sequence to lock the OTP Area is not followed.
>> 2. Certain functions are quiet generic to cope with OTP specific activity.
>> This patch re-implements OTP support for OneNAND device.
>>
>> How it is done?
>> For all blocks, 8th word is available to the user.
>> However,in case of OTP Block, 8th word of sector 0, page 0 is reserved as OTP Locking Bit area.
>> Therefore, in case of OTP Block, user usage on this area is prohibited.
>> Condition specific values are entered in the 8th word, sector0, page 0 of the OTP block during the
>> process of issuing an OTP-Lock.
>> The possible conditions are:-
>> 1. Only 1st Block Lock
>> 2. Only OTP Block Lock
>> 3. Lock both the 1st Block and the OTP Block
>>
>> What Other feature additions have been done in this patch?
>> This patch adds feature for:-
>> 1. Only 1st Block Lock
>> 2. Lock both the 1st Block and the OTP Blocks
>>
>> Re-implemented OTP support for OneNAND
>> Added following features to OneNAND
>> 1. Lock only 1st Block in OneNAND
>> 2. Lock BOTH 1st Block and OTP Block in OneNAND
>>
>> Signed-off-by: Amul Kumar Saha <amul.saha at samsung.com>
>> ---
>> drivers/mtd/onenand/onenand_base.c | 287 +++++++++++++++++++++++++++++++++----
>> include/linux/mtd/onenand.h | 2
>> 2 files changed, 260 insertions(+), 29 deletions(-)
>>
>> diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c
>> index cb405bc..4d28268 100644
>> --- a/drivers/mtd/onenand/onenand_base.c
>> +++ b/drivers/mtd/onenand/onenand_base.c
>> @@ -11,7 +11,9 @@
>> *
>> * Vishak G <vishak.g at samsung.com>, Rohit Hagargundgi <h.rohit at samsung.com>
>> * Flex-OneNAND support
>> - * Copyright (C) Samsung Electronics, 2008
>> + * Amul Kumar Saha <amul.saha at samsung.com>
>> + * OTP support
>> + * Copyright (C) Samsung Electronics, 2009
>> *
>> * This program is free software; you can redistribute it and/or modify
>> * it under the terms of the GNU General Public License version 2 as
>> @@ -43,6 +45,18 @@ MODULE_PARM_DESC(flex_bdry, "SLC Boundary information for Flex-OneNAND"
>> " : 0->Set boundary in unlocked status"
>> " : 1->Set boundary in locked status");
>>
>> +/* Default OneNAND/Flex-OneNAND OTP options*/
>> +static int otp;
>> +
>> +module_param(otp, int, 0400);
>> +MODULE_PARM_DESC(otp, "Corresponding behaviour of OneNAND in OTP"
>> + "Syntax : otp=LOCK_TYPE"
>> + "LOCK_TYPE : Keys issued, for specific OTP Lock type"
>> + " : 0 -> Default (No Blocks Locked)"
>> + " : 1 -> OTP Block lock"
>> + " : 2 -> 1st Block lock"
>> + " : 3 -> BOTH OTP Block and 1st Block lock");
>> +
>> /**
>> * onenand_oob_128 - oob info for Flex-Onenand with 4KB page
>> * For now, we expose only 64 out of 80 ecc bytes
>> @@ -308,6 +322,81 @@ int flexonenand_region(struct mtd_info *mtd, loff_t addr)
>> EXPORT_SYMBOL(flexonenand_region);
>>
>> /**
>> + * onenand_otp_command - Send OTP specific command to OneNAND device
>> + * @param mtd MTD device structure
>> + * @param cmd the command to be sent
>> + * @param addr offset to read from or write to
>> + * @param len number of bytes to read or write
>> + */
>> +static int onenand_otp_command(struct mtd_info *mtd, int cmd, loff_t addr,
>> + size_t len)
>> +{
>> + struct onenand_chip *this = mtd->priv;
>> + int value, block, page;
>> +
>> + /* Address translation */
>> + switch (cmd) {
>> + case ONENAND_CMD_OTP_ACCESS:
>> + block = (int) (addr >> this->erase_shift);
>> + page = -1;
>> + break;
>> +
>> + default:
>> + block = (int) (addr >> this->erase_shift);
>> + page = (int) (addr >> this->page_shift);
>> +
>> + if (ONENAND_IS_2PLANE(this)) {
>> + /* Make the even block number */
>> + block &= ~1;
>> + /* Is it the odd plane? */
>> + if (addr & this->writesize)
>> + block++;
>> + page >>= 1;
>> + }
>> + page &= this->page_mask;
>> + break;
>> + }
>> +
>> + if (block != -1) {
>> + /* Write 'DFS, FBA' of Flash */
>> + value = onenand_block_address(this, block);
>> + this->write_word(value, this->base +
>> + ONENAND_REG_START_ADDRESS1);
>> + }
>> +
>> + if (page != -1) {
>> + /* Now we use page size operation */
>> + int sectors = 4, count = 4;
>> + int dataram;
>> +
>> + switch (cmd) {
>> + default:
>> + if (ONENAND_IS_2PLANE(this) && cmd == ONENAND_CMD_PROG)
>> + cmd = ONENAND_CMD_2X_PROG;
>> + dataram = ONENAND_CURRENT_BUFFERRAM(this);
>> + break;
>> + }
>> +
>> + /* Write 'FPA, FSA' of Flash */
>> + value = onenand_page_address(page, sectors);
>> + this->write_word(value, this->base +
>> + ONENAND_REG_START_ADDRESS8);
>> +
>> + /* Write 'BSA, BSC' of DataRAM */
>> + value = onenand_buffer_address(dataram, sectors, count);
>> + this->write_word(value, this->base + ONENAND_REG_START_BUFFER);
>> + }
>> +
>> + /* Interrupt clear */
>> + this->write_word(ONENAND_INT_CLEAR, this->base + ONENAND_REG_INTERRUPT);
>> +
>> + /* Write command */
>> + this->write_word(cmd, this->base + ONENAND_REG_COMMAND);
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> * onenand_command - [DEFAULT] Send command to OneNAND device
>> * @param mtd MTD device structure
>> * @param cmd the command to be sent
>> @@ -1969,6 +2058,133 @@ static int onenand_write_ops_nolock(struct mtd_info *mtd, loff_t to,
>>
>>
>> /**
>> + * onenand_otp_write_oob_nolock - [Internal] OneNAND write out-of-band, specific to OTP
>> + * @param mtd MTD device structure
>> + * @param to offset to write to
>> + * @param len number of bytes to write
>> + * @param retlen pointer to variable to store the number of written bytes
>> + * @param buf the data to write
>> + *
>> + * OneNAND write out-of-band only for OTP
>> + */
>> +static int onenand_otp_write_oob_nolock(struct mtd_info *mtd, loff_t to,
>> + struct mtd_oob_ops *ops)
>> +{
>> + struct onenand_chip *this = mtd->priv;
>> + int column, ret = 0, oobsize;
>> + int written = 0;
>> + u_char *oobbuf;
>> + size_t len = ops->ooblen;
>> + const u_char *buf = ops->oobbuf;
>> + int block, value, status;
>> +
>> + to += ops->ooboffs;
>> +
>> + /* Initialize retlen, in case of early exit */
>> + ops->oobretlen = 0;
>> +
>> + oobsize = mtd->oobsize;
>> +
>> + column = to & (mtd->oobsize - 1);
>> +
>> + oobbuf = this->oob_buf;
>> +
>> + /* Loop until all data write */
>> + while (written < len) {
>> + int thislen = min_t(int, oobsize, len - written);
>> +
>> + cond_resched();
>> +
>> + block = (int) (to >> this->erase_shift);
>> + /*
>> + * Write 'DFS, FBA' of Flash
>> + * Add: F100h DQ=DFS, FBA
>> + */
>> +
>> + value = onenand_block_address(this, block);
>> + this->write_word(value, this->base +
>> + ONENAND_REG_START_ADDRESS1);
>> +
>> + /*
>> + * Select DataRAM for DDP
>> + * Add: F101h DQ=DBS
>> + */
>> +
>> + value = onenand_bufferram_address(this, block);
>> + this->write_word(value, this->base +
>> + ONENAND_REG_START_ADDRESS2);
>> + ONENAND_SET_NEXT_BUFFERRAM(this);
>> +
>> + /*
>> + * Enter OTP access mode
>> + */
>> + this->command(mtd, ONENAND_CMD_OTP_ACCESS, 0, 0);
>> + this->wait(mtd, FL_OTPING);
>> +
>> + /* We send data to spare ram with oobsize
>> + * to prevent byte access */
>> + memcpy(oobbuf + column, buf, thislen);
>> +
>> + /*
>> + * Write Data into DataRAM
>> + * Add: 8th Word
>> + * in sector0/spare/page0
>> + * DQ=XXFCh
>> + */
>> + this->write_bufferram(mtd, ONENAND_SPARERAM,
>> + oobbuf, 0, mtd->oobsize);
>> +
>> + onenand_otp_command(mtd, ONENAND_CMD_PROGOOB, to, mtd->oobsize);
>> + onenand_update_bufferram(mtd, to, 0);
>> + if (ONENAND_IS_2PLANE(this)) {
>> + ONENAND_SET_BUFFERRAM1(this);
>> + onenand_update_bufferram(mtd, to + this->writesize, 0);
>> + }
>> +
>> + ret = this->wait(mtd, FL_WRITING);
>> + if (ret) {
>> + printk(KERN_ERR "%s: write failed %d\n", __func__, ret);
>> + break;
>> + }
>> +
>> + /* Exit OTP access mode */
>> + this->command(mtd, ONENAND_CMD_RESET, 0, 0);
>> + this->wait(mtd, FL_RESETING);
>> +
>> + status = this->read_word(this->base + ONENAND_REG_CTRL_STATUS);
>> + status &= 0x60;
>> +
>> + if (status == 0x60) {
>> + printk(KERN_DEBUG "\nBLOCK\tSTATUS\n");
>> + printk(KERN_DEBUG "1st Block\tLOCKED\n");
>> + printk(KERN_DEBUG "OTP Block\tLOCKED\n");
>> + } else if (status == 0x20) {
>> + printk(KERN_DEBUG "\nBLOCK\tSTATUS\n");
>> + printk(KERN_DEBUG "1st Block\tLOCKED\n");
>> + printk(KERN_DEBUG "OTP Block\tUN-LOCKED\n");
>> + } else if (status == 0x40) {
>> + printk(KERN_DEBUG "\nBLOCK\tSTATUS\n");
>> + printk(KERN_DEBUG "1st Block\tUN-LOCKED\n");
>> + printk(KERN_DEBUG "OTP Block\tLOCKED\n");
>> + } else {
>> + printk(KERN_DEBUG "Reboot to check\n");
>> + }
>> +
>> + written += thislen;
>> + if (written == len)
>> + break;
>> +
>> + to += mtd->writesize;
>> + buf += thislen;
>> + column = 0;
>> + }
>> +
>> + ops->oobretlen = written;
>> +
>> + return ret;
>> +}
>> +
>> +/**
>> * onenand_write_oob_nolock - [Internal] OneNAND write out-of-band
>> * @param mtd MTD device structure
>> * @param to offset to write to
>> @@ -2693,11 +2909,11 @@ static int do_otp_lock(struct mtd_info *mtd, loff_t from, size_t len,
>> struct mtd_oob_ops ops;
>> int ret;
>>
>> - /* Enter OTP access mode */
>> - this->command(mtd, ONENAND_CMD_OTP_ACCESS, 0, 0);
>> - this->wait(mtd, FL_OTPING);
>> -
>> if (FLEXONENAND(this)) {
>> +
>> + /* Enter OTP access mode */
>> + this->command(mtd, ONENAND_CMD_OTP_ACCESS, 0, 0);
>> + this->wait(mtd, FL_OTPING);
>> /*
>> * For Flex-OneNAND, we write lock mark to 1st word of sector 4 of
>> * main area of page 49.
>> @@ -2708,19 +2924,19 @@ static int do_otp_lock(struct mtd_info *mtd, loff_t from, size_t len,
>> ops.oobbuf = NULL;
>> ret = onenand_write_ops_nolock(mtd, mtd->writesize * 49, &ops);
>> *retlen = ops.retlen;
>> +
>> + /* Exit OTP access mode */
>> + this->command(mtd, ONENAND_CMD_RESET, 0, 0);
>> + this->wait(mtd, FL_RESETING);
>> } else {
>> ops.mode = MTD_OOB_PLACE;
>> ops.ooblen = len;
>> ops.oobbuf = buf;
>> ops.ooboffs = 0;
>> - ret = onenand_write_oob_nolock(mtd, from, &ops);
>> + ret = onenand_otp_write_oob_nolock(mtd, from, &ops);
>> *retlen = ops.oobretlen;
>> }
>>
>> - /* Exit OTP access mode */
>> - this->command(mtd, ONENAND_CMD_RESET, 0, 0);
>> - this->wait(mtd, FL_RESETING);
>> -
>> return ret;
>> }
>>
>> @@ -2751,16 +2967,21 @@ static int onenand_otp_walk(struct mtd_info *mtd, loff_t from, size_t len,
>> if (density < ONENAND_DEVICE_DENSITY_512Mb)
>> otp_pages = 20;
>> else
>> - otp_pages = 10;
>> + otp_pages = 50;
>>
>> if (mode == MTD_OTP_FACTORY) {
>> from += mtd->writesize * otp_pages;
>> - otp_pages = 64 - otp_pages;
>> + otp_pages = ONENAND_PAGES_PER_BLOCK - otp_pages;
>> }
>>
>> /* Check User/Factory boundary */
>> - if (((mtd->writesize * otp_pages) - (from + len)) < 0)
>> - return 0;
>> + if (mode == MTD_OTP_USER) {
>> + if (((mtd->writesize * otp_pages) - (from + len)) < 0)
>> + return 0;
>> + } else {
>> + if (((mtd->writesize * otp_pages) - len) < 0)
>> + return 0;
>> + }
>>
>> onenand_get_device(mtd, FL_OTPING);
>> while (len > 0 && otp_pages > 0) {
>> @@ -2783,13 +3004,12 @@ static int onenand_otp_walk(struct mtd_info *mtd, loff_t from, size_t len,
>> *retlen += sizeof(struct otp_info);
>> } else {
>> size_t tmp_retlen;
>> - int size = len;
>>
>> ret = action(mtd, from, len, &tmp_retlen, buf);
>>
>> - buf += size;
>> - len -= size;
>> - *retlen += size;
>> + buf += tmp_retlen;
>> + len -= tmp_retlen;
>> + *retlen += tmp_retlen;
>>
>> if (ret)
>> break;
>> @@ -2902,21 +3122,11 @@ static int onenand_lock_user_prot_reg(struct mtd_info *mtd, loff_t from,
>> u_char *buf = FLEXONENAND(this) ? this->page_buf : this->oob_buf;
>> size_t retlen;
>> int ret;
>> + unsigned int otp_lock_offset = ONENAND_OTP_LOCK_OFFSET;
>>
>> memset(buf, 0xff, FLEXONENAND(this) ? this->writesize
>> : mtd->oobsize);
>> /*
>> - * Note: OTP lock operation
>> - * OTP block : 0xXXFC
>> - * 1st block : 0xXXF3 (If chip support)
>> - * Both : 0xXXF0 (If chip support)
>> - */
>> - if (FLEXONENAND(this))
>> - buf[FLEXONENAND_OTP_LOCK_OFFSET] = 0xFC;
>> - else
>> - buf[ONENAND_OTP_LOCK_OFFSET] = 0xFC;
>> -
>> - /*
>> * Write lock mark to 8th word of sector0 of page0 of the spare0.
>> * We write 16 bytes spare area instead of 2 bytes.
>> * For Flex-OneNAND, we write lock mark to 1st word of sector 4 of
>> @@ -2926,6 +3136,25 @@ static int onenand_lock_user_prot_reg(struct mtd_info *mtd, loff_t from,
>> from = 0;
>> len = FLEXONENAND(this) ? mtd->writesize : 16;
>>
>> + /*
>> + * Note: OTP lock operation
>> + * OTP block : 0xXXFC XX 1111 1100
>> + * 1st block : 0xXXF3 (If chip support) XX 1111 0011
>> + * Both : 0xXXF0 (If chip support) XX 1111 0000
>> + */
>> + if (FLEXONENAND(this))
>> + otp_lock_offset = FLEXONENAND_OTP_LOCK_OFFSET;
>> +
>> + /* ONENAND_OTP_AREA | ONENAND_OTP_BLOCK0 | ONENAND_OTP_AREA_BLOCK0 */
>> + if (otp == 1)
>> + buf[otp_lock_offset] = 0xFC;
>> + else if (otp == 2)
>> + buf[otp_lock_offset] = 0xF3;
>> + else if (otp == 3)
>> + buf[otp_lock_offset] = 0xF0;
>> + else if (otp != 0)
>> + printk(KERN_DEBUG "[OneNAND] Invalid option selected for OTP\n");
>> +
>> ret = onenand_otp_walk(mtd, from, len, &retlen, buf, do_otp_lock, MTD_OTP_USER);
>>
>> return ret ? : retlen;
>> diff --git a/include/linux/mtd/onenand.h b/include/linux/mtd/onenand.h
>> index 4e49f33..3c00b55 100644
>> --- a/include/linux/mtd/onenand.h
>> +++ b/include/linux/mtd/onenand.h
>> @@ -152,6 +152,8 @@ struct onenand_chip {
>> /*
>> * Helper macros
>> */
>> +#define ONENAND_PAGES_PER_BLOCK (1<<6)
>> +
>> #define ONENAND_CURRENT_BUFFERRAM(this) (this->bufferram_index)
>> #define ONENAND_NEXT_BUFFERRAM(this) (this->bufferram_index ^ 1)
>> #define ONENAND_SET_NEXT_BUFFERRAM(this) (this->bufferram_index ^= 1)
>>
>>
> --
> Best Regards,
> Artem Bityutskiy (Артём Битюцкий)
>
^ permalink raw reply
* Re: About the mxc nand driver
From: Sascha Hauer @ 2009-10-20 13:32 UTC (permalink / raw)
To: Juergen Beisert; +Cc: Vladimir Barinov, mathieu.berland, linux-mtd, Eric Benard
In-Reply-To: <200910201528.55271.jbe@pengutronix.de>
On Tue, Oct 20, 2009 at 03:28:54PM +0200, Juergen Beisert wrote:
> On Dienstag, 20. Oktober 2009, Sascha Hauer wrote:
> > [...]
> > The original Freescale driver uses this ecc layout for 8/16 bit busses:
> >
> > static struct nand_ecclayout nand_hw_eccoob_8 = {
> > .eccbytes = 5,
> > .eccpos = {6, 7, 8, 9, 10},
> > .oobfree = {{0, 5}, {11, 5}}
> > };
>
> This one is bogus, as it describes an 8 byte OOB with more than 8 bytes...
Well, in current mainline driver it is, but in the Freescale driver
where this snipped has been copied from it is used for 8bit busses
rather than 8bit oob size.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: About the mxc nand driver
From: Juergen Beisert @ 2009-10-20 13:28 UTC (permalink / raw)
To: Sascha Hauer; +Cc: Vladimir Barinov, mathieu.berland, linux-mtd, Eric Benard
In-Reply-To: <20091020131157.GR8818@pengutronix.de>
On Dienstag, 20. Oktober 2009, Sascha Hauer wrote:
> [...]
> The original Freescale driver uses this ecc layout for 8/16 bit busses:
>
> static struct nand_ecclayout nand_hw_eccoob_8 = {
> .eccbytes = 5,
> .eccpos = {6, 7, 8, 9, 10},
> .oobfree = {{0, 5}, {11, 5}}
> };
This one is bogus, as it describes an 8 byte OOB with more than 8 bytes... In
the current driver this layout is used for soft ECC. Maybe this is the
reason, why there wasn't any failure messages, because all people are using
hardware ECC.
jbe
--
Pengutronix e.K. | Juergen Beisert |
Linux Solutions for Science and Industry | Phone: +49-8766-939 228 |
Vertretung Sued/Muenchen, Germany | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de/ |
^ permalink raw reply
* About the mxc nand driver
From: Sascha Hauer @ 2009-10-20 13:11 UTC (permalink / raw)
To: linux-mtd
Cc: Vladimir Barinov, mathieu.berland, Juergen Beisert,
Robert Schwebel, Eric Benard
Hi all,
I frequently get reports about bugs in the Freescale mxc nand driver.
Juergen Beisert and me invested some time to see what's going on in this
driver, so here are some results.
>
> /* OOB placement block for use with hardware ecc generation */
> static struct nand_ecclayout nand_hw_eccoob_8 = {
> .eccbytes = 5,
> .eccpos = {6, 7, 8, 9, 10},
> .oobfree = {{0, 5}, {11, 5}, }
> };
>
> static struct nand_ecclayout nand_hw_eccoob_16 = {
> .eccbytes = 5,
> .eccpos = {6, 7, 8, 9, 10},
> .oobfree = {{0, 5}, {11, 5}, }
> };
>
> static struct nand_ecclayout nand_hw_eccoob_64 = {
> .eccbytes = 20,
> .eccpos = {6, 7, 8, 9, 10, 22, 23, 24, 25, 26,
> 38, 39, 40, 41, 42, 54, 55, 56, 57, 58},
> .oobfree = {{2, 4}, {11, 10}, {27, 10}, {43, 10}, {59, 5}, }
> };
>
>
> /* Define some generic bad / good block scan pattern which are used
> * while scanning a device for factory marked good / bad blocks. */
> static uint8_t scan_ff_pattern[] = { 0xff, 0xff };
>
> static struct nand_bbt_descr smallpage_memorybased = {
> .options = NAND_BBT_SCAN2NDPAGE,
> .offs = 5,
> .len = 1,
> .pattern = scan_ff_pattern
> };
This has been introduced by Eric Benard. He said he needs this to make
the driver work on 2k flashes because otherwise all blocks on his nand
are marked as bad. Could it be that Eric somehow destroyed his oob data?
Without this largepage_memorybased would be used which has .offs = 0, .len = 2.
When Eric does not have 0xff,0xff here (but really *should* have, but
has used a broken driver some time ago) all blocks would be marked as bad.
Looking at it, the original Freescale driver passes .offset = 0, length = 5
to the 2k ecclayout, so jffs2 will probably overwrite bytes 0 and 1.
This does not hurt the Freescale driver since it uses a flash based
bbt but breaks when he starts using the Mainline driver which does not
use a flash based bbt.
>
> if (this->ecc.mode == NAND_ECC_HW) {
> switch (mtd->oobsize) {
> case 8:
> this->ecc.layout = &nand_hw_eccoob_8;
This has been introduced by Vladimir Barinov and seems wrong. The
original Freescale driver has a nand_hw_eccoob_8 and a
nand_hw_eccoob_16, but it is used for 8bit bus width vs. 16bit bus width
and *not* for 8 byte oob size (do we have these chips anyway nowadays?)
The original Freescale driver uses this ecc layout for 8/16 bit busses:
static struct nand_ecclayout nand_hw_eccoob_8 = {
.eccbytes = 5,
.eccpos = {6, 7, 8, 9, 10},
.oobfree = {{0, 5}, {11, 5}}
};
static struct nand_ecclayout nand_hw_eccoob_16 = {
.eccbytes = 5,
.eccpos = {6, 7, 8, 9, 10},
.oobfree = {{0, 6}, {12, 4}}
};
I understand the first one, but doesn't the second one specify a oobfree
which overlaps with ecc data?
When I understand the mtd layer correctly it uses byte 5 of the oob data
for bad block detection, but this is passed to oobfree causing potential
corruption, so we should probably skip byte 5 from oobfree.
The original Freescale driver uses NAND_USE_FLASH_BBT and
NAND_BBT_CREATE | NAND_BBT_WRITE, so we do not need to store the bad
block information after the first scan anymore. Should we use this here
aswell?
> break;
> case 16:
> this->ecc.layout = &nand_hw_eccoob_16;
> break;
> case 64:
> this->ecc.layout = &nand_hw_eccoob_64;
> break;
> default:
> /* page size not handled by HW ECC */
> /* switching back to soft ECC */
> this->ecc.size = 512;
> this->ecc.bytes = 3;
> this->ecc.layout = &nand_hw_eccoob_8;
> this->ecc.mode = NAND_ECC_SOFT;
> this->ecc.calculate = NULL;
> this->ecc.correct = NULL;
> this->ecc.hwctl = NULL;
> tmp = readw(host->regs + NFC_CONFIG1);
> tmp &= ~NFC_ECC_EN;
> writew(tmp, host->regs + NFC_CONFIG1);
> break;
Which default does this code handle? Has anyone tested it?
As a conclusion I would:
- revert Erics patch
- make sure we do not pass byte 5 for small pages to oobfree
- make sure we do not pass bytes 0/1 for large pages to oobfree
- switch to use a flash based bbt?
I think doing this we break several systems out there, but do we have
another chance?
As a general note I really want to make this driver work on i.MX35/25. I
already have several patches in my queue, but they have to be rebased on
the current driver and I'm still working on it.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: NAND: Add flags to the probe calls to control scan behaviour
From: Ben Dooks @ 2009-10-20 13:11 UTC (permalink / raw)
To: dedekind1; +Cc: linux-mtd, Simtec Linux Team
In-Reply-To: <1256040395.29856.228.camel@localhost>
Artem Bityutskiy wrote:
> On Mon, 2009-10-19 at 11:36 +0100, Ben Dooks wrote:
>> I'd rather it be silent if it cannot find a device, as a number of our boards
>> have slots where NAND devices may be fitted by the customer and as such all
>> possibilities are registered with the NAND driver.
>>
>>> Could you please elaborate why more why is this needed a bit more? What
>>> is the driver?
>> Because customers get scared when errors with '!!!' turn up.
>
>>> Why not to just remove that print at all?
>> Possible, but what about the case where there is a legitimate problem with
>> the device that is supposed to be there.
>
> I would go for this instead. I think the drivers can print an error
> message themselves.
I was trying to avoid touching the 40+ drivers which use this
code. This would also be a bit of a code bloat as we would have
some interesting checks for the precise error code (was the failure
no-chip, no memory, some other problem?)
> Or can this be done using the chip->options ?
I will look into this.
--
Ben Dooks, Software Engineer, Simtec Electronics
http://www.simtec.co.uk/
^ permalink raw reply
* Re: [PATCH v5 2/3] MTD: OneNAND: multiblock erase support
From: Adrian Hunter @ 2009-10-20 13:09 UTC (permalink / raw)
To: Korhonen Mika.2 (EXT-Ardites/Oulu)
Cc: amul.saha@samsung.com, Bityutskiy Artem (Nokia-D/Helsinki),
kyungmin.park@samsung.com, linux-mtd@lists.infradead.org
In-Reply-To: <1255341165-1972-3-git-send-email-ext-mika.2.korhonen@nokia.com>
Korhonen Mika.2 (EXT-Ardites/Oulu) wrote:
> Add support for multiblock erase command. OneNANDs (excluding Flex-OneNAND)
> are capable of simultaneous erase of up to 64 eraseblocks which is much faster.
>
> This changes the erase requests for regions covering multiple eraseblocks
> to be performed using multiblock erase.
>
> Signed-off-by: Mika Korhonen <ext-mika.2.korhonen@nokia.com>
> ---
Just a comment about fail_addr
> drivers/mtd/onenand/omap2.c | 22 ++++-
> drivers/mtd/onenand/onenand_base.c | 173 +++++++++++++++++++++++++++++++++++-
> include/linux/mtd/flashchip.h | 4 +-
> include/linux/mtd/onenand_regs.h | 2 +
> 4 files changed, 194 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mtd/onenand/omap2.c b/drivers/mtd/onenand/omap2.c
> index 0108ed4..2dafd09 100644
> --- a/drivers/mtd/onenand/omap2.c
> +++ b/drivers/mtd/onenand/omap2.c
> @@ -112,10 +112,24 @@ static int omap2_onenand_wait(struct mtd_info *mtd, int state)
> unsigned long timeout;
> u32 syscfg;
>
> - if (state == FL_RESETING) {
> - int i;
> + if (state == FL_RESETING || state == FL_PREPARING_ERASE ||
> + state == FL_VERIFYING_ERASE) {
> + int i = 21;
> + unsigned int intr_flags = ONENAND_INT_MASTER;
> +
> + switch (state) {
> + case FL_RESETING:
> + intr_flags |= ONENAND_INT_RESET;
> + break;
> + case FL_PREPARING_ERASE:
> + intr_flags |= ONENAND_INT_ERASE;
> + break;
> + case FL_VERIFYING_ERASE:
> + i = 101;
> + break;
> + }
>
> - for (i = 0; i < 20; i++) {
> + while (--i) {
> udelay(1);
> intr = read_reg(c, ONENAND_REG_INTERRUPT);
> if (intr & ONENAND_INT_MASTER)
> @@ -126,7 +140,7 @@ static int omap2_onenand_wait(struct mtd_info *mtd, int state)
> wait_err("controller error", state, ctrl, intr);
> return -EIO;
> }
> - if (!(intr & ONENAND_INT_RESET)) {
> + if ((intr & intr_flags) != intr_flags) {
> wait_err("timeout", state, ctrl, intr);
> return -EIO;
> }
> diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c
> index 51f4782..7355f62 100644
> --- a/drivers/mtd/onenand/onenand_base.c
> +++ b/drivers/mtd/onenand/onenand_base.c
> @@ -32,6 +32,13 @@
>
> #include <asm/io.h>
>
> +/*
> + * Multiblock erase if number of blocks to erase is 2 or more.
> + * Maximum number of blocks for simultaneous erase is 64.
> + */
> +#define MB_ERASE_MIN_BLK_COUNT 2
> +#define MB_ERASE_MAX_BLK_COUNT 64
> +
> /* Default Flex-OneNAND boundary and lock respectively */
> static int flex_bdry[MAX_DIES * 2] = { -1, 0, -1, 0 };
>
> @@ -339,6 +346,8 @@ static int onenand_command(struct mtd_info *mtd, int cmd, loff_t addr, size_t le
> break;
>
> case ONENAND_CMD_ERASE:
> + case ONENAND_CMD_MULTIBLOCK_ERASE:
> + case ONENAND_CMD_ERASE_VERIFY:
> case ONENAND_CMD_BUFFERRAM:
> case ONENAND_CMD_OTP_ACCESS:
> block = onenand_block(this, addr);
> @@ -483,7 +492,7 @@ static int onenand_wait(struct mtd_info *mtd, int state)
> if (interrupt & flags)
> break;
>
> - if (state != FL_READING)
> + if (state != FL_READING && state != FL_PREPARING_ERASE)
> cond_resched();
> }
> /* To get correct interrupt status in timeout case */
> @@ -516,6 +525,18 @@ static int onenand_wait(struct mtd_info *mtd, int state)
> return -EIO;
> }
>
> + if (state == FL_PREPARING_ERASE && !(interrupt & ONENAND_INT_ERASE)) {
> + printk(KERN_ERR "%s: mb erase timeout! ctrl=0x%04x intr=0x%04x\n",
> + __func__, ctrl, interrupt);
> + return -EIO;
> + }
> +
> + if (!(interrupt & ONENAND_INT_MASTER)) {
> + printk(KERN_ERR "%s: timeout! ctrl=0x%04x intr=0x%04x\n",
> + __func__, ctrl, interrupt);
> + return -EIO;
> + }
> +
> /* If there's controller error, it's a real error */
> if (ctrl & ONENAND_CTRL_ERROR) {
> printk(KERN_ERR "%s: controller error = 0x%04x\n",
> @@ -2168,6 +2189,148 @@ static int onenand_block_isbad_nolock(struct mtd_info *mtd, loff_t ofs, int allo
> return bbm->isbad_bbt(mtd, ofs, allowbbt);
> }
>
> +
> +static int onenand_multiblock_erase_verify(struct mtd_info *mtd,
> + struct erase_info *instr)
> +{
> + struct onenand_chip *this = mtd->priv;
> + loff_t addr = instr->addr;
> + int len = instr->len;
> + unsigned int block_size = (1 << this->erase_shift);
> + int ret = 0;
> +
> + while (len) {
> + this->command(mtd, ONENAND_CMD_ERASE_VERIFY, addr, block_size);
> + ret = this->wait(mtd, FL_VERIFYING_ERASE);
> + if (ret) {
> + printk(KERN_ERR "%s: Failed verify, block %d\n",
> + __func__, onenand_block(this, addr));
> + instr->state = MTD_ERASE_FAILED;
> + instr->fail_addr = addr;
> + return -1;
> + }
> + len -= block_size;
> + addr += block_size;
> + }
> + return 0;
> +}
> +
> +/**
> + * onenand_multiblock_erase - [Internal] erase block(s) using multiblock erase
> + * @param mtd MTD device structure
> + * @param instr erase instruction
> + * @param region erase region
> + *
> + * Erase one or more blocks up to 64 block at a time
> + */
> +static int onenand_multiblock_erase(struct mtd_info *mtd,
> + struct erase_info *instr,
> + unsigned int block_size)
> +{
> + struct onenand_chip *this = mtd->priv;
> + loff_t addr = instr->addr;
> + int len = instr->len;
> + int eb_count = 0;
> + int ret = 0;
> + int bdry_block = 0;
> +
> + instr->state = MTD_ERASING;
> +
> + if (ONENAND_IS_DDP(this)) {
> + loff_t bdry_addr = this->chipsize >> 1;
> + if (addr < bdry_addr && (addr + len) > bdry_addr)
> + bdry_block = bdry_addr >> this->erase_shift;
> + }
> +
> + /* Pre-check bbs */
> + while (len) {
> + /* Check if we have a bad block, we do not erase bad blocks */
> + if (onenand_block_isbad_nolock(mtd, addr, 0)) {
> + printk(KERN_WARNING "%s: attempt to erase a bad block "
> + "at addr 0x%012llx\n",
> + __func__, (unsigned long long) addr);
> + instr->state = MTD_ERASE_FAILED;
> + return -EIO;
> + }
> + len -= block_size;
> + addr += block_size;
> + }
> +
> + len = instr->len;
> + addr = instr->addr;
> +
> + /* loop over 64 eb batches */
> + while (len) {
> + struct erase_info verify_instr = *instr;
> + int max_eb_count = MB_ERASE_MAX_BLK_COUNT;
> +
> + verify_instr.addr = addr;
> + verify_instr.len = 0;
> +
> + /* do not cross chip boundary */
> + if (bdry_block) {
> + int this_block = (addr >> this->erase_shift);
> +
> + if (this_block < bdry_block) {
> + max_eb_count = min(max_eb_count,
> + (bdry_block - this_block));
> + }
> + }
> +
> + eb_count = 0;
> +
> + while (len > block_size && eb_count < (max_eb_count - 1)) {
> + this->command(mtd, ONENAND_CMD_MULTIBLOCK_ERASE,
> + addr, block_size);
> + onenand_invalidate_bufferram(mtd, addr, block_size);
> +
> + ret = this->wait(mtd, FL_PREPARING_ERASE);
> + if (ret) {
> + printk(KERN_ERR "%s: Failed multiblock erase, "
> + "block %d\n", __func__,
> + onenand_block(this, addr));
> + instr->state = MTD_ERASE_FAILED;
> + instr->fail_addr = addr;
This might lead a caller to believe all eraseblocks before fail_addr have been erased.
Better to leave it MTD_FAIL_ADDR_UNKNOWN in this case.
> + return -EIO;
> + }
> +
> + len -= block_size;
> + addr += block_size;
> + eb_count++;
> + }
> +
> + /* last block of 64-eb series */
> + cond_resched();
> + this->command(mtd, ONENAND_CMD_ERASE, addr, block_size);
> + onenand_invalidate_bufferram(mtd, addr, block_size);
> +
> + ret = this->wait(mtd, FL_ERASING);
> + /* Check if it is write protected */
> + if (ret) {
> + printk(KERN_ERR "%s: Failed erase, block %d\n",
> + __func__, onenand_block(this, addr));
> + instr->state = MTD_ERASE_FAILED;
> + instr->fail_addr = addr;
Ditto
> + return -EIO;
> + }
> +
> + len -= block_size;
> + addr += block_size;
> + eb_count++;
> +
> + /* verify */
> + verify_instr.len = eb_count * block_size;
> + if (onenand_multiblock_erase_verify(mtd, &verify_instr)) {
> + instr->state = verify_instr.state;
> + instr->fail_addr = verify_instr.fail_addr;
> + return -EIO;
> + }
> +
> + }
> + return 0;
> +}
> +
> +
> /**
> * onenand_block_by_block_erase - [Internal] erase block(s) using regular erase
> * @param mtd MTD device structure
> @@ -2301,7 +2464,13 @@ static int onenand_erase(struct mtd_info *mtd, struct erase_info *instr)
> /* Grab the lock and see if the device is available */
> onenand_get_device(mtd, FL_ERASING);
>
> - ret = onenand_block_by_block_erase(mtd, instr, region, block_size);
> + if (region || instr->len < MB_ERASE_MIN_BLK_COUNT * block_size) {
> + /* region is set for Flex-OneNAND (no mb erase) */
> + ret = onenand_block_by_block_erase(mtd, instr,
> + region, block_size);
> + } else {
> + ret = onenand_multiblock_erase(mtd, instr, block_size);
> + }
>
> /* Deselect and wake up anyone waiting on the device */
> onenand_release_device(mtd);
> diff --git a/include/linux/mtd/flashchip.h b/include/linux/mtd/flashchip.h
> index f350a48..d0bf422 100644
> --- a/include/linux/mtd/flashchip.h
> +++ b/include/linux/mtd/flashchip.h
> @@ -41,9 +41,11 @@ typedef enum {
> /* These 2 come from nand_state_t, which has been unified here */
> FL_READING,
> FL_CACHEDPRG,
> - /* These 2 come from onenand_state_t, which has been unified here */
> + /* These 4 come from onenand_state_t, which has been unified here */
> FL_RESETING,
> FL_OTPING,
> + FL_PREPARING_ERASE,
> + FL_VERIFYING_ERASE,
>
> FL_UNKNOWN
> } flstate_t;
> diff --git a/include/linux/mtd/onenand_regs.h b/include/linux/mtd/onenand_regs.h
> index acadbf5..cd6f3b4 100644
> --- a/include/linux/mtd/onenand_regs.h
> +++ b/include/linux/mtd/onenand_regs.h
> @@ -131,6 +131,8 @@
> #define ONENAND_CMD_LOCK_TIGHT (0x2C)
> #define ONENAND_CMD_UNLOCK_ALL (0x27)
> #define ONENAND_CMD_ERASE (0x94)
> +#define ONENAND_CMD_MULTIBLOCK_ERASE (0x95)
> +#define ONENAND_CMD_ERASE_VERIFY (0x71)
> #define ONENAND_CMD_RESET (0xF0)
> #define ONENAND_CMD_OTP_ACCESS (0x65)
> #define ONENAND_CMD_READID (0x90)
^ permalink raw reply
* Re: [PATCH 1/2] mtd: Add __nand_calculate_ecc() to NAND ECC functions
From: vimal singh @ 2009-10-20 12:58 UTC (permalink / raw)
To: Akinobu Mita; +Cc: Artem Bityutskiy, linux-mtd, David Woodhouse
In-Reply-To: <1255932706-23824-1-git-send-email-akinobu.mita@gmail.com>
On Mon, Oct 19, 2009 at 11:41 AM, Akinobu Mita <akinobu.mita@gmail.com> wrote:
> Add __nand_calculate_ecc() which does not take struct mtd_info.
> The built-in 256/512 software ECC calculation and correction tester
> will use it.
>
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
> Cc: linux-mtd@lists.infradead.org
> ---
> drivers/mtd/nand/nand_ecc.c | 25 ++++++++++++++++++++-----
> include/linux/mtd/nand_ecc.h | 6 ++++++
> 2 files changed, 26 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mtd/nand/nand_ecc.c b/drivers/mtd/nand/nand_ecc.c
> index db7ae9d..809fb53 100644
> --- a/drivers/mtd/nand/nand_ecc.c
> +++ b/drivers/mtd/nand/nand_ecc.c
> @@ -150,20 +150,19 @@ static const char addressbits[256] = {
> };
>
> /**
> - * nand_calculate_ecc - [NAND Interface] Calculate 3-byte ECC for 256/512-byte
> + * __nand_calculate_ecc - [NAND Interface] Calculate 3-byte ECC for 256/512-byte
> * block
> - * @mtd: MTD block structure
> * @buf: input buffer with raw data
> + * @eccsize: data bytes per ecc step (256 or 512)
> * @code: output buffer with ECC
> */
> -int nand_calculate_ecc(struct mtd_info *mtd, const unsigned char *buf,
> +void __nand_calculate_ecc(const unsigned char *buf, unsigned int eccsize,
> unsigned char *code)
> {
> int i;
> const uint32_t *bp = (uint32_t *)buf;
> /* 256 or 512 bytes/ecc */
> - const uint32_t eccsize_mult =
> - (((struct nand_chip *)mtd->priv)->ecc.size) >> 8;
> + const uint32_t eccsize_mult = eccsize >> 8;
> uint32_t cur; /* current value in buffer */
> /* rp0..rp15..rp17 are the various accumulated parities (per byte) */
> uint32_t rp0, rp1, rp2, rp3, rp4, rp5, rp6, rp7;
> @@ -412,6 +411,22 @@ int nand_calculate_ecc(struct mtd_info *mtd, const unsigned char *buf,
> (invparity[par & 0x55] << 2) |
> (invparity[rp17] << 1) |
> (invparity[rp16] << 0);
> +}
> +EXPORT_SYMBOL(__nand_calculate_ecc);
> +
> +/**
> + * nand_calculate_ecc - [NAND Interface] Calculate 3-byte ECC for 256/512-byte
> + * block
> + * @mtd: MTD block structure
> + * @buf: input buffer with raw data
> + * @code: output buffer with ECC
> + */
> +int nand_calculate_ecc(struct mtd_info *mtd, const unsigned char *buf,
> + unsigned char *code)
> +{
> + __nand_calculate_ecc(buf,
> + ((struct nand_chip *)mtd->priv)->ecc.size, code);
> +
> return 0;
> }
> EXPORT_SYMBOL(nand_calculate_ecc);
> diff --git a/include/linux/mtd/nand_ecc.h b/include/linux/mtd/nand_ecc.h
> index 052ea8c..9cb10ff 100644
> --- a/include/linux/mtd/nand_ecc.h
> +++ b/include/linux/mtd/nand_ecc.h
> @@ -16,6 +16,12 @@
> struct mtd_info;
>
> /*
> + * Calculate 3 byte ECC code for eccsize byte block
> + */
> +void __nand_calculate_ecc(const u_char *dat, unsigned int eccsize,
> + u_char *ecc_code);
> +
> +/*
> * Calculate 3 byte ECC code for 256 byte block
> */
Could you please correct above comment too?
--
Regards,
Vimal Singh
> int nand_calculate_ecc(struct mtd_info *mtd, const u_char *dat, u_char *ecc_code);
> --
> 1.5.4.3
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
>
^ permalink raw reply
* Re: giving room for Linux filesystems on MLC/SLC
From: Artem Bityutskiy @ 2009-10-20 12:39 UTC (permalink / raw)
To: Emmanuel Michon; +Cc: linux-mtd
In-Reply-To: <81859150910200356i1e80bb4er1b97046f43415938@mail.gmail.com>
On Tue, 2009-10-20 at 12:56 +0200, Emmanuel Michon wrote:
> Hello,
>
> we're preparing in my company the software for a general purpose (.5K,
> 2K, 4KB page)
> hardware MLC/SLC controller, especially how we're going to
> choose the byte offsets of a page+spare for `metadata' and `bad
> blocks' (those are excluded by our ECC check hardware).
>
> 0 4096+epsilon
> | metadata | data | bb info | data
>
> Our primary goal is to be compatible with our proprietary filesystem,
> but if we can enable Linux family of MTD FS with little effort, let's
> prepare it
>
> `bad block' information is a hardware stuff (unconsistently, poorly)
> documented to be near the start of spare area.
>
> For some reason we have this 4 byte metadata at the start of each
> page. Does this software concept apply to ubifs and friends and should
> it be sized differently?
We work with an abstract flash mode: it consists of eraseblock, each
eraseblock has several min. I/O units. So if in your cases the driver
hides these meta-data bytes, it should be fine.
However, current UBIFS is hard-coded with an assumption that min. I/O
size is power of 2. This would probably have to change. There was no
fundamental reason why we did it like this, we just wanted to save some
CPU cycles and avoid divisions. But this can be changed.
> Btw, any success stories about UBIFS+MLC since the FAQ report on this topic?
I have not heard them.
--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
^ permalink raw reply
* Re: NAND: Add flags to the probe calls to control scan behaviour
From: Artem Bityutskiy @ 2009-10-20 12:06 UTC (permalink / raw)
To: Ben Dooks; +Cc: linux-mtd, Simtec Linux Team
In-Reply-To: <4ADC4137.1060204@simtec.co.uk>
On Mon, 2009-10-19 at 11:36 +0100, Ben Dooks wrote:
> I'd rather it be silent if it cannot find a device, as a number of our boards
> have slots where NAND devices may be fitted by the customer and as such all
> possibilities are registered with the NAND driver.
>
> > Could you please elaborate why more why is this needed a bit more? What
> > is the driver?
>
> Because customers get scared when errors with '!!!' turn up.
> > Why not to just remove that print at all?
>
> Possible, but what about the case where there is a legitimate problem with
> the device that is supposed to be there.
I would go for this instead. I think the drivers can print an error
message themselves.
Or can this be done using the chip->options ?
--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox