* [RFC][PATCH] b43: rfkill: use HI enabled bit for all devices
@ 2010-11-17 19:56 Rafał Miłecki
2010-11-17 20:17 ` Michael Büsch
0 siblings, 1 reply; 5+ messages in thread
From: Rafał Miłecki @ 2010-11-17 19:56 UTC (permalink / raw)
To: linux-wireless, John W. Linville; +Cc: b43-dev, Rafał Miłecki
Devices which use LO enabled bit are covered by b43legacy
Signed-off-by: Rafa? Mi?ecki <zajec5@gmail.com>
---
Is this alright to use inline for this? Is my WARN_ON OK?
---
drivers/net/wireless/b43/main.c | 2 ++
drivers/net/wireless/b43/rfkill.c | 21 +++------------------
drivers/net/wireless/b43/rfkill.h | 4 ++--
3 files changed, 7 insertions(+), 20 deletions(-)
diff --git a/drivers/net/wireless/b43/main.c b/drivers/net/wireless/b43/main.c
index fa48803..9b71bb1 100644
--- a/drivers/net/wireless/b43/main.c
+++ b/drivers/net/wireless/b43/main.c
@@ -4527,6 +4527,8 @@ static int b43_op_start(struct ieee80211_hw *hw)
}
}
+ /* we don't expect older devices which need other RFKILL check */
+ B43_WARN_ON(dev->dev->id.revision < 3);
/* XXX: only do if device doesn't support rfkill irq */
wiphy_rfkill_start_polling(hw->wiphy);
diff --git a/drivers/net/wireless/b43/rfkill.c b/drivers/net/wireless/b43/rfkill.c
index 78016ae..012ed2f 100644
--- a/drivers/net/wireless/b43/rfkill.c
+++ b/drivers/net/wireless/b43/rfkill.c
@@ -26,25 +26,10 @@
/* Returns TRUE, if the radio is enabled in hardware. */
-bool b43_is_hw_radio_enabled(struct b43_wldev *dev)
+inline bool b43_is_hw_radio_enabled(struct b43_wldev *dev)
{
- if (dev->phy.rev >= 3 || dev->phy.type == B43_PHYTYPE_LP) {
- if (!(b43_read32(dev, B43_MMIO_RADIO_HWENABLED_HI)
- & B43_MMIO_RADIO_HWENABLED_HI_MASK))
- return 1;
- } else {
- /* To prevent CPU fault on PPC, do not read a register
- * unless the interface is started; however, on resume
- * for hibernation, this routine is entered early. When
- * that happens, unconditionally return TRUE.
- */
- if (b43_status(dev) < B43_STAT_STARTED)
- return 1;
- if (b43_read16(dev, B43_MMIO_RADIO_HWENABLED_LO)
- & B43_MMIO_RADIO_HWENABLED_LO_MASK)
- return 1;
- }
- return 0;
+ return !(b43_read32(dev, B43_MMIO_RADIO_HWENABLED_HI)
+ & B43_MMIO_RADIO_HWENABLED_HI_MASK);
}
/* The poll callback for the hardware button. */
diff --git a/drivers/net/wireless/b43/rfkill.h b/drivers/net/wireless/b43/rfkill.h
index f046c3c..7aa8a5a 100644
--- a/drivers/net/wireless/b43/rfkill.h
+++ b/drivers/net/wireless/b43/rfkill.h
@@ -4,8 +4,8 @@
struct ieee80211_hw;
struct b43_wldev;
-void b43_rfkill_poll(struct ieee80211_hw *hw);
+inline bool b43_is_hw_radio_enabled(struct b43_wldev *dev);
-bool b43_is_hw_radio_enabled(struct b43_wldev *dev);
+void b43_rfkill_poll(struct ieee80211_hw *hw);
#endif /* B43_RFKILL_H_ */
--
1.6.0.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* [RFC][PATCH] b43: rfkill: use HI enabled bit for all devices
2010-11-17 19:56 [RFC][PATCH] b43: rfkill: use HI enabled bit for all devices Rafał Miłecki
@ 2010-11-17 20:17 ` Michael Büsch
2010-11-17 20:23 ` Rafał Miłecki
0 siblings, 1 reply; 5+ messages in thread
From: Michael Büsch @ 2010-11-17 20:17 UTC (permalink / raw)
To: Rafał Miłecki; +Cc: linux-wireless, John W. Linville, b43-dev
On Wed, 2010-11-17 at 20:56 +0100, Rafa? Mi?ecki wrote:
> Devices which use LO enabled bit are covered by b43legacy
>
> Signed-off-by: Rafa? Mi?ecki <zajec5@gmail.com>
> ---
> Is this alright to use inline for this? Is my WARN_ON OK?
> ---
> drivers/net/wireless/b43/main.c | 2 ++
> drivers/net/wireless/b43/rfkill.c | 21 +++------------------
> drivers/net/wireless/b43/rfkill.h | 4 ++--
> 3 files changed, 7 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/net/wireless/b43/main.c b/drivers/net/wireless/b43/main.c
> index fa48803..9b71bb1 100644
> --- a/drivers/net/wireless/b43/main.c
> +++ b/drivers/net/wireless/b43/main.c
> @@ -4527,6 +4527,8 @@ static int b43_op_start(struct ieee80211_hw *hw)
> }
> }
>
> + /* we don't expect older devices which need other RFKILL check */
> + B43_WARN_ON(dev->dev->id.revision < 3);
Do we really need that check? The driver is full of assumptions that
the core revision is >=5.
I also think this assertion is misplaced in the interface start
handler. If you want it, do it in the rfkill check function. (for
nondebug build it's a no-op)
> /* XXX: only do if device doesn't support rfkill irq */
> wiphy_rfkill_start_polling(hw->wiphy);
>
> diff --git a/drivers/net/wireless/b43/rfkill.c b/drivers/net/wireless/b43/rfkill.c
> index 78016ae..012ed2f 100644
> --- a/drivers/net/wireless/b43/rfkill.c
> +++ b/drivers/net/wireless/b43/rfkill.c
> @@ -26,25 +26,10 @@
>
>
> /* Returns TRUE, if the radio is enabled in hardware. */
> -bool b43_is_hw_radio_enabled(struct b43_wldev *dev)
> +inline bool b43_is_hw_radio_enabled(struct b43_wldev *dev)
inline doesn't make sense here.
> {
> - if (dev->phy.rev >= 3 || dev->phy.type == B43_PHYTYPE_LP) {
> - if (!(b43_read32(dev, B43_MMIO_RADIO_HWENABLED_HI)
> - & B43_MMIO_RADIO_HWENABLED_HI_MASK))
> - return 1;
> - } else {
> - /* To prevent CPU fault on PPC, do not read a register
> - * unless the interface is started; however, on resume
> - * for hibernation, this routine is entered early. When
> - * that happens, unconditionally return TRUE.
> - */
> - if (b43_status(dev) < B43_STAT_STARTED)
> - return 1;
> - if (b43_read16(dev, B43_MMIO_RADIO_HWENABLED_LO)
> - & B43_MMIO_RADIO_HWENABLED_LO_MASK)
> - return 1;
> - }
> - return 0;
> + return !(b43_read32(dev, B43_MMIO_RADIO_HWENABLED_HI)
> + & B43_MMIO_RADIO_HWENABLED_HI_MASK);
> }
Getting rid of that old crap is a good idea. So ACK on this part.
> /* The poll callback for the hardware button. */
> diff --git a/drivers/net/wireless/b43/rfkill.h b/drivers/net/wireless/b43/rfkill.h
> index f046c3c..7aa8a5a 100644
> --- a/drivers/net/wireless/b43/rfkill.h
> +++ b/drivers/net/wireless/b43/rfkill.h
> @@ -4,8 +4,8 @@
> struct ieee80211_hw;
> struct b43_wldev;
>
> -void b43_rfkill_poll(struct ieee80211_hw *hw);
> +inline bool b43_is_hw_radio_enabled(struct b43_wldev *dev);
>
> -bool b43_is_hw_radio_enabled(struct b43_wldev *dev);
> +void b43_rfkill_poll(struct ieee80211_hw *hw);
>
> #endif /* B43_RFKILL_H_ */
doesn't make sense.
--
Greetings Michael.
^ permalink raw reply [flat|nested] 5+ messages in thread* [RFC][PATCH] b43: rfkill: use HI enabled bit for all devices
2010-11-17 20:17 ` Michael Büsch
@ 2010-11-17 20:23 ` Rafał Miłecki
2010-11-17 20:29 ` Michael Büsch
0 siblings, 1 reply; 5+ messages in thread
From: Rafał Miłecki @ 2010-11-17 20:23 UTC (permalink / raw)
To: Michael Büsch; +Cc: linux-wireless, John W. Linville, b43-dev
W dniu 17 listopada 2010 21:17 u?ytkownik Michael B?sch <mb@bu3sch.de> napisa?:
> On Wed, 2010-11-17 at 20:56 +0100, Rafa? Mi?ecki wrote:
>> Devices which use LO enabled bit are covered by b43legacy
>>
>> Signed-off-by: Rafa? Mi?ecki <zajec5@gmail.com>
>> ---
>> Is this alright to use inline for this? Is my WARN_ON OK?
>> ---
>> ?drivers/net/wireless/b43/main.c ? | ? ?2 ++
>> ?drivers/net/wireless/b43/rfkill.c | ? 21 +++------------------
>> ?drivers/net/wireless/b43/rfkill.h | ? ?4 ++--
>> ?3 files changed, 7 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/net/wireless/b43/main.c b/drivers/net/wireless/b43/main.c
>> index fa48803..9b71bb1 100644
>> --- a/drivers/net/wireless/b43/main.c
>> +++ b/drivers/net/wireless/b43/main.c
>> @@ -4527,6 +4527,8 @@ static int b43_op_start(struct ieee80211_hw *hw)
>> ? ? ? ? ? ? ? }
>> ? ? ? }
>>
>> + ? ? /* we don't expect older devices which need other RFKILL check */
>> + ? ? B43_WARN_ON(dev->dev->id.revision < 3);
>
> Do we really need that check? The driver is full of assumptions that
> the core revision is >=5.
Thanks for info, wasn't aware of that.
> I also think this assertion is misplaced in the interface start
> handler. If you want it, do it in the rfkill check function. (for
> nondebug build it's a no-op)
>
>> ? ? ? /* XXX: only do if device doesn't support rfkill irq */
>> ? ? ? wiphy_rfkill_start_polling(hw->wiphy);
>>
>> diff --git a/drivers/net/wireless/b43/rfkill.c b/drivers/net/wireless/b43/rfkill.c
>> index 78016ae..012ed2f 100644
>> --- a/drivers/net/wireless/b43/rfkill.c
>> +++ b/drivers/net/wireless/b43/rfkill.c
>> @@ -26,25 +26,10 @@
>>
>>
>> ?/* Returns TRUE, if the radio is enabled in hardware. */
>> -bool b43_is_hw_radio_enabled(struct b43_wldev *dev)
>> +inline bool b43_is_hw_radio_enabled(struct b43_wldev *dev)
>
> inline doesn't make sense here.
Err, tip for compiler for optimization? To avoid some JUMPs in generated ASM?
>> {
>> - ? ? if (dev->phy.rev >= 3 || dev->phy.type == B43_PHYTYPE_LP) {
>> - ? ? ? ? ? ? if (!(b43_read32(dev, B43_MMIO_RADIO_HWENABLED_HI)
>> - ? ? ? ? ? ? ? ? ? & B43_MMIO_RADIO_HWENABLED_HI_MASK))
>> - ? ? ? ? ? ? ? ? ? ? return 1;
>> - ? ? } else {
>> - ? ? ? ? ? ? /* To prevent CPU fault on PPC, do not read a register
>> - ? ? ? ? ? ? ?* unless the interface is started; however, on resume
>> - ? ? ? ? ? ? ?* for hibernation, this routine is entered early. When
>> - ? ? ? ? ? ? ?* that happens, unconditionally return TRUE.
>> - ? ? ? ? ? ? ?*/
>> - ? ? ? ? ? ? if (b43_status(dev) < B43_STAT_STARTED)
>> - ? ? ? ? ? ? ? ? ? ? return 1;
>> - ? ? ? ? ? ? if (b43_read16(dev, B43_MMIO_RADIO_HWENABLED_LO)
>> - ? ? ? ? ? ? ? ? & B43_MMIO_RADIO_HWENABLED_LO_MASK)
>> - ? ? ? ? ? ? ? ? ? ? return 1;
>> - ? ? }
>> - ? ? return 0;
>> + ? ? return !(b43_read32(dev, B43_MMIO_RADIO_HWENABLED_HI)
>> + ? ? ? ? ? ? & B43_MMIO_RADIO_HWENABLED_HI_MASK);
>> ?}
>
> Getting rid of that old crap is a good idea. So ACK on this part.
>
>> /* The poll callback for the hardware button. */
>> diff --git a/drivers/net/wireless/b43/rfkill.h b/drivers/net/wireless/b43/rfkill.h
>> index f046c3c..7aa8a5a 100644
>> --- a/drivers/net/wireless/b43/rfkill.h
>> +++ b/drivers/net/wireless/b43/rfkill.h
>> @@ -4,8 +4,8 @@
>> ?struct ieee80211_hw;
>> ?struct b43_wldev;
>>
>> -void b43_rfkill_poll(struct ieee80211_hw *hw);
>> +inline bool b43_is_hw_radio_enabled(struct b43_wldev *dev);
>>
>> -bool b43_is_hw_radio_enabled(struct b43_wldev *dev);
>> +void b43_rfkill_poll(struct ieee80211_hw *hw);
>>
>> ?#endif /* B43_RFKILL_H_ */
>
> doesn't make sense.
"inline" was added to match rfkill.c change. Order was changed to
match rfkill.c order, but I guess we can live without that.
--
Rafa?
^ permalink raw reply [flat|nested] 5+ messages in thread* [RFC][PATCH] b43: rfkill: use HI enabled bit for all devices
2010-11-17 20:23 ` Rafał Miłecki
@ 2010-11-17 20:29 ` Michael Büsch
2010-11-17 21:02 ` Rafał Miłecki
0 siblings, 1 reply; 5+ messages in thread
From: Michael Büsch @ 2010-11-17 20:29 UTC (permalink / raw)
To: Rafał Miłecki; +Cc: linux-wireless, John W. Linville, b43-dev
On Wed, 2010-11-17 at 21:23 +0100, Rafa? Mi?ecki wrote:
> >> /* Returns TRUE, if the radio is enabled in hardware. */
> >> -bool b43_is_hw_radio_enabled(struct b43_wldev *dev)
> >> +inline bool b43_is_hw_radio_enabled(struct b43_wldev *dev)
> >
> > inline doesn't make sense here.
>
> Err, tip for compiler for optimization? To avoid some JUMPs in generated ASM?
Inline doesn't really work that way. In this case it might generate
an inline version for callers inside of rfkill.c and an
always-out-of-line version for other callers.
If you really want it inline (Which I think isn't really necessary
as this isn't a fastpath), you'll need to make it static inline
and put it into rfkill.h
--
Greetings Michael.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [RFC][PATCH] b43: rfkill: use HI enabled bit for all devices
2010-11-17 20:29 ` Michael Büsch
@ 2010-11-17 21:02 ` Rafał Miłecki
0 siblings, 0 replies; 5+ messages in thread
From: Rafał Miłecki @ 2010-11-17 21:02 UTC (permalink / raw)
To: Michael Büsch; +Cc: linux-wireless, John W. Linville, b43-dev
W dniu 17 listopada 2010 21:29 u?ytkownik Michael B?sch <mb@bu3sch.de> napisa?:
> On Wed, 2010-11-17 at 21:23 +0100, Rafa? Mi?ecki wrote:
>> >> ?/* Returns TRUE, if the radio is enabled in hardware. */
>> >> -bool b43_is_hw_radio_enabled(struct b43_wldev *dev)
>> >> +inline bool b43_is_hw_radio_enabled(struct b43_wldev *dev)
>> >
>> > inline doesn't make sense here.
>>
>> Err, tip for compiler for optimization? To avoid some JUMPs in generated ASM?
>
> Inline doesn't really work that way. In this case it might generate
> an inline version for callers inside of rfkill.c and an
> always-out-of-line version for other callers.
> If you really want it inline (Which I think isn't really necessary
> as this isn't a fastpath), you'll need to make it static inline
> and put it into rfkill.h
Huh, I got no idea inline works differently for local calls and calls
from other files. That's tricky.
Thanks, I'll send V2.
--
Rafa?
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-11-17 21:02 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-11-17 19:56 [RFC][PATCH] b43: rfkill: use HI enabled bit for all devices Rafał Miłecki
2010-11-17 20:17 ` Michael Büsch
2010-11-17 20:23 ` Rafał Miłecki
2010-11-17 20:29 ` Michael Büsch
2010-11-17 21:02 ` Rafał Miłecki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).