Re: [Qemu-devel] [PATCH v2 1/2] e1000: Clear MDIC register when PHY addr is invalid

[Jason Wang]

----- 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 <address@hidden>
> Signed-off-by: Jeff Westfahl <address@hidden>
> ---

Looks correct.

Reviewed-by: Jason Wang <address@hidden>
>  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
> 
> 
>