Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 12 May 2016 12:11:08 -0700
From:      John Baldwin <jhb@freebsd.org>
To:        Scott Long <scottl@freebsd.org>
Cc:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r299544 - head/sys/dev/an
Message-ID:  <1887870.BCG0T4KVbU@ralph.baldwin.cx>
In-Reply-To: <201605121747.u4CHlUWt094185@repo.freebsd.org>
References:  <201605121747.u4CHlUWt094185@repo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Thursday, May 12, 2016 05:47:30 PM Scott Long wrote:
> Author: scottl
> Date: Thu May 12 17:47:30 2016
> New Revision: 299544
> URL: https://svnweb.freebsd.org/changeset/base/299544
> 
> Log:
>   Move mutex initialization from PCI probe to PCI attach.  Drivers are not
>   allowed to create any persistent state in their probe routine because it's
>   not guaranteed that they'll win the election and be allowed to attach.

Except that if they return 0 (which most shouldn't) it is guaranteed.
an_probe() used to return 0, but it was changed to return BUS_PROBE_DEFAULT
without fixing this.  (Oops)

an_probe() could just be fixed to destroy the lock and the lock init could
then be moved into an_attach().

> Modified: head/sys/dev/an/if_an_pci.c
> ==============================================================================
> --- head/sys/dev/an/if_an_pci.c	Thu May 12 16:34:59 2016	(r299543)
> +++ head/sys/dev/an/if_an_pci.c	Thu May 12 17:47:30 2016	(r299544)
> @@ -119,16 +119,16 @@ static int
>  an_probe_pci(device_t dev)
>  {
>  	struct an_type		*t;
> -	struct an_softc *sc = device_get_softc(dev);
> +	uint16_t vid, did;
>  
> -	bzero(sc, sizeof(struct an_softc));

This wasn't necessary before.

>  	t = an_devs;
> +	vid = pci_get_vendor(dev);
> +	did = pci_get_device(dev);
>  
>  	while (t->an_name != NULL) {
> -		if (pci_get_vendor(dev) == t->an_vid &&
> -		    pci_get_device(dev) == t->an_did) {
> +		if (vid == t->an_vid &&
> +		    did == t->an_did) {
>  			device_set_desc(dev, t->an_name);
> -			an_pci_probe(dev);
>  			return(BUS_PROBE_DEFAULT);
>  		}
>  		t++;
> @@ -145,8 +145,16 @@ an_attach_pci(dev)
>  	int 			flags, error = 0;
>  
>  	sc = device_get_softc(dev);
> +	bzero(sc, sizeof(struct an_softc));

This isn't necessary now (softc's are pre-zeroed by new-bus).

>  	flags = device_get_flags(dev);
>  
> +	/*
> +	 * Setup the lock in PCI attachment since it skips the an_probe
> +	 * function.
> +	 */
> +	mtx_init(&sc->an_mtx, device_get_nameunit(dev), MTX_NETWORK_LOCK,
> +	    MTX_DEF);
> +
>  	if (pci_get_vendor(dev) == AIRONET_VENDORID &&
>  	    pci_get_device(dev) == AIRONET_DEVICEID_MPI350) {
>  		sc->mpi350 = 1;
> 

-- 
John Baldwin



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1887870.BCG0T4KVbU>