[ath9k-devel] [PATCH v0 3/6] support for antenna configuration profiles

[ath9k-devel] [PATCH v0 3/6] support for antenna configuration profiles

[Daniel Golle]

This implements extant support for the AR9285 as found in the ALL0258N.
The current way profiles are passed from platform_data is coinned to control
the AntCtrlCommon register, as this is what is needed to switch the antenna
of the ALL0258N.
If there are any known devices using GPIO or AR_PHY_SWITCH_COM2 for this task
this could be further generalized into having an array of
struct { u32 register; u32 value; }
instead of just a single val field.
However, if even this is still not enough we could use a function pointer and
supply a profile switching function -- I'd rather avoid this as it would
introduce lots of coupling between the board setup and wifi stack which would
make things really ugly.
Anyway, for me the simpliest solution is enough, if anyone knows an
antenna-selection implementation not covered by setting a value to
AR_PGT_SWITCH_COM, let me know the details please.

During initialization profiles are added from ath9k_platform_data to both,
wiphy->extant and ah->switch_com_profiles. (cosmetics: you prefere an array of an
anonymous struct or should there be another named struct switch_com_profile in
ath9k_platform.h ?)

get_extant matches the current value of AR_PHY_SWITCH_COM against the profiles
and returns the id of the matching profile.
Vice versa, set_extant looks up a profile for the gives id and sets
AR_PHY_SWICH_COM to the value stored in the profile.

Currently, the profiles supplied from platform_data don't go through any sanity-
checking. If we consider board-setup code to be potentially wrong, we could at
least make sure that the value of pModal->antCtrlCommon is contained in one of
the profiles and otherwise add a 'default'-profile and/or entirely drop all
profile definitions as a miss-match here points to that the profiles don't match
the actual wifi module. If you believe we should have this, please let me know
and I'll implement it accordingly.

Another head-ache is the kcalloc in ath9k_set_hw_capab, as there is currently
no way to handle the case if that goes wrong (it really shouldn't, but it's not
clean in the way it works now). Pre-allocate n profiles in another place? 
Or just don't add any profiles if kcalloc fails?
In ath9k_platform_data I already assume there are a maximum of 4 profiles in
order to avoid needing any k?alloc call in that place.

Signed-off-by: Daniel Golle <dgolle@allnet.de>

--- a/include/linux/ath9k_platform.h
+++ b/include/linux/ath9k_platform.h
@@ -20,6 +20,7 @@
 #define _LINUX_ATH9K_PLATFORM_H
 
 #define ATH9K_PLAT_EEP_MAX_WORDS	2048
+#define ATH9K_PLAT_MAX_SWITCHCOM_PROFILES	4
 
 struct ath9k_platform_data {
 	u16 eeprom_data[ATH9K_PLAT_EEP_MAX_WORDS];
@@ -32,6 +33,11 @@ struct ath9k_platform_data {
 	bool is_clk_25mhz;
 	int (*get_mac_revision)(void);
 	int (*external_reset)(void);
+	struct {
+		u32 val;
+		char *name;
+		char *desc;
+	} switchcom_profiles[ATH9K_PLAT_MAX_SWITCHCOM_PROFILES];
 };

 #endif /* _LINUX_ATH9K_PLATFORM_H */
--- a/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c
+++ b/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c
@@ -5130,6 +5130,15 @@ unsigned int ar9003_get_paprd_scale_fact
 	}
 }
 
+static void ath9k_hw_ar9300_set_switch_com(struct ath_hw *ah, u32 val)
+{
+}
+
+static u32 ath9k_hw_ar9300_get_switch_com(struct ath_hw *ah)
+{
+	return 0;
+}
+
 const struct eeprom_ops eep_ar9300_ops = {
 	.check_eeprom = ath9k_hw_ar9300_check_eeprom,
 	.get_eeprom = ath9k_hw_ar9300_get_eeprom,
@@ -5140,5 +5149,7 @@ const struct eeprom_ops eep_ar9300_ops =
 	.set_board_values = ath9k_hw_ar9300_set_board_values,
 	.set_addac = ath9k_hw_ar9300_set_addac,
 	.set_txpower = ath9k_hw_ar9300_set_txpower,
-	.get_spur_channel = ath9k_hw_ar9300_get_spur_channel
+	.get_spur_channel = ath9k_hw_ar9300_get_spur_channel,
+	.set_switch_com = ath9k_hw_ar9300_set_switch_com,
+	.get_switch_com = ath9k_hw_ar9300_get_switch_com
 };
--- a/drivers/net/wireless/ath/ath9k/eeprom.h
+++ b/drivers/net/wireless/ath/ath9k/eeprom.h
@@ -655,6 +655,8 @@ struct eeprom_ops {
 			   u16 cfgCtl, u8 twiceAntennaReduction,
 			   u8 powerLimit, bool test);
 	u16 (*get_spur_channel)(struct ath_hw *ah, u16 i, bool is2GHz);
+	void (*set_switch_com)(struct ath_hw *ah, u32 val);
+	u32 (*get_switch_com)(struct ath_hw *ah);
 };
 
 void ath9k_hw_analog_shift_regwrite(struct ath_hw *ah, u32 reg, u32 val);
--- a/drivers/net/wireless/ath/ath9k/eeprom_4k.c
+++ b/drivers/net/wireless/ath/ath9k/eeprom_4k.c
@@ -1103,6 +1103,19 @@ static u16 ath9k_hw_4k_get_spur_channel(
 #undef EEP_MAP4K_SPURCHAN
 }
 
+static void ath9k_hw_4k_set_switch_com(struct ath_hw *ah, u32 *val)
+{
+	struct ar5416_eeprom_4k *eep = &ah->eeprom.map4k;
+	struct modal_eep_4k_header *pModal = &eep->modalHeader;
+
+	REG_WRITE(ah, AR_PHY_SWITCH_COM, pModal->antCtrlCommon = val);
+}
+
+static u32 ath9k_hw_4k_get_switch_com(struct ath_hw *ah)
+{
+	return REG_READ(ah, AR_PHY_SWITCH_COM);
+}
+
 const struct eeprom_ops eep_4k_ops = {
 	.check_eeprom		= ath9k_hw_4k_check_eeprom,
 	.get_eeprom		= ath9k_hw_4k_get_eeprom,
@@ -1112,5 +1125,7 @@ const struct eeprom_ops eep_4k_ops = {
 	.get_eeprom_rev		= ath9k_hw_4k_get_eeprom_rev,
 	.set_board_values	= ath9k_hw_4k_set_board_values,
 	.set_txpower		= ath9k_hw_4k_set_txpower,
-	.get_spur_channel	= ath9k_hw_4k_get_spur_channel
+	.get_spur_channel	= ath9k_hw_4k_get_spur_channel,
+	.set_switch_com		= ath9k_hw_4k_set_switch_com,
+	.get_switch_com		= ath9k_hw_4k_get_switch_com
 };
--- a/drivers/net/wireless/ath/ath9k/eeprom_9287.c
+++ b/drivers/net/wireless/ath/ath9k/eeprom_9287.c
@@ -1062,6 +1062,15 @@ static u16 ath9k_hw_ar9287_get_spur_chan
 #undef EEP_MAP9287_SPURCHAN
 }
 
+static void ath9k_hw_ar9287_set_switch_com(struct ath_hw *ah, u32 val)
+{
+}
+
+static u32 ath9k_hw_ar9287_get_switch_com(struct ath_hw *ah)
+{
+	return 0;
+}
+
 const struct eeprom_ops eep_ar9287_ops = {
 	.check_eeprom		= ath9k_hw_ar9287_check_eeprom,
 	.get_eeprom		= ath9k_hw_ar9287_get_eeprom,
@@ -1071,5 +1080,7 @@ const struct eeprom_ops eep_ar9287_ops =
 	.get_eeprom_rev		= ath9k_hw_ar9287_get_eeprom_rev,
 	.set_board_values	= ath9k_hw_ar9287_set_board_values,
 	.set_txpower		= ath9k_hw_ar9287_set_txpower,
-	.get_spur_channel	= ath9k_hw_ar9287_get_spur_channel
+	.get_spur_channel	= ath9k_hw_ar9287_get_spur_channel,
+	.set_switch_com		= ath9k_hw_ar9287_set_switch_com,
+	.get_switch_com		= ath9k_hw_ar9287_get_switch_com
 };
--- a/drivers/net/wireless/ath/ath9k/eeprom_def.c
+++ b/drivers/net/wireless/ath/ath9k/eeprom_def.c
@@ -1420,6 +1420,15 @@ static u16 ath9k_hw_def_get_spur_channel
 #undef EEP_DEF_SPURCHAN
 }
 
+static void ath9k_hw_def_set_switch_com(struct ath_hw *ah, u32 val)
+{
+}
+
+static u32 ath9k_hw_def_get_switch_com(struct ath_hw *ah)
+{
+	return 0;
+}
+
 const struct eeprom_ops eep_def_ops = {
 	.check_eeprom		= ath9k_hw_def_check_eeprom,
 	.get_eeprom		= ath9k_hw_def_get_eeprom,
@@ -1430,5 +1439,7 @@ const struct eeprom_ops eep_def_ops = {
 	.set_board_values	= ath9k_hw_def_set_board_values,
 	.set_addac		= ath9k_hw_def_set_addac,
 	.set_txpower		= ath9k_hw_def_set_txpower,
-	.get_spur_channel	= ath9k_hw_def_get_spur_channel
+	.get_spur_channel	= ath9k_hw_def_get_spur_channel,
+	.set_switch_com		= ath9k_hw_def_set_switch_com,
+	.get_switch_com		= ath9k_hw_def_get_switch_com
 };
--- a/drivers/net/wireless/ath/ath9k/hw.h
+++ b/drivers/net/wireless/ath/ath9k/hw.h
@@ -537,6 +537,25 @@ struct ath_hw_radar_conf {
 	bool ext_channel;
 };
 
+
+/**
+ * struct ath_hw_switch_com_profile - external antenna switch settings
+ *
+ * This structure holds a platform_data-supplied alternative value
+ * of AR_PHY_SWITCH_COM.
+ *
+ * @id: unique identifier
+ * @switch_com_value: bit-field to be set in register AR_PHY_SWITCH_COM
+ * @name: profile name
+ * @desc: profile description
+ */
+struct ath_hw_switch_com_profile {
+	int id;
+	u32 switch_com_value;
+	char *name;
+	char *desc;
+};
+
 /**
  * struct ath_hw_private_ops - callbacks used internally by hardware code
  *
@@ -876,6 +895,16 @@ struct ath_hw {
 	bool is_clk_25mhz;
 	int (*get_mac_revision)(void);
 	int (*external_reset)(void);
+
+	/*
+	 * External antenna switch configuration profiles supplied via
+	 * platform_data.
+	 *
+	 * Always make sure there is a matching profile for the default
+	 * value of AR_PHY_SWITCH_COM (read from eeprom field antCtrlCommon)
+	 * or don't have any profiles at all if only the default value is used.
+	 */
+	struct ath_hw_switch_com_profile *switch_com_profiles;
 };
 
 struct ath_bus_ops {
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -525,7 +525,7 @@ static int ath9k_init_softc(u16 devid, s
 	struct ath9k_platform_data *pdata = sc->dev->platform_data;
 	struct ath_hw *ah = NULL;
 	struct ath_common *common;
-	int ret = 0, i;
+	int ret = 0, i, j;
 	int csz = 0;
 
 	ah = kzalloc(sizeof(struct ath_hw), GFP_KERNEL);
@@ -550,6 +550,33 @@ static int ath9k_init_softc(u16 devid, s
 		ah->is_clk_25mhz = pdata->is_clk_25mhz;
 		ah->get_mac_revision = pdata->get_mac_revision;
 		ah->external_reset = pdata->external_reset;
+		/* count valid profiles in platform data */
+		j=0;
+		for(i=0;i<ATH9K_PLAT_MAX_SWITCHCOM_PROFILES;i++) {
+			if ( pdata->switchcom_profiles[i].name )
+				j++;
+		}
+
+		/* allocate space for the j profiles we found + 1 to terminate the array */
+		if ( j>0 && ! ah->switch_com_profiles ) {
+			ah->switch_com_profiles = kcalloc(j+1, sizeof(struct ath_hw_switch_com_profile), GFP_KERNEL);
+			if ( ! ah->switch_com_profiles )
+				return -ENOMEM;
+		}
+
+		/* populate the space from platform_data */
+		j=0;
+		for(i=0;i<ATH9K_PLAT_MAX_SWITCHCOM_PROFILES;i++) {
+			if ( pdata->switchcom_profiles[i].name ) {
+				ah->switch_com_profiles[j].id = i;
+				ah->switch_com_profiles[j].switch_com_value = pdata->switchcom_profiles[i].val;
+				ah->switch_com_profiles[j].name = pdata->switchcom_profiles[i].name;
+				ah->switch_com_profiles[j].desc = pdata->switchcom_profiles[i].desc;
+				j++;
+			}
+		}
+		if ( j>0 )
+			ah->switch_com_profiles[j].id = -1;
 	}
 
 	common = ath9k_hw_common(ah);
@@ -666,6 +693,7 @@ void ath9k_set_hw_capab(struct ath_softc
 {
 	struct ath_hw *ah = sc->sc_ah;
 	struct ath_common *common = ath9k_hw_common(ah);
+	int i;
 
 	hw->flags = IEEE80211_HW_RX_INCLUDES_FCS |
 		IEEE80211_HW_HOST_BROADCAST_PS_BUFFERING |
@@ -713,6 +741,29 @@ void ath9k_set_hw_capab(struct ath_softc
 	sc->ant_rx = hw->wiphy->available_antennas_rx;
 	sc->ant_tx = hw->wiphy->available_antennas_tx;
 
+	if (ah->switch_com_profiles) {
+		hw->wiphy->flags |= WIPHY_FLAG_HAS_EXTANT_SWITCH;
+
+		/* allocate profile structure in wiphy for all available profiles + 1 */
+		for (i=0; ah->switch_com_profiles[i].id >= 0; i++);
+		hw->wiphy->extant = kcalloc(i+1, sizeof(struct wiphy_extant_profile), GFP_KERNEL);
+
+		// if ( ! hw->wiphy->extant )
+			// return -ENOMEM; can't do that, maybe preallocate n profiles in advance?
+
+		/* populate wiphy->exant structure */
+		for (i=0; ah->switch_com_profiles[i].id >= 0; i++) {
+			hw->wiphy->extant[i].id = ah->switch_com_profiles[i].id;
+			hw->wiphy->extant[i].name = ah->switch_com_profiles[i].name;
+			hw->wiphy->extant[i].desc = ah->switch_com_profiles[i].desc;
+			ath_dbg(common, ATH_DBG_CONFIG,
+				"available switchcom profile %u (%s)\n",
+				hw->wiphy->extant[i].id, hw->wiphy->extant[i].name);
+		}
+		/* terminate with dummy */
+		hw->wiphy->extant[i].id = -1;
+	}
+
 #ifdef CONFIG_ATH9K_RATE_CONTROL
 	hw->rate_control_algorithm = "ath9k_rate_control";
 #endif
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -2421,6 +2421,57 @@ static int ath9k_get_antenna(struct ieee
 	return 0;
 }
 
+static int ath9k_set_extant(struct ieee80211_hw *hw, u32 extant)
+{
+	struct ath_softc *sc = hw->priv;
+	struct ath_hw *ah = sc->sc_ah;
+	struct ath_hw_switch_com_profile *easp;
+	u32 switch_com_value = 0;
+
+	if (!ah->switch_com_profiles)
+		return -EINVAL;
+
+	/* find matching value for profile id */
+	for(easp = ah->switch_com_profiles;easp->id >= 0; easp++) {
+		if ( easp->id == extant ) {
+			switch_com_value = easp->switch_com_value;
+			break;
+		}
+	}
+	if (!switch_com_value)
+		return -EINVAL;
+
+	ah->eep_ops->set_switch_com(ah, switch_com_value);
+
+	return 0;
+}
+
+static int ath9k_get_extant(struct ieee80211_hw *hw, u32 *extant)
+{
+	struct ath_softc *sc = hw->priv;
+	struct ath_hw *ah = sc->sc_ah;
+	struct ath_hw_switch_com_profile *easp;
+	u32 switch_com_value;
+	int found = 0;
+
+	if (!ah->switch_com_profiles)
+		return -EINVAL;
+
+	switch_com_value = ah->eep_ops->get_switch_com(ah);
+
+	/* find matching profile id for value */
+	for(easp = ah->switch_com_profiles;easp->id >= 0; easp++) {
+		if ( easp->switch_com_value == switch_com_value ) {
+			*extant = easp->id;
+			found = 1;
+		}
+	}
+	if ( ! found )
+		return -EINVAL; // really shouldn't happen
+
+	return 0;
+}
+
 struct ieee80211_ops ath9k_ops = {
 	.tx 		    = ath9k_tx,
 	.start 		    = ath9k_start,
@@ -2449,4 +2500,6 @@ struct ieee80211_ops ath9k_ops = {
 	.get_stats	    = ath9k_get_stats,
 	.set_antenna	    = ath9k_set_antenna,
 	.get_antenna	    = ath9k_get_antenna,
+	.set_extant	    = ath9k_set_extant,
+	.get_extant	    = ath9k_get_extant,
 };

[ath9k-devel] [PATCH v0 3/6] support for antenna configuration profiles

[Adrian Chadd]

Hi,

I'd personally suggest not overriding the pModal EEPROM entry with
your currently configured antenna profile.
You have a perfectly good accessor method, why not use that?


Adrian

[ath9k-devel] [PATCH v0 3/6] support for antenna configuration profiles

[Daniel Golle]

On 11/14/2011 03:44 PM, Adrian Chadd wrote
> I'd personally suggest not overriding the pModal EEPROM entry with
> your currently configured antenna profile.
> You have a perfectly good accessor method, why not use that?
As far as I can see the value stored in pModal->antCtrlCommon isn't actually
used anywhere besides in ath9k_hw_4k_set_board_values, which is only called once
during the init on module load. So not changing it at all and only setting the
AR_PHY_SWITCH_COM register should be fine. Agree?

[ath9k-devel] [PATCH v0 3/6] support for antenna configuration profiles

[Adrian Chadd]

On 16 November 2011 06:29, Daniel Golle <dgolle@allnet.de> wrote:
> On 11/14/2011 03:44 PM, Adrian Chadd wrote
>> I'd personally suggest not overriding the pModal EEPROM entry with
>> your currently configured antenna profile.
>> You have a perfectly good accessor method, why not use that?
> As far as I can see the value stored in pModal->antCtrlCommon isn't actually
> used anywhere besides in ath9k_hw_4k_set_board_values, which is only called once
> during the init on module load. So not changing it at all and only setting the
> AR_PHY_SWITCH_COM register should be fine. Agree?

I still think that treating the modal data as read-only is the cleaner
solution. Right now that's how it's treated and it makes things like
EEPROM dumps (which you can get via the debug interface) consistent.

I suggest doing this for other reasons - in particular, I've seen an
AR9220 board here with an external antenna switch and two 16-bit
antenna configuration words. The "antenna diversity" algorithm (which
I think is a cross between classic fast-diversity, slow-diversity and
multi-chain diversity) then chooses which antenna profile to use. If
you create a method which returns the antenna common bits then we can
use exactly the same method for what I've just described as well as
your external antenna control bitmap.

Thanks,



Adrian