* [U-Boot-Users] PATCH - Fix oob data copied into supplied buffer
@ 2007-09-21 0:24 Troy Kisky
2007-09-21 1:33 ` ksi at koi8.net
0 siblings, 1 reply; 6+ messages in thread
From: Troy Kisky @ 2007-09-21 0:24 UTC (permalink / raw)
To: u-boot
This patch correctly sets the oobavail variable
and fixes a bug where the oob data was not valid when
there where multiple groups in oobfree.
It also defines a new variable so that you can tell which
parts of a large page have ecc errors.
Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
diff --git a/drivers/nand/nand_base.c b/drivers/nand/nand_base.c
index 151f535..45b8fb0 100644
--- a/drivers/nand/nand_base.c
+++ b/drivers/nand/nand_base.c
@@ -931,7 +931,7 @@ static int nand_write_page (struct mtd_info *mtd, struct nand_chip *this, int pa
for (i = 0; i < eccbytes; i++, eccidx++)
oob_buf[oob_config[eccidx]] = ecc_code[i];
/* If the hardware ecc provides syndromes then
- * the ecc code must be written immidiately after
+ * the ecc code must be written immediately after
* the data bytes (words) */
if (this->options & NAND_HWECC_SYNDROME)
this->write_buf(mtd, ecc_code, eccbytes);
@@ -1092,7 +1092,7 @@ static int nand_read (struct mtd_info *mtd, loff_t from, size_t len, size_t * re
return nand_read_ecc (mtd, from, len, retlen, buf, NULL, NULL);
}
-
+ulong eccReadMask; /* bitmask off ecc err groups*/
/**
* nand_read_ecc - [MTD Interface] Read data with ECC
* @mtd: MTD device structure
@@ -1267,6 +1267,7 @@ static int nand_read_ecc (struct mtd_info *mtd, loff_t from, size_t len,
ecc_code[j] = oob_data[oob_config[j]];
/* correct data, if neccecary */
+ eccReadMask = 0;
for (i = 0, j = 0, datidx = 0; i < this->eccsteps; i++, datidx += ecc) {
ecc_status = this->correct_data(mtd, &data_poi[datidx], &ecc_code[j], &ecc_calc[j]);
@@ -1283,6 +1284,7 @@ static int nand_read_ecc (struct mtd_info *mtd, loff_t from, size_t len,
}
if (ecc_status == -1) {
+ eccReadMask |= (1<<i);
DEBUG (MTD_DEBUG_LEVEL0, "nand_read_ecc: " "Failed ECC read, page 0x%08x\n", page);
ecc_failed++;
}
@@ -1299,7 +1301,7 @@ static int nand_read_ecc (struct mtd_info *mtd, loff_t from, size_t len,
for (i = 0, j = 0; j < mtd->oobavail; i++) {
int from = oobsel->oobfree[i][0];
int num = oobsel->oobfree[i][1];
- memcpy(&oob_buf[oob], &oob_data[from], num);
+ memcpy(&oob_buf[oob+j], &oob_data[from], num);
j+= num;
}
oob += mtd->oobavail;
@@ -1644,8 +1646,10 @@ static int nand_write_ecc (struct mtd_info *mtd, loff_t to, size_t len,
this->select_chip(mtd, chipnr);
/* Check, if it is write protected */
- if (nand_check_wp(mtd))
+ if (nand_check_wp(mtd)) {
+ printk (KERN_NOTICE "nand_write_ecc: Device is write protected\n");
goto out;
+ }
/* if oobsel is NULL, use chip defaults */
if (oobsel == NULL)
@@ -2486,12 +2490,9 @@ int nand_scan (struct mtd_info *mtd, int maxchips)
/* The number of bytes available for the filesystem to place fs dependend
* oob data */
- if (this->options & NAND_BUSWIDTH_16) {
- mtd->oobavail = mtd->oobsize - (this->autooob->eccbytes + 2);
- if (this->autooob->eccbytes & 0x01)
- mtd->oobavail--;
- } else
- mtd->oobavail = mtd->oobsize - (this->autooob->eccbytes + 1);
+ mtd->oobavail = 0;
+ for (i=0; this->autooob->oobfree[i][1]; i++)
+ mtd->oobavail += this->autooob->oobfree[i][1];
/*
* check ECC mode, default to software
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot-Users] PATCH - Fix oob data copied into supplied buffer
2007-09-21 0:24 [U-Boot-Users] PATCH - Fix oob data copied into supplied buffer Troy Kisky
@ 2007-09-21 1:33 ` ksi at koi8.net
2007-09-21 2:15 ` Troy Kisky
0 siblings, 1 reply; 6+ messages in thread
From: ksi at koi8.net @ 2007-09-21 1:33 UTC (permalink / raw)
To: u-boot
On Thu, 20 Sep 2007, Troy Kisky wrote:
This doesn't look right to me.
First chunk fixes a minor spelling error. Then I can't understand why
introduce a new variable. Everything works fine with existing structures and
it's been like this for ages, both in Linux kernel and U-Boot.
> This patch correctly sets the oobavail variable
> and fixes a bug where the oob data was not valid when
> there where multiple groups in oobfree.
> It also defines a new variable so that you can tell which
> parts of a large page have ecc errors.
>
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
>
>
> diff --git a/drivers/nand/nand_base.c b/drivers/nand/nand_base.c
> index 151f535..45b8fb0 100644
> --- a/drivers/nand/nand_base.c
> +++ b/drivers/nand/nand_base.c
> @@ -931,7 +931,7 @@ static int nand_write_page (struct mtd_info *mtd,
> struct nand_chip *this, int pa
> for (i = 0; i < eccbytes; i++, eccidx++)
> oob_buf[oob_config[eccidx]] =
> ecc_code[i];
> /* If the hardware ecc provides syndromes then
> - * the ecc code must be written immidiately
> after
> + * the ecc code must be written immediately
> after
> * the data bytes (words) */
> if (this->options & NAND_HWECC_SYNDROME)
> this->write_buf(mtd, ecc_code,
> eccbytes);
> @@ -1092,7 +1092,7 @@ static int nand_read (struct mtd_info *mtd, loff_t
> from, size_t len, size_t * re
> return nand_read_ecc (mtd, from, len, retlen, buf, NULL, NULL);
> }
>
> -
> +ulong eccReadMask; /* bitmask off ecc err groups*/
> /**
> * nand_read_ecc - [MTD Interface] Read data with ECC
> * @mtd: MTD device structure
> @@ -1267,6 +1267,7 @@ static int nand_read_ecc (struct mtd_info *mtd,
> loff_t from, size_t len,
> ecc_code[j] = oob_data[oob_config[j]];
>
> /* correct data, if neccecary */
> + eccReadMask = 0;
> for (i = 0, j = 0, datidx = 0; i < this->eccsteps; i++,
> datidx += ecc) {
> ecc_status = this->correct_data(mtd,
> &data_poi[datidx], &ecc_code[j], &ecc_calc[j]);
>
> @@ -1283,6 +1284,7 @@ static int nand_read_ecc (struct mtd_info *mtd,
> loff_t from, size_t len,
> }
>
> if (ecc_status == -1) {
> + eccReadMask |= (1<<i);
> DEBUG (MTD_DEBUG_LEVEL0, "nand_read_ecc:
> " "Failed ECC read, page 0x%08x\n", page);
> ecc_failed++;
> }
> @@ -1299,7 +1301,7 @@ static int nand_read_ecc (struct mtd_info *mtd,
> loff_t from, size_t len,
> for (i = 0, j = 0; j < mtd->oobavail;
> i++) {
> int from =
> oobsel->oobfree[i][0];
> int num = oobsel->oobfree[i][1];
> - memcpy(&oob_buf[oob],
> &oob_data[from], num);
> + memcpy(&oob_buf[oob+j],
> &oob_data[from], num);
> j+= num;
> }
> oob += mtd->oobavail;
> @@ -1644,8 +1646,10 @@ static int nand_write_ecc (struct mtd_info *mtd,
> loff_t to, size_t len,
> this->select_chip(mtd, chipnr);
>
> /* Check, if it is write protected */
> - if (nand_check_wp(mtd))
> + if (nand_check_wp(mtd)) {
> + printk (KERN_NOTICE "nand_write_ecc: Device is write
> protected\n");
> goto out;
> + }
>
> /* if oobsel is NULL, use chip defaults */
> if (oobsel == NULL)
> @@ -2486,12 +2490,9 @@ int nand_scan (struct mtd_info *mtd, int
> maxchips)
>
> /* The number of bytes available for the filesystem to place fs
> dependend
> * oob data */
> - if (this->options & NAND_BUSWIDTH_16) {
> - mtd->oobavail = mtd->oobsize - (this->autooob->eccbytes
> + 2);
> - if (this->autooob->eccbytes & 0x01)
> - mtd->oobavail--;
> - } else
> - mtd->oobavail = mtd->oobsize - (this->autooob->eccbytes
> + 1);
> + mtd->oobavail = 0;
> + for (i=0; this->autooob->oobfree[i][1]; i++)
> + mtd->oobavail += this->autooob->oobfree[i][1];
>
> /*
> * check ECC mode, default to software
>
> ------------------------------------------------------------------------
> -
> This SF.net email is sponsored by: Microsoft
> Defy all challenges. Microsoft(R) Visual Studio 2005.
> http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
> _______________________________________________
> U-Boot-Users mailing list
> U-Boot-Users at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/u-boot-users
>
---
******************************************************************
* KSI at home KOI8 Net < > The impossible we do immediately. *
* Las Vegas NV, USA < > Miracles require 24-hour notice. *
******************************************************************
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot-Users] PATCH - Fix oob data copied into supplied buffer
2007-09-21 1:33 ` ksi at koi8.net
@ 2007-09-21 2:15 ` Troy Kisky
2007-09-22 23:03 ` Wolfgang Denk
0 siblings, 1 reply; 6+ messages in thread
From: Troy Kisky @ 2007-09-21 2:15 UTC (permalink / raw)
To: u-boot
ksi at koi8.net wrote:
> On Thu, 20 Sep 2007, Troy Kisky wrote:
>
> This doesn't look right to me.
>
> First chunk fixes a minor spelling error. Then I can't understand why
> introduce a new variable. Everything works fine with existing structures
> and
> it's been like this for ages, both in Linux kernel and U-Boot.
>
If you look at the kernel, you'll see that oobavail is set there the way I did it.
The other bug doesn't show up unless you have multiple oobfree groups. The new variable
is used when reading or writing pages that an initial program loader reads. It's purpose
is to correct pages without using a bad block table. I'll submit that code if this is approved.
Troy
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot-Users] PATCH - Fix oob data copied into supplied buffer
2007-09-21 2:15 ` Troy Kisky
@ 2007-09-22 23:03 ` Wolfgang Denk
2007-09-24 23:41 ` Troy Kisky
0 siblings, 1 reply; 6+ messages in thread
From: Wolfgang Denk @ 2007-09-22 23:03 UTC (permalink / raw)
To: u-boot
In message <46F3294C.2030206@boundarydevices.com> you wrote:
>
> > First chunk fixes a minor spelling error. Then I can't understand why
> > introduce a new variable. Everything works fine with existing structures
> > and
> > it's been like this for ages, both in Linux kernel and U-Boot.
> >
> If you look at the kernel, you'll see that oobavail is set there the way I did it.
> The other bug doesn't show up unless you have multiple oobfree groups. The new variable
> is used when reading or writing pages that an initial program loader reads. It's purpose
> is to correct pages without using a bad block table. I'll submit that code if this is approved.
kis's comments were legitimate.
Please add this variable only when adding code that uses it. I will
always reject patches that attempt to add unused code or variables.
Also, please describe *all* your changes.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
About the use of language: it is impossible to sharpen a pencil with
a blunt ax. It is equally vain to try to do it with ten blunt axes
instead. -- Edsger Dijkstra
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot-Users] PATCH - Fix oob data copied into supplied buffer
2007-09-22 23:03 ` Wolfgang Denk
@ 2007-09-24 23:41 ` Troy Kisky
2008-04-21 6:46 ` Stefan Roese
0 siblings, 1 reply; 6+ messages in thread
From: Troy Kisky @ 2007-09-24 23:41 UTC (permalink / raw)
To: u-boot
Wolfgang Denk wrote:
> In message <46F3294C.2030206@boundarydevices.com> you wrote:
>>> First chunk fixes a minor spelling error. Then I can't understand why
>>> introduce a new variable. Everything works fine with existing structures
>>> and
>>> it's been like this for ages, both in Linux kernel and U-Boot.
>>>
>> If you look at the kernel, you'll see that oobavail is set there the way I did it.
>> The other bug doesn't show up unless you have multiple oobfree groups. The new variable
>> is used when reading or writing pages that an initial program loader reads. It's purpose
>> is to correct pages without using a bad block table. I'll submit that code if this is approved.
>
> kis's comments were legitimate.
>
> Please add this variable only when adding code that uses it. I will
> always reject patches that attempt to add unused code or variables.
I should have known that, guess I have a lazy streak in me.
>
> Also, please describe *all* your changes.
Sorry, I didn't realize that you wanted typo fixes, and error messages described.
>
>
> Best regards,
>
> Wolfgang Denk
>
First segment fixes a typo
Second segment fixes a bug where oob data may be copied incorrectly.
Third segment adds an error message when exiting due to write protect.
Forth segment fixes a bug where oobavail may be set incorrectly.
All segments are independent and may be accepted or rejected as deemed appropriate.
Do they need to be separate patches?
Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
diff --git a/drivers/nand/nand_base.c b/drivers/nand/nand_base.c
index 151f535..2da1d46 100644
--- a/drivers/nand/nand_base.c
+++ b/drivers/nand/nand_base.c
@@ -931,7 +931,7 @@ static int nand_write_page (struct mtd_info *mtd, struct nand_chip *this, int pa
for (i = 0; i < eccbytes; i++, eccidx++)
oob_buf[oob_config[eccidx]] = ecc_code[i];
/* If the hardware ecc provides syndromes then
- * the ecc code must be written immidiately after
+ * the ecc code must be written immediately after
* the data bytes (words) */
if (this->options & NAND_HWECC_SYNDROME)
this->write_buf(mtd, ecc_code, eccbytes);
@@ -1299,7 +1299,7 @@ static int nand_read_ecc (struct mtd_info *mtd, loff_t from, size_t len,
for (i = 0, j = 0; j < mtd->oobavail; i++) {
int from = oobsel->oobfree[i][0];
int num = oobsel->oobfree[i][1];
- memcpy(&oob_buf[oob], &oob_data[from], num);
+ memcpy(&oob_buf[oob+j], &oob_data[from], num);
j+= num;
}
oob += mtd->oobavail;
@@ -1644,8 +1644,10 @@ static int nand_write_ecc (struct mtd_info *mtd, loff_t to, size_t len,
this->select_chip(mtd, chipnr);
/* Check, if it is write protected */
- if (nand_check_wp(mtd))
+ if (nand_check_wp(mtd)) {
+ printk (KERN_NOTICE "nand_write_ecc: Device is write protected\n");
goto out;
+ }
/* if oobsel is NULL, use chip defaults */
if (oobsel == NULL)
@@ -2486,12 +2488,9 @@ int nand_scan (struct mtd_info *mtd, int maxchips)
/* The number of bytes available for the filesystem to place fs dependend
* oob data */
- if (this->options & NAND_BUSWIDTH_16) {
- mtd->oobavail = mtd->oobsize - (this->autooob->eccbytes + 2);
- if (this->autooob->eccbytes & 0x01)
- mtd->oobavail--;
- } else
- mtd->oobavail = mtd->oobsize - (this->autooob->eccbytes + 1);
+ mtd->oobavail = 0;
+ for (i=0; this->autooob->oobfree[i][1]; i++)
+ mtd->oobavail += this->autooob->oobfree[i][1];
/*
* check ECC mode, default to software
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot-Users] PATCH - Fix oob data copied into supplied buffer
2007-09-24 23:41 ` Troy Kisky
@ 2008-04-21 6:46 ` Stefan Roese
0 siblings, 0 replies; 6+ messages in thread
From: Stefan Roese @ 2008-04-21 6:46 UTC (permalink / raw)
To: u-boot
On Tuesday 25 September 2007, Troy Kisky wrote:
> First segment fixes a typo
> Second segment fixes a bug where oob data may be copied incorrectly.
> Third segment adds an error message when exiting due to write protect.
> Forth segment fixes a bug where oobavail may be set incorrectly.
>
> All segments are independent and may be accepted or rejected as deemed
> appropriate. Do they need to be separate patches?
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
Sorry, I missed this for a long time and Wolfgang just reminded me of it.
Added to u-boot-nand-flash repo. Thanks.
Best regards,
Stefan
=====================================================================
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de
=====================================================================
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2008-04-21 6:46 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-09-21 0:24 [U-Boot-Users] PATCH - Fix oob data copied into supplied buffer Troy Kisky
2007-09-21 1:33 ` ksi at koi8.net
2007-09-21 2:15 ` Troy Kisky
2007-09-22 23:03 ` Wolfgang Denk
2007-09-24 23:41 ` Troy Kisky
2008-04-21 6:46 ` Stefan Roese
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox