[ath9k-devel] [PATCH 08/24] ath9k: pass only one argument to hw attach

[ath9k-devel] [PATCH 08/24] ath9k: pass only one argument to hw attach

[Luis R. Rodriguez]

The softc is cached and set within the ath_hw struct.

Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---

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(-)

diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
index d8ae289..301ef04 100644
--- a/drivers/net/wireless/ath/ath9k/hw.c
+++ b/drivers/net/wireless/ath/ath9k/hw.c
@@ -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;
 	}
diff --git a/drivers/net/wireless/ath/ath9k/hw.h b/drivers/net/wireless/ath/ath9k/hw.h
index c769dd6..35cf9f8 100644
--- a/drivers/net/wireless/ath/ath9k/hw.h
+++ b/drivers/net/wireless/ath/ath9k/hw.h
@@ -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);
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 605d328..6242950 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -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; "
-- 
1.6.3.3

[PATCH 08/24] ath9k: pass only one argument to hw attach

[Luis R. Rodriguez]

The softc is cached and set within the ath_hw struct.

Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---

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(-)

diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
index d8ae289..301ef04 100644
--- a/drivers/net/wireless/ath/ath9k/hw.c
+++ b/drivers/net/wireless/ath/ath9k/hw.c
@@ -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;
 	}
diff --git a/drivers/net/wireless/ath/ath9k/hw.h b/drivers/net/wireless/ath/ath9k/hw.h
index c769dd6..35cf9f8 100644
--- a/drivers/net/wireless/ath/ath9k/hw.h
+++ b/drivers/net/wireless/ath/ath9k/hw.h
@@ -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);
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 605d328..6242950 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -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; "
-- 
1.6.3.3

[ath9k-devel] [PATCH 08/24] ath9k: pass only one argument to hw attach

[Pavel Roskin]

On Mon, 2009-08-03 at 12:33 -0700, Luis R. Rodriguez wrote:
> 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.

-- 
Regards,
Pavel Roskin

Re: [ath9k-devel] [PATCH 08/24] ath9k: pass only one argument to hw attach

[Pavel Roskin]

On Mon, 2009-08-03 at 12:33 -0700, Luis R. Rodriguez wrote:
> 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.

-- 
Regards,
Pavel Roskin

[ath9k-devel] [PATCH 08/24] ath9k: pass only one argument to hw attach

[Luis R. Rodriguez]

On Mon, Aug 3, 2009 at 12:58 PM, Pavel Roskin<proski@gnu.org> wrote:
> On Mon, 2009-08-03 at 12:33 -0700, Luis R. Rodriguez wrote:
>> 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.

> 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.

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

Patches welcomed.

  Luis

Re: [ath9k-devel] [PATCH 08/24] ath9k: pass only one argument to hw attach

[Luis R. Rodriguez]

On Mon, Aug 3, 2009 at 12:58 PM, Pavel Roskin<proski@gnu.org> wrote:
> On Mon, 2009-08-03 at 12:33 -0700, Luis R. Rodriguez wrote:
>> 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.

> 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.

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

Patches welcomed.

  Luis

[ath9k-devel] [PATCH 00/24] ath9k: hw initiazliation cleanup

[Luis R. Rodriguez]

This series consists of a few hardware initialization cleanups
and small fixups. This goal was to make hardware initialization
easier to read but also make it easier to add support for new
hardware. The small fixups just came along the way.

Luis R. Rodriguez (24):
  ath9k: remove usage of AR_SREV_*() wrapper to detect supported hw
  ath9k: use a switch for revising supported hw mac revisions
  ath9k: propagate hw initialization errors
  ath9k: move memory allocation of ath_hw to ath_init()
  ath9k: move devid cache setting to ath_init()
  ath9k: move cache setting of softc ah prior to attach
  ath9k: call hw initializer directly
  ath9k: pass only one argument to hw attach
  ath9k: move hw macrevision checker to helper
  ath9k: rename ath9k_hw_newstate() to ath9k_hw_init_defaults()
  ath9k: rename  ath9k_hw_set_defaults() to ath9k_hw_init_config()
  ath9k: remove debug message for no memoery on ath_init()
  ath9k: break up hw initialization into a few more helpers
  ath9k: describe hw initialization better
  ath9k: distinguish between device initialization and ath_softc init
  ath9k: remove !NULL check before kfree()
  ath9k: use helper macro to kfree and nullify on ath9k_hw_rfdetach()
  ath9k: rename ath9k_hw_rfdetach() to ath9k_hw_rf_free()
  ath9k: rename ath9k_hw_ani_detach() to ath9k_hw_ani_disable()
  ath9k: set ah to null after freeing
  ath9k: set sc->sc_ah to NULL after freeing it
  ath9k: call ath9k_hw_detach() once upon hw init failure
  ath9k: remove dangling error check on keycache reset on hw init
  ath9k: remove spurious check for channel on keycache reset

 drivers/net/wireless/ath/ath9k/ahb.c    |    2 +-
 drivers/net/wireless/ath/ath9k/ani.c    |    6 +-
 drivers/net/wireless/ath/ath9k/ani.h    |    4 +-
 drivers/net/wireless/ath/ath9k/ath9k.h  |    2 +-
 drivers/net/wireless/ath/ath9k/eeprom.c |    2 +-
 drivers/net/wireless/ath/ath9k/eeprom.h |    2 +-
 drivers/net/wireless/ath/ath9k/hw.c     |  259 +++++++++++++++----------------
 drivers/net/wireless/ath/ath9k/hw.h     |    6 +-
 drivers/net/wireless/ath/ath9k/main.c   |   59 +++++---
 drivers/net/wireless/ath/ath9k/pci.c    |    2 +-
 drivers/net/wireless/ath/ath9k/phy.c    |   53 ++-----
 11 files changed, 195 insertions(+), 202 deletions(-)

[ath9k-devel] [PATCH 08/24] ath9k: pass only one argument to hw attach

[Luis R. Rodriguez]

The softc is cached and set within the ath_hw struct.

Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 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(-)

diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
index d8ae289..301ef04 100644
--- a/drivers/net/wireless/ath/ath9k/hw.c
+++ b/drivers/net/wireless/ath/ath9k/hw.c
@@ -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;
 	}
diff --git a/drivers/net/wireless/ath/ath9k/hw.h b/drivers/net/wireless/ath/ath9k/hw.h
index c769dd6..35cf9f8 100644
--- a/drivers/net/wireless/ath/ath9k/hw.h
+++ b/drivers/net/wireless/ath/ath9k/hw.h
@@ -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);
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 605d328..6242950 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -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; "
-- 
1.6.3.3