| Submitter | Stefan Reinauer |
|---|---|
| Date | 2010-04-16 16:21:10 |
| Message ID | <4BC88E76.5050709@coresystems.de> |
| Download | mbox | patch |
| Permalink | /patch/1239/ |
| State | Accepted |
| Headers | show |
Comments
On 4/16/10 6:21 PM, Stefan Reinauer wrote: > Look at the return value of wait_82802ab() > > Signed-off-by: Stefan Reinauer <stepan@coresystems.de> > > Index: sst49lfxxxc.c > =================================================================== > --- sst49lfxxxc.c (revision 993) > +++ sst49lfxxxc.c (working copy) > @@ -84,6 +84,7 @@ > chip_writeb(0xD0, bios + address); > > status = wait_82802ab(bios); > + print_status_82802ab(status); > > if (check_erased_range(flash, address, sector_size)) { > msg_cerr("ERASE FAILED!\n"); > ping? :-)
Am Freitag, den 14.05.2010, 14:48 +0200 schrieb Stefan Reinauer: > On 4/16/10 6:21 PM, Stefan Reinauer wrote: > > Look at the return value of wait_82802ab() > > Index: sst49lfxxxc.c > > =================================================================== > > --- sst49lfxxxc.c (revision 993) > > +++ sst49lfxxxc.c (working copy) > > @@ -84,6 +84,7 @@ > > chip_writeb(0xD0, bios + address); > > > > status = wait_82802ab(bios); > > + print_status_82802ab(status); [...] basically a good idea, but printing one status dump per erased block seems a bit excessive, even in verbose mode, as there are some chips with 4K sector size. Could you limit status printing to "status != 0x80", as this is "ready, no error bits"? Regards, Michael Karcher
On 5/14/10 3:06 PM, Michael Karcher wrote: > Am Freitag, den 14.05.2010, 14:48 +0200 schrieb Stefan Reinauer: > >> On 4/16/10 6:21 PM, Stefan Reinauer wrote: >> >>> Look at the return value of wait_82802ab() >>> Index: sst49lfxxxc.c >>> =================================================================== >>> --- sst49lfxxxc.c (revision 993) >>> +++ sst49lfxxxc.c (working copy) >>> @@ -84,6 +84,7 @@ >>> chip_writeb(0xD0, bios + address); >>> >>> status = wait_82802ab(bios); >>> + print_status_82802ab(status); >>> > [...] > basically a good idea, but printing one status dump per erased block > seems a bit excessive, even in verbose mode, as there are some chips > with 4K sector size. Could you limit status printing to "status != > 0x80", as this is "ready, no error bits"? > That's what the 82802ab and sharp lhf00l04 do, too. So if you think the behavior should be changed, you should change it in all three places. Stefan
On 14.05.2010 15:51, Stefan Reinauer wrote: > On 5/14/10 3:06 PM, Michael Karcher wrote: > >> Am Freitag, den 14.05.2010, 14:48 +0200 schrieb Stefan Reinauer: >> >> >>> On 4/16/10 6:21 PM, Stefan Reinauer wrote: >>> >>> >>>> Look at the return value of wait_82802ab() >>>> Index: sst49lfxxxc.c >>>> =================================================================== >>>> --- sst49lfxxxc.c (revision 993) >>>> +++ sst49lfxxxc.c (working copy) >>>> @@ -84,6 +84,7 @@ >>>> chip_writeb(0xD0, bios + address); >>>> >>>> status = wait_82802ab(bios); >>>> + print_status_82802ab(status); >>>> >>>> >> [...] >> basically a good idea, but printing one status dump per erased block >> seems a bit excessive, even in verbose mode, as there are some chips >> with 4K sector size. Could you limit status printing to "status != >> 0x80", as this is "ready, no error bits"? >> >> > That's what the 82802ab and sharp lhf00l04 do, too. So if you think the > behavior should be changed, you should change it in all three places. > That's the plan. Regards, Carl-Daniel
On Sat, May 15, 2010 at 12:29:18AM +0200, Carl-Daniel Hailfinger wrote: > >>>> Look at the return value of wait_82802ab() > >>>> Index: sst49lfxxxc.c > >>>> =================================================================== > >>>> --- sst49lfxxxc.c (revision 993) > >>>> +++ sst49lfxxxc.c (working copy) > >>>> @@ -84,6 +84,7 @@ > >>>> chip_writeb(0xD0, bios + address); > >>>> > >>>> status = wait_82802ab(bios); > >>>> + print_status_82802ab(status); > >>>> > >>>> > >> [...] > >> basically a good idea, but printing one status dump per erased block > >> seems a bit excessive, even in verbose mode, as there are some chips > >> with 4K sector size. Could you limit status printing to "status != > >> 0x80", as this is "ready, no error bits"? > >> > > That's what the 82802ab and sharp lhf00l04 do, too. So if you think the > > behavior should be changed, you should change it in all three places. > > That's the plan. In the mean-time I guess we can get this in (r1347), if only to reduce the patch queue size. Uwe.
Am Sonntag, den 19.06.2011, 19:11 +0200 schrieb Uwe Hermann: > On Sat, May 15, 2010 at 12:29:18AM +0200, Carl-Daniel Hailfinger wrote: > > >>>> Look at the return value of wait_82802ab() > > >>>> Index: sst49lfxxxc.c > > >>>> =================================================================== > > >>>> --- sst49lfxxxc.c (revision 993) > > >>>> +++ sst49lfxxxc.c (working copy) > > >>>> @@ -84,6 +84,7 @@ > > >>>> chip_writeb(0xD0, bios + address); > > >>>> > > >>>> status = wait_82802ab(bios); > > >>>> + print_status_82802ab(status); > > >>>> > > >>>> > > >> [...] > > >> basically a good idea, but printing one status dump per erased block > > >> seems a bit excessive, even in verbose mode, as there are some chips > > >> with 4K sector size. Could you limit status printing to "status != > > >> 0x80", as this is "ready, no error bits"? > > >> > > > That's what the 82802ab and sharp lhf00l04 do, too. So if you think the > > > behavior should be changed, you should change it in all three places. > > > > That's the plan. > > In the mean-time I guess we can get this in (r1347), if only to reduce > the patch queue size. Getting this in is a good idea, as IIRC this is the point gcc 4.6 disliked (for not using the status variable). Not printing the chip status 512 times also is a good ides, so I sent a patch that implements "print status only if abnormal". Sorry for working against getting the patch queue small, but we shouldn't forget it. BTW: the 82802ab and lhf00l04 *block* erase procedures operate on blocks around 64k, while the sst49 *sector* erase procedure that just was changed in this commit is for 4k sectors, so dumping the status here is much more excessive as in the other cases - probably that's the reason why the status printing was not in. Regards, Michael Karcher
Patch
Index: sst49lfxxxc.c =================================================================== --- sst49lfxxxc.c (revision 993) +++ sst49lfxxxc.c (working copy) @@ -84,6 +84,7 @@ chip_writeb(0xD0, bios + address); status = wait_82802ab(bios); + print_status_82802ab(status); if (check_erased_range(flash, address, sector_size)) { msg_cerr("ERASE FAILED!\n");