From owner-freebsd-net@FreeBSD.ORG Thu Oct 13 22:11:51 2011 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 59E46106564A for ; Thu, 13 Oct 2011 22:11:51 +0000 (UTC) (envelope-from lacombar@gmail.com) Received: from mail-ww0-f42.google.com (mail-ww0-f42.google.com [74.125.82.42]) by mx1.freebsd.org (Postfix) with ESMTP id E26178FC14 for ; Thu, 13 Oct 2011 22:11:49 +0000 (UTC) Received: by wwg7 with SMTP id 7so303006wwg.1 for ; Thu, 13 Oct 2011 15:11:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; bh=Fhw+NKNDevV1FHlJm+Xoxe99vLBzB7+qnu7fZ400utc=; b=DyX+K/dZOnxZXahkyWq7Bn8gRVCuAdTZDpliML/tMOb8I0Le67OhMg3xb123qtxeqI ShmJ06ynvj343OvrH4dDaP9/1MYNqvnfCRZFkkWfjCZ37iKC62OLbXK3VnId8Tk06Brw s6CMcWZPFk3SiyecPqMQy9iS5gewJmsGePLms= MIME-Version: 1.0 Received: by 10.216.210.216 with SMTP id u66mr127245weo.49.1318543908227; Thu, 13 Oct 2011 15:11:48 -0700 (PDT) Received: by 10.180.103.198 with HTTP; Thu, 13 Oct 2011 15:11:48 -0700 (PDT) In-Reply-To: <20111013215405.GB13219@michelle.cdnetworks.com> References: <4E94637A.5090607@gmail.com> <20111011171029.GA5661@michelle.cdnetworks.com> <4E959F06.6040906@gmail.com> <20111012170347.GA9138@michelle.cdnetworks.com> <4e969a67.YJyWMt0xI7pFL+xJ%perryh@pluto.rain.com> <20111013204747.GA13219@michelle.cdnetworks.com> <20111013215405.GB13219@michelle.cdnetworks.com> Date: Thu, 13 Oct 2011 18:11:48 -0400 Message-ID: From: Arnaud Lacombe To: pyunyh@gmail.com Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: fodillemlinkarim@gmail.com, freebsd-net@freebsd.org, perryh@pluto.rain.com, kob6558@gmail.com Subject: Re: if_msk.c link negotiation / packet drops X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 13 Oct 2011 22:11:51 -0000 Hi, On Thu, Oct 13, 2011 at 5:54 PM, YongHyeon PYUN wrote: > 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 wrote= : >> > On Thu, Oct 13, 2011 at 12:59:35AM -0700, perryh@pluto.rain.com wrote: >> >> YongHyeon PYUN 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 =3D 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. > Actually, it should not be needed, the generic implementation of PHY_READREG() is doing an MIIBUS_READREG() which is forwarded to the parent (miibusN here), then its parent (msk(4)). my bad, - Arnaud >> =A0- Arnaud >> >