Date: Wed, 10 Oct 2007 15:51:25 -0400 From: John Baldwin <jhb@freebsd.org> To: freebsd-current@freebsd.org, pyunyh@gmail.com Cc: Bill Paul <wpaul@freebsd.org>, freebsd-net@freebsd.org Subject: Re: Bug in vr(4) driver Message-ID: <200710101551.26081.jhb@freebsd.org> In-Reply-To: <20070828010310.GA85263@cdnetworks.co.kr> References: <20070827201809.0367616A418@hub.freebsd.org> <20070828010310.GA85263@cdnetworks.co.kr>
next in thread | previous in thread | raw e-mail | index | archive | help
On Monday 27 August 2007 09:03:10 pm Pyun YongHyeon wrote: > 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? Pyun, I'd say to go ahead and commit the patch. -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200710101551.26081.jhb>