diff mbox

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

Message ID 20160821143105.27487-2-martin.blumenstingl@googlemail.com (mailing list archive)
State Changes Requested
Delegated to: Kalle Valo
Headers show

Commit Message

Martin Blumenstingl Aug. 21, 2016, 2:31 p.m. UTC
Add documentation how devicetree can be used to configure ath9k based
devices.

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

Comments

Arnd Bergmann Aug. 22, 2016, 9:08 a.m. UTC | #1
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
Martin Blumenstingl Aug. 28, 2016, 8:51 p.m. UTC | #2
On Mon, Aug 22, 2016 at 11:08 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Sunday, August 21, 2016 4:31:03 PM CEST Martin Blumenstingl wrote:
>> +pci {
>> +       pcie@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
Arnd Bergmann Aug. 29, 2016, 12:12 p.m. UTC | #3
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
diff mbox

Patch

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