On Mon, Oct 09, 2017 at 11:15:09AM -0700, Andrey Smirnov wrote: > On Mon, Oct 9, 2017 at 7:24 AM, Uwe Kleine-König > <u.kleine-koenig@xxxxxxxxxxxxxx> wrote: > > On Mon, Oct 09, 2017 at 11:36:16AM +0200, Uwe Kleine-König wrote: > >> I don't understand all the consequences of this patch yet, but this makes reading > >> out the flash chip connected to an i210 work for me. > >> --- > >> drivers/net/e1000/eeprom.c | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/net/e1000/eeprom.c b/drivers/net/e1000/eeprom.c > >> index 739bc17a519e..482a969f8d56 100644 > >> --- a/drivers/net/e1000/eeprom.c > >> +++ b/drivers/net/e1000/eeprom.c > >> @@ -709,8 +709,8 @@ static int e1000_flash_mode_wait_for_idle(struct e1000_hw *hw) > >> * execution by polling only FLSWCTL.DONE */ > >> > >> const int ret = e1000_poll_reg(hw, E1000_FLSWCTL, > >> - E1000_FLSWCTL_DONE | E1000_FLSWCTL_GLDONE, > >> - E1000_FLSWCTL_DONE | E1000_FLSWCTL_GLDONE, > >> + E1000_FLSWCTL_DONE, > >> + E1000_FLSWCTL_DONE, > > > > I tested a bit with and without this change at it seems as long as > > nothing "strange" happens, testing for both FLSWCTL.DONE and > > FLSWCTL.GLDONE (i.e. not applying my HACK patch) works fine. > > > > Still I think only testing for FLSWCTL.DONE is better because it works > > well even if the state machine is in the middle of a read request and > > then changing the command (which is always done after > > e1000_flash_mode_wait_for_idle()) should work well. > > > > I'll resend with a better commit log once I tested this. > > > > Alexey: I didn't understand the comment above the patched line, maybe > > I'm missing something? > > > > Assuming this question is for me: what I meant by that comment is that Ah yes, sorry. It seems I have to participate in the cut'n'paste seminar, too. > all of the flash related operations (read, write, erase) already poll > for E1000_FLSWCTL_DONE as their last step, so the only time that the > state of that bit would be unknown would be right after reset, the > first time any of those functions is executed. Right after reset or when someone poked in the hardware using mw/md, or if a previous transaction was not completed (nor sure this can happen though). > As for GLDONE bit, I tried to stick to algorithms described in > "3.3.5.5 Software Flash Program Flow via the Flash-Mode Interface" and > "3.3.5.6 Software Flash Read Flow via the Flash-Mode Interface" of Rev > 2.8 of the datasheet and that's where that "requrement" is coming > from. 3.3.5.5 and 3.3.5.6 in Rev 3.1 only check .DONE at the start and for the last step tell (for 3.3.5.5): FLSWCTL.GLDONE bit is set by hardware when the last byte programmed has been written. So I'd not use that to check if I can start. Still more as there is: But software can stop the transaction in the middle as long as it got the DONE bit read as 1b. > I haven't touched this particular HW/area in more than a year, so, > since you actually work with it and can actually test things out I'll > defer to you to judge if certain HW checks are needed or not. My code > is by no means complete or exhaustively tested, so I have no problem > believing that I got some of the parts wrong. OK. So I will send a v2 that drops checking for GLDONE. Thanks for your replies. Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ | _______________________________________________ barebox mailing list barebox@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/barebox
