Net: ath5k, split hw into hw, phy and initvals

PATCH In review Jiri Slaby 4 participants 9 messages Sep 24, 2007 Networking drivers (wireless)

lore

Share & tools

lore

Jiri Slaby

author

2007-09-24 09:02 UTC #0 lore raw

PATCH 1/2 Net: ath5k, split hw into hw, phy and initvals

From: Nick Kossifidis <[email protected]>

ath5k, split hw into hw, phy and initvals

Separate the hw code into logical pieces hw, phy and initvals for better readability and maintainability.

This include suggested file renames (_hw removal).

Changes-licensed-under: ISC

Signed-off-by Nick Kossifidis <[email protected]>

Signed-off-by Jiri Slaby <[email protected]>

Cc Luis R. Rodriguez <[email protected]>

---
commit a17245819b3b7910783dfb65f33ef2057f9e8ed4
tree 55df284e20c91742a55917e2fed3337b735ad6a8
parent 657bdad86e3a284f0195df4b78b833d21beb727f
author Jiri Slaby <[email protected]> Mon, 24 Sep 2007 10:41:12 +0200 committer Jiri Slaby <[email protected]> Mon, 24 Sep 2007 10:41:12 +0200

 drivers/net/wireless/Makefile         |    3 
 drivers/net/wireless/ath5k.h          |  221 ++--
 drivers/net/wireless/ath5k_hw.c       | 1306 --------------------------
 drivers/net/wireless/ath5k_hw.h       | 1513 ------------------------------
 drivers/net/wireless/ath5k_initvals.c | 1090 +++++++++++++++++++++
 drivers/net/wireless/ath5k_phy.c      | 1674 +++++++++++++++++++++++++++++++++
 6 files changed, 2889 insertions(+), 2918 deletions(-)

drivers/net/wireless/Makefile +2 -1

index 02f8b71..3f976f8 100644
@@ -49,4 +49,5 @@ rtl8187-objs		:= rtl8187_dev.o rtl8187_rtl8225.o
 obj-$(CONFIG_RTL8187)	+= rtl8187.o
 
 obj-$(CONFIG_ATH5K)	+= ath5k.o
-ath5k-objs		= ath5k_base.o ath5k_hw.o ath5k_regdom.o
+ath5k-objs		= ath5k_base.o ath5k_hw.o ath5k_regdom.o \
+			  ath5k_phy.o ath5k_initvals.o

drivers/net/wireless/ath5k.h +103 ~~-118~~

271 lines not on this page · raw message

drivers/net/wireless/ath5k_hw.c +8 ~~-1298~~

1,392 lines not on this page · raw message

drivers/net/wireless/ath5k_hw.h +12 ~~-1501~~

1,331 lines not on this page · raw message

drivers/net/wireless/ath5k_initvals.c new file +1090 -0

1,090 lines not on this page · raw message

drivers/net/wireless/ath5k_phy.c new file +1674 -0

1,674 lines not on this page · raw message

The rest of this diff is not shown. View the raw message.

Part of the diff is not shown here. View the raw message.

Jiri Slaby

author

2007-09-24 09:03 UTC #1 lore raw

PATCH 2/2 Net: ath5k, use int as retval

ath5k, use int as retval

Changes-licensed-under: ISC

Signed-off-by Jiri Slaby <[email protected]>

Cc Luis R. Rodriguez <[email protected]>

---
commit a1b01e7e3ac4dbbe96268f94c1eb25b3932220d4
tree b587f200071c5e50e4cb79775f06630a5b7b5bb5
parent a17245819b3b7910783dfb65f33ef2057f9e8ed4
author Jiri Slaby <[email protected]> Mon, 24 Sep 2007 10:53:57 +0200 committer Jiri Slaby <[email protected]> Mon, 24 Sep 2007 10:53:57 +0200

 drivers/net/wireless/ath5k.h      |   27 +++---
 drivers/net/wireless/ath5k_base.c |    2 
 drivers/net/wireless/ath5k_hw.c   |  169 +++++++++++++++++--------------------
 3 files changed, 91 insertions(+), 107 deletions(-)

drivers/net/wireless/ath5k.h +12 ~~-15~~

index b069f45..69e2efe 100644
@@ -979,10 +979,10 @@ extern int ath5k_hw_stop_rx_dma(struct ath_hw *hal);
 extern u32 ath5k_hw_get_rx_buf(struct ath_hw *hal);
 extern void ath5k_hw_put_rx_buf(struct ath_hw *hal, u32 phys_addr);
 extern int ath5k_hw_tx_start(struct ath_hw *hal, unsigned int queue);
-extern bool ath5k_hw_stop_tx_dma(struct ath_hw *hal, unsigned int queue);
+extern int ath5k_hw_stop_tx_dma(struct ath_hw *hal, unsigned int queue);
 extern u32 ath5k_hw_get_tx_buf(struct ath_hw *hal, unsigned int queue);
 extern int ath5k_hw_put_tx_buf(struct ath_hw *hal, unsigned int queue, u32 phys_addr);
-extern bool ath5k_hw_update_tx_triglevel(struct ath_hw *hal, bool increase);
+extern int ath5k_hw_update_tx_triglevel(struct ath_hw *hal, bool increase);
 /* Interrupt handling */
 extern bool ath5k_hw_is_intr_pending(struct ath_hw *hal);
 extern int ath5k_hw_get_isr(struct ath_hw *hal, enum ath5k_int *interrupt_mask);

@@ -990,19 +990,19 @@ extern enum ath5k_int ath5k_hw_set_intr(struct ath_hw *hal, enum ath5k_int new_m
 /* EEPROM access functions */
 extern int ath5k_hw_set_regdomain(struct ath_hw *hal, u16 regdomain);
 /* Protocol Control Unit Functions */
-extern void ath5k_hw_set_opmode(struct ath_hw *hal);
+extern int ath5k_hw_set_opmode(struct ath_hw *hal);
 /* BSSID Functions */
 extern void ath5k_hw_get_lladdr(struct ath_hw *hal, u8 *mac);
-extern bool ath5k_hw_set_lladdr(struct ath_hw *hal, const u8 *mac);
+extern int ath5k_hw_set_lladdr(struct ath_hw *hal, const u8 *mac);
 extern void ath5k_hw_set_associd(struct ath_hw *hal, const u8 *bssid, u16 assoc_id);
-extern bool ath5k_hw_set_bssid_mask(struct ath_hw *hal, const u8 *mask);
+extern int ath5k_hw_set_bssid_mask(struct ath_hw *hal, const u8 *mask);
 /* Receive start/stop functions */
 extern void ath5k_hw_start_rx_pcu(struct ath_hw *hal);
 extern void ath5k_hw_stop_pcu_recv(struct ath_hw *hal);
 /* RX Filter functions */
 extern void ath5k_hw_set_mcast_filter(struct ath_hw *hal, u32 filter0, u32 filter1);
-extern bool ath5k_hw_set_mcast_filterindex(struct ath_hw *hal, u32 index);
-extern bool ath5k_hw_clear_mcast_filter_idx(struct ath_hw *hal, u32 index);
+extern int ath5k_hw_set_mcast_filterindex(struct ath_hw *hal, u32 index);
+extern int ath5k_hw_clear_mcast_filter_idx(struct ath_hw *hal, u32 index);
 extern u32 ath5k_hw_get_rx_filter(struct ath_hw *ah);
 extern void ath5k_hw_set_rx_filter(struct ath_hw *ah, u32 filter);
 /* Beacon related functions */

@@ -1010,14 +1010,14 @@ extern u32 ath5k_hw_get_tsf32(struct ath_hw *hal);
 extern u64 ath5k_hw_get_tsf64(struct ath_hw *hal);
 extern void ath5k_hw_reset_tsf(struct ath_hw *hal);
 extern void ath5k_hw_init_beacon(struct ath_hw *hal, u32 next_beacon, u32 interval);
-extern void ath5k_hw_set_beacon_timers(struct ath_hw *hal, const struct ath5k_beacon_state *state);
+extern int ath5k_hw_set_beacon_timers(struct ath_hw *hal, const struct ath5k_beacon_state *state);
 extern void ath5k_hw_reset_beacon(struct ath_hw *hal);
-extern bool ath5k_hw_wait_for_beacon(struct ath_hw *hal, unsigned long phys_addr);
+extern int ath5k_hw_wait_for_beacon(struct ath_hw *hal, unsigned long phys_addr);
 extern void ath5k_hw_update_mib_counters(struct ath_hw *hal, struct ath5k_mib_stats *statistics);
 /* ACK/CTS Timeouts */
-extern bool ath5k_hw_set_ack_timeout(struct ath_hw *hal, unsigned int timeout);
+extern int ath5k_hw_set_ack_timeout(struct ath_hw *hal, unsigned int timeout);
 extern unsigned int ath5k_hw_get_ack_timeout(struct ath_hw *hal);
-extern bool ath5k_hw_set_cts_timeout(struct ath_hw *hal, unsigned int timeout);
+extern int ath5k_hw_set_cts_timeout(struct ath_hw *hal, unsigned int timeout);
 extern unsigned int ath5k_hw_get_cts_timeout(struct ath_hw *hal);
 /* Key table (WEP) functions */
 extern int ath5k_hw_reset_key(struct ath_hw *hal, u16 entry);

@@ -1031,7 +1031,7 @@ extern int ath5k_hw_get_tx_queueprops(struct ath_hw *hal, int queue, struct ath5
 extern void ath5k_hw_release_tx_queue(struct ath_hw *hal, unsigned int queue);
 extern int ath5k_hw_reset_tx_queue(struct ath_hw *hal, unsigned int queue);
 extern u32 ath5k_hw_num_tx_pending(struct ath_hw *hal, unsigned int queue);
-extern bool ath5k_hw_set_slot_time(struct ath_hw *hal, unsigned int slot_time);
+extern int ath5k_hw_set_slot_time(struct ath_hw *hal, unsigned int slot_time);
 extern unsigned int ath5k_hw_get_slot_time(struct ath_hw *hal);
 /* Hardware Descriptor Functions */
 extern int ath5k_hw_setup_rx_desc(struct ath_hw *hal, struct ath_desc *desc, u32 size, unsigned int flags);

@@ -1047,9 +1047,6 @@ extern u16 ath5k_get_regdomain(struct ath_hw *hal);
 /* Misc functions */
 extern void ath5k_hw_dump_state(struct ath_hw *hal);
 extern int ath5k_hw_get_capability(struct ath_hw *hal, enum ath5k_capability_type cap_type, u32 capability, u32 *result);
-bool ath5k_hw_query_pspoll_support(struct ath_hw *hal);
-bool ath5k_hw_enable_pspoll(struct ath_hw *hal, u8 *bssid, u16 assoc_id);
-bool ath5k_hw_disable_pspoll(struct ath_hw *hal);
 
 
 /* Initial register settings functions */

drivers/net/wireless/ath5k_base.c +1 -1

index fead9a7..665f775 100644
@@ -579,7 +579,7 @@ static void ath_beacon_send(struct ath_softc *sc)
 	 * This should never fail since we check above that no frames
 	 * are still pending on the queue.
 	 */
-	if (unlikely(!ath5k_hw_stop_tx_dma(ah, sc->bhalq))) {
+	if (unlikely(ath5k_hw_stop_tx_dma(ah, sc->bhalq))) {
 		printk(KERN_WARNING "ath: beacon queue %u didn't stop?\n",
 				sc->bhalq);
 		/* NB: the HAL still stops DMA, so proceed */

drivers/net/wireless/ath5k_hw.c +78 ~~-91~~

476 lines not on this page · raw message

Luis R. Rodriguez

2007-09-24 17:24 UTC #2 lore raw

In reply to Jiri Slaby

Quoted 7 lines · Jiri Slaby

From: Nick Kossifidis <[email protected]>
ath5k, split hw into hw, phy and initvals
Separate the hw code into logical pieces hw, phy and initvals for better readability and maintainability.

Quoted 6 lines

This include suggested file renames (_hw removal).
Changes-licensed-under: ISC
Signed-off-by Nick Kossifidis <[email protected]>
Signed-off-by Jiri Slaby <[email protected]>
Cc Luis R. Rodriguez <[email protected]>

<-- snip patch -->

Quoted 2 lines

[Message clipped]
http://mail.google.com/mail/?view=lg&msg=11536c180ed66ff1

I didn't want to take this approach before as it seemed we wanted to try to keep drivers without their own directory and consolidate drivers files into as many few files as necessary. Since it seems that has changed and it has been made clear that we *can* use a directory for each driver then I'd suggest we apply a similar patch but first move ath5k* to ath5k/.

But anyway, I NACK this patch for the following reasons:

MadWifi is not a non-profit yet and as such it is advised to use individual copyright notices.

* ath5k_hw_set_rfgain_opt() is defined once and used once, can we instead make it inline? If we really are not going to re-use this later why do we put it into a routine on its own?

* #if 0's need to go, lets just remove this code instead.

Luis

Johannes Berg

maintainer

2007-09-24 17:26 UTC #3 lore raw

Quoted 3 lines · Luis R. Rodriguez

* ath5k_hw_set_rfgain_opt() is defined once and used once, can we instead make it inline? If we really are not going to re-use this later why do we put it into a routine on its own?

Why mark it inline then? The compiler will inline it anyway if it's only used once. And I haven't looked at the code but making it an own routine could possibly make the code more readable, especially if lots of indentation is involved.`

johannes

Christoph Hellwig

2007-09-24 17:36 UTC #4 lore raw

Quoted 10 lines · Johannes Berg

Quoted 3 lines · Luis R. Rodriguez
* ath5k_hw_set_rfgain_opt() is defined once and used once, can we instead make it inline? If we really are not going to re-use this later why do we put it into a routine on its own?
Why mark it inline then? The compiler will inline it anyway if it's only used once. And I haven't looked at the code but making it an own routine could possibly make the code more readable, especially if lots of indentation is involved.`

Seconded. Splitting things up into logically sound subroutines is good programming style.

Luis R. Rodriguez

2007-09-24 17:51 UTC #5 lore raw

Quoted 7 lines · Christoph Hellwig

Quoted 6 lines · Johannes Berg
Quoted 3 lines · Luis R. Rodriguez
* ath5k_hw_set_rfgain_opt() is defined once and used once, can we instead make it inline? If we really are not going to re-use this later why do we put it into a routine on its own?

Quoted 7 lines

Quoted 4 lines
Why mark it inline then? The compiler will inline it anyway if it's only used once. And I haven't looked at the code but making it an own routine could possibly make the code more readable, especially if lots of indentation is involved.`
Seconded. Splitting things up into logically sound subroutines is good programming style.

Then that comment can be ignored, thanks.

Luis

Jiri Slaby

author

2007-09-24 18:40 UTC #6 lore raw

Quoted 1 line · Luis R. Rodriguez

Then that comment can be ignored, thanks.

Me too, corrected patchset on the road...

Jiri Slaby

author

2007-09-24 18:42 UTC #7 lore raw

In reply to Luis R. Rodriguez

Quoted 1 line · Luis R. Rodriguez

* #if 0's need to go, lets just remove this code instead.

Which code do you mean exactly, there are also parts that might be needed in the future?

thanks,

Signature · 2 lines

Jiri Slaby ([email protected])
Faculty of Informatics, Masaryk University

Luis R. Rodriguez

2007-09-24 18:51 UTC #8 lore raw

Quoted 4 lines · Jiri Slaby

Quoted 1 line · Luis R. Rodriguez
* #if 0's need to go, lets just remove this code instead.
Which code do you mean exactly,

These hunks:

+#if 0
/*
* Get power mode (sleep state)
* TODO:Remove ?
@@ -1385,7 +1148,7 @@ ath5k_hw_get_power_mode(struct ath_hw *hal)

       AR5K_TRACE;
       return hal->ah_power_mode;

}
-
+#endif

/***********************\
DMA Related Functions
@@ -1816,6 +1579,7 @@ enum ath5k_int ath5k_hw_set_intr(struct ath_hw *hal, enum ath5k_int new_mask)

       return old_mask;

}

+#if 0
/*
* Enable HW radar detection
*/
@@ -1858,7 +1622,7 @@ ath5k_hw_radar_alert(struct ath_hw *hal, bool enable)

       /*Re-enable interrupts*/
       ath5k_hw_reg_write(hal, AR5K_IER_ENABLE, AR5K_IER);

}
-

Quoted 1 line

there are also parts that might be needed in the future?

Then we can add then in the future. We can also remove turbo for this same reason, I don't see us introducing it for a while. I should have just removed turbo with my 'internalizing turbo' as its useless code right now. But can patch work can be sent in later though. Mind you madwifi-old-openhal will still have all this code we're not using right now. So we'll always be able to pull from there.

Luis