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

[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

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[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-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 .../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@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>;
+			qca,disable-5ghz;
+		};
+	};
+};
-- 
2.9.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [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
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [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?

Re: [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-r2nGTMty4D4@public.gmane.org> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [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-r2nGTMty4D4@public.gmane.org> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [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-r2nGTMty4D4@public.gmane.org> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [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

Re: [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

[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-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[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-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Bjørn Mork]

Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Oleksij Rempel]

[-- Attachment #1.1: Type: text/plain, Size: 827 bytes --]

Am 10.07.2016 um 02:19 schrieb Bjørn Mork:
> Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> 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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 213 bytes --]

Re: [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-yOkvZcmFvRU@public.gmane.org> wrote:
> Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[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-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 .../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@0 {
+		reg = <0 0 0 0 0>;
+		#interrupt-cells = <1>;
+		#size-cells = <2>;
+		#address-cells = <3>;
+		device_type = "pci";
+
+		ath9k@0,0 {
+			compatible = "pci168c,0030";
+			reg = <0 0 0 0 0>;
+			qca,disable-5ghz;
+		};
+	};
+};
-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [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@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@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

Re: [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-r2nGTMty4D4@public.gmane.org> wrote:
> On Sunday, August 21, 2016 4:31:03 PM CEST Martin Blumenstingl wrote:
>> +pci {
>> +       pcie@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@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@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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [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@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@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

[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-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[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-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 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

[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

[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-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 .../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@168c,002d {
+		compatible = "pci168c,002d";
+		reg = <0x7000 0 0 0 0x1000>;
+		qca,disable-5ghz;
+	};
+};
-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Oleksij Rempel]

[-- Attachment #1.1.1: Type: text/plain, Size: 3989 bytes --]

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@80040000 {
                compatible = "some-clock-controller";
                #clock-cells = <1>;
                clocks = <&osc25m>;
                reg = <0x80040000 0x204>;
        };


&pci0 {
	ath9k@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@168c,002d {
> +		compatible = "pci168c,002d";
> +		reg = <0x7000 0 0 0 0x1000>;
> +		qca,disable-5ghz;
> +	};
> +};
> 


-- 
Regards,
Oleksij


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 213 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
ath9k-devel mailing list
ath9k-devel@lists.ath9k.org
https://lists.ath9k.org/mailman/listinfo/ath9k-devel

Re: [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-YEK0n+YFykbzxQdaRaTXBw@public.gmane.org> 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@80040000 {
>                 compatible = "some-clock-controller";
>                 #clock-cells = <1>;
>                 clocks = <&osc25m>;
>                 reg = <0x80040000 0x204>;
>         };
>
>
> &pci0 {
>         ath9k@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

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

[Oleksij Rempel]

[-- Attachment #1.1.1: Type: text/plain, Size: 4629 bytes --]

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@80040000 {
>>                 compatible = "some-clock-controller";
>>                 #clock-cells = <1>;
>>                 clocks = <&osc25m>;
>>                 reg = <0x80040000 0x204>;
>>         };
>>
>>
>> &pci0 {
>>         ath9k@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


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 213 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
ath9k-devel mailing list
ath9k-devel@lists.ath9k.org
https://lists.ath9k.org/mailman/listinfo/ath9k-devel

Re: [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-YEK0n+YFykbzxQdaRaTXBw@public.gmane.org> 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@80040000 {
> >                 compatible = "some-clock-controller";
> >                 #clock-cells = <1>;
> >                 clocks = <&osc25m>;
> >                 reg = <0x80040000 0x204>;
> >         };
> >
> >
> > &pci0 {
> >         ath9k@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

Re: [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-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.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-YEK0n+YFykbzxQdaRaTXBw@public.gmane.org> 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@80040000 {
>> >                 compatible = "some-clock-controller";
>> >                 #clock-cells = <1>;
>> >                 clocks = <&osc25m>;
>> >                 reg = <0x80040000 0x204>;
>> >         };
>> >
>> >
>> > &pci0 {
>> >         ath9k@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).

Re: [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-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> ---
>  .../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@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
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[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-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[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-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 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

[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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[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-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 .../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@168c,002d {
+		compatible = "pci168c,002d";
+		reg = <0x7000 0 0 0 0x1000>;
+		qca,no-eeprom;
+	};
+};
-- 
2.10.0

Re: [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

Re: [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

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

[Kalle Valo]

Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
> Add documentation how devicetree can be used to configure ath9k based
> devices.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>

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

Re: [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-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> ---
>  .../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@168c,002d {

wifi@...

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

Re: [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@168c,002d {
>
> wifi@...
will fix that, thanks for the hint


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

[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-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[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-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[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@... instead of ath9k@...), 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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[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-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 .../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@168c,002d {
+		compatible = "pci168c,002d";
+		reg = <0x7000 0 0 0 0x1000>;
+		qca,no-eeprom;
+	};
+};
-- 
2.10.0

Re: [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-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> ---
>  .../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-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Kalle Valo]

Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
> Add documentation how devicetree can be used to configure ath9k based
> devices.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.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

[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-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[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-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 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