Message ID | m1skqbhmh2.fsf@frodo.ebiederm.org |
---|---|
State | Superseded |
Headers | show |
On Sat, 2008-11-01 at 01:36 -0700, Eric W. Biederman wrote: > Likewise the addresses in jedec_probe whose word address ends > in the bits 10 also need their low bit set. Some of these places in jedec_probe.c use cfi_build_cmd_addr() directly, and this version of the patch puts the low-bit-mangling into cfi_gen_send_cmd() so those bits miss out...
David Woodhouse <dwmw2@infradead.org> writes: > On Sat, 2008-11-01 at 01:36 -0700, Eric W. Biederman wrote: >> Likewise the addresses in jedec_probe whose word address ends >> in the bits 10 also need their low bit set. > > Some of these places in jedec_probe.c use cfi_build_cmd_addr() directly, > and this version of the patch puts the low-bit-mangling into > cfi_gen_send_cmd() so those bits miss out... Sorry. It's late and was tired. One more try. This time as a patch series. The first patch is the bug fix. The second patch kills the unnecessary arguments to cfi_send_gen_cmd, making the code that uses it more readable. Eric
On Sat, 2008-11-01 at 03:29 -0700, Eric W. Biederman wrote: > @@ -1808,9 +1808,7 @@ static inline u32 jedec_read_mfr(struct map_info *map, uint32_t base, > * several first banks can contain 0x7f instead of actual ID > */ > do { > - uint32_t ofs = cfi_build_cmd_addr(0 + (bank << 8), > - cfi_interleave(cfi), > - cfi->device_type); > + uint32_t ofs = cfi_build_cmd_addr(0 + (bank << 8), map, cfi); > mask = (1 << (cfi->device_type * 8)) - 1; > result = map_read(map, base + ofs); > bank++; I think this one is still broken by your patch -- it's the exception to your observation that we only ever use addresses ending in 00, 55 or aa. > > + /* Modify the unlock address if we are in compatiblity mode. > + * For 16bit devices on 8 bit busses > + * and 32bit devices on 16 bit busses > + * set the low bit of the alternating bit sequence of the address. > + */ > + if (((type * interleave) > bankwidth) && (cmd_addr & 2)) > + addr |= (type >> 1)*interleave; Perhaps '&& ((cmd_addr & 0xff) == 0xaa)' is the answer?
On Sat, 2008-11-01 at 10:43 +0000, David Woodhouse wrote: > I think this one is still broken by your patch -- it's the exception to > your observation that we only ever use addresses ending in 00, 55 or aa. No, I misunderstood how that works -- your patch is fine. Although I think I'll split patch 1/2 into cleanup vs. the actual fix, too.
David Woodhouse <dwmw2@infradead.org> writes: > On Sat, 2008-11-01 at 10:43 +0000, David Woodhouse wrote: >> I think this one is still broken by your patch -- it's the exception to >> your observation that we only ever use addresses ending in 00, 55 or aa. > > No, I misunderstood how that works -- your patch is fine. Although I > think I'll split patch 1/2 into cleanup vs. the actual fix, too. Sounds good. The (cmd_ofs & 0xff) == 0xaa might be less prone to false positives. Or perhaps it just doesn't make sense for the id reads in jedec_probe. Sounds like you have a handle on it though. Eric
diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c index a972cc6..9e7a236 100644 --- a/drivers/mtd/chips/cfi_cmdset_0002.c +++ b/drivers/mtd/chips/cfi_cmdset_0002.c @@ -362,19 +362,6 @@ struct mtd_info *cfi_cmdset_0002(struct map_info *map, int primary) /* Set the default CFI lock/unlock addresses */ cfi->addr_unlock1 = 0x555; cfi->addr_unlock2 = 0x2aa; - /* Modify the unlock address if we are in compatibility mode */ - if ( /* x16 in x8 mode */ - ((cfi->device_type == CFI_DEVICETYPE_X8) && - (cfi->cfiq->InterfaceDesc == - CFI_INTERFACE_X8_BY_X16_ASYNC)) || - /* x32 in x16 mode */ - ((cfi->device_type == CFI_DEVICETYPE_X16) && - (cfi->cfiq->InterfaceDesc == - CFI_INTERFACE_X16_BY_X32_ASYNC))) - { - cfi->addr_unlock1 = 0xaaa; - cfi->addr_unlock2 = 0x555; - } } /* CFI mode */ else if (cfi->cfi_mode == CFI_MODE_JEDEC) { diff --git a/include/linux/mtd/cfi.h b/include/linux/mtd/cfi.h index d6fb115..70499e2 100644 --- a/include/linux/mtd/cfi.h +++ b/include/linux/mtd/cfi.h @@ -429,7 +429,17 @@ static inline uint32_t cfi_send_gen_cmd(u_char cmd, uint32_t cmd_addr, uint32_t int type, map_word *prev_val) { map_word val; - uint32_t addr = base + cfi_build_cmd_addr(cmd_addr, cfi_interleave(cfi), type); + unsigned bankwidth = map_bankwidth(map); + unsigned interleave = cfi_interleave(cfi); + uint32_t addr = cfi_build_cmd_addr(cmd_addr, interleave, type); + + /* Modify the unlock address if we are in compatiblity mode. + * For 16bit devices on 8 bit busses + * and 32bit devices on 16 bit busses + * set the low bit of the alternating bit sequence of the address. + */ + if (((type * interleave) > bankwidth) && (cmd_addr & 2)) + addr |= (type >> 1)*interleave; val = cfi_build_cmd(cmd, map, cfi);