diff mbox

[v2,1/2] e1000: Clear MDIC register when PHY addr is invalid

Message ID 1418275427-18921-2-git-send-email-rich.tollerton@ni.com
State New
Headers show

Commit Message

Richard Tollerton Dec. 11, 2014, 5:23 a.m. UTC 
Some drivers probe PHY addresses beyond the first one, and
also (unfortunately) don't check for MDIC_ERROR on reads. It appears
that the driver assumes that the data reads will return zero in this
case (invalid PHY address). Anecdotally, hardware is believed to do
this; but qemu wasn't, and instead was reusing the previous value of the
MDIC register. This screwed up driver init.

To fix, don't reuse the current value of the MDIC register when an
invalid PHY address is referenced; let the value be set to precisely
MDIC_ERROR|MDIC_READY.

Signed-off-by: Richard Tollerton <rich.tollerton@ni.com>
Signed-off-by: Jeff Westfahl <jeff.westfahl@ni.com>
---
 hw/net/e1000.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jason Wang Dec. 18, 2014, 5:08 a.m. UTC | #1 
----- Original Message -----
> Some drivers probe PHY addresses beyond the first one, and
> also (unfortunately) don't check for MDIC_ERROR on reads. It appears
> that the driver assumes that the data reads will return zero in this
> case (invalid PHY address). Anecdotally, hardware is believed to do
> this; but qemu wasn't, and instead was reusing the previous value of the
> MDIC register. This screwed up driver init.
> 
> To fix, don't reuse the current value of the MDIC register when an
> invalid PHY address is referenced; let the value be set to precisely
> MDIC_ERROR|MDIC_READY.
> 
> Signed-off-by: Richard Tollerton <rich.tollerton@ni.com>
> Signed-off-by: Jeff Westfahl <jeff.westfahl@ni.com>
> ---

Looks correct.

Reviewed-by: Jason Wang <jasowang@redhat.com>
>  hw/net/e1000.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> index e33a4da..44ae3a8 100644
> --- a/hw/net/e1000.c
> +++ b/hw/net/e1000.c
> @@ -457,7 +457,7 @@ set_mdic(E1000State *s, int index, uint32_t val)
>      uint32_t addr = ((val & E1000_MDIC_REG_MASK) >> E1000_MDIC_REG_SHIFT);
>  
>      if ((val & E1000_MDIC_PHY_MASK) >> E1000_MDIC_PHY_SHIFT != 1) // phy #
> -        val = s->mac_reg[MDIC] | E1000_MDIC_ERROR;
> +        val = E1000_MDIC_ERROR;
>      else if (val & E1000_MDIC_OP_READ) {
>          DBGOUT(MDIC, "MDIC read reg 0x%x\n", addr);
>          if (!(phy_regcap[addr] & PHY_R)) {
> --
> 2.1.3
> 
> 
>
diff mbox

Patch

diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index e33a4da..44ae3a8 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -457,7 +457,7 @@  set_mdic(E1000State *s, int index, uint32_t val)
     uint32_t addr = ((val & E1000_MDIC_REG_MASK) >> E1000_MDIC_REG_SHIFT);
 
     if ((val & E1000_MDIC_PHY_MASK) >> E1000_MDIC_PHY_SHIFT != 1) // phy #
-        val = s->mac_reg[MDIC] | E1000_MDIC_ERROR;
+        val = E1000_MDIC_ERROR;
     else if (val & E1000_MDIC_OP_READ) {
         DBGOUT(MDIC, "MDIC read reg 0x%x\n", addr);
         if (!(phy_regcap[addr] & PHY_R)) {