Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 9 Mar 2017 03:09:55 +0200
From:      Konstantin Belousov <kostikbel@gmail.com>
To:        Oleksandr Tymoshenko <gonzo@FreeBSD.org>
Cc:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r314933 - head/sys/dev/spibus
Message-ID:  <20170309010955.GV30979@kib.kiev.ua>
In-Reply-To: <201703090100.v2910Rjd050904@repo.freebsd.org>
References:  <201703090100.v2910Rjd050904@repo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Mar 09, 2017 at 01:00:27AM +0000, Oleksandr Tymoshenko wrote:
> Author: gonzo
> Date: Thu Mar  9 01:00:27 2017
> New Revision: 314933
> URL: https://svnweb.freebsd.org/changeset/base/314933
> 
> Log:
>   [spigen] make spigen device ready to be compiled as a module
>   
>   - Add flag to indicate that device is opened by userland
>   - Replace "always fail" detach method with proper detach implementation
>   
>   MFC after:	1 week
> 
> Modified:
>   head/sys/dev/spibus/spigen.c
> 
> Modified: head/sys/dev/spibus/spigen.c
> ==============================================================================
> --- head/sys/dev/spibus/spigen.c	Thu Mar  9 00:58:21 2017	(r314932)
> +++ head/sys/dev/spibus/spigen.c	Thu Mar  9 01:00:27 2017	(r314933)
> @@ -53,6 +53,9 @@ __FBSDID("$FreeBSD$");
>  
>  #include "spibus_if.h"
>  
> +#define	SPIGEN_OPEN		(1 << 0)
> +#define	SPIGEN_MMAP_BUSY	(1 << 1)
> +
>  struct spigen_softc {
>  	device_t sc_dev;
>  	struct cdev *sc_cdev;
> @@ -63,8 +66,8 @@ struct spigen_softc {
>  	vm_object_t sc_mmap_buffer;     /* command, then data */
>  	vm_offset_t sc_mmap_kvaddr;
>  	size_t sc_mmap_buffer_size;
> -	int sc_mmap_busy;
>  	int sc_debug;
> +	int sc_flags;
>  };
>  
>  #ifdef FDT
> @@ -191,10 +194,24 @@ spigen_attach(device_t dev)
>  }
>  
>  static int 
> -spigen_open(struct cdev *dev, int oflags, int devtype, struct thread *td)
> +spigen_open(struct cdev *cdev, int oflags, int devtype, struct thread *td)
>  {
> +	int error;
> +	device_t dev;
> +	struct spigen_softc *sc;
>  
> -	return (0);
> +	error = 0;
> +	dev = cdev->si_drv1;
> +	sc = device_get_softc(dev);
> +
> +	mtx_lock(&sc->sc_mtx);
> +	if (sc->sc_flags & SPIGEN_OPEN)
> +		error = EBUSY;
> +	else
> +		sc->sc_flags |= SPIGEN_OPEN;
> +	mtx_unlock(&sc->sc_mtx);
> +
> +	return (error);
>  }
>  
>  static int
> @@ -264,7 +281,7 @@ spigen_transfer_mmapped(struct cdev *cde
>  	int error = 0;
>  
>  	mtx_lock(&sc->sc_mtx);
> -	if (sc->sc_mmap_busy)
> +	if (sc->sc_flags & SPIGEN_MMAP_BUSY)
>  		error = EBUSY;
>  	else if (stm->stm_command_length > sc->sc_command_length_max ||
>  	    stm->stm_data_length > sc->sc_data_length_max)
> @@ -275,7 +292,7 @@ spigen_transfer_mmapped(struct cdev *cde
>  	    stm->stm_command_length + stm->stm_data_length)
>  		error = ENOMEM;
>  	if (error == 0)
> -		sc->sc_mmap_busy = 1;
> +		sc->sc_flags |= SPIGEN_MMAP_BUSY;
>  	mtx_unlock(&sc->sc_mtx);
>  	if (error)
>  		return (error);
> @@ -288,8 +305,8 @@ spigen_transfer_mmapped(struct cdev *cde
>  	error = SPIBUS_TRANSFER(device_get_parent(dev), dev, &transfer);
>  
>  	mtx_lock(&sc->sc_mtx);
> -	KASSERT(sc->sc_mmap_busy, ("mmap no longer marked busy"));
> -	sc->sc_mmap_busy = 0;
> +	KASSERT((sc->sc_flags & SPIGEN_MMAP_BUSY), ("mmap no longer marked busy"));
> +	sc->sc_flags &= ~(SPIGEN_MMAP_BUSY);
>  	mtx_unlock(&sc->sc_mtx);
>  	return (error);
>  }
> @@ -391,6 +408,7 @@ spigen_close(struct cdev *cdev, int ffla
>  		sc->sc_mmap_buffer = NULL;
>  		sc->sc_mmap_buffer_size = 0;
>  	}
> +	sc->sc_flags &= ~(SPIGEN_OPEN);
>  	mtx_unlock(&sc->sc_mtx);
>  	return (0);
>  }
> @@ -398,8 +416,23 @@ spigen_close(struct cdev *cdev, int ffla
>  static int
>  spigen_detach(device_t dev)
>  {
> +	struct spigen_softc *sc;
> +
> +	sc = device_get_softc(dev);
> +
> +	mtx_lock(&sc->sc_mtx);
> +	if (sc->sc_flags & SPIGEN_OPEN) {
> +		mtx_unlock(&sc->sc_mtx);
> +		return (EBUSY);
> +	}
> +	mtx_unlock(&sc->sc_mtx);
This does not really work, since another thread might have already
started executing open, but did not executed enough to set the flag. The
destroy_dev(9) KPI guarantees that there is no other threads calling
into devsw methods when the device is teared down. If you need to do an
additional cleanup, do it after the call to destroy_dev().

BTW, since si_drv1 + open are used, you might close the usual race with
the make_dev_s(9) KPI.

>  
> -	return (EIO);
> +	mtx_destroy(&sc->sc_mtx);
> +
> +        if (sc->sc_cdev)
> +                destroy_dev(sc->sc_cdev);
> +	
> +	return (0);
>  }
>  
>  static devclass_t spigen_devclass;



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