Re: [PATCH 1/1] e1000e: Added ICR clearing by corresponding IMS bit.

[Jason Wang]

On 2020/5/29 下午3:18, Jason Wang wrote:
> On 2020/5/13 下午7:31, andrew@daynix.com wrote:
> > From: Andrew Melnychenko <andrew@daynix.com>
> >
> > Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1707441
> > Added ICR clearing if there is IMS bit - according to the note by
> > section 13.3.27 of the 8257X developers manual.
> >
> > Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
> > ---
> >   hw/net/e1000e_core.c | 10 ++++++++++
> >   hw/net/trace-events  |  1 +
> >   2 files changed, 11 insertions(+)
> >
> > diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c
> > index d5676871fa..10212d7932 100644
> > --- a/hw/net/e1000e_core.c
> > +++ b/hw/net/e1000e_core.c
> > @@ -2624,6 +2624,16 @@ e1000e_mac_icr_read(E1000ECore *core, int index)
> >           e1000e_clear_ims_bits(core, core->mac[IAM]);
> >       }
> >   +    /*
> > +     * PCIe* GbE Controllers Open Source Software Developer's Manual
> > +     * 13.3.27 Interrupt Cause Read Register
> > +     */
> > +    if ((core->mac[ICR] & E1000_ICR_ASSERTED) &&
> > +        (core->mac[ICR] & core->mac[IMS])) {
> > + trace_e1000e_irq_icr_clear_icr_bit_ims(core->mac[ICR], 
> > core->mac[IMS]);
> > +        core->mac[ICR] = 0;
> > +    }
> > +
>
>
> Hi Andrew:
>
> So my comments still. I think we need to implement 82574l behavior (if 
> you go through e1000e.c all chapters it mentioned is for 82574l 
> datasheet not the one you pointed to me).
>
> And actually the 82574l behavior is much more simpler.


To be more specific.

See chapter 7.4.5 which describes the ICR clearing.

It has three methods for clearing: auto-clear, clear-on-write and 
clear-on-read.

And in the part of "Read to clear" it said:

"""
All bits in the ICR register are cleared on a read to ICR.

"""

So there's no need to IMS and other stuffs here.

Thanks


> Thanks