2016-06-23 15:13:37

by Martin Blumenstingl

[permalink] [raw]
Subject: [RFC] ath9k: add devicetree support to ath9k

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"



2016-06-23 15:13:39

by Martin Blumenstingl

[permalink] [raw]
Subject: [PATCH 2/2] Documentation: dt: net: add ath9k wireless device binding

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

Signed-off-by: Martin Blumenstingl <[email protected]>
---
.../devicetree/bindings/net/wireless/ath,ath9k.txt | 40 ++++++++++++++++++++++
1 file changed, 40 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/wireless/ath,ath9k.txt

diff --git a/Documentation/devicetree/bindings/net/wireless/ath,ath9k.txt b/Documentation/devicetree/bindings/net/wireless/ath,ath9k.txt
new file mode 100644
index 0000000..d6f5471
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/wireless/ath,ath9k.txt
@@ -0,0 +1,40 @@
+* 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 "ath,ath9k"
+
+Optional properties:
+- reg: Address and length of the register set for the device.
+- ath,gpio-mask: The GPIO mask
+- ath,gpio-val: The GPIO value
+- ath,led-pin: The GPIO number to which the LED is connected
+- ath,led-active-high: The LED is active when the GPIO is HIGH
+- ath,clk-25mhz: Defines that at 25MHz clock is used
+- ath,eeprom-name: The name of the file which contains the EEPROM data (which
+ will be loaded via request_firmware)
+- ath,check-eeprom-endianness: Allow checking the EEPROM endianness and
+ swapping of the EEPROM data if required
+- ath,disable-2ghz: Disables the 2.4GHz band, even if enabled in the EEPROM
+- ath,disable-5ghz: Disables the 5GHz band, even if enabled in the EEPROM
+
+In this example, the node is defined as child node of the PCI controller.
+
+pci {
+ pcie@0 {
+ reg = <0 0 0 0 0>;
+ #interrupt-cells = <1>;
+ #size-cells = <2>;
+ #address-cells = <3>;
+ device_type = "pci";
+
+ ath9k@0,0 {
+ reg = <0 0 0 0 0>;
+ device_type = "pci";
+ ath,disable-5ghz;
+ };
+ };
+};
--
2.9.0


2016-06-23 16:18:53

by Christian Lamparter

[permalink] [raw]
Subject: Re: [PATCH 2/2] Documentation: dt: net: add ath9k wireless device binding

On Thursday, June 23, 2016 05:13:28 PM Martin Blumenstingl wrote:
> Add documentation how devicetree can be used to configure ath9k based
> devices.
>
> Signed-off-by: Martin Blumenstingl <[email protected]>

You need to CC' the devicetree maintainers:
Mark Rutland <[email protected]>
Rob Herring <[email protected]>
[email protected]

for all patches which touch Documentation/devicetree/... .

Also, I know (from experience) that they would prefer it, if you put
the device tree binding patch at the top of the series (i.e. make it:
[PATCH 1/2] dt-bindings...). ;-)

> ---
> .../devicetree/bindings/net/wireless/ath,ath9k.txt | 40 ++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/wireless/ath,ath9k.txt
>
> diff --git a/Documentation/devicetree/bindings/net/wireless/ath,ath9k.txt b/Documentation/devicetree/bindings/net/wireless/ath,ath9k.txt
> new file mode 100644
> index 0000000..d6f5471
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/wireless/ath,ath9k.txt
> @@ -0,0 +1,40 @@
> +* 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 "ath,ath9k"
Documentation/devicetree/bindings/vendor-prefixes.txt has an entry for
Qualcomm Atheros, Inc. => qca. I would use that instead, given that this
is a new binding, so there's '"no"' legacy code to worry about.

> +
> +Optional properties:
> +- reg: Address and length of the register set for the device.
> +- ath,gpio-mask: The GPIO mask
> +- ath,gpio-val: The GPIO value
> +- ath,led-pin: The GPIO number to which the LED is connected
> +- ath,led-active-high: The LED is active when the GPIO is HIGH
> +- ath,clk-25mhz: Defines that at 25MHz clock is used
> +- ath,eeprom-name: The name of the file which contains the EEPROM data (which
> + will be loaded via request_firmware)
> +- ath,check-eeprom-endianness: Allow checking the EEPROM endianness and
> + swapping of the EEPROM data if required
> +- ath,disable-2ghz: Disables the 2.4GHz band, even if enabled in the EEPROM
> +- ath,disable-5ghz: Disables the 5GHz band, even if enabled in the EEPROM
> +
> +In this example, the node is defined as child node of the PCI controller.
> +
> +pci {
> + pcie@0 {
> + reg = <0 0 0 0 0>;
> + #interrupt-cells = <1>;
> + #size-cells = <2>;
> + #address-cells = <3>;
> + device_type = "pci";
> +
> + ath9k@0,0 {
compatible = "qca,ath9k"; ?

> + reg = <0 0 0 0 0>;
> + device_type = "pci";
> + ath,disable-5ghz;
> + };
> + };
> +};
>


2016-06-23 15:13:38

by Martin Blumenstingl

[permalink] [raw]
Subject: [PATCH 1/2] ath9k: parse the device configuration from an OF node

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 <[email protected]>
---
drivers/net/wireless/ath/ath9k/init.c | 70 +++++++++++++++++++++++++++++++++++
1 file changed, 70 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index a0f4a52..0f76137 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,70 @@ 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);
+ const char *mac, *eeprom_name;
+ int led_pin, ret;
+ u32 gpio_data;
+
+ if (!np)
+ return 0;
+
+ ath_dbg(common, CONFIG, "parsing configuration from OF node\n");
+
+ if (!of_property_read_u32(np, "ath,led-pin", &led_pin))
+ ah->led_pin = led_pin;
+
+ if (!of_property_read_u32(np, "ath,gpio-mask", &gpio_data))
+ ah->gpio_mask = gpio_data;
+
+ if (!of_property_read_u32(np, "ath,gpio-val", &gpio_data))
+ ah->gpio_val = gpio_data;
+
+ if (of_property_read_bool(np, "ath,clk-25mhz"))
+ ah->is_clk_25mhz = true;
+
+ if (!of_property_read_u32(np, "ath,gpio-mask", &gpio_data))
+ ah->gpio_mask = gpio_data;
+
+ if (!of_property_read_u32(np, "ath,gpio-val", &gpio_data))
+ ah->gpio_val = gpio_data;
+
+ if (of_property_read_bool(np, "ath,clk-25mhz"))
+ ah->is_clk_25mhz = true;
+
+ if (of_property_read_bool(np, "ath,led-active-high"))
+ ah->config.led_active_high = true;
+
+ if (of_property_read_bool(np, "ath,disable-2ghz"))
+ ah->disable_2ghz = true;
+
+ if (of_property_read_bool(np, "ath,disable-5ghz"))
+ ah->disable_5ghz = true;
+
+ if (of_property_read_bool(np, "ath,check-eeprom-endianness"))
+ ah->ah_flags &= ~AH_NO_EEP_SWAP;
+ else
+ ah->ah_flags |= AH_NO_EEP_SWAP;
+
+ if (!of_property_read_string(np, "ath,eeprom-name", &eeprom_name)) {
+ 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 +677,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


2016-06-23 15:59:16

by Christian Lamparter

[permalink] [raw]
Subject: Re: [PATCH 1/2] ath9k: parse the device configuration from an OF node

On Thursday, June 23, 2016 05:13:27 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.
>
> Signed-off-by: Martin Blumenstingl <[email protected]>
> ---
> drivers/net/wireless/ath/ath9k/init.c | 70 +++++++++++++++++++++++++++++++++++
> 1 file changed, 70 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
> index a0f4a52..0f76137 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,70 @@ 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);
> + const char *mac, *eeprom_name;
> + int led_pin, ret;
> + u32 gpio_data;
> +
> + if (!np)
> + return 0;

Can you please add a of_device_is_available check here too? So
we can skip it with the status property?

> +
> + ath_dbg(common, CONFIG, "parsing configuration from OF node\n");
> +
> + if (!of_property_read_u32(np, "ath,led-pin", &led_pin))
> + ah->led_pin = led_pin;
> +
> + if (!of_property_read_u32(np, "ath,gpio-mask", &gpio_data))
> + ah->gpio_mask = gpio_data;
> +
> + if (!of_property_read_u32(np, "ath,gpio-val", &gpio_data))
> + ah->gpio_val = gpio_data;
> +
> + if (of_property_read_bool(np, "ath,clk-25mhz"))
> + ah->is_clk_25mhz = true;
> +
> + if (!of_property_read_u32(np, "ath,gpio-mask", &gpio_data))
> + ah->gpio_mask = gpio_data;
^^
Duplicated? (see 11 lines above)

> +
> + if (!of_property_read_u32(np, "ath,gpio-val", &gpio_data))
> + ah->gpio_val = gpio_data;
^^
Duplicated?

> +
> + if (of_property_read_bool(np, "ath,clk-25mhz"))
> + ah->is_clk_25mhz = true;
^^
Duplicated?

> +
> + if (of_property_read_bool(np, "ath,led-active-high"))
> + ah->config.led_active_high = true;
> +
> + if (of_property_read_bool(np, "ath,disable-2ghz"))
> + ah->disable_2ghz = true;
> +
> + if (of_property_read_bool(np, "ath,disable-5ghz"))
> + ah->disable_5ghz = true;
> +
> + if (of_property_read_bool(np, "ath,check-eeprom-endianness"))
> + ah->ah_flags &= ~AH_NO_EEP_SWAP;
> + else
> + ah->ah_flags |= AH_NO_EEP_SWAP;
> +
> + if (!of_property_read_string(np, "ath,eeprom-name", &eeprom_name)) {
> + 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 +677,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;