| Message ID | 20130110120339.GA21794@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Jan 10, 2013 at 02:03:39PM +0200, Michael S. Tsirkin wrote: > Add code comment to clarify the reason we set ICS with ICR: > the reason was previously undocumented and git > log (commit b1332393cdd7d023de8f1f8aa136ee7866a18968) > confused rather than clarified the comments. > Digging in the mailing list archives gives the real reason > https://lists.gnu.org/archive/html/qemu-devel/2009-06/msg00401.html > > Add code comment with an explanation supplied by Bill Paul. > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> This is v2 of the patch. > --- > hw/e1000.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/hw/e1000.c b/hw/e1000.c > index 92fb00a..d10119c 100644 > --- a/hw/e1000.c > +++ b/hw/e1000.c > @@ -230,7 +230,17 @@ set_interrupt_cause(E1000State *s, int index, uint32_t val) > val |= E1000_ICR_INT_ASSERTED; > } > s->mac_reg[ICR] = val; > + > + /* > + * Make sure ICR and ICS registers have the same value. > + * The spec says that the ICS register is write-only. However in practice, > + * on real hardware ICS is readable, and for reads it has the same value as > + * ICR (except that ICS does not have the clear on read behaviour of ICR). > + * > + * The VxWorks PRO/1000 driver uses this behaviour. > + */ > s->mac_reg[ICS] = val; > + > qemu_set_irq(s->dev.irq[0], (s->mac_reg[IMS] & s->mac_reg[ICR]) != 0); > } > > -- > MST
On Thu, Jan 10, 2013 at 02:03:39PM +0200, Michael S. Tsirkin wrote: > Add code comment to clarify the reason we set ICS with ICR: > the reason was previously undocumented and git > log (commit b1332393cdd7d023de8f1f8aa136ee7866a18968) > confused rather than clarified the comments. > Digging in the mailing list archives gives the real reason > https://lists.gnu.org/archive/html/qemu-devel/2009-06/msg00401.html > > Add code comment with an explanation supplied by Bill Paul. > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > --- > hw/e1000.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) Thanks, applied to the net tree: https://github.com/stefanha/qemu/commits/net Stefan
diff --git a/hw/e1000.c b/hw/e1000.c index 92fb00a..d10119c 100644 --- a/hw/e1000.c +++ b/hw/e1000.c @@ -230,7 +230,17 @@ set_interrupt_cause(E1000State *s, int index, uint32_t val) val |= E1000_ICR_INT_ASSERTED; } s->mac_reg[ICR] = val; + + /* + * Make sure ICR and ICS registers have the same value. + * The spec says that the ICS register is write-only. However in practice, + * on real hardware ICS is readable, and for reads it has the same value as + * ICR (except that ICS does not have the clear on read behaviour of ICR). + * + * The VxWorks PRO/1000 driver uses this behaviour. + */ s->mac_reg[ICS] = val; + qemu_set_irq(s->dev.irq[0], (s->mac_reg[IMS] & s->mac_reg[ICR]) != 0); }
Add code comment to clarify the reason we set ICS with ICR: the reason was previously undocumented and git log (commit b1332393cdd7d023de8f1f8aa136ee7866a18968) confused rather than clarified the comments. Digging in the mailing list archives gives the real reason https://lists.gnu.org/archive/html/qemu-devel/2009-06/msg00401.html Add code comment with an explanation supplied by Bill Paul. Signed-off-by: Michael S. Tsirkin <mst@redhat.com> --- hw/e1000.c | 10 ++++++++++ 1 file changed, 10 insertions(+)