Re: [PATCH 3/3] HACK: e1000: don't check for FLSWCTL.GLDONE when waiting for idle

[Andrey Smirnov <andrew.smirnov@gmail.com>]

On Mon, Oct 9, 2017 at 7:24 AM, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> 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
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.

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.

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.

Thanks,
Andrey Smirnov

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox