[PATCH] mtd: AT49BV6416 has swapped erase regions

[PATCH] mtd: AT49BV6416 has swapped erase regions

[Haavard Skinnemoen]

The CFI information read from AT49BV6416 lists the erase regions in the
wrong order, causing problems when trying to erase or update the first
or last 64K block.

Work around this by inverting the "top boot" flag, which will
effectively reverse the order of the erase regions.

This chip is obsolete, but it's used in some existing designs.

Signed-off-by: Haavard Skinnemoen <haavard.skinnemoen@atmel.com>
---
I'd like this fix to be applied before 2.6.27. However, it's not a big
deal since the affected regions are write protected on STK1000, which
is the only board I know that's affected by this.

This bug will hurt people who are un-protecting those sectors in order
to reflash the boot loader from Linux.

 drivers/mtd/chips/cfi_cmdset_0002.c |   16 ++++++++++++----
 1 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index a972cc6..c6c7e75 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -178,10 +178,18 @@ static void fixup_convert_atmel_pri(struct mtd_info *mtd, void *param)
 	if (atmel_pri.Features & 0x02)
 		extp->EraseSuspend = 2;
 
-	if (atmel_pri.BottomBoot)
-		extp->TopBottom = 2;
-	else
-		extp->TopBottom = 3;
+	/* Some chips got it backwards... */
+	if (cfi->id == AT49BV6416) {
+		if (atmel_pri.BottomBoot)
+			extp->TopBottom = 3;
+		else
+			extp->TopBottom = 2;
+	} else {
+		if (atmel_pri.BottomBoot)
+			extp->TopBottom = 2;
+		else
+			extp->TopBottom = 3;
+	}
 
 	/* burst write mode not supported */
 	cfi->cfiq->BufWriteTimeoutTyp = 0;
-- 
1.5.6.5

Re: [PATCH] mtd: AT49BV6416 has swapped erase regions

[Leon Woestenberg]

Haavard,

On Tue, Sep 30, 2008 at 1:55 PM, Haavard Skinnemoen
<haavard.skinnemoen@atmel.com> wrote:
> The CFI information read from AT49BV6416 lists the erase regions in the
> wrong order, causing problems when trying to erase or update the first
> or last 64K block.
>
Are there other Atmel designs with this bug or only the chip with this CFI ID?

I ask, because the check in u-boot is too generic; I found u-boot
wrongly assumes wrong order for another Atmel part, not checking on a
specific CFI ID.

(Need to lookup the part numbers later at home).

Regards,
-- 
Leon

Re: [PATCH] mtd: AT49BV6416 has swapped erase regions

[Haavard Skinnemoen]

"Leon Woestenberg" <leon.woestenberg@gmail.com> wrote:
> Haavard,
> 
> On Tue, Sep 30, 2008 at 1:55 PM, Haavard Skinnemoen
> <haavard.skinnemoen@atmel.com> wrote:
> > The CFI information read from AT49BV6416 lists the erase regions in the
> > wrong order, causing problems when trying to erase or update the first
> > or last 64K block.
> >
> Are there other Atmel designs with this bug or only the chip with this CFI ID?

That's a good question. I guess there may be others in the same family
with the same bug. I don't really know.

The replacement, AT49BV642D, does not have this bug.

> I ask, because the check in u-boot is too generic; I found u-boot
> wrongly assumes wrong order for another Atmel part, not checking on a
> specific CFI ID.

That's interesting...especially since u-boot only reads the low byte
of the JEDEC ID, so adding a fixup for one particular ID may match tons
of chips with 16-bit IDs.

I'll have to check the latest u-boot and see if it breaks any of my
boards.

Haavard

Re: [PATCH] mtd: AT49BV6416 has swapped erase regions

[Leon Woestenberg]

Hello Haavard, all,

this topic has now diverted to u-boot, as Linux is fixed.  I am
posting in the existing thread to keep the information coherent.

On Tue, Sep 30, 2008 at 2:22 PM, Haavard Skinnemoen
<haavard.skinnemoen@atmel.com> wrote:
> "Leon Woestenberg" <leon.woestenberg@gmail.com> wrote:
>> Haavard,
>> On Tue, Sep 30, 2008 at 1:55 PM, Haavard Skinnemoen
>> <haavard.skinnemoen@atmel.com> wrote:
>> > The CFI information read from AT49BV6416 lists the erase regions in the
>> > wrong order, causing problems when trying to erase or update the first
>> > or last 64K block.
>> >
>> Are there other Atmel designs with this bug or only the chip with this CFI ID?
>
> That's a good question. I guess there may be others in the same family
> with the same bug. I don't really know.
>
> The replacement, AT49BV642D, does not have this bug.
>
>> I ask, because the check in u-boot is too generic; I found u-boot
>> wrongly assumes wrong order for another Atmel part, not checking on a
>> specific CFI ID.
>
> That's interesting...especially since u-boot only reads the low byte
> of the JEDEC ID, so adding a fixup for one particular ID may match tons
> of chips with 16-bit IDs.
>
> I'll have to check the latest u-boot and see if it breaks any of my
> boards.
>
I just checked on a custom design, AP7000 with AT49BV320DT, which
reports its top boot bit correctly.
info->device_id == c4 for this part.

The u-boot flash_fixup_atmel() currently reverses geometry whenever
the top boot bit is set, which seems wrong:

I have to force this to zero to make that custom board work in this
piece of code:

/* Check the "top boot" bit in the PRI */
if (info->ext_addr && !(flash_read_uchar(info, info->ext_addr + 6) & 1))
  reverse_geometry = 1;

I was hesitating to come up with a patch, because some of the check is
#ifdef'd out, and I may have missed
a u-boot convention that I should layout my sectors as
bottom-boot-block in u-boot, and reverse_geometry if 'top'?

Indeed the single byte ID check may need attention. I have to lookup
the CFI specs to be sure.

Regards,
-- 
Leon