if_msk.c link negotiation / packet drops

[YongHyeon PYUN]

On Thu, Oct 13, 2011 at 05:02:58PM -0400, Arnaud Lacombe wrote:
> Hi,
> 
> On Thu, Oct 13, 2011 at 4:47 PM, YongHyeon PYUN <pyunyh at gmail.com> wrote:
> > On Thu, Oct 13, 2011 at 12:59:35AM -0700, perryh at pluto.rain.com wrote:
> >> YongHyeon PYUN <pyunyh at gmail.com> wrote:
> >> > On Wed, Oct 12, 2011 at 10:07:02AM -0400, Karim wrote:
> >> > > ... why are we ORing the same call twice isn't the same thing
> >> > > as calling it once:
> >> > >
> >> > > bmsr = PHY_READ(sc, E1000_SR) | PHY_READ(sc, E1000_SR);
> >> >
> >> > The E1000_SR_LINK_STATUS bit is latched low so it should be read
> >> > twice.
> >>
> >> It might not be a bad idea to check the generated code to be sure
> >> that the read _is_ being done twice. ?An optimizer might well come
> >> to the same conclusion as Karim, and discard the "redundant" second
> >> instance (unless there's a "volatile" declaration somewhere in the
> >> expansion of PHY_READ, to explicitly indicate that it has side
> >> effects).
> >
> > Last time I checked it, compiler generated correct code.
> > Tried again on amd64 and I can still see the code is there.
> >
> What about other architecture (especially i386) ? which optimization

Don't use i386 so I don't know.

> level did you use ? which compiler version ?

CURRENT, default optimization(O2).

> 
> About the last question, I know for sure that there has been change in
> FreeBSD's gcc between 7-STABLE, and FreeBSD -CURRENT.
> 
> I agree with perryh@ than such hardware requirement _requires_ being
> explicit in the code, ie proper `volatile' marking.
> 

I'm not saying adding more safe belt is bad idea. If you have a
patch please submit it. I don't like touching every PHY drivers.

>  - Arnaud
>