Patchwork Bad assumption about ID field definition for Samsung NAND?

login
register
mail settings
Submitter Brian Norris
Date Aug. 20, 2010, 9:01 p.m.
Message ID <4C6EED3B.4000001@broadcom.com>
Download mbox | patch
Permalink /patch/62319/
State Accepted
Commit cfe3fdadb16162327773ef01a575a32000b8c7f4
Headers show

Comments

Brian Norris - Aug. 20, 2010, 9:01 p.m.
> Can I have a signed-off-by for it?

I don't know what's "legal" here. I'm appending the patch with a
sign-off for me and Tilman (since Tilman authored it). Hopefully that's
ok.

> Brian, I have a distinct impression that there's at least one more patch
> from you that I really ought to be sending to Linus for 2.6.36, but I
> can't find it right now. Other than this and what's already in
> mtd-2.6.git, is there anything else?

I'm really no expert on how inclusion for different versions goes; I
just send 'em to you! Anyway, this patch is *very* important:
	* [PATCH] mtd: nand: Fix regression in BBM detection
	http://lists.infradead.org/pipermail/linux-mtd/2010-August/031594.html
It addresses issues I overlooked with a lot of Hynix small-page NAND
(and others).

Other patches - they are ready, but not as important:
	* Spansion ORNAND
	http://lists.infradead.org/pipermail/linux-mtd/2010-August/031603.html
	* New Samsung MLC OOB sizes
	http://lists.infradead.org/pipermail/linux-mtd/2010-August/031621.html

No one has decided between these two (it's a "choose one or the other"
situation). They may or may not be ready:
	mtd: nand: Expand nand_ecc_layout, deprecate ioctl ECCGETLAYOUT
	Cover page: http://lists.infradead.org/pipermail/linux-mtd/2010-August/031591.html
	Choice 1: http://lists.infradead.org/pipermail/linux-mtd/2010-August/031593.html
	Choice 2: http://lists.infradead.org/pipermail/linux-mtd/2010-August/031598.html
	Explanation: http://lists.infradead.org/pipermail/linux-mtd/2010-August/031619.html

And since you asked, the trivial...
	Indentation errors :)
	http://lists.infradead.org/pipermail/linux-mtd/2010-August/031588.html

You already got this one, I believe:
	Fixing a typo in on a buswidth option
	http://lists.infradead.org/pipermail/linux-mtd/2010-August/031518.html

Thanks for looking out for me :)

Brian

----------------------------------------------------------------

Apparently, the check for a 6-byte ID string is NOT sufficient to
determine whether or not a Samsung chip uses their new MLC detection
scheme or the old, standard scheme. This adds a condition to check cell
type.

Signed-off-by: Tilman Sauerbeck <tilman@code-monkey.de>
Signed-off-by: Brian Norris <norris@broadcom.com>
---
 drivers/mtd/nand/nand_base.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
David Woodhouse - Aug. 20, 2010, 9:34 p.m.
On Fri, 2010-08-20 at 14:01 -0700, Brian Norris wrote:
> > Can I have a signed-off-by for it?
> 
> I don't know what's "legal" here. I'm appending the patch with a
> sign-off for me and Tilman (since Tilman authored it). Hopefully that's
> ok.

You have to cut and paste Tilman's own Signed-off-by: header; the magic
doesn't work if you type it yourself. :)

> > Brian, I have a distinct impression that there's at least one more patch
> > from you that I really ought to be sending to Linus for 2.6.36, but I
> > can't find it right now. Other than this and what's already in
> > mtd-2.6.git, is there anything else?
> 
> I'm really no expert on how inclusion for different versions goes; I
> just send 'em to you! Anyway, this patch is *very* important:
> 	* [PATCH] mtd: nand: Fix regression in BBM detection
> 	http://lists.infradead.org/pipermail/linux-mtd/2010-August/031594.html
> It addresses issues I overlooked with a lot of Hynix small-page NAND
> (and others).

That's in the tree already:
http://git.infradead.org/mtd-2.6.git/commitdiff/065a1ed8 

It's also https://bugzilla.kernel.org/show_bug.cgi?id=16639

> Other patches - they are ready, but not as important:
> 	* Spansion ORNAND
> 	http://lists.infradead.org/pipermail/linux-mtd/2010-August/031603.html
> 	* New Samsung MLC OOB sizes
> 	http://lists.infradead.org/pipermail/linux-mtd/2010-August/031621.html

Those aren't regressions or important bug-fixes, so given the timing I
think they've missed the merge window and are now candidates for 2.6.37
rather than 2.6.36?

I'll merge them as soon as I've asked Linus to pull what's in the tree
right now. Unless you object to my classification?

> No one has decided between these two (it's a "choose one or the other"
> situation). They may or may not be ready:
> 	mtd: nand: Expand nand_ecc_layout, deprecate ioctl ECCGETLAYOUT
> 	Cover page: http://lists.infradead.org/pipermail/linux-mtd/2010-August/031591.html
> 	Choice 1: http://lists.infradead.org/pipermail/linux-mtd/2010-August/031593.html
> 	Choice 2: http://lists.infradead.org/pipermail/linux-mtd/2010-August/031598.html
> 	Explanation: http://lists.infradead.org/pipermail/linux-mtd/2010-August/031619.html

Yeah, definitely 2.6.37 material.

> And since you asked, the trivial...
> 	Indentation errors :)
> 	http://lists.infradead.org/pipermail/linux-mtd/2010-August/031588.html

That can wait for 2.6.37 too.

> You already got this one, I believe:
> 	Fixing a typo in on a buswidth option
> 	http://lists.infradead.org/pipermail/linux-mtd/2010-August/031518.html

Yep, that's already in 2.6.36-rc1.

> Thanks for looking out for me :)
> 
> Brian
>
> ----------------------------------------------------------------
> 
> Apparently, the check for a 6-byte ID string is NOT sufficient to
> determine whether or not a Samsung chip uses their new MLC detection
> scheme or the old, standard scheme. This adds a condition to check cell
> type.
> 
> Signed-off-by: Tilman Sauerbeck <tilman@code-monkey.de>
> Signed-off-by: Brian Norris <norris@broadcom.com>
> ---

Just FYI; not to criticise when you're doing such excellent work -- this
would ideally have a From: and Subject: "header" indicating that Tilman
is the author, and giving the first line of the commit comment. That
way, running 'git-am' on it would fairly much work. Not that it's a
problem for me to do it either, of course.

Thanks again.
Brian Norris - Aug. 20, 2010, 10:05 p.m.
On 08/20/2010 02:34 PM, David Woodhouse wrote:
> On Fri, 2010-08-20 at 14:01 -0700, Brian Norris wrote:
>>> Can I have a signed-off-by for it?
>>
>> I don't know what's "legal" here. I'm appending the patch with a
>> sign-off for me and Tilman (since Tilman authored it). Hopefully that's
>> ok.
>
> You have to cut and paste Tilman's own Signed-off-by: header; the magic
> doesn't work if you type it yourself. :)

I'm glad someone has a sense of humor (or humour, depending on the country)

>> I'm really no expert on how inclusion for different versions goes; I
>> just send 'em to you! Anyway, this patch is *very* important:
>> 	* [PATCH] mtd: nand: Fix regression in BBM detection
>> 	http://lists.infradead.org/pipermail/linux-mtd/2010-August/031594.html
>> It addresses issues I overlooked with a lot of Hynix small-page NAND
>> (and others).
>
> That's in the tree already:
> http://git.infradead.org/mtd-2.6.git/commitdiff/065a1ed8

I realized that right after sending this.

>> Other patches - they are ready, but not as important:
>> 	* Spansion ORNAND
>> 	http://lists.infradead.org/pipermail/linux-mtd/2010-August/031603.html
>> 	* New Samsung MLC OOB sizes
>> 	http://lists.infradead.org/pipermail/linux-mtd/2010-August/031621.html
>
> Those aren't regressions or important bug-fixes, so given the timing I
> think they've missed the merge window and are now candidates for 2.6.37
> rather than 2.6.36?
>
> I'll merge them as soon as I've asked Linus to pull what's in the tree
> right now. Unless you object to my classification?

No objection.

> Just FYI; not to criticise when you're doing such excellent work -- this
> would ideally have a From: and Subject: "header" indicating that Tilman
> is the author, and giving the first line of the commit comment. That
> way, running 'git-am' on it would fairly much work. Not that it's a
> problem for me to do it either, of course.

I see. (Thanks for the compliment, btw, I'm rather new to this)

Brian

Patch

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index a3c7473..172a299 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2866,6 +2866,7 @@  static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
 		 */
 		if (id_data[0] == id_data[6] && id_data[1] == id_data[7] &&
 				id_data[0] == NAND_MFR_SAMSUNG &&
+				(chip->cellinfo & NAND_CI_CELLTYPE_MSK) &&
 				id_data[5] != 0x00) {
 			/* Calc pagesize */
 			mtd->writesize = 2048 << (extid & 0x03);