| Message ID | 20160906214623.20424-2-martin.blumenstingl@googlemail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
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; > + }; > +}; >
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. Regards, Martin [0] https://github.com/openwrt/mt76/blob/master/eeprom.c#L79
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?
On Tue, Sep 06, 2016 at 11:46:21PM +0200, Martin Blumenstingl wrote: > Add documentation how devicetree can be used to configure ath9k based > devices. > > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> > --- > .../devicetree/bindings/net/wireless/qca,ath9k.txt | 39 ++++++++++++++++++++++ > 1 file changed, 39 insertions(+) > create mode 100644 Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt > > diff --git a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt > new file mode 100644 > index 0000000..77b9202 > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt > @@ -0,0 +1,39 @@ > +* Qualcomm Atheros ath9k wireless devices > + > +This node provides properties for configuring the ath9k wireless device. The > +node is expected to be specified as a child node of the PCI controller to > +which the wireless chip is connected. > + > +Required properties: > +- compatible: For PCI and PCIe devices this should be an identifier following > + the format as defined in "PCI Bus Binding to Open Firmware" > + Revision 2.1. One of the possible formats is "pciVVVV,DDDD" > + where VVVV is the PCI vendor ID and DDDD is PCI device ID. > + > +Optional properties: > +- reg: Address and length of the register set for the device. reg is not optional. > +- qca,clk-25mhz: Defines that a 25MHz clock is used > +- qca,no-eeprom: Indicates that there is no physical EEPROM connected to the > + ath9k wireless chip (in this case the calibration / > + EEPROM data will be loaded from userspace using the > + kernel firmware loader). > +- qca,disable-2ghz: Overrides the settings from the EEPROM and disables the > + 2.4GHz band. Setting this property is only needed > + when the RF circuit does not support the 2.4GHz band > + while it is enabled nevertheless in the EEPROM. > +- qca,disable-5ghz: Overrides the settings from the EEPROM and disables the > + 5GHz band. Setting this property is only needed when > + the RF circuit does not support the 5GHz band while > + it is enabled nevertheless in the EEPROM. > +- mac-address: See ethernet.txt in the parent directory > +- local-mac-address: See ethernet.txt in the parent directory > + > + > +In this example, the node is defined as child node of the PCI controller: > +&pci0 { > + ath9k@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 >
On Fri, Sep 09, 2016 at 10:57:06PM +0200, Martin Blumenstingl wrote: > On Fri, Sep 9, 2016 at 9:48 AM, Oleksij Rempel <linux@rempel-privat.de> wrote: > >> +Optional properties: > >> +- reg: Address and length of the register set for the device. > >> +- qca,clk-25mhz: Defines that a 25MHz clock is used > > > > Some SoCs even Atheros WiSoCs use WiFi clock for System Clock. In this > > case we need to use clock framework any way, so why not in this case too? > > Provide dummy static clock in DT and connect it with this node? > > > > osc25m: oscillator { > > compatible = "fixed-clock"; > > #clock-cells = <0>; > > clock-frequency = <25000000>; > > clock-accuracy = <30000>; > > }; > > > > acc: clock-controller@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
On Fri, Sep 16, 2016 at 2:45 PM, Rob Herring <robh@kernel.org> wrote: > On Fri, Sep 09, 2016 at 10:57:06PM +0200, Martin Blumenstingl wrote: >> On Fri, Sep 9, 2016 at 9:48 AM, Oleksij Rempel <linux@rempel-privat.de> wrote: >> >> +Optional properties: >> >> +- reg: Address and length of the register set for the device. >> >> +- qca,clk-25mhz: Defines that a 25MHz clock is used >> > >> > Some SoCs even Atheros WiSoCs use WiFi clock for System Clock. In this >> > case we need to use clock framework any way, so why not in this case too? >> > Provide dummy static clock in DT and connect it with this node? >> > >> > osc25m: oscillator { >> > compatible = "fixed-clock"; >> > #clock-cells = <0>; >> > clock-frequency = <25000000>; >> > clock-accuracy = <30000>; >> > }; >> > >> > acc: clock-controller@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).
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; + }; +};
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