| Message ID | 4BEA5F2F.8090107@web.de |
|---|---|
| State | New |
| Headers | show |
Am Wednesday 12 May 2010 09:56:31 schrieb Jan Kiszka: > OK, that was a hard nut. After various dead ends, I think I found an > possible solution. Can you give this a try? [..] > Still requires proper patch split up, and I need to think about possible > side effects. Thanks, the patch is working. But i noticed another minor bug. The cfi02 doesn't handle 'read flash id' on 16bit accesses correctly. It always returns 8 bit. I used something like if (width == 2) ret = pfl->ident[0] << 8 | pfl->ident[1]; /* rsp. ident[1]/ident[2] */ within the 0x90 reading as a quick workaround.
Michael Walle wrote: > Am Wednesday 12 May 2010 09:56:31 schrieb Jan Kiszka: >> OK, that was a hard nut. After various dead ends, I think I found an >> possible solution. Can you give this a try? > [..] >> Still requires proper patch split up, and I need to think about possible >> side effects. > Thanks, the patch is working. Unfortunately, now that resetting the mode on read is fixed, my whole optimization does not work any, writing to flash takes decades again. Back to the drawing board... > > But i noticed another minor bug. The cfi02 doesn't handle 'read flash id' on > 16bit accesses correctly. It always returns 8 bit. I used something like > > if (width == 2) > ret = pfl->ident[0] << 8 | pfl->ident[1]; /* rsp. ident[1]/ident[2] */ > > within the 0x90 reading as a quick workaround. Are you sure that this is valid? The whole cfi_table is also only provided byte-wise, same in cfi01. Jan
Am Thursday 13 May 2010 09:38:43 schrieb Jan Kiszka: > > But i noticed another minor bug. The cfi02 doesn't handle 'read flash id' > > on 16bit accesses correctly. It always returns 8 bit. I used something > > like > > > > if (width == 2) > > ret = pfl->ident[0] << 8 | pfl->ident[1]; /* rsp. ident[1]/ident[2] > > */ > > > > within the 0x90 reading as a quick workaround. > > Are you sure that this is valid? The whole cfi_table is also only > provided byte-wise, same in cfi01. At least the JEDEC ID read returns 16 bit values with x16 devices. Have a look at: http://www.spansion.com/Support/Datasheets/s29gl128_256n_sp_a2_e.pdf Table II on page 51 micromonitor (the program i tested with) and uboot uses 16bit reads to read the flash id. Have a look at http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=blob;f=drivers/mtd/cfi_flash.c;h=3267c5de36d1b12a190f93f9a3048ded598f84aa;hb=HEAD#l1535
Michael Walle wrote: > Am Thursday 13 May 2010 09:38:43 schrieb Jan Kiszka: >>> But i noticed another minor bug. The cfi02 doesn't handle 'read flash id' >>> on 16bit accesses correctly. It always returns 8 bit. I used something >>> like >>> >>> if (width == 2) >>> ret = pfl->ident[0] << 8 | pfl->ident[1]; /* rsp. ident[1]/ident[2] >>> */ >>> >>> within the 0x90 reading as a quick workaround. >> Are you sure that this is valid? The whole cfi_table is also only >> provided byte-wise, same in cfi01. > > At least the JEDEC ID read returns 16 bit values with x16 devices. Have a look > at: > http://www.spansion.com/Support/Datasheets/s29gl128_256n_sp_a2_e.pdf > Table II on page 51 > > micromonitor (the program i tested with) and uboot uses 16bit reads to read > the flash id. Have a look at > http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=blob;f=drivers/mtd/cfi_flash.c;h=3267c5de36d1b12a190f93f9a3048ded598f84aa;hb=HEAD#l1535 > Right, I came to the same conclusion based on chip I'm using for the Musicpal model. Working on a proper fix - now that I think to have found a solution for the XIP vs. mode switch conflict. Jan
Jan Kiszka wrote: > Michael Walle wrote: >> Am Thursday 13 May 2010 09:38:43 schrieb Jan Kiszka: >>>> But i noticed another minor bug. The cfi02 doesn't handle 'read flash id' >>>> on 16bit accesses correctly. It always returns 8 bit. I used something >>>> like >>>> >>>> if (width == 2) >>>> ret = pfl->ident[0] << 8 | pfl->ident[1]; /* rsp. ident[1]/ident[2] >>>> */ >>>> >>>> within the 0x90 reading as a quick workaround. >>> Are you sure that this is valid? The whole cfi_table is also only >>> provided byte-wise, same in cfi01. >> At least the JEDEC ID read returns 16 bit values with x16 devices. Have a look >> at: >> http://www.spansion.com/Support/Datasheets/s29gl128_256n_sp_a2_e.pdf >> Table II on page 51 >> >> micromonitor (the program i tested with) and uboot uses 16bit reads to read >> the flash id. Have a look at >> http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=blob;f=drivers/mtd/cfi_flash.c;h=3267c5de36d1b12a190f93f9a3048ded598f84aa;hb=HEAD#l1535 >> > > Right, I came to the same conclusion based on chip I'm using for the > Musicpal model. Working on a proper fix - now that I think to have found > a solution for the XIP vs. mode switch conflict. Wait! Access to ident[0..3] is already correct as those fields are stored and returned as 16-bit values. I guess you just did not pass the proper IDs when registering your pflash_cfi02 instance. But this still leaves us with the 8-bit entries of the cfi_table. Jan
Jan Kiszka wrote: > Jan Kiszka wrote: >> Michael Walle wrote: >>> Am Thursday 13 May 2010 09:38:43 schrieb Jan Kiszka: >>>>> But i noticed another minor bug. The cfi02 doesn't handle 'read flash id' >>>>> on 16bit accesses correctly. It always returns 8 bit. I used something >>>>> like >>>>> >>>>> if (width == 2) >>>>> ret = pfl->ident[0] << 8 | pfl->ident[1]; /* rsp. ident[1]/ident[2] >>>>> */ >>>>> >>>>> within the 0x90 reading as a quick workaround. >>>> Are you sure that this is valid? The whole cfi_table is also only >>>> provided byte-wise, same in cfi01. >>> At least the JEDEC ID read returns 16 bit values with x16 devices. Have a look >>> at: >>> http://www.spansion.com/Support/Datasheets/s29gl128_256n_sp_a2_e.pdf >>> Table II on page 51 >>> >>> micromonitor (the program i tested with) and uboot uses 16bit reads to read >>> the flash id. Have a look at >>> http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=blob;f=drivers/mtd/cfi_flash.c;h=3267c5de36d1b12a190f93f9a3048ded598f84aa;hb=HEAD#l1535 >>> >> Right, I came to the same conclusion based on chip I'm using for the >> Musicpal model. Working on a proper fix - now that I think to have found >> a solution for the XIP vs. mode switch conflict. > > Wait! Access to ident[0..3] is already correct as those fields are > stored and returned as 16-bit values. I guess you just did not pass the > proper IDs when registering your pflash_cfi02 instance. ... or you suffered from a be/le issue. In contrast to data, the ID is not swapped according to the byte order that was specified during init. Does your target byte order differ from your host? > > But this still leaves us with the 8-bit entries of the cfi_table. > The CFI table is only returned byte-wise, even in 16- or 32-bit mode. But I guess it should be properly swapped to the right byte order nevertheless. Jan
Am Thursday 13 May 2010 14:38:07 schrieben Sie: > >> Right, I came to the same conclusion based on chip I'm using for the > >> Musicpal model. Working on a proper fix - now that I think to have found > >> a solution for the XIP vs. mode switch conflict. nice :) i'll try it as soon as it's ready. > > Wait! Access to ident[0..3] is already correct as those fields are > > stored and returned as 16-bit values. I guess you just did not pass the > > proper IDs when registering your pflash_cfi02 instance. > ... or you suffered from a be/le issue. In contrast to data, the ID is > not swapped according to the byte order that was specified during init. > Does your target byte order differ from your host? oh sorry, i wrongly assumed id[0..3] were the bytes for the manufacturer/device id. using the correct 16 bit values for id0 and id2 works perfectly.
diff --git a/exec-all.h b/exec-all.h index 1016de2..b070da9 100644 --- a/exec-all.h +++ b/exec-all.h @@ -329,6 +329,10 @@ static inline tb_page_addr_t get_page_addr_code(CPUState *env1, target_ulong add if (unlikely(env1->tlb_table[mmu_idx][page_index].addr_code != (addr & TARGET_PAGE_MASK))) { ldub_code(addr); + if (unlikely(env1->tlb_table[mmu_idx][page_index].addr_code & + TLB_INVALID_MASK)) { + ldub_code(addr); + } } pd = env1->tlb_table[mmu_idx][page_index].addr_code & ~TARGET_PAGE_MASK; if (pd > IO_MEM_ROM && !(pd & IO_MEM_ROMD)) { diff --git a/hw/pflash_cfi02.c b/hw/pflash_cfi02.c index f3d3f41..201e410 100644 --- a/hw/pflash_cfi02.c +++ b/hw/pflash_cfi02.c @@ -40,7 +40,7 @@ #include "qemu-timer.h" #include "block.h" -//#define PFLASH_DEBUG +#define PFLASH_DEBUG #ifdef PFLASH_DEBUG #define DPRINTF(fmt, ...) \ do { \ @@ -112,7 +112,7 @@ static uint32_t pflash_read (pflash_t *pfl, target_phys_addr_t offset, DPRINTF("%s: offset " TARGET_FMT_plx "\n", __func__, offset); ret = -1; - if (pfl->rom_mode) { + if (!pfl->rom_mode) { /* Lazy reset of to ROMD mode */ if (pfl->wcycle == 0)