On Mon, 2011-01-17 at 08:04 +0200, ext Juuso Oikarinen wrote: > On Fri, 2011-01-14 at 15:16 +0200, ext Luciano Coelho wrote: > > Hi Juuso, > > > > On Fri, 2011-01-14 at 12:48 +0100, juuso.oikarinen@xxxxxxxxx wrote: > > > diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c > > > index 9076555..863e660 100644 > > > --- a/drivers/net/wireless/wl12xx/main.c > > > +++ b/drivers/net/wireless/wl12xx/main.c > > > @@ -917,12 +917,10 @@ out: > > > return ret; > > > } > > > > Again we have this kind of unlock-lock case. Wouldn't it be better to > > move the cancel_work calls outside this function and call them after > > __wl1271_plt_stop() has returned? Yeah, there will be duplicate code if > > we do this, but I think it's a bit safer, isn't it? > > > > Heh, I did consider this too. But I did come to the conclusion that in > this case it is safer to unlock/lock than to duplicate the code - > duplicating this type of code has its risks too, and in this case > nothing is actually performed after the last mutex-lock - except for > unlocking in again. > > I'll change this to benefit both PoVs: I'll create a separate function > to cancel the works, and just call that function from two places. That > way, the to-be cancelled work items are in a single place. > And then I realise we can't easily do that. In the typical case (user space stops plt mode) there is no work after the last mutex lock, but in this module-removal scenario there is. To solve that, we'd have to extend the knowledge about the plt mode onto the bus specific modules (sdio/spi) because they call the wl12xx unregister_hw function with the mutex already held. So in essence, I would like to keep the current patch :o -Juuso -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
