[RFC] ath9k: add devicetree support to ath9k

[RFC] 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"

[PATCH 1/2] 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 | 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

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

[Christian Lamparter]

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 <martin.blumenstingl@googlemail.com>
> ---
>  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;

[PATCH 2/2] 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/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

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

[Christian Lamparter]

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 <martin.blumenstingl@googlemail.com>

You need to CC' the devicetree maintainers:
Mark Rutland <mark.rutland@arm.com>
Rob Herring <robh+dt@kernel.org>
devicetree@vger.kernel.org

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;
> +		};
> +	};
> +};
>