Date: Tue, 11 Jul 2006 14:56:32 -0600 (MDT) From: Warner Losh <imp@bsdimp.com> To: jhb@FreeBSD.org Cc: freebsd-hackers@FreeBSD.org, mag@intron.ac, matthias.andree@gmx.de, julian@elischer.org, des@des.no, delphij@delphij.net Subject: Re: kern/99979: Get Ready for Kernel Module in C++ Message-ID: <20060711.145632.41678446.imp@bsdimp.com> In-Reply-To: <200607111548.40898.jhb@freebsd.org> References: <200607111413.37238.jhb@freebsd.org> <20060711.124811.1878034486.imp@bsdimp.com> <200607111548.40898.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
> It's less ugly than it used to be, esp. with the bus_read_X() stuff. There's > no separate bus_alloc_resource/bus_setup_intr for interrupts though for > example, just bus_setup_intr() equivalent. This is pretty simple though: > > /* OS X */ > IOMemoryMap *myBarMap; > void *myBar; > u_int32_t reg; > > /* BAR 0 */ > myBarMap = provider->mapDeviceMemoryWithRegister(kIOPCIConfigBaseAddress0); > myBar = myBarMap->getVirtualAddress(); > > reg = OSReadLittleInt32(myBar, FOO_REG); > reg |= BAR_FLAG; > OSWriteLittleIntew(myBar, FOO_REG, reg); > > myBar->release(); > > In FreeBSD-7 this is something like: > > struct resource *res; > int rid; > u_int32_t reg; uint32_t is the standards compliant spelling :-) > rid = PCIR_BAR(0); > res = bus_alloc_resource_any(dev, SYS_RES_MEMORY, &rid, RF_ACTIVE); The above should be shortenable to: res = bus_alloc_resource_any(dev, SYS_RES_MEMORY, PCIR_BAR(0), RF_ACTIVE); because we don't need to have rid be a pointer. > /* XXX: Not sure about endian.. stream_read maybe? */ no. bus_read is correct, because the pci bus is little endian by definition. bus_stream_read will read/write from native endian to bus endian. > reg = bus_read_4(res, FOO_REG); > reg |= BAR_FLAG; > bus_write_4(res, FOO_REG, reg); > > bus_release_resource(dev, SYS_RES_MEMORY, rid, res); We should be able to shorten that to: bus_release_resource(dev, res); these days. > Which is very similar though there is some clutter (bus_release_resource() > should take fewer args, like just dev and res, and it would be nice to say > something like map_bar(dev, PCIR_BAR(0)) and get back a resource *). Agreed. If you have mutliple resources, you can stack them up as well. > Usually drivers come up with macros to hide the bus handles and tags which is > an indication that they were quite unsightly (thankfully that's been fixed in > 7). :) I usually do: #define RD4(sc, r) bus_read_4(sc->res, (r)) #define WR4(sc, r, v) bus_write_4(sc->res, (r), (v)) which makes usage even shorter. It also helps me hid cross-version differences... > Anyways, if you took FreeBSD-7 and cut down some of the gunk similar to OS X > you'd have something like: > > struct resource *res; > u_int32_t reg; > > res = pci_alloc_bar(dev, PCIR_BAR(0)); > > /* XXX: endian question again */ > reg = bus_read_4(res, FOO_REG); > reg |= BAR_FLAG; > bus_write_4(res, FOO_REG, reg); > > bus_release_resource(dev, res); > > Which is at least somewhat better I think. In the case where you don't care what kind of mapping you get, that would work great. It wouldn't be too hard to implement a pci_alloc_bar that does the default allocation and mapping. Changing some of the other APIs would be more difficult... > > Also, in FreeBSD, the resources are already allocated by the bus > > code. It just changes ownership to the child when the request comes > > in... > > Yes, this has been a recent improvement. Yes. We also do it in a lazy way so that if the resource is allocated or not is transparent to the driver and the system. If it could be decoded, we reserve it. Warner
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20060711.145632.41678446.imp>