Re: Threads related SIGSEGV in random.c (diff, v2)
On Thursday, September 27, 2012, Philip Guenther wrote: > On Thu, 27 Sep 2012, Alexey Suslikov wrote: > > Removing only local variables part reverts us to previous behavior (i.e. > > crashes). > > My guess is your program is calling srandom(), srandomdev(), initstate() > or setstate() as well. Your diff doesn't protect the alteration of state, > end_ptr, fptr, and rptr on those paths, so a call to initstate() while > another thread is in random() can walk fptr and/or rptr out of the state > array. Add the necessary locking in them and run your tests again. > > If not, well, crank up your debugging skills. What was the line of code > that actually triggered the crash? Where did the bogus pointer come from? > > Crash: Program received signal SIGSEGV, Segmentation fault. [Switching to thread 1006387] 0x0cb33345cf6e in random () at /usr/src/lib/libc/stdlib/random.c:387 387 *fptr += *rptr; Back trace: Thread 10 (thread 1003160): #0 0x0cb33344135a in _thread_sys___thrsleep () at :2 #1 0x0cb3315fac2a in pthread_cond_wait (condp=0xcb32a79c4b0, mutexp=Variable "mutexp" is not available. ) at /usr/src/lib/librthread/rthread_sync.c:500 #2 0x0cb129f836ba in gwlist_consume () from /usr/local/sbin/bearerbox #3 0x0cb129f121f1 in boxc_sender () from /usr/local/sbin/bearerbox #4 0x0cb129f828dd in new_thread () from /usr/local/sbin/bearerbox #5 0x0cb3315f911e in _rthread_start (v=Variable "v" is not available. ) at /usr/src/lib/librthread/rthread.c:122 #6 0x0cb333434f9b in __tfork_thread () at /usr/src/lib/libc/arch/amd64/sys/tfork_thread.S:75 Cannot access memory at address 0xcb32b27c000 0x0cb33345cf6e 387 *fptr += *rptr; > > > I'm starting to believe that static globals are not good. > > They are incredibly good at what they do. If you're trying to say that > they fundamentally can't be thread-safe, you'll need some extraordinary > evidence for such a claim. > > What good they do? Cheers, Alexey
Threads related SIGSEGV in random.c (diff, v2)
On Thursday, September 27, 2012, Alexey Suslikov wrote: > On Thursday, September 27, 2012, Philip Guenther wrote: > >> On Thu, 27 Sep 2012, Alexey Suslikov wrote: >> > Removing only local variables part reverts us to previous behavior (i.e. >> > crashes). >> >> My guess is your program is calling srandom(), srandomdev(), initstate() >> or setstate() as well. Your diff doesn't protect the alteration of state, >> end_ptr, fptr, and rptr on those paths, so a call to initstate() while >> another thread is in random() can walk fptr and/or rptr out of the state >> array. Add the necessary locking in them and run your tests again. >> >> If not, well, crank up your debugging skills. What was the line of code >> that actually triggered the crash? Where did the bogus pointer come from? >> >> > Crash: > > Program received signal SIGSEGV, Segmentation fault. > [Switching to thread 1006387] > 0x0cb33345cf6e in random () at /usr/src/lib/libc/stdlib/random.c:387 > 387 *fptr += *rptr; > > Back trace: > > Thread 10 (thread 1003160): > #0 0x0cb33344135a in _thread_sys___thrsleep () at :2 > #1 0x0cb3315fac2a in pthread_cond_wait (condp=0xcb32a79c4b0, > mutexp=Variable "mutexp" is not available. > ) at /usr/src/lib/librthread/rthread_sync.c:500 > #2 0x0cb129f836ba in gwlist_consume () from /usr/local/sbin/bearerbox > #3 0x0cb129f121f1 in boxc_sender () from /usr/local/sbin/bearerbox > #4 0x0cb129f828dd in new_thread () from /usr/local/sbin/bearerbox > #5 0x0cb3315f911e in _rthread_start (v=Variable "v" is not available. > ) at /usr/src/lib/librthread/rthread.c:122 > #6 0x0cb333434f9b in __tfork_thread () at > /usr/src/lib/libc/arch/amd64/sys/tfork_thread.S:75 > Cannot access memory at address 0xcb32b27c000 > 0x0cb33345cf6e 387 *fptr += *rptr; > > >> >> > I'm starting to believe that static globals are not good. >> >> They are incredibly good at what they do. If you're trying to say that >> they fundamentally can't be thread-safe, you'll need some extraordinary >> evidence for such a claim. >> >> > What good they do? > Philip, can you help us to write threaded test case (spawning a number of threads each calling random)?
¡Los mejores precios para tu proximo viaje!
[IMAGE][IMAGE][IMAGE] [IMAGE]Buzios Enero- FebreroU$S 1270.- [IMAGE]Imbassai Temporada bajaU$S 1740.- [IMAGE]Natalen Baja!U$S 1505.- [IMAGE]Porto Seguro en EneroU$S 1429.- [IMAGE] [IMAGE]Cancúnen Baja!U$S 2310.- [IMAGE]CubaHabana- VaraderoU$S 1655.- [IMAGE]Puerto PlataEneroU$S 2835.- [IMAGE]Isla Margarita Enero a MarzoU$S 1590.- [IMAGE] [IMAGE]Perú e Isla de Pascuaen NoviembreU$S 2305.- [IMAGE]Perú MágicoGrupal en OctubreU$S 1950.- [IMAGE]Punta del Este feriado de Octubre$ 1630.- [IMAGE]SALINAS-Ecuadorplaya U$S 1680.- [IMAGE] [IMAGE]Dubai, Vietnam, Camboya & ThailandiaU$S 7080.- [IMAGE]India y Nepal Grupal en MarzoU$S 5385.- [IMAGE]Marruecos- Grupalen Octubre U$S 3765.- [IMAGE]Tailandia playasen FebreroU$S 4799.- [IMAGE]Si no desea seguir recibiendo novedades de Nau Turismo haga click aquÃ.
smtpd alias expansion updated, please test
Hi, I have just commited a rather deep change to the alias expansion logic in smtpd. It fixes a bug reported by halex@ in virtual maps, where an alias expanding to two different virtual users would end up being sent only to the first one. Alias expansion is tricky. We want to make sure that the change doesn't introduce a regression. So please test and report problems you notice to gil...@openbsd.org and e...@openbsd.org. Thank you. Eric.
Re: Intel azalia(4) and MSI
> Date: Mon, 24 Sep 2012 03:49:41 -0400
> From: Brad Smith
>
> I've always wondered why this workaround was not removed once MSI
> support was added. This was added before we had MSI support to
> workaround some Intel azalia(4) being setup by the BIOS as far
> as I know to use MSI and thus interrupts on system that were
> affected by this did not work at all for azalia(4).
Because there still are conditions under which we would not be able to
use MSI.
Perhaps we should change the generic PCI code to explicitly disable
MSIs if they are turned on. But until we do that, this workaround
needs to stay in place.
> Index: azalia.c
> ===
> RCS file: /home/cvs/src/sys/dev/pci/azalia.c,v
> retrieving revision 1.200
> diff -u -p -r1.200 azalia.c
> --- azalia.c 10 May 2012 22:46:48 - 1.200
> +++ azalia.c 2 Aug 2012 10:41:02 -
> @@ -488,7 +488,6 @@ azalia_pci_attach(struct device *parent,
> azalia_t *sc;
> struct pci_attach_args *pa;
> pcireg_t v;
> - uint8_t reg;
> pci_intr_handle_t ih;
> const char *interrupt_str;
>
> @@ -511,13 +510,6 @@ azalia_pci_attach(struct device *parent,
> sc->subid = pci_conf_read(pa->pa_pc, pa->pa_tag, PCI_SUBSYS_ID_REG);
>
> azalia_configure_pci(sc);
> -
> - /* disable MSI, use INTx instead */
> - if (PCI_VENDOR(sc->pciid) == PCI_VENDOR_INTEL) {
> - reg = azalia_pci_read(sc->pc, sc->tag, ICH_PCI_MMC);
> - reg &= ~(ICH_PCI_MMC_ME);
> - azalia_pci_write(sc->pc, sc->tag, ICH_PCI_MMC, reg);
> - }
>
> /* interrupt */
> if (pci_intr_map_msi(pa, &ih) && pci_intr_map(pa, &ih)) {
>
> --
> This message has been scanned for viruses and
> dangerous content by MailScanner, and is
> believed to be clean.
For a man who's in search.
Hello cutie! )) I'm sure we will be good friends. I'm Angelina and no guess I like u I looked through ur photographs and I found you are really pretty. Can't wait to see "+1" in my inbox (;
Re: ahd(4): Support Adaptec 39320LPE adapter
> Date: Mon, 24 Sep 2012 04:09:16 -0400
> From: Brad Smith
>
> Add support for the Adaptec 39320LPE adapter.
>
> >From FreeBSD
Don't see a downside, so ok kettenis@
> Index: ahd_pci.c
> ===
> RCS file: /home/cvs/src/sys/dev/pci/ahd_pci.c,v
> retrieving revision 1.18
> diff -u -p -r1.18 ahd_pci.c
> --- ahd_pci.c 31 May 2009 04:47:59 - 1.18
> +++ ahd_pci.c 24 Sep 2012 08:00:01 -
> @@ -119,6 +119,7 @@ ahd_compose_id(u_int device, u_int vendo
> #define ID_AHA_39320D_B 0x801C900500419005ull
> #define ID_AHA_39320D_HP 0x8011900500AC0E11ull
> #define ID_AHA_39320D_B_HP 0x801C900500AC0E11ull
> +#define ID_AHA_39320LPE 0x8017900500459005ull
> #define ID_AIC7902_PCI_REV_A40x3
> #define ID_AIC7902_PCI_REV_B00x10
> #define SUBID_HP 0x0E11
> @@ -218,6 +219,11 @@ struct ahd_pci_identity ahd_pci_ident_ta
> },
> {
> ID_AHA_39320D_B_HP,
> + ID_ALL_MASK,
> + ahd_aic7902_setup
> + },
> + {
> + ID_AHA_39320LPE,
> ID_ALL_MASK,
> ahd_aic7902_setup
> },
>
> --
> This message has been scanned for viruses and
> dangerous content by MailScanner, and is
> believed to be clean.
Re: acpiec madness (HP laptop people pay attention to this one) - tested on HP Compaq 6510b, 8510p, 8510w, 8710p, 8710w platforms
> Date: Mon, 24 Sep 2012 22:06:24 +0400
> From: Denis Lapshin
>
> Two years ago Marco issued a patch
> http://marc.info/?l=openbsd-tech&m=128612230314484&w=4 in order to
> prevent incorrect reading acpitz on HP Compaq (or any HP laptops with
> acpiec madness) by adding some delays to have data prepared to
> read/write.
>
> It has been tested and works perfectly fine on some HP laptops: 6510b,
> 8510p/w, 8710p/w.
>
> Please add this patch into the source tree to have it by default.
This is just papering over the problem. Marco didn't really
understand what he was doing. Seriously, making a local variable
volatile?
> Index: acpiec.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/acpiec.c,v
> retrieving revision 1.43
> diff -u -p -r1.43 acpiec.c
> --- acpiec.c 8 Aug 2010 17:25:41 - 1.43
> +++ acpiec.c 29 Sep 2010 04:24:13 -
> @@ -92,7 +92,7 @@ void
> acpiec_wait(struct acpiec_softc *sc, u_int8_t mask, u_int8_t val)
> {
> static int acpiecnowait;
> - u_int8_tstat;
> + volatile u_int8_t stat;
>
> dnprintf(40, "%s: EC wait_ns for: %b == %02x\n",
> DEVNAME(sc), (int)mask,
> @@ -104,8 +104,14 @@ acpiec_wait(struct acpiec_softc *sc, u_i
> if (cold || (stat & EC_STAT_BURST))
> delay(1);
> else
> - tsleep(&acpiecnowait, PWAIT, "acpiec", 1);
> + tsleep(&acpiecnowait, PWAIT, "ecstat", 1);
> }
> +
> + /* delay to make sure the data is actually ready */
> + if (cold)
> + delay(10);
> + else
> + tsleep(&acpiecnowait, PWAIT, "ecout", 1);
>
> dnprintf(40, "%s: EC wait_ns, stat: %b\n", DEVNAME(sc), (int)stat,
> "\20\x8IGN\x7SMI\x6SCI\05BURST\04CMD\03IGN\02IBF\01OBF");
Re: Intel azalia(4) and MSI
On Fri, Sep 28, 2012 at 04:06:32AM +0200, Mark Kettenis wrote:
> > Date: Mon, 24 Sep 2012 03:49:41 -0400
> > From: Brad Smith
> >
> > I've always wondered why this workaround was not removed once MSI
> > support was added. This was added before we had MSI support to
> > workaround some Intel azalia(4) being setup by the BIOS as far
> > as I know to use MSI and thus interrupts on system that were
> > affected by this did not work at all for azalia(4).
>
> Because there still are conditions under which we would not be able to
> use MSI.
I had 6 people reply off list indicating their Intel azalia controllers
didn't seem to have any regressions with the previous diff applied.
Can you expand upon this and explain what you mean. What are these
conditions? It would be nice to mention what those conditions are
within the comment.
> Perhaps we should change the generic PCI code to explicitly disable
> MSIs if they are turned on. But until we do that, this workaround
> needs to stay in place.
Since MSI is being disabled for the Intel azalia controller and the
datasheet does confirm that is what this code is doing shouldn't
we be telling the PCI layer MSI is disabled too like so? It doesn't
make sense to have MSI disabled for the controller and then the
PCI layer still thinking MSI should be used.
Index: azalia.c
===
RCS file: /home/cvs/src/sys/dev/pci/azalia.c,v
retrieving revision 1.201
diff -u -p -r1.201 azalia.c
--- azalia.c12 Aug 2012 19:32:22 - 1.201
+++ azalia.c28 Sep 2012 02:17:31 -
@@ -515,6 +515,7 @@ azalia_pci_attach(struct device *parent,
/* disable MSI, use INTx instead */
if (PCI_VENDOR(sc->pciid) == PCI_VENDOR_INTEL) {
+ pa->pa_flags &= ~PCI_FLAGS_MSI_ENABLED;
reg = azalia_pci_read(sc->pc, sc->tag, ICH_PCI_MMC);
reg &= ~(ICH_PCI_MMC_ME);
azalia_pci_write(sc->pc, sc->tag, ICH_PCI_MMC, reg);
--
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.
Re: gem(4): simplify gem_attach_pci() variant detection code a bit
> Date: Wed, 26 Sep 2012 15:32:37 -0400 > From: Brad Smith > > Simplify the gem(4) variant detection code a bit. I don't see the benefit and prefer to keep the code as it is now. > Index: if_gem_pci.c > === > RCS file: /home/cvs/src/sys/dev/pci/if_gem_pci.c,v > retrieving revision 1.31 > diff -u -p -r1.31 if_gem_pci.c > --- if_gem_pci.c 15 Oct 2009 17:54:56 - 1.31 > +++ if_gem_pci.c 6 Mar 2011 06:50:12 - > @@ -231,18 +231,10 @@ gem_attach_pci(struct device *parent, st > sc->sc_variant = GEM_SUN_GEM; > else if (PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_SUN_ERINETWORK) > sc->sc_variant = GEM_SUN_ERI; > - else if (PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_APPLE_INTREPID2_GMAC) > - sc->sc_variant = GEM_APPLE_GMAC; > - else if (PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_APPLE_PANGEA_GMAC) > - sc->sc_variant = GEM_APPLE_GMAC; > - else if (PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_APPLE_SHASTA_GMAC) > - sc->sc_variant = GEM_APPLE_GMAC; > - else if (PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_APPLE_UNINORTHGMAC) > - sc->sc_variant = GEM_APPLE_GMAC; > - else if (PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_APPLE_UNINORTH2GMAC) > - sc->sc_variant = GEM_APPLE_GMAC; > else if (PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_APPLE_K2_GMAC) > sc->sc_variant = GEM_APPLE_K2_GMAC; > + else > + sc->sc_variant = GEM_APPLE_GMAC; > > #define PCI_GEM_BASEADDR 0x10 > if (pci_mapreg_map(pa, PCI_GEM_BASEADDR, type, 0, > > -- > This message has been scanned for viruses and > dangerous content by MailScanner, and is > believed to be clean.
Re: hardware VLAN tagging for vr(4)
> Date: Thu, 27 Sep 2012 09:54:35 +1000
> From: Darren Tucker
>
> Hi all.
>
> This diff adds hardware 802.1q VLAN tagging support to vr(4) (just
> tag/untag, it doesn't do anything with the VLAN CAM filters).
>
> As far as I know, the capability is only in the VT6105M Rhine III chip
> which is used, amongst other places, in the pcengines ALIX machines.
>
> If anyone has a vr(4) (any revision), especially if you use VLANs,
> I'd be interested to hear if this works for you.
>
> I'm not a kernel guy, so it's quite possible I made some basic mistake
> in there somewhere, but it works for me on an ALIX:
>
> vr0 at pci0 dev 9 function 0 "VIA VT6105M RhineIII" rev 0x96: irq 10, address
> 00:0d:b9:7e:21:5c
> ukphy0 at vr0 phy 1: Generic IEEE 802.3u media interface, rev. 3: OUI
> 0x004063, model 0x0034
>
> $ ifconfig vr0 hwfeatures | grep hwfeatures
>
> hwfeatures=8037
>
> As an aside, I found what I believe is a latent bug in the existing
> driver. When it's done with the TX descriptor, it sets the "owner"
> bit to tell the chip that it's good to go:
>
> #define VR_TXSTAT_OWN 0x8000
> #define VR_TXOWN(x) x->vr_ptr->vr_status
>
> VR_TXOWN(cur_tx) = htole32(VR_TXSTAT_OWN);
>
> which expands to:
>
> cur_tx->vr_ptr->vr_status = htole32(0x8000);
>
> which zeroes the 31 least significant bits in the first word of the
> TX descriptor. Sadly, this includes the VLAN ID, which caused me some
> head-scratching trying to figure out why my tagged frames were all in
> VLAN 0.
>
> On the plus side, nothing else currently uses those bits, so right
> now it doesn't matter. My questions is: why hide that behind a macro?
> or at least, if you're going to, why not go the whole hog and put the
> entire thing in there?
>
> #define VR_TXOWN(x) ((x)->vr_ptr->vr_status |= htole32(VR_TXSTAT_OWN))
Yes. I consider using macros in the LHS of an assignment bad style.
But I'd just drop the macro entirely. I think having the code show
explicitly that vr_status is getting changed is better, and would
probably have prevented the latent bug you mention above.
Diff looks good to me, but I don't have the hardware to test this.
ok kettenis@
> Index: sys/dev/pci/if_vr.c
> ===
> RCS file: /cvs/src/sys/dev/pci/if_vr.c,v
> retrieving revision 1.114
> diff -u -p -r1.114 if_vr.c
> --- sys/dev/pci/if_vr.c 30 Jan 2012 09:11:30 - 1.114
> +++ sys/dev/pci/if_vr.c 26 Sep 2012 23:13:45 -
> @@ -62,6 +62,7 @@
> */
>
> #include "bpfilter.h"
> +#include "vlan.h"
>
> #include
> #include
> @@ -83,6 +84,11 @@
> #include
> #include
>
> +#if NVLAN > 0
> +#include
> +#include
> +#endif
> +
> #if NBPFILTER > 0
> #include
> #endif
> @@ -633,6 +639,12 @@ vr_attach(struct device *parent, struct
> if (sc->vr_quirks & VR_Q_CSUM)
> ifp->if_capabilities |= IFCAP_CSUM_IPv4|IFCAP_CSUM_TCPv4|
> IFCAP_CSUM_UDPv4;
> +
> +#if NVLAN > 0
> + /* if the hardware can do VLAN tagging, say so. */
> + if (sc->vr_quirks & VR_Q_HWTAG)
> + ifp->if_capabilities |= IFCAP_VLAN_HWTAGGING;
> +#endif
> #ifndef SMALL_KERNEL
> if (sc->vr_revid >= REV_ID_VT3065_A) {
> ifp->if_capabilities |= IFCAP_WOL;
> @@ -880,6 +892,23 @@ vr_rxeof(struct vr_softc *sc)
> cur_rx->vr_map->dm_mapsize, BUS_DMASYNC_POSTREAD);
> bus_dmamap_unload(sc->sc_dmat, cur_rx->vr_map);
>
> +#if NVLAN > 0
> + /*
> + * If there's a tagged packet, the 802.1q header will be at the
> + * 4-byte boundary following the CRC. There will be 2 bytes
> + * TPID (0x8100) and 2 bytes TCI (including VLAN ID).
> + * This isn't in the data sheet.
> + */
> + if (rxctl & VR_RXCTL_TAG) {
> + u_int16_t vtag;
> + int offset = ((total_len + 3) & ~3) + 2;
> +
> + bcopy(m->m_data + offset, &vtag, sizeof(vtag));
> + m->m_pkthdr.ether_vtag = ntohs(vtag);
> + m->m_flags |= M_VLANTAG;
> + }
> +#endif
> +
> /*
>* The VIA Rhine chip includes the CRC with every
>* received frame, and there's no way to turn this
> @@ -1002,7 +1031,7 @@ vr_txeof(struct vr_softc *sc)
> sc->vr_flags |= VR_F_RESTART;
> break;
> }
> - VR_TXOWN(cur_tx) = htole32(VR_TXSTAT_OWN);
> + VR_TXOWN(cur_tx) |= htole32(VR_TXSTAT_OWN);
> CSR_WRITE_4(sc, VR_TXADDR, cur_tx->vr_paddr);
> break;
> }
> @@ -1170,6 +1199,16 @@ vr_encap(struct vr_softc *sc, struct vr_
> struct mbuf *m_new = NULL;
> u_int32_t vr_flags = 0, vr_status = 0;
>
> +#if NVLAN
Re: gem(4): simplify gem_attach_pci() variant detection code a bit
On Wed, Sep 26, 2012 at 15:32, Brad Smith wrote: > Simplify the gem(4) variant detection code a bit. > > OK? Wouldn't a switch be a nicer simplification? > > > Index: if_gem_pci.c > === > RCS file: /home/cvs/src/sys/dev/pci/if_gem_pci.c,v > retrieving revision 1.31 > diff -u -p -r1.31 if_gem_pci.c > --- if_gem_pci.c 15 Oct 2009 17:54:56 - 1.31 > +++ if_gem_pci.c 6 Mar 2011 06:50:12 - > @@ -231,18 +231,10 @@ gem_attach_pci(struct device *parent, st > sc->sc_variant = GEM_SUN_GEM; > else if (PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_SUN_ERINETWORK) > sc->sc_variant = GEM_SUN_ERI; > - else if (PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_APPLE_INTREPID2_GMAC) > - sc->sc_variant = GEM_APPLE_GMAC; > - else if (PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_APPLE_PANGEA_GMAC) > - sc->sc_variant = GEM_APPLE_GMAC; > - else if (PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_APPLE_SHASTA_GMAC) > - sc->sc_variant = GEM_APPLE_GMAC; > - else if (PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_APPLE_UNINORTHGMAC) > - sc->sc_variant = GEM_APPLE_GMAC; > - else if (PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_APPLE_UNINORTH2GMAC) > - sc->sc_variant = GEM_APPLE_GMAC; > else if (PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_APPLE_K2_GMAC) > sc->sc_variant = GEM_APPLE_K2_GMAC; > + else > + sc->sc_variant = GEM_APPLE_GMAC; > > #define PCI_GEM_BASEADDR 0x10 > if (pci_mapreg_map(pa, PCI_GEM_BASEADDR, type, 0,
Re: gem(4): simplify gem_attach_pci() variant detection code a bit
I prefer a switch too, it shows you acknowledged those variants, anyhow i think the current idiom is bad and should be changed.
Re: gem(4): simplify gem_attach_pci() variant detection code a bit
On Wed, Sep 26, 2012 at 03:32:37PM -0400, Brad Smith wrote:
> Simplify the gem(4) variant detection code a bit.
>
> OK?
How about this..
Index: if_gem_pci.c
===
RCS file: /home/cvs/src/sys/dev/pci/if_gem_pci.c,v
retrieving revision 1.32
diff -u -p -r1.32 if_gem_pci.c
--- if_gem_pci.c3 Apr 2011 15:36:02 - 1.32
+++ if_gem_pci.c28 Sep 2012 05:16:00 -
@@ -227,22 +227,19 @@ gem_attach_pci(struct device *parent, st
sc->sc_pci = 1; /* X should all be done in bus_dma. */
- if (PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_SUN_GEMNETWORK)
+ switch (PCI_PRODUCT(pa->pa_id)) {
+ case PCI_PRODUCT_SUN_GEMNETWORK:
sc->sc_variant = GEM_SUN_GEM;
- else if (PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_SUN_ERINETWORK)
+ break;
+ case PCI_PRODUCT_SUN_ERINETWORK:
sc->sc_variant = GEM_SUN_ERI;
- else if (PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_APPLE_INTREPID2_GMAC)
- sc->sc_variant = GEM_APPLE_GMAC;
- else if (PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_APPLE_PANGEA_GMAC)
- sc->sc_variant = GEM_APPLE_GMAC;
- else if (PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_APPLE_SHASTA_GMAC)
- sc->sc_variant = GEM_APPLE_GMAC;
- else if (PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_APPLE_UNINORTHGMAC)
- sc->sc_variant = GEM_APPLE_GMAC;
- else if (PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_APPLE_UNINORTH2GMAC)
- sc->sc_variant = GEM_APPLE_GMAC;
- else if (PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_APPLE_K2_GMAC)
+ break;
+ case PCI_PRODUCT_APPLE_K2_GMAC:
sc->sc_variant = GEM_APPLE_K2_GMAC;
+ break;
+ default:
+ sc->sc_variant = GEM_APPLE_GMAC;
+ }
#define PCI_GEM_BASEADDR 0x10
if (pci_mapreg_map(pa, PCI_GEM_BASEADDR, type, 0,
--
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.
