All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Enric Balletbò i Serra" <eballetbo@gmail.com>
To: dedekind1@gmail.com
Cc: Kyungmin Park <kmpark@infradead.org>,
	linux-omap@vger.kernel.org, linux-mtd@lists.infradead.org,
	Rohit Hagargundgi <h.rohit@samsung.com>
Subject: Re: Possible bug in onenand_base ?
Date: Mon, 12 Jul 2010 16:59:27 +0200	[thread overview]
Message-ID: <AANLkTikDPqoO_VGJtBKWHutPOb60XAIUAYOu7kIRxMyQ@mail.gmail.com> (raw)
In-Reply-To: <1278584577.20321.2.camel@localhost>

Hello,

2010/7/8 Artem Bityutskiy <dedekind1@gmail.com>:
> On Thu, 2010-07-08 at 12:11 +0200, Enric Balletbò i Serra wrote:
>> Hello,
>>
>> 2010/7/8 Artem Bityutskiy <dedekind1@gmail.com>:
>> > On Thu, 2010-07-08 at 11:55 +0200, Enric Balletbò i Serra wrote:
>> >> Hello,
>> >>
>> >> I made new tests regarding this issue. Looks like the problem is
>> >> reading from the OneNAND device.
>> >
>> > Did you try older kernel and then bisecting who is responsible for the
>> > breakage?
>>
>> Yes, before commit
>>
>> 5988af2319781bc8e0ce418affec4e09cfa77907 (mtd: Flex-OneNAND support)
>>
>> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=5988af2319781bc8e0ce418affec4e09cfa77907
>>
>> my  OneNAND is working, after the commit, the OneNAND support is broken.
>
> Ok, we could revert it, but it is better to fix it. CCing the author of
> the commit.


Comparing the onenand_base.c file before commit

  5988af2319781bc8e0ce418affec4e09cfa77907 (mtd: Flex-OneNAND support)

and after the commit I made a little patch which seems solves the
issue. The patch has two parts.

The first part replaces line 380 with  'page = (int) (addr >>
this->page_shift);' like before apply the Flex-OneNAND support. I
suppose this is not the better solution, but with this the nandtest
tool works for me. Maybe the author of the commit can clarify this.

The second part (1964) adds two lines which I think are missed when
the Flex-OneNAND support was applied.

diff --git a/drivers/mtd/onenand/onenand_base.c
b/drivers/mtd/onenand/onenand_base.c
index 26caf25..a39d906 100644
--- a/drivers/mtd/onenand/onenand_base.c
+++ b/drivers/mtd/onenand/onenand_base.c
@@ -377,7 +377,7 @@ static int onenand_command(struct mtd_info *mtd,
int cmd, loff_t addr, size_t le

        default:
                block = onenand_block(this, addr);
-               page = (int) (addr - onenand_addr(this, block)) >>
this->page_shift;
+               page = (int) (addr >> this->page_shift);

                if (ONENAND_IS_2PLANE(this)) {
                        /* Make the even block number */
@@ -1961,6 +1961,9 @@ static int onenand_write_ops_nolock(struct
mtd_info *mtd, loff_t to,

                        /* In partial page write we don't update bufferram */
                        onenand_update_bufferram(mtd, to, !ret && !subpage);
+                       ONENAND_SET_BUFFERRAM1(this);
+                       onenand_update_bufferram(mtd, to +
this->writesize, !ret && !subpage);
+
                        if (ret) {
                                printk(KERN_ERR "%s: write failed %d\n",
                                        __func__, ret);

Best regards,

Enric
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: "Enric Balletbò i Serra" <eballetbo@gmail.com>
To: dedekind1@gmail.com
Cc: Rohit Hagargundgi <h.rohit@samsung.com>,
	linux-mtd@lists.infradead.org, linux-omap@vger.kernel.org,
	Kyungmin Park <kmpark@infradead.org>
Subject: Re: Possible bug in onenand_base ?
Date: Mon, 12 Jul 2010 16:59:27 +0200	[thread overview]
Message-ID: <AANLkTikDPqoO_VGJtBKWHutPOb60XAIUAYOu7kIRxMyQ@mail.gmail.com> (raw)
In-Reply-To: <1278584577.20321.2.camel@localhost>

Hello,

2010/7/8 Artem Bityutskiy <dedekind1@gmail.com>:
> On Thu, 2010-07-08 at 12:11 +0200, Enric Balletbò i Serra wrote:
>> Hello,
>>
>> 2010/7/8 Artem Bityutskiy <dedekind1@gmail.com>:
>> > On Thu, 2010-07-08 at 11:55 +0200, Enric Balletbò i Serra wrote:
>> >> Hello,
>> >>
>> >> I made new tests regarding this issue. Looks like the problem is
>> >> reading from the OneNAND device.
>> >
>> > Did you try older kernel and then bisecting who is responsible for the
>> > breakage?
>>
>> Yes, before commit
>>
>> 5988af2319781bc8e0ce418affec4e09cfa77907 (mtd: Flex-OneNAND support)
>>
>> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=5988af2319781bc8e0ce418affec4e09cfa77907
>>
>> my  OneNAND is working, after the commit, the OneNAND support is broken.
>
> Ok, we could revert it, but it is better to fix it. CCing the author of
> the commit.


Comparing the onenand_base.c file before commit

  5988af2319781bc8e0ce418affec4e09cfa77907 (mtd: Flex-OneNAND support)

and after the commit I made a little patch which seems solves the
issue. The patch has two parts.

The first part replaces line 380 with  'page = (int) (addr >>
this->page_shift);' like before apply the Flex-OneNAND support. I
suppose this is not the better solution, but with this the nandtest
tool works for me. Maybe the author of the commit can clarify this.

The second part (1964) adds two lines which I think are missed when
the Flex-OneNAND support was applied.

diff --git a/drivers/mtd/onenand/onenand_base.c
b/drivers/mtd/onenand/onenand_base.c
index 26caf25..a39d906 100644
--- a/drivers/mtd/onenand/onenand_base.c
+++ b/drivers/mtd/onenand/onenand_base.c
@@ -377,7 +377,7 @@ static int onenand_command(struct mtd_info *mtd,
int cmd, loff_t addr, size_t le

        default:
                block = onenand_block(this, addr);
-               page = (int) (addr - onenand_addr(this, block)) >>
this->page_shift;
+               page = (int) (addr >> this->page_shift);

                if (ONENAND_IS_2PLANE(this)) {
                        /* Make the even block number */
@@ -1961,6 +1961,9 @@ static int onenand_write_ops_nolock(struct
mtd_info *mtd, loff_t to,

                        /* In partial page write we don't update bufferram */
                        onenand_update_bufferram(mtd, to, !ret && !subpage);
+                       ONENAND_SET_BUFFERRAM1(this);
+                       onenand_update_bufferram(mtd, to +
this->writesize, !ret && !subpage);
+
                        if (ret) {
                                printk(KERN_ERR "%s: write failed %d\n",
                                        __func__, ret);

Best regards,

Enric

  reply	other threads:[~2010-07-12 14:59 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-04-30 10:05 Possible bug in onenand_base ? Enric Balletbò i Serra
2010-04-30 10:05 ` Enric Balletbò i Serra
2010-05-05  5:56 ` Artem Bityutskiy
2010-05-05  5:56   ` Artem Bityutskiy
2010-05-06  3:46 ` Kyungmin Park
2010-05-06  3:46   ` Kyungmin Park
2010-05-06 11:22   ` Enric Balletbò i Serra
2010-05-06 11:22     ` Enric Balletbò i Serra
2010-05-06 11:44     ` Kyungmin Park
2010-05-06 11:44       ` Kyungmin Park
2010-05-06 13:02       ` Enric Balletbò i Serra
2010-05-06 13:02         ` Enric Balletbò i Serra
2010-05-12 10:29         ` Enric Balletbò i Serra
2010-05-12 10:29           ` Enric Balletbò i Serra
2010-05-12 13:16           ` Enric Balletbò i Serra
2010-05-12 13:16             ` Enric Balletbò i Serra
2010-07-08  9:55             ` Enric Balletbò i Serra
2010-07-08  9:55               ` Enric Balletbò i Serra
2010-07-08  9:58               ` Artem Bityutskiy
2010-07-08  9:58                 ` Artem Bityutskiy
2010-07-08 10:11                 ` Enric Balletbò i Serra
2010-07-08 10:11                   ` Enric Balletbò i Serra
2010-07-08 10:22                   ` Artem Bityutskiy
2010-07-08 10:22                     ` Artem Bityutskiy
2010-07-12 14:59                     ` Enric Balletbò i Serra [this message]
2010-07-12 14:59                       ` Enric Balletbò i Serra
2010-07-14  8:52 ROHIT H.S
2010-07-14 14:37 ` Enric Balletbò i Serra
2010-07-15  9:25   ` Rohit Hassan Sathyanarayan
2010-07-15 11:19     ` Enric Balletbò i Serra
2010-07-16 12:04       ` Rohit Hassan Sathyanarayan
2010-07-18 16:47 ` Artem Bityutskiy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=AANLkTikDPqoO_VGJtBKWHutPOb60XAIUAYOu7kIRxMyQ@mail.gmail.com \
    --to=eballetbo@gmail.com \
    --cc=dedekind1@gmail.com \
    --cc=h.rohit@samsung.com \
    --cc=kmpark@infradead.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=linux-omap@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.