ath9k: pass only one argument to hw attach

PATCH New Luis R. Rodriguez 2 participants 3 messages Aug 3, 2009 Qualcomm Atheros ATH9K wireless driver

lore

Share & tools

lore

Luis R. Rodriguez

author

2009-08-03 19:33 UTC #0 lore raw

PATCH 8/24 ath9k: pass only one argument to hw attach

The softc is cached and set within the ath_hw struct.

Signed-off-by Luis R. Rodriguez <[email protected]>

---

Resending, johill informs me this one didn't make it out.

 drivers/net/wireless/ath/ath9k/hw.c   |   12 ++++++------
 drivers/net/wireless/ath/ath9k/hw.h   |    2 +-
 drivers/net/wireless/ath/ath9k/main.c |    2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

drivers/net/wireless/ath/ath9k/hw.c +6 -6

index d8ae289..301ef04 100644
@@ -627,7 +627,7 @@ static bool ath9k_hw_devid_supported(u16 devid)
 	return false;
 }
 
-int ath9k_hw_attach(struct ath_hw *ah, struct ath_softc *sc)
+int ath9k_hw_attach(struct ath_hw *ah)
 {
 	int r;
 	u32 i, j;

@@ -641,13 +641,13 @@ int ath9k_hw_attach(struct ath_hw *ah, struct ath_softc *sc)
 	ath9k_hw_set_defaults(ah);
 
 	if (!ath9k_hw_set_reset_reg(ah, ATH9K_RESET_POWER_ON)) {
-		DPRINTF(sc, ATH_DBG_FATAL, "Couldn't reset chip\n");
+		DPRINTF(ah->ah_sc, ATH_DBG_FATAL, "Couldn't reset chip\n");
 		r = -EIO;
 		goto bad;
 	}
 
 	if (!ath9k_hw_setpower(ah, ATH9K_PM_AWAKE)) {
-		DPRINTF(sc, ATH_DBG_FATAL, "Couldn't wakeup chip\n");
+		DPRINTF(ah->ah_sc, ATH_DBG_FATAL, "Couldn't wakeup chip\n");
 		r = -EIO;
 		goto bad;
 	}

@@ -663,7 +663,7 @@ int ath9k_hw_attach(struct ath_hw *ah, struct ath_softc *sc)
 		}
 	}
 
-	DPRINTF(sc, ATH_DBG_RESET, "serialize_regmode is %d\n",
+	DPRINTF(ah->ah_sc, ATH_DBG_RESET, "serialize_regmode is %d\n",
 		ah->config.serialize_regmode);
 
 	switch (ah->hw_version.macVersion) {

@@ -676,7 +676,7 @@ int ath9k_hw_attach(struct ath_hw *ah, struct ath_softc *sc)
 	case AR_SREV_VERSION_9287:
 		break;
 	default:
-		DPRINTF(sc, ATH_DBG_FATAL,
+		DPRINTF(ah->ah_sc, ATH_DBG_FATAL,
 			"Mac Chip Rev 0x%02x.%x is not supported by "
 			"this driver\n", ah->hw_version.macVersion,
 			ah->hw_version.macRev);

@@ -945,7 +945,7 @@ int ath9k_hw_attach(struct ath_hw *ah, struct ath_softc *sc)
 
 	r = ath9k_hw_init_macaddr(ah);
 	if (r) {
-		DPRINTF(sc, ATH_DBG_FATAL,
+		DPRINTF(ah->ah_sc, ATH_DBG_FATAL,
 			"Failed to initialize MAC address\n");
 		goto bad;
 	}

drivers/net/wireless/ath/ath9k/hw.h +1 -1

index c769dd6..35cf9f8 100644
@@ -544,7 +544,7 @@ struct ath_hw {
 /* Attach, Detach, Reset */
 const char *ath9k_hw_probe(u16 vendorid, u16 devid);
 void ath9k_hw_detach(struct ath_hw *ah);
-int ath9k_hw_attach(struct ath_hw *ah, struct ath_softc *sc);
+int ath9k_hw_attach(struct ath_hw *ah);
 void ath9k_hw_rfdetach(struct ath_hw *ah);
 int ath9k_hw_reset(struct ath_hw *ah, struct ath9k_channel *chan,
 		   bool bChannelChange);

drivers/net/wireless/ath/ath9k/main.c +1 -1

index 605d328..6242950 100644
@@ -1334,7 +1334,7 @@ static int ath_init(u16 devid, struct ath_softc *sc)
 	ah->hw_version.devid = devid;
 	sc->sc_ah = ah;
 
-	r = ath9k_hw_attach(ah, sc);
+	r = ath9k_hw_attach(ah);
 	if (r) {
 		DPRINTF(sc, ATH_DBG_FATAL,
 			"Unable to attach hardware; "

Signature · 1 line

1.6.3.3

Pavel Roskin

2009-08-03 19:58 UTC #1 lore raw

PATCH 8/24 ath9k: pass only one argument to hw attach

Quoted 1 line · Luis R. Rodriguez

The softc is cached and set within the ath_hw struct.

That's not meant to criticize this particular patch, but what's the point in distinguishing sc from ah these days? I believe "ah" used to refer to the closed part of the sc structure that was only touched by Atheros HAL, hence the name.

Since there is no HAL anymore, ah should disappear.

As the first step, ah should be allocated as part of sc. Then ah and sc should stop pointing to each other. Instead, macros based on offsetof should be used. Then ah should be dissolved and replaced with sc throughout the code.

That's probably a good task for an aspiring kernel hacker, as it requires little specialized knowledge.

Signature · 2 lines

Regards,
Pavel Roskin

Luis R. Rodriguez

author

2009-08-03 20:28 UTC #2 lore raw

PATCH 8/24 ath9k: pass only one argument to hw attach

Quoted 9 lines · Pavel Roskin

Quoted 1 line · Luis R. Rodriguez
The softc is cached and set within the ath_hw struct.
That's not meant to criticize this particular patch, but what's the point in distinguishing sc from ah these days? ?I believe "ah" used to refer to the closed part of the sc structure that was only touched by Atheros HAL, hence the name.
Since there is no HAL anymore, ah should disappear.

I started looking to do this but stopped for a few reasons.

1) It seems softc still serves some good purpose to us:
a) Rate control stuff
b) Place holder for timers, spin_locks, etc
c) Configuration knobs not yet available on upper layers (tx/rx
chainmask is one, for example)
2) ath9k virtual wiphy stuff

Rate control is one chapter in itself, but the general idea is to get proper HT support added to mac80211 rate control API and to one rate control algorithm, say minstrel. To really accomplish this we should strive to allow users to select between the private driver rate control algorithm or minstrel on ath9k.

I don't see a point to move timers/spinlocks to hw related structs, at least yet.

We then should see what knobs we can make generic or not.

Virtual wiphy is very specific to ath9k so far, some serious work is required to see if we can move some of this up later.

Quoted 4 lines

As the first step, ah should be allocated as part of sc. ?Then ah and sc should stop pointing to each other. ?Instead, macros based on offsetof should be used. ?Then ah should be dissolved and replaced with sc throughout the code.

That sounds reasonable.

Quoted 2 lines

That's probably a good task for an aspiring kernel hacker, as it requires little specialized knowledge.

Patches welcomed.

Luis