Bryan Rittmeyer wrote:
Hello again. I've been reviewing the zd1211rw code with the thought of
working on ad-hoc mode.
While reading I found the amusing locking in zd_mac_clear().
I think the point is to make sure that nobody is holding the lock before
we finish clearing up. The code is obviously inaccurate because if that
was even possible, someone could and would take it immediately after the
spin_unlock.
There is actually even more redundancy in the clear path, see the
attached patch. When the clearing happens, the network interface is down
and our interrupts are disabled, so yes, no need for locking. Also, the
whole structure gets freed immediately after clear, so there is no point
in zeroing everything again.
The only risk is that our future housekeeping thread is still running,
but it goes without saying that we must make sure that it is fully
stopped when the interface is brought down when we come to implement it.
Should probably get Ulrich's opinion on this before committing, the
confusing interleaving of layers is his thing ;)
Daniel
diff --git a/zd_chip.c b/zd_chip.c
index 65def95..91c3e90 100644
--- a/zd_chip.c
+++ b/zd_chip.c
@@ -42,12 +42,8 @@ void zd_chip_init(struct zd_chip *chip,
void zd_chip_clear(struct zd_chip *chip)
{
- mutex_lock(&chip->mutex);
zd_usb_clear(&chip->usb);
- zd_rf_clear(&chip->rf);
- mutex_unlock(&chip->mutex);
mutex_destroy(&chip->mutex);
- memset(chip, 0, sizeof(*chip));
}
static int scnprint_mac_oui(const u8 *addr, char *buffer, size_t size)
diff --git a/zd_mac.c b/zd_mac.c
index 114b23d..4bb5a14 100644
--- a/zd_mac.c
+++ b/zd_mac.c
@@ -124,11 +124,7 @@ out:
void zd_mac_clear(struct zd_mac *mac)
{
- /* Aquire the lock. */
- spin_lock(&mac->lock);
- spin_unlock(&mac->lock);
zd_chip_clear(&mac->chip);
- memset(mac, 0, sizeof(*mac));
}
static int reset_mode(struct zd_mac *mac)
diff --git a/zd_rf.c b/zd_rf.c
index 4198062..45bd775 100644
--- a/zd_rf.c
+++ b/zd_rf.c
@@ -54,11 +54,6 @@ void zd_rf_init(struct zd_rf *rf)
memset(rf, 0, sizeof(*rf));
}
-void zd_rf_clear(struct zd_rf *rf)
-{
- memset(rf, 0, sizeof(*rf));
-}
-
int zd_rf_init_hw(struct zd_rf *rf, u8 type)
{
int r, t;
diff --git a/zd_rf.h b/zd_rf.h
index 0276e1e..0da2cf8 100644
--- a/zd_rf.h
+++ b/zd_rf.h
@@ -64,7 +64,6 @@ struct zd_rf {
const char *zd_rf_name(u8 type);
void zd_rf_init(struct zd_rf *rf);
-void zd_rf_clear(struct zd_rf *rf);
int zd_rf_init_hw(struct zd_rf *rf, u8 type);
int zd_rf_reset_hw(struct zd_rf *rf);
diff --git a/zd_usb.c b/zd_usb.c
index b773ec1..bfa3208 100644
--- a/zd_usb.c
+++ b/zd_usb.c
@@ -865,7 +865,6 @@ void zd_usb_clear(struct zd_usb *usb)
{
usb_set_intfdata(usb->intf, NULL);
usb_put_intf(usb->intf);
- memset(usb, 0, sizeof(*usb));
/* FIXME: usb_interrupt, usb_tx, usb_rx? */
}
_______________________________________________
Zd1211-devs mailing list - http://zd1211.ath.cx/
Unsubscribe: https://lists.sourceforge.net/lists/listinfo/zd1211-devs
