Date: Wed, 8 Mar 2017 17:12:39 -0800 From: Oleksandr Tymoshenko <gonzo@FreeBSD.org> To: Konstantin Belousov <kostikbel@gmail.com> 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: <20170309011239.GA77155@bluezbox.com> In-Reply-To: <20170309010955.GV30979@kib.kiev.ua> References: <201703090100.v2910Rjd050904@repo.freebsd.org> <20170309010955.GV30979@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
Konstantin Belousov (kostikbel@gmail.com) wrote: > 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. Thanks for the pointers, I'll clean up this code in next iteration. -- gonzo
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20170309011239.GA77155>