[RFC AR6000 patch V3] Set hardware unavailable during wext_ioctl when the suspend or rfkill is send

Werner Almesberger werner at openmoko.org
Tue Mar 31 15:11:37 CEST 2009


Michael Trimarchi wrote:
> I hope that this one is little bit better :)

*Much* nicer ! :-) However, ...

> +    init_rwsem(&ar->arHwAvail);
> +    up_write(&ar->arHwAvail);

The rwsem is already unlocked after init_rwsem. So I'm not sure what
the up_write does. Probably nothing good.

Did you test if it works ?

> +#define DECLARE_LOCKED_VERSION(_name) \

Nice use of macros ! In your original patch, I hadn't even noticed
just how many functions you had to change :)

One small nit: mixed-case names aren't common in the kernel. How
about lock_ar6000 or such ?

> +    ret = _name(dev, info, p, data); \
> +    ret = ar6000_hw_avail_unlock(ar); \
> +    return ret; \

Ignoring return values ? Tsk, tsk :-) Since it cannot fail, you
probably don't want ar6000_hw_avail_unlock to return a status at all.

In fact, you could trim this down to simply

	AR_SOFTC_T *ar = (AR_SOFTC_T *) netdev_priv(dev);	\
	int ret = -EAGAIN;					\
								\
	if (down_read_trylock(&ar->arHwAvail))			\
		ret = _name(dev, info, p, data);		\
	up_read(&ar->arHwAvail);				\
	return ret;						\
}

This compiles into even less code than your version (after fixing
it) and you can see all that's happening at one glance.

Thanks,
- Werner



More information about the openmoko-kernel mailing list