| Message ID | 0730056dfbd2e9b64ff874bddfb72b6dea72a2a0.1385967754.git.peter.crosthwaite@xilinx.com |
|---|---|
| State | New |
| Headers | show |
On 2 December 2013 07:13, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote: > The Specific address registers can be enabled or disabled by software. > QEMU was assuming they where always enabled. Implement the "were" > disable/enable feature. SARs are disabled by writing to the lower half > register. They are re-enabled by then writing the upper half. > > Reported-by: Deepika Dhamija <deepika@xilinx.com> > Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> > --- > > hw/net/cadence_gem.c | 20 +++++++++++++++++++- > 1 file changed, 19 insertions(+), 1 deletion(-) > > diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c > index 6f11d6a..c6eb9ab 100644 > --- a/hw/net/cadence_gem.c > +++ b/hw/net/cadence_gem.c > @@ -382,6 +382,7 @@ typedef struct GemState { > > unsigned rx_desc[2]; > > + bool sar_active[4]; This state needs to be added to the VMState struct as well. > } GemState; > > /* The broadcast MAC address: 0xFFFFFFFFFFFF */ > @@ -603,7 +604,7 @@ static int gem_mac_address_filter(GemState *s, const uint8_t *packet) > /* Check all 4 specific addresses */ > gem_spaddr = (uint8_t *)&(s->regs[GEM_SPADDR1LO]); > for (i = 3; i >= 0; i--) { > - if (!memcmp(packet, gem_spaddr + 8 * i, 6)) { > + if (s->sar_active[i] && !memcmp(packet, gem_spaddr + 8 * i, 6)) { > return GEM_RX_SAR_ACCEPT + i; > } > } > @@ -985,6 +986,7 @@ static void gem_phy_reset(GemState *s) > > static void gem_reset(DeviceState *d) > { > + int i; > GemState *s = GEM(d); > > DB_PRINT("\n"); > @@ -1004,6 +1006,10 @@ static void gem_reset(DeviceState *d) > s->regs[GEM_DESCONF5] = 0x002f2145; > s->regs[GEM_DESCONF6] = 0x00000200; > > + for (i = 0; i < 4; ++i) { "i++" is more idiomatic for C. Otherwise looks good. thanks -- PMM
On Mon, Dec 2, 2013 at 10:23 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 2 December 2013 07:13, Peter Crosthwaite > <peter.crosthwaite@xilinx.com> wrote: >> The Specific address registers can be enabled or disabled by software. >> QEMU was assuming they where always enabled. Implement the > > "were" > Fixed >> disable/enable feature. SARs are disabled by writing to the lower half >> register. They are re-enabled by then writing the upper half. >> >> Reported-by: Deepika Dhamija <deepika@xilinx.com> >> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> >> --- >> >> hw/net/cadence_gem.c | 20 +++++++++++++++++++- >> 1 file changed, 19 insertions(+), 1 deletion(-) >> >> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c >> index 6f11d6a..c6eb9ab 100644 >> --- a/hw/net/cadence_gem.c >> +++ b/hw/net/cadence_gem.c >> @@ -382,6 +382,7 @@ typedef struct GemState { >> >> unsigned rx_desc[2]; >> >> + bool sar_active[4]; > > This state needs to be added to the VMState struct as well. > Done. >> } GemState; >> >> /* The broadcast MAC address: 0xFFFFFFFFFFFF */ >> @@ -603,7 +604,7 @@ static int gem_mac_address_filter(GemState *s, const uint8_t *packet) >> /* Check all 4 specific addresses */ >> gem_spaddr = (uint8_t *)&(s->regs[GEM_SPADDR1LO]); >> for (i = 3; i >= 0; i--) { >> - if (!memcmp(packet, gem_spaddr + 8 * i, 6)) { >> + if (s->sar_active[i] && !memcmp(packet, gem_spaddr + 8 * i, 6)) { >> return GEM_RX_SAR_ACCEPT + i; >> } >> } >> @@ -985,6 +986,7 @@ static void gem_phy_reset(GemState *s) >> >> static void gem_reset(DeviceState *d) >> { >> + int i; >> GemState *s = GEM(d); >> >> DB_PRINT("\n"); >> @@ -1004,6 +1006,10 @@ static void gem_reset(DeviceState *d) >> s->regs[GEM_DESCONF5] = 0x002f2145; >> s->regs[GEM_DESCONF6] = 0x00000200; >> >> + for (i = 0; i < 4; ++i) { > > "i++" is more idiomatic for C. > > Otherwise looks good. > Fixed, although I do see both used quite liberally. I'll use foo++ when local coding style doesn't contradict. Regards, Peter > thanks > -- PMM >
diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c index 6f11d6a..c6eb9ab 100644 --- a/hw/net/cadence_gem.c +++ b/hw/net/cadence_gem.c @@ -382,6 +382,7 @@ typedef struct GemState { unsigned rx_desc[2]; + bool sar_active[4]; } GemState; /* The broadcast MAC address: 0xFFFFFFFFFFFF */ @@ -603,7 +604,7 @@ static int gem_mac_address_filter(GemState *s, const uint8_t *packet) /* Check all 4 specific addresses */ gem_spaddr = (uint8_t *)&(s->regs[GEM_SPADDR1LO]); for (i = 3; i >= 0; i--) { - if (!memcmp(packet, gem_spaddr + 8 * i, 6)) { + if (s->sar_active[i] && !memcmp(packet, gem_spaddr + 8 * i, 6)) { return GEM_RX_SAR_ACCEPT + i; } } @@ -985,6 +986,7 @@ static void gem_phy_reset(GemState *s) static void gem_reset(DeviceState *d) { + int i; GemState *s = GEM(d); DB_PRINT("\n"); @@ -1004,6 +1006,10 @@ static void gem_reset(DeviceState *d) s->regs[GEM_DESCONF5] = 0x002f2145; s->regs[GEM_DESCONF6] = 0x00000200; + for (i = 0; i < 4; ++i) { + s->sar_active[i] = false; + } + gem_phy_reset(s); gem_update_int_status(s); @@ -1153,6 +1159,18 @@ static void gem_write(void *opaque, hwaddr offset, uint64_t val, s->regs[GEM_IMR] |= val; gem_update_int_status(s); break; + case GEM_SPADDR1LO: + case GEM_SPADDR2LO: + case GEM_SPADDR3LO: + case GEM_SPADDR4LO: + s->sar_active[(offset - GEM_SPADDR1LO) / 2] = false; + break; + case GEM_SPADDR1HI: + case GEM_SPADDR2HI: + case GEM_SPADDR3HI: + case GEM_SPADDR4HI: + s->sar_active[(offset - GEM_SPADDR1HI) / 2] = true; + break; case GEM_PHYMNTNC: if (val & GEM_PHYMNTNC_OP_W) { uint32_t phy_addr, reg_num;
The Specific address registers can be enabled or disabled by software. QEMU was assuming they where always enabled. Implement the disable/enable feature. SARs are disabled by writing to the lower half register. They are re-enabled by then writing the upper half. Reported-by: Deepika Dhamija <deepika@xilinx.com> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> --- hw/net/cadence_gem.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)