[ath9k-devel] RFC: expose antCtrlCommon aka. AR_PHY_SWITCH_COM via nl80211

[ath9k-devel] RFC: expose antCtrlCommon aka. AR_PHY_SWITCH_COM via nl80211

[Daniel Golle]

Hi everybody!
The ALL0258N (AR7420 + AR9285) uses an external antenna switch which is
controlled by setting values into the AR_PHY_SWITCH_COM register.
Thinking about how to implement this in ath9k on OpenWrt, my solution would be
to pass an array of allowed values via platform-data and then expose
get_switchcom, set_switchcom and get_available_switchcom via nl80211.
Is that the way to do it?
Please comment.

Cheers

Daniel

[ath9k-devel] RFC: expose antCtrlCommon aka. AR_PHY_SWITCH_COM via nl80211

[Adrian Chadd]

On 2 November 2011 10:14, Daniel Golle <dgolle@allnet.de> wrote:
> Hi everybody!
> The ALL0258N (AR7420 + AR9285) uses an external antenna switch which is
> controlled by setting values into the AR_PHY_SWITCH_COM register.
> Thinking about how to implement this in ath9k on OpenWrt, my solution would be
> to pass an array of allowed values via platform-data and then expose
> get_switchcom, set_switchcom and get_available_switchcom via nl80211.
> Is that the way to do it?


How's the antenna switch hooked up?
Ie, what are the permissable values?

There's an antenna switch field in the EEPROM, so I take it that you
require multiple "valid" values? What/why would you choose between
them?


Adrian

[ath9k-devel] RFC: expose antCtrlCommon aka. AR_PHY_SWITCH_COM via nl80211

[Daniel Golle]

Hi Adrian,

On 11/02/2011 06:22 PM, Adrian Chadd wrote:
> There's an antenna switch field in the EEPROM, so I take it that you
> require multiple "valid" values? What/why would you choose between
> them?
according to the OEM it's allowed to set 0x09a00210 for horizontal or 0x0a900120
for vertical. I assume that the TX-antenna is meant by that, i.e. 0x09a00210
means horizontal-TX/vertical-RX.

[ath9k-devel] RFC: expose antCtrlCommon aka. AR_PHY_SWITCH_COM via nl80211

[Felix Fietkau]

On 2011-11-02 6:49 PM, Daniel Golle wrote:
> Hi Adrian,
>
> On 11/02/2011 06:22 PM, Adrian Chadd wrote:
>>  There's an antenna switch field in the EEPROM, so I take it that you
>>  require multiple "valid" values? What/why would you choose between
>>  them?
> according to the OEM it's allowed to set 0x09a00210 for horizontal or 0x0a900120
> for vertical. I assume that the TX-antenna is meant by that, i.e. 0x09a00210
> means horizontal-TX/vertical-RX.
>  From what i understand, misconfiguration could damage the RX-part of the card,
> so the user should only be allowed to change the value to anything else than
> what is stored in the eeprom if we know it's a valid setting.
>
> As far as I understand the EEPROM only stores a single value for the
> antenna-switch setting, i.e. if there multiple valid values these must come from
> somewhere else.
>
> I don't know the exact electronic details of the implementation, so I'd go with
> a list of known-to-be-good values (passed to the driver via platform-data)
> instead of a causal check (e.g. don't allow to set the TX and RX to the same
> antenna or whatever you imagine to possibly be an invalid setting).
I think passing such 'magic' values from user space to the driver is a 
bad idea. The abstraction should happen within the driver and exposed 
via the antenna control API.

- Felix

[ath9k-devel] RFC: expose antCtrlCommon aka. AR_PHY_SWITCH_COM via nl80211

[Adrian Chadd]

On 2 November 2011 10:52, Felix Fietkau <nbd@openwrt.org> wrote:

>> I don't know the exact electronic details of the implementation, so I'd go with
>> a list of known-to-be-good values (passed to the driver via platform-data)
>> instead of a causal check (e.g. don't allow to set the TX and RX to the same
>> antenna or whatever you imagine to possibly be an invalid setting).

> I think passing such 'magic' values from user space to the driver is a
> bad idea. The abstraction should happen within the driver and exposed
> via the antenna control API.

+1. I'll see if we can document what those bits are, but yes, please
pass them in via platform_data in the board setup code (ie, all
in-kernel) and then let the user select from a list of valid ones.

+1 if you add a user-parsable tag and description  to the list of values, eg:

{ 0x1, "vert", "vertical RX" },
{ 0x2, "horiz", "horizontal RX" },
{ -1, NULL, NULL}


Adrian

[ath9k-devel] RFC: expose antCtrlCommon aka. AR_PHY_SWITCH_COM via nl80211

[Daniel Golle]

On 11/02/2011 06:52 PM, Felix Fietkau wrote:
>> I don't know the exact electronic details of the implementation, so I'd go with
>> a list of known-to-be-good values (passed to the driver via platform-data)
>> instead of a causal check (e.g. don't allow to set the TX and RX to the same
>> antenna or whatever you imagine to possibly be an invalid setting).
> I think passing such 'magic' values from user space to the driver is a
> bad idea. The abstraction should happen within the driver and exposed 
> via the antenna control API.
I agree that this should be abstracted away from the user.
You reckon the user should set the values via
iw phy phy0 set antenna 1 2
 -> set (0x09a0,0x0210)
and
iw phy phy0 set antenna 2 1
 -> set (0x0a90,0x0120)
and let the driver know that only (1,2) and (2,1) are valid settings?

it'd be nice to (additionally?) implement
iw phy phy0 set antenna horizontal

i would then imagine something like
struct ath9k_platform_switchcom_profile {
	char *name;
	u16 tx_antenna;
	u16 rx_antenna;
}

and extend ath9k_platform_data by
u8 um_switchcom_profiles;
ath9k_platform_switchcom_profile *profiles;

As available_anntenas_rx and available_anntenas_tx are just bitmasks for now,
this first of all requires a way to handle the information about available
antenna-combinations in struct wiphy. (right?)


Cheers

Daniel

[ath9k-devel] RFC: expose antCtrlCommon aka. AR_PHY_SWITCH_COM via nl80211

[Felix Fietkau]

On 2011-11-02 7:16 PM, Daniel Golle wrote:
> On 11/02/2011 06:52 PM, Felix Fietkau wrote:
>>>  I don't know the exact electronic details of the implementation, so I'd go with
>>>  a list of known-to-be-good values (passed to the driver via platform-data)
>>>  instead of a causal check (e.g. don't allow to set the TX and RX to the same
>>>  antenna or whatever you imagine to possibly be an invalid setting).
>>  I think passing such 'magic' values from user space to the driver is a
>>  bad idea. The abstraction should happen within the driver and exposed
>>  via the antenna control API.
> I agree that this should be abstracted away from the user.
> You reckon the user should set the values via
> iw phy phy0 set antenna 1 2
>   ->  set (0x09a0,0x0210)
> and
> iw phy phy0 set antenna 2 1
>   ->  set (0x0a90,0x0120)
> and let the driver know that only (1,2) and (2,1) are valid settings?
>
> it'd be nice to (additionally?) implement
> iw phy phy0 set antenna horizontal
>
> i would then imagine something like
> struct ath9k_platform_switchcom_profile {
> 	char *name;
> 	u16 tx_antenna;
> 	u16 rx_antenna;
> }
>
> and extend ath9k_platform_data by
> u8 um_switchcom_profiles;
> ath9k_platform_switchcom_profile *profiles;
>
> As available_anntenas_rx and available_anntenas_tx are just bitmasks for now,
> this first of all requires a way to handle the information about available
> antenna-combinations in struct wiphy. (right?)
The chip has 4 external bits which can be used to drive an antenna 
switch. I'm currently looking at the details of the AR_PHY_SWITCH_COM 
register - it controls the values of those 4 bits based on its internal 
antenna selection state.

Idle:			0x0000000f
Transmit ant1:		0x000000f0
Transmit ant2:		0x00000f00
Receive ant1, LNA1:	0x0000f000
Receive ant2, LNA1:	0x000f0000
Receive ant1, LNA2:	0x00f00000
Receive ant2, LNA2:	0x0f000000
Receive ant1+ant2:	0xf0000000

So the difference between those two AR_PHY_SWITCH_COM values is that 
they swap the antenna switch bits for antenna1 vs antenna2 (other 
settings and combinations seem to be undefined).

To figure out the remaining settings for this hardware, please give me 
the values of registers 0x99ac (AR_PHY_MULTICHAIN_GAIN_CTL) and 0xA208 
(AR_PHY_CCK_DETECT) on your device.

- Felix

[ath9k-devel] RFC: expose antCtrlCommon aka. AR_PHY_SWITCH_COM via nl80211

[Daniel Golle]

Hi Felix!

Thank you very much for that info!

On 11/02/2011 08:50 PM, Felix Fietkau wrote:
> To figure out the remaining settings for this hardware, please give me 
> the values of registers 0x99ac (AR_PHY_MULTICHAIN_GAIN_CTL) and 0xA208 
> (AR_PHY_CCK_DETECT) on your device.
from debugfs regdump:
0x99ac (AR_PHY_MULTICHAIN_GAIN_CTL)  0x1a63dfcd
0xA208 (AR_PHY_CCK_DETECT)           0x9a6bffcd


Cheers

Daniel