Message ID | 1278770625-6757-1-git-send-email-tsutsui@ceres.dti.ne.jp |
---|---|
State | New |
Headers | show |
On Sat, Jul 10, 2010 at 11:03:45PM +0900, Izumi Tsutsui wrote: > This change fixes initialization of e1000's microwire EEPROM internal > state values so that qemu's e1000 emulation works on NetBSD, > which doesn't use Intel's em driver but has its own wm driver > for the Intel i8254x Gigabit Ethernet. > > Previously set_eecd() function in e1000.c clears EEPROM internal state > values on SK rising edge during CS==L, but according to FM93C06 EEPROM > (which is MicroWire compatible) data sheet, EEPROM internal status > should be cleared on CS rise edge regardless of SK input: > "... a rising edge on this (CS) signal is required to reset the internal > state-machine to accept a new cycle .." > and nothing should be changed during CS (chip select) is inactive. > > Intel's em driver seems to explicitly raise SK output after CS is negated > in em_standby_eeprom() so many other OSes that use Intel's driver > don't have this problem even on the previous e1000.c implementation, > but I can't find any articles that say the MICROWIRE or EEPROM spec > requires such sequence, and actually hardware works fine without it > (i.e. real i82540EM has been working on NetBSD). > > This fix also changes initialization to clear each state value in > struct eecd_state individually rather than using memset() against > the whole structre. The old_eecd member stores the last SK and CS > signal levels and it should be preserved even after reset of internal > EEPROM state to detect next signal edges for proper EEPROM emulation. > > Signed-off-by: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp> > --- > hw/e1000.c | 17 ++++++++--------- > 1 files changed, 8 insertions(+), 9 deletions(-) Thanks, applied. > diff --git a/hw/e1000.c b/hw/e1000.c > index 0da65f9..db9143d 100644 > --- a/hw/e1000.c > +++ b/hw/e1000.c > @@ -262,21 +262,20 @@ set_eecd(E1000State *s, int index, uint32_t val) > > s->eecd_state.old_eecd = val & (E1000_EECD_SK | E1000_EECD_CS | > E1000_EECD_DI|E1000_EECD_FWE_MASK|E1000_EECD_REQ); > + if (!(E1000_EECD_CS & val)) // CS inactive; nothing to do > + return; > + if (E1000_EECD_CS & (val ^ oldval)) { // CS rise edge; reset state > + s->eecd_state.val_in = 0; > + s->eecd_state.bitnum_in = 0; > + s->eecd_state.bitnum_out = 0; > + s->eecd_state.reading = 0; > + } > if (!(E1000_EECD_SK & (val ^ oldval))) // no clock edge > return; > if (!(E1000_EECD_SK & val)) { // falling edge > s->eecd_state.bitnum_out++; > return; > } > - if (!(val & E1000_EECD_CS)) { // rising, no CS (EEPROM reset) > - memset(&s->eecd_state, 0, sizeof s->eecd_state); > - /* > - * restore old_eecd's E1000_EECD_SK (known to be on) > - * to avoid false detection of a clock edge > - */ > - s->eecd_state.old_eecd = E1000_EECD_SK; > - return; > - } > s->eecd_state.val_in <<= 1; > if (val & E1000_EECD_DI) > s->eecd_state.val_in |= 1; > -- > 1.6.6.2 > > >
diff --git a/hw/e1000.c b/hw/e1000.c index 0da65f9..db9143d 100644 --- a/hw/e1000.c +++ b/hw/e1000.c @@ -262,21 +262,20 @@ set_eecd(E1000State *s, int index, uint32_t val) s->eecd_state.old_eecd = val & (E1000_EECD_SK | E1000_EECD_CS | E1000_EECD_DI|E1000_EECD_FWE_MASK|E1000_EECD_REQ); + if (!(E1000_EECD_CS & val)) // CS inactive; nothing to do + return; + if (E1000_EECD_CS & (val ^ oldval)) { // CS rise edge; reset state + s->eecd_state.val_in = 0; + s->eecd_state.bitnum_in = 0; + s->eecd_state.bitnum_out = 0; + s->eecd_state.reading = 0; + } if (!(E1000_EECD_SK & (val ^ oldval))) // no clock edge return; if (!(E1000_EECD_SK & val)) { // falling edge s->eecd_state.bitnum_out++; return; } - if (!(val & E1000_EECD_CS)) { // rising, no CS (EEPROM reset) - memset(&s->eecd_state, 0, sizeof s->eecd_state); - /* - * restore old_eecd's E1000_EECD_SK (known to be on) - * to avoid false detection of a clock edge - */ - s->eecd_state.old_eecd = E1000_EECD_SK; - return; - } s->eecd_state.val_in <<= 1; if (val & E1000_EECD_DI) s->eecd_state.val_in |= 1;
This change fixes initialization of e1000's microwire EEPROM internal state values so that qemu's e1000 emulation works on NetBSD, which doesn't use Intel's em driver but has its own wm driver for the Intel i8254x Gigabit Ethernet. Previously set_eecd() function in e1000.c clears EEPROM internal state values on SK rising edge during CS==L, but according to FM93C06 EEPROM (which is MicroWire compatible) data sheet, EEPROM internal status should be cleared on CS rise edge regardless of SK input: "... a rising edge on this (CS) signal is required to reset the internal state-machine to accept a new cycle .." and nothing should be changed during CS (chip select) is inactive. Intel's em driver seems to explicitly raise SK output after CS is negated in em_standby_eeprom() so many other OSes that use Intel's driver don't have this problem even on the previous e1000.c implementation, but I can't find any articles that say the MICROWIRE or EEPROM spec requires such sequence, and actually hardware works fine without it (i.e. real i82540EM has been working on NetBSD). This fix also changes initialization to clear each state value in struct eecd_state individually rather than using memset() against the whole structre. The old_eecd member stores the last SK and CS signal levels and it should be preserved even after reset of internal EEPROM state to detect next signal edges for proper EEPROM emulation. Signed-off-by: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp> --- hw/e1000.c | 17 ++++++++--------- 1 files changed, 8 insertions(+), 9 deletions(-)