From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from metis.ext.pengutronix.de ([2001:67c:670:201:290:27ff:fe1d:cc33]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1e1czS-0008JV-3y for barebox@lists.infradead.org; Mon, 09 Oct 2017 18:41:24 +0000 Date: Mon, 9 Oct 2017 20:41:00 +0200 From: Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= Message-ID: <20171009184100.obdgbcj4acz6efyy@pengutronix.de> References: <20171009093616.12686-1-u.kleine-koenig@pengutronix.de> <20171009093616.12686-4-u.kleine-koenig@pengutronix.de> <20171009142408.2g7k4mrfjq3tffwa@pengutronix.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Sender: "barebox" Errors-To: barebox-bounces+u.kleine-koenig=pengutronix.de@lists.infradead.org Subject: Re: [PATCH 3/3] HACK: e1000: don't check for FLSWCTL.GLDONE when waiting for idle To: Andrey Smirnov Cc: "barebox@lists.infradead.org" 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=F6nig > wrote: > > On Mon, Oct 09, 2017 at 11:36:16AM +0200, Uwe Kleine-K=F6nig wrote: > >> I don't understand all the consequences of this patch yet, but this ma= kes 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 e= 1000_hw *hw) > >> * execution by polling only FLSWCTL.DONE */ > >> > >> const int ret =3D e1000_poll_reg(hw, E1000_FLSWCTL, > >> - E1000_FLSWCTL_DONE | E1000_FLSWCT= L_GLDONE, > >> - E1000_FLSWCTL_DONE | E1000_FLSWCT= L_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=F6nig | Industrial Linux Solutions | http://www.pengutronix.de/ | _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox