On Mon, 2009-04-06 at 12:12 +0200, Michael Buesch wrote: > On Monday 06 April 2009 05:54:29 Larry Finger wrote: > > Using the latest rfkill patches posted by Johannes, I am hitting the following > > > > if (WARN_ON(!wl->rfkill.registered)) > > return -EINVAL; > > I think we should simply remove the warning. > In the previous rfkill implementation I had a reason to _not_ warn here. > (It had one valid codepath where it was called. I don't remember the details...) > > Anyway, please feel free to do a patch that removes the warning (but not the check). Actually, I think the check is invalid, or rather, rfkill.registered needs to be set before registering the rfkill struct. See, right now, and I'm not entirely happy with this, during rfkill_register I call the set_block callback. The reason I'm not really happy with that is that I said calling driver -> rfkill -> driver is bad, but on the other hand due to locking constraints across the subsystems the driver cannot take the same locks around rfkill_register as in the callbacks. Additionally, the driver must be ready to service rfkill calls _before_ rfkill_register() is called, everything else results in a race condition. Compare with the shared interrupt handlers, for instance. The reason I call it during rfkill_register is that I need to sync the software state to the driver's block state, and that needs to be done somewhere. Doing that from a work struct would be possible, but wouldn't solve the locking constraints nor would it actually help since rfkill_register can potentially sleep, so the work could still be executed before rfkill_register returns -- the driver still needs to be able to handle rfkill callbacks before rfkill_register returns. Therefore, the warning is spurious, but because of a driver bug elsewhere -- I think the rfkill.registered = 1 assignment should be moved up to before rfkill_register() (and be = 0 again if that fails unexpectedly); I'll do that in my patch. johannes
Attachment:
signature.asc
Description: This is a digitally signed message part
