[ath9k-devel] [RFC v3] ath9k: add devicetree support to ath9k

[ath9k-devel] [RFC v3] ath9k: add devicetree support to ath9k

[Martin Blumenstingl]

This series adds support for configuring ath9k based devices via
devicetree. This was tested on PCI(e) based devices. This should work
for AHB based devices as well as soon as the ath79 platform is ready
to populate the ath9k wmac via devicetree.

This series depends on my previous series:
"ath9k: extend and improve handling of ath9k_platform_data"

changes in v1 -> v2:
This addresses all issues found by Christian Lamparter (thanks!), the
detailed changes are documented in each patch.

changes in v2 -> v3:
The qca,eeprom-name was property was replaced by qca,no-eeprom, which
moves the filename for the eeprom firmware from the devicetree to ath9k
(where the filename is now auto-generated). Additionally this contains
fixes for the documentation (some properties were missing, an invalid
property in the example, etc.). The detailed changes are documented in
each patch.

[ath9k-devel] [PATCH RFC v3 1/3] Documentation: dt: net: add ath9k wireless device binding

[Martin Blumenstingl]

Add documentation how devicetree can be used to configure ath9k based
devices.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
changes in v2 -> v3:
- improved wording of the qca,disable-2ghz and qca,disable-5ghz properties
- replaced qca,eeprom-name with qca,no-eeprom (the eeprom name is now
  built automatically within ath9k). The naming was chosen to stay
  consistent with other drivers (such as davicom-dm9000 and via-velocity)
- added reference to the mac-address and local-mac-address properties
- removed invalid device_type property from the example

 .../devicetree/bindings/net/wireless/qca,ath9k.txt | 46 ++++++++++++++++++++++
 1 file changed, 46 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt

diff --git a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
new file mode 100644
index 0000000..ff83fd4
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
@@ -0,0 +1,46 @@
+* Qualcomm Atheros ath9k wireless devices
+
+This node provides properties for configuring the ath9k wireless device. The
+node is expected to be specified as a child node of the PCI controller to
+which the wireless chip is connected.
+
+Required properties:
+- compatible: Should be "qca,ath9k"
+
+Optional properties:
+- reg: Address and length of the register set for the device.
+- qca,gpio-mask: The GPIO mask
+- qca,gpio-val: The GPIO value
+- qca,led-pin: The GPIO number to which the LED is connected
+- qca,led-active-high: The LED is active when the GPIO is HIGH
+- qca,clk-25mhz: Defines that at 25MHz clock is used
+- qca,no-eeprom: Indicates that there is on physical EEPROM connected
+			to the ath9k wireless chip (in this case the
+			calibration / EEPROM data will be loaded from
+			userspace using the firmware mechanism)
+- qca,check-eeprom-endianness: Allow checking the EEPROM endianness and
+				swapping of the EEPROM data if required
+- qca,disable-2ghz: Overrides the settings from the EEPROM and disables
+			the 2.4GHz band, even if enabled in the EEPROM
+- qca,disable-5ghz: Overrides the settings from the EEPROM and disables
+			the 5GHz band, even if enabled in the EEPROM
+- mac-address: See ethernet.txt in the parent directory
+- local-mac-address: See ethernet.txt in the parent directory
+
+In this example, the node is defined as child node of the PCI controller.
+
+pci {
+	pcie at 0 {
+		reg = <0 0 0 0 0>;
+		#interrupt-cells = <1>;
+		#size-cells = <2>;
+		#address-cells = <3>;
+		device_type = "pci";
+
+		ath9k at 0,0 {
+			compatible = "qca,ath9k";
+			reg = <0 0 0 0 0>;
+			qca,disable-5ghz;
+		};
+	};
+};
-- 
2.9.0

[ath9k-devel] [PATCH RFC v3 1/3] Documentation: dt: net: add ath9k wireless device binding

[Julian Calaby]

Hi Martin,

On Fri, Jun 24, 2016 at 10:34 PM, Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
> Add documentation how devicetree can be used to configure ath9k based
> devices.
>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
> changes in v2 -> v3:
> - improved wording of the qca,disable-2ghz and qca,disable-5ghz properties
> - replaced qca,eeprom-name with qca,no-eeprom (the eeprom name is now
>   built automatically within ath9k). The naming was chosen to stay
>   consistent with other drivers (such as davicom-dm9000 and via-velocity)
> - added reference to the mac-address and local-mac-address properties
> - removed invalid device_type property from the example
>
>  .../devicetree/bindings/net/wireless/qca,ath9k.txt | 46 ++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
>
> diff --git a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
> new file mode 100644
> index 0000000..ff83fd4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
> @@ -0,0 +1,46 @@
> +* Qualcomm Atheros ath9k wireless devices
> +
> +This node provides properties for configuring the ath9k wireless device. The
> +node is expected to be specified as a child node of the PCI controller to
> +which the wireless chip is connected.
> +
> +Required properties:
> +- compatible: Should be "qca,ath9k"
> +
> +Optional properties:
> +- reg: Address and length of the register set for the device.
> +- qca,gpio-mask: The GPIO mask
> +- qca,gpio-val: The GPIO value
> +- qca,led-pin: The GPIO number to which the LED is connected
> +- qca,led-active-high: The LED is active when the GPIO is HIGH
> +- qca,clk-25mhz: Defines that at 25MHz clock is used
> +- qca,no-eeprom: Indicates that there is on physical EEPROM connected

Typo: ...Indicates that there is _no_ physical EEPROM...

Thanks,

-- 
Julian Calaby

Email: julian.calaby at gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

[ath9k-devel] [PATCH RFC v3 1/3] Documentation: dt: net: add ath9k wireless device binding

[Kalle Valo]

Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:

> Add documentation how devicetree can be used to configure ath9k based
> devices.
>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
> changes in v2 -> v3:
> - improved wording of the qca,disable-2ghz and qca,disable-5ghz properties
> - replaced qca,eeprom-name with qca,no-eeprom (the eeprom name is now
>   built automatically within ath9k). The naming was chosen to stay
>   consistent with other drivers (such as davicom-dm9000 and via-velocity)
> - added reference to the mac-address and local-mac-address properties
> - removed invalid device_type property from the example
>
>  .../devicetree/bindings/net/wireless/qca,ath9k.txt | 46 ++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt

For device tree bindings document changes you should CC the device tree
list:

$ scripts/get_maintainer.pl -f Documentation/devicetree/bindings/net/wireless/
Kalle Valo <kvalo@codeaurora.org> (maintainer:NETWORKING DRIVERS (WIRELESS))
Rob Herring <robh+dt@kernel.org> (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS)
Mark Rutland <mark.rutland@arm.com> (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS)
linux-wireless at vger.kernel.org (open list:NETWORKING DRIVERS (WIRELESS))
netdev at vger.kernel.org (open list:NETWORKING DRIVERS)
devicetree at vger.kernel.org (open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS)
linux-kernel at vger.kernel.org (open list)

-- 
Kalle Valo

[ath9k-devel] [PATCH v3 2/3] ath9k: add a helper to get the string representation of ath_bus_type

[Martin Blumenstingl]

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
this is a new patch which didn't exist in v2 yet, it prepares the new
function ath_bus_type_to_string which will be used in patch #3

 drivers/net/wireless/ath/ath.h  |  2 ++
 drivers/net/wireless/ath/main.c | 15 +++++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
index da7a7c8..be0d292 100644
--- a/drivers/net/wireless/ath/ath.h
+++ b/drivers/net/wireless/ath/ath.h
@@ -327,4 +327,6 @@ static inline const char *ath_opmode_to_string(enum nl80211_iftype opmode)
 }
 #endif
 
+const char *ath_bus_type_to_string(enum ath_bus_type bustype);
+
 #endif /* ATH_H */
diff --git a/drivers/net/wireless/ath/main.c b/drivers/net/wireless/ath/main.c
index 338d723..90427cb 100644
--- a/drivers/net/wireless/ath/main.c
+++ b/drivers/net/wireless/ath/main.c
@@ -90,3 +90,18 @@ void ath_printk(const char *level, const struct ath_common* common,
 	va_end(args);
 }
 EXPORT_SYMBOL(ath_printk);
+
+const char *ath_bus_type_to_string(enum ath_bus_type bustype)
+{
+	switch (bustype) {
+	case ATH_PCI:
+		return "pci";
+	case ATH_AHB:
+		return "ahb";
+	case ATH_USB:
+		return "usb";
+	default:
+		return "unknown";
+	}
+}
+EXPORT_SYMBOL(ath_bus_type_to_string);
-- 
2.9.0

[ath9k-devel] [PATCH v3 2/3] ath9k: add a helper to get the string representation of ath_bus_type

[Felix Fietkau]

On 2016-06-24 14:34, Martin Blumenstingl wrote:
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
> this is a new patch which didn't exist in v2 yet, it prepares the new
> function ath_bus_type_to_string which will be used in patch #3
> 
>  drivers/net/wireless/ath/ath.h  |  2 ++
>  drivers/net/wireless/ath/main.c | 15 +++++++++++++++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
> index da7a7c8..be0d292 100644
> --- a/drivers/net/wireless/ath/ath.h
> +++ b/drivers/net/wireless/ath/ath.h
> @@ -327,4 +327,6 @@ static inline const char *ath_opmode_to_string(enum nl80211_iftype opmode)
>  }
>  #endif
>  
> +const char *ath_bus_type_to_string(enum ath_bus_type bustype);
> +
>  #endif /* ATH_H */
> diff --git a/drivers/net/wireless/ath/main.c b/drivers/net/wireless/ath/main.c
> index 338d723..90427cb 100644
> --- a/drivers/net/wireless/ath/main.c
> +++ b/drivers/net/wireless/ath/main.c
> @@ -90,3 +90,18 @@ void ath_printk(const char *level, const struct ath_common* common,
>  	va_end(args);
>  }
>  EXPORT_SYMBOL(ath_printk);
> +
> +const char *ath_bus_type_to_string(enum ath_bus_type bustype)
> +{
> +	switch (bustype) {
> +	case ATH_PCI:
> +		return "pci";
> +	case ATH_AHB:
> +		return "ahb";
> +	case ATH_USB:
> +		return "usb";
> +	default:
> +		return "unknown";
> +	}
> +}
> +EXPORT_SYMBOL(ath_bus_type_to_string);
Why not simply add a const char* array with ath_bus_type as index?

- Felix

[ath9k-devel] [PATCH v3 2/3] ath9k: add a helper to get the string representation of ath_bus_type

[Martin Blumenstingl]

On Mon, Jun 27, 2016 at 12:26 PM, Felix Fietkau <nbd@nbd.name> wrote:
> On 2016-06-24 14:34, Martin Blumenstingl wrote:
>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>> ---
>> this is a new patch which didn't exist in v2 yet, it prepares the new
>> function ath_bus_type_to_string which will be used in patch #3
>>
>>  drivers/net/wireless/ath/ath.h  |  2 ++
>>  drivers/net/wireless/ath/main.c | 15 +++++++++++++++
>>  2 files changed, 17 insertions(+)
>>
>> diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
>> index da7a7c8..be0d292 100644
>> --- a/drivers/net/wireless/ath/ath.h
>> +++ b/drivers/net/wireless/ath/ath.h
>> @@ -327,4 +327,6 @@ static inline const char *ath_opmode_to_string(enum nl80211_iftype opmode)
>>  }
>>  #endif
>>
>> +const char *ath_bus_type_to_string(enum ath_bus_type bustype);
>> +
>>  #endif /* ATH_H */
>> diff --git a/drivers/net/wireless/ath/main.c b/drivers/net/wireless/ath/main.c
>> index 338d723..90427cb 100644
>> --- a/drivers/net/wireless/ath/main.c
>> +++ b/drivers/net/wireless/ath/main.c
>> @@ -90,3 +90,18 @@ void ath_printk(const char *level, const struct ath_common* common,
>>       va_end(args);
>>  }
>>  EXPORT_SYMBOL(ath_printk);
>> +
>> +const char *ath_bus_type_to_string(enum ath_bus_type bustype)
>> +{
>> +     switch (bustype) {
>> +     case ATH_PCI:
>> +             return "pci";
>> +     case ATH_AHB:
>> +             return "ahb";
>> +     case ATH_USB:
>> +             return "usb";
>> +     default:
>> +             return "unknown";
>> +     }
>> +}
>> +EXPORT_SYMBOL(ath_bus_type_to_string);
> Why not simply add a const char* array with ath_bus_type as index?
Would you add this const char* array directly in ath.h or would you
define it as extern there and set it in main.c?
I would still provide the ath_bus_type_to_string function, but make it
static inline in that case.

[ath9k-devel] [PATCH RFC v3 3/3] ath9k: parse the device configuration from an OF node

[Martin Blumenstingl]

This makes it possible to configure ath9k based devices using
devicetree. That makes some out-of-tree "convert devicetree to
ath9k_platform_data glue"-code obsolete.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
changes in v2 -> v3:
- replaced qca,eeprom-name with a boolean "qca,no-eeprom". The name of the
  eeprom firmware file is now generated based on the following pattern:
  ath9k-eeprom-<bus>-<id>.bin

 drivers/net/wireless/ath/ath9k/init.c | 68 +++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index a0f4a52..9521bc8 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -20,6 +20,8 @@
 #include <linux/slab.h>
 #include <linux/ath9k_platform.h>
 #include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_net.h>
 #include <linux/relay.h>
 #include <net/ieee80211_radiotap.h>
 
@@ -555,6 +557,68 @@ static int ath9k_init_platform(struct ath_softc *sc)
 	return 0;
 }
 
+static int ath9k_of_init(struct ath_softc *sc)
+{
+	struct device_node *np = sc->dev->of_node;
+	struct ath_hw *ah = sc->sc_ah;
+	struct ath_common *common = ath9k_hw_common(ah);
+	enum ath_bus_type bus_type = common->bus_ops->ath_bus_type;
+	const char *mac;
+	char eeprom_name[100];
+	int led_pin, ret;
+	u32 gpio_data;
+
+	if (!of_device_is_available(np))
+		return 0;
+
+	ath_dbg(common, CONFIG, "parsing configuration from OF node\n");
+
+	if (!of_property_read_u32(np, "qca,led-pin", &led_pin))
+		ah->led_pin = led_pin;
+
+	if (!of_property_read_u32(np, "qca,gpio-mask", &gpio_data))
+		ah->gpio_mask = gpio_data;
+
+	if (!of_property_read_u32(np, "qca,gpio-val", &gpio_data))
+		ah->gpio_val = gpio_data;
+
+	if (of_property_read_bool(np, "qca,clk-25mhz"))
+		ah->is_clk_25mhz = true;
+
+	if (of_property_read_bool(np, "qca,led-active-high"))
+		ah->config.led_active_high = true;
+
+	if (of_property_read_bool(np, "qca,disable-2ghz"))
+		ah->disable_2ghz = true;
+
+	if (of_property_read_bool(np, "qca,disable-5ghz"))
+		ah->disable_5ghz = true;
+
+	if (of_property_read_bool(np, "qca,check-eeprom-endianness"))
+		ah->ah_flags &= ~AH_NO_EEP_SWAP;
+	else
+		ah->ah_flags |= AH_NO_EEP_SWAP;
+
+	if (of_property_read_bool(np, "qca,no-eeprom")) {
+		/* ath9k-eeprom-<bus>-<id>.bin */
+		scnprintf(eeprom_name, sizeof(eeprom_name),
+			  "ath9k-eeprom-%s-%s.bin",
+			  ath_bus_type_to_string(bus_type), dev_name(ah->dev));
+
+		ret = ath9k_eeprom_request(sc, eeprom_name);
+		if (ret)
+			return ret;
+	}
+
+	mac = of_get_mac_address(np);
+	if (mac)
+		ether_addr_copy(common->macaddr, mac);
+
+	ah->ah_flags &= ~AH_USE_EEPROM;
+
+	return 0;
+}
+
 static int ath9k_init_softc(u16 devid, struct ath_softc *sc,
 			    const struct ath_bus_ops *bus_ops)
 {
@@ -611,6 +675,10 @@ static int ath9k_init_softc(u16 devid, struct ath_softc *sc,
 	if (ret)
 		return ret;
 
+	ret = ath9k_of_init(sc);
+	if (ret)
+		return ret;
+
 	if (ath9k_led_active_high != -1)
 		ah->config.led_active_high = ath9k_led_active_high == 1;
 
-- 
2.9.0

[ath9k-devel] [PATCH RFC v3 3/3] ath9k: parse the device configuration from an OF node

[Christian Lamparter]

On Friday, June 24, 2016 02:34:30 PM Martin Blumenstingl wrote:
> This makes it possible to configure ath9k based devices using
> devicetree. That makes some out-of-tree "convert devicetree to
> ath9k_platform_data glue"-code obsolete.

Hm, what about the embedded ath9k pcie chips that need the early
pci-fixup routine for the device to work properly [0], [1]? How
will this be handled/integrated? I know that the ar71xx and the
lantiq platforms use similar pci-fixup routines that need a few
bytes from the eeprom/cal data. So lantiq has a few extra properties:
"ath,pci-slot", "ath,device-id" and "ath,eep-flash". 

As an example: the AR9280 in the Cisco Z1 AP is initially 
0x168c:0xff1f (<-- ath9k doesn't know about that id). The
pci-fixup routine will change it to  0x168c:0x002A. Only 
then ath9k can take it over and will initialize it.
Thing is: this is all currently done by platform code for
those architectures... And currently, the request_firmware
doesn't work for caldata on UBI partitions.

Regards,
Christian

[0] <https://git.lede-project.org/?p=source.git;a=blob;f=target/linux/ar71xx/files/arch/mips/ath79/pci-ath9k-fixup.c;h=22023518069bc9b15b898f12f342b89563358e6a;hb=HEAD>

[1] <https://dev.openwrt.org/browser/trunk/target/linux/lantiq/patches-4.4/0035-owrt-lantiq-wifi-and-ethernet-eeprom-handling.patch>

[ath9k-devel] [PATCH RFC v3 3/3] ath9k: parse the device configuration from an OF node

[Martin Blumenstingl]

On Sat, Jun 25, 2016 at 2:01 PM, Christian Lamparter
<chunkeey@googlemail.com> wrote:
> On Friday, June 24, 2016 02:34:30 PM Martin Blumenstingl wrote:
>> This makes it possible to configure ath9k based devices using
>> devicetree. That makes some out-of-tree "convert devicetree to
>> ath9k_platform_data glue"-code obsolete.
>
> Hm, what about the embedded ath9k pcie chips that need the early
> pci-fixup routine for the device to work properly [0], [1]? How
> will this be handled/integrated? I know that the ar71xx and the
> lantiq platforms use similar pci-fixup routines that need a few
> bytes from the eeprom/cal data. So lantiq has a few extra properties:
> "ath,pci-slot", "ath,device-id" and "ath,eep-flash".
that is exactly the use-case I want to use your owl-loader for (see
[0], it's a small kernel module which adds the PCI configuration for
ath9k devices).
This makes ath,pci-slot and ath,eep-flash obsolete.
As far as I'm aware ath,device-id is a bit of a special case (mtd_read
issues when the caldata is stored at an unaligned position on NOR
flash). So this might be obsolete as well when using owl-loader.

> As an example: the AR9280 in the Cisco Z1 AP is initially
> 0x168c:0xff1f (<-- ath9k doesn't know about that id). The
> pci-fixup routine will change it to  0x168c:0x002A. Only
> then ath9k can take it over and will initialize it.
> Thing is: this is all currently done by platform code for
> those architectures... And currently, the request_firmware
> doesn't work for caldata on UBI partitions.
request_firmware is working on UBI partitions in many cases.
It's just not working when request_firmware is called too early (and
this is not UBI specific, other filesystems might be affected as
well): if it is called before rootfs is mounted (which is the case
when you call it from a PCI fixup function) then it's not working
(like you said).
The "solution" to this is to compile the driver as kernel module (once
this is loadable everything else should be readable as well).
Not only ath9k is affected by this "issue", this is simply a
limitation of request_firmware and/or the linux boot chain.

A few words regarding your owl-loader:
First of all I would like to say "thank you"!
Mathias and I are working on changing the lantiq target in LEDE to use
owl-loader for all (ath9k) devices.
All I had to do was to add another OWL PCI ID, implement a fallback
for the firmware filename when there is no ath9k_platform_data (I'm
using the same pattern as in PATCH 3/3 in this series). You can find
the WIP code here: [1]


Regards,
Martin


[0] https://patchwork.ozlabs.org/patch/607682/
[1] https://github.com/xdarklight/source/commits/ath9k-owl-loader-20160624

[ath9k-devel] [PATCH RFC v3 3/3] ath9k: parse the device configuration from an OF node

[Christian Lamparter]

On Saturday, June 25, 2016 05:08:29 PM Martin Blumenstingl wrote:
> On Sat, Jun 25, 2016 at 2:01 PM, Christian Lamparter
> <chunkeey@googlemail.com> wrote:
> > On Friday, June 24, 2016 02:34:30 PM Martin Blumenstingl wrote:
> >> This makes it possible to configure ath9k based devices using
> >> devicetree. That makes some out-of-tree "convert devicetree to
> >> ath9k_platform_data glue"-code obsolete.
> >
> > Hm, what about the embedded ath9k pcie chips that need the early
> > pci-fixup routine for the device to work properly [0], [1]? How
> > will this be handled/integrated? I know that the ar71xx and the
> > lantiq platforms use similar pci-fixup routines that need a few
> > bytes from the eeprom/cal data. So lantiq has a few extra properties:
> > "ath,pci-slot", "ath,device-id" and "ath,eep-flash".
> that is exactly the use-case I want to use your owl-loader for (see
> [0], it's a small kernel module which adds the PCI configuration for
> ath9k devices).
Well, we also cooked up a userspace hack for OpenWRT/LEDE which would
work with the existing code (for the case you mentioned below) [2]. 
Furthermore it also works for other devices, as long as the fw is in
/lib/firmware and not in a subdirectory (But this can be fixed in 5
minutes with better bash foo). One thing that needs to be considered
though: That script interferes with procd firmware loading if the
"timing" is right. However usually procd has already finished all
firmware requests by then.

> This makes ath,pci-slot and ath,eep-flash obsolete.
> As far as I'm aware ath,device-id is a bit of a special case (mtd_read
> issues when the caldata is stored at an unaligned position on NOR
> flash). So this might be obsolete as well when using owl-loader.

The problem with the owl-loader is/was that it sticks around
when it has initialized all the cards. Unloading a module by
itself is tough. One way out would be to add it to ath9k's pci.c.
The question is: will such a feature have support from the ath9k
folks?

> > As an example: the AR9280 in the Cisco Z1 AP is initially
> > 0x168c:0xff1f (<-- ath9k doesn't know about that id). The
> > pci-fixup routine will change it to  0x168c:0x002A. Only
> > then ath9k can take it over and will initialize it.
> > Thing is: this is all currently done by platform code for
> > those architectures... And currently, the request_firmware
> > doesn't work for caldata on UBI partitions.
> request_firmware is working on UBI partitions in many cases.
> It's just not working when request_firmware is called too early (and
> this is not UBI specific, other filesystems might be affected as
> well): if it is called before rootfs is mounted (which is the case
> when you call it from a PCI fixup function) then it's not working
> (like you said).
> The "solution" to this is to compile the driver as kernel module (once
> this is loadable everything else should be readable as well).
> Not only ath9k is affected by this "issue", this is simply a
> limitation of request_firmware and/or the linux boot chain.
> 
> A few words regarding your owl-loader:
> First of all I would like to say "thank you"!
> Mathias and I are working on changing the lantiq target in LEDE to use
> owl-loader for all (ath9k) devices.
> All I had to do was to add another OWL PCI ID, implement a fallback
> for the firmware filename when there is no ath9k_platform_data (I'm
> using the same pattern as in PATCH 3/3 in this series). You can find
> the WIP code here: [1]

I've added lede-dev and Luis since this is relevant for them.
Maybe between the sysloadfw.sh and owl-loader, there's another
solution we overlooked so far? I know Luis has been digging
around in the firmware_class and added the sysdata API. But
from what I can tell, this would ?break? LEDE/OpenWRT's
userspace helper, since the sysfs interface in 
/sys/class/firmware which is used by procd to upload the data
is gone with sysdata or am I wrong?
 
Regards,
Christian

> [0] https://patchwork.ozlabs.org/patch/607682/
> [1] https://github.com/xdarklight/source/commits/ath9k-owl-loader-20160624
[2] <https://github.com/riptidewave93/Openwrt-Z1/commit/9a38c60a1206b4010fbfb626fc7b2ec69bbe232a>

[ath9k-devel] [PATCH RFC v3 3/3] ath9k: parse the device configuration from an OF node

[Martin Blumenstingl]

On Sat, Jun 25, 2016 at 9:26 PM, Christian Lamparter
<chunkeey@googlemail.com> wrote:
> The problem with the owl-loader is/was that it sticks around
> when it has initialized all the cards. Unloading a module by
> itself is tough. One way out would be to add it to ath9k's pci.c.
> The question is: will such a feature have support from the ath9k
> folks?
owl-loader seems very small (<7KiB) and it only allocates a few bytes
dynamically. Even if you move this code to ath9k you will still have
the same problem: as long as ath9k is not unloaded this code will hang
around in memory.
But apart from this - moving it to the kernel might have some benefits
though as it could be shared between ath9k and ath5k (as some ath5k
seem to require a similar "fixup" as well).

> I've added lede-dev and Luis since this is relevant for them.
> Maybe between the sysloadfw.sh and owl-loader, there's another
> solution we overlooked so far? I know Luis has been digging
> around in the firmware_class and added the sysdata API. But
> from what I can tell, this would ?break? LEDE/OpenWRT's
> userspace helper, since the sysfs interface in
> /sys/class/firmware which is used by procd to upload the data
> is gone with sysdata or am I wrong?
good idea to keep lede-dev in the loop, as they will be affected (in
my opinion: positively) by this change.


Regards,
Martin

[ath9k-devel] [PATCH RFC v3 3/3] ath9k: parse the device configuration from an OF node

[Luis R. Rodriguez]

On Mon, Jun 27, 2016 at 01:38:43AM +0200, Martin Blumenstingl wrote:
> On Sat, Jun 25, 2016 at 9:26 PM, Christian Lamparter
> <chunkeey@googlemail.com> wrote:
> > I've added lede-dev and Luis since this is relevant for them.
> > Maybe between the sysloadfw.sh and owl-loader, there's another
> > solution we overlooked so far? I know Luis has been digging
> > around in the firmware_class and added the sysdata API. But
> > from what I can tell, this would ?break? LEDE/OpenWRT's
> > userspace helper, since the sysfs interface in
> > /sys/class/firmware which is used by procd to upload the data
> > is gone with sysdata or am I wrong?
> good idea to keep lede-dev in the loop, as they will be affected (in
> my opinion: positively) by this change.

We cannot remove the /sys/class/firmware usermode helper, it however should be
compartamentalized to only a few device drivers that we *know* definitely need
it. So far there are only 2 device drivers that we've identified as needing it
and as such only those drivers should implicate use of it.

In the future, should the sysdata API get merged, the implications are that the
further features of the firmware API will be added for sysdata users, but
perhaps not for the old API as that entails silly collateral evolutions to
the API or new exported symbols.

I highly encourage use of the usermode helper to be reconsidered and simply
abandoned. There are many reasons why it was a bad idea, for details refer to
the thread [0].

[0] https://lkml.kernel.org/r/1466117661-22075-1-git-send-email-mcgrof at kernel.org

  Luis

[ath9k-devel] [PATCH v4 0/3] add devicetree support to ath9k

[Martin Blumenstingl]

This series adds support for configuring ath9k based devices via
devicetree. This was tested on PCI(e) based devices. This should work
for AHB based devices as well as soon as the ath79 platform is ready
to populate the ath9k wmac via devicetree.

Changes since v3:
- dropped RFC status
- dropped all GPIO and LED related devicetree properties (namely:
  qca,gpio-mask, qca,gpio-val, qca,led-pin, qca,led-active-high).
  These are only needed as long as ath9k doesn't register itself as a
  GPIO controller. However, patches for this are already available in
  LEDE and OpenWrt. Instead of adding new "legacy" properties we should
  try to get the GPIO controller patches upstream.
- rewording of large parts of the documentation which now makes clear
  when specific properties should be set (or not set) any the reason
  behind it. Thanks to everyone who suggested improvements or pointed
  out typos.
- use an array for accessing the string representation of the bus types
  (as suggested by Felix)


Martin Blumenstingl (3):
  Documentation: dt: net: add ath9k wireless device binding
  ath9k: add a helper to get the string representation of ath_bus_type
  ath9k: parse the device configuration from an OF node

 .../devicetree/bindings/net/wireless/qca,ath9k.txt | 59 ++++++++++++++++++++++
 drivers/net/wireless/ath/ath.h                     |  6 +++
 drivers/net/wireless/ath/ath9k/init.c              | 55 ++++++++++++++++++++
 drivers/net/wireless/ath/main.c                    |  7 +++
 4 files changed, 127 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt

-- 
2.9.0

[ath9k-devel] [PATCH v4 1/3] Documentation: dt: net: add ath9k wireless device binding

[Martin Blumenstingl]

Add documentation how devicetree can be used to configure ath9k based
devices.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 .../devicetree/bindings/net/wireless/qca,ath9k.txt | 59 ++++++++++++++++++++++
 1 file changed, 59 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt

diff --git a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
new file mode 100644
index 0000000..7c62c59
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
@@ -0,0 +1,59 @@
+* Qualcomm Atheros ath9k wireless devices
+
+This node provides properties for configuring the ath9k wireless device. The
+node is expected to be specified as a child node of the PCI controller to
+which the wireless chip is connected.
+
+Required properties:
+- compatible: Should be "qca,ath9k"
+
+Optional properties:
+- reg: Address and length of the register set for the device.
+- qca,clk-25mhz: Defines that a 25MHz clock is used
+- qca,no-eeprom: Indicates that there is no physical EEPROM connected to the
+			ath9k wireless chip (in this case the calibration /
+			EEPROM data will be loaded from userspace using the
+			kernel firmware loader).
+- qca,check-eeprom-endianness: When enabled, the driver checks if the
+				endianness of the EEPROM (based on the two
+				magic bytes at the start of the EEPROM)
+				matches the host system's native endianness.
+				The data will be swapped accordingly if there
+				is a mismatch.
+				Leaving this disabled means that the EEPROM
+				data will be interpreted in the system's
+				native endianness, regardless of the magic
+				bytes.
+				Enable this option only when the magic bytes
+				are known to indicate the correct endianness
+				of the EEPROM data, because otherwise the
+				EEPROM data may be swapped and thus may be
+				parsed incorrectly.
+- qca,disable-2ghz: Overrides the settings from the EEPROM and disables the
+			2.4GHz band. Setting this property is only needed
+			when the RF circuit does not support the 2.4GHz band
+			while it is enabled nevertheless in the EEPROM.
+- qca,disable-5ghz: Overrides the settings from the EEPROM and disables the
+			5GHz band. Setting this property is only needed when
+			the RF circuit does not support the 5GHz band while
+			it is enabled nevertheless in the EEPROM.
+- mac-address: See ethernet.txt in the parent directory
+- local-mac-address: See ethernet.txt in the parent directory
+
+In this example, the node is defined as child node of the PCI controller.
+
+pci {
+	pcie at 0 {
+		reg = <0 0 0 0 0>;
+		#interrupt-cells = <1>;
+		#size-cells = <2>;
+		#address-cells = <3>;
+		device_type = "pci";
+
+		ath9k at 0,0 {
+			compatible = "qca,ath9k";
+			reg = <0 0 0 0 0>;
+			qca,disable-5ghz;
+		};
+	};
+};
-- 
2.9.0

[ath9k-devel] [PATCH v4 1/3] Documentation: dt: net: add ath9k wireless device binding

[Arnd Bergmann]

On Sunday, July 10, 2016 1:28:32 AM CEST Martin Blumenstingl wrote:
> +- qca,check-eeprom-endianness: When enabled, the driver checks if the
> +                               endianness of the EEPROM (based on the two
> +                               magic bytes at the start of the EEPROM)
> +                               matches the host system's native endianness.
> +                               The data will be swapped accordingly if there
> +                               is a mismatch.
> +                               Leaving this disabled means that the EEPROM
> +                               data will be interpreted in the system's
> +                               native endianness, regardless of the magic
> +                               bytes.
> +                               Enable this option only when the magic bytes
> +                               are known to indicate the correct endianness
> +                               of the EEPROM data, because otherwise the
> +                               EEPROM data may be swapped and thus may be
> +                               parsed incorrectly.

Defining properties in terms of "native" endianess is usually not a good
idea, as some architectures (ARMv6+, PowerPC, ...) allow running either
big-endian or little-endian without changing the endianess of device
registers in the process.

It's better to document the property as defaulting to a specific endianess
unless you have an override or the autodetection flag, regardless of
the CPU endianess.

	Arnd

[ath9k-devel] [PATCH v4 1/3] Documentation: dt: net: add ath9k wireless device binding

[Martin Blumenstingl]

On Sun, Jul 10, 2016 at 10:52 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Sunday, July 10, 2016 1:28:32 AM CEST Martin Blumenstingl wrote:
>> +- qca,check-eeprom-endianness: When enabled, the driver checks if the
>> +                               endianness of the EEPROM (based on the two
>> +                               magic bytes at the start of the EEPROM)
>> +                               matches the host system's native endianness.
>> +                               The data will be swapped accordingly if there
>> +                               is a mismatch.
>> +                               Leaving this disabled means that the EEPROM
>> +                               data will be interpreted in the system's
>> +                               native endianness, regardless of the magic
>> +                               bytes.
>> +                               Enable this option only when the magic bytes
>> +                               are known to indicate the correct endianness
>> +                               of the EEPROM data, because otherwise the
>> +                               EEPROM data may be swapped and thus may be
>> +                               parsed incorrectly.
>
> Defining properties in terms of "native" endianess is usually not a good
> idea, as some architectures (ARMv6+, PowerPC, ...) allow running either
> big-endian or little-endian without changing the endianess of device
> registers in the process.
>
> It's better to document the property as defaulting to a specific endianess
> unless you have an override or the autodetection flag, regardless of
> the CPU endianess.
ath9k reads the data from the EEPROM into memory. With that property
disabled ath9k simply assumes that the endianness of the values in the
EEPROM are having the correct endianness for the host system (in other
words: no swap is being applied).
I am not sure I understand you correctly, but isn't what you are
explaining an issue in the ath9k code, rather than in this
documentation?

[ath9k-devel] [PATCH v4 1/3] Documentation: dt: net: add ath9k wireless device binding

[Arnd Bergmann]

On Sunday, July 10, 2016 10:54:50 PM CEST Martin Blumenstingl wrote:
> On Sun, Jul 10, 2016 at 10:52 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Sunday, July 10, 2016 1:28:32 AM CEST Martin Blumenstingl wrote:
> >> +- qca,check-eeprom-endianness: When enabled, the driver checks if the
> >> +                               endianness of the EEPROM (based on the two
> >> +                               magic bytes at the start of the EEPROM)
> >> +                               matches the host system's native endianness.
> >> +                               The data will be swapped accordingly if there
> >> +                               is a mismatch.
> >> +                               Leaving this disabled means that the EEPROM
> >> +                               data will be interpreted in the system's
> >> +                               native endianness, regardless of the magic
> >> +                               bytes.
> >> +                               Enable this option only when the magic bytes
> >> +                               are known to indicate the correct endianness
> >> +                               of the EEPROM data, because otherwise the
> >> +                               EEPROM data may be swapped and thus may be
> >> +                               parsed incorrectly.
> >
> > Defining properties in terms of "native" endianess is usually not a good
> > idea, as some architectures (ARMv6+, PowerPC, ...) allow running either
> > big-endian or little-endian without changing the endianess of device
> > registers in the process.
> >
> > It's better to document the property as defaulting to a specific endianess
> > unless you have an override or the autodetection flag, regardless of
> > the CPU endianess.
> ath9k reads the data from the EEPROM into memory. With that property
> disabled ath9k simply assumes that the endianness of the values in the
> EEPROM are having the correct endianness for the host system (in other
> words: no swap is being applied).
> I am not sure I understand you correctly, but isn't what you are
> explaining an issue in the ath9k code, rather than in this
> documentation?

I looked at the code more to find that out now, but I'm more confused
now, as the eeprom seems to be read as a byte stream, and the endianess
conversion that the driver performs is not on the data values in it,
but seems to instead swap the bytes in each 16-bit word, regardless
of the contents (the values inside of the byte stream are always
interpreted as big-endian). Is that a correct observation?

What I see in ath_pci_eeprom_read() is that the 16-bit words are taken
from the lower 16 bit of the little-endian AR_EEPROM_STATUS_DATA
register. As this is coming from a PCI register, it must have a device
specific endianess that is identical on all CPUs, so in the description
above, mentioning CPU endianess is a bit confusing. I could not find
the code that does the conditional byteswap, instead this function

static bool ar9300_eeprom_read_byte(struct ath_hw *ah, int address,
                                    u8 *buffer)
{
        u16 val;

        if (unlikely(!ath9k_hw_nvram_read(ah, address / 2, &val)))
                return false;

        *buffer = (val >> (8 * (address % 2))) & 0xff;
        return true;
}

evidently assumes that the lower 8 bit of the 16-bit data from PCI
come first, i.e. it byteswaps on big-endian CPUs to get the bytestream
back into the order in which it is stored in the EEPROM.

Interestingly, this also seems to happen for ath_ahb_eeprom_read()
even though on that one the value does not get swapped by the bus
accessor, so presumably big-endian machines with a ahb based ath9k
store their eeprom byte-reversed?

	Arnd

[ath9k-devel] [PATCH v4 1/3] Documentation: dt: net: add ath9k wireless device binding

[Martin Blumenstingl]

On Mon, Jul 11, 2016 at 12:01 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>> ath9k reads the data from the EEPROM into memory. With that property
>> disabled ath9k simply assumes that the endianness of the values in the
>> EEPROM are having the correct endianness for the host system (in other
>> words: no swap is being applied).
>> I am not sure I understand you correctly, but isn't what you are
>> explaining an issue in the ath9k code, rather than in this
>> documentation?
>
> I looked at the code more to find that out now, but I'm more confused
> now, as the eeprom seems to be read as a byte stream, and the endianess
> conversion that the driver performs is not on the data values in it,
> but seems to instead swap the bytes in each 16-bit word, regardless
> of the contents (the values inside of the byte stream are always
> interpreted as big-endian). Is that a correct observation?
that seems to be the case for the ar9003 eeprom. Other implementations
are doing it different, look at ath9k_hw_ar9287_check_eeprom for
example: first ath9k_hw_nvram_swap_data checks the two magic bytes at
the beginning of the data and swaps the bytes in each 16-bit word if
the magic bytes don't match the magic bytes for the "native system
endianness" (see AR5416_EEPROM_MAGIC). Then more swapping is applied.
I asked for more details about the EEPROM format (specifically the
endianness part) here [0] as I don't have access to the datasheets
(all I have is the ath9k code)

> What I see in ath_pci_eeprom_read() is that the 16-bit words are taken
> from the lower 16 bit of the little-endian AR_EEPROM_STATUS_DATA
> register. As this is coming from a PCI register, it must have a device
> specific endianess that is identical on all CPUs, so in the description
> above, mentioning CPU endianess is a bit confusing. I could not find
> the code that does the conditional byteswap, instead this function
>
> static bool ar9300_eeprom_read_byte(struct ath_hw *ah, int address,
>                                     u8 *buffer)
> {
>         u16 val;
>
>         if (unlikely(!ath9k_hw_nvram_read(ah, address / 2, &val)))
>                 return false;
>
>         *buffer = (val >> (8 * (address % 2))) & 0xff;
>         return true;
> }
>
> evidently assumes that the lower 8 bit of the 16-bit data from PCI
> come first, i.e. it byteswaps on big-endian CPUs to get the bytestream
> back into the order in which it is stored in the EEPROM.
Please have a look at the ath9k_hw_nvram_swap_data function and
eeprom_ops.check_eeprom (for example ath9k_hw_ar9287_check_eeprom).
These are performing the conditional swapping (in addition to whatever
previous swapping there was).
The basic code works like this: read the full EEPROM data into memory
(either from PCI bus, ath9k_platform_data or request_firmware), then
eeprom_ops.check_eeprom will call ath9k_hw_nvram_swap_data for 16-bit
word swapping and afterwards the check_eeprom implementation will doe
further swapping.
Apart from that your findings seem correct (at least this is identical
to how I would interpret the code).

> Interestingly, this also seems to happen for ath_ahb_eeprom_read()
> even though on that one the value does not get swapped by the bus
> accessor, so presumably big-endian machines with a ahb based ath9k
> store their eeprom byte-reversed?
on AHB the eeprom data has to be provided via ath9k_platform_data /
request_firmware mechanism. Thus there is no bus specific swapping,
only the ath9k_hw_nvram_swap_data / eeprom_ops.check_eeprom swapping
is applied in this case.


[0] http://thread.gmane.org/gmane.linux.kernel.wireless.general/153569

[ath9k-devel] [PATCH v4 1/3] Documentation: dt: net: add ath9k wireless device binding

[Arnd Bergmann]

On Monday, July 11, 2016 11:21:26 PM CEST Martin Blumenstingl wrote:
> On Mon, Jul 11, 2016 at 12:01 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> >> ath9k reads the data from the EEPROM into memory. With that property
> >> disabled ath9k simply assumes that the endianness of the values in the
> >> EEPROM are having the correct endianness for the host system (in other
> >> words: no swap is being applied).
> >> I am not sure I understand you correctly, but isn't what you are
> >> explaining an issue in the ath9k code, rather than in this
> >> documentation?
> >
> > I looked at the code more to find that out now, but I'm more confused
> > now, as the eeprom seems to be read as a byte stream, and the endianess
> > conversion that the driver performs is not on the data values in it,
> > but seems to instead swap the bytes in each 16-bit word, regardless
> > of the contents (the values inside of the byte stream are always
> > interpreted as big-endian). Is that a correct observation?
> that seems to be the case for the ar9003 eeprom. Other implementations
> are doing it different, look at ath9k_hw_ar9287_check_eeprom for
> example: first ath9k_hw_nvram_swap_data checks the two magic bytes at
> the beginning of the data and swaps the bytes in each 16-bit word if
> the magic bytes don't match the magic bytes for the "native system
> endianness" (see AR5416_EEPROM_MAGIC). Then more swapping is applied.
> I asked for more details about the EEPROM format (specifically the
> endianness part) here [0] as I don't have access to the datasheets
> (all I have is the ath9k code)

Ok.

> > What I see in ath_pci_eeprom_read() is that the 16-bit words are taken
> > from the lower 16 bit of the little-endian AR_EEPROM_STATUS_DATA
> > register. As this is coming from a PCI register, it must have a device
> > specific endianess that is identical on all CPUs, so in the description
> > above, mentioning CPU endianess is a bit confusing. I could not find
> > the code that does the conditional byteswap, instead this function
> >
> > static bool ar9300_eeprom_read_byte(struct ath_hw *ah, int address,
> >                                     u8 *buffer)
> > {
> >         u16 val;
> >
> >         if (unlikely(!ath9k_hw_nvram_read(ah, address / 2, &val)))
> >                 return false;
> >
> >         *buffer = (val >> (8 * (address % 2))) & 0xff;
> >         return true;
> > }
> >
> > evidently assumes that the lower 8 bit of the 16-bit data from PCI
> > come first, i.e. it byteswaps on big-endian CPUs to get the bytestream
> > back into the order in which it is stored in the EEPROM.
> Please have a look at the ath9k_hw_nvram_swap_data function and
> eeprom_ops.check_eeprom (for example ath9k_hw_ar9287_check_eeprom).
> These are performing the conditional swapping (in addition to whatever
> previous swapping there was).
> The basic code works like this: read the full EEPROM data into memory
> (either from PCI bus, ath9k_platform_data or request_firmware), then
> eeprom_ops.check_eeprom will call ath9k_hw_nvram_swap_data for 16-bit
> word swapping and afterwards the check_eeprom implementation will doe
> further swapping.
> Apart from that your findings seem correct (at least this is identical
> to how I would interpret the code).

Ok, so my interpretation of what this is done for is that the
swap in ath9k_hw_nvram_swap_data() is done to compensate for
the data that is read byte-reversed from the PCI bus and it
is does not swap when the data is read from a file. The result
is a structure with big-endian 16-bit and 32-bit members but
all fields in the right place.

The swapping in ath9k_hw_ar9287_check_eeprom() then turns the
big-endian fields into little-endian fields so it can be used
on little-endian CPUs without going through le16_to_cpu().

However, the whole thing still looks fragile to me as it
doesn't seem to handle the case where we want to swap the
values but not the bus.

My guess is that we still want to fix the driver to handle
this more consistently in order to decide whether a DT property
is needed or not.

> > Interestingly, this also seems to happen for ath_ahb_eeprom_read()
> > even though on that one the value does not get swapped by the bus
> > accessor, so presumably big-endian machines with a ahb based ath9k
> > store their eeprom byte-reversed?
> on AHB the eeprom data has to be provided via ath9k_platform_data /
> request_firmware mechanism. Thus there is no bus specific swapping,
> only the ath9k_hw_nvram_swap_data / eeprom_ops.check_eeprom swapping
> is applied in this case.

I guess the header then indicates that none of the swapping is
performed.

	Arnd

[ath9k-devel] [PATCH v4 1/3] Documentation: dt: net: add ath9k wireless device binding

[Kalle Valo]

Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:

> Add documentation how devicetree can be used to configure ath9k based
> devices.
>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>  .../devicetree/bindings/net/wireless/qca,ath9k.txt | 59 ++++++++++++++++++++++
>  1 file changed, 59 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
>
> diff --git a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
> new file mode 100644
> index 0000000..7c62c59
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
> @@ -0,0 +1,59 @@
> +* Qualcomm Atheros ath9k wireless devices
> +
> +This node provides properties for configuring the ath9k wireless device. The
> +node is expected to be specified as a child node of the PCI controller to
> +which the wireless chip is connected.
> +
> +Required properties:
> +- compatible: Should be "qca,ath9k"

Isn't this supposed to use the chipset name? ath9k is the driver name
and something like ar9462 is the chip name. I know in ath10k we used
"qca,ath10k" but I'm starting to suspect that was a mistake.

-- 
Kalle Valo

[ath9k-devel] [PATCH v4 1/3] Documentation: dt: net: add ath9k wireless device binding

[Arnd Bergmann]

On Wednesday, July 13, 2016 10:02:39 AM CEST Kalle Valo wrote:
> Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:
> 
> > Add documentation how devicetree can be used to configure ath9k based
> > devices.
> >
> > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> > ---
> >  .../devicetree/bindings/net/wireless/qca,ath9k.txt | 59 ++++++++++++++++++++++
> >  1 file changed, 59 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
> >
> > diff --git a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
> > new file mode 100644
> > index 0000000..7c62c59
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
> > @@ -0,0 +1,59 @@
> > +* Qualcomm Atheros ath9k wireless devices
> > +
> > +This node provides properties for configuring the ath9k wireless device. The
> > +node is expected to be specified as a child node of the PCI controller to
> > +which the wireless chip is connected.
> > +
> > +Required properties:
> > +- compatible: Should be "qca,ath9k"
> 
> Isn't this supposed to use the chipset name? ath9k is the driver name
> and something like ar9462 is the chip name. I know in ath10k we used
> "qca,ath10k" but I'm starting to suspect that was a mistake.
> 

You are right, but it's actually more complicated than that. For PCI
devices, the format of the compatible strings is defined in
http://www.o3one.org/hwdocs/openfirmware/pci_supplement_2_1.pdf
and it doesn't use the "vendor,device" syntax at all but instead
uses pciVVVV,DDDD where VVVV and DDDD are the hexadecimal (without
leading 0x) vendor and device ID numbers. The document also specifies
seven other formats and in theory you should list them all, but
that really only makes sense for Open Firmware that programatically
creates those strings.

For a hand-written DTS source, I'd just use the shortest one of those.
Linux actually doesn't care at all, as PCI drivers don't use the
compatible string but instead look at the config register values
that were read by the PCI core.

For USB, we do basically the same thing, but have very few examples
of that as Linux only recently started supporting this.

For SDIO devices we should have done the same thing but screwed up
when we made the generic binding and we have no policy, the example
even lists one that makes no sense at all ("brcm,bcm43xx-fmac")
as it is neither a specific device nor something derived from the IDs.

For on-chip devices, we should follow the common policy for on-chip
devices and use the "vendor,chip-funcion" with fallbacks for
compatible devices such as:

	compatible = "tplink,tp9343-wifi", "qca,qca9561-wifi", "atheros,ar9001-wifi";

For a chip that is branded by TP-Link and derived from a Qualcomm Atheros
chip it is 100% compatible with, and in turn derived from an older
implementation (just guessing which one was the original ath9k).


	Arnd

[ath9k-devel] [PATCH v4 2/3] ath9k: add a helper to get the string representation of ath_bus_type

[Martin Blumenstingl]

This can be used when the ath_bus_type has to be presented in a log
message or firmware filename.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/net/wireless/ath/ath.h  | 6 ++++++
 drivers/net/wireless/ath/main.c | 7 +++++++
 2 files changed, 13 insertions(+)

diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
index da7a7c8..f3f2784 100644
--- a/drivers/net/wireless/ath/ath.h
+++ b/drivers/net/wireless/ath/ath.h
@@ -327,4 +327,10 @@ static inline const char *ath_opmode_to_string(enum nl80211_iftype opmode)
 }
 #endif
 
+extern const char *ath_bus_type_strings[];
+static inline const char *ath_bus_type_to_string(enum ath_bus_type bustype)
+{
+	return ath_bus_type_strings[bustype];
+}
+
 #endif /* ATH_H */
diff --git a/drivers/net/wireless/ath/main.c b/drivers/net/wireless/ath/main.c
index 338d723..89f4b05 100644
--- a/drivers/net/wireless/ath/main.c
+++ b/drivers/net/wireless/ath/main.c
@@ -90,3 +90,10 @@ void ath_printk(const char *level, const struct ath_common* common,
 	va_end(args);
 }
 EXPORT_SYMBOL(ath_printk);
+
+const char *ath_bus_type_strings[] = {
+	[ATH_PCI] = "pci",
+	[ATH_AHB] = "ahb",
+	[ATH_USB] = "usb",
+};
+EXPORT_SYMBOL(ath_bus_type_strings);
-- 
2.9.0

[ath9k-devel] [PATCH v4 3/3] ath9k: parse the device configuration from an OF node

[Martin Blumenstingl]

This makes it possible to configure ath9k based devices using
devicetree. That makes some out-of-tree "convert devicetree to
ath9k_platform_data glue"-code obsolete.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/net/wireless/ath/ath9k/init.c | 55 +++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index edc74fc..9dd7ed5 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -20,6 +20,8 @@
 #include <linux/slab.h>
 #include <linux/ath9k_platform.h>
 #include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_net.h>
 #include <linux/relay.h>
 #include <net/ieee80211_radiotap.h>
 
@@ -555,6 +557,55 @@ static int ath9k_init_platform(struct ath_softc *sc)
 	return 0;
 }
 
+static int ath9k_of_init(struct ath_softc *sc)
+{
+	struct device_node *np = sc->dev->of_node;
+	struct ath_hw *ah = sc->sc_ah;
+	struct ath_common *common = ath9k_hw_common(ah);
+	enum ath_bus_type bus_type = common->bus_ops->ath_bus_type;
+	const char *mac;
+	char eeprom_name[100];
+	int ret;
+
+	if (!of_device_is_available(np))
+		return 0;
+
+	ath_dbg(common, CONFIG, "parsing configuration from OF node\n");
+
+	if (of_property_read_bool(np, "qca,clk-25mhz"))
+		ah->is_clk_25mhz = true;
+
+	if (of_property_read_bool(np, "qca,disable-2ghz"))
+		ah->disable_2ghz = true;
+
+	if (of_property_read_bool(np, "qca,disable-5ghz"))
+		ah->disable_5ghz = true;
+
+	if (of_property_read_bool(np, "qca,check-eeprom-endianness"))
+		ah->ah_flags &= ~AH_NO_EEP_SWAP;
+	else
+		ah->ah_flags |= AH_NO_EEP_SWAP;
+
+	if (of_property_read_bool(np, "qca,no-eeprom")) {
+		/* ath9k-eeprom-<bus>-<id>.bin */
+		scnprintf(eeprom_name, sizeof(eeprom_name),
+			  "ath9k-eeprom-%s-%s.bin",
+			  ath_bus_type_to_string(bus_type), dev_name(ah->dev));
+
+		ret = ath9k_eeprom_request(sc, eeprom_name);
+		if (ret)
+			return ret;
+	}
+
+	mac = of_get_mac_address(np);
+	if (mac)
+		ether_addr_copy(common->macaddr, mac);
+
+	ah->ah_flags &= ~AH_USE_EEPROM;
+
+	return 0;
+}
+
 static int ath9k_init_softc(u16 devid, struct ath_softc *sc,
 			    const struct ath_bus_ops *bus_ops)
 {
@@ -611,6 +662,10 @@ static int ath9k_init_softc(u16 devid, struct ath_softc *sc,
 	if (ret)
 		return ret;
 
+	ret = ath9k_of_init(sc);
+	if (ret)
+		return ret;
+
 	if (ath9k_led_active_high != -1)
 		ah->config.led_active_high = ath9k_led_active_high == 1;
 
-- 
2.9.0

[ath9k-devel] [PATCH v4 3/3] ath9k: parse the device configuration from an OF node

[Bjørn Mork]

Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:

> +	if (of_property_read_bool(np, "qca,clk-25mhz"))
> +		ah->is_clk_25mhz = true;
> +
> +	if (of_property_read_bool(np, "qca,disable-2ghz"))
> +		ah->disable_2ghz = true;
> +
> +	if (of_property_read_bool(np, "qca,disable-5ghz"))
> +		ah->disable_5ghz = true;

This is bike-shedding, but how about

	ah->is_clk_25mhz = of_property_read_bool(np, "qca,clk-25mhz");
	ah->disable_2ghz = of_property_read_bool(np, "qca,disable-2ghz");
	ah->disable_5ghz = of_property_read_bool(np, "qca,disable-5ghz");

instead?


Bj?rn

[ath9k-devel] [PATCH v4 3/3] ath9k: parse the device configuration from an OF node

[Oleksij Rempel]

Am 10.07.2016 um 02:19 schrieb Bj?rn Mork:
> Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:
> 
>> +	if (of_property_read_bool(np, "qca,clk-25mhz"))
>> +		ah->is_clk_25mhz = true;
>> +
>> +	if (of_property_read_bool(np, "qca,disable-2ghz"))
>> +		ah->disable_2ghz = true;
>> +
>> +	if (of_property_read_bool(np, "qca,disable-5ghz"))
>> +		ah->disable_5ghz = true;
> 
> This is bike-shedding, but how about
> 
> 	ah->is_clk_25mhz = of_property_read_bool(np, "qca,clk-25mhz");
> 	ah->disable_2ghz = of_property_read_bool(np, "qca,disable-2ghz");
> 	ah->disable_5ghz = of_property_read_bool(np, "qca,disable-5ghz");
> 
> instead?

Hm... i assume each WiFi hw in the world can reuse this settings, may be
it i worth to use generic names?

-- 
Regards,
Oleksij

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 213 bytes
Desc: OpenPGP digital signature
Url : http://lists.ath9k.org/pipermail/ath9k-devel/attachments/20160710/c6337c74/attachment.pgp 

[ath9k-devel] [PATCH v4 3/3] ath9k: parse the device configuration from an OF node

[Martin Blumenstingl]

On Sun, Jul 10, 2016 at 2:19 AM, Bj?rn Mork <bjorn@mork.no> wrote:
> Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:
>
>> +     if (of_property_read_bool(np, "qca,clk-25mhz"))
>> +             ah->is_clk_25mhz = true;
>> +
>> +     if (of_property_read_bool(np, "qca,disable-2ghz"))
>> +             ah->disable_2ghz = true;
>> +
>> +     if (of_property_read_bool(np, "qca,disable-5ghz"))
>> +             ah->disable_5ghz = true;
>
> This is bike-shedding, but how about
>
>         ah->is_clk_25mhz = of_property_read_bool(np, "qca,clk-25mhz");
>         ah->disable_2ghz = of_property_read_bool(np, "qca,disable-2ghz");
>         ah->disable_5ghz = of_property_read_bool(np, "qca,disable-5ghz");
I'm fine with either way - I'll simply adhere to the coding style that
the ath9k devs want to use, so just let me know which one you prefer.

[ath9k-devel] [PATCH v5 0/3] add devicetree support to ath9k

[Martin Blumenstingl]

This series adds support for configuring ath9k based devices via
devicetree. This was tested on PCI(e) based devices. This should work
for AHB based devices as well as soon as the ath79 platform is ready
to populate the ath9k wmac via devicetree.

Changes since v4:
- dropped "qca,check-eeprom-endianness" for now - this will be part
  of a separate patch
- update documentation to fix the "compatible" string (as there is a
  PCI device specific format)


Martin Blumenstingl (3):
  Documentation: dt: net: add ath9k wireless device binding
  ath9k: add a helper to get the string representation of ath_bus_type
  ath9k: parse the device configuration from an OF node

 .../devicetree/bindings/net/wireless/qca,ath9k.txt | 47 ++++++++++++++++++++
 drivers/net/wireless/ath/ath.h                     |  6 +++
 drivers/net/wireless/ath/ath9k/init.c              | 51 ++++++++++++++++++++++
 drivers/net/wireless/ath/main.c                    |  7 +++
 4 files changed, 111 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt

-- 
2.9.3

[ath9k-devel] [PATCH v5 1/3] Documentation: dt: net: add ath9k wireless device binding

[Martin Blumenstingl]

Add documentation how devicetree can be used to configure ath9k based
devices.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 .../devicetree/bindings/net/wireless/qca,ath9k.txt | 47 ++++++++++++++++++++++
 1 file changed, 47 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt

diff --git a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
new file mode 100644
index 0000000..98065ad
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
@@ -0,0 +1,47 @@
+* Qualcomm Atheros ath9k wireless devices
+
+This node provides properties for configuring the ath9k wireless device. The
+node is expected to be specified as a child node of the PCI controller to
+which the wireless chip is connected.
+
+Required properties:
+- compatible: For PCI and PCIe devices this should be an identifier following
+		the format as defined in "PCI Bus Binding to Open Firmware"
+		Revision 2.1. One of the possible formats is "pciVVVV,DDDD"
+		where VVVV is the PCI vendor ID and DDDD is PCI device ID.
+
+Optional properties:
+- reg: Address and length of the register set for the device.
+- qca,clk-25mhz: Defines that a 25MHz clock is used
+- qca,no-eeprom: Indicates that there is no physical EEPROM connected to the
+			ath9k wireless chip (in this case the calibration /
+			EEPROM data will be loaded from userspace using the
+			kernel firmware loader).
+- qca,disable-2ghz: Overrides the settings from the EEPROM and disables the
+			2.4GHz band. Setting this property is only needed
+			when the RF circuit does not support the 2.4GHz band
+			while it is enabled nevertheless in the EEPROM.
+- qca,disable-5ghz: Overrides the settings from the EEPROM and disables the
+			5GHz band. Setting this property is only needed when
+			the RF circuit does not support the 5GHz band while
+			it is enabled nevertheless in the EEPROM.
+- mac-address: See ethernet.txt in the parent directory
+- local-mac-address: See ethernet.txt in the parent directory
+
+In this example, the node is defined as child node of the PCI controller.
+
+pci {
+	pcie at 0 {
+		reg = <0 0 0 0 0>;
+		#interrupt-cells = <1>;
+		#size-cells = <2>;
+		#address-cells = <3>;
+		device_type = "pci";
+
+		ath9k at 0,0 {
+			compatible = "pci168c,0030";
+			reg = <0 0 0 0 0>;
+			qca,disable-5ghz;
+		};
+	};
+};
-- 
2.9.3

[ath9k-devel] [PATCH v5 1/3] Documentation: dt: net: add ath9k wireless device binding

[Arnd Bergmann]

On Sunday, August 21, 2016 4:31:03 PM CEST Martin Blumenstingl wrote:
> +pci {
> +       pcie at 0 {
> +               reg = <0 0 0 0 0>;

It's not clear what these two nodes refer to in the example. Is the top-level
node the PCI host bridge, and the second node the first PCIe port?

Maybe add the properties from a real system here to make this a little
clearer.

The unit address for the slot should be "00,0", not "0" here.

> +               #interrupt-cells = <1>;
> +               #size-cells = <2>;
> +               #address-cells = <3>;
> +               device_type = "pci";
> +

I think this needs an empty "ranges" property to be a valid bridge node.

> +               ath9k at 0,0 {

According to the PCI binding, the name should be the same as the
compatible string here, or match the class code in the table.

> +                       compatible = "pci168c,0030";
> +                       reg = <0 0 0 0 0>;

Are the device/fn numbers all zero on your system? This is a bit
confusing, as it's not immediately clear what the reg properties
refers to. Also, I think the length should reflect the actual length
of the config space, either 0x100 or 0x1000.

> +                       qca,disable-5ghz;
> +               };
> +       };
> +};

	Arnd

[ath9k-devel] [PATCH v5 1/3] Documentation: dt: net: add ath9k wireless device binding

[Martin Blumenstingl]

On Mon, Aug 22, 2016 at 11:08 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Sunday, August 21, 2016 4:31:03 PM CEST Martin Blumenstingl wrote:
>> +pci {
>> +       pcie at 0 {
>> +               reg = <0 0 0 0 0>;
>
> It's not clear what these two nodes refer to in the example. Is the top-level
> node the PCI host bridge, and the second node the first PCIe port?
>
> Maybe add the properties from a real system here to make this a little
> clearer.
>
> The unit address for the slot should be "00,0", not "0" here.
good point - I'll skip the part where I am describing the bridge as
well and keep it simple.

>> +               ath9k at 0,0 {
>
> According to the PCI binding, the name should be the same as the
> compatible string here, or match the class code in the table.
The original example was from an actual system (where an ath9k is
connected to the PCIe bug). Unfortunately the PCIe driver contains
some hacks, so I'm not sure if these values serve as a good example.
Thus I took an example from a device where the ath9k chip is connected
via PCI (no "express" - found in sysfs at:
/sys/bus/pci/devices/0000:00:0e.0):
&pci0 {
    ath9k at 168c,002d {
        compatible = "pci168c,002d";
        reg = <0x7000 0 0 0 0>;
        qca,disable-5ghz;
    };
};

>> +                       compatible = "pci168c,0030";
>> +                       reg = <0 0 0 0 0>;
>
> Are the device/fn numbers all zero on your system? This is a bit
> confusing, as it's not immediately clear what the reg properties
> refers to. Also, I think the length should reflect the actual length
> of the config space, either 0x100 or 0x1000.
The first issue is solved with the updated example (see above).
Where would the size go (is it the second-last or last value)?


Thanks for your patience!
Regards,
Martin

[ath9k-devel] [PATCH v5 1/3] Documentation: dt: net: add ath9k wireless device binding

[Arnd Bergmann]

On Sunday 28 August 2016, Martin Blumenstingl wrote:
> On Mon, Aug 22, 2016 at 11:08 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Sunday, August 21, 2016 4:31:03 PM CEST Martin Blumenstingl wrote:
> >> +               ath9k at 0,0 {
> >
> > According to the PCI binding, the name should be the same as the
> > compatible string here, or match the class code in the table.
> The original example was from an actual system (where an ath9k is
> connected to the PCIe bug). Unfortunately the PCIe driver contains
> some hacks, so I'm not sure if these values serve as a good example.
> Thus I took an example from a device where the ath9k chip is connected
> via PCI (no "express" - found in sysfs at:
> /sys/bus/pci/devices/0000:00:0e.0):
> &pci0 {
>     ath9k at 168c,002d {
>         compatible = "pci168c,002d";
>         reg = <0x7000 0 0 0 0>;
>         qca,disable-5ghz;
>     };
> };

Ok, that would be a better example.


> >> +                       compatible = "pci168c,0030";
> >> +                       reg = <0 0 0 0 0>;
> >
> > Are the device/fn numbers all zero on your system? This is a bit
> > confusing, as it's not immediately clear what the reg properties
> > refers to. Also, I think the length should reflect the actual length
> > of the config space, either 0x100 or 0x1000.
> The first issue is solved with the updated example (see above).
> Where would the size go (is it the second-last or last value)?

The last one.

	Arnd

[ath9k-devel] [PATCH v5 2/3] ath9k: add a helper to get the string representation of ath_bus_type

[Martin Blumenstingl]

This can be used when the ath_bus_type has to be presented in a log
message or firmware filename.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/net/wireless/ath/ath.h  | 6 ++++++
 drivers/net/wireless/ath/main.c | 7 +++++++
 2 files changed, 13 insertions(+)

diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
index da7a7c8..f3f2784 100644
--- a/drivers/net/wireless/ath/ath.h
+++ b/drivers/net/wireless/ath/ath.h
@@ -327,4 +327,10 @@ static inline const char *ath_opmode_to_string(enum nl80211_iftype opmode)
 }
 #endif
 
+extern const char *ath_bus_type_strings[];
+static inline const char *ath_bus_type_to_string(enum ath_bus_type bustype)
+{
+	return ath_bus_type_strings[bustype];
+}
+
 #endif /* ATH_H */
diff --git a/drivers/net/wireless/ath/main.c b/drivers/net/wireless/ath/main.c
index 338d723..89f4b05 100644
--- a/drivers/net/wireless/ath/main.c
+++ b/drivers/net/wireless/ath/main.c
@@ -90,3 +90,10 @@ void ath_printk(const char *level, const struct ath_common* common,
 	va_end(args);
 }
 EXPORT_SYMBOL(ath_printk);
+
+const char *ath_bus_type_strings[] = {
+	[ATH_PCI] = "pci",
+	[ATH_AHB] = "ahb",
+	[ATH_USB] = "usb",
+};
+EXPORT_SYMBOL(ath_bus_type_strings);
-- 
2.9.3

[ath9k-devel] [PATCH v5 3/3] ath9k: parse the device configuration from an OF node

[Martin Blumenstingl]

This allows configuring ath9k based PCI devices using devicetree.
There is some out-of-tree code to "convert devicetree to
ath9k_platform_data" (for example in OpenWrt and LEDE) which becomes
obsolete with this patch.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/net/wireless/ath/ath9k/init.c | 51 +++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index cfa3fe8..c123145 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -20,6 +20,8 @@
 #include <linux/slab.h>
 #include <linux/ath9k_platform.h>
 #include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_net.h>
 #include <linux/relay.h>
 #include <net/ieee80211_radiotap.h>
 
@@ -555,6 +557,51 @@ static int ath9k_init_platform(struct ath_softc *sc)
 	return 0;
 }
 
+static int ath9k_of_init(struct ath_softc *sc)
+{
+	struct device_node *np = sc->dev->of_node;
+	struct ath_hw *ah = sc->sc_ah;
+	struct ath_common *common = ath9k_hw_common(ah);
+	enum ath_bus_type bus_type = common->bus_ops->ath_bus_type;
+	const char *mac;
+	char eeprom_name[100];
+	int ret;
+
+	if (!of_device_is_available(np))
+		return 0;
+
+	ath_dbg(common, CONFIG, "parsing configuration from OF node\n");
+
+	if (of_property_read_bool(np, "qca,clk-25mhz"))
+		ah->is_clk_25mhz = true;
+
+	if (of_property_read_bool(np, "qca,disable-2ghz"))
+		ah->disable_2ghz = true;
+
+	if (of_property_read_bool(np, "qca,disable-5ghz"))
+		ah->disable_5ghz = true;
+
+	if (of_property_read_bool(np, "qca,no-eeprom")) {
+		/* ath9k-eeprom-<bus>-<id>.bin */
+		scnprintf(eeprom_name, sizeof(eeprom_name),
+			  "ath9k-eeprom-%s-%s.bin",
+			  ath_bus_type_to_string(bus_type), dev_name(ah->dev));
+
+		ret = ath9k_eeprom_request(sc, eeprom_name);
+		if (ret)
+			return ret;
+	}
+
+	mac = of_get_mac_address(np);
+	if (mac)
+		ether_addr_copy(common->macaddr, mac);
+
+	ah->ah_flags &= ~AH_USE_EEPROM;
+	ah->ah_flags |= AH_NO_EEP_SWAP;
+
+	return 0;
+}
+
 static int ath9k_init_softc(u16 devid, struct ath_softc *sc,
 			    const struct ath_bus_ops *bus_ops)
 {
@@ -611,6 +658,10 @@ static int ath9k_init_softc(u16 devid, struct ath_softc *sc,
 	if (ret)
 		return ret;
 
+	ret = ath9k_of_init(sc);
+	if (ret)
+		return ret;
+
 	if (ath9k_led_active_high != -1)
 		ah->config.led_active_high = ath9k_led_active_high == 1;
 
-- 
2.9.3

[ath9k-devel] [PATCH v6 0/3] add devicetree support to ath9k

[Martin Blumenstingl]

This series adds support for configuring ath9k based devices via
devicetree. This was tested on PCI(e) based devices. This should work
for AHB based devices as well as soon as the ath79 platform is ready
to populate the ath9k wmac via devicetree.

Changes since v5:
- updated the example in the documentation (keeping it at a bare
  minimum: removed the PCI bridge, use a better real-world example with
  less-confusing device/fn numbers, added the actual size of the config
  space to the reg property)


Martin Blumenstingl (3):
  Documentation: dt: net: add ath9k wireless device binding
  ath9k: add a helper to get the string representation of ath_bus_type
  ath9k: parse the device configuration from an OF node

 .../devicetree/bindings/net/wireless/qca,ath9k.txt | 39 +++++++++++++++++
 drivers/net/wireless/ath/ath.h                     |  6 +++
 drivers/net/wireless/ath/ath9k/init.c              | 51 ++++++++++++++++++++++
 drivers/net/wireless/ath/main.c                    |  7 +++
 4 files changed, 103 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt

-- 
2.9.3

[ath9k-devel] [PATCH v6 1/3] Documentation: dt: net: add ath9k wireless device binding

[Martin Blumenstingl]

Add documentation how devicetree can be used to configure ath9k based
devices.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 .../devicetree/bindings/net/wireless/qca,ath9k.txt | 39 ++++++++++++++++++++++
 1 file changed, 39 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt

diff --git a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
new file mode 100644
index 0000000..77b9202
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
@@ -0,0 +1,39 @@
+* Qualcomm Atheros ath9k wireless devices
+
+This node provides properties for configuring the ath9k wireless device. The
+node is expected to be specified as a child node of the PCI controller to
+which the wireless chip is connected.
+
+Required properties:
+- compatible: For PCI and PCIe devices this should be an identifier following
+		the format as defined in "PCI Bus Binding to Open Firmware"
+		Revision 2.1. One of the possible formats is "pciVVVV,DDDD"
+		where VVVV is the PCI vendor ID and DDDD is PCI device ID.
+
+Optional properties:
+- reg: Address and length of the register set for the device.
+- qca,clk-25mhz: Defines that a 25MHz clock is used
+- qca,no-eeprom: Indicates that there is no physical EEPROM connected to the
+			ath9k wireless chip (in this case the calibration /
+			EEPROM data will be loaded from userspace using the
+			kernel firmware loader).
+- qca,disable-2ghz: Overrides the settings from the EEPROM and disables the
+			2.4GHz band. Setting this property is only needed
+			when the RF circuit does not support the 2.4GHz band
+			while it is enabled nevertheless in the EEPROM.
+- qca,disable-5ghz: Overrides the settings from the EEPROM and disables the
+			5GHz band. Setting this property is only needed when
+			the RF circuit does not support the 5GHz band while
+			it is enabled nevertheless in the EEPROM.
+- mac-address: See ethernet.txt in the parent directory
+- local-mac-address: See ethernet.txt in the parent directory
+
+
+In this example, the node is defined as child node of the PCI controller:
+&pci0 {
+	ath9k at 168c,002d {
+		compatible = "pci168c,002d";
+		reg = <0x7000 0 0 0 0x1000>;
+		qca,disable-5ghz;
+	};
+};
-- 
2.9.3

[ath9k-devel] [PATCH v6 1/3] Documentation: dt: net: add ath9k wireless device binding

[Oleksij Rempel]

Hallo all,

if it is not too late i would add my two cents :)


Am 06.09.2016 um 23:46 schrieb Martin Blumenstingl:
> Add documentation how devicetree can be used to configure ath9k based
> devices.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>  .../devicetree/bindings/net/wireless/qca,ath9k.txt | 39 ++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
> new file mode 100644
> index 0000000..77b9202
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
> @@ -0,0 +1,39 @@
> +* Qualcomm Atheros ath9k wireless devices
> +
> +This node provides properties for configuring the ath9k wireless device. The
> +node is expected to be specified as a child node of the PCI controller to
> +which the wireless chip is connected.
> +
> +Required properties:
> +- compatible: For PCI and PCIe devices this should be an identifier following
> +		the format as defined in "PCI Bus Binding to Open Firmware"
> +		Revision 2.1. One of the possible formats is "pciVVVV,DDDD"
> +		where VVVV is the PCI vendor ID and DDDD is PCI device ID.
> +
> +Optional properties:
> +- reg: Address and length of the register set for the device.
> +- qca,clk-25mhz: Defines that a 25MHz clock is used

Some SoCs even Atheros WiSoCs use WiFi clock for System Clock. In this
case we need to use clock framework any way, so why not in this case too?
Provide dummy static clock in DT and connect it with this node?
	
        osc25m: oscillator {
                compatible = "fixed-clock";
                #clock-cells = <0>;
                clock-frequency = <25000000>;
                clock-accuracy = <30000>;
        };

        acc: clock-controller at 80040000 {
                compatible = "some-clock-controller";
                #clock-cells = <1>;
                clocks = <&osc25m>;
                reg = <0x80040000 0x204>;
        };


&pci0 {
	ath9k at 168c,002d {
		compatible = "pci168c,002d";
		reg = <0x7000 0 0 0 0x1000>;
		clocks = <&osc25m>;
		qca,disable-5ghz;
	};
};


driver will need to use clk_get and compare frequency instead of
of_property_read_bool(np, "qca,clk-25mhz"). In this case you will need
to define source clock only one time and reuse it by all affected DT
nodes. If we have 40MHz clock you will only need to change it in
fixed-clock.


> +- qca,no-eeprom: Indicates that there is no physical EEPROM connected to the
> +			ath9k wireless chip (in this case the calibration /
> +			EEPROM data will be loaded from userspace using the
> +			kernel firmware loader).
> +- qca,disable-2ghz: Overrides the settings from the EEPROM and disables the
> +			2.4GHz band. Setting this property is only needed
> +			when the RF circuit does not support the 2.4GHz band
> +			while it is enabled nevertheless in the EEPROM.
> +- qca,disable-5ghz: Overrides the settings from the EEPROM and disables the
> +			5GHz band. Setting this property is only needed when
> +			the RF circuit does not support the 5GHz band while
> +			it is enabled nevertheless in the EEPROM.

This option can be reused for every WiFi controller. Not only for qca.
So may be instead of adding vendor specific name, make it reusable for
all vendors. Instead of qca,disable-5ghz and qca,disable-2ghz make
disable-5ghz and disable-2ghz?


> +- mac-address: See ethernet.txt in the parent directory
> +- local-mac-address: See ethernet.txt in the parent directory
> +
> +
> +In this example, the node is defined as child node of the PCI controller:
> +&pci0 {
> +	ath9k at 168c,002d {
> +		compatible = "pci168c,002d";
> +		reg = <0x7000 0 0 0 0x1000>;
> +		qca,disable-5ghz;
> +	};
> +};
> 


-- 
Regards,
Oleksij

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 213 bytes
Desc: OpenPGP digital signature
Url : http://lists.ath9k.org/pipermail/ath9k-devel/attachments/20160909/4af330a2/attachment.pgp 

[ath9k-devel] [PATCH v6 1/3] Documentation: dt: net: add ath9k wireless device binding

[Martin Blumenstingl]

On Fri, Sep 9, 2016 at 9:48 AM, Oleksij Rempel <linux@rempel-privat.de> wrote:
>> +Optional properties:
>> +- reg: Address and length of the register set for the device.
>> +- qca,clk-25mhz: Defines that a 25MHz clock is used
>
> Some SoCs even Atheros WiSoCs use WiFi clock for System Clock. In this
> case we need to use clock framework any way, so why not in this case too?
> Provide dummy static clock in DT and connect it with this node?
>
>         osc25m: oscillator {
>                 compatible = "fixed-clock";
>                 #clock-cells = <0>;
>                 clock-frequency = <25000000>;
>                 clock-accuracy = <30000>;
>         };
>
>         acc: clock-controller at 80040000 {
>                 compatible = "some-clock-controller";
>                 #clock-cells = <1>;
>                 clocks = <&osc25m>;
>                 reg = <0x80040000 0x204>;
>         };
>
>
> &pci0 {
>         ath9k at 168c,002d {
>                 compatible = "pci168c,002d";
>                 reg = <0x7000 0 0 0 0x1000>;
>                 clocks = <&osc25m>;
>                 qca,disable-5ghz;
>         };
> };
>
>
> driver will need to use clk_get and compare frequency instead of
> of_property_read_bool(np, "qca,clk-25mhz"). In this case you will need
> to define source clock only one time and reuse it by all affected DT
> nodes. If we have 40MHz clock you will only need to change it in
> fixed-clock.
that does sound like a good idea, thanks for that input!
However, I will remove the property for the first version because I am
trying to get PCI(e) devices supported. OF support for AHB (WiSoC)
needs more work (in other places, like ahb.c) anyways.
But this suggestion should be remembered!

>> +- qca,no-eeprom: Indicates that there is no physical EEPROM connected to the
>> +                     ath9k wireless chip (in this case the calibration /
>> +                     EEPROM data will be loaded from userspace using the
>> +                     kernel firmware loader).
>> +- qca,disable-2ghz: Overrides the settings from the EEPROM and disables the
>> +                     2.4GHz band. Setting this property is only needed
>> +                     when the RF circuit does not support the 2.4GHz band
>> +                     while it is enabled nevertheless in the EEPROM.
>> +- qca,disable-5ghz: Overrides the settings from the EEPROM and disables the
>> +                     5GHz band. Setting this property is only needed when
>> +                     the RF circuit does not support the 5GHz band while
>> +                     it is enabled nevertheless in the EEPROM.
>
> This option can be reused for every WiFi controller. Not only for qca.
> So may be instead of adding vendor specific name, make it reusable for
> all vendors. Instead of qca,disable-5ghz and qca,disable-2ghz make
> disable-5ghz and disable-2ghz?
I am personally fine with anything that fits best into the grand
scheme of things.
There are three scenarios I can think of which may be influenced by
devicetree configuration:
a) let the driver decide automatically whether the 2.4GHz and/or 5GHz
band is/are enabled
b) explicitly disable either bands (because the driver thinks due to
whatever reason that a band is supported while in reality it isn't)
c) explicitly enable a band (for example because the driver cannot
automagically detect which band should be enabled)

for ath9k we default to a) but also allow b): the EEPROM (device
specific calibration data) contains information about which bands are
enabled (or not). But some manufacturers are shipping devices for
example with the 5G band enabled, while the actual hardware doesn't
support it.

Felix' mt76 driver for example defaults to case a) but allows
overriding (= forcefully enabling or disabling) a specific band.

If we decide how this should look like in the devicetree then I can go
ahead and implement it accordingly.


Regards,
Martin


[0] https://github.com/openwrt/mt76/blob/master/eeprom.c#L79

[ath9k-devel] [PATCH v6 1/3] Documentation: dt: net: add ath9k wireless device binding

[Oleksij Rempel]

Am 09.09.2016 um 22:57 schrieb Martin Blumenstingl:
> On Fri, Sep 9, 2016 at 9:48 AM, Oleksij Rempel <linux@rempel-privat.de> wrote:
>>> +Optional properties:
>>> +- reg: Address and length of the register set for the device.
>>> +- qca,clk-25mhz: Defines that a 25MHz clock is used
>>
>> Some SoCs even Atheros WiSoCs use WiFi clock for System Clock. In this
>> case we need to use clock framework any way, so why not in this case too?
>> Provide dummy static clock in DT and connect it with this node?
>>
>>         osc25m: oscillator {
>>                 compatible = "fixed-clock";
>>                 #clock-cells = <0>;
>>                 clock-frequency = <25000000>;
>>                 clock-accuracy = <30000>;
>>         };
>>
>>         acc: clock-controller at 80040000 {
>>                 compatible = "some-clock-controller";
>>                 #clock-cells = <1>;
>>                 clocks = <&osc25m>;
>>                 reg = <0x80040000 0x204>;
>>         };
>>
>>
>> &pci0 {
>>         ath9k at 168c,002d {
>>                 compatible = "pci168c,002d";
>>                 reg = <0x7000 0 0 0 0x1000>;
>>                 clocks = <&osc25m>;
>>                 qca,disable-5ghz;
>>         };
>> };
>>
>>
>> driver will need to use clk_get and compare frequency instead of
>> of_property_read_bool(np, "qca,clk-25mhz"). In this case you will need
>> to define source clock only one time and reuse it by all affected DT
>> nodes. If we have 40MHz clock you will only need to change it in
>> fixed-clock.
> that does sound like a good idea, thanks for that input!
> However, I will remove the property for the first version because I am
> trying to get PCI(e) devices supported. OF support for AHB (WiSoC)
> needs more work (in other places, like ahb.c) anyways.
> But this suggestion should be remembered!
> 
>>> +- qca,no-eeprom: Indicates that there is no physical EEPROM connected to the
>>> +                     ath9k wireless chip (in this case the calibration /
>>> +                     EEPROM data will be loaded from userspace using the
>>> +                     kernel firmware loader).
>>> +- qca,disable-2ghz: Overrides the settings from the EEPROM and disables the
>>> +                     2.4GHz band. Setting this property is only needed
>>> +                     when the RF circuit does not support the 2.4GHz band
>>> +                     while it is enabled nevertheless in the EEPROM.
>>> +- qca,disable-5ghz: Overrides the settings from the EEPROM and disables the
>>> +                     5GHz band. Setting this property is only needed when
>>> +                     the RF circuit does not support the 5GHz band while
>>> +                     it is enabled nevertheless in the EEPROM.
>>
>> This option can be reused for every WiFi controller. Not only for qca.
>> So may be instead of adding vendor specific name, make it reusable for
>> all vendors. Instead of qca,disable-5ghz and qca,disable-2ghz make
>> disable-5ghz and disable-2ghz?
> I am personally fine with anything that fits best into the grand
> scheme of things.
> There are three scenarios I can think of which may be influenced by
> devicetree configuration:
> a) let the driver decide automatically whether the 2.4GHz and/or 5GHz
> band is/are enabled
> b) explicitly disable either bands (because the driver thinks due to
> whatever reason that a band is supported while in reality it isn't)
> c) explicitly enable a band (for example because the driver cannot
> automagically detect which band should be enabled)
> 
> for ath9k we default to a) but also allow b): the EEPROM (device
> specific calibration data) contains information about which bands are
> enabled (or not). But some manufacturers are shipping devices for
> example with the 5G band enabled, while the actual hardware doesn't
> support it.
> 
> Felix' mt76 driver for example defaults to case a) but allows
> overriding (= forcefully enabling or disabling) a specific band.
> 
> If we decide how this should look like in the devicetree then I can go
> ahead and implement it accordingly.

To avoid possible conflict with other device-tree bindings i would
suggest this kind of naming:
ieee80211-5ghz-enalbe
ieee80211-5ghz-disable
ieee80211-2.4ghz-enalbe
ieee80211-2.4ghz-disable

i assume at some point we would get even more eeprom overrides. For
example disable/enable some modulation and so on.

we would need 80211 specific functions to read this overrides from
device-tree and/or acpi.

any feedback from DT maintainers?
-- 
Regards,
Oleksij

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 213 bytes
Desc: OpenPGP digital signature
Url : http://lists.ath9k.org/pipermail/ath9k-devel/attachments/20160913/4cb6251e/attachment.pgp 

[ath9k-devel] [PATCH v6 1/3] Documentation: dt: net: add ath9k wireless device binding

[Rob Herring]

On Fri, Sep 09, 2016 at 10:57:06PM +0200, Martin Blumenstingl wrote:
> On Fri, Sep 9, 2016 at 9:48 AM, Oleksij Rempel <linux@rempel-privat.de> wrote:
> >> +Optional properties:
> >> +- reg: Address and length of the register set for the device.
> >> +- qca,clk-25mhz: Defines that a 25MHz clock is used
> >
> > Some SoCs even Atheros WiSoCs use WiFi clock for System Clock. In this
> > case we need to use clock framework any way, so why not in this case too?
> > Provide dummy static clock in DT and connect it with this node?
> >
> >         osc25m: oscillator {
> >                 compatible = "fixed-clock";
> >                 #clock-cells = <0>;
> >                 clock-frequency = <25000000>;
> >                 clock-accuracy = <30000>;
> >         };
> >
> >         acc: clock-controller at 80040000 {
> >                 compatible = "some-clock-controller";
> >                 #clock-cells = <1>;
> >                 clocks = <&osc25m>;
> >                 reg = <0x80040000 0x204>;
> >         };
> >
> >
> > &pci0 {
> >         ath9k at 168c,002d {
> >                 compatible = "pci168c,002d";
> >                 reg = <0x7000 0 0 0 0x1000>;
> >                 clocks = <&osc25m>;
> >                 qca,disable-5ghz;
> >         };
> > };
> >
> >
> > driver will need to use clk_get and compare frequency instead of
> > of_property_read_bool(np, "qca,clk-25mhz"). In this case you will need
> > to define source clock only one time and reuse it by all affected DT
> > nodes. If we have 40MHz clock you will only need to change it in
> > fixed-clock.
> that does sound like a good idea, thanks for that input!
> However, I will remove the property for the first version because I am
> trying to get PCI(e) devices supported. OF support for AHB (WiSoC)
> needs more work (in other places, like ahb.c) anyways.
> But this suggestion should be remembered!
> 
> >> +- qca,no-eeprom: Indicates that there is no physical EEPROM connected to the
> >> +                     ath9k wireless chip (in this case the calibration /
> >> +                     EEPROM data will be loaded from userspace using the
> >> +                     kernel firmware loader).
> >> +- qca,disable-2ghz: Overrides the settings from the EEPROM and disables the
> >> +                     2.4GHz band. Setting this property is only needed
> >> +                     when the RF circuit does not support the 2.4GHz band
> >> +                     while it is enabled nevertheless in the EEPROM.
> >> +- qca,disable-5ghz: Overrides the settings from the EEPROM and disables the
> >> +                     5GHz band. Setting this property is only needed when
> >> +                     the RF circuit does not support the 5GHz band while
> >> +                     it is enabled nevertheless in the EEPROM.
> >
> > This option can be reused for every WiFi controller. Not only for qca.
> > So may be instead of adding vendor specific name, make it reusable for
> > all vendors. Instead of qca,disable-5ghz and qca,disable-2ghz make
> > disable-5ghz and disable-2ghz?

Fine, but if you do this then document in a common location.

> I am personally fine with anything that fits best into the grand
> scheme of things.
> There are three scenarios I can think of which may be influenced by
> devicetree configuration:
> a) let the driver decide automatically whether the 2.4GHz and/or 5GHz
> band is/are enabled
> b) explicitly disable either bands (because the driver thinks due to
> whatever reason that a band is supported while in reality it isn't)
> c) explicitly enable a band (for example because the driver cannot
> automagically detect which band should be enabled)
> 
> for ath9k we default to a) but also allow b): the EEPROM (device
> specific calibration data) contains information about which bands are
> enabled (or not). But some manufacturers are shipping devices for
> example with the 5G band enabled, while the actual hardware doesn't
> support it.

And you can't determine that based on different device IDs? If you can 
use vendor/device ID, then you should. If not, then this is fine.

Is it possible to have no EEPROM at all? If so, you might want to just 
put the raw EEPROM data into DT rather than a property for every field 
(I'm assuming there's more than just what you have properties for now).

Rob

[ath9k-devel] [PATCH v6 1/3] Documentation: dt: net: add ath9k wireless device binding

[Martin Blumenstingl]

On Fri, Sep 16, 2016 at 2:45 PM, Rob Herring <robh@kernel.org> wrote:
> On Fri, Sep 09, 2016 at 10:57:06PM +0200, Martin Blumenstingl wrote:
>> On Fri, Sep 9, 2016 at 9:48 AM, Oleksij Rempel <linux@rempel-privat.de> wrote:
>> >> +Optional properties:
>> >> +- reg: Address and length of the register set for the device.
>> >> +- qca,clk-25mhz: Defines that a 25MHz clock is used
>> >
>> > Some SoCs even Atheros WiSoCs use WiFi clock for System Clock. In this
>> > case we need to use clock framework any way, so why not in this case too?
>> > Provide dummy static clock in DT and connect it with this node?
>> >
>> >         osc25m: oscillator {
>> >                 compatible = "fixed-clock";
>> >                 #clock-cells = <0>;
>> >                 clock-frequency = <25000000>;
>> >                 clock-accuracy = <30000>;
>> >         };
>> >
>> >         acc: clock-controller at 80040000 {
>> >                 compatible = "some-clock-controller";
>> >                 #clock-cells = <1>;
>> >                 clocks = <&osc25m>;
>> >                 reg = <0x80040000 0x204>;
>> >         };
>> >
>> >
>> > &pci0 {
>> >         ath9k at 168c,002d {
>> >                 compatible = "pci168c,002d";
>> >                 reg = <0x7000 0 0 0 0x1000>;
>> >                 clocks = <&osc25m>;
>> >                 qca,disable-5ghz;
>> >         };
>> > };
>> >
>> >
>> > driver will need to use clk_get and compare frequency instead of
>> > of_property_read_bool(np, "qca,clk-25mhz"). In this case you will need
>> > to define source clock only one time and reuse it by all affected DT
>> > nodes. If we have 40MHz clock you will only need to change it in
>> > fixed-clock.
>> that does sound like a good idea, thanks for that input!
>> However, I will remove the property for the first version because I am
>> trying to get PCI(e) devices supported. OF support for AHB (WiSoC)
>> needs more work (in other places, like ahb.c) anyways.
>> But this suggestion should be remembered!
>>
>> >> +- qca,no-eeprom: Indicates that there is no physical EEPROM connected to the
>> >> +                     ath9k wireless chip (in this case the calibration /
>> >> +                     EEPROM data will be loaded from userspace using the
>> >> +                     kernel firmware loader).
>> >> +- qca,disable-2ghz: Overrides the settings from the EEPROM and disables the
>> >> +                     2.4GHz band. Setting this property is only needed
>> >> +                     when the RF circuit does not support the 2.4GHz band
>> >> +                     while it is enabled nevertheless in the EEPROM.
>> >> +- qca,disable-5ghz: Overrides the settings from the EEPROM and disables the
>> >> +                     5GHz band. Setting this property is only needed when
>> >> +                     the RF circuit does not support the 5GHz band while
>> >> +                     it is enabled nevertheless in the EEPROM.
>> >
>> > This option can be reused for every WiFi controller. Not only for qca.
>> > So may be instead of adding vendor specific name, make it reusable for
>> > all vendors. Instead of qca,disable-5ghz and qca,disable-2ghz make
>> > disable-5ghz and disable-2ghz?
>
> Fine, but if you do this then document in a common location.
what about Documentation/devicetree/bindings/net/wireless/ieee80211-common.txt?

>> I am personally fine with anything that fits best into the grand
>> scheme of things.
>> There are three scenarios I can think of which may be influenced by
>> devicetree configuration:
>> a) let the driver decide automatically whether the 2.4GHz and/or 5GHz
>> band is/are enabled
>> b) explicitly disable either bands (because the driver thinks due to
>> whatever reason that a band is supported while in reality it isn't)
>> c) explicitly enable a band (for example because the driver cannot
>> automagically detect which band should be enabled)
>>
>> for ath9k we default to a) but also allow b): the EEPROM (device
>> specific calibration data) contains information about which bands are
>> enabled (or not). But some manufacturers are shipping devices for
>> example with the 5G band enabled, while the actual hardware doesn't
>> support it.
>
> And you can't determine that based on different device IDs? If you can
> use vendor/device ID, then you should. If not, then this is fine.
one example is the TP-Link TW-8970 which uses a AR9381. ath9k
identifies this chip as AR9380 (probably because both are *very*
similar). The former does not support the 5G band, the latter does
(but unfortunately - even though it's not supported - the EEPROM data
on the TW-8970 indicates that 5G is supported)

> Is it possible to have no EEPROM at all? If so, you might want to just
> put the raw EEPROM data into DT rather than a property for every field
> (I'm assuming there's more than just what you have properties for now).
In theory: yes
However, most devices we are talking about are legacy ones without
devicetree support in the bootloader (in other words: hand-written
.dts files).

[ath9k-devel] [PATCH v6 1/3] Documentation: dt: net: add ath9k wireless device binding

[Rob Herring]

On Tue, Sep 06, 2016 at 11:46:21PM +0200, Martin Blumenstingl wrote:
> Add documentation how devicetree can be used to configure ath9k based
> devices.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>  .../devicetree/bindings/net/wireless/qca,ath9k.txt | 39 ++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
> new file mode 100644
> index 0000000..77b9202
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
> @@ -0,0 +1,39 @@
> +* Qualcomm Atheros ath9k wireless devices
> +
> +This node provides properties for configuring the ath9k wireless device. The
> +node is expected to be specified as a child node of the PCI controller to
> +which the wireless chip is connected.
> +
> +Required properties:
> +- compatible: For PCI and PCIe devices this should be an identifier following
> +		the format as defined in "PCI Bus Binding to Open Firmware"
> +		Revision 2.1. One of the possible formats is "pciVVVV,DDDD"
> +		where VVVV is the PCI vendor ID and DDDD is PCI device ID.
> +
> +Optional properties:
> +- reg: Address and length of the register set for the device.

reg is not optional.

> +- qca,clk-25mhz: Defines that a 25MHz clock is used
> +- qca,no-eeprom: Indicates that there is no physical EEPROM connected to the
> +			ath9k wireless chip (in this case the calibration /
> +			EEPROM data will be loaded from userspace using the
> +			kernel firmware loader).
> +- qca,disable-2ghz: Overrides the settings from the EEPROM and disables the
> +			2.4GHz band. Setting this property is only needed
> +			when the RF circuit does not support the 2.4GHz band
> +			while it is enabled nevertheless in the EEPROM.
> +- qca,disable-5ghz: Overrides the settings from the EEPROM and disables the
> +			5GHz band. Setting this property is only needed when
> +			the RF circuit does not support the 5GHz band while
> +			it is enabled nevertheless in the EEPROM.
> +- mac-address: See ethernet.txt in the parent directory
> +- local-mac-address: See ethernet.txt in the parent directory
> +
> +
> +In this example, the node is defined as child node of the PCI controller:
> +&pci0 {
> +	ath9k at 168c,002d {

unit address is not the vendor/device ID, but the reg value. So 
'@7000,0,0' I think in this case. Double check the OF PCI bus binding.


> +		compatible = "pci168c,002d";
> +		reg = <0x7000 0 0 0 0x1000>;
> +		qca,disable-5ghz;
> +	};
> +};
> -- 
> 2.9.3
> 

[ath9k-devel] [PATCH v6 2/3] ath9k: add a helper to get the string representation of ath_bus_type

[Martin Blumenstingl]

This can be used when the ath_bus_type has to be presented in a log
message or firmware filename.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/net/wireless/ath/ath.h  | 6 ++++++
 drivers/net/wireless/ath/main.c | 7 +++++++
 2 files changed, 13 insertions(+)

diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
index da7a7c8..f3f2784 100644
--- a/drivers/net/wireless/ath/ath.h
+++ b/drivers/net/wireless/ath/ath.h
@@ -327,4 +327,10 @@ static inline const char *ath_opmode_to_string(enum nl80211_iftype opmode)
 }
 #endif
 
+extern const char *ath_bus_type_strings[];
+static inline const char *ath_bus_type_to_string(enum ath_bus_type bustype)
+{
+	return ath_bus_type_strings[bustype];
+}
+
 #endif /* ATH_H */
diff --git a/drivers/net/wireless/ath/main.c b/drivers/net/wireless/ath/main.c
index 338d723..89f4b05 100644
--- a/drivers/net/wireless/ath/main.c
+++ b/drivers/net/wireless/ath/main.c
@@ -90,3 +90,10 @@ void ath_printk(const char *level, const struct ath_common* common,
 	va_end(args);
 }
 EXPORT_SYMBOL(ath_printk);
+
+const char *ath_bus_type_strings[] = {
+	[ATH_PCI] = "pci",
+	[ATH_AHB] = "ahb",
+	[ATH_USB] = "usb",
+};
+EXPORT_SYMBOL(ath_bus_type_strings);
-- 
2.9.3

[ath9k-devel] [PATCH v6 3/3] ath9k: parse the device configuration from an OF node

[Martin Blumenstingl]

This allows configuring ath9k based PCI devices using devicetree.
There is some out-of-tree code to "convert devicetree to
ath9k_platform_data" (for example in OpenWrt and LEDE) which becomes
obsolete with this patch.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/net/wireless/ath/ath9k/init.c | 51 +++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index cfa3fe8..c123145 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -20,6 +20,8 @@
 #include <linux/slab.h>
 #include <linux/ath9k_platform.h>
 #include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_net.h>
 #include <linux/relay.h>
 #include <net/ieee80211_radiotap.h>
 
@@ -555,6 +557,51 @@ static int ath9k_init_platform(struct ath_softc *sc)
 	return 0;
 }
 
+static int ath9k_of_init(struct ath_softc *sc)
+{
+	struct device_node *np = sc->dev->of_node;
+	struct ath_hw *ah = sc->sc_ah;
+	struct ath_common *common = ath9k_hw_common(ah);
+	enum ath_bus_type bus_type = common->bus_ops->ath_bus_type;
+	const char *mac;
+	char eeprom_name[100];
+	int ret;
+
+	if (!of_device_is_available(np))
+		return 0;
+
+	ath_dbg(common, CONFIG, "parsing configuration from OF node\n");
+
+	if (of_property_read_bool(np, "qca,clk-25mhz"))
+		ah->is_clk_25mhz = true;
+
+	if (of_property_read_bool(np, "qca,disable-2ghz"))
+		ah->disable_2ghz = true;
+
+	if (of_property_read_bool(np, "qca,disable-5ghz"))
+		ah->disable_5ghz = true;
+
+	if (of_property_read_bool(np, "qca,no-eeprom")) {
+		/* ath9k-eeprom-<bus>-<id>.bin */
+		scnprintf(eeprom_name, sizeof(eeprom_name),
+			  "ath9k-eeprom-%s-%s.bin",
+			  ath_bus_type_to_string(bus_type), dev_name(ah->dev));
+
+		ret = ath9k_eeprom_request(sc, eeprom_name);
+		if (ret)
+			return ret;
+	}
+
+	mac = of_get_mac_address(np);
+	if (mac)
+		ether_addr_copy(common->macaddr, mac);
+
+	ah->ah_flags &= ~AH_USE_EEPROM;
+	ah->ah_flags |= AH_NO_EEP_SWAP;
+
+	return 0;
+}
+
 static int ath9k_init_softc(u16 devid, struct ath_softc *sc,
 			    const struct ath_bus_ops *bus_ops)
 {
@@ -611,6 +658,10 @@ static int ath9k_init_softc(u16 devid, struct ath_softc *sc,
 	if (ret)
 		return ret;
 
+	ret = ath9k_of_init(sc);
+	if (ret)
+		return ret;
+
 	if (ath9k_led_active_high != -1)
 		ah->config.led_active_high = ath9k_led_active_high == 1;
 
-- 
2.9.3

[ath9k-devel] [PATCH v7 0/3] add devicetree support to ath9k

[Martin Blumenstingl]

This series adds support for configuring ath9k based devices via
devicetree. This was tested on PCI(e) based devices. This should work
for AHB based devices as well (adding more AHB specific properties may
still be needed) as soon as the ath79 platform is ready to populate the
ath9k wmac via devicetree.

Changes since v6:
- updated documentation to reflect that "reg" is a mandatory attribute
- dropped "qca,clk-25mhz" as it is only useful for AHB devices. OF
  support for AHB devices needs more work, so we can still introduce it
  later
- removed qca,disable-2ghz and qca,disable-5ghz as these will be
  introduced as generic bindings in a separate series


Changes since v5:
- updated the example in the documentation (keeping it at a bare
  minimum: removed the PCI bridge, use a better real-world example with
  less-confusing device/fn numbers, added the actual size of the config
  space to the reg property)


Martin Blumenstingl (3):
  Documentation: dt: net: add ath9k wireless device binding
  ath9k: add a helper to get the string representation of ath_bus_type
  ath9k: parse the device configuration from an OF node

 .../devicetree/bindings/net/wireless/qca,ath9k.txt | 30 ++++++++++++++++
 drivers/net/wireless/ath/ath.h                     |  6 ++++
 drivers/net/wireless/ath/ath9k/init.c              | 42 ++++++++++++++++++++++
 drivers/net/wireless/ath/main.c                    |  7 ++++
 4 files changed, 85 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt

-- 
2.10.0

[ath9k-devel] [PATCH v7 1/3] Documentation: dt: net: add ath9k wireless device binding

[Martin Blumenstingl]

Add documentation how devicetree can be used to configure ath9k based
devices.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 .../devicetree/bindings/net/wireless/qca,ath9k.txt | 30 ++++++++++++++++++++++
 1 file changed, 30 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt

diff --git a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
new file mode 100644
index 0000000..9b58ede
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
@@ -0,0 +1,30 @@
+* Qualcomm Atheros ath9k wireless devices
+
+This node provides properties for configuring the ath9k wireless device. The
+node is expected to be specified as a child node of the PCI controller to
+which the wireless chip is connected.
+
+Required properties:
+- compatible: For PCI and PCIe devices this should be an identifier following
+		the format as defined in "PCI Bus Binding to Open Firmware"
+		Revision 2.1. One of the possible formats is "pciVVVV,DDDD"
+		where VVVV is the PCI vendor ID and DDDD is PCI device ID.
+- reg: Address and length of the register set for the device.
+
+Optional properties:
+- qca,no-eeprom: Indicates that there is no physical EEPROM connected to the
+			ath9k wireless chip (in this case the calibration /
+			EEPROM data will be loaded from userspace using the
+			kernel firmware loader).
+- mac-address: See ethernet.txt in the parent directory
+- local-mac-address: See ethernet.txt in the parent directory
+
+
+In this example, the node is defined as child node of the PCI controller:
+&pci0 {
+	ath9k at 168c,002d {
+		compatible = "pci168c,002d";
+		reg = <0x7000 0 0 0 0x1000>;
+		qca,no-eeprom;
+	};
+};
-- 
2.10.0

[ath9k-devel] [v7, 1/3] Documentation: dt: net: add ath9k wireless device binding

[Kalle Valo]

Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:
> Add documentation how devicetree can be used to configure ath9k based
> devices.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

Looks good to me but missing the ack from DT maintainers, will wait for
that.

3 patches set to Deferred.

9359805 [v7,1/3] Documentation: dt: net: add ath9k wireless device binding
9359801 [v7,2/3] ath9k: add a helper to get the string representation of ath_bus_type
9359803 [v7,3/3] ath9k: parse the device configuration from an OF node

-- 
https://patchwork.kernel.org/patch/9359805/

Documentation about submitting wireless patches and checking status
from patchwork:

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

[ath9k-devel] [PATCH v7 1/3] Documentation: dt: net: add ath9k wireless device binding

[Rob Herring]

On Sun, Oct 02, 2016 at 11:47:41PM +0200, Martin Blumenstingl wrote:
> Add documentation how devicetree can be used to configure ath9k based
> devices.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>  .../devicetree/bindings/net/wireless/qca,ath9k.txt | 30 ++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
> new file mode 100644
> index 0000000..9b58ede
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
> @@ -0,0 +1,30 @@
> +* Qualcomm Atheros ath9k wireless devices
> +
> +This node provides properties for configuring the ath9k wireless device. The
> +node is expected to be specified as a child node of the PCI controller to
> +which the wireless chip is connected.
> +
> +Required properties:
> +- compatible: For PCI and PCIe devices this should be an identifier following
> +		the format as defined in "PCI Bus Binding to Open Firmware"
> +		Revision 2.1. One of the possible formats is "pciVVVV,DDDD"
> +		where VVVV is the PCI vendor ID and DDDD is PCI device ID.

Are there some known values you can document here? Like the one in the 
example.

> +- reg: Address and length of the register set for the device.
> +
> +Optional properties:
> +- qca,no-eeprom: Indicates that there is no physical EEPROM connected to the
> +			ath9k wireless chip (in this case the calibration /
> +			EEPROM data will be loaded from userspace using the
> +			kernel firmware loader).
> +- mac-address: See ethernet.txt in the parent directory
> +- local-mac-address: See ethernet.txt in the parent directory
> +
> +
> +In this example, the node is defined as child node of the PCI controller:
> +&pci0 {
> +	ath9k at 168c,002d {

wifi at ...

> +		compatible = "pci168c,002d";
> +		reg = <0x7000 0 0 0 0x1000>;
> +		qca,no-eeprom;
> +	};
> +};
> -- 
> 2.10.0
> 

[ath9k-devel] [PATCH v7 1/3] Documentation: dt: net: add ath9k wireless device binding

[Martin Blumenstingl]

On Sun, Oct 9, 2016 at 3:28 AM, Rob Herring <robh@kernel.org> wrote:
> On Sun, Oct 02, 2016 at 11:47:41PM +0200, Martin Blumenstingl wrote:
>> Add documentation how devicetree can be used to configure ath9k based
>> devices.
>>
>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>> ---
>>  .../devicetree/bindings/net/wireless/qca,ath9k.txt | 30 ++++++++++++++++++++++
>>  1 file changed, 30 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
>> new file mode 100644
>> index 0000000..9b58ede
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
>> @@ -0,0 +1,30 @@
>> +* Qualcomm Atheros ath9k wireless devices
>> +
>> +This node provides properties for configuring the ath9k wireless device. The
>> +node is expected to be specified as a child node of the PCI controller to
>> +which the wireless chip is connected.
>> +
>> +Required properties:
>> +- compatible: For PCI and PCIe devices this should be an identifier following
>> +             the format as defined in "PCI Bus Binding to Open Firmware"
>> +             Revision 2.1. One of the possible formats is "pciVVVV,DDDD"
>> +             where VVVV is the PCI vendor ID and DDDD is PCI device ID.
>
> Are there some known values you can document here? Like the one in the
> example.
what do you have in mind there?

there are lots of supported device IDs just with vendor 0x168c (Atheros):
echo $(grep 'PCI_VDEVICE(ATHEROS'
drivers/net/wireless/ath/ath9k/pci.c; grep '{
PCI_DEVICE_SUB(PCI_VENDOR_ID_ATHEROS'
drivers/net/wireless/ath/ath9k/pci.c -A1) | grep -Eo "0x[
0-9A-Z]{4}" | sort -u
Also some device IDs seem to be used by different chips, for example
0x0029 is used for both, AR9220 and AR9223 - see [0]

I could extend the documentation with something like:
vendor ID is usually 168c (Atheros) and device ID is one of the
following, depending on the actual chipset:
- 002d: AR9227 (PCI variant of AR9287)
- 002e: AR9287 (PCI-e)
- ...

>> +- reg: Address and length of the register set for the device.
>> +
>> +Optional properties:
>> +- qca,no-eeprom: Indicates that there is no physical EEPROM connected to the
>> +                     ath9k wireless chip (in this case the calibration /
>> +                     EEPROM data will be loaded from userspace using the
>> +                     kernel firmware loader).
>> +- mac-address: See ethernet.txt in the parent directory
>> +- local-mac-address: See ethernet.txt in the parent directory
>> +
>> +
>> +In this example, the node is defined as child node of the PCI controller:
>> +&pci0 {
>> +     ath9k at 168c,002d {
>
> wifi at ...
will fix that, thanks for the hint


[0] https://wireless.wiki.kernel.org/en/users/drivers/ath9k/devices

[ath9k-devel] [PATCH v7 2/3] ath9k: add a helper to get the string representation of ath_bus_type

[Martin Blumenstingl]

This can be used when the ath_bus_type has to be presented in a log
message or firmware filename.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/net/wireless/ath/ath.h  | 6 ++++++
 drivers/net/wireless/ath/main.c | 7 +++++++
 2 files changed, 13 insertions(+)

diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
index da7a7c8..f3f2784 100644
--- a/drivers/net/wireless/ath/ath.h
+++ b/drivers/net/wireless/ath/ath.h
@@ -327,4 +327,10 @@ static inline const char *ath_opmode_to_string(enum nl80211_iftype opmode)
 }
 #endif
 
+extern const char *ath_bus_type_strings[];
+static inline const char *ath_bus_type_to_string(enum ath_bus_type bustype)
+{
+	return ath_bus_type_strings[bustype];
+}
+
 #endif /* ATH_H */
diff --git a/drivers/net/wireless/ath/main.c b/drivers/net/wireless/ath/main.c
index 338d723..89f4b05 100644
--- a/drivers/net/wireless/ath/main.c
+++ b/drivers/net/wireless/ath/main.c
@@ -90,3 +90,10 @@ void ath_printk(const char *level, const struct ath_common* common,
 	va_end(args);
 }
 EXPORT_SYMBOL(ath_printk);
+
+const char *ath_bus_type_strings[] = {
+	[ATH_PCI] = "pci",
+	[ATH_AHB] = "ahb",
+	[ATH_USB] = "usb",
+};
+EXPORT_SYMBOL(ath_bus_type_strings);
-- 
2.10.0

[ath9k-devel] [PATCH v7 3/3] ath9k: parse the device configuration from an OF node

[Martin Blumenstingl]

This allows setting the MAC address and specifying that the firmware
will be requested from userspace (because there might not be a hardware
EEPROM connected to the chip) for ath9k based PCI devices using
the device tree.

There is some out-of-tree code to "convert devicetree to
ath9k_platform_data" (for example in OpenWrt and LEDE) which becomes
obsolete with this patch.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/net/wireless/ath/ath9k/init.c | 42 +++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index cfa3fe8..b7c8ff9 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -20,6 +20,8 @@
 #include <linux/slab.h>
 #include <linux/ath9k_platform.h>
 #include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_net.h>
 #include <linux/relay.h>
 #include <net/ieee80211_radiotap.h>
 
@@ -555,6 +557,42 @@ static int ath9k_init_platform(struct ath_softc *sc)
 	return 0;
 }
 
+static int ath9k_of_init(struct ath_softc *sc)
+{
+	struct device_node *np = sc->dev->of_node;
+	struct ath_hw *ah = sc->sc_ah;
+	struct ath_common *common = ath9k_hw_common(ah);
+	enum ath_bus_type bus_type = common->bus_ops->ath_bus_type;
+	const char *mac;
+	char eeprom_name[100];
+	int ret;
+
+	if (!of_device_is_available(np))
+		return 0;
+
+	ath_dbg(common, CONFIG, "parsing configuration from OF node\n");
+
+	if (of_property_read_bool(np, "qca,no-eeprom")) {
+		/* ath9k-eeprom-<bus>-<id>.bin */
+		scnprintf(eeprom_name, sizeof(eeprom_name),
+			  "ath9k-eeprom-%s-%s.bin",
+			  ath_bus_type_to_string(bus_type), dev_name(ah->dev));
+
+		ret = ath9k_eeprom_request(sc, eeprom_name);
+		if (ret)
+			return ret;
+	}
+
+	mac = of_get_mac_address(np);
+	if (mac)
+		ether_addr_copy(common->macaddr, mac);
+
+	ah->ah_flags &= ~AH_USE_EEPROM;
+	ah->ah_flags |= AH_NO_EEP_SWAP;
+
+	return 0;
+}
+
 static int ath9k_init_softc(u16 devid, struct ath_softc *sc,
 			    const struct ath_bus_ops *bus_ops)
 {
@@ -611,6 +649,10 @@ static int ath9k_init_softc(u16 devid, struct ath_softc *sc,
 	if (ret)
 		return ret;
 
+	ret = ath9k_of_init(sc);
+	if (ret)
+		return ret;
+
 	if (ath9k_led_active_high != -1)
 		ah->config.led_active_high = ath9k_led_active_high == 1;
 
-- 
2.10.0

[ath9k-devel] [PATCH v8 0/3] add devicetree support to ath9k

[Martin Blumenstingl]

This series adds support for configuring ath9k based devices via
devicetree. This was tested on PCI(e) based devices. This should work
for AHB based devices as well (adding more AHB specific properties may
still be needed) as soon as the ath79 platform is ready to populate the
ath9k wmac via devicetree.

Changes since v7:
- renamed node in the example (use wifi at ... instead of ath9k at ...), thanks
  Rob Herring for the hint
- added the QCA PCI vendor ID and a list of PCI IDs with the corresponding
  chipsets. This documents all known PCI IDs, but some device IDs are used
  by multiple chipsets the list of chipset(s) per device ID may be
  incomplete. Also some chipsets seem to use multiple PCI device IDs, such
  as AR9485 as documented here: https://wiki.debian.org/ath9k


Changes since v6:
- updated documentation to reflect that "reg" is a mandatory attribute
- dropped "qca,clk-25mhz" as it is only useful for AHB devices. OF
  support for AHB devices needs more work, so we can still introduce it
  later
- removed qca,disable-2ghz and qca,disable-5ghz as these will be
  introduced as generic bindings in a separate series


Changes since v5:
- updated the example in the documentation (keeping it at a bare
  minimum: removed the PCI bridge, use a better real-world example with
  less-confusing device/fn numbers, added the actual size of the config
  space to the reg property)


Martin Blumenstingl (3):
  Documentation: dt: net: add ath9k wireless device binding
  ath9k: add a helper to get the string representation of ath_bus_type
  ath9k: parse the device configuration from an OF node

 .../devicetree/bindings/net/wireless/qca,ath9k.txt | 48 ++++++++++++++++++++++
 drivers/net/wireless/ath/ath.h                     |  6 +++
 drivers/net/wireless/ath/ath9k/init.c              | 42 +++++++++++++++++++
 drivers/net/wireless/ath/main.c                    |  7 ++++
 4 files changed, 103 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt

-- 
2.10.0

[ath9k-devel] [PATCH v8 1/3] Documentation: dt: net: add ath9k wireless device binding

[Martin Blumenstingl]

Add documentation how devicetree can be used to configure ath9k based
devices.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 .../devicetree/bindings/net/wireless/qca,ath9k.txt | 48 ++++++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt

diff --git a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
new file mode 100644
index 0000000..b7396c8
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
@@ -0,0 +1,48 @@
+* Qualcomm Atheros ath9k wireless devices
+
+This node provides properties for configuring the ath9k wireless device. The
+node is expected to be specified as a child node of the PCI controller to
+which the wireless chip is connected.
+
+Required properties:
+- compatible: For PCI and PCIe devices this should be an identifier following
+		the format as defined in "PCI Bus Binding to Open Firmware"
+		Revision 2.1. One of the possible formats is "pciVVVV,DDDD"
+		where VVVV is the PCI vendor ID and DDDD is PCI device ID.
+		Typically QCA's PCI vendor ID 168c is used while the PCI device
+		ID depends on the chipset - see the following (possibly
+		incomplete) list:
+			- 0023 for AR5416
+			- 0024 for AR5418
+			- 0027 for AR9160
+			- 0029 for AR9220 and AR9223
+			- 002a for AR9280 and AR9283
+			- 002b for AR9285
+			- 002c for AR2427
+			- 002d for AR9227
+			- 002e for AR9287
+			- 0030 for AR9380, AR9381 and AR9382
+			- 0032 for AR9485
+			- 0033 for AR9580 and AR9590
+			- 0034 for AR9462
+			- 0036 for AR9565
+			- 0037 for AR9485
+- reg: Address and length of the register set for the device.
+
+Optional properties:
+- qca,no-eeprom: Indicates that there is no physical EEPROM connected to the
+			ath9k wireless chip (in this case the calibration /
+			EEPROM data will be loaded from userspace using the
+			kernel firmware loader).
+- mac-address: See ethernet.txt in the parent directory
+- local-mac-address: See ethernet.txt in the parent directory
+
+
+In this example, the node is defined as child node of the PCI controller:
+&pci0 {
+	wifi at 168c,002d {
+		compatible = "pci168c,002d";
+		reg = <0x7000 0 0 0 0x1000>;
+		qca,no-eeprom;
+	};
+};
-- 
2.10.0

[ath9k-devel] [PATCH v8 1/3] Documentation: dt: net: add ath9k wireless device binding

[Rob Herring]

On Sun, Oct 16, 2016 at 10:59:05PM +0200, Martin Blumenstingl wrote:
> Add documentation how devicetree can be used to configure ath9k based
> devices.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>  .../devicetree/bindings/net/wireless/qca,ath9k.txt | 48 ++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt

Acked-by: Rob Herring <robh@kernel.org>

[ath9k-devel] [v8, 1/3] Documentation: dt: net: add ath9k wireless device binding

[Kalle Valo]

Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:
> Add documentation how devicetree can be used to configure ath9k based
> devices.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> Acked-by: Rob Herring <robh@kernel.org>

3 patches applied to ath-next branch of ath.git, thanks.

fc383ffdb91a Documentation: dt: net: add ath9k wireless device binding
b40ded2ad75c ath9k: add a helper to get the string representation of ath_bus_type
138b41253d9c ath9k: parse the device configuration from an OF node

-- 
https://patchwork.kernel.org/patch/9378317/

Documentation about submitting wireless patches and checking status
from patchwork:

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

[ath9k-devel] [PATCH v8 2/3] ath9k: add a helper to get the string representation of ath_bus_type

[Martin Blumenstingl]

This can be used when the ath_bus_type has to be presented in a log
message or firmware filename.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/net/wireless/ath/ath.h  | 6 ++++++
 drivers/net/wireless/ath/main.c | 7 +++++++
 2 files changed, 13 insertions(+)

diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
index da7a7c8..f3f2784 100644
--- a/drivers/net/wireless/ath/ath.h
+++ b/drivers/net/wireless/ath/ath.h
@@ -327,4 +327,10 @@ static inline const char *ath_opmode_to_string(enum nl80211_iftype opmode)
 }
 #endif
 
+extern const char *ath_bus_type_strings[];
+static inline const char *ath_bus_type_to_string(enum ath_bus_type bustype)
+{
+	return ath_bus_type_strings[bustype];
+}
+
 #endif /* ATH_H */
diff --git a/drivers/net/wireless/ath/main.c b/drivers/net/wireless/ath/main.c
index 338d723..89f4b05 100644
--- a/drivers/net/wireless/ath/main.c
+++ b/drivers/net/wireless/ath/main.c
@@ -90,3 +90,10 @@ void ath_printk(const char *level, const struct ath_common* common,
 	va_end(args);
 }
 EXPORT_SYMBOL(ath_printk);
+
+const char *ath_bus_type_strings[] = {
+	[ATH_PCI] = "pci",
+	[ATH_AHB] = "ahb",
+	[ATH_USB] = "usb",
+};
+EXPORT_SYMBOL(ath_bus_type_strings);
-- 
2.10.0

[ath9k-devel] [PATCH v8 3/3] ath9k: parse the device configuration from an OF node

[Martin Blumenstingl]

This allows setting the MAC address and specifying that the firmware
will be requested from userspace (because there might not be a hardware
EEPROM connected to the chip) for ath9k based PCI devices using
the device tree.

There is some out-of-tree code to "convert devicetree to
ath9k_platform_data" (for example in OpenWrt and LEDE) which becomes
obsolete with this patch.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/net/wireless/ath/ath9k/init.c | 42 +++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index cfa3fe8..b7c8ff9 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -20,6 +20,8 @@
 #include <linux/slab.h>
 #include <linux/ath9k_platform.h>
 #include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_net.h>
 #include <linux/relay.h>
 #include <net/ieee80211_radiotap.h>
 
@@ -555,6 +557,42 @@ static int ath9k_init_platform(struct ath_softc *sc)
 	return 0;
 }
 
+static int ath9k_of_init(struct ath_softc *sc)
+{
+	struct device_node *np = sc->dev->of_node;
+	struct ath_hw *ah = sc->sc_ah;
+	struct ath_common *common = ath9k_hw_common(ah);
+	enum ath_bus_type bus_type = common->bus_ops->ath_bus_type;
+	const char *mac;
+	char eeprom_name[100];
+	int ret;
+
+	if (!of_device_is_available(np))
+		return 0;
+
+	ath_dbg(common, CONFIG, "parsing configuration from OF node\n");
+
+	if (of_property_read_bool(np, "qca,no-eeprom")) {
+		/* ath9k-eeprom-<bus>-<id>.bin */
+		scnprintf(eeprom_name, sizeof(eeprom_name),
+			  "ath9k-eeprom-%s-%s.bin",
+			  ath_bus_type_to_string(bus_type), dev_name(ah->dev));
+
+		ret = ath9k_eeprom_request(sc, eeprom_name);
+		if (ret)
+			return ret;
+	}
+
+	mac = of_get_mac_address(np);
+	if (mac)
+		ether_addr_copy(common->macaddr, mac);
+
+	ah->ah_flags &= ~AH_USE_EEPROM;
+	ah->ah_flags |= AH_NO_EEP_SWAP;
+
+	return 0;
+}
+
 static int ath9k_init_softc(u16 devid, struct ath_softc *sc,
 			    const struct ath_bus_ops *bus_ops)
 {
@@ -611,6 +649,10 @@ static int ath9k_init_softc(u16 devid, struct ath_softc *sc,
 	if (ret)
 		return ret;
 
+	ret = ath9k_of_init(sc);
+	if (ret)
+		return ret;
+
 	if (ath9k_led_active_high != -1)
 		ah->config.led_active_high = ath9k_led_active_high == 1;
 
-- 
2.10.0