From owner-freebsd-net@FreeBSD.ORG Tue Aug 28 01:03:22 2007 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 CDFB316A417 for ; Tue, 28 Aug 2007 01:03:22 +0000 (UTC) (envelope-from pyunyh@gmail.com) Received: from wa-out-1112.google.com (wa-out-1112.google.com [209.85.146.176]) by mx1.freebsd.org (Postfix) with ESMTP id 86B8813C457 for ; Tue, 28 Aug 2007 01:03:22 +0000 (UTC) (envelope-from pyunyh@gmail.com) Received: by wa-out-1112.google.com with SMTP id k17so2327985waf for ; Mon, 27 Aug 2007 18:03:21 -0700 (PDT) DKIM-Signature: a=rsa-sha1; c=relaxed/relaxed; d=gmail.com; s=beta; h=domainkey-signature:received:received:received:received:date:from:to:cc:subject:message-id:reply-to:references:mime-version:content-type:content-disposition:in-reply-to:user-agent; b=lUFpwrIcYX1wSsYUxMdi7zqkaGOVrhLJmxBNx7jNvFQKsJBgmcuMjc315dT5gJ1VYVPGrHiLAkuvhRUSooTEwcI97taud4qqGMs/Tjeo4BpeQx06O7fX2OFltuyos7pHFh7w8QLI0d+JtQV/ZckWjOhVMWfJVKsYTwq1xEZKoDg= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=beta; h=received:date:from:to:cc:subject:message-id:reply-to:references:mime-version:content-type:content-disposition:in-reply-to:user-agent; b=K5UV9g2t3fOd9CRv7lZKMfI6fpeo0iCRa4TbzdgMT+6fRXucnC76bSxamDVQrmPu0xwgs5+vXm9S5eAL0itm8YhU+jlFImzBFjML75zzicZJXA0qjrMr82uOuNSqaS7r8Qjfk+jerr6SlWL52iXZ1IsJIHrPm/LXzMVTiAh89FA= Received: by 10.115.90.1 with SMTP id s1mr3236427wal.1188263001696; Mon, 27 Aug 2007 18:03:21 -0700 (PDT) Received: from michelle.cdnetworks.co.kr ( [211.53.35.84]) by mx.google.com with ESMTPS id v37sm9883040wah.2007.08.27.18.03.16 (version=TLSv1/SSLv3 cipher=OTHER); Mon, 27 Aug 2007 18:03:20 -0700 (PDT) Received: from michelle.cdnetworks.co.kr (localhost.cdnetworks.co.kr [127.0.0.1]) by michelle.cdnetworks.co.kr (8.13.5/8.13.5) with ESMTP id l7S13AP6085766 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 28 Aug 2007 10:03:11 +0900 (KST) (envelope-from pyunyh@gmail.com) Received: (from yongari@localhost) by michelle.cdnetworks.co.kr (8.13.5/8.13.5/Submit) id l7S13AFG085765; Tue, 28 Aug 2007 10:03:10 +0900 (KST) (envelope-from pyunyh@gmail.com) Date: Tue, 28 Aug 2007 10:03:10 +0900 From: Pyun YongHyeon To: Bill Paul Message-ID: <20070828010310.GA85263@cdnetworks.co.kr> References: <20070827201809.0367616A418@hub.freebsd.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="9jxsPFA5p3P2qPhR" Content-Disposition: inline In-Reply-To: <20070827201809.0367616A418@hub.freebsd.org> User-Agent: Mutt/1.4.2.1i Cc: freebsd-net@FreeBSD.ORG, freebsd-current@FreeBSD.ORG Subject: Re: Bug in vr(4) driver X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: pyunyh@gmail.com List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 28 Aug 2007 01:03:22 -0000 --9jxsPFA5p3P2qPhR Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Mon, Aug 27, 2007 at 08:18:08PM +0000, Bill Paul wrote: > > I recently started writing a driver for the Via Rhine family of chips > for VxWorks (they turn up on various x86-based single board systems, > and I figured it'd be nice to actually support them out of the box), > and along the way, I noticed a subtle bug in the FreeBSD vr(4) driver. > > The vr_attach() routine unconditionally does this for all supported > chips: > > /* > * Windows may put the chip in suspend mode when it > * shuts down. Be sure to kick it in the head to wake it > * up again. > */ > VR_CLRBIT(sc, VR_STICKHW, (VR_STICKHW_DS0|VR_STICKHW_DS1)); > > The problem is, the VR_STICKHW register is not valid on all Rhine > devices. The VT86C100A chip, which is present on the D-Link DFE-530TX > boards, doesn't support power management, and its register space is > only 128 bytes wide. The VR_STICKHW register offset falls outside this > range. This may go unnoticed in most scenarios, but if you happen to have > another PCI device in your system which is assigned the register > space immediately after that of the Rhine, the vr(4) driver will > incorrectly stomp it. In my case, the BIOS on my test board decided > to put the register space for my PRO/100 ethernet board right next > to the Rhine, and the Rhine driver ended up clobbering the IMR register > of the PRO/100 device. (Long story short: the board kept locking up on > boot. Took me the better part of the morning suss out why.) > > The strictly correct thing to do would be to check the PCI config space > to make sure the device supports the power management capability and only > write to the VR_STICKHW register if it does. A less strictly correct > but equally effective thing to do would be: > > /* > * Windows may put the chips that support power management into > * suspend mode when it shuts down. Be sure to kick it in the > * head to wake it up again. > */ > if (pci_get_device(dev) != VIA_DEVICEID_RHINE) > VR_CLRBIT(sc, VR_STICKHW, (VR_STICKHW_DS0|VR_STICKHW_DS1)); > > This is basically the fix I put into my VxWorks driver. I suggest someone > update the FreeBSD driver as well. > Hi, I don't have vr(4) hardwares(if I had I would have converted vr(4) to use bus_dma(9)). Would you review/test the attached patch? -- Regards, Pyun YongHyeon --9jxsPFA5p3P2qPhR Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="vr.diff" Index: if_vr.c =================================================================== RCS file: /home/ncvs/src/sys/pci/if_vr.c,v retrieving revision 1.126 diff -u -r1.126 if_vr.c --- if_vr.c 23 Apr 2007 12:19:02 -0000 1.126 +++ if_vr.c 28 Aug 2007 01:00:34 -0000 @@ -90,6 +90,7 @@ #include +#include #include #define VR_USEIOSPACE @@ -513,6 +514,7 @@ struct ifnet *ifp; int error = 0, rid; struct vr_type *t; + int pmc; sc = device_get_softc(dev); sc->vr_dev = dev; @@ -591,7 +593,8 @@ * shuts down. Be sure to kick it in the head to wake it * up again. */ - VR_CLRBIT(sc, VR_STICKHW, (VR_STICKHW_DS0|VR_STICKHW_DS1)); + if (pci_find_extcap(dev, PCIY_PMG, &pmc) == 0) + VR_CLRBIT(sc, VR_STICKHW, (VR_STICKHW_DS0|VR_STICKHW_DS1)); /* Reset the adapter. */ vr_reset(sc); --9jxsPFA5p3P2qPhR--