* [PATCH] rfkill: clarify usage of rfkill_force_state() and rfkill->get_state() [not found] <20080918145236.GJ1583@khazad-dum.debian.net> @ 2008-09-18 15:19 ` Henrique de Moraes Holschuh 2008-09-18 15:24 ` Ivo van Doorn 0 siblings, 1 reply; 8+ messages in thread From: Henrique de Moraes Holschuh @ 2008-09-18 15:19 UTC (permalink / raw) To: John Linville Cc: linux-wireless, linux-kernel, Henrique de Moraes Holschuh, Ivo van Doorn, Larry Finger rfkill_force_state() is used to update the rfkill core's idea of what is really happening RIGHT NOW to the transmitter hardware in a PUSH model. rfkill->get_state() does the same, in a PULL model. Neither of them change the real hardware radio state through a call to rfkill->toggle_radio() or anything of the sort, so they must deal with the real, current state of the hardware. Change some documentation to make that more clear (I hope). Signed-off-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br> Cc: Ivo van Doorn <IvDoorn@gmail.com> Cc: Larry Finger <Larry.Finger@lwfinger.net> --- Documentation/rfkill.txt | 10 ++++++++++ include/linux/rfkill.h | 3 +++ net/rfkill/rfkill.c | 3 +++ 3 files changed, 16 insertions(+), 0 deletions(-) diff --git a/Documentation/rfkill.txt b/Documentation/rfkill.txt index b65f079..0143f1e 100644 --- a/Documentation/rfkill.txt +++ b/Documentation/rfkill.txt @@ -461,6 +461,16 @@ mandatory when the device has a hardware rfkill line, or when something else like the firmware could cause its state to be changed without going through the rfkill class. +**** ATTENTION **** +Both the state returned by the rfkill->get_state() hook, and the state +passed to rfkill_force_state() MUST BE THE REAL, CURRENT STATE OF THE +HARDWARE TRANSMITTER. Never use for these functions the "desired" state, +"user requested" state, or anything of the sort. + +Also, be warned that rfkill_force_state() cannot be called from atomic context, +so interrupt handlers will have to make use of a workqueue to do it. +**** ATTENTION **** + Some hardware provides events when its status changes. In these cases, it is best for the driver to not provide a get_state hook, and instead register the rfkill class *already* with the correct status, and keep it updated using diff --git a/include/linux/rfkill.h b/include/linux/rfkill.h index 4cd64b0..92b9c72 100644 --- a/include/linux/rfkill.h +++ b/include/linux/rfkill.h @@ -80,6 +80,9 @@ enum rfkill_state { * may be called from atomic context, should return 0 on success. * Either this handler OR judicious use of rfkill_force_state() is * MANDATORY for any driver capable of RFKILL_STATE_HARD_BLOCKED. + * THIS HOOK MUST RETURN THE CURRENT, REAL STATE OF THE HARDWARE + * TRANSMITTER, and not the "desired state", "user requested state" + * or anything of the sort. * @led_trigger: A LED trigger for this button's LED. * @dev: Device structure integrating the switch into device tree. * @node: Used to place switch into list of all switches known to the diff --git a/net/rfkill/rfkill.c b/net/rfkill/rfkill.c index f949a48..64a53f1 100644 --- a/net/rfkill/rfkill.c +++ b/net/rfkill/rfkill.c @@ -333,6 +333,9 @@ EXPORT_SYMBOL_GPL(rfkill_restore_states); * a notification by the firmware/hardware of the current *real* * state of the radio rfkill switch. * + * The state passed to this function MUST MATCH THE CURRENT, + * REAL HARDWARE STATE OF THE TRANSMITTER. + * * Devices which are subject to external changes on their rfkill * state (such as those caused by a hardware rfkill line) MUST * have their driver arrange to call rfkill_force_state() as soon -- 1.5.6.5 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] rfkill: clarify usage of rfkill_force_state() and rfkill->get_state() 2008-09-18 15:19 ` [PATCH] rfkill: clarify usage of rfkill_force_state() and rfkill->get_state() Henrique de Moraes Holschuh @ 2008-09-18 15:24 ` Ivo van Doorn 2008-09-18 16:43 ` Henrique de Moraes Holschuh 0 siblings, 1 reply; 8+ messages in thread From: Ivo van Doorn @ 2008-09-18 15:24 UTC (permalink / raw) To: Henrique de Moraes Holschuh Cc: John Linville, linux-wireless, linux-kernel, Larry Finger On Thursday 18 September 2008, Henrique de Moraes Holschuh wrote: > rfkill_force_state() is used to update the rfkill core's idea of what is > really happening RIGHT NOW to the transmitter hardware in a PUSH model. > > rfkill->get_state() does the same, in a PULL model. > > Neither of them change the real hardware radio state through a call to > rfkill->toggle_radio() or anything of the sort, so they must deal with the > real, current state of the hardware. > > Change some documentation to make that more clear (I hope). > > Signed-off-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br> > Cc: Ivo van Doorn <IvDoorn@gmail.com> See my other mail I just send out. But I don't quite agree on this change of rfkill event interpretation. Ivo > Cc: Larry Finger <Larry.Finger@lwfinger.net> > --- > Documentation/rfkill.txt | 10 ++++++++++ > include/linux/rfkill.h | 3 +++ > net/rfkill/rfkill.c | 3 +++ > 3 files changed, 16 insertions(+), 0 deletions(-) > > diff --git a/Documentation/rfkill.txt b/Documentation/rfkill.txt > index b65f079..0143f1e 100644 > --- a/Documentation/rfkill.txt > +++ b/Documentation/rfkill.txt > @@ -461,6 +461,16 @@ mandatory when the device has a hardware rfkill line, or when something else > like the firmware could cause its state to be changed without going through the > rfkill class. > > +**** ATTENTION **** > +Both the state returned by the rfkill->get_state() hook, and the state > +passed to rfkill_force_state() MUST BE THE REAL, CURRENT STATE OF THE > +HARDWARE TRANSMITTER. Never use for these functions the "desired" state, > +"user requested" state, or anything of the sort. > + > +Also, be warned that rfkill_force_state() cannot be called from atomic context, > +so interrupt handlers will have to make use of a workqueue to do it. > +**** ATTENTION **** > + > Some hardware provides events when its status changes. In these cases, it is > best for the driver to not provide a get_state hook, and instead register the > rfkill class *already* with the correct status, and keep it updated using > diff --git a/include/linux/rfkill.h b/include/linux/rfkill.h > index 4cd64b0..92b9c72 100644 > --- a/include/linux/rfkill.h > +++ b/include/linux/rfkill.h > @@ -80,6 +80,9 @@ enum rfkill_state { > * may be called from atomic context, should return 0 on success. > * Either this handler OR judicious use of rfkill_force_state() is > * MANDATORY for any driver capable of RFKILL_STATE_HARD_BLOCKED. > + * THIS HOOK MUST RETURN THE CURRENT, REAL STATE OF THE HARDWARE > + * TRANSMITTER, and not the "desired state", "user requested state" > + * or anything of the sort. > * @led_trigger: A LED trigger for this button's LED. > * @dev: Device structure integrating the switch into device tree. > * @node: Used to place switch into list of all switches known to the > diff --git a/net/rfkill/rfkill.c b/net/rfkill/rfkill.c > index f949a48..64a53f1 100644 > --- a/net/rfkill/rfkill.c > +++ b/net/rfkill/rfkill.c > @@ -333,6 +333,9 @@ EXPORT_SYMBOL_GPL(rfkill_restore_states); > * a notification by the firmware/hardware of the current *real* > * state of the radio rfkill switch. > * > + * The state passed to this function MUST MATCH THE CURRENT, > + * REAL HARDWARE STATE OF THE TRANSMITTER. > + * > * Devices which are subject to external changes on their rfkill > * state (such as those caused by a hardware rfkill line) MUST > * have their driver arrange to call rfkill_force_state() as soon ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] rfkill: clarify usage of rfkill_force_state() and rfkill->get_state() 2008-09-18 15:24 ` Ivo van Doorn @ 2008-09-18 16:43 ` Henrique de Moraes Holschuh 2008-09-18 16:45 ` Johannes Berg 2008-09-18 17:40 ` Ivo van Doorn 0 siblings, 2 replies; 8+ messages in thread From: Henrique de Moraes Holschuh @ 2008-09-18 16:43 UTC (permalink / raw) To: Ivo van Doorn; +Cc: John Linville, linux-wireless, linux-kernel, Larry Finger On Thu, 18 Sep 2008, Ivo van Doorn wrote: > On Thursday 18 September 2008, Henrique de Moraes Holschuh wrote: > > rfkill_force_state() is used to update the rfkill core's idea of what is > > really happening RIGHT NOW to the transmitter hardware in a PUSH model. > > > > rfkill->get_state() does the same, in a PULL model. > > > > Neither of them change the real hardware radio state through a call to > > rfkill->toggle_radio() or anything of the sort, so they must deal with the > > real, current state of the hardware. > > > > Change some documentation to make that more clear (I hope). > > > > Signed-off-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br> > > Cc: Ivo van Doorn <IvDoorn@gmail.com> > > See my other mail I just send out. I did, and just replied to it. But only now do I think I realised what you meant: That even if the driver tries to keep set txpower off separate from rfkill, if it uses the hardware soft-rfkill bit to implement both, it cannot use that to feed information to rfkill_force_state() directly. Argh. > But I don't quite agree on this change of rfkill event interpretation. Well, there is no intended change in interpretation, but I don't know how to word it in a way that means "the real current hardware state as far as rfkill is concerned". Because suppose it is a driver doing txpower off AND software rfkill using the *same* hardware bit (a sigle software rfkill bit). Now it must do something like this in pseudo-code: 1. if (the bit is disabled (i.e. SW rfkill is NOT ACTIVE)) { rfkill-SW-status = disabled; } else if (the bit is enabled (i.e. SW rfkill is ACTIVE)) { if (tx power off is NOT ACTIVE) rfkill-SW-status = enabled; else rfkill-SW-status = whatever the user asked } THEN, it should use rfkill-sw-status, along with the hw rfkill line status, to synthesize the state it must pass to rfkill_force_status(). ICK. Of course, if the driver has another way to implement txpower off that does not clash with sw rfkill, the above is unneeded. How would you put that into words for the rfkill documentation? -- "One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] rfkill: clarify usage of rfkill_force_state() and rfkill->get_state() 2008-09-18 16:43 ` Henrique de Moraes Holschuh @ 2008-09-18 16:45 ` Johannes Berg 2008-09-18 17:32 ` Ivo van Doorn 2008-09-18 17:40 ` Ivo van Doorn 1 sibling, 1 reply; 8+ messages in thread From: Johannes Berg @ 2008-09-18 16:45 UTC (permalink / raw) To: Henrique de Moraes Holschuh Cc: Ivo van Doorn, John Linville, linux-wireless, linux-kernel, Larry Finger [-- Attachment #1: Type: text/plain, Size: 800 bytes --] On Thu, 2008-09-18 at 13:43 -0300, Henrique de Moraes Holschuh wrote: > Now it must do something like this in pseudo-code: > > 1. if (the bit is disabled (i.e. SW rfkill is NOT ACTIVE)) { > rfkill-SW-status = disabled; > } else if (the bit is enabled (i.e. SW rfkill is ACTIVE)) { > if (tx power off is NOT ACTIVE) > rfkill-SW-status = enabled; > else > rfkill-SW-status = whatever the user asked > } > > THEN, it should use rfkill-sw-status, along with the hw rfkill line status, > to synthesize the state it must pass to rfkill_force_status(). > > ICK. Of course, if the driver has another way to implement txpower off that > does not clash with sw rfkill, the above is unneeded. Why are we not handling soft-rfkill in mac80211 entirely? johannes [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] rfkill: clarify usage of rfkill_force_state() and rfkill->get_state() 2008-09-18 16:45 ` Johannes Berg @ 2008-09-18 17:32 ` Ivo van Doorn 2008-09-18 17:52 ` Johannes Berg 0 siblings, 1 reply; 8+ messages in thread From: Ivo van Doorn @ 2008-09-18 17:32 UTC (permalink / raw) To: Johannes Berg Cc: Henrique de Moraes Holschuh, John Linville, linux-wireless, linux-kernel, Larry Finger On Thursday 18 September 2008, Johannes Berg wrote: > On Thu, 2008-09-18 at 13:43 -0300, Henrique de Moraes Holschuh wrote: > > > Now it must do something like this in pseudo-code: > > > > 1. if (the bit is disabled (i.e. SW rfkill is NOT ACTIVE)) { > > rfkill-SW-status = disabled; > > } else if (the bit is enabled (i.e. SW rfkill is ACTIVE)) { > > if (tx power off is NOT ACTIVE) > > rfkill-SW-status = enabled; > > else > > rfkill-SW-status = whatever the user asked > > } > > > > THEN, it should use rfkill-sw-status, along with the hw rfkill line status, > > to synthesize the state it must pass to rfkill_force_status(). > > > > ICK. Of course, if the driver has another way to implement txpower off that > > does not clash with sw rfkill, the above is unneeded. > > Why are we not handling soft-rfkill in mac80211 entirely? Ideal situation would indeed be that mac80211 registers a rfkill structure and listens to rfkill events. This would help drivers by only needing to register a rfkill structure for state-change events without any need for listeners. I was considering such a patch some time ago, but needed to figure out how to work with the state-override capabilities (HW_BLOCK and SOFT_BLOCK) and didn't work on it any further since. Ivo ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] rfkill: clarify usage of rfkill_force_state() and rfkill->get_state() 2008-09-18 17:32 ` Ivo van Doorn @ 2008-09-18 17:52 ` Johannes Berg 2008-09-18 18:12 ` Ivo van Doorn 0 siblings, 1 reply; 8+ messages in thread From: Johannes Berg @ 2008-09-18 17:52 UTC (permalink / raw) To: Ivo van Doorn Cc: Henrique de Moraes Holschuh, John Linville, linux-wireless, linux-kernel, Larry Finger [-- Attachment #1: Type: text/plain, Size: 811 bytes --] On Thu, 2008-09-18 at 19:32 +0200, Ivo van Doorn wrote: > Ideal situation would indeed be that mac80211 registers a rfkill structure > and listens to rfkill events. This would help drivers by only needing to > register a rfkill structure for state-change events without any need for > listeners. Yup. > I was considering such a patch some time ago, but needed to figure out > how to work with the state-override capabilities (HW_BLOCK and SOFT_BLOCK) > and didn't work on it any further since. So make the struct part of the hw structure? Then drivers can just use that to force hard events. Or actually, no, don't do this, make a new mac80211 call: ieee80211_inform_hardblocked(BLOCK/OPEN) which makes sure we can also try to not associate in this case in the future... johannes [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] rfkill: clarify usage of rfkill_force_state() and rfkill->get_state() 2008-09-18 17:52 ` Johannes Berg @ 2008-09-18 18:12 ` Ivo van Doorn 0 siblings, 0 replies; 8+ messages in thread From: Ivo van Doorn @ 2008-09-18 18:12 UTC (permalink / raw) To: Johannes Berg Cc: Henrique de Moraes Holschuh, John Linville, linux-wireless, linux-kernel, Larry Finger On Thursday 18 September 2008, Johannes Berg wrote: > On Thu, 2008-09-18 at 19:32 +0200, Ivo van Doorn wrote: > > > Ideal situation would indeed be that mac80211 registers a rfkill structure > > and listens to rfkill events. This would help drivers by only needing to > > register a rfkill structure for state-change events without any need for > > listeners. > > Yup. > > > I was considering such a patch some time ago, but needed to figure out > > how to work with the state-override capabilities (HW_BLOCK and SOFT_BLOCK) > > and didn't work on it any further since. > > So make the struct part of the hw structure? Then drivers can just use > that to force hard events. Or actually, no, don't do this, make a new > mac80211 call: > > ieee80211_inform_hardblocked(BLOCK/OPEN) > > which makes sure we can also try to not associate in this case in the > future... Yeah, unfortunately that wasn't the probablematic part. ;) Anyway when I have some time available I'll see if I can sort it out and make it work. But that will not be for another week or 2. Ivo ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] rfkill: clarify usage of rfkill_force_state() and rfkill->get_state() 2008-09-18 16:43 ` Henrique de Moraes Holschuh 2008-09-18 16:45 ` Johannes Berg @ 2008-09-18 17:40 ` Ivo van Doorn 1 sibling, 0 replies; 8+ messages in thread From: Ivo van Doorn @ 2008-09-18 17:40 UTC (permalink / raw) To: Henrique de Moraes Holschuh Cc: John Linville, linux-wireless, linux-kernel, Larry Finger On Thursday 18 September 2008, Henrique de Moraes Holschuh wrote: > On Thu, 18 Sep 2008, Ivo van Doorn wrote: > > On Thursday 18 September 2008, Henrique de Moraes Holschuh wrote: > > > rfkill_force_state() is used to update the rfkill core's idea of what is > > > really happening RIGHT NOW to the transmitter hardware in a PUSH model. > > > > > > rfkill->get_state() does the same, in a PULL model. > > > > > > Neither of them change the real hardware radio state through a call to > > > rfkill->toggle_radio() or anything of the sort, so they must deal with the > > > real, current state of the hardware. > > > > > > Change some documentation to make that more clear (I hope). > > > > > > Signed-off-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br> > > > Cc: Ivo van Doorn <IvDoorn@gmail.com> > > > > See my other mail I just send out. > > I did, and just replied to it. > > But only now do I think I realised what you meant: That even if the driver > tries to keep set txpower off separate from rfkill, if it uses the hardware > soft-rfkill bit to implement both, it cannot use that to feed information to > rfkill_force_state() directly. > > Argh. Indeed, you need to differentiate between RFKILL and RADIO states. > > But I don't quite agree on this change of rfkill event interpretation. > > Well, there is no intended change in interpretation, but I don't know how to > word it in a way that means "the real current hardware state as far as > rfkill is concerned". Well my interpretation is that rfkill events and thus rfkill_force_state() calls should be done for RFKILL state changes only. And RADIO state should be ignored since they don't matter for rfkill. > Because suppose it is a driver doing txpower off AND software rfkill using > the *same* hardware bit (a sigle software rfkill bit). > > Now it must do something like this in pseudo-code: > > 1. if (the bit is disabled (i.e. SW rfkill is NOT ACTIVE)) { > rfkill-SW-status = disabled; > } else if (the bit is enabled (i.e. SW rfkill is ACTIVE)) { > if (tx power off is NOT ACTIVE) > rfkill-SW-status = enabled; > else > rfkill-SW-status = whatever the user asked > } > > THEN, it should use rfkill-sw-status, along with the hw rfkill line status, > to synthesize the state it must pass to rfkill_force_status(). > > ICK. Of course, if the driver has another way to implement txpower off that > does not clash with sw rfkill, the above is unneeded. > > How would you put that into words for the rfkill documentation? The driver is required to keep track of the userspace configuration settings, when rfkill sends BLOCK to driver it should disable the radio, when rfkill send UNBLOCK to the driver it should restore to the userspace configuration settings (which can either be an enabled or disabled radio). Ivo ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2008-09-18 18:12 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20080918145236.GJ1583@khazad-dum.debian.net>
2008-09-18 15:19 ` [PATCH] rfkill: clarify usage of rfkill_force_state() and rfkill->get_state() Henrique de Moraes Holschuh
2008-09-18 15:24 ` Ivo van Doorn
2008-09-18 16:43 ` Henrique de Moraes Holschuh
2008-09-18 16:45 ` Johannes Berg
2008-09-18 17:32 ` Ivo van Doorn
2008-09-18 17:52 ` Johannes Berg
2008-09-18 18:12 ` Ivo van Doorn
2008-09-18 17:40 ` Ivo van Doorn
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox