PERFORCE change 120788 for review

[Attilio Rao]

Rui Paulo wrote:
> http://perforce.freebsd.org/chv.cgi?CH=120788
> 
> Change 120788 by rpaulo at rpaulo_epsilon on 2007/06/02 17:55:58
> 
> 	Add locking.	
> 
> Affected files ...
> 
> .. //depot/projects/soc2007/rpaulo-macbook/dev/asmc/asmc.c#8 edit
> .. //depot/projects/soc2007/rpaulo-macbook/dev/asmc/asmcvar.h#4 edit
> 
> Differences ...
> 
> ==== //depot/projects/soc2007/rpaulo-macbook/dev/asmc/asmc.c#8 (text+ko) ====
> 
> @@ -23,7 +23,7 @@
>   * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
>   * POSSIBILITY OF SUCH DAMAGE.
>   *
> - * $P4: //depot/projects/soc2007/rpaulo-macbook/dev/asmc/asmc.c#7 $
> + * $P4: //depot/projects/soc2007/rpaulo-macbook/dev/asmc/asmc.c#8 $
>   *
>   */
>  
> @@ -46,7 +46,8 @@
>  #include <sys/conf.h>
>  #include <sys/kernel.h>
>  #include <sys/sysctl.h>
> -#include <sys/sbuf.h>
> +#include <sys/lock.h>
> +#include <sys/mutex.h>
>  
>  #include <isa/isavar.h>

You should mantain includes sorted by name (where possible).

> @@ -239,6 +240,8 @@
>  
>  	model = asmc_match(dev);
>  
> +	mtx_init(&sc->sc_mtx, "asmc_mtx", NULL, MTX_SPIN);
> +
>  	asmc_init(dev);

You don't need to write again 'mtx' in the description. Just add 
something descriptive for the mutexes (since you alredy know it is a 
spin mutexes).


> @@ -740,6 +756,9 @@
>  {
>  	uint8_t type;
>  	device_t dev = (device_t) arg;
> +	struct asmc_softc *sc = device_get_softc(dev);
> +
> +	mtx_lock_spin(&sc->sc_mtx);	
>  
>  	type = inb(ASMC_INTPORT);
>  
> @@ -757,6 +776,8 @@
>  		device_printf(dev, "%s unknown interrupt\n", __func__);
>  	}
>  
> +	mtx_unlock_spin(&sc->sc_mtx);

I'm not sure you really need to maintain the lock for so long. 
Probabilly, you just need for the duration of inb().

> ==== //depot/projects/soc2007/rpaulo-macbook/dev/asmc/asmcvar.h#4 (text+ko) ====
> 
> @@ -23,7 +23,7 @@
>   * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
>   * POSSIBILITY OF SUCH DAMAGE.
>   *
> - * $P4: //depot/projects/soc2007/rpaulo-macbook/dev/asmc/asmcvar.h#3 $
> + * $P4: //depot/projects/soc2007/rpaulo-macbook/dev/asmc/asmcvar.h#4 $
>   *
>   */
>  
> @@ -49,6 +49,8 @@
>  	int			sc_rid;
>  	struct resource		*sc_res;
>  	void			*sc_cookie;
> +
> +	struct mtx		sc_mtx;
>  };

The extra white-line is not needed and since mutexes are not so light, 
it would be better to put it on the top of the structure.

Attilio