> Date: Fri, 28 Sep 2012 09:31:34 +0200
> From: "Christiano F. Haesbaert" <[email protected]>
>
> On Fri, Sep 28, 2012 at 02:42:18AM -0400, Brad Smith wrote:
> > 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.c 3 Apr 2011 15:36:02 -0000 1.32
> > +++ if_gem_pci.c 28 Sep 2012 05:16:00 -0000
> > @@ -227,22 +227,19 @@ gem_attach_pci(struct device *parent, st
> >
> > sc->sc_pci = 1; /* XXXXX 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,
> >
>
> Ok by me, but when I said acknowledge I meant this, I'm ok with either,
> if kettenis doesn't mind :=).
Yes, I'd really like to see the complete PCI ID to variant mapping.
However...
> Index: if_gem_pci.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/if_gem_pci.c,v
> retrieving revision 1.32
> diff -d -u -p -r1.32 if_gem_pci.c
> --- if_gem_pci.c 3 Apr 2011 15:36:02 -0000 1.32
> +++ if_gem_pci.c 28 Sep 2012 07:26:23 -0000
> @@ -227,22 +227,27 @@ gem_attach_pci(struct device *parent, st
>
> sc->sc_pci = 1; /* XXXXX 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;
> + case PCI_PRODUCT_APPLE_INTREPID2_GMAC:
> + case PCI_PRODUCT_APPLE_PANGEA_GMAC:
> + case PCI_PRODUCT_APPLE_SHASTA_GMAC:
> + case PCI_PRODUCT_APPLE_UNINORTHGMAC:
> + case PCI_PRODUCT_APPLE_UNINORTH2GMAC:
> + sc->sc_variant = GEM_APPLE_GMAC;
> + break;
> + default:
> + printf(": unknown variant 0x%x\n", sc->sc_variant);
> + return;
as sthen@ points out, the printf in the default case will never get
hit, so it should probably be removed or turned into a KASSERT.
Also, I'd prefer if the K2_GMAC case would come last since that
variant came after the others.
