| Message ID | 20120609160825.GC6488@elgon.mountain |
|---|---|
| State | Accepted |
| Commit | 48f8b641297df49021093763a3271119a84990a2 |
| Headers | show |
On Sat, 2012-06-09 at 19:08 +0300, Dan Carpenter wrote: > The intent here was clearly to set result to true if the 0x40000000 flag > was set. But instead there was a | vs & typo and we always set result > to true. > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > I do not have the hardware to test this. The original code is clearly > buggy, but what about if 0x40000000 is the wrong flag? Looks like it is the right flag, I've checked wiki.laptop.org/images/5/5c/88ALP01_Datasheet_July_2007.pdf and your patch looks correct.
Pushed this patch to l2-mtd.git and added CC to -stable. The patch seems to be a real bug-fix. Daniel, could you please check it - it will be shame if it hits -stable and breaks something. Thanks! On Sat, 2012-06-09 at 19:08 +0300, Dan Carpenter wrote: > The intent here was clearly to set result to true if the 0x40000000 flag > was set. But instead there was a | vs & typo and we always set result > to true. > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > I do not have the hardware to test this. The original code is clearly > buggy, but what about if 0x40000000 is the wrong flag? > > diff --git a/drivers/mtd/nand/cafe_nand.c b/drivers/mtd/nand/cafe_nand.c > index f3594a6..ac0d967 100644 > --- a/drivers/mtd/nand/cafe_nand.c > +++ b/drivers/mtd/nand/cafe_nand.c > @@ -102,7 +102,7 @@ static const char *part_probes[] = { "cmdlinepart", "RedBoot", NULL }; > static int cafe_device_ready(struct mtd_info *mtd) > { > struct cafe_priv *cafe = mtd->priv; > - int result = !!(cafe_readl(cafe, NAND_STATUS) | 0x40000000); > + int result = !!(cafe_readl(cafe, NAND_STATUS) & 0x40000000); > uint32_t irqs = cafe_readl(cafe, NAND_IRQ); > > cafe_writel(cafe, irqs, NAND_IRQ); > > ______________________________________________________ > Linux MTD discussion mailing list > http://lists.infradead.org/mailman/listinfo/linux-mtd/
On Mon, Jun 18, 2012 at 5:25 AM, Artem Bityutskiy <artem.bityutskiy@linux.intel.com> wrote: > Pushed this patch to l2-mtd.git and added CC to -stable. The patch seems > to be a real bug-fix. Daniel, could you please check it - it will be > shame if it hits -stable and breaks something. Thanks for catching this, sorry for the delay in looking closely at it. I double-checked the device specifications and indeed, Dan's interpretation is correct. I also tested the patch. System still remains functional and basic nand performance seems to be the same. Thanks! Daniel
diff --git a/drivers/mtd/nand/cafe_nand.c b/drivers/mtd/nand/cafe_nand.c index f3594a6..ac0d967 100644 --- a/drivers/mtd/nand/cafe_nand.c +++ b/drivers/mtd/nand/cafe_nand.c @@ -102,7 +102,7 @@ static const char *part_probes[] = { "cmdlinepart", "RedBoot", NULL }; static int cafe_device_ready(struct mtd_info *mtd) { struct cafe_priv *cafe = mtd->priv; - int result = !!(cafe_readl(cafe, NAND_STATUS) | 0x40000000); + int result = !!(cafe_readl(cafe, NAND_STATUS) & 0x40000000); uint32_t irqs = cafe_readl(cafe, NAND_IRQ); cafe_writel(cafe, irqs, NAND_IRQ);
The intent here was clearly to set result to true if the 0x40000000 flag was set. But instead there was a | vs & typo and we always set result to true. Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- I do not have the hardware to test this. The original code is clearly buggy, but what about if 0x40000000 is the wrong flag?